diff --git a/.changeset/issue-79-onauthchange-bridge.md b/.changeset/issue-79-onauthchange-bridge.md new file mode 100644 index 00000000..c3d05bc4 --- /dev/null +++ b/.changeset/issue-79-onauthchange-bridge.md @@ -0,0 +1,18 @@ +--- +"@aws-blocks/bb-auth-oidc": patch +--- + +fix(bb-auth-oidc): bridge a successful client callback into auth-common's onAuthChange + +A successful client-PKCE `handleRedirectCallback()` only notified this OIDC +client's own `onAuthStateChange` listeners. Components subscribed via +`@aws-blocks/auth-common`'s `onAuthChange` — and `` — +never heard about the sign-in, so a React SPA wouldn't re-render after +completing the redirect exchange (only server-initiated sign-in updated them). + +`handleRedirectCallback()` now also calls `broadcastAuthChange(user)` on success, +and `signOut()` calls `broadcastAuthChange(null)`, firing the same-window +`blocks-auth-change` event and the cross-tab `BroadcastChannel`, so every +auth-common consumer (and other open tabs) re-render on both sign-in and sign-out. +The README documents the `onAuthChange`/`broadcastAuthChange` wiring and adds an +OIDC + React SPA example. diff --git a/packages/bb-auth-oidc/README.md b/packages/bb-auth-oidc/README.md index 6e8bcfcc..5b3a4ec4 100644 --- a/packages/bb-auth-oidc/README.md +++ b/packages/bb-auth-oidc/README.md @@ -63,6 +63,51 @@ auth.signIn('google', { redirectPath: '/auth-return' }); `redirectPath` becomes the OAuth `redirect_uri`, so it must be a page your frontend serves **and** a redirect URI registered with the provider (the stub IdP accepts any HTTPS or localhost URL, so local/sandbox needs no registration). +### Re-rendering your UI on sign-in (React SPA) + +`signIn()` and `handleRedirectCallback()` drive the OIDC exchange; to make your app re-render once it completes, subscribe to auth-state changes. Two complementary hooks are available: + +- **`auth.onAuthStateChange(cb)`** — this OIDC client's own listener. Fires for this client instance on `signIn()` kickoff, on a successful `handleRedirectCallback()`, and on `signOut()`. +- **`onAuthChange(authApi, cb)`** from `@aws-blocks/blocks/ui` — the shared `@aws-blocks/auth-common` subscription that also backs ``. It updates **across components and browser tabs**. + +A successful `handleRedirectCallback()` notifies **both**: it calls the local listeners *and* bridges into `@aws-blocks/auth-common` by calling `broadcastAuthChange(user)` for you, so `onAuthChange` consumers (and ``) re-render on client-PKCE sign-in — not just on server-initiated sign-in. You don't call `broadcastAuthChange()` yourself for sign-in; the client does. Because it fires both, a component that subscribes to **both** `auth.onAuthStateChange()` and `onAuthChange()` will have its handler invoked twice on a single client-PKCE sign-in — harmless if your handler is idempotent, but prefer one per component. + +```tsx +import { useEffect, useState } from 'react'; +import { authApi } from 'aws-blocks'; +import { onAuthChange } from '@aws-blocks/blocks/ui'; + +// Dedicated callback route (e.g. /auth-return) — completes the PKCE exchange. +export function AuthCallback() { + useEffect(() => { + authApi.getClient() + .then((auth) => auth.handleRedirectCallback()) + .catch((err) => console.error('OIDC callback failed', err)); + }, []); + return

Signing you in…

; +} + +// Any component — re-renders when auth state changes (this tab + other tabs). +export function useUser() { + const [user, setUser] = useState(null); + // onAuthChange returns an unsubscribe fn; returning it cleans up on unmount. + useEffect(() => onAuthChange(authApi, setUser), []); + return user; +} + +export function SignInButton() { + const user = useUser(); + if (user) return Hi, {user.username}; + return ( + + ); +} +``` + +`onAuthChange` invokes your callback **synchronously** with the current user (from a shared cache) for the first paint, then again whenever auth state changes, and returns an unsubscribe function — return it from `useEffect` to wire up cleanup. The same broadcast also reaches other open tabs, so signing in (or out) in one tab updates them all. In the dedicated-callback pattern above, though, `handleRedirectCallback()` *broadcasts* the sign-in rather than priming that shared cache, so a `useUser()` that mounts **after** the callback fired starts from a cache miss: it paints once as signed-out, then self-corrects when its own async `getAuthState()` resolves — an expected, transient flash. + ### Which flow to use - **Server-initiated** (`GET /aws-blocks/auth/signin/` — a link or the `` button): the backend owns the callback and sets the session cookie. This is the default for **same-origin** apps (frontend and API on one origin: local dev, single deployed origin, or the sandbox front door). diff --git a/packages/bb-auth-oidc/src/index.browser.test.ts b/packages/bb-auth-oidc/src/index.browser.test.ts index 341aa03e..912a867c 100644 --- a/packages/bb-auth-oidc/src/index.browser.test.ts +++ b/packages/bb-auth-oidc/src/index.browser.test.ts @@ -19,8 +19,39 @@ const CURRENT_PAGE = 'http://localhost:3000/dashboard'; const AUTHORIZE_URL = 'https://idp.example.com/authorize'; let navigatedTo = ''; +let reloaded = false; let store: Map; +/** + * Cross-tab payloads captured from `BroadcastChannel.postMessage` so tests can + * assert the OIDC client bridged its sign-in into `@aws-blocks/auth-common`. + * Cleared (in place — the stub closes over this exact array) on each install. + */ +const broadcasts: unknown[] = []; +let savedBroadcastChannel: unknown; + +/** + * A no-op `BroadcastChannel`. `auth-common`'s `broadcastAuthChange()` lazily + * opens a real channel via `getChannel()`; a real one is a ref'd libuv handle + * that keeps `node --test` alive and hangs the run. This records posts instead. + * + * `auth-common` caches that channel in a module-level singleton it never resets, + * so whichever test triggers the first `broadcastAuthChange()` pins the live + * instance for the rest of the run. Every stub instance posts to the SAME + * module-level `broadcasts` array (emptied per test in `installBrowserGlobals`), + * so the captured posts stay correct regardless of which describe block ran + * first — preserve that shared-array invariant if this stub is refactored. + */ +class StubBroadcastChannel { + name: string; + onmessage: ((ev: unknown) => void) | null = null; + constructor(name: string) { this.name = name; } + postMessage(msg: unknown): void { broadcasts.push(msg); } + addEventListener(): void {} + removeEventListener(): void {} + close(): void {} +} + function installBrowserGlobals(currentHref: string): void { const url = new URL(currentHref); const locationStub = { @@ -28,10 +59,22 @@ function installBrowserGlobals(currentHref: string): void { set href(v: string) { navigatedTo = v; }, origin: url.origin, pathname: url.pathname, + reload() { reloaded = true; }, }; store = new Map(); - - (globalThis as any).window = { location: locationStub }; + broadcasts.length = 0; + reloaded = false; + + // Back `window` with a real EventTarget so auth-common's + // `window.dispatchEvent(new CustomEvent('blocks-auth-change', …))` works and + // tests can listen for the same-window auth-change event. + const target = new EventTarget(); + (globalThis as any).window = { + location: locationStub, + addEventListener: target.addEventListener.bind(target), + removeEventListener: target.removeEventListener.bind(target), + dispatchEvent: target.dispatchEvent.bind(target), + }; (globalThis as any).sessionStorage = { getItem: (k: string) => store.get(k) ?? null, setItem: (k: string, v: string) => { store.set(k, v); }, @@ -40,12 +83,19 @@ function installBrowserGlobals(currentHref: string): void { // The client builds `redirect_uri` against window.location.href; some // code paths also read the global `location`. Mirror it. (globalThis as any).location = locationStub; + + // Swap in the no-op BroadcastChannel before any broadcastAuthChange() call + // caches a (real) channel instance. + savedBroadcastChannel = (globalThis as any).BroadcastChannel; + (globalThis as any).BroadcastChannel = StubBroadcastChannel; } function clearBrowserGlobals(): void { delete (globalThis as any).window; delete (globalThis as any).sessionStorage; delete (globalThis as any).location; + if (savedBroadcastChannel === undefined) delete (globalThis as any).BroadcastChannel; + else (globalThis as any).BroadcastChannel = savedBroadcastChannel; navigatedTo = ''; } @@ -418,3 +468,188 @@ describe('AuthOIDCClient.handleRedirectCallback — idempotency under double inv assert.strictEqual(exchangeCalls, 2, 'the fresh flow runs its own second exchange'); }); }); + +describe('AuthOIDCClient.handleRedirectCallback — @aws-blocks/auth-common bridge', () => { + const STATE = 'state-bridge'; + const BARE_USER = { userId: 'iss:sub', username: 'alice', email: 'alice@example.invalid', provider: 'google' }; + let originalFetch: typeof globalThis.fetch; + + beforeEach(() => { + installBrowserGlobals(`http://localhost:3000/spa-callback?code=auth-code&state=${STATE}`); + process.env.BLOCKS_API_URL = 'http://localhost:3000/aws-blocks/api'; + store.set('__blocks_oidc_pending', JSON.stringify({ + provider: 'google', + verifier: 'v', + state: STATE, + nonce: 'n', + callbackUrl: 'http://localhost:3000/spa-callback', + appState: 'app-state', + })); + originalFetch = globalThis.fetch; + }); + + afterEach(() => { + globalThis.fetch = originalFetch; + delete process.env.BLOCKS_API_URL; + clearBrowserGlobals(); + }); + + function stubExchangeOk(body: unknown): void { + globalThis.fetch = (async () => ({ ok: true, json: async () => body })) as unknown as typeof globalThis.fetch; + } + + test('dispatches a same-window auth-change event so on-page onAuthChange consumers re-render', async () => { + stubExchangeOk({ user: BARE_USER }); + // auth-common's broadcastAuthChange() fires a 'blocks-auth-change' + // CustomEvent on window; onAuthChange listeners on THIS page rely on it. + let detail: any = null; + (globalThis as any).window.addEventListener('blocks-auth-change', (e: any) => { detail = e.detail; }); + + const client = makeClient(); + const user = await client.handleRedirectCallback(); + + assert.ok(user, 'callback should resolve a user'); + assert.ok(detail, 'a blocks-auth-change event should have been dispatched on window'); + assert.strictEqual(detail.type, 'auth-change'); + assert.strictEqual(detail.user.userId, 'iss:sub'); + assert.strictEqual(detail.user.username, 'alice'); + }); + + test('posts the signed-in user across tabs via BroadcastChannel', async () => { + stubExchangeOk({ user: BARE_USER }); + const client = makeClient(); + await client.handleRedirectCallback(); + + assert.strictEqual(broadcasts.length, 1, 'exactly one cross-tab post should have been made'); + const msg = broadcasts[0] as any; + assert.strictEqual(msg.type, 'auth-change'); + assert.strictEqual(msg.user.userId, 'iss:sub'); + }); + + test('does NOT broadcast when the callback fails (state mismatch)', async () => { + stubExchangeOk({ user: BARE_USER }); + // Tamper the stored state so validation throws before any exchange. + store.set('__blocks_oidc_pending', JSON.stringify({ + provider: 'google', verifier: 'v', state: 'a-different-state', nonce: 'n', + callbackUrl: 'http://localhost:3000/spa-callback', + })); + let dispatched = false; + (globalThis as any).window.addEventListener('blocks-auth-change', () => { dispatched = true; }); + + const client = makeClient(); + await assert.rejects(() => client.handleRedirectCallback(), /state mismatch/); + + assert.strictEqual(dispatched, false, 'no auth-change event on a failed callback'); + assert.strictEqual(broadcasts.length, 0, 'no cross-tab post on a failed callback'); + }); +}); + +describe('AuthOIDCClient.signOut — @aws-blocks/auth-common bridge', () => { + let originalFetch: typeof globalThis.fetch; + + beforeEach(() => { + installBrowserGlobals('http://localhost:3000/dashboard'); + process.env.BLOCKS_API_URL = 'http://localhost:3000/aws-blocks/api'; + originalFetch = globalThis.fetch; + // The /aws-blocks/auth/signout POST just needs to resolve OK. + globalThis.fetch = (async () => ({ ok: true, json: async () => ({}) })) as unknown as typeof globalThis.fetch; + }); + + afterEach(() => { + globalThis.fetch = originalFetch; + delete process.env.BLOCKS_API_URL; + clearBrowserGlobals(); + }); + + test('posts a signed-out (null) user across tabs via BroadcastChannel', async () => { + const client = makeClient(); + await client.signOut(); + + assert.strictEqual(broadcasts.length, 1, 'exactly one cross-tab post should have been made'); + const msg = broadcasts[0] as any; + assert.strictEqual(msg.type, 'auth-change'); + assert.strictEqual(msg.user, null, 'sign-out broadcasts a null user so other tabs re-render'); + }); + + test('dispatches a same-window auth-change(null) event before reloading', async () => { + // Other tabs rely on the cross-tab post above; same-tab onAuthChange + // consumers rely on this same-window event. The page then reloads. + let detail: any = 'unset'; + (globalThis as any).window.addEventListener('blocks-auth-change', (e: any) => { detail = e.detail; }); + + const client = makeClient(); + await client.signOut(); + + assert.notStrictEqual(detail, 'unset', 'a blocks-auth-change event should have been dispatched on window'); + assert.strictEqual(detail.type, 'auth-change'); + assert.strictEqual(detail.user, null); + assert.strictEqual(reloaded, true, 'signOut should reload the page after broadcasting'); + }); +}); + +describe('AuthOIDCClient.signOut — server-side (no window / BroadcastChannel)', () => { + let originalFetch: typeof globalThis.fetch; + let savedWindow: unknown; + let savedLocation: unknown; + let savedSessionStorage: unknown; + let savedBroadcastChannelGlobal: unknown; + let signoutPosted: boolean; + + beforeEach(() => { + // Emulate SSR: strip the browser globals that broadcastAuthChange() (a + // BroadcastChannel + window.dispatchEvent) and the reload depend on. + // Snapshot first so a sibling describe that installed them isn't disturbed. + // Note: Node ships a real global BroadcastChannel, so it must be removed + // too — otherwise an un-guarded broadcast would open a live channel. + savedWindow = (globalThis as any).window; + savedLocation = (globalThis as any).location; + savedSessionStorage = (globalThis as any).sessionStorage; + savedBroadcastChannelGlobal = (globalThis as any).BroadcastChannel; + delete (globalThis as any).window; + delete (globalThis as any).location; + delete (globalThis as any).sessionStorage; + delete (globalThis as any).BroadcastChannel; + + // Reset the shared capture state (installBrowserGlobals normally does this) + // so the assertions below can't observe a sibling test's broadcast/reload. + broadcasts.length = 0; + reloaded = false; + + // _getBaseUrl() resolves from this without touching window. + process.env.BLOCKS_API_URL = 'http://localhost:3000/aws-blocks/api'; + originalFetch = globalThis.fetch; + signoutPosted = false; + globalThis.fetch = (async (url: any, init?: any) => { + if (String(url).endsWith('/aws-blocks/auth/signout') && init?.method === 'POST') { + signoutPosted = true; + } + return { ok: true, json: async () => ({}) }; + }) as unknown as typeof globalThis.fetch; + }); + + afterEach(() => { + globalThis.fetch = originalFetch; + delete process.env.BLOCKS_API_URL; + // Restore exactly what we snapshotted so sibling tests are unaffected. + if (savedWindow === undefined) delete (globalThis as any).window; + else (globalThis as any).window = savedWindow; + if (savedLocation === undefined) delete (globalThis as any).location; + else (globalThis as any).location = savedLocation; + if (savedSessionStorage === undefined) delete (globalThis as any).sessionStorage; + else (globalThis as any).sessionStorage = savedSessionStorage; + if (savedBroadcastChannelGlobal === undefined) delete (globalThis as any).BroadcastChannel; + else (globalThis as any).BroadcastChannel = savedBroadcastChannelGlobal; + }); + + test('completes the server-side sign-out without a window and never broadcasts', async () => { + const client = makeClient(); + // Pre-fix this rejected: broadcastAuthChange(null) ran before the window + // guard, so getChannel()/window.dispatchEvent threw a ReferenceError after + // the sign-out POST had already completed — stranding the returned promise. + await assert.doesNotReject(() => client.signOut()); + + assert.strictEqual(signoutPosted, true, 'the server-side sign-out POST should still run'); + assert.strictEqual(broadcasts.length, 0, 'no cross-tab broadcast should be attempted with no window'); + assert.strictEqual(reloaded, false, 'no page reload server-side'); + }); +}); diff --git a/packages/bb-auth-oidc/src/index.browser.ts b/packages/bb-auth-oidc/src/index.browser.ts index 19a535db..13eda9f5 100644 --- a/packages/bb-auth-oidc/src/index.browser.ts +++ b/packages/bb-auth-oidc/src/index.browser.ts @@ -9,6 +9,8 @@ */ import { ApiError, registerMiddleware } from '@aws-blocks/core/client'; +import { broadcastAuthChange } from '@aws-blocks/auth-common/ui'; +import type { AuthUser } from '@aws-blocks/auth-common'; // Provider helpers are pure config builders — safe to ship to the browser. export { @@ -207,7 +209,7 @@ function notify(user: unknown | null, meta: AuthStateMeta | null): void { */ export class AuthOIDCClient< Provider extends string = string, - User = { userId: string; username: string }, + User extends AuthUser = { userId: string; username: string }, > { readonly providers: readonly Provider[]; @@ -406,6 +408,22 @@ export class AuthOIDCClient< const user = (body.user ?? body) as User; lastUser = user; notify(user, { state: pending.appState }); + // Bridge into @aws-blocks/auth-common so its `onAuthChange` subscribers and + // `` re-render on sign-in (same window AND other tabs), + // not just this client's own `onAuthStateChange` listeners. + // + // We broadcast rather than write auth-common's state directly — two known, + // non-blocking consequences: + // - It doesn't prime auth-common's shared cache (keyed by AuthStateApi and + // written via a private `updateState`; this client holds no such handle), + // so a component that subscribes via `onAuthChange` *after* this fires + // paints `null` for one frame, then self-corrects on its `getAuthState()`. + // - A same-tab `` listens only on the cross-tab + // BroadcastChannel (which never fires in the originating tab), so it + // won't react here; `onAuthChange` / `` do. + // The `User` generic is constrained to `extends AuthUser`, so the exchange + // response is structurally a superset of AuthUser's { userId, username }. + broadcastAuthChange(user); return user; } finally { // Release the guard once this exchange settles (success or failure) so a @@ -422,7 +440,18 @@ export class AuthOIDCClient< await fetch(`${baseUrl}${this.signOutPath}`, { method: 'POST', credentials: 'include' }); lastUser = null; notify(null, null); + // Bridge the sign-out into @aws-blocks/auth-common too, symmetrically with + // handleRedirectCallback(). The reload below only repaints THIS tab, and + // BroadcastChannel.postMessage never fires in the originating tab, so other + // open tabs' onAuthChange / consumers would keep + // rendering the signed-in user until their own next reload without this. + // + // broadcastAuthChange() opens a BroadcastChannel and dispatches a window + // event, so it needs the same browser globals as the reload — guard both + // together so a server-side sign-out (no window) doesn't throw a + // ReferenceError after the sign-out POST above has already succeeded. if (typeof window !== 'undefined' && window.location) { + broadcastAuthChange(null); window.location.reload(); } }