Extend authorization request and auth flow expiry to 60 minutes - #4539
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change increases authentication flow expiry to 3600 seconds and adds configurable authorization request validity. Positive values apply as seconds-based durations. Non-positive values use a 60-minute default. Configuration, tests, deployment templates, and documentation are updated. ChangesAuthentication and authorization request expiry
Estimated code review effort: 2 (Simple) | ~12 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
docs/content/deployment/configuration.mdxESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. 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 |
| return &authorizationRequestStore{ | ||
| storeProvider: storeProvider, | ||
| validityPeriod: 10 * time.Minute, | ||
| validityPeriod: 60 * time.Minute, |
There was a problem hiding this comment.
IMO better to give a configuration for this too. Otherwise this again will easily expire when the auth flow expiry time is increased
There was a problem hiding this comment.
Introduced a config.
190a430 to
098e056
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
098e056 to
e7a08c8
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
| "validity_period": 600 | ||
| }, | ||
| "authorization_request": { | ||
| "validity_period": 3600 |
There was a problem hiding this comment.
Need to add this to helm and other deployment templates too
Raise the OAuth2 authorization request context validity from 10 minutes and the authentication flow context expiry from 30 minutes to 60 minutes, so a user who takes longer over sign-in is not dropped mid-flow. Make the authorization request context validity configurable through oauth.authorization_request.validity_period. A non-positive value falls back to the built-in 60 minute default.
e7a08c8 to
d694e36
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@install/helm/README.md`:
- Line 506: Update the description for
configuration.oauth.authorizationRequest.validityPeriod in the Helm README to
state that zero or negative values fall back to 3600 seconds, while preserving
the existing explanation and default value.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9fb6154a-146e-401c-b083-2d4998724681
📒 Files selected for processing (13)
backend/cmd/server/bootstrap/02-server-configurations.yamlbackend/cmd/server/config/default.jsonbackend/internal/flow/flowexec/constants.gobackend/internal/flow/flowexec/service_test.gobackend/internal/oauth/oauth2/authz/auth_req_store.gobackend/internal/oauth/oauth2/authz/auth_req_store_test.gobackend/internal/oauth/oauth2/authz/constants.gobackend/internal/oauth/oauth2/authz/init.gobackend/pkg/thunderidengine/config/config.godocs/content/deployment/configuration.mdxinstall/helm/README.mdinstall/helm/conf/deployment.yamlinstall/helm/values.yaml
🚧 Files skipped from review as they are similar to previous changes (10)
- backend/cmd/server/config/default.json
- backend/internal/oauth/oauth2/authz/auth_req_store.go
- backend/internal/oauth/oauth2/authz/init.go
- backend/pkg/thunderidengine/config/config.go
- backend/internal/oauth/oauth2/authz/constants.go
- backend/cmd/server/bootstrap/02-server-configurations.yaml
- backend/internal/oauth/oauth2/authz/auth_req_store_test.go
- docs/content/deployment/configuration.mdx
- backend/internal/flow/flowexec/constants.go
- backend/internal/flow/flowexec/service_test.go

Purpose
Extend two sign-in lifetimes to 60 minutes, and make the authorization request context validity configurable.
oauth.authorization_request.validity_periodauthFlow.expirySeconds/defaultAuthFlowExpiry)The authorization request context was the shorter of the two, so a user who took more than 10 minutes over sign-in lost the OAuth context while the flow was still alive, and the callback failed with
invalid_requestinstead of completing.Approach
Configurable authorization request validity
oauth.authorization_request.validity_period(seconds), defaulting to3600indefault.json, alongside the existingoauth.authorization_code.validity_period. Overridable fromdeployment.yamllike any other OAuth setting.newAuthorizationRequestStorenow takes the configured value, wired inauthz/init.gofrom theoauthconfig.Configalready passed toInitialize.defaultAuthzRequestValidity(60 minutes), matching howtoken_revocation.sync_interval_secondshandles the same case.Auth flow expiry
flowexec/constants.go-defaultAuthFlowExpiry1800 -> 3600 (the fallback used when no server config supplies a value).02-server-configurations.yaml-authFlow.expirySeconds1800 -> 3600 (the bootstrapped server config).Both layers are updated so the effective value is 60 minutes whether or not the bootstrap config is present. Flow expiry was already configurable through the server-config
flowsection, so no new setting is needed there.Only
authFlowis changed.registrationFlowis already 3600;recoveryFlowandsignOutFlow(1800) anduserOnboardingFlow(86400) are out of scope.Docs in
deployment/configuration.mdxgain a row for the new setting, and the flow examples are updated to match the new default.Related Issues
Related PRs
Checklist
breaking changelabel added.Security checks
Summary by CodeRabbit