Skip to content

Drain deferred gaps#59

Merged
awais786 merged 2 commits into
mainfrom
drain-deferred-gaps
Jun 4, 2026
Merged

Drain deferred gaps#59
awais786 merged 2 commits into
mainfrom
drain-deferred-gaps

Conversation

@awais786

@awais786 awais786 commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator

Motivation / Background

This Pull Request has been created because:

  • Resolves #issue-id

Detail

This Pull Request changes:

Additional information

TIP: Provide additional information such as screenshots, benchmarks, reference to other repositories or alternative solutions

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature
  • CHANGELOG files are updated for the behavior change or additional feature (minor bug fixes and documentation changes should not be included)
  • This PR contains API changes and API documentation is updated accordingly (for critical or behavior change, please inform related parties about them).

awais786 added 2 commits June 4, 2026 15:03
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.
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.
@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown

SSO Spec Coverage Audit

Contract source: vendor/openspec/specs/ (snapshot vendored in this repo)

Status Count
✅ Covered (test tag found) 65
⚠️ Deferred (in deferred doc) 23
❌ Missing (no tag, no defer) 0
Total requirements 88

All 88 spec requirements are accounted for.

@awais786 awais786 merged commit 1d02288 into main Jun 4, 2026
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant