fix(event-ledger): route JWT auth away from the API key evaluator - #1074
Draft
shelleyshen-0 wants to merge 1 commit into
Draft
fix(event-ledger): route JWT auth away from the API key evaluator#1074shelleyshen-0 wants to merge 1 commit into
shelleyshen-0 wants to merge 1 commit into
Conversation
The policy provider funnelled every credential to the API key policy evaluator, whose contract requires an opaque API key. JWT-bearing callers were therefore rejected, and because the per-route scope wrappers were inert under that provider, token scopes were never enforced either. Split the two credentials into independent paths chosen by token shape. A JWT is verified against the configured JWKS and then authorized by the per-route scope check. An API key is forwarded to the evaluator as before and skips the scope check, since it carries no scopes. Requests the evaluator authorizes are marked so the scope wrapper lets them through. This removes the request-clone and no-op ResponseWriter workaround that let the JWT parser run inside the policy middleware, and passes the issuer and audience options through to the parser. Also accept the evaluator's actual verdict field name. It reports "allowed" while the response type only read "allow", so successful evaluations deserialized as denials. The existing client test hardcoded the wrong shape and masked this. Adds coverage for both paths, including scope enforcement driven through the real parser against a generated ES256 key and JWKS endpoint.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
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.
TL;DR
JWT callers were sent to the API key policy evaluator, which accepts only an API key and rejects anything without one, so SIS and NVCA writes always failed. Route JWTs to local verification and scope checking instead, and accept the evaluator's actual verdict field so valid API keys are no longer read as denials.
Additional Details
Two independent defects made the service reject every request in self-managed deployments:
apiKeyinput field, so a JWT produced a 400 that surfaced as 401.result.allow, but the evaluator returnsresult.allowed, so approvals deserialized as denials.Scope enforcement was also inert: the per-route scope wrappers were disabled under this provider, so token scopes were never compared against route requirements.
A dispatcher now selects the path by token shape. A JWT is verified against the configured JWKS and authorized by the route's scope check. An API key goes to the evaluator as before and skips the scope check, since it carries none. Managed deployments are unchanged: JWTs are still verified and then delegated to the policy decision point.
For the Reviewer
internal/middleware/dual_auth.go— new dispatcherinternal/middleware/policy.go— JWT handling removed, reads claims from context, accepts both verdict field namesinternal/middleware/jwt.go— scope check passes through for evaluator-authorized requestscmd/api/startup/run_service.go— wiringFor QA
Verified on a self-managed stack with an OpenBao-signed token for the
sis-apirole, both in-cluster and through the gateway:POST /v3/ledger/cloudeventsGETevents and statsGETevents and statsGETeventsGET /healthUnit tests cover both paths, including scope enforcement driven through the real parser against a generated ES256 key and JWKS endpoint.
Note: requires the api-keys registration in #1062 for the API key path.
Issues
Relates to #82
Checklist