Repository navigation
feat: configure login parameters and expose Kubernetes identity - #37
Merged
sunib merged 4 commits intoOct 5, 2026
Merged
Conversation
- Login parameters: -login-config-file (chart: login) names extra query parameters for the issuer's authorization endpoint by the issuer's own names, each with a default, allowFromRequest and allowedValues. A login link gives values as oidc.<name>. Anything else, krm-foyer's own parameters (state, nonce, PKCE, client, redirect URI, scope, request, credentials) included, is a 400 login-parameter-refused before a login starts, with the value neither shown nor logged. Values are encoded once and never changed. A failed login's retry link repeats listed choices only, never a free hint. - Session claims: /auth/session adds displayName, groups and an optional connector, from JSON Pointers into the verified ID token, in a fixed shape; a claim of the wrong type fails the login as session-claims-invalid. - /auth/whoami: a SelfSubjectReview with the user's own token through the gate, sharing one implementation with the shared watches' reviews. Refusals pass through; failures, empty answers and redirects are 503s. - Attribution: the e2e API server maps name and email to the two user.extra keys gitops-reverser reads; specs check them in /auth/whoami, the audit event and an admission request, with spoofed identity headers. - The browser helper's login() takes parameters, and loginURL() builds the same link for an anchor or a QR code. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (37)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe pull request adds configurable issuer login parameters and ID-token session claims. It also implements ChangesLogin and identity
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~60 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant Browser
participant WhoAmI as Streams.WhoAmI
participant Gate
participant API as Kubernetes API server
Browser->>WhoAmI: GET /auth/whoami
WhoAmI->>Gate: Admit request
Gate-->>WhoAmI: Session credential
WhoAmI->>API: SelfSubjectReview with session token
API-->>WhoAmI: UserInfo or API error
WhoAmI-->>Browser: Identity response or error status
Merge Risk: ⚪ Minimal · up to No actionable merge-blocking issue is established; the PR is mergeable after normal checks. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The inspected changes keep login options under deployment control and resolve Kubernetes identity using the signed-in user's token. No introduced privilege escalation or identity substitution was established. Remaining uncertainty concerns deployment-specific identity policies and compatibility outside the repository. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 74.07% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 54 functions across 23 files. (14 skipped: 14 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
Accepted login parameters and a long return path could seal into a login cookie over 4,096 bytes once JSON-escaped, encrypted and base64-encoded; the browser dropped it and the login failed at its callback. begin now checks the cookie's name and value against 4,000 bytes and the login answers 400 login-too-large before reaching the issuer. Also corrects two docs: the roadmap no longer asks for shared session storage, and the plan says the plaintext token goes to the API server. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…end them
The gate fails closed: a session check that does not answer within one
interval counts as an ended session. Every stream test ran with a 20ms
interval, so on a loaded CI runner a check that would have answered was
sometimes not scheduled in time, and a stream was cut short
("session_ended" in its log) before its first event. Reproduced locally
with 12 copies of the test binary at once, on this branch and on the one
below it.
The stream tests now keep the gate's default interval, as the proxy tests
already do, and the two that end a session opt into the short one.
TestACredentialWithoutLiveIsCutShort accepts the abort before the
response head that its first check can cause, as endsAborted does for
the other cut-short tests.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR 2 of the audience release plan. It is stacked on #36; rebase it onto main once #36 merges.
-login-config-file(chart:login) names the issuer's extra authorization parameters, each with a default,allowFromRequestandallowedValues. Links pass values asoidc.<name>. Anything else, including state, nonce, PKCE, client, redirect URI, scope, request and credential parameters, gets400 login-parameter-refusedbefore a login starts. Values are never logged or echoed./auth/sessionaddsdisplayName,groupsand an optionalconnector, read through JSON Pointers into the verified token, always in a fixed shape./auth/whoami: a SelfSubjectReview with the user's token through the gate, sharing one implementation with the shared watches' reviews. Refusals pass through; failures and redirects give 503.configbutler.ai/claims/display-nameandconfigbutler.ai/claims/email. Specs check both in whoami, the audit event, and an admission request captured with a ValidatingAdmissionPolicy in Warn mode, while the browser sends spoofed identity headers.login(returnTo, params)andloginURL().Unit tests, lint and a mutation check of the parameter boundary pass.
task verifyis green locally, including the new e2e specs, on a fresh fixture. A reused fixture loses its Dex and test-issuer host aliases when the authentication config changes and the API server restarts; that is a fixture limitation, and CI always starts fresh.🤖 Generated with Claude Code
Summary by CodeRabbit
/auth/whoamiendpoint that reports the identity resolved by the Kubernetes API server.