refactor(#276): extract ConnectionSession — phase 2

CHANGELOG.md

@@ -10,6 +10,18 @@ auto-generated per-PR notes; this file is the curated, human-readable history.
 ## [Unreleased]
 
 ### Changed
+- **Authentication and connection lifecycle extracted into `ConnectionSession`**
+  (#276 Phase 2). OAuth PKCE login/refresh, Basic-auth probing, IdP config
+  resolution, identity, sign-in/out, and the cross-tab dashboard auth handoff
+  now live in `src/application/connection-session.ts` — constructible without
+  `App`/`AppState`/DOM, with rendering inverted through an injected
+  `onAuthLost` shell callback (the session never renders or toasts). The app
+  exposes it as `app.conn`; `app.chCtx` remains the same single live ClickHouse
+  context object (now session-owned). The scalar auth fields
+  (`token`/`refreshToken`/`authMode`/`chAuth`/`basicUserClaim`/`idpId`) left
+  the `App` contract; view/bootstrap-consumed members stay as one-line
+  delegates slated for removal in the issue's Phase 5. Behavior, storage keys,
+  handoff message pinning, and error strings are byte-identical.
 - **Query execution extracted into an application service** (#276 Phases 0–1,
   the first step of the app.ts → services refactor). The shared
   request/stream/normalize core (`runReadInto`) and the multiquery-script

src/core/jwt.ts

@@ -4,7 +4,8 @@
 
 /** A decoded JWT payload — `exp` (seconds since epoch) is the one claim this
  *  module reads itself; other claims (email/preferred_username/sub/…) pass
- *  through untyped for `app.js`'s `chUsername`. */
+ *  through untyped for the ConnectionSession's `chUsername`
+ *  (`application/connection-session.ts`, #276 Phase 2). */
 export interface JwtPayload {
   exp?: number;
   [key: string]: unknown;

src/core/pkce.ts

@@ -7,13 +7,13 @@
  *  `ArrayBuffer`-backed form (`new Uint8Array(n)`'s actual type) since
  *  lib.dom's own `Crypto.getRandomValues` requires exactly that, not the
  *  wider `ArrayBufferLike` (which also admits `SharedArrayBuffer`). */
-interface RandomBytesSource {
+export interface RandomBytesSource {
   getRandomValues(array: Uint8Array<ArrayBuffer>): Uint8Array<ArrayBuffer>;
 }
 
 /** The minimal Web Crypto surface `generatePKCE` uses — real Web Crypto (the
  *  global `crypto`, browser or Node's `webcrypto`) or an injectable stub. */
-interface PkceCrypto extends RandomBytesSource {
+export interface PkceCrypto extends RandomBytesSource {
   subtle: { digest(algorithm: string, data: BufferSource): Promise<ArrayBuffer> };
 }
 

src/ui/app.types.ts

@@ -15,6 +15,7 @@ import type { CodeViewerFactory } from '../editor/code-viewer.types.js';
 import type { QueryTab as Tab, AppState as State, SpecValidationService } from '../state.js';
 import type { ConfigDoc, ResolvedIdpConfig } from '../net/oauth-config.js';
 import type { QueryExecutionService } from '../application/query-execution-service.js';
+import type { ConnectionSession, SessionChCtx } from '../application/connection-session.js';
 import type { SpecValidatorFn } from '../core/spec-draft.js';
 import type { SavedQueryV2 } from '../generated/json-schema.types.js';
 import type { DynamicSources } from '../core/spec-completion.js';
@@ -100,15 +101,14 @@ export interface AppDom {
   varStripDeferHooked?: boolean;
 }
 
-export interface ChCtx {
-  fetch: typeof fetch;
-  origin: string;
-  authConfirmed: boolean;
-  getToken(): Promise<string | null>;
-  refresh(): Promise<boolean>;
-  authHeader(token: string): string;
-  onSignedOut(detail?: string): void;
-}
+/** The live ClickHouse auth context every query call site reads/mutates —
+ * a structural alias of `application/connection-session.ts`'s own
+ * `SessionChCtx` (the session is the one place that constructs and mutates
+ * it now, #276 Phase 2) rather than a second, independently-maintained
+ * copy of the same shape. `src/ui/**` may depend on `src/application/**`,
+ * never the reverse — connection-session.ts redeclares this shape itself
+ * rather than importing it from here. */
+export type ChCtx = SessionChCtx;
 
 export interface ActionsRegistry {
   run(opts?: Json): void | Promise<void>;
@@ -167,8 +167,15 @@ export interface App {
   hardenedVars: Set<string>;
   root: Element | null;
   document: Document;
-  token: string | null;
-  refreshToken: string | null;
+
+  /** The auth + config + ClickHouse connection lifecycle (#276 Phase 2) —
+   *  OAuth PKCE login/refresh, Basic probing, IdP config resolution, and
+   *  cross-tab auth handoff, constructible without App/AppState/DOM
+   *  (`src/application/connection-session.ts`). The identity/auth members
+   *  below (`isSignedIn`/`email`/`host`/…) are Phase-2 delegates onto this —
+   *  shells/bootstrap consume those; a future phase re-points them to
+   *  `app.conn` directly. */
+  conn: ConnectionSession;
 
   // Editor ports (injected seams — #143/#212).
   sqlEditor: EditorPort;
@@ -196,16 +203,16 @@ export interface App {
   /** Ad-hoc, consumer-attached (chart-render.js), not initialized by createApp. */
   chart?: { destroy(): void };
 
-  // Identity / auth.
+  // Identity / auth — Phase-2 delegates onto `conn` (see its doc comment
+  // above). `authMode`/`chAuth`/`basicUserClaim`/`idpId`/`selectIdp`/
+  // `chUsername` moved onto `app.conn` itself (`conn.authMode()`/
+  // `conn.chAuth()`/`conn.basicUserClaim()`/`conn.idpId()`/`conn.selectIdp()`)
+  // — no other module read them off `App` directly (verified #276 Phase 2),
+  // so they aren't re-delegated here.
   host(): string;
   activeTab(): Tab;
   isSignedIn(): boolean;
   email(): string;
-  chUsername(jwtPayload: Json): string;
-  authMode: 'oauth' | 'basic';
-  chAuth: 'bearer' | 'basic';
-  basicUserClaim: string;
-  idpId: string | null;
   hostHint: string;
   basePath: string;
   setTokens(id: string, refresh?: string): void;
@@ -219,7 +226,6 @@ export interface App {
    * not just `{id}`); login.ts read the real shape all along behind its own
    * `LoginIdpsResult` widening, now redundant (dropped there). */
   loadIdps(): Promise<ConfigDoc>;
-  selectIdp(id: string): void;
   /** Same real shape as `loadConfig` (`null` when config couldn't be loaded —
    * fail-soft, see app.ts's own `ensureConfig`). */
   ensureConfig(): Promise<ResolvedIdpConfig | null>;

tests/helpers/auth-fixtures.ts

@@ -0,0 +1,33 @@
+// Shared auth test fixtures (#276 Phase 2). `jwt()` and `memStorage()` were
+// being hand-copied per spec file (app.test.ts / dashboard.test.ts still carry
+// their own older embedded variants — consolidate them here opportunistically
+// when those files are next touched); new specs import from here instead of
+// adding another copy.
+
+/** A valid-looking JWT (base64url header.payload.sig). `decodeJwtPayload`
+ *  only ever reads segment [1]; the signature is never verified client-side. */
+export function jwt(payload: Record<string, unknown>): string {
+  const b = (o: unknown): string => btoa(JSON.stringify(o)).replace(/\+/g, '-').replace(/\//g, '_').replace(/=+$/, '');
+  return `${b({ alg: 'RS256' })}.${b(payload)}.sig`;
+}
+
+/** A Map-backed sessionStorage fake — the three methods the auth code uses,
+ *  plus the backing `_map` for direct assertions. Structurally satisfies
+ *  `connection-session.ts`'s `SessionStorageLike` (and the narrower
+ *  `core/auth-handoff.js` `StorageLike`). */
+export interface MemStorage {
+  getItem(key: string): string | null;
+  setItem(key: string, value: string): void;
+  removeItem(key: string): void;
+  _map: Map<string, string>;
+}
+
+export function memStorage(initial: Record<string, string> = {}): MemStorage {
+  const m = new Map(Object.entries(initial));
+  return {
+    getItem: (k) => (m.has(k) ? (m.get(k) as string) : null),
+    setItem: (k, v) => { m.set(k, v); },
+    removeItem: (k) => { m.delete(k); },
+    _map: m,
+  };
+}

tests/helpers/fake-app.ts

@@ -30,6 +30,48 @@ import type { StreamResult } from '../../src/core/stream.js';
 import type { ChartJsConfigResult } from '../../src/core/chart-data.js';
 import type { ChartInstance } from '../../src/ui/chart-render.js';
 import type { QueryExecutionService } from '../../src/application/query-execution-service.js';
+import type { ConnectionSession } from '../../src/application/connection-session.js';
+
+// Production invariant (#276 Phase 2): `app.chCtx` and `app.conn.chCtx` are
+// THE SAME live object (app.ts aliases `conn.chCtx`) — the defaults + the
+// makeApp merge below preserve that identity so a fixture mutation through
+// either alias observes shared state, exactly like the real app.
+const chCtxDefaults: ChCtx = {
+  fetch, origin: '', authConfirmed: true,
+  getToken: async () => null, refresh: async () => false, authHeader: () => '', onSignedOut: () => {},
+};
+
+// A minimal `ConnectionSession` stub (#276 Phase 2) — every render-module test
+// exercises `app.conn` only incidentally (through the `App`-level auth
+// delegates it backs, e.g. `isSignedIn`/`email`/`ensureConfig`), never
+// directly; this is inert, never-called-by-default plumbing so `appDefaults`
+// still satisfies the full `App` contract.
+const connDefaults: ConnectionSession = {
+  basePath: '',
+  hostHint: '',
+  chCtx: chCtxDefaults,
+  token: () => null,
+  refreshToken: () => null,
+  authMode: () => 'basic',
+  idpId: () => null,
+  chAuth: () => 'basic',
+  basicUserClaim: () => 'sub',
+  isSignedIn: () => true,
+  email: () => '',
+  host: () => '',
+  loadIdps: async () => ({ idps: [], basicLogin: true, hosts: [] }) as ConfigDoc,
+  selectIdp: () => {},
+  resolveConfig: async () => ({}) as ResolvedIdpConfig,
+  ensureConfig: async () => null,
+  setTokens: () => {},
+  getToken: async () => null,
+  beginOAuth: async () => {},
+  connectBasic: async () => {},
+  signOut: () => {},
+  ensureFreshToken: async () => true,
+  grantHandoffTo: () => {},
+  receiveAuthHandoff: async () => false,
+};
 
 // A stand-in for the Chart.js constructor: records its canvas + config and
 // exposes a destroy() spy, so the chart glue is testable without a real
@@ -68,8 +110,7 @@ const appDefaults: App = {
   build: 'v0.0.0-test',
   root: null,
   document,
-  token: null,
-  refreshToken: null,
+  conn: connDefaults,
   sqlEditor: {} as App['sqlEditor'],
   specEditor: {} as App['specEditor'],
   CodeViewer: () => ({ setText: () => {}, setLanguage: () => {}, setWrap: () => {}, focus: () => {}, destroy: () => {} }),
@@ -87,23 +128,14 @@ const appDefaults: App = {
   activeTab: () => ({}) as App['activeTab'] extends () => infer T ? T : never,
   isSignedIn: () => true,
   email: () => '',
-  chUsername: () => '',
-  authMode: 'basic',
-  chAuth: 'basic',
-  basicUserClaim: 'sub',
-  idpId: null,
   hostHint: '',
   basePath: '',
   setTokens: () => {},
   loadConfig: async () => ({}) as ResolvedIdpConfig,
   loadIdps: async () => ({ idps: [], basicLogin: true, hosts: [] }) as ConfigDoc,
-  selectIdp: () => {},
   ensureConfig: async () => null,
   ensureFreshToken: async () => true,
-  chCtx: {
-    fetch, origin: '', authConfirmed: true,
-    getToken: async () => null, refresh: async () => false, authHeader: () => '', onSignedOut: () => {},
-  },
+  chCtx: chCtxDefaults,
   showLogin: () => {},
   signOut: () => {},
   receiveAuthHandoff: async () => false,
@@ -169,14 +201,19 @@ const appDefaults: App = {
  * onSignedOut }` (a caller overriding one ChCtx member, not the whole
  * service) would otherwise fail the constraint even though the nested merge
  * below happily accepts a partial sub-object. */
-type AppOverrides = Partial<Omit<App, 'dom' | 'chCtx' | 'actions' | 'exec'>> & {
+type AppOverrides = Partial<Omit<App, 'dom' | 'chCtx' | 'actions' | 'exec' | 'conn'>> & {
   dom?: Partial<AppDom>;
   chCtx?: Partial<ChCtx>;
   actions?: Partial<ActionsRegistry>;
   /** Partial like `dom`/`chCtx`/`actions` above — most fixtures only care
    *  about overriding `executeRead` (the #185 detached-read seam / dashboard
    *  tile transport); `executeScript`/`kill` fall back to the base stubs. */
   exec?: Partial<QueryExecutionService>;
+  /** Partial like the rest. `conn.chCtx` cannot be overridden here — the
+   *  merge below always re-points it at the shared merged `chCtx` object
+   *  (the production identity invariant); override `chCtx` at the top level
+   *  and both aliases see it. */
+  conn?: Partial<Omit<ConnectionSession, 'chCtx'>>;
 };
 
 // `overrides` is generic so its properties keep their OWN precise call-site
@@ -297,14 +334,18 @@ export function makeApp<O extends AppOverrides = Record<string, never>>(override
       updateSaveBtn: vi.fn(),
     },
   };
+  // One merged chCtx object shared by BOTH aliases (app.chCtx and
+  // app.conn.chCtx) — the production identity invariant (#276 Phase 2).
+  const chCtx = { ...appDefaults.chCtx, ...base.chCtx, ...(overrides.chCtx ?? {}) };
   const merged = {
     ...appDefaults,
     ...base,
     ...overrides,
     dom: { ...appDefaults.dom, ...base.dom, ...(overrides.dom ?? {}) },
-    chCtx: { ...appDefaults.chCtx, ...base.chCtx, ...(overrides.chCtx ?? {}) },
+    chCtx,
     actions: { ...appDefaults.actions, ...base.actions, ...(overrides.actions ?? {}) },
     exec: { ...appDefaults.exec, ...base.exec, ...(overrides.exec ?? {}) },
+    conn: { ...connDefaults, ...(overrides.conn ?? {}), chCtx },
   };
   // Assignability check only (a variable reference, not a fresh literal, so
   // this never trips an excess-property error) — `merged`'s own inferred type

tests/unit/app.test.ts

@@ -362,7 +362,7 @@ describe('createApp basics', () => {
   });
   it('reads the stored token and derives identity', () => {
     const app = createApp(env());
-    expect(app.token).toBe(validToken);
+    expect(app.conn.token()).toBe(validToken);
     expect(app.isSignedIn()).toBe(true);
     expect(app.email()).toBe('me@example.com');
     expect(app.host()).toBe('ch.example');
@@ -493,7 +493,7 @@ describe('renderApp shell', () => {
     expect(qs(menu, '.um-id').textContent).toBe('me@example.com');
     expect(qs(menu, '.um-build').textContent).toBe(app.build); // build stamp ('dev' here)
     qs(menu, '.um-item.danger').dispatchEvent(new Event('click', { bubbles: true }));
-    expect(app.token).toBeNull();
+    expect(app.conn.token()).toBeNull();
     expect(e.sessionStorage!.getItem('oauth_id_token')).toBeNull();
     expect(qs(app.root, '.login-screen')).not.toBeNull();
     expect(qs(document, '.user-menu')).toBeNull(); // closed
@@ -525,7 +525,7 @@ describe('renderApp shell', () => {
     const e = env({ sessionStorage: memSession({ oauth_verifier: 'v', oauth_state: 's' }) });
     const app = createApp(e);
     app.setTokens('tok');
-    expect(app.token).toBe('tok');
+    expect(app.conn.token()).toBe('tok');
     expect(e.sessionStorage!.getItem('oauth_id_token')).toBe('tok');
     expect(e.sessionStorage!.getItem('oauth_verifier')).toBeNull();
     expect(e.sessionStorage!.getItem('oauth_state')).toBeNull();
@@ -2626,7 +2626,7 @@ describe('auth flows', () => {
     const app = createApp(e);
     const ok = await app.chCtx.refresh();
     expect(ok).toBe(true);
-    expect(app.token).toBe(validToken);
+    expect(app.conn.token()).toBe(validToken);
   });
   it('getToken returns null + clears when refresh fails', async () => {
     const e = env({
@@ -2683,7 +2683,7 @@ describe('credentials (basic) sign-in', () => {
 
   it('restores a basic session from sessionStorage', () => {
     const app = createApp(env({ sessionStorage: memSession(basicSession) }));
-    expect(app.authMode).toBe('basic');
+    expect(app.conn.authMode()).toBe('basic');
     expect(app.isSignedIn()).toBe(true);
     expect(app.email()).toBe('demo');
     expect(app.host()).toBe('gh.example:8443');
@@ -2721,9 +2721,9 @@ describe('credentials (basic) sign-in', () => {
       fetch: makeFetch([[(u, sql) => /SELECT 1/.test(sql), resp({ json: { data: [{ '1': 1 }] } })]]),
     });
     const app = createApp(e);
-    expect(app.authMode).toBe('oauth');
+    expect(app.conn.authMode()).toBe('oauth');
     await app.actions.connect({ username: 'demo', password: 'demo', host: '' });
-    expect(app.authMode).toBe('basic');
+    expect(app.conn.authMode()).toBe('basic');
     expect(e.sessionStorage!.getItem('ch_basic_auth')).toBe(creds);
     expect(e.sessionStorage!.getItem('ch_basic_user')).toBe('demo');
     expect(e.sessionStorage!.getItem('ch_basic_origin')).toBe('https://ch.example');
@@ -2749,15 +2749,15 @@ describe('credentials (basic) sign-in', () => {
     });
     const app = createApp(e);
     await expect(app.actions.connect({ username: 'demo', password: 'wrong', host: '' })).rejects.toThrow();
-    expect(app.authMode).toBe('oauth');
+    expect(app.conn.authMode()).toBe('oauth');
     expect(e.sessionStorage!.getItem('ch_basic_auth')).toBeNull();
   });
   it('signing out of a basic session resets mode, origin, and stored creds', () => {
     const e = env({ sessionStorage: memSession(basicSession) });
     const app = createApp(e);
     app.renderApp();
     app.signOut();
-    expect(app.authMode).toBe('oauth');
+    expect(app.conn.authMode()).toBe('oauth');
     expect(app.chCtx.origin).toBe('https://ch.example');
     expect(e.sessionStorage!.getItem('ch_basic_auth')).toBeNull();
     expect(qs(app.root, '.login-screen')).not.toBeNull();
@@ -3098,7 +3098,7 @@ describe('exhaustive controller coverage', () => {
     });
     const app = createApp(e);
     await app.chCtx.refresh();
-    expect(app.refreshToken).toBe('rt2');
+    expect(app.conn.refreshToken()).toBe('rt2');
     expect(e.sessionStorage!.getItem('oauth_refresh_token')).toBe('rt2');
   });
 
@@ -3319,7 +3319,7 @@ describe('exhaustive controller coverage', () => {
     const app = createApp(e);
     app.renderApp();
     await app.ensureConfig();
-    expect(app.chAuth).toBe('basic');
+    expect(app.conn.chAuth()).toBe('basic');
     app.activeTab().sqlDraft = 'SELECT 1';
     await app.actions.run();
     const q = asMock(e.fetch!).mock.calls.find((c) => c[1] && c[1].body === 'SELECT 1')!;
@@ -3342,7 +3342,7 @@ describe('exhaustive controller coverage', () => {
     const app = createApp(e);
     app.renderApp();
     await app.ensureConfig();
-    expect(app.basicUserClaim).toBe('nickname');
+    expect(app.conn.basicUserClaim()).toBe('nickname');
     app.activeTab().sqlDraft = 'SELECT 1';
     await app.actions.run();
     const q = asMock(e.fetch!).mock.calls.find((c) => c[1] && c[1].body === 'SELECT 1')!;