feat: lower auth_code_ttl default to 300s and enforce 900s maximum - #4822
Conversation
|
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (8)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe OAuth2 authorization-code TTL is reduced to 300 seconds and capped at 900 seconds across backend validation, issuance, UI behavior, schemas, and docs. ChangesOAuth2 Auth Code TTL Default/Max Enforcement
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@transports/bifrost-http/handlers/config.go`:
- Around line 579-589: Validate auth_code_ttl before any live runtime mutation
in the config update flow, since the current check in the config handler happens
after state-changing calls like DropExcessRequests, MCP manager/in-memory config
updates, and compat plugin reload. Move this validation earlier in the same
handler (using the effectiveOAuth2Config and effectiveAuthMode checks) so an
over-max value fails fast before any in-memory or core state is changed, and
remove the later duplicate auth_code_ttl guard. Ensure the update path remains
rollback-aware so runtime and persistent state stay aligned.
In `@ui/app/workspace/config/views/mcpView.tsx`:
- Around line 233-239: The auth_code_ttl UI validation is stricter than the
config schema and incorrectly blocks valid values from 1 to 59 seconds. Update
the save-time validation in mcpView.tsx (the oauthModeActive/authCodeTTL check)
and the auth_code_ttl input’s min so they match the schema source of truth:
allow 1 through 900 seconds. Keep the existing toast/error flow, but change the
threshold logic to align with the schema and the related input handling in the
same view.
🪄 Autofix (Beta)
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: 4b772a55-61e5-47a9-a01b-31c363d1ada1
📒 Files selected for processing (9)
docs/mcp/gateway-auth.mdxdocs/openapi/openapi.jsondocs/openapi/schemas/management/config.yamlframework/configstore/tables/mcpoauth2server.gotransports/bifrost-http/handlers/config.gotransports/bifrost-http/handlers/mcpoauth2issuance.gotransports/bifrost-http/lib/config.gotransports/config.schema.jsonui/app/workspace/config/views/mcpView.tsx
Confidence Score: 4/5Safe to merge after the upgrade-path discrepancy in validateClientConfig is resolved or documented; all three enforcement layers are implemented correctly and consistently. The load-time validator hard-fails boot for any stored auth_code_ttl above 900, but the PR's Breaking Changes section tells operators the value will be clamped at load time with a warning log. An operator who previously stored auth_code_ttl=1000 will upgrade and find the server unable to start, with no API path to fix it since the server won't boot. The three-layer enforcement is otherwise correct and internally consistent, and the new tests cover the key branches well. transports/bifrost-http/lib/config.go — validateClientConfig behavior (hard boot failure) needs to either be corrected to clamp-and-warn or have the Breaking Changes documentation updated to accurately describe the upgrade impact. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["PUT /api/config with auth_code_ttl"] --> B{auth_code_ttl > 900?}
B -- Yes --> C[400 Bad Request]
B -- No --> D[Persist to DB]
E[Server startup: LoadConfig] --> F[loadClientConfig]
F --> G[validateClientConfig]
G --> H{auth_code_ttl > 900?}
H -- Yes --> I["Boot failure — LoadConfig error"]
H -- No --> J[Server starts]
K["GET /oauth2/authorize"] --> L[handleAuthorize]
L --> M{TTL <= 0?}
M -- Yes --> N["DefaultAuthCodeTTL = 300s"]
M -- No --> O{TTL > 900?}
O -- Yes --> P["clamp to MaxAuthCodeTTL = 900s"]
O -- No --> Q[use configured TTL]
N --> R[Mint auth code]
P --> R
Q --> R
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A["PUT /api/config with auth_code_ttl"] --> B{auth_code_ttl > 900?}
B -- Yes --> C[400 Bad Request]
B -- No --> D[Persist to DB]
E[Server startup: LoadConfig] --> F[loadClientConfig]
F --> G[validateClientConfig]
G --> H{auth_code_ttl > 900?}
H -- Yes --> I["Boot failure — LoadConfig error"]
H -- No --> J[Server starts]
K["GET /oauth2/authorize"] --> L[handleAuthorize]
L --> M{TTL <= 0?}
M -- Yes --> N["DefaultAuthCodeTTL = 300s"]
M -- No --> O{TTL > 900?}
O -- Yes --> P["clamp to MaxAuthCodeTTL = 900s"]
O -- No --> Q[use configured TTL]
N --> R[Mint auth code]
P --> R
Q --> R
Reviews (3): Last reviewed commit: "feat: clamp mcp oauth auth token timeout..." | Re-trigger Greptile |
b47b7d3 to
ead7564
Compare
ead7564 to
4883dcd
Compare
Merge activity
|

Summary
Reduces the default OAuth2 authorization code TTL from 600 seconds to 300 seconds and enforces a hard maximum of 900 seconds (15 minutes). This limits the window during which a leaked one-time authorization code could be exploited.
Changes
DefaultAuthCodeTTLreduced from 600s to 300s;MaxAuthCodeTTLconstant introduced at 900s/api/config) now rejectsauth_code_ttlvalues exceeding 900s when OAuth or both auth modes are active, returning a 400 errorapplyClientConfigDefaultsclamps any over-max value loaded fromconfig.jsonor the database (bypassing the API), with a warning logmaxattribute on the number inputmaximum: 900,default: 300, and revised descriptionsType of change
Affected areas
How to test
Breaking changes
The default
auth_code_ttldrops from 600s to 300s. Any deployment relying on the previous default will now issue shorter-lived authorization codes. Stored values above 900s will be clamped at load time with a warning log and rejected via the API going forward.Security considerations
Authorization codes are single-use but represent a brief window of exploitability if intercepted. Reducing the default TTL to 5 minutes and capping the maximum at 15 minutes limits the exposure window for leaked codes. The cap is enforced at three layers: API validation, config load, and code issuance, ensuring no path can produce a code with a TTL exceeding 900 seconds.
Checklist
docs/contributing/README.mdand followed the guidelines