feat: add nonce claim support to oidc server#686
Conversation
|
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)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds OIDC nonce propagation: frontend params and authorize payload include Changes
Sequence DiagramsequenceDiagram
participant Client
participant Frontend
participant OIDCController
participant OIDCService
participant Database
Client->>Frontend: Authorization request (includes nonce)
Frontend->>OIDCController: POST /api/oidc/authorize (nonce)
OIDCController->>OIDCService: StoreCode(AuthorizeRequest{..., nonce})
OIDCService->>Database: INSERT OidcCode (code, sub, scope, nonce)
Database-->>OIDCService: Code stored
OIDCService-->>OIDCController: authorization code
OIDCController-->>Frontend: code
Frontend->>OIDCController: POST /token (code)
OIDCController->>OIDCService: GenerateAccessToken(code)
OIDCService->>Database: SELECT OidcCode by code
Database-->>OIDCService: OidcCode{sub, scope, nonce}
OIDCService->>OIDCService: generateIDToken(sub, scope, nonce)
OIDCService->>Database: INSERT OidcToken (access, id, sub, scope, nonce)
Database-->>OIDCService: Token stored
OIDCService-->>OIDCController: tokens (access + id)
OIDCController-->>Frontend: tokens
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/service/oidc_service.go`:
- Line 424: The call to generateIDToken is passing the wrong
parameter—codeEntry.Sub is used where the scope should be passed; update the
call in the authorization code exchange path (the service.generateIDToken
invocation) to pass codeEntry.Scope as the third argument (keeping
codeEntry.Nonce as the fourth) so the generated ID token receives the correct
scope claim (consistent with the RefreshAccessToken usage that passes
entry.Scope, entry.Nonce).
- Around line 447-455: The CreateOidcToken call is missing the Nonce field in
the CreateOidcTokenParams payload, so persist the nonce by adding Nonce:
codeEntry.Nonce to the params used in service.queries.CreateOidcToken; update
the parameter list where CreateOidcTokenParams is constructed (in the function
containing CreateOidcToken call) so entry.Nonce is stored and later available to
RefreshAccessToken and ID token generation.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
frontend/src/lib/hooks/oidc.tsfrontend/src/pages/authorize-page.tsxinternal/assets/migrations/000006_oidc_nonce.down.sqlinternal/assets/migrations/000006_oidc_nonce.up.sqlinternal/controller/oidc_controller.gointernal/repository/models.gointernal/repository/oidc_queries.sql.gointernal/service/oidc_service.gosql/oidc_queries.sqlsql/oidc_schemas.sqlsqlc.yml
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #686 +/- ##
==========================================
- Coverage 15.27% 15.19% -0.08%
==========================================
Files 50 50
Lines 3673 3692 +19
==========================================
Hits 561 561
- Misses 3055 3074 +19
Partials 57 57 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary by CodeRabbit
New Features
Chores