Skip to content

docs(rfc): rfc 0029 specified — §5 acceptance criteria - #420

Merged
jensholdgaard merged 4 commits into
mainfrom
rfc0029-specified
Jul 7, 2026
Merged

docs(rfc): rfc 0029 specified — §5 acceptance criteria#420
jensholdgaard merged 4 commits into
mainfrom
rfc0029-specified

Conversation

@jensholdgaard

@jensholdgaard jensholdgaard commented Jul 7, 2026

Copy link
Copy Markdown
Owner

Takes RFC 0029 (OIDC bearer layer) draftedspecified on maintainer sign-off (2026-07-07).

Writes the §5 acceptance criteria promised at the drafted gate, RFC0029.1.7:

  • .1 config resolution${env} substitution, required audience, the at-least-one rule, and the unconditional tokens: [] startup error (the coexistence rule settled in docs(rfc): rfc 0029 — oidc bearer layer (issuer-agnostic, dex-validated), drafted #419 review); open-mode warning unchanged.
  • .2 verification matrix — fixture issuer (local keypair, no container): valid accepted; expired / nbf-beyond-skew / wrong-aud / wrong-iss / bad-sig / alg:none / HMAC-downgrade / non-JWT all rejected as one undifferentiated 401 before wire decode, nothing reaching the WAL.
  • .3–.5 enforcement invariance — the RFC 0026 §5.3–.6 contracts hold verbatim with an OIDC-resolved (name, tenants) binding: whole-batch ingest rule, 401→400→403 on query + MCP, wildcard parity, static+OIDC coexistence, open-mode parity.
  • .6 JWKS rotation — unseen-kid refresh mid-run; withdrawn key rejected post-refresh.
  • .7 Dex end-to-end — real Dex container (testcontainers, CI-gated per the RFC 0019 pattern), client-credentials mint → ingest/query/MCP, short-TTL expiry, and telemetry/audit parity (unchanged error.type values, ingest_denied carrying the name_claim value, no JWT material on any surface).

§6 updated to name which scenarios each test tier discharges. Docs-only; mdbook build clean. Invariant §3.7 (multi-tenancy): the criteria pin the binding derivation's second source to the existing enforcement contracts rather than adding new ones.

Next rung: red (#[ignore]d stubs) on the usual cadence.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Documentation
    • Updated the OIDC bearer-layer RFC from draft to specified status.
    • Reworked acceptance criteria into scenario-based requirements, including token verification failures, tenant binding behavior, wildcard handling, token resolution order, JWKS rotation, and telemetry/audit expectations.
    • Clarified the testing approach with both fixture-based and end-to-end validation paths.

…gn-off 2026-07-07

Seven scenarios: config resolution incl. the unconditional empty-list
rule, the fixture-issuer verification matrix (one undifferentiated
401), claim-binding enforcement invariance vs RFC 0026 §5.3-.6,
wildcard parity, static+OIDC coexistence, mid-run JWKS rotation, and
the real-Dex testcontainers end-to-end with telemetry/audit parity.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 7, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@jensholdgaard, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 41 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: b859899d-0292-4633-84de-2249f0eddb9d

📥 Commits

Reviewing files that changed from the base of the PR and between 8f9135d and e020161.

📒 Files selected for processing (1)
  • docs/rfcs/0029-oidc-bearer-layer.md
📝 Walkthrough

Walkthrough

This PR updates the OIDC bearer-layer RFC document, changing its lifecycle status from "drafted" to "specified" and rewriting the acceptance criteria and testing strategy sections into detailed, scenario-based specifications (RFC0029.1–RFC0029.7).

Changes

RFC 0029 Documentation Update

Layer / File(s) Summary
Lifecycle status change
docs/rfcs/0029-oidc-bearer-layer.md
RFC status metadata changed from drafted to specified.
Scenario-based acceptance criteria and testing strategy
docs/rfcs/0029-oidc-bearer-layer.md
Acceptance criteria rewritten into scenarios RFC0029.1–RFC0029.7 covering config resolution, OIDC verification rejection matrix, claim-to-tenant binding enforcement invariance, static/OIDC coexistence, JWKS rotation, and Dex end-to-end telemetry/audit parity; testing strategy section updated to map fixture issuer and testcontainers coverage to these scenarios.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • jensholdgaard/ourios#374: Introduces the RFC 0026 authentication and tenant-binding behavior that this RFC's acceptance criteria explicitly reference as unchanged enforcement invariance.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly states the RFC 0029 status change and scope of the acceptance-criteria update.
Description check ✅ Passed It covers the RFC status change, detailed acceptance criteria, related context, and validation notes, though it doesn't use the template headings.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch rfc0029-specified

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Moves RFC 0029 (OIDC bearer layer) from drafted to specified by filling in the promised §5 acceptance criteria and aligning §6 testing strategy with those scenarios, formalizing the verification/enforcement expectations for the OIDC-based auth layer that plugs into RFC 0026’s existing (name, tenants) enforcement.

Changes:

  • Updates RFC 0029 lifecycle status to specified.
  • Replaces the §5 placeholder with concrete, numbered acceptance scenarios RFC0029.1–.7 (fixture issuer + real Dex arm).
  • Refines §6 testing strategy to map test tiers to the acceptance scenarios.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread docs/rfcs/0029-oidc-bearer-layer.md Outdated
Comment thread docs/rfcs/0029-oidc-bearer-layer.md Outdated
…e-issuer tier

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment thread docs/rfcs/0029-oidc-bearer-layer.md Outdated
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment thread docs/rfcs/0029-oidc-bearer-layer.md Outdated
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

@jensholdgaard
jensholdgaard merged commit 47fa28f into main Jul 7, 2026
22 checks passed
jensholdgaard added a commit that referenced this pull request Jul 7, 2026
#426)

* feat(auth): rfc 0029 green — dex end-to-end acceptance (.7) and status flip

The last §5 arm: a real Dex container (testcontainers, CI-gated like
the localstack job) mints client-credentials tokens the served binary
verifies against Dex's real JWKS.

- The static client carries the tenant list via
  clientCredentialsClaims.groups (tenant_claim: groups) and the name
  label via its display name (name_claim: name, scope profile) — the
  OTel Collector oauth2client flow verbatim.
- Arms: startup discovery against Dex; in-claim gRPC ingest acks;
  cross-tenant batch PERMISSION_DENIED; in-claim query 200; MCP 401
  bearer-less / success with the Dex bearer; real-TTL expiry (8 s
  tokens, zero skew) collapses to the undifferentiated 401; SIGTERM
  flushes the audit sink and the read-back ingest_denied event carries
  the name_claim value; no JWT material in error bodies or the log
  surface.
- Image: Dex `master` pinned by digest — the client-credentials grant
  and clientCredentialsClaims are merged upstream (dexidp/dex#4691)
  but post-v2.45.1; the RFC §6 note + ci.yml comment record the bump
  path to v2.46. New required `dex oidc (testcontainers)` CI job runs
  the arm by exact name.
- reqwest (rustls + json) joins the server dev-deps for minting and
  readiness polling — the same stack the verifier itself uses.

RFC 0029 status red → green: .1–.6 discharged across #420#425, .7
lands here and runs in this PR's own required CI job.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* test(auth): surface dex container logs on readiness timeout

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* test(auth): dex needs one connector — enablePasswordDB, inert for client-credentials

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* test(auth): dex arm hardening — ttl headroom, port retry, async sigterm; ci creds + rfc quote

20s tokens with the expiry wait driven by the response's expires_in;
container start retries a fresh port on the reserve race; SIGTERM via
tokio Command with an asserted status; persist-credentials: false on
the dex job checkout; the RFC image note no longer swallows the
following paragraph.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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.

2 participants