From 2071587728b1ce44b9606734ea3deae69774673d Mon Sep 17 00:00:00 2001 From: kjgbot Date: Thu, 10 Sep 2026 15:45:40 +0200 Subject: [PATCH 1/2] fix(observer): auto-login from ?key= URL param MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Guard the observer dashboard's `?key=` auto-login path (used by the "Join as observer" links that embed either an `rk_live_` workspace key or an `ot_live_` observer token). The reader in `RelaySessionProvider` is functionally correct today but has silently regressed twice — once when the observer-token prefix was introduced (#230) and again when the stream-token minting reshuffled the effect body (#240) — because nothing directly asserted the URL param actually flows into the login POST. Extract the effect body into a pure `resolveObserverSession` helper and cover every branch (both prefixes, rejected key, no key with a cookie session, malformed key, network error, missing wsToken) with unit tests. Add a jsdom integration test that mounts the provider with `?key=ot_live_...` in the URL and asserts the login POST fires with the exact token and the authenticated children render. Co-Authored-By: Claude Opus 4.7 (1M context) Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82 --- CHANGELOG.md | 6 +- package-lock.json | 20 +- packages/observer-dashboard/package.json | 3 + .../components/RelaySessionProvider.test.tsx | 154 ++++++++++ .../src/components/RelaySessionProvider.tsx | 74 ++--- .../src/lib/observer-auto-login.test.ts | 270 ++++++++++++++++++ .../src/lib/observer-auto-login.ts | 114 ++++++++ packages/observer-dashboard/vitest.config.ts | 6 + 8 files changed, 583 insertions(+), 64 deletions(-) create mode 100644 packages/observer-dashboard/src/components/RelaySessionProvider.test.tsx create mode 100644 packages/observer-dashboard/src/lib/observer-auto-login.test.ts create mode 100644 packages/observer-dashboard/src/lib/observer-auto-login.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index b6830cef..a0b8ba64 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,11 @@ This project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). Packages without a separate changelog are covered by the cross-package notes below. -## [Unreleased] +## [Unreleased - Patch] + +### Fixed + +- Observer dashboard: pin the `?key=` auto-login path with a dedicated `resolveObserverSession` helper and regression tests so the "Join as observer" URL shape (both `rk_live_` and `ot_live_`) cannot silently regress again. ## [8.8.0] - 2026-09-10 diff --git a/package-lock.json b/package-lock.json index e752779a..05948238 100644 --- a/package-lock.json +++ b/package-lock.json @@ -146,7 +146,6 @@ "integrity": "sha512-Aup7aUOfpbAUg2ROOJN6Iw5f9DMBlzu0mIkm/malLQFN/YQgO48wCj0Kxa3sEHJvPVFg7siR+qRInwXd2qhQKw==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@babel/helper-validator-identifier": "^7.29.7", "js-tokens": "^4.0.0", @@ -162,7 +161,6 @@ "integrity": "sha512-qehxGkRj55h/ff8EMaJ+cYhyaKlHIxqYDn682wQD7RNp9UujOQsHog2uS0r2vzr4pW+sXf90NeeayjcNaX3fFg==", "dev": true, "license": "MIT", - "peer": true, "engines": { "node": ">=6.9.0" } @@ -2517,7 +2515,6 @@ "integrity": "sha512-o4PXJQidqJl82ckFaXUeoAW+XysPLauYI43Abki5hABd853iMhitooc6znOnczgbTYmEP6U6/y1ZyKAIsvMKGg==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@babel/code-frame": "^7.10.4", "@babel/runtime": "^7.12.5", @@ -2651,8 +2648,7 @@ "resolved": "https://registry.npmjs.org/@types/aria-query/-/aria-query-5.0.4.tgz", "integrity": "sha512-rfT93uj5s0PRL7EzccGMs3brplhcrghnDoV26NqKhCAS1hVo+WdNsPvE/yb6ilfr5hi2MEk6d5EWJTKdxg8jVw==", "dev": true, - "license": "MIT", - "peer": true + "license": "MIT" }, "node_modules/@types/better-sqlite3": { "version": "7.6.13", @@ -3461,7 +3457,6 @@ "integrity": "sha512-quJQXlTSUGL2LH9SUXo8VwsY4soanhgo6LNSm84E1LBcE8s3O0wpdiRzyR9z/ZZJMlMWv37qOOb9pdJlMUEKFQ==", "dev": true, "license": "MIT", - "peer": true, "engines": { "node": ">=8" } @@ -3495,7 +3490,6 @@ "integrity": "sha512-b0P0sZPKtyu8HkeRAfCq0IfURZK+SuwMjY1UXGBU27wpAiTwQAIlq56IbIO+ytk/JjS1fMR14ee5WBBfKi5J6A==", "dev": true, "license": "Apache-2.0", - "peer": true, "dependencies": { "dequal": "^2.0.3" } @@ -4241,8 +4235,7 @@ "resolved": "https://registry.npmjs.org/dom-accessibility-api/-/dom-accessibility-api-0.5.16.tgz", "integrity": "sha512-X7BJ2yElsnOJ30pZF4uIIDfBEVgF4XEBxL9Bxhy6dnrm5hkzqmsWHGTiHqRiITNhMyFLyAiWndIJP7Z1NTteDg==", "dev": true, - "license": "MIT", - "peer": true + "license": "MIT" }, "node_modules/drizzle-kit": { "version": "0.31.10", @@ -6211,7 +6204,6 @@ "integrity": "sha512-h5bgJWpxJNswbU7qCrV0tIKQCaS3blPDrqKWx+QxzuzL1zGUzij9XCWLrSLsJPu5t+eWA/ycetzYAO5IOMcWAQ==", "dev": true, "license": "MIT", - "peer": true, "bin": { "lz-string": "bin/bin.js" } @@ -7740,7 +7732,6 @@ "integrity": "sha512-Qb1gy5OrP5+zDf2Bvnzdl3jsTf1qXVMazbvCoKhtKqVs4/YK4ozX4gKQJJVyNe+cajNPn0KoC0MC3FUmaHWEmQ==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "ansi-regex": "^5.0.1", "ansi-styles": "^5.0.0", @@ -7756,7 +7747,6 @@ "integrity": "sha512-Cxwpt2SfTzTtXcfOlzGEee8O+c+MmUgGrNiBcXnuWxuFJHe6a5Hz7qwhwe5OgaSYI0IJvkLqWX1ASG+cJOkEiA==", "dev": true, "license": "MIT", - "peer": true, "engines": { "node": ">=10" }, @@ -7951,8 +7941,7 @@ "resolved": "https://registry.npmjs.org/react-is/-/react-is-17.0.2.tgz", "integrity": "sha512-w2GsyukL62IJnlaff/nRegPQR94C/XXamvMWmSHRJ4y7Ts/4ocGRmTHvOs8PSE6pB3dWOrD/nueuU5sduBsQ4w==", "dev": true, - "license": "MIT", - "peer": true + "license": "MIT" }, "node_modules/react-markdown": { "version": "10.1.0", @@ -10800,9 +10789,12 @@ }, "devDependencies": { "@tailwindcss/postcss": "^4.0.0", + "@testing-library/dom": "^10.4.1", + "@testing-library/react": "^16.3.3", "@types/node": "^22.0.0", "@types/react": "^18.3.0", "@types/react-dom": "^18.3.0", + "jsdom": "^25.0.1", "postcss": "^8.5.0", "tailwindcss": "^4.0.0", "typescript": "^5.7.0" diff --git a/packages/observer-dashboard/package.json b/packages/observer-dashboard/package.json index c51430f9..26d8575e 100644 --- a/packages/observer-dashboard/package.json +++ b/packages/observer-dashboard/package.json @@ -22,9 +22,12 @@ }, "devDependencies": { "@tailwindcss/postcss": "^4.0.0", + "@testing-library/dom": "^10.4.1", + "@testing-library/react": "^16.3.3", "@types/node": "^22.0.0", "@types/react": "^18.3.0", "@types/react-dom": "^18.3.0", + "jsdom": "^25.0.1", "postcss": "^8.5.0", "tailwindcss": "^4.0.0", "typescript": "^5.7.0" diff --git a/packages/observer-dashboard/src/components/RelaySessionProvider.test.tsx b/packages/observer-dashboard/src/components/RelaySessionProvider.test.tsx new file mode 100644 index 00000000..1a6b8a15 --- /dev/null +++ b/packages/observer-dashboard/src/components/RelaySessionProvider.test.tsx @@ -0,0 +1,154 @@ +// @vitest-environment jsdom + +/** + * Integration cover for the `?key=` auto-login flow at the component + * boundary. The unit tests in `../lib/observer-auto-login.test.ts` pin the + * request/response contract; this test pins the wiring — that + * `useSearchParams` is actually read, that the login POST fires with the key + * from the URL, and that the authenticated children render instead of the + * spinner. If a future refactor drops the URL reader, drops the `` + * wrap, or accidentally regresses the effect to skip the login POST, this + * test fails. + */ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { act, cleanup, render, screen, waitFor } from '@testing-library/react'; +import type { ReactNode } from 'react'; + +// Mock `next/navigation` so we don't need a real Next.js route tree. The key +// piece is `useSearchParams` returning an object whose `.get('key')` yields +// the ot_live_ token; the rest of the router is a no-op stub. Both the router +// object and the searchParams object are stable references — the real Next +// hooks return stable identities per navigation, and if we return a fresh +// object each render the provider's useEffect fires in a loop. +const routerReplace = vi.fn(); +const stableRouter = { + replace: routerReplace, + push: () => undefined, + prefetch: () => undefined, + back: () => undefined, + forward: () => undefined, + refresh: () => undefined, +}; +vi.mock('next/navigation', () => ({ + useRouter: () => stableRouter, + useSearchParams: () => currentSearchParams, +})); + +// Mock the RelayProvider so it does not try to open a real websocket in jsdom. +// The `data-testid` marker is what the test asserts against to confirm the +// authenticated children are rendered. +vi.mock('@relaycast/react', () => ({ + RelayProvider: ({ children, apiKey }: { children: ReactNode; apiKey: string }) => ( +
+ {children} +
+ ), +})); + +let currentSearchParams: URLSearchParams = new URLSearchParams(); +function setSearchParams(qs: string) { + currentSearchParams = new URLSearchParams(qs); +} + +import { RelaySessionProvider } from './RelaySessionProvider'; + +function jsonResponse(body: unknown, status = 200): Response { + return new Response(JSON.stringify(body), { + status, + headers: { 'Content-Type': 'application/json' }, + }); +} + +describe('RelaySessionProvider auto-login from ?key=', () => { + afterEach(() => { + cleanup(); + vi.restoreAllMocks(); + vi.unstubAllGlobals(); + routerReplace.mockReset(); + currentSearchParams = new URLSearchParams(); + }); + + it('reads ot_live_ from the URL, POSTs to /login, and renders authenticated children', async () => { + setSearchParams('key=ot_live_192c87a128285f51a33ba902680ef67a56e6edc2cc79fbe3'); + + const fetchMock = vi.fn(async (input) => { + const url = typeof input === 'string' ? input : (input as Request).url; + if (url.endsWith('/observer/api/auth/login')) { + return jsonResponse({ success: true }); + } + if (url.endsWith('/observer/api/auth/session')) { + return jsonResponse({ + authenticated: true, + apiKey: 'ot_live_from_login', + agentToken: 'ot_live_from_login', + wsToken: 'ot_live_from_login', + baseUrl: 'https://cast.agentrelay.com', + }); + } + throw new Error(`Unexpected fetch to ${url}`); + }); + vi.stubGlobal('fetch', fetchMock); + + await act(async () => { + render( + +
dashboard
+ , + ); + }); + + // The RelayProvider mock renders once the auto-login flow reaches + // `authenticated`. If the URL reader regresses (key ignored, effect skipped, + // or router bounces to /login before the POST fires) this wait times out. + await waitFor(() => { + expect(screen.getByTestId('relay-provider')).toBeTruthy(); + }); + expect(screen.getByTestId('dashboard-child')).toBeTruthy(); + expect(screen.getByTestId('relay-provider').getAttribute('data-api-key')).toBe( + 'ot_live_from_login', + ); + + // Login POST must include the exact URL-provided token. + const loginCall = fetchMock.mock.calls.find( + ([url]) => typeof url === 'string' && url.endsWith('/observer/api/auth/login'), + ); + expect(loginCall).toBeDefined(); + expect(loginCall?.[1]?.body).toBe( + JSON.stringify({ + apiKey: 'ot_live_192c87a128285f51a33ba902680ef67a56e6edc2cc79fbe3', + }), + ); + + // Once authenticated, the URL is cleaned so bookmarks / refresh don't + // re-POST the key on every reload. + await waitFor(() => { + expect(routerReplace).toHaveBeenCalledWith('/'); + }); + }); + + it('bounces to /login when the URL key is rejected by /login', async () => { + setSearchParams('key=ot_live_bad'); + + const fetchMock = vi.fn(async (input) => { + const url = typeof input === 'string' ? input : (input as Request).url; + if (url.endsWith('/observer/api/auth/login')) { + return new Response(JSON.stringify({ success: false }), { status: 401 }); + } + throw new Error(`Unexpected fetch to ${url}`); + }); + vi.stubGlobal('fetch', fetchMock); + + await act(async () => { + render( + +
dashboard
+ , + ); + }); + + await waitFor(() => { + expect(routerReplace).toHaveBeenCalledWith('/login'); + }); + expect(screen.queryByTestId('relay-provider')).toBeNull(); + }); +}); diff --git a/packages/observer-dashboard/src/components/RelaySessionProvider.tsx b/packages/observer-dashboard/src/components/RelaySessionProvider.tsx index 95707f78..089619bb 100644 --- a/packages/observer-dashboard/src/components/RelaySessionProvider.tsx +++ b/packages/observer-dashboard/src/components/RelaySessionProvider.tsx @@ -3,72 +3,48 @@ import { useEffect, useRef, useState } from 'react'; import { useRouter, useSearchParams } from 'next/navigation'; import { RelayProvider } from '@relaycast/react'; -import { setAuth } from '../lib/auth'; import { resetActivityIfWorkspaceChanged } from '../lib/activity-store'; - -interface Session { - apiKey: string; - agentToken: string; - wsToken: string | null; - baseUrl: string; -} +import { + resolveObserverSession, + type ObserverSessionData, +} from '../lib/observer-auto-login'; export function RelaySessionProvider({ children }: { children: React.ReactNode }) { const router = useRouter(); const searchParams = useSearchParams(); - const [session, setSession] = useState(null); + const [session, setSession] = useState(null); const [checking, setChecking] = useState(true); const requestSeq = useRef(0); useEffect(() => { const seq = ++requestSeq.current; + // Read the `?key=` param synchronously so a query-string arrival (the + // "Join as observer" link shape used by Pear and the site) triggers + // auto-login before we probe the existing cookie session. This is the + // regression-prone path — keep the read explicit and covered by + // observer-auto-login.test.ts. const keyParam = searchParams.get('key'); async function initSession() { - try { - if (keyParam?.startsWith('rk_live_') || keyParam?.startsWith('ot_live_')) { - const success = await setAuth(keyParam); - if (seq !== requestSeq.current) return; - if (!success) { - router.replace('/login'); - return; - } - } - - const res = await fetch('/observer/api/auth/session'); - if (seq !== requestSeq.current) return; - - if (!res.ok) { - router.replace('/login'); - return; - } + const outcome = await resolveObserverSession({ keyParam }); + if (seq !== requestSeq.current) return; - const data = await res.json(); - if (seq !== requestSeq.current) return; - - if (data?.authenticated) { - // Drop another workspace's cached activity before this dashboard - // mounts, so switching keys never hydrates stale cross-workspace events. - resetActivityIfWorkspaceChanged(data.apiKey); - setSession({ - apiKey: data.apiKey, - agentToken: data.agentToken, - // Never fall back to the REST/admin credential for the socket; a - // missing stream token means the realtime stream stays offline. - wsToken: data.wsToken ?? null, - baseUrl: data.baseUrl, - }); - if (keyParam) router.replace('/'); - } else { - router.replace('/login'); - } - } catch { - if (seq !== requestSeq.current) return; + if (outcome.kind === 'unauthenticated') { router.replace('/login'); - } finally { - if (seq !== requestSeq.current) return; setChecking(false); + return; + } + + // Drop another workspace's cached activity before this dashboard mounts, + // so switching keys never hydrates stale cross-workspace events. + resetActivityIfWorkspaceChanged(outcome.session.apiKey); + setSession(outcome.session); + // Strip the key from the URL only after the session is established, so + // the URL change doesn't race with the session fetch. + if (outcome.consumedKeyParam) { + router.replace('/'); } + setChecking(false); } initSession(); diff --git a/packages/observer-dashboard/src/lib/observer-auto-login.test.ts b/packages/observer-dashboard/src/lib/observer-auto-login.test.ts new file mode 100644 index 00000000..f7123170 --- /dev/null +++ b/packages/observer-dashboard/src/lib/observer-auto-login.test.ts @@ -0,0 +1,270 @@ +/** + * Guardrails for the `?key=` auto-login flow. + * + * The observer dashboard supports arriving at `/observer?key=ot_live_...` (or + * `rk_live_...`) — the "Join as observer" links depend on this shape and it + * has silently regressed before. These tests pin every branch of + * `resolveObserverSession` so a future refactor that drops the URL reader + * or reshapes the login/session round-trip fails loudly. + */ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { + isAcceptedObserverKey, + resolveObserverSession, +} from './observer-auto-login'; + +function jsonResponse(body: unknown, status = 200): Response { + return new Response(JSON.stringify(body), { + status, + headers: { 'Content-Type': 'application/json' }, + }); +} + +function stubFetchSequence(...responses: Array) { + const fetchMock = vi.fn(); + for (const response of responses) { + if (response instanceof Error) { + fetchMock.mockRejectedValueOnce(response); + } else { + fetchMock.mockResolvedValueOnce(response); + } + } + return fetchMock; +} + +describe('isAcceptedObserverKey', () => { + it('accepts workspace and observer tokens', () => { + expect(isAcceptedObserverKey('rk_live_abc')).toBe(true); + expect(isAcceptedObserverKey('ot_live_xyz')).toBe(true); + }); + + it('rejects anything else, including nullish values', () => { + expect(isAcceptedObserverKey(null)).toBe(false); + expect(isAcceptedObserverKey(undefined)).toBe(false); + expect(isAcceptedObserverKey('')).toBe(false); + expect(isAcceptedObserverKey('rk_test_abc')).toBe(false); + expect(isAcceptedObserverKey('ot_test_abc')).toBe(false); + expect(isAcceptedObserverKey('Bearer ot_live_abc')).toBe(false); + }); +}); + +describe('resolveObserverSession', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('auto-logs-in with an ot_live_ token from the URL and hydrates the session', async () => { + const fetchImpl = stubFetchSequence( + jsonResponse({ success: true }), + jsonResponse({ + authenticated: true, + apiKey: 'ot_live_from_url', + agentToken: 'ot_live_from_url', + wsToken: 'ot_live_from_url', + baseUrl: 'https://cast.agentrelay.com', + }), + ); + + const outcome = await resolveObserverSession({ + keyParam: 'ot_live_192c87a128285f51a33ba902680ef67a56e6edc2cc79fbe3', + fetchImpl: fetchImpl as unknown as typeof fetch, + }); + + // The login POST must fire with the exact URL-provided token — this is the + // reader that regressed silently in the past. + expect(fetchImpl).toHaveBeenNthCalledWith( + 1, + '/observer/api/auth/login', + expect.objectContaining({ + method: 'POST', + body: JSON.stringify({ + apiKey: 'ot_live_192c87a128285f51a33ba902680ef67a56e6edc2cc79fbe3', + }), + }), + ); + expect(fetchImpl).toHaveBeenNthCalledWith(2, '/observer/api/auth/session'); + expect(outcome).toEqual({ + kind: 'authenticated', + consumedKeyParam: true, + session: { + apiKey: 'ot_live_from_url', + agentToken: 'ot_live_from_url', + wsToken: 'ot_live_from_url', + baseUrl: 'https://cast.agentrelay.com', + }, + }); + }); + + it('auto-logs-in with an rk_live_ workspace key from the URL', async () => { + const fetchImpl = stubFetchSequence( + jsonResponse({ success: true }), + jsonResponse({ + authenticated: true, + apiKey: 'rk_live_admin', + agentToken: 'rk_live_admin', + // Login route mints a scoped `ot_live_` token for the stream so the + // socket never carries the admin key. + wsToken: 'ot_live_minted_for_stream', + baseUrl: 'https://cast.agentrelay.com', + }), + ); + + const outcome = await resolveObserverSession({ + keyParam: 'rk_live_admin', + fetchImpl: fetchImpl as unknown as typeof fetch, + }); + + expect(outcome).toMatchObject({ + kind: 'authenticated', + consumedKeyParam: true, + session: { wsToken: 'ot_live_minted_for_stream' }, + }); + }); + + it('signals unauthenticated when the login POST rejects the URL key', async () => { + const fetchImpl = stubFetchSequence( + new Response(JSON.stringify({ success: false, error: 'Invalid API key' }), { + status: 401, + }), + ); + + const outcome = await resolveObserverSession({ + keyParam: 'ot_live_bad', + fetchImpl: fetchImpl as unknown as typeof fetch, + }); + + // Must not fall through to the session route on a rejected login — that + // would either 401 or return the previous session's identity. + expect(fetchImpl).toHaveBeenCalledTimes(1); + expect(outcome).toEqual({ kind: 'unauthenticated', reason: 'invalid-key' }); + }); + + it('signals unauthenticated when login returns 200 but success=false', async () => { + const fetchImpl = stubFetchSequence(jsonResponse({ success: false })); + + const outcome = await resolveObserverSession({ + keyParam: 'ot_live_stale', + fetchImpl: fetchImpl as unknown as typeof fetch, + }); + + expect(outcome).toEqual({ kind: 'unauthenticated', reason: 'invalid-key' }); + }); + + it('skips the login POST when no key is present and returns the existing session', async () => { + const fetchImpl = stubFetchSequence( + jsonResponse({ + authenticated: true, + apiKey: 'rk_live_from_cookie', + agentToken: 'rk_live_from_cookie', + wsToken: 'ot_live_from_cookie', + baseUrl: 'https://cast.agentrelay.com', + }), + ); + + const outcome = await resolveObserverSession({ + keyParam: null, + fetchImpl: fetchImpl as unknown as typeof fetch, + }); + + expect(fetchImpl).toHaveBeenCalledTimes(1); + expect(fetchImpl).toHaveBeenCalledWith('/observer/api/auth/session'); + expect(outcome).toEqual({ + kind: 'authenticated', + consumedKeyParam: false, + session: { + apiKey: 'rk_live_from_cookie', + agentToken: 'rk_live_from_cookie', + wsToken: 'ot_live_from_cookie', + baseUrl: 'https://cast.agentrelay.com', + }, + }); + }); + + it('ignores an obviously-malformed key param and probes the session cookie instead', async () => { + // e.g. `?key=` followed by a stray value from a bookmark or referer; the + // dashboard should not POST that upstream, but must still probe the + // existing cookie session so a returning user is not falsely bounced. + const fetchImpl = stubFetchSequence( + jsonResponse({ + authenticated: true, + apiKey: 'rk_live_from_cookie', + agentToken: 'rk_live_from_cookie', + wsToken: null, + baseUrl: 'https://cast.agentrelay.com', + }), + ); + + const outcome = await resolveObserverSession({ + keyParam: 'not-a-token', + fetchImpl: fetchImpl as unknown as typeof fetch, + }); + + expect(fetchImpl).toHaveBeenCalledTimes(1); + expect(fetchImpl).toHaveBeenCalledWith('/observer/api/auth/session'); + expect(outcome).toMatchObject({ + kind: 'authenticated', + consumedKeyParam: false, + session: { wsToken: null }, + }); + }); + + it('signals unauthenticated when the session probe returns 401', async () => { + const fetchImpl = stubFetchSequence( + new Response(JSON.stringify({ authenticated: false }), { status: 401 }), + ); + + const outcome = await resolveObserverSession({ + keyParam: null, + fetchImpl: fetchImpl as unknown as typeof fetch, + }); + + expect(outcome).toEqual({ kind: 'unauthenticated', reason: 'no-session' }); + }); + + it('signals unauthenticated when the session probe returns 200 but authenticated=false', async () => { + const fetchImpl = stubFetchSequence(jsonResponse({ authenticated: false })); + + const outcome = await resolveObserverSession({ + keyParam: null, + fetchImpl: fetchImpl as unknown as typeof fetch, + }); + + expect(outcome).toEqual({ kind: 'unauthenticated', reason: 'no-session' }); + }); + + it('never falls back to the admin key when the session omits wsToken', async () => { + // Sessions created before the minted-token cookie existed have no wsToken; + // the resolver must leave it null so the socket declines to connect rather + // than opening with the admin/REST key. + const fetchImpl = stubFetchSequence( + jsonResponse({ + authenticated: true, + apiKey: 'rk_live_legacy', + agentToken: 'rk_live_legacy', + // wsToken intentionally absent + baseUrl: 'https://cast.agentrelay.com', + }), + ); + + const outcome = await resolveObserverSession({ + keyParam: null, + fetchImpl: fetchImpl as unknown as typeof fetch, + }); + + if (outcome.kind !== 'authenticated') { + throw new Error('expected authenticated outcome'); + } + expect(outcome.session.wsToken).toBeNull(); + }); + + it('collapses network errors into an unauthenticated outcome so the UI can redirect', async () => { + const fetchImpl = stubFetchSequence(new Error('network down')); + + const outcome = await resolveObserverSession({ + keyParam: 'ot_live_abc', + fetchImpl: fetchImpl as unknown as typeof fetch, + }); + + expect(outcome).toEqual({ kind: 'unauthenticated', reason: 'error' }); + }); +}); diff --git a/packages/observer-dashboard/src/lib/observer-auto-login.ts b/packages/observer-dashboard/src/lib/observer-auto-login.ts new file mode 100644 index 00000000..e21764b7 --- /dev/null +++ b/packages/observer-dashboard/src/lib/observer-auto-login.ts @@ -0,0 +1,114 @@ +/** + * Auto-login flow for the observer dashboard. + * + * The dashboard supports arriving at `/observer?key=` with either a + * workspace key (`rk_live_...`) or a scoped observer token (`ot_live_...`) in + * the URL. This module owns the "read the key, POST to /login, then GET the + * session" sequence so it can be unit-tested without mounting React — the + * component-level useEffect just calls `resolveObserverSession` and reacts to + * the outcome. + * + * Keeping this shape stable is load-bearing for the "Join as observer" links + * (which embed an `ot_live_` token in the URL) — the URL param reader has + * regressed silently in the past because there was no direct test guarding it. + */ +export interface ObserverSessionData { + apiKey: string; + agentToken: string; + /** + * The realtime-stream credential. Intentionally NOT backfilled from the + * workspace admin key: the socket must never carry an admin key. `null` means + * the realtime stream stays offline until the next login. + */ + wsToken: string | null; + baseUrl: string; +} + +export type AutoLoginOutcome = + /** A key was present in the URL and the login call accepted it. */ + | { kind: 'authenticated'; consumedKeyParam: true; session: ObserverSessionData } + /** No key in the URL, but an existing cookie session is still valid. */ + | { kind: 'authenticated'; consumedKeyParam: false; session: ObserverSessionData } + /** The user must be sent to `/login` (bad key, unauthenticated, or errored). */ + | { kind: 'unauthenticated'; reason: 'invalid-key' | 'no-session' | 'error' }; + +export interface ResolveObserverSessionOptions { + /** The `?key=` value from `URLSearchParams`, or null when absent. */ + keyParam: string | null; + /** + * Fetch implementation, injectable so tests never touch the real network and + * production always uses the browser's `fetch`. + */ + fetchImpl?: typeof fetch; +} + +/** + * Both credential shapes accepted by `/observer/api/auth/login`. The dashboard + * validates the prefix client-side so an obviously-malformed URL param never + * even reaches the login route. + */ +export function isAcceptedObserverKey(value: string | null | undefined): value is string { + return typeof value === 'string' && (value.startsWith('rk_live_') || value.startsWith('ot_live_')); +} + +/** + * Drive the auto-login-then-session sequence. + * + * When `keyParam` is a valid credential, POST it to the dashboard's login + * route to set the httpOnly cookies, then GET the session route to hydrate the + * `RelayProvider` props. When there is no key, skip straight to the session + * fetch (existing cookie flow). Any failure lands in `{kind: 'unauthenticated'}` + * so the caller can redirect to `/login`. + */ +export async function resolveObserverSession( + opts: ResolveObserverSessionOptions, +): Promise { + const doFetch = opts.fetchImpl ?? fetch; + const consumedKeyParam = isAcceptedObserverKey(opts.keyParam); + + try { + if (consumedKeyParam) { + const loginRes = await doFetch('/observer/api/auth/login', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ apiKey: opts.keyParam }), + }); + let loginBody: { success?: boolean } | null = null; + try { + loginBody = (await loginRes.json()) as { success?: boolean }; + } catch { + loginBody = null; + } + if (!loginRes.ok || loginBody?.success !== true) { + return { kind: 'unauthenticated', reason: 'invalid-key' }; + } + } + + const sessionRes = await doFetch('/observer/api/auth/session'); + if (!sessionRes.ok) { + return { kind: 'unauthenticated', reason: 'no-session' }; + } + const data = (await sessionRes.json()) as + | (ObserverSessionData & { authenticated?: boolean }) + | null; + if (!data?.authenticated) { + return { kind: 'unauthenticated', reason: 'no-session' }; + } + + const session: ObserverSessionData = { + apiKey: data.apiKey, + agentToken: data.agentToken, + // Never fall back to the admin/REST key for the socket — a missing stream + // token means the realtime feed stays offline until re-login. + wsToken: data.wsToken ?? null, + baseUrl: data.baseUrl, + }; + + if (consumedKeyParam) { + return { kind: 'authenticated', consumedKeyParam: true, session }; + } + return { kind: 'authenticated', consumedKeyParam: false, session }; + } catch { + return { kind: 'unauthenticated', reason: 'error' }; + } +} diff --git a/packages/observer-dashboard/vitest.config.ts b/packages/observer-dashboard/vitest.config.ts index 7382f40e..0827ee63 100644 --- a/packages/observer-dashboard/vitest.config.ts +++ b/packages/observer-dashboard/vitest.config.ts @@ -1,6 +1,12 @@ import { defineConfig } from 'vitest/config'; export default defineConfig({ + esbuild: { + // Use the React automatic JSX runtime so component-level tests don't need + // to `import React` at the top of every file. Next.js applies the same + // transform in the app itself. + jsx: 'automatic', + }, test: { globals: true, }, From 975b1a9f92abb6b9584a032faee00bc139132517 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Thu, 10 Sep 2026 16:16:49 +0200 Subject: [PATCH 2/2] test(observer): pin URL-key-first ordering under stale-cookie scenarios MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A report claimed shared observer links fail on browsers that ever previously logged in — supposedly because the dashboard probes /session first, sees a 401 from stale cookies, and redirects to /login without ever using the URL `?key=`. Reproducing this against `next dev` + headless Chrome (fresh cookie jar for a revoked token, then navigating to /observer?key=) shows the current flow works: RelaySessionProvider POSTs /login first, which overwrites the stale cookies via Set-Cookie, and the follow-up /session GET returns the fresh identity. Same result on `main` and on this branch — no functional regression is present. Since the described failure mode (session-first ordering) *would* actually break the shared-link flow if it ever slipped back in, add explicit guards for it: - observer-auto-login.test.ts: a resolver-level test that fails loudly if /session is fetched before /login when a URL key is present. - RelaySessionProvider.test.tsx: a jsdom integration test that mounts the provider with `?key=` and a fetch mock that throws if /session is probed before /login, and asserts the RelayProvider receives the URL identity (not any cookie-derived one). - observer-auto-login.ts: a "ORDERING IS LOAD-BEARING" doc comment on `resolveObserverSession` explaining exactly why URL-key POST must come before the session GET. No production code changes. Empirical verification: headless Chrome + `next dev` with a mock engine, cookies seeded to a revoked token, navigate to /observer?key=. The login POST fires, Set-Cookie overwrites the stale jar, /session returns the fresh identity, dashboard renders. Same result before and after these changes. Co-Authored-By: Claude Opus 4.7 (1M context) Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82 --- .../components/RelaySessionProvider.test.tsx | 62 +++++++++++++++++ .../src/lib/observer-auto-login.test.ts | 69 +++++++++++++++++++ .../src/lib/observer-auto-login.ts | 9 +++ 3 files changed, 140 insertions(+) diff --git a/packages/observer-dashboard/src/components/RelaySessionProvider.test.tsx b/packages/observer-dashboard/src/components/RelaySessionProvider.test.tsx index 1a6b8a15..2a721eb0 100644 --- a/packages/observer-dashboard/src/components/RelaySessionProvider.test.tsx +++ b/packages/observer-dashboard/src/components/RelaySessionProvider.test.tsx @@ -126,6 +126,68 @@ describe('RelaySessionProvider auto-login from ?key=', () => { }); }); + it('URL key still takes precedence when the browser has stale session cookies (returning-user shared-link scenario)', async () => { + // Repro guard for "shared observer links fail on any browser that has + // ever logged in before". The browser carries stale httpOnly cookies for + // a revoked session; if the provider probed /session first it would 401 + // and bounce to /login without ever POSTing the URL key. The mock below + // fails the test loudly if the provider ever calls /session before the + // URL-key login POST. + setSearchParams('key=ot_live_shared_link_token'); + + const callOrder: string[] = []; + const fetchMock = vi.fn(async (input, init) => { + const url = typeof input === 'string' ? input : (input as Request).url; + if (url.endsWith('/observer/api/auth/session') && callOrder.length === 0) { + // Simulated stale-cookies path: /session probed with no prior /login + // would return 401 in production. Surface this as a test failure so + // an ordering regression is caught here, not in the field. + throw new Error( + '/session probed before /login — URL key must take precedence over stale cookies', + ); + } + if (url.endsWith('/observer/api/auth/login') && init?.method === 'POST') { + callOrder.push('login'); + return jsonResponse({ success: true }); + } + if (url.endsWith('/observer/api/auth/session')) { + callOrder.push('session'); + // Post-login /session returns the fresh identity the login route + // just installed (login's Set-Cookie overwrote the stale cookies). + return jsonResponse({ + authenticated: true, + apiKey: 'ot_live_shared_link_token', + agentToken: 'ot_live_shared_link_token', + wsToken: 'ot_live_shared_link_token', + baseUrl: 'https://cast.agentrelay.com', + }); + } + throw new Error(`Unexpected fetch to ${url}`); + }); + vi.stubGlobal('fetch', fetchMock); + + await act(async () => { + render( + +
dashboard
+ , + ); + }); + + await waitFor(() => { + expect(screen.getByTestId('relay-provider')).toBeTruthy(); + }); + // The RelayProvider must receive the FRESH URL-supplied identity, not any + // remnant of the (revoked) cookie session. + expect(screen.getByTestId('relay-provider').getAttribute('data-api-key')).toBe( + 'ot_live_shared_link_token', + ); + // Ordering is load-bearing. + expect(callOrder).toEqual(['login', 'session']); + // Must not bounce to /login mid-flow. + expect(routerReplace).not.toHaveBeenCalledWith('/login'); + }); + it('bounces to /login when the URL key is rejected by /login', async () => { setSearchParams('key=ot_live_bad'); diff --git a/packages/observer-dashboard/src/lib/observer-auto-login.test.ts b/packages/observer-dashboard/src/lib/observer-auto-login.test.ts index f7123170..62ba805a 100644 --- a/packages/observer-dashboard/src/lib/observer-auto-login.test.ts +++ b/packages/observer-dashboard/src/lib/observer-auto-login.test.ts @@ -267,4 +267,73 @@ describe('resolveObserverSession', () => { expect(outcome).toEqual({ kind: 'unauthenticated', reason: 'error' }); }); + + it('shared observer link auto-logs-in even when the browser already has stale cookies for a revoked session', async () => { + // Repro guard for the "shared link fails on returning browsers" report: + // the browser has cookies from a prior (now-revoked) session, so a naive + // session-first flow would 401 and bounce to /login without ever using + // the URL key. The URL key MUST take precedence — we assert the login + // POST fires first (with the fresh URL key), the follow-up session GET + // returns the *new* identity (because login overwrote the cookies), and + // no unauthenticated outcome slips through. + // + // We simulate the request sequence directly: fetch #1 is the login POST + // (must fire before any session probe), fetch #2 is the session GET + // (must see the new identity established by login). + const callOrder: string[] = []; + const fetchImpl = vi.fn(async (input) => { + const url = typeof input === 'string' ? input : (input as Request).url; + if (url === '/observer/api/auth/login') { + callOrder.push('login'); + // Login accepts the URL key and (in production) overwrites the stale + // cookies via Set-Cookie. Signal success back to the resolver. + return jsonResponse({ success: true }); + } + if (url === '/observer/api/auth/session') { + if (callOrder.length === 0) { + // If, hypothetically, session were probed BEFORE login, the stale + // cookies would come back as 401. Fail the test loudly so any future + // ordering regression is caught here rather than in production. + throw new Error( + 'session probed before login — URL key must take precedence over stale cookies', + ); + } + callOrder.push('session'); + // Post-login session probe. In production the browser now carries + // the fresh cookies; we return the fresh identity that the login + // route just installed. + return jsonResponse({ + authenticated: true, + apiKey: 'ot_live_fresh_from_url', + agentToken: 'ot_live_fresh_from_url', + wsToken: 'ot_live_fresh_from_url', + baseUrl: 'https://cast.agentrelay.com', + }); + } + throw new Error(`unexpected fetch ${url}`); + }); + + const outcome = await resolveObserverSession({ + keyParam: 'ot_live_fresh_from_url', + fetchImpl: fetchImpl as unknown as typeof fetch, + }); + + // The URL key wins over the (stale) cookie session. + expect(outcome).toEqual({ + kind: 'authenticated', + consumedKeyParam: true, + session: { + apiKey: 'ot_live_fresh_from_url', + agentToken: 'ot_live_fresh_from_url', + wsToken: 'ot_live_fresh_from_url', + baseUrl: 'https://cast.agentrelay.com', + }, + }); + + // Ordering is load-bearing: login FIRST, then session. Reversing this + // is the exact regression the report describes. + expect(fetchImpl.mock.calls[0]?.[0]).toBe('/observer/api/auth/login'); + expect(fetchImpl.mock.calls[1]?.[0]).toBe('/observer/api/auth/session'); + expect(fetchImpl).toHaveBeenCalledTimes(2); + }); }); diff --git a/packages/observer-dashboard/src/lib/observer-auto-login.ts b/packages/observer-dashboard/src/lib/observer-auto-login.ts index e21764b7..8956c94e 100644 --- a/packages/observer-dashboard/src/lib/observer-auto-login.ts +++ b/packages/observer-dashboard/src/lib/observer-auto-login.ts @@ -59,6 +59,15 @@ export function isAcceptedObserverKey(value: string | null | undefined): value i * `RelayProvider` props. When there is no key, skip straight to the session * fetch (existing cookie flow). Any failure lands in `{kind: 'unauthenticated'}` * so the caller can redirect to `/login`. + * + * ORDERING IS LOAD-BEARING: a valid URL `?key=` MUST hit `/login` before + * `/session`. Reversing the order breaks the "shared observer link on a + * returning browser" flow — the returning browser carries cookies from a + * prior (possibly revoked) session, so a session-first probe would 401 and + * bounce to `/login` without ever using the URL key. The URL key must take + * precedence over any existing cookies. Guarded by + * `observer-auto-login.test.ts` (unit) and `RelaySessionProvider.test.tsx` + * (integration). */ export async function resolveObserverSession( opts: ResolveObserverSessionOptions,