fix(auth): require Bearer/Basic scheme and route exclusively#1677
Open
AmanGIT07 wants to merge 1 commit into
Open
fix(auth): require Bearer/Basic scheme and route exclusively#1677AmanGIT07 wants to merge 1 commit into
AmanGIT07 wants to merge 1 commit into
Conversation
The Authorization header parser used strings.TrimPrefix, which is a no-op when the prefix is absent. As a result a bare PAT (no scheme) was treated as a token, a bare base64 credential was treated as a secret, and every Bearer header also leaked into the secret slot because both blocks ran unconditionally. Scheme matching was also case-sensitive, breaking RFC 7235. Replace the parser with a small helper that requires the scheme, matches it case-insensitively via strings.EqualFold, and routes Bearer to UserTokenGatewayKey and Basic to UserSecretGatewayKey — never both. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR consolidates Authorization header parsing across three interceptor methods into shared helper functions, then adds comprehensive test coverage. Bearer/JWT/PAT tokens are routed to the token gateway key, Basic credentials to the secret gateway key, with case-insensitive scheme matching and JWT expiration validation. Tests validate credential extraction, routing across all transports, backward compatibility with existing ChangesAuthorization Header Refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
Contributor
Author
Test resultsUnit tests
Added to
Live E2E against a local Frontier
Rows 11–13 are the rejections that the prior code accepted (bare credentials with no scheme). Rows 14–16 are wrong-scheme combinations that the prior code partially honored by writing the raw header into the secret slot. |
Coverage Report for CI Build 27009627415Coverage increased (+0.2%) to 43.406%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
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
2 participants
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.
Summary
The Authorization header parser in
pkg/server/connect_interceptors/session.goaccepted credentials with no scheme prefix, populated both the token and secret metadata slots from a single header, and matchedBearer/Basiccase-sensitively. This PR tightens the parser so that the scheme is required, matched case-insensitively, and routes credentials to exactly one slot.Changes
extractAuthCredentials(header, scheme)— returns the credentials portion of an Authorization header, matching the scheme case-insensitively viastrings.EqualFold(RFC 7235).applyAuthorizationHeader(md, header, patConf)— routes Bearer credentials toUserTokenGatewayKey(PAT or Frontier-issued JWT) and Basic credentials toUserSecretGatewayKey. The two paths are mutually exclusive.strings.TrimPrefixblocks inWrapStreamingHandler,WrapUnary, andUnaryConnectRequestHeadersAnnotatorwith a single call to the helper.session_test.gocovering the helper with 14 scheme-parsing cases, 14 routing cases (PAT, valid JWT, expired JWT, JWT missing JTI, Basic, mixed-case schemes, bare credentials, unknown scheme, empty), and an empty-PAT-prefix edge case.Behavior
Bearer fpt_xxxbearer fpt_xxx/BEARER fpt_xxxfpt_xxx(no scheme)Basic Y2lkOnNlYw==basic Y2lkOnNlYw==basic Y2lkOnNlYw==(broken)Y2lkOnNlYw==(no scheme)Test Plan
go test ./pkg/server/connect_interceptors/...— all subtests passgolangci-lint run ./pkg/server/connect_interceptors/...— cleanAuthorization: Bearer fpt_<pat>against a running server🤖 Generated with Claude Code