diff --git a/apps/backend/internal/backendapp/middleware.go b/apps/backend/internal/backendapp/middleware.go index 6b7c7bc363..55c6194fdb 100644 --- a/apps/backend/internal/backendapp/middleware.go +++ b/apps/backend/internal/backendapp/middleware.go @@ -38,6 +38,7 @@ func corsMiddleware() gin.HandlerFunc { } c.Header("Access-Control-Allow-Methods", "GET, POST, PATCH, PUT, DELETE, OPTIONS") c.Header("Access-Control-Allow-Headers", "Origin, Content-Type, Authorization, X-Kandev-Interim-Settings-Interlock, Upgrade, Connection, Sec-WebSocket-Key, Sec-WebSocket-Version, Sec-WebSocket-Protocol") + c.Header("Access-Control-Expose-Headers", "WWW-Authenticate") if c.Request.Method == "OPTIONS" { c.AbortWithStatus(http.StatusNoContent) diff --git a/apps/backend/internal/backendapp/middleware_test.go b/apps/backend/internal/backendapp/middleware_test.go index dc309b1f11..27a085d26d 100644 --- a/apps/backend/internal/backendapp/middleware_test.go +++ b/apps/backend/internal/backendapp/middleware_test.go @@ -31,6 +31,32 @@ func TestCORSMiddlewareEchoesOriginForCredentialedRequests(t *testing.T) { } } +func TestCORSMiddlewareExposesAuthenticationChallengeHeader(t *testing.T) { + gin.SetMode(gin.TestMode) + router := gin.New() + router.Use(corsMiddleware()) + router.GET("/protected", func(c *gin.Context) { + c.Header("WWW-Authenticate", "Bearer") + c.Status(http.StatusUnauthorized) + }) + + req := httptest.NewRequest(http.MethodGet, "/protected", nil) + req.Host = "localhost:38429" + req.Header.Set("Origin", "http://localhost:37429") + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusUnauthorized { + t.Fatalf("status = %d, want %d", rec.Code, http.StatusUnauthorized) + } + if got := rec.Header().Get("WWW-Authenticate"); got != "Bearer" { + t.Fatalf("WWW-Authenticate = %q, want Bearer", got) + } + if got := rec.Header().Get("Access-Control-Expose-Headers"); !strings.Contains(got, "WWW-Authenticate") { + t.Fatalf("Access-Control-Expose-Headers = %q, want WWW-Authenticate", got) + } +} + func TestCORSMiddlewareAllowsInterimSettingsInterlockHeader(t *testing.T) { gin.SetMode(gin.TestMode) router := gin.New() diff --git a/apps/web/components/github/github-connection-dialog.test.tsx b/apps/web/components/github/github-connection-dialog.test.tsx index b6f91f6bd6..19659af900 100644 --- a/apps/web/components/github/github-connection-dialog.test.tsx +++ b/apps/web/components/github/github-connection-dialog.test.tsx @@ -17,6 +17,7 @@ const changeConnectionLabel = "Change connection"; const registrationDisplayName = "Work automation"; const githubAppLabel = "GitHub App"; const saveChangesLabel = "Save changes"; +const personalAccessTokenLabel = "Personal access token"; const WORKSPACE_ID = "workspace-1"; const DESKTOP_CONNECTION_TEST_ID = "github-connection-desktop"; const taskAccess: TaskGitCredentialsState = { @@ -155,7 +156,7 @@ describe("GitHubConnectionDialog task access", () => { .getByTestId("github-task-access-option-executor") .querySelector('[role="radio"]')!; fireEvent.click(executor); - fireEvent.change(screen.getByLabelText("Personal access token"), { + fireEvent.change(screen.getByLabelText(personalAccessTokenLabel), { target: { value: "ghp_replacement" }, }); @@ -196,7 +197,7 @@ describe("GitHubConnectionDialog task access", () => { mocks.setConnection.mockRejectedValue(new Error("Token rejected")); render(); fireEvent.click(screen.getByRole("button", { name: changeConnectionLabel })); - const token = screen.getByLabelText("Personal access token") as HTMLInputElement; + const token = screen.getByLabelText(personalAccessTokenLabel) as HTMLInputElement; fireEvent.change(token, { target: { value: "ghp_retry_me" } }); fireEvent.click( screen.getByTestId("github-task-access-option-executor").querySelector('[role="radio"]')!, @@ -207,6 +208,32 @@ describe("GitHubConnectionDialog task access", () => { await screen.findByText(/Some settings were saved/); expect(token.value).toBe("ghp_retry_me"); }); + + it("keeps an invalid PAT draft in the open connection dialog", async () => { + const onSaved = vi.fn(); + mocks.setConnection.mockRejectedValue(new Error("GitHub token is invalid")); + render( + + + , + ); + + fireEvent.click(screen.getByRole("button", { name: changeConnectionLabel })); + const dialog = await screen.findByTestId(DESKTOP_CONNECTION_TEST_ID); + const token = screen.getByLabelText(personalAccessTokenLabel) as HTMLInputElement; + fireEvent.change(token, { target: { value: "ghp_invalid" } }); + fireEvent.click(screen.getByRole("button", { name: saveChangesLabel })); + + await screen.findByText("GitHub token is invalid"); + expect(token.value).toBe("ghp_invalid"); + expect(dialog.getAttribute("data-state")).toBe("open"); + expect(onSaved).not.toHaveBeenCalled(); + }); }); afterEach(() => cleanup()); @@ -216,7 +243,7 @@ describe("GitHubConnectionDialog", () => { fireEvent.click(screen.getByRole("button", { name: changeConnectionLabel })); expect(await screen.findByTestId(DESKTOP_CONNECTION_TEST_ID)).toBeTruthy(); expect(screen.getByLabelText("Connection method")).toBeTruthy(); - expect(screen.getAllByText("Personal access token")).not.toHaveLength(0); + expect(screen.getAllByText(personalAccessTokenLabel)).not.toHaveLength(0); expect(screen.getByText("GitHub CLI account")).toBeTruthy(); fireEvent.click(screen.getByText(githubAppLabel, { selector: "span" })); diff --git a/apps/web/e2e/tests/auth/auth-lifecycle.spec.ts b/apps/web/e2e/tests/auth/auth-lifecycle.spec.ts index e79be853c3..1bd109cfd8 100644 --- a/apps/web/e2e/tests/auth/auth-lifecycle.spec.ts +++ b/apps/web/e2e/tests/auth/auth-lifecycle.spec.ts @@ -50,6 +50,25 @@ test.describe.serial("opt-in authentication", () => { await context.close(); }); + test("exposes a session challenge to split-origin browsers", async ({ browser, backend }) => { + const context = await browser.newContext(); + const page = await context.newPage(); + await page.goto(`http://127.0.0.1:${backend.port}/login`); + + const challenge = await page.evaluate(async (apiBaseUrl) => { + const response = await fetch(`${apiBaseUrl}/api/v1/workspaces`, { + credentials: "include", + }); + return { + status: response.status, + challenge: response.headers.get("WWW-Authenticate"), + }; + }, backend.baseUrl); + + expect(challenge).toEqual({ status: 401, challenge: "Bearer" }); + await context.close(); + }); + test("setup wizard promotes the first admin and signs them in", async ({ browser, backend }) => { const context = await browser.newContext({ baseURL: backend.frontendUrl }); diff --git a/apps/web/e2e/tests/integrations/provider-auth-errors.spec.ts b/apps/web/e2e/tests/integrations/provider-auth-errors.spec.ts new file mode 100644 index 0000000000..66a243676c --- /dev/null +++ b/apps/web/e2e/tests/integrations/provider-auth-errors.spec.ts @@ -0,0 +1,161 @@ +import { expect, type Route } from "@playwright/test"; +import { test } from "../../fixtures/test-base"; +import { GitHubAuthSettingsPage } from "../../pages/github-auth-settings-page"; + +async function fulfillProviderUnauthorized(route: Route, error: string) { + await route.fulfill({ + status: 401, + contentType: "application/json", + body: JSON.stringify({ error }), + }); +} + +test.describe("integration provider authentication errors", () => { + test("keeps GitHub on the page and shows the search error", async ({ + testPage, + apiClient, + seedData, + }) => { + await apiClient.mockGitHubReset(); + await apiClient.mockGitHubSetAppRegistration({ + id: "provider-auth-errors-app", + display_name: "Provider auth errors", + app_id: 2026, + }); + await apiClient.mockGitHubSetWorkspaceConnection(seedData.workspaceId, { + source: "github_app_installation", + status: "active", + app_registration_id: "provider-auth-errors-app", + installation_id: 2026, + installation_account_login: "workspace-github", + installation_account_type: "Organization", + }); + await apiClient.mockGitHubSetPersonalConnection(seedData.workspaceId, { + login: "personal-github", + status: "active", + github_user_id: 101, + access_expires_at: "2030-01-01T00:00:00Z", + }); + await testPage.route("**/api/v1/github/user/prs**", (route) => + fulfillProviderUnauthorized(route, "GitHub credentials expired"), + ); + + await testPage.goto("/github"); + + await expect(testPage).toHaveURL(/\/github$/); + await expect(testPage.getByText("GitHub credentials expired", { exact: true })).toBeVisible(); + }); + + test("keeps GitLab on the page and shows the search error", async ({ + testPage, + apiClient, + seedData, + }) => { + await apiClient.configureGitLab(seedData.workspaceId); + await apiClient.mockGitLabSetUser(seedData.workspaceId, "gitlab-user"); + await testPage.route("**/api/v1/gitlab/user/mrs**", (route) => + fulfillProviderUnauthorized(route, "GitLab credentials expired"), + ); + + await testPage.goto("/gitlab"); + + await expect(testPage).toHaveURL(/\/gitlab$/); + await expect(testPage.getByText("GitLab credentials expired", { exact: true })).toBeVisible(); + }); + + test("keeps Jira on the page and shows the authentication error", async ({ + testPage, + apiClient, + seedData, + }) => { + await apiClient.setJiraConfig({ + siteUrl: "https://acme.atlassian.net", + email: "alice@example.com", + secret: "jira-api-token", + workspaceId: seedData.workspaceId, + }); + await apiClient.waitForIntegrationAuthHealthy("jira", { workspaceId: seedData.workspaceId }); + await testPage.route("**/api/v1/jira/tickets**", (route) => + fulfillProviderUnauthorized(route, "jira api: status 401: credentials expired"), + ); + + await testPage.goto("/jira"); + + await expect(testPage).toHaveURL(/\/jira$/); + await expect( + testPage.getByRole("heading", { name: "Jira authentication required" }), + ).toBeVisible(); + }); + + test("keeps Linear on the page and shows the authentication error", async ({ + testPage, + apiClient, + seedData, + }) => { + await apiClient.setLinearConfig({ + secret: "lin_api_expired", + workspaceId: seedData.workspaceId, + }); + await apiClient.waitForIntegrationAuthHealthy("linear", { + workspaceId: seedData.workspaceId, + }); + await testPage.route("**/api/v1/linear/issues**", (route) => + fulfillProviderUnauthorized(route, "linear api: status 401: credentials expired"), + ); + + await testPage.goto("/linear"); + + await expect(testPage).toHaveURL(/\/linear$/); + await expect( + testPage.getByRole("heading", { name: "Linear authentication required" }), + ).toBeVisible(); + }); + + test("keeps an invalid GitHub PAT draft visible and the connection active", async ({ + testPage, + apiClient, + seedData, + }) => { + await apiClient.mockGitHubReset(); + await apiClient.mockGitHubSetWorkspaceConnection(seedData.workspaceId, { + source: "pat", + status: "active", + login: "existing-github", + }); + await testPage.route("**/api/v1/github/workspace-connection?*", async (route) => { + if (route.request().method() === "PUT") { + await route.fulfill({ + status: 400, + contentType: "application/json", + body: JSON.stringify({ error: "GitHub token is invalid" }), + }); + return; + } + await route.continue(); + }); + + const settings = new GitHubAuthSettingsPage(testPage); + await settings.goto(seedData.workspaceId); + await expect( + testPage.getByTestId("github-workspace-automation").getByText("existing-github", { + exact: true, + }), + ).toBeVisible(); + const surface = await settings.openConnection(); + const token = surface.getByRole("textbox", { name: "Personal access token" }); + await token.fill("ghp_invalid"); + await surface.getByRole("button", { name: "Save changes" }).click(); + + await expect(testPage).toHaveURL( + new RegExp(`/settings/workspace/${seedData.workspaceId}/integrations/github$`), + ); + await expect(testPage.getByText("GitHub token is invalid", { exact: true })).toBeVisible(); + await expect(token).toHaveValue("ghp_invalid"); + await expect(surface).toHaveAttribute("data-state", "open"); + await expect( + testPage.getByTestId("github-workspace-automation").getByText("existing-github", { + exact: true, + }), + ).toBeVisible(); + }); +}); diff --git a/apps/web/lib/api/client.test.ts b/apps/web/lib/api/client.test.ts index ce808ed668..cf72876705 100644 --- a/apps/web/lib/api/client.test.ts +++ b/apps/web/lib/api/client.test.ts @@ -1,10 +1,11 @@ import { afterEach, describe, expect, it, vi } from "vitest"; -import { fetchJson } from "./client"; +import { ApiError, fetchJson, setOnUnauthorized } from "./client"; const interlockToken = "replayable-per-boot-value"; describe("fetchJson", () => { afterEach(() => { + setOnUnauthorized(null); vi.unstubAllGlobals(); delete (window as unknown as { __KANDEV_BOOT_PAYLOAD__?: unknown }).__KANDEV_BOOT_PAYLOAD__; }); @@ -62,4 +63,48 @@ describe("fetchJson", () => { interlock: interlockToken, }); }); + + it("notifies the app for a Kandev session challenge", async () => { + const unauthorized = vi.fn(); + setOnUnauthorized(unauthorized); + vi.stubGlobal( + "fetch", + vi.fn().mockResolvedValue( + new Response(JSON.stringify({ error: "authentication required" }), { + status: 401, + headers: { + "Content-Type": "application/json", + "WWW-Authenticate": "Bearer", + }, + }), + ), + ); + + await expect( + fetchJson("/api/v1/workspaces", { baseUrl: "http://kandev.test" }), + ).rejects.toBeInstanceOf(ApiError); + expect(unauthorized).toHaveBeenCalledOnce(); + }); + + it("keeps an unchallenged provider 401 in the calling integration", async () => { + const unauthorized = vi.fn(); + setOnUnauthorized(unauthorized); + vi.stubGlobal( + "fetch", + vi.fn().mockResolvedValue( + new Response(JSON.stringify({ error: "GitHub credentials are invalid" }), { + status: 401, + headers: { "Content-Type": "application/json" }, + }), + ), + ); + + const request = fetchJson("/api/v1/github/user/prs", { baseUrl: "http://kandev.test" }); + await expect(request).rejects.toMatchObject({ + name: "ApiError", + status: 401, + message: "GitHub credentials are invalid", + }); + expect(unauthorized).not.toHaveBeenCalled(); + }); }); diff --git a/apps/web/lib/api/client.ts b/apps/web/lib/api/client.ts index 7228e032e1..d8f828964c 100644 --- a/apps/web/lib/api/client.ts +++ b/apps/web/lib/api/client.ts @@ -32,16 +32,21 @@ function resolveUrl(pathOrUrl: string, baseUrl: string) { return `${baseUrl}${pathOrUrl}`; } -// Notified whenever a request comes back 401. The auth gate (src/main.tsx) -// registers a callback that clears the store's auth slice and redirects to -// the login page, so a session that expires mid-use doesn't leave the SPA -// stuck making requests against a stale authenticated identity. +// Notified when a request carries a Kandev session challenge. The auth gate +// (src/main.tsx) registers a callback that clears the store's auth slice and +// redirects to the login page, so a session that expires mid-use doesn't leave +// the SPA stuck making requests against a stale authenticated identity. let onUnauthorized: (() => void) | null = null; export function setOnUnauthorized(cb: (() => void) | null): void { onUnauthorized = cb; } +function isKandevAuthChallenge(response: Response): boolean { + const challenge = response.headers.get("WWW-Authenticate"); + return challenge?.split(",").some((value) => /^Bearer(?:\s|$)/i.test(value.trim())) ?? false; +} + async function throwFromResponse(response: Response): Promise { let body: unknown = null; let message = `Request failed: ${response.status} ${response.statusText}`; @@ -68,7 +73,7 @@ export async function fetchJson(pathOrUrl: string, options?: ApiRequestOption headers: buildRequestHeaders(options), }); if (!response.ok) { - if (response.status === 401) onUnauthorized?.(); + if (response.status === 401 && isKandevAuthChallenge(response)) onUnauthorized?.(); await throwFromResponse(response); } if (response.status === 204) return undefined as T; diff --git a/docs/plans/integration-provider-auth-errors/plan.md b/docs/plans/integration-provider-auth-errors/plan.md new file mode 100644 index 0000000000..3f2f9cbba8 --- /dev/null +++ b/docs/plans/integration-provider-auth-errors/plan.md @@ -0,0 +1,99 @@ +--- +spec: docs/specs/auth/spec.md +related_spec: docs/specs/integrations/github-authentication.md +created: 2026-07-31 +status: done +--- + +# Implementation Plan: Integration Provider Authentication Errors + +## Overview + +The shared web API client currently treats every HTTP 401 as an expired Kandev browser session. +GitHub, GitLab, Jira, and Linear also return 401 when their own credentials expire, so the global +handler navigates to `/login` before each integration page can render its existing loading error. +Gate the global redirect on the backend's Kandev session challenge, then prove provider errors stay +local and invalid GitHub PAT saves remain visible and retryable. + +## Confirmed root cause + +- `apps/web/lib/api/client.ts` calls the global `onUnauthorized` callback for every 401 response. +- `apps/web/src/main.tsx` implements that callback with `window.location.assign("/login")`. +- Kandev's global auth middleware identifies a real session challenge with + `WWW-Authenticate: Bearer`; upstream-provider 401 responses from integration handlers do not. +- The four provider data pages (`/github`, `/gitlab`, `/jira`, `/linear`) already catch request + failures and render result errors, but the unconditional global redirect prevents users from + seeing them. +- GitHub PAT replacement is already validated before persistence and the connection dialog already + retains failed drafts; focused coverage is missing for a connection-only invalid PAT rejection. + +## Frontend + +### Shared API authentication classification + +- Update `apps/web/lib/api/client.ts` so `onUnauthorized` runs only for a 401 carrying Kandev's + `WWW-Authenticate: Bearer` session challenge. +- Continue parsing and throwing `ApiError` for every non-2xx response so provider pages and settings + forms receive the original sanitized error message. +- Add `apps/web/lib/api/client.test.ts` covering challenged and unchallenged 401 responses. + +### GitHub PAT save feedback + +- Extend `apps/web/components/github/github-connection-dialog.test.tsx` with a connection-only + invalid-PAT rejection. Assert the dialog stays open, the error toast is visible, the token draft + is retained, and `onSaved` is not called. +- No production component change is expected unless the regression exposes behavior that differs + from the amended spec. + +### Mobile contract + +The repair changes request classification only. It does not change composition, navigation, +overlays, touch behavior, scrolling, or viewport-dependent interaction. Desktop and mobile share +the same API client and provider-error state, so focused unit/component coverage plus the existing +mobile integration settings suites satisfy mobile parity; no new mobile-only composition is needed. + +## Tests + +- **What:** a Kandev session challenge still clears the stale browser identity. + **File:** `apps/web/lib/api/client.test.ts`. + **How:** mock a 401 response with `WWW-Authenticate: Bearer` and assert `onUnauthorized` runs once + before `ApiError` is thrown. +- **What:** an integration/provider 401 remains a normal request error. + **File:** `apps/web/lib/api/client.test.ts`. + **How:** mock a 401 without the challenge header, assert `onUnauthorized` is not called, and assert + the provider error body becomes the thrown `ApiError` message. +- **What:** an invalid GitHub PAT is visible and retryable. + **File:** `apps/web/components/github/github-connection-dialog.test.tsx`. + **How:** reject `setGitHubWorkspaceConnection`, save a PAT-only draft, and assert the open dialog, + retained input, error toast, and unchanged saved callback. + +## E2E Tests + +- **Scenario:** GIVEN configured GitHub, GitLab, Jira, and Linear integrations, WHEN each provider + data request returns an unchallenged 401, THEN its page remains on the provider route and displays + the loading error instead of navigating to `/login`. + **File:** `apps/web/e2e/tests/integrations/provider-auth-errors.spec.ts`. + **What to verify:** seed each integration with existing E2E helpers, intercept its primary data + endpoint with a sanitized 401 response, assert the provider-specific error text and unchanged URL. +- **Scenario:** GIVEN GitHub settings with a connected workspace, WHEN a replacement PAT save is + rejected as invalid, THEN the connection surface remains open, displays the error, and retains the + submitted token for correction. + **File:** `apps/web/e2e/tests/integrations/provider-auth-errors.spec.ts`. + **What to verify:** intercept `PUT /api/v1/github/workspace-connection`, submit the dialog, and + assert no `/login` navigation, visible error feedback, retained PAT input, and existing connection. + +## Implementation Tasks + +- [x] [Task 01: Classify session challenges and preserve PAT errors](task-01-classify-session-challenges.md) +- [x] [Task 02: Prove provider page behavior end to end](task-02-provider-auth-e2e.md) + +Execution is sequential: Task 02 depends on Task 01. No task is parallel-safe because the E2E proof +depends on the shared API-client behavior implemented by Task 01. + +## Risks And Out Of Scope + +- The challenge check must preserve redirect behavior for truly expired Kandev sessions. +- Provider error bodies remain subject to each backend handler's existing sanitization; this repair + does not redesign provider-specific copy or HTTP status mappings. +- No credential schema, persistence, health-poller, settings layout, or mobile composition changes. +- No new behavior for integration settings beyond GitHub's requested invalid-PAT save feedback. diff --git a/docs/plans/integration-provider-auth-errors/task-01-classify-session-challenges.md b/docs/plans/integration-provider-auth-errors/task-01-classify-session-challenges.md new file mode 100644 index 0000000000..0d210a2329 --- /dev/null +++ b/docs/plans/integration-provider-auth-errors/task-01-classify-session-challenges.md @@ -0,0 +1,80 @@ +--- +id: "01-classify-session-challenges" +title: "Classify session challenges and preserve PAT errors" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +spec: "../../specs/auth/spec.md" +related_spec: "../../specs/integrations/github-authentication.md" +--- + +# Task 01: Classify session challenges and preserve PAT errors + +## Intent + +Prevent third-party provider 401 responses from invoking the global Kandev login redirect while +preserving the redirect for a real Kandev session challenge. Prove that an invalid GitHub PAT save +remains visible and retryable in the existing connection dialog. + +## Acceptance + +1. `fetchJson` invokes `onUnauthorized` only for a 401 carrying Kandev's + `WWW-Authenticate: Bearer` challenge and still throws `ApiError` for both challenged and + unchallenged failures. +2. An unchallenged provider 401 preserves its parsed error message for the calling integration UI. +3. A PAT-only GitHub connection save failure leaves the dialog open, retains the submitted token, + displays the error, and does not call the saved callback. + +## TDD + +1. Add the challenged/unchallenged 401 unit cases and the PAT-only dialog rejection case. +2. Run them against current code and confirm the unchallenged-401 case fails because + `onUnauthorized` is called. +3. Implement the smallest shared-client classification change. +4. Rerun the focused tests and refactor only if needed. + +## Files Likely Touched + +- `apps/web/lib/api/client.ts` +- `apps/web/lib/api/client.test.ts` +- `apps/web/components/github/github-connection-dialog.test.tsx` + +## Dependencies + +None. + +## Parallelism + +`sequential` — Task 02 depends on this behavior. + +## Verification + +```bash +cd apps && pnpm --filter @kandev/web test -- lib/api/client.test.ts components/github/github-connection-dialog.test.tsx +``` + +## Inputs + +- `docs/specs/auth/spec.md` — session challenge versus provider failure behavior. +- `docs/specs/integrations/github-authentication.md` — invalid PAT save behavior. +- `apps/backend/internal/auth/httpmw/middleware.go` — authoritative Kandev session challenge. +- `apps/web/src/main.tsx` — global unauthorized callback. + +## Output Contract + +Verification record: + +- RED: before the challenge gate, the focused provider-401 test failed because + `onUnauthorized` was invoked for an unchallenged 401. +- Changed files: `apps/web/lib/api/client.ts`, + `apps/web/lib/api/client.test.ts`, and + `apps/web/components/github/github-connection-dialog.test.tsx`. +- Final focused result: 14 tests passed; the managed provider-auth E2E spec passed all 5 + scenarios; web typecheck and changed-file lint passed. +- Follow-up review fix: `apps/backend/internal/backendapp/middleware.go` now exposes + `WWW-Authenticate` to split-origin browsers, with a regression test in + `apps/backend/internal/backendapp/middleware_test.go`. +- Remaining risk: session redirect classification depends on the backend emitting the + `WWW-Authenticate: Bearer` challenge; same-origin and split-origin paths now both preserve + that signal. diff --git a/docs/plans/integration-provider-auth-errors/task-02-provider-auth-e2e.md b/docs/plans/integration-provider-auth-errors/task-02-provider-auth-e2e.md new file mode 100644 index 0000000000..45e00542a5 --- /dev/null +++ b/docs/plans/integration-provider-auth-errors/task-02-provider-auth-e2e.md @@ -0,0 +1,67 @@ +--- +id: "02-provider-auth-e2e" +title: "Prove provider page behavior end to end" +status: done +wave: 2 +depends_on: ["01-classify-session-challenges"] +plan: "plan.md" +spec: "../../specs/auth/spec.md" +related_spec: "../../specs/integrations/github-authentication.md" +--- + +# Task 02: Prove provider page behavior end to end + +## Intent + +Exercise the four provider data pages and GitHub PAT settings through the browser so a future +regression cannot turn an integration credential failure into a Kandev login redirect. + +## Acceptance + +1. GitHub, GitLab, Jira, and Linear pages each remain on their own route and show a provider loading + error after their primary data endpoint returns an unchallenged 401. +2. GitHub settings retain an invalid replacement PAT, keep the connection surface open, show the + backend error, and keep the existing connection active. +3. The focused production-build E2E spec passes without adding mobile-specific UI or changing page + composition. + +## TDD + +1. Add the browser scenarios using existing integration configuration helpers and route + interception for sanitized provider 401 responses. +2. Run the managed spec and correct fixture or selector assumptions exposed by the real browser + surface before interpreting a failure as a product regression. +3. Rerun after Task 01 and confirm the provider routes and error UI remain visible. + +## Files Likely Touched + +- `apps/web/e2e/tests/integrations/provider-auth-errors.spec.ts` + +## Dependencies + +Task 01 complete. + +## Parallelism + +`sequential` — validates Task 01 through real SPA routing. + +## Verification + +```bash +cd apps/web && pnpm e2e:run tests/integrations/provider-auth-errors.spec.ts +``` + +## Inputs + +- `apps/web/e2e/helpers/api-client.ts` — GitHub, GitLab, Jira, and Linear configuration helpers. +- `apps/web/e2e/pages/github-auth-settings-page.ts` — GitHub connection dialog selectors. +- The existing provider page search hooks and list error renderers under `apps/web/app/` and + `apps/web/components/{github,gitlab,jira,linear}/`. + +## Output Contract + +The first managed run exposed two test assumptions (GitHub personal identity requires an +App-backed workspace seed, and the PAT input must be selected by its textbox role); it did not +reach a product redirect failure. After correcting those assumptions, the final managed runner +passed all five scenarios. No failure artifacts remain relevant, and the remaining risk is limited +to provider-specific backend error-body wording outside the sanitized responses covered here. diff --git a/docs/specs/auth/spec.md b/docs/specs/auth/spec.md index c67a33cc4e..a92124f7a5 100644 --- a/docs/specs/auth/spec.md +++ b/docs/specs/auth/spec.md @@ -1,6 +1,7 @@ --- status: building created: 2026-07-24 +amended: 2026-07-31 owner: tbd --- @@ -61,6 +62,11 @@ the local single-user install with a login screen it never asked for. anonymous visitors), `/api/v1/features`, credential endpoints (login/setup/invite-accept), and self-authenticating webhooks (automation `X-Webhook-Secret`, office channel HMAC, plugin webhooks). +- **Session challenges are distinct from provider authentication failures.** + The browser clears its Kandev identity and opens `/login` only when a 401 is + a Kandev session challenge. A third-party integration may also return 401 + when GitHub, GitLab, Jira, or Linear rejects its own credential; that failure + remains on the current page and is rendered by the integration surface. - **Disable.** An admin can turn auth off again (unless env-forced). Ownership data is retained; everyone reaching the instance has full access again. @@ -80,6 +86,19 @@ the local single-user install with a login screen it never asked for. - A server bound to non-loopback interfaces with auth disabled logs a prominent startup warning. - Sessions/PATs of disabled users fail closed immediately. +- Third-party provider authentication failures do not clear the authenticated + Kandev user, replace the current route, or trigger a login redirect. + +## Scenarios + +- **GIVEN** an authenticated Kandev browser session, **WHEN** a protected API + request receives a Kandev session challenge, **THEN** the browser clears the + stale Kandev identity and navigates to `/login`. +- **GIVEN** an authenticated Kandev browser session and an expired or invalid + third-party integration credential, **WHEN** GitHub, GitLab, Jira, or Linear + data loading returns 401 without a Kandev session challenge, **THEN** the + browser stays on the integration route and displays the provider loading + error. ## Known v1 limits diff --git a/docs/specs/integrations/github-authentication.md b/docs/specs/integrations/github-authentication.md index 0c0e784e6e..be474e4435 100644 --- a/docs/specs/integrations/github-authentication.md +++ b/docs/specs/integrations/github-authentication.md @@ -429,6 +429,12 @@ post-signature processing failures produce `failing`; a later valid successful d `github_app_registration_in_use` with a non-secret binding count. - Changing workspace auth while a flow is open makes the stale callback fail without reverting the newer connection. +- A PAT replacement is validated against GitHub before it replaces the current workspace + connection. An invalid PAT leaves the previous connection unchanged, keeps the submitted draft + available for correction, and shows the validation error in the connection dialog. +- If a previously valid GitHub credential expires or is revoked, My GitHub stays on the current + route and renders a reconnect/loading error instead of treating GitHub's 401 as an expired Kandev + login session. ## Persistence Guarantees @@ -550,6 +556,12 @@ registration and never creates a global default. - **GIVEN** a PAT or named CLI connection draft and a changed task access mode, **WHEN** the user presses the dialog's single **Save changes** action, **THEN** both drafts are persisted, the dialog closes only after both succeed, and reopening shows the selected account and task mode. +- **GIVEN** a user enters an invalid replacement PAT, **WHEN** GitHub rejects it during **Save + changes**, **THEN** the dialog remains open, the submitted PAT remains available for correction, + an error is shown, and the previously active workspace connection is unchanged. +- **GIVEN** a configured PAT has expired or been revoked, **WHEN** the user opens My GitHub and the + provider data request returns 401, **THEN** the page remains on `/github`, shows an authentication + loading error, and does not navigate to the Kandev login screen. - **GIVEN** a changed task access mode but no PAT or CLI connection change, **WHEN** the user presses **Save changes**, **THEN** only the task policy is persisted and the selected automation identity remains unchanged.