Repository navigation
feat(auth): carry typed refusal reasons and verified attribution - #5996
Conversation
Share refusal reasons and optional verified root/tier across HTTP auth and embedded admission. Keep legacy refusals unattributed, and count expired credentials in relay metrics and stats. Co-Authored-By: GPT-6.1-Sol <noreply@openai.com>
# Conflicts: # quest/m1/README.md # rs/moq-relay/MIGRATE.md
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 16 minutes. View limit details
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. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b50a4f719
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
# Conflicts: # rs/moq-auth/src/key.rs
…r's root and tier Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05ae3924da
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Refusals always serializes all six kinds since #5996. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Problem
The auth client discarded refusal bodies, so the relay could neither distinguish expired credentials nor attribute refusals to a verified root and tier.
Approach
Share a typed refusal between HTTP auth and embedded admission. Verify signatures, claims, and key scope before attributing a token refusal; never attribute the dialed path. Preserve legacy plain-text 401/403 refusals and map unknown reason strings to
refused. Count expired refusals in metrics and per-root stats, for sessions, gateways, and the HTTP/fetchand/announcedendpoints alike, and remove the completed quest.Impact
moq_auth::Error::Refusedandmoq_relay::auth::Error::Refusedcarrymoq_auth::Refusal; affectedserve::Refusalvariants carry verified roots. Migration entries inrs/moq-auth/MIGRATE.mdandrs/moq-relay/MIGRATE.mddescribe the changes.reason(refused,forbidden,expired), optional verifiedroot, and optionaltier. Typed forbidden refusals preserve HTTP 403 at relay endpoints. An expired refusal at revalidation ends the credential lease as expired.RefusalSchemaandRefusalin@moq/authmirror the HTTP shape.sessions_refused.expiredis an additive cumulative counter. Refusal roots supply the stats label; absent roots remain unattributed. No MoQ transport framing or negotiation changes.Alternatives
Keep all refusals generic, losing expiry information. Using the dialed path for attribution would allow unbounded attacker-chosen stats entries.
Decisions
Iterate rounds (no maintainer decision needed):
Failing
Test(bursts_cross_a_transport_peer::impaired, "the route was withdrawn while the peer link was up")origin/main: the failure is the shaper ingress-gate flake fixed by moq-dev/moq#5943, which names this exact message and merged after this PR's runKey::verifyand the newverify_claimsduplicated the decode/validate/scope stepsverifycallsverify_claimsand adds the time check, so the two cannot driftExpiry clock after merging moq-dev/moq#5987
Key::verifyand serve's expired-token check both readmoq_clock::wall(), as main now doesSystemTime::now()in serveCodex: HTTP admission recorded refusals unattributed
admit_httprecords throughrecord_auth, with a regression testValidation
expiredrefusal became a generic refusal error.web::tests::http_refusals_keep_the_deciders_root_and_tierfails without the HTTP fix.just checkafter mergingorigin/main: 7858 Rust tests passed (31 existing skips),@moq/authpassed. Under machine load above 20,bench/relay.test.pyhit its 15s wall timeout (1.3s on rerun) and an earlier run failedmoq-uring clock::tests::host_tracks_moq_clock's 1ms slack (passed on rerun and in the full rerun). Neither is touched here.0a7b42259:just test interop --publishers rust,python,go,cpp --subscribers rust,python,go,cpp,js-native-node,js-native-bun,c,gst --timeout 30. The browser portion of--allwas blocked locally by the pinned Chromium runtime missing libglib. This PR changes no MoQ wire.Follow-ups
moq-uring clock::tests::host_tracks_moq_clockcompares two wall-clock reads with a 1ms slack, so it fails under load.(Written by GPT-6.1-Sol and Claude Opus 5.5)
🤖 Generated with Claude Code