Allow authorization header to be omitted #457
Conversation
Endpoints configured for the anybody (`*`) authorization should work without an Authorization header. This change allows requests without the header to be processed, returning a default user identity, which will obviously receive the anybody role (`*`) (because everyone receives it), so if the configuration allows particular endpoints to be reachable by the `*` role, guests (no auth header) users will be authorized to use it.
WalkthroughAdds a default no-auth tuple in the auth interface, updates JWK token dependency to return this tuple when the Authorization header is absent and to use a typed AuthTuple, adjusts JWT roles resolver to skip claims parsing for a no-user token, and updates unit tests accordingly. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant R as Request
participant A as JwkTokenAuthDependency
participant Z as Authorization Resolver
C->>R: HTTP request
R->>A: __call__(request)
alt Authorization header missing
A-->>R: NO_AUTH_TUPLE (DEFAULT_USER_UID, DEFAULT_USER_NAME, NO_USER_TOKEN)
else Header present
A->>A: Extract bearer token
A->>A: Decode/validate JWT
A-->>R: AuthTuple(user_id, user_name, token)
end
R->>Z: Resolve roles with (user_id, user_name, token)
alt token == NO_USER_TOKEN
Z->>Z: Return {} (empty claims)
else token present
Z->>Z: Parse claims from token
end
Z-->>C: Roles/authorization outcome
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/auth/interface.py (1)
20-20: Solid addition: a single source of truth for the guest auth tuple.Minor: consider a short docstring comment above NO_AUTH_TUPLE clarifying that NO_USER_TOKEN is a sentinel (not JSON) and is only compared by equality checks.
- NO_AUTH_TUPLE: AuthTuple = (DEFAULT_USER_UID, DEFAULT_USER_NAME, NO_USER_TOKEN) + # Guest identity used when no Authorization header is present; token is a sentinel. + NO_AUTH_TUPLE: AuthTuple = (DEFAULT_USER_UID, DEFAULT_USER_NAME, NO_USER_TOKEN)tests/unit/auth/test_jwk_token.py (1)
271-276: Good assertions for guest tuple; add a test ensuring '*' access actually works.This test validates the tuple only. Please add a test that proves endpoints (or resolvers/access) gated by '*' are reachable without headers (see suggestions in resolvers.py comment).
I can add a focused unit test for JwtRolesResolver or GenericAccessResolver to lock this behavior in CI—want me to push that?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
src/auth/interface.py(1 hunks)src/auth/jwk_token.py(2 hunks)src/authorization/resolvers.py(2 hunks)tests/unit/auth/test_jwk_token.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/auth/jwk_token.py (2)
src/auth/interface.py (1)
AuthInterface(23-28)src/auth/utils.py (1)
extract_user_token(7-26)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-pr
- GitHub Check: e2e_tests
🔇 Additional comments (6)
src/authorization/resolvers.py (2)
12-12: Import is appropriate and consistent with usage.
75-78: Review Comment Outdated: NO_USER_TOKEN Branch Not PresentI inspected
src/authorization/resolvers.pyand did not find anyconstants.NO_USER_TOKENcheck or a_get_claimsmethod in the currentJwtRolesResolver. The code forresolve_rolessimply pulls JWT claims and applies the configured rules:async def resolve_roles(self, auth: AuthTuple) -> UserRoles: jwt_claims = self._get_claims(auth) return { role for rule in self.role_rules for role in self.evaluate_role_rules(rule, jwt_claims) }Since there is no guest-token branch in this class (and no reference to
NO_USER_TOKEN), the suggested diff cannot be applied as written. Please ignore the original comment or update it against the current implementation of claim extraction.Likely an incorrect or invalid review comment.
src/auth/interface.py (1)
12-13: Imports of default guest constants look good.src/auth/jwk_token.py (2)
22-22: Updated imports match the new return type and guest tuple usage.
123-129: Add debug log for missing Authorization header
- Verified via ripgrep that every
await dependency(...)call unpacks three values (no two-variable unpacking found), so no downstream changes are required.- Apply the following diff to emit a debug message when returning the guest tuple:
--- a/src/auth/jwk_token.py +++ b/src/auth/jwk_token.py @@ async def __call__(self, request: Request) -> AuthTuple: - if not request.headers.get("Authorization"): - return NO_AUTH_TUPLE + if not request.headers.get("Authorization"): + logger.debug("No Authorization header; returning guest auth tuple (NO_AUTH_TUPLE)") + return NO_AUTH_TUPLEtests/unit/auth/test_jwk_token.py (1)
15-15: Constants import aligns tests with the new guest semantics.
2 participants
Description
Endpoints configured for the anybody (
*) role should work without an Authorization header.This change allows requests without the header to be processed, returning a default user identity, which will obviously receive the anybody role (
*) (because everyone receives it), so if the configuration allows particular endpoints to be reachable by the*role, guests (no auth header) users will be authorized to use it.This is good for the readiness/liveness endpoints
Type of change
Related Tickets & Documents
Checklist before requesting a review
Testing
Summary by CodeRabbit