Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions docs/spec-coverage-deferred.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ Format: `<module>#<requirement>` — `<category>` — 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.
Expand All @@ -44,9 +43,9 @@ Format: `<module>#<requirement>` — `<category>` — 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

Expand All @@ -56,11 +55,10 @@ Format: `<module>#<requirement>` — `<category>` — 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.)

Expand Down
138 changes: 138 additions & 0 deletions tests/auth/onboarding-complete-flag.spec.ts
Original file line number Diff line number Diff line change
@@ -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 = <the joined workspace's 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<string> {
const cookies = await ctx.cookies(baseUrl);
return cookies.map((c) => `${c.name}=${c.value}`).join("; ");
}

async function getJSON<T>(cookieHeader: string, url: string): Promise<T> {
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<PlaneMe>(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<PlaneSettings>(
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);
});
});
105 changes: 105 additions & 0 deletions tests/auth/session-store-redis-backed.spec.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
Loading