From f53ad03ed132e01f1dbf84a8db9e09e5f5364207 Mon Sep 17 00:00:00 2001 From: interacsean Date: Thu, 27 Aug 2026 10:53:00 +1000 Subject: [PATCH 1/2] fix(auth): end sessions the server has rejected MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Raises @tailor-platform/auth-public-client from ^0.5.1 to ^0.6.0. When a grant expired or was revoked, 0.5.x left isAuthenticated true and reattached the dead token to every request, stranding apps in a permanent gateway-unauthorized loop that only a manual IndexedDB clear recovered from (planning#1495, auth-public-client#113). 0.6.0 acts on any token-endpoint rejection and emits logout + auth_state_changed, which is already what AuthProvider reacts to — so the fix lands without a source change here. Transient failures (5xx, timeout, network) still leave the session intact. Consumers who detected dead sessions by catching Error("No valid access token") lose that signal: fetch and getAuthHeaders now throw the underlying error. Called out in the changeset. - add a regression test covering the 0.6.0 teardown event sequence (logout followed by auth_state_changed) yielding exactly one login - document session-expiry behaviour in the authentication guide Co-Authored-By: Claude Opus 5 --- .changeset/hungry-donkeys-repeat.md | 9 ++++ docs/concepts/authentication.md | 37 +++++++++++-- packages/core/package.json | 2 +- .../core/src/contexts/auth-context.test.tsx | 53 +++++++++++++++++++ pnpm-lock.yaml | 16 +++--- 5 files changed, 103 insertions(+), 14 deletions(-) create mode 100644 .changeset/hungry-donkeys-repeat.md diff --git a/.changeset/hungry-donkeys-repeat.md b/.changeset/hungry-donkeys-repeat.md new file mode 100644 index 00000000..b883af61 --- /dev/null +++ b/.changeset/hungry-donkeys-repeat.md @@ -0,0 +1,9 @@ +--- +"@tailor-platform/app-shell": patch +--- + +Fix sessions hanging permanently after the server rejects the grant, by raising `@tailor-platform/auth-public-client` to `^0.6.0`. + +When a refresh token expired or was revoked, the auth client kept `isAuthenticated` true and reattached the dead token to every request, so apps sat in a permanent `{"errors":[{"message":"unauthorized","type":"Gateway"}]}` loop that only a manual IndexedDB clear recovered from. Any token-endpoint rejection now ends the session (`use_dpop_nonce` excepted), emitting `logout` and `auth_state_changed`. `AuthProvider` responds as it already does for a signed-out user: `guardComponent` renders, and with `autoLogin` the app redirects to sign-in. Transient failures still leave the session intact — a 5xx, a timeout, and a network failure are unchanged. + +One behaviour change worth checking even though this is a patch: `fetch` and `getAuthHeaders` no longer throw `Error("No valid access token")` when a refresh is rejected — they throw the underlying error. If your app detects dead sessions by matching that message, it has stopped detecting them; listen for `logout` / `auth_state_changed` instead, which fire on exactly that condition. Grepping the installed package for the string will not tell you whether you are affected, because the throw still exists for the genuinely-no-token case — check your own error handling. diff --git a/docs/concepts/authentication.md b/docs/concepts/authentication.md index c1ce3a71..00c73c2a 100644 --- a/docs/concepts/authentication.md +++ b/docs/concepts/authentication.md @@ -127,7 +127,7 @@ const authClient = createAuthClient({ ### Using `authClient.fetch` with a GraphQL Client -Pass `authClient.fetch` directly to your GraphQL client (e.g., urql). It transparently handles DPoP proof generation and token refresh on every request: +Pass `authClient.fetch` directly to your GraphQL client (e.g., urql). It transparently handles DPoP proof generation and token refresh on every request. If the server rejects the grant outright, it ends the session rather than replaying a dead token — see [Session expiry](#session-expiry): ```tsx import { createAuthClient, AuthProvider } from "@tailor-platform/app-shell"; @@ -204,10 +204,10 @@ function ChatScreen() { ### `EnhancedAuthClient` Methods -| Method / Property | Type | Description | -| ----------------- | -------------- | ------------------------------------------------------------------------- | -| `getAppUri()` | `() => string` | Returns the `appUri` used to create this client | -| `fetch` | `typeof fetch` | Authenticated fetch with built-in DPoP proof generation and token refresh | +| Method / Property | Type | Description | +| ----------------- | -------------- | ---------------------------------------------------------------------------------------------------------------- | +| `getAppUri()` | `() => string` | Returns the `appUri` used to create this client | +| `fetch` | `typeof fetch` | Authenticated fetch with built-in DPoP proof generation, token refresh, and session teardown on a rejected grant | ## `AuthProvider` Props @@ -224,6 +224,33 @@ The authentication provider works seamlessly with AppShell's data layer, automat - OAuth2 token management - Authenticated fetch with DPoP proof generation - Session persistence and token refresh +- Session teardown when the server rejects the grant - Automatic redirects for protected routes (via `autoLogin`) OAuth callback parameters (`code`, `state`) are automatically cleaned from the URL after a successful login — no dedicated callback page is needed. + +## Session expiry + +Access tokens are refreshed transparently, so a session normally outlives individual token lifetimes without the app doing anything. + +When a refresh cannot succeed — the refresh token has expired, been revoked, or is otherwise rejected by the token endpoint — the auth client ends the session instead of retrying indefinitely. It clears stored tokens, sets `isAuthenticated` to `false`, and emits `logout` followed by `auth_state_changed`. + +What the user sees follows from the props you already pass: + +- With `autoLogin`, `AuthProvider` redirects to sign-in. +- With `guardComponent`, the guard renders in place of your app. +- With neither, `useAuth().isAuthenticated` flips to `false` and your own UI decides. + +Transient failures are treated differently and deliberately do not end the session: a 5xx, a request timeout, and a network failure all leave it intact so a brief outage does not sign users out. + +To react to teardown yourself — clearing app-level caches, for instance — subscribe to the client's events: + +```tsx +useEffect(() => { + return authClient.addEventListener((event) => { + if (event.type === "logout") { + clearAppCaches(); + } + }); +}, [authClient]); +``` diff --git a/packages/core/package.json b/packages/core/package.json index b5d7c66d..ff964eb3 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -55,7 +55,7 @@ "@internationalized/date": "3.12.2", "@standard-schema/spec": "^1.1.0", "@tailor-platform/app-shell-vite-plugin": "workspace:*", - "@tailor-platform/auth-public-client": "^0.5.1", + "@tailor-platform/auth-public-client": "^0.6.0", "change-case": "^5.4.4", "class-variance-authority": "^0.7.1", "clsx": "^2.1.1", diff --git a/packages/core/src/contexts/auth-context.test.tsx b/packages/core/src/contexts/auth-context.test.tsx index f77e69b5..ba03408e 100644 --- a/packages/core/src/contexts/auth-context.test.tsx +++ b/packages/core/src/contexts/auth-context.test.tsx @@ -728,6 +728,59 @@ describe("AuthProvider", () => { }); }); + it("should trigger exactly one login for the teardown event sequence", async () => { + // auth-public-client 0.6.0 tears a server-rejected session down itself, + // emitting `logout` immediately followed by `auth_state_changed`. Only the + // latter drives auto-login, so the pair must still yield a single redirect. + let authEventListener: ((event: { type: string; data?: unknown }) => void) | undefined; + + const mockAddEventListener = vi.fn( + (listener: (event: { type: string; data?: unknown }) => void) => { + authEventListener = listener; + return () => {}; + }, + ); + + let currentState = { + isAuthenticated: true, + error: null as string | null, + isReady: true, + }; + + const mockLogin = vi.fn().mockResolvedValue(undefined); + const mockClient = createMockAuthClient(undefined, { + login: mockLogin, + addEventListener: mockAddEventListener, + getState: vi.fn(() => currentState), + }); + + render( + +
Content
+ , + ); + + await waitFor(() => { + expect(mockLogin).not.toHaveBeenCalled(); + }); + + // logoutImpl resets to ready + unauthenticated before emitting. + currentState = { + isAuthenticated: false, + error: null, + isReady: true, + }; + + act(() => { + authEventListener?.({ type: "logout", data: currentState }); + authEventListener?.({ type: "auth_state_changed", data: currentState }); + }); + + await waitFor(() => { + expect(mockLogin).toHaveBeenCalledTimes(1); + }); + }); + it("should not login when autoLogin is false", async () => { const state = { isAuthenticated: false, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 197933a2..73d9e102 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -255,8 +255,8 @@ importers: specifier: workspace:* version: link:../vite-plugin '@tailor-platform/auth-public-client': - specifier: ^0.5.1 - version: 0.5.1 + specifier: ^0.6.0 + version: 0.6.0 change-case: specifier: ^5.4.4 version: 5.4.4 @@ -291,6 +291,9 @@ importers: '@fontsource-variable/noto-sans-jp': specifier: 5.3.0 version: 5.3.0 + '@microsoft/api-extractor': + specifier: ^7.57.0 + version: 7.58.12(@types/node@25.9.5) '@tailwindcss/postcss': specifier: 'catalog:' version: 4.3.0 @@ -360,9 +363,6 @@ importers: vitest: specifier: 'catalog:' version: 4.1.10(@opentelemetry/api@1.9.1)(@types/node@25.9.5)(@vitest/coverage-v8@4.1.10)(happy-dom@20.9.0)(vite@7.3.5(@types/node@25.9.5)(jiti@2.7.0)(lightningcss@1.32.0)(tsx@4.23.1)(yaml@2.9.0)) - '@microsoft/api-extractor': - specifier: ^7.57.0 - version: 7.58.12(@types/node@25.9.5) packages/sdk-plugin: devDependencies: @@ -2325,8 +2325,8 @@ packages: '@swc/helpers@0.5.23': resolution: {integrity: sha512-5lSsMOTXURePglDfvuAQUqkGek9Hg2kksOYay2m0+XR++b2NWYL/4sWyuvVBIs8oKnJaxkdi9whaL/sqN13afw==} - '@tailor-platform/auth-public-client@0.5.1': - resolution: {integrity: sha512-7H42uyLPaxjeF/LM/cBMohR1mpjYiL3yteswJlXKpOI1HvoMOGcIfEMwZoeYa1hojAkdzI4f6LMtsDqnv1SegQ==} + '@tailor-platform/auth-public-client@0.6.0': + resolution: {integrity: sha512-4AxqQeN7RubldLMIyOLwsB7OzKv+6K4u83EW1eUppKZS0z6TgnWmiewQ3XoTa/g7tyvWqXvYP21egSQpqgo7Hg==} engines: {node: '>=18.0.0'} '@tailor-platform/function-kysely-tailordb@0.1.3': @@ -5886,7 +5886,7 @@ snapshots: dependencies: tslib: 2.8.1 - '@tailor-platform/auth-public-client@0.5.1': + '@tailor-platform/auth-public-client@0.6.0': dependencies: openid-client: 6.8.1 From 207fffb48fcead04eb788a7470f68df8ca5538f6 Mon Sep 17 00:00:00 2001 From: interacsean Date: Thu, 27 Aug 2026 12:48:05 +1000 Subject: [PATCH 2/2] fix(auth): correct transient-failure claim, harden teardown test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two issues from adversarial review of this PR. The regression test was a tautology. Asserting only the final login count passes even with the auth_state_changed filter removed, because loginInFlightRef absorbs the duplicate call — verified by injecting that exact regression and watching the test stay green. It now asserts that a `logout` event alone is inert before firing auth_state_changed, which is what makes it discriminating; the same mutation now fails it. The transient-failure guarantee was wrong. isAuthoritativeRefreshRejection keys off the response shape, not the underlying cause: oauth4webapi builds a ResponseBodyError for any 4xx carrying an `error` code, and 0.6.0 treats everything except use_dpop_nonce as authoritative. So 400 {"error":"server_error"} ends the session, as does a 5xx carrying a WWW-Authenticate header (checkOAuthBodyError runs the challenge check before the 4xx-only body parse). Only a timeout, a network failure, and a bare 5xx are actually safe. Docs and changeset now say that instead of promising blip tolerance the code does not deliver. Co-Authored-By: Claude Opus 5 --- .changeset/hungry-donkeys-repeat.md | 2 +- docs/concepts/authentication.md | 4 +++- .../core/src/contexts/auth-context.test.tsx | 19 +++++++++++++++---- 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/.changeset/hungry-donkeys-repeat.md b/.changeset/hungry-donkeys-repeat.md index b883af61..0230a4dc 100644 --- a/.changeset/hungry-donkeys-repeat.md +++ b/.changeset/hungry-donkeys-repeat.md @@ -4,6 +4,6 @@ Fix sessions hanging permanently after the server rejects the grant, by raising `@tailor-platform/auth-public-client` to `^0.6.0`. -When a refresh token expired or was revoked, the auth client kept `isAuthenticated` true and reattached the dead token to every request, so apps sat in a permanent `{"errors":[{"message":"unauthorized","type":"Gateway"}]}` loop that only a manual IndexedDB clear recovered from. Any token-endpoint rejection now ends the session (`use_dpop_nonce` excepted), emitting `logout` and `auth_state_changed`. `AuthProvider` responds as it already does for a signed-out user: `guardComponent` renders, and with `autoLogin` the app redirects to sign-in. Transient failures still leave the session intact — a 5xx, a timeout, and a network failure are unchanged. +When a refresh token expired or was revoked, the auth client kept `isAuthenticated` true and reattached the dead token to every request, so apps sat in a permanent `{"errors":[{"message":"unauthorized","type":"Gateway"}]}` loop that only a manual IndexedDB clear recovered from. Any token-endpoint rejection now ends the session (`use_dpop_nonce` excepted), emitting `logout` and `auth_state_changed`. `AuthProvider` responds as it already does for a signed-out user: `guardComponent` renders, and with `autoLogin` the app redirects to sign-in. A timeout, a network failure, and a plain 5xx still leave the session intact. Note that the boundary is the response shape rather than the underlying cause: any 4xx carrying an `error` code ends the session, so a token endpoint that reports overload as `400 {"error":"server_error"}` will sign users out. One behaviour change worth checking even though this is a patch: `fetch` and `getAuthHeaders` no longer throw `Error("No valid access token")` when a refresh is rejected — they throw the underlying error. If your app detects dead sessions by matching that message, it has stopped detecting them; listen for `logout` / `auth_state_changed` instead, which fire on exactly that condition. Grepping the installed package for the string will not tell you whether you are affected, because the throw still exists for the genuinely-no-token case — check your own error handling. diff --git a/docs/concepts/authentication.md b/docs/concepts/authentication.md index 00c73c2a..323b30ab 100644 --- a/docs/concepts/authentication.md +++ b/docs/concepts/authentication.md @@ -241,7 +241,9 @@ What the user sees follows from the props you already pass: - With `guardComponent`, the guard renders in place of your app. - With neither, `useAuth().isAuthenticated` flips to `false` and your own UI decides. -Transient failures are treated differently and deliberately do not end the session: a 5xx, a request timeout, and a network failure all leave it intact so a brief outage does not sign users out. +Not every failed refresh ends the session. A request timeout, a network failure, and a plain 5xx from the token endpoint all leave it intact, so a dropped connection does not sign users out. + +The line is drawn on the shape of the response, not on how transient the underlying cause is. Any rejection the token endpoint returns as a 4xx carrying an `error` code ends the session — the sole exception is `use_dpop_nonce`. A server that reports overload as `400 {"error":"server_error"}` or `400 {"error":"temporarily_unavailable"}` will therefore sign users out, as will a 5xx that carries a `WWW-Authenticate` header. If you operate the token endpoint, prefer a bare 5xx for conditions you want treated as retryable. To react to teardown yourself — clearing app-level caches, for instance — subscribe to the client's events: diff --git a/packages/core/src/contexts/auth-context.test.tsx b/packages/core/src/contexts/auth-context.test.tsx index ba03408e..22fb85ce 100644 --- a/packages/core/src/contexts/auth-context.test.tsx +++ b/packages/core/src/contexts/auth-context.test.tsx @@ -728,10 +728,14 @@ describe("AuthProvider", () => { }); }); - it("should trigger exactly one login for the teardown event sequence", async () => { + it("should drive auto-login from auth_state_changed only, not from logout", async () => { // auth-public-client 0.6.0 tears a server-rejected session down itself, // emitting `logout` immediately followed by `auth_state_changed`. Only the - // latter drives auto-login, so the pair must still yield a single redirect. + // latter may drive auto-login: if `logout` drove it too, the pair would + // race two redirects. Asserting `logout` alone is inert is what makes this + // test discriminating — asserting only the final count would still pass + // with the event filter removed, because loginInFlightRef would absorb + // the duplicate. let authEventListener: ((event: { type: string; data?: unknown }) => void) | undefined; const mockAddEventListener = vi.fn( @@ -764,15 +768,22 @@ describe("AuthProvider", () => { expect(mockLogin).not.toHaveBeenCalled(); }); - // logoutImpl resets to ready + unauthenticated before emitting. + // logoutImpl resets to ready + unauthenticated before it emits anything, + // so the state is already login-eligible when `logout` arrives. currentState = { isAuthenticated: false, error: null, isReady: true, }; - act(() => { + await act(async () => { authEventListener?.({ type: "logout", data: currentState }); + }); + + // `logout` must not be what triggers the redirect. + expect(mockLogin).not.toHaveBeenCalled(); + + await act(async () => { authEventListener?.({ type: "auth_state_changed", data: currentState }); });