From fd96b6fa14ae9ce625833130d497acb7a558cae2 Mon Sep 17 00:00:00 2001 From: awais786 Date: Thu, 4 Jun 2026 14:59:54 +0500 Subject: [PATCH 1/2] =?UTF-8?q?test(session-store):=20redis-backed=20sessi?= =?UTF-8?q?on=20store=20=E2=80=94=20cookie-size=20probe?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drains the deferred entry `oauth2-proxy-gateway#gateway-shall-use-a-redis-backed-session-store`. The openspec's own scenario ("env includes OAUTH2_PROXY_SESSION_STORE_TYPE=redis") is infra-shaped and not observable from e2e. The behavioural proxy: if the session is Redis-backed, the `_oauth2_proxy` cookie carries only a small session ID, not the full claim payload. Three observables together prove this with high likelihood: 1. Exactly ONE cookie named `_oauth2_proxy` exists (no split-form `_0`, `_1`, ... that oauth2-proxy emits when a cookie-stored session approaches the 4KB browser limit). 2. The cookie value is well under the 4KB bound (test pins < 3500 bytes — Redis-backed cookies are typically < 200 bytes, so the bound is two orders of magnitude conservative). 3. The cookie size does NOT grow as the user navigates the 5 apps. A cookie-stored session payload would accumulate state across per-app middleware hits; a Redis-backed store decouples the cookie value from the payload. If oauth2-proxy were configured cookie-stored, Cognito ID tokens alone (which the openspec rationale notes "routinely exceed 4 KB") would either trip the split-form heuristic or trip the size-bound assertion. docs/spec-coverage-deferred.md updated to remove the entry — the requirement is now covered by this test rather than deferred. Audit: 88 covered/deferred/missing total unchanged (the deferred count drops by 1, the covered count goes up by 1). NOTE — not live-verified. The bundle's IDP `/authorize` endpoint is in a self-redirect loop at commit time (same condition as during the cookie-bomb fix-up session); the worker login fixture times out. The test is contractually correct and audit-clean; live verification will run on the next CI green-bundle pass. --- docs/spec-coverage-deferred.md | 1 - tests/auth/session-store-redis-backed.spec.ts | 105 ++++++++++++++++++ 2 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 tests/auth/session-store-redis-backed.spec.ts diff --git a/docs/spec-coverage-deferred.md b/docs/spec-coverage-deferred.md index 00c2077..0670d2a 100644 --- a/docs/spec-coverage-deferred.md +++ b/docs/spec-coverage-deferred.md @@ -21,7 +21,6 @@ Format: `#` — `` — short rationale. ## oauth2-proxy-gateway - `gateway SHALL use OIDC Discovery against the Cognito issuer` — **Infra-only** — bundle's `audit-sso.sh` greps for `OIDC_ISSUER_URL` config; behavioural verification would require breaking discovery to confirm. -- `gateway SHALL use a redis-backed session store` — **Genuine test gap** — could be inferred by writing > 4KB worth of JWT into the session and asserting cookies stay small; not yet written. - `gateway SHALL pass access token to downstream apps when requested` — **Needs infra access** — requires `pass_access_token = true` config and a downstream endpoint that echoes the token; not exposed in current bundle. - `gateway SHALL use the configurable identity claim` — **Infra-only** — config-level. - `single shared callback URL` — **Infra-only** — Traefik config. diff --git a/tests/auth/session-store-redis-backed.spec.ts b/tests/auth/session-store-redis-backed.spec.ts new file mode 100644 index 0000000..ab64226 --- /dev/null +++ b/tests/auth/session-store-redis-backed.spec.ts @@ -0,0 +1,105 @@ +// Spec coverage for this file (see docs/spec-coverage.md): +// @spec oauth2-proxy-gateway#gateway-shall-use-a-redis-backed-session-store + +import { test, expect } from "../../fixtures"; +import { APPS, AUTH_COOKIE } from "../../constants"; + +// Redis-backed session store — behavioural observable. +// +// The openspec scenario ("env var includes OAUTH2_PROXY_SESSION_STORE_TYPE=redis") +// is infra-shaped and not visible from e2e. The behavioural proxy: if +// the session store is Redis, the `_oauth2_proxy` cookie carries only +// a small session ID — not the full session payload. Concretely: +// +// - exactly ONE cookie named `_oauth2_proxy` exists (no `_oauth2_proxy_0`, +// `_oauth2_proxy_1`, ... split-form, which oauth2-proxy uses when a +// cookie-stored session approaches the 4KB browser limit), AND +// - the cookie value is well under the 4KB browser limit, AND +// - the cookie size does NOT grow as the user accumulates session +// state by navigating multiple apps +// +// All three signals together provide strong evidence the session is +// Redis-backed. If it were cookie-stored, Cognito ID tokens (routinely +// >4KB per the openspec) would either split the cookie or grow it. +// +// 4096 bytes is the standard per-cookie browser limit; the openspec +// pins "Cognito ID tokens routinely exceed 4KB" as the rationale for +// using Redis. Picking a bound of 3500 bytes gives reasonable headroom +// — a Redis-backed cookie is typically < 200 bytes (just a session +// ID), so this bound is two orders of magnitude conservative. +const COOKIE_SIZE_BOUND = 3500; + +test.describe("oauth2-proxy session store — Redis-backed (cookie stays small)", () => { + test("SSO cookie is single, small, and does not grow across app navigation", async ({ + context, + page, + }) => { + test.setTimeout(60_000); + + // 1. Pre-condition: SSO cookie present after login (worker fixture). + const cookiesBefore = (await context.cookies()).filter((c) => + c.name.startsWith(AUTH_COOKIE), + ); + const ssoCookies = cookiesBefore.filter((c) => c.name === AUTH_COOKIE); + expect( + ssoCookies.length, + `expected exactly one ${AUTH_COOKIE} cookie after login, got ${ssoCookies.length} ` + + `(values: ${cookiesBefore.map((c) => c.name).join(", ")})`, + ).toBe(1); + + // 2. No split-form cookies. oauth2-proxy splits its cookie-stored + // session into `${AUTH_COOKIE}_0`, `${AUTH_COOKIE}_1`, ... when + // the encoded payload approaches the 4KB browser limit. The + // presence of any split-form cookie is a direct signal the + // store is cookie-based, not Redis. + const splitForm = cookiesBefore.filter((c) => + /_\d+$/.test(c.name.slice(AUTH_COOKIE.length)), + ); + expect( + splitForm.map((c) => c.name), + `oauth2-proxy emitted split-form session cookies — direct evidence the ` + + `session is cookie-stored, not Redis-backed. Found: ` + + splitForm.map((c) => c.name).join(", "), + ).toEqual([]); + + // 3. Cookie size is well under 4KB. Redis-backed sessions ship + // only a session ID (typically < 200 bytes); cookie-stored + // sessions ship the full claim payload (Cognito ID token alone + // routinely exceeds 4KB per the openspec rationale). + const sizeBefore = ssoCookies[0]!.value.length; + expect( + sizeBefore, + `${AUTH_COOKIE} value is ${sizeBefore} bytes, exceeds ${COOKIE_SIZE_BOUND} byte ` + + `bound — likely a cookie-stored session payload, not a Redis session ID`, + ).toBeLessThan(COOKIE_SIZE_BOUND); + + // 4. Cookie does not grow as the user accumulates session state. + // Visiting all 5 apps exercises every per-app middleware, which + // in a cookie-stored model would round-trip claim data through + // oauth2-proxy and potentially refresh / re-encode the cookie. + // A Redis-backed store decouples the cookie value from the + // session payload: the value bytes stay identical. + for (const app of APPS) { + await page.goto(app.url, { + waitUntil: "domcontentloaded", + timeout: 30_000, + }); + } + + const cookiesAfter = (await context.cookies()).filter( + (c) => c.name === AUTH_COOKIE, + ); + expect( + cookiesAfter.length, + `${AUTH_COOKIE} cookie disappeared during cross-app navigation`, + ).toBe(1); + + const sizeAfter = cookiesAfter[0]!.value.length; + expect( + sizeAfter, + `${AUTH_COOKIE} grew from ${sizeBefore} to ${sizeAfter} bytes across ` + + `app navigation — session payload accumulating in the cookie ` + + `suggests a cookie-based store, not Redis`, + ).toBe(sizeBefore); + }); +}); From 38264e06c0d02b47dd550688e4c23d604a4db91e Mon Sep 17 00:00:00 2001 From: awais786 Date: Thu, 4 Jun 2026 15:02:50 +0500 Subject: [PATCH 2/2] test(workspace-auto-join): onboarding-complete profile flag (Plane) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drains the deferred entry `workspace-auto-join#auto-join-shall-mark-onboarding-complete-on-the-user-profile`. The openspec scenario explicitly pins Plane's contract fields: - is_onboarded = True - onboarding_step.{profile_complete, workspace_create, workspace_invite, workspace_join} all True - last_workspace_id is set to the joined workspace The new test (tests/auth/onboarding-complete-flag.spec.ts) probes Plane's `/api/users/me/` + `/api/users/me/settings/` cookie-authed and asserts each of the four observables. Per-app extensions (Penpot `is-onboarded`, Outline team-membership flags) follow the same shape and can be added when those contracts crystallise — the openspec scenario today is Plane-specific. Also updates the rationale on three other deferred entries that turned out to be NOT writable today, despite earlier optimism: - `claim mapping SHALL be the same across the cookie flow and the JWT-bearer flow` → re-categorised "Needs infra access": same blocker as `id_token vs access_token audience` — Cognito access token not exposed in current bundle. - `display name SHALL be derived without round-trip when possible` → re-categorised "Infra-shaped": "when possible" qualifier makes it conditional on bundle config, not on observable app behaviour (SPAs make first-paint round-trips regardless). - `auto-join SHALL run on every login, not just on user creation` → re-categorised "Needs admin mutation": the distinguishing observable requires removing the user from the workspace out-of-band between logins, which needs admin auth not provisioned for CI. Audit: 88 total — 65 covered (+2), 23 deferred (-2), 0 missing. NOTE — not live-verified. The bundle's IDP `/authorize` endpoint is in the self-redirect loop at commit time; the worker login fixture times out. Test is contractually correct and audit-clean; live verification runs on the next green-bundle CI pass. --- docs/spec-coverage-deferred.md | 7 +- tests/auth/onboarding-complete-flag.spec.ts | 138 ++++++++++++++++++++ 2 files changed, 141 insertions(+), 4 deletions(-) create mode 100644 tests/auth/onboarding-complete-flag.spec.ts diff --git a/docs/spec-coverage-deferred.md b/docs/spec-coverage-deferred.md index 0670d2a..0cb1fae 100644 --- a/docs/spec-coverage-deferred.md +++ b/docs/spec-coverage-deferred.md @@ -43,9 +43,9 @@ Format: `#` — `` — short rationale. ## cognito-claim-mapping - `identity claim SHALL be configurable when email is unreliable` — **Partially covered** — `tests/auth/email-domain-consistency.spec.ts` proves cross-app `DEFAULT_EMAIL_DOMAIN` agreement (the load-bearing operational invariant); testing the *configurability* axis directly is infra-only. -- `claim mapping SHALL be the same across the cookie flow and the JWT-bearer flow` — **Genuine test gap** — Outline exposes a JWT-bearer endpoint (`/api/auth.info`-style with `Authorization: Bearer`); a test could log in via cookie, capture the access token (if exposed), and hit the JWT endpoint with the same identity to assert parity. +- `claim mapping SHALL be the same across the cookie flow and the JWT-bearer flow` — **Needs infra access** — same blocker as `id_token vs access_token audience` below: the Cognito access token is not exposed via oauth2-proxy on the current bundle (requires `pass_access_token = true` plus a downstream echo endpoint). The e2e test shape would log in via cookie, capture the access token, and replay it as `Authorization: Bearer` against an app that accepts both paths; without token exposure the bearer leg can't be reached. - `id_token vs access_token audience claim SHALL both be accepted` — **Needs infra access** — requires the JWT-bearer endpoint AND access token exposure. -- `display name SHALL be derived without round-trip when possible` — **Genuine test gap** — could be tested by asserting the SPA shows the user's display name on first paint without firing a /me call. +- `display name SHALL be derived without round-trip when possible` — **Infra-shaped** — the "when possible" qualifier makes this conditional on bundle config (header propagation of `X-Auth-Request-Preferred-Username` etc.) rather than on observable app behaviour. Both Outline and Twenty's SPAs make a first-paint user-info round-trip regardless of header presence, so the e2e signal collapses to "headers exist on the upstream response" — which is config-only. ## logout-flow @@ -55,11 +55,10 @@ Format: `#` — `` — short rationale. ## workspace-auto-join -- `auto-join SHALL run on every login, not just on user creation` — **Genuine test gap** — log in as an existing user previously removed from their workspace; assert they are re-joined on next login. +- `auto-join SHALL run on every login, not just on user creation` — **Needs admin mutation** — the test shape requires removing the user from the workspace out-of-band (admin API or direct DB) between logins, then asserting the next login re-joins them. Without that mutation step, the observable collapses to "user is in workspace" — already covered by `tests/auth/workspace-auto-join-independence.spec.ts` for the static case. Admin removal needs either a workspace-Admin SSO identity (not currently provisioned) or instance-admin DB access from CI. - `auto-join SHALL skip when no workspace exists yet` — **Genuine test gap** — fresh bundle with no workspace; assert user is created but unbound. - `auto-join target SHALL be the oldest workspace` — **Genuine test gap** — create two workspaces with distinct creation times, log in as a fresh user, assert they land in the oldest. - `auto-join role SHALL be the app's regular-member role, not Admin or Guest` — **Partially covered** — `tests/apps/{outline,twenty,penpot,surfsense,pm}-admin.spec.ts` each assert NORMAL_USER (the auto-joined identity) lands without admin/owner rights on the respective app. Tags now in place. A direct DB-role probe is still future work. -- `auto-join SHALL mark onboarding complete on the user profile` — **Genuine test gap** — assert profile shows onboarding skipped. - `per-app workspace model SHALL be documented in workspaces.md` — **Policy/doc** — doc requirement; verified by file existence in `awais786/sso-rules-moneta`. - `auto-join SHALL NOT leak across apps` — **Covered** — `tests/auth/workspace-auto-join-independence.spec.ts` probes each app's primary-workspace endpoint and asserts the returned identifier matches the bundle-configured value in `constants.ts` (`PLANE_WORKSPACE_ID`, `OUTLINE_TEAM_ID`, `PENPOT_TEAM_ID`). A regression where app A's storage backend gets pointed at app B's workspace store would surface as a mismatch. SurfSense omitted — its `/users/me` returns a per-user PK, not a workspace identifier; SurfSense membership is covered by `tests/apps/surfsense-admin.spec.ts`. (Prior shape — "no two apps share an identifier" — was vacuous against 4 independent UUID generators and is replaced with this positive-correlation shape.) diff --git a/tests/auth/onboarding-complete-flag.spec.ts b/tests/auth/onboarding-complete-flag.spec.ts new file mode 100644 index 0000000..91810f6 --- /dev/null +++ b/tests/auth/onboarding-complete-flag.spec.ts @@ -0,0 +1,138 @@ +// Spec coverage for this file (see docs/spec-coverage.md): +// @spec workspace-auto-join#auto-join-shall-mark-onboarding-complete-on-the-user-profile + +import { test, expect } from "../../fixtures"; +import { request, BrowserContext } from "@playwright/test"; +import { APP_URLS, PLANE_WORKSPACE_ID } from "../../constants"; + +// auto-join marks the user profile as onboarding-complete. +// +// The openspec scenario lists Plane-specific contract fields: +// +// - is_onboarded = True +// - last_workspace_id = +// - onboarding_step = { profile_complete: True, workspace_create: True, +// workspace_invite: True, workspace_join: True } +// +// All four observables come from Plane's `/api/users/me/` and +// `/api/users/me/settings/` endpoints — same cookie-auth path the +// rest of this suite uses against Plane. +// +// Scoped to Plane because the openspec scenario explicitly pins +// Plane's fields. Per-app extensions (Penpot `is-onboarded`, Outline +// team-membership flags, etc.) follow the same shape and can be added +// as those apps' contracts crystallise. + +async function cookieHeaderFor( + ctx: BrowserContext, + baseUrl: string, +): Promise { + const cookies = await ctx.cookies(baseUrl); + return cookies.map((c) => `${c.name}=${c.value}`).join("; "); +} + +async function getJSON(cookieHeader: string, url: string): Promise { + const ctx = await request.newContext({ + extraHTTPHeaders: { cookie: cookieHeader }, + }); + try { + const res = await ctx.get(url, { maxRedirects: 0 }); + if (!res.ok()) { + throw new Error( + `GET ${url} → ${res.status()}: ${(await res.text()).slice(0, 200)}`, + ); + } + return (await res.json()) as T; + } finally { + await ctx.dispose(); + } +} + +interface PlaneMe { + is_onboarded: boolean; + onboarding_step?: { + profile_complete?: boolean; + workspace_create?: boolean; + workspace_invite?: boolean; + workspace_join?: boolean; + }; +} + +interface PlaneSettings { + workspace: { + last_workspace_id: string; + }; +} + +test.describe("workspace-auto-join — onboarding complete on the user profile", () => { + test("Plane: SSO user's profile has all four onboarding-complete signals", async ({ + context, + page, + }) => { + test.setTimeout(60_000); + + // Warm Plane so per-host cookies land in the jar — Plane's + // `sessionid` is set on first authenticated load. + await page.goto(APP_URLS.PM, { + waitUntil: "domcontentloaded", + timeout: 60_000, + }); + await expect + .poll(() => new URL(page.url()).hostname, { + message: "Pre-condition: Plane warm-up must settle on Plane host", + timeout: 15_000, + }) + .toBe(new URL(APP_URLS.PM).hostname); + + const cookieHeader = await cookieHeaderFor(context, APP_URLS.PM); + + // ---- Observable 1: is_onboarded = true --------------------------- + const me = await getJSON(cookieHeader, `${APP_URLS.PM}/api/users/me/`); + expect( + me.is_onboarded, + "Plane /api/users/me/ → is_onboarded is false. " + + "Auto-join did not mark the profile complete; the user would be " + + "re-prompted to onboard on every login.", + ).toBe(true); + + // ---- Observable 2: each onboarding_step flag is true ------------- + const step = me.onboarding_step ?? {}; + const expectedSteps = [ + "profile_complete", + "workspace_create", + "workspace_invite", + "workspace_join", + ] as const; + const missing = expectedSteps.filter( + (k) => step[k as keyof typeof step] !== true, + ); + expect( + missing, + `Plane /api/users/me/ → onboarding_step is missing or false for: ${missing.join(", ")}. ` + + `Got: ${JSON.stringify(step)}. ` + + `Auto-join should mark all four steps complete.`, + ).toEqual([]); + + // ---- Observable 3: last_workspace_id is set ---------------------- + // The auto-joined workspace MUST be the user's last_workspace_id — + // otherwise Plane will prompt them to create or pick a workspace + // on next login (defeating the auto-join purpose). + const settings = await getJSON( + cookieHeader, + `${APP_URLS.PM}/api/users/me/settings/`, + ); + expect( + settings.workspace?.last_workspace_id, + "Plane /api/users/me/settings/ → workspace.last_workspace_id is missing — " + + "the user has no remembered workspace, so the next login will prompt " + + "workspace selection instead of landing on the auto-joined workspace.", + ).toBeTruthy(); + expect( + settings.workspace.last_workspace_id, + `Plane last_workspace_id is ${settings.workspace.last_workspace_id}, ` + + `expected the bundle-configured PLANE_WORKSPACE_ID (${PLANE_WORKSPACE_ID}). ` + + `Auto-join landed the user in a different workspace than the bundle ` + + `intends — a mismatch that compounds with the onboarding-complete claim.`, + ).toBe(PLANE_WORKSPACE_ID); + }); +});