feat: support for the prompt parameter in the oidc flow#948
Open
steveiliop56 wants to merge 5 commits into
Open
feat: support for the prompt parameter in the oidc flow#948steveiliop56 wants to merge 5 commits into
steveiliop56 wants to merge 5 commits into
Conversation
dosubot
Bot
added
the
size:M
Contributor
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds ChangesOIDC prompt support, auth_time claim, and oidc_prompt frontend parameter
Sequence Diagram(s)sequenceDiagram
participant Client
participant authorize as /oidc/authorize
participant AuthorizePage
participant LoginPage
participant authorizeComplete as /oidc/authorize-complete
Client->>authorize: GET with prompt=login or prompt=none
alt prompt=none and unauthenticated
authorize-->>Client: redirect callback with login_required + state
else prompt=login
authorize->>LoginPage: redirect /login?oidc_prompt=login
LoginPage->>LoginPage: suppress post-auth redirect
LoginPage->>authorizeComplete: proceed after login with oidc_prompt=undefined
authorizeComplete->>Client: issue token with auth_time claim
else prompt=none and authenticated
authorize->>AuthorizePage: redirect /authorize?oidc_prompt=none&oidc_ticket=...
AuthorizePage->>AuthorizePage: shouldAutoAuthorize=true
AuthorizePage->>authorizeComplete: useEffect invokes authorization mutation
authorizeComplete->>Client: issue token with auth_time claim
else normal flow
authorize->>LoginPage: redirect to /login
LoginPage->>authorizeComplete: proceed with oidc_prompt cleared
authorizeComplete->>Client: issue token with auth_time claim
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~28 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Rycochet
reviewed
Jun 19, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/service/oidc_service.go (1)
618-628:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftPersist
auth_timefor refresh-issued ID tokens.
GenerateAccessTokencreates the OIDC session without savingcodeEntry.AuthTime, soRefreshAccessTokenhas to callgenerateIDToken(..., 0)and refreshed ID tokens omit the claim. StoreAuthTimewith the OIDC session and reuse it during refresh so clients don’t lose theauth_timecontract after the initial code exchange.Also applies to: 666-668
🤖 Prompt for 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. In `@internal/service/oidc_service.go` around lines 618 - 628, The CreateOIDCSession call is not persisting the AuthTime from codeEntry, causing refreshed ID tokens to lose the auth_time claim. Add the AuthTime field to the CreateOIDCSessionParams struct and pass codeEntry.AuthTime when creating the OIDC session. Then update RefreshAccessToken to retrieve the stored AuthTime from the OIDC session and pass it to generateIDToken instead of passing 0, ensuring the auth_time claim is preserved across token refreshes.internal/controller/oidc_controller.go (1)
179-219:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftKeep authenticated
prompt=nonerequests non-interactive.After the unauthenticated check, an authenticated
prompt=nonerequest still creates a frontend ticket and redirects to/oidc/authorize, which requires user interaction. Silent OIDC checks should either complete server-side and redirect with a code, or return an OIDC error such asinteraction_required/consent_required.🤖 Prompt for 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. In `@internal/controller/oidc_controller.go` around lines 179 - 219, After the initial check for unauthenticated users with `prompt=none`, add additional logic to handle authenticated users who also have `prompt=none` set. When an authenticated user requests authorization with `prompt=none`, the request should not proceed to create an authorize ticket and redirect to the interactive `/oidc/authorize` screen. Instead, handle it non-interactively by either completing the authorization server-side and redirecting with an authorization code, or returning an OIDC error such as `interaction_required` or `consent_required` via the controller.authorizeError method. Add this check immediately after the existing `prompt=none` and unauthenticated validation and before the ticket creation logic.
🤖 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 `@internal/controller/oidc_controller.go`:
- Around line 179-201: The current code uses exact string comparisons for
req.Prompt ("none" and "login"), but the OIDC prompt parameter is
space-delimited and can contain multiple values. Parse req.Prompt using
strings.Fields to extract individual tokens, then validate that if "none" is
present it cannot be combined with other values, and branch on token membership
instead of exact string equality. Update the condition checking req.Prompt ==
"none" to check if "none" exists in the parsed tokens, update the condition
checking req.Prompt == "login" to check if "login" exists in the parsed tokens,
and add validation to reject invalid combinations where "none" appears with
other values.
---
Outside diff comments:
In `@internal/controller/oidc_controller.go`:
- Around line 179-219: After the initial check for unauthenticated users with
`prompt=none`, add additional logic to handle authenticated users who also have
`prompt=none` set. When an authenticated user requests authorization with
`prompt=none`, the request should not proceed to create an authorize ticket and
redirect to the interactive `/oidc/authorize` screen. Instead, handle it
non-interactively by either completing the authorization server-side and
redirecting with an authorization code, or returning an OIDC error such as
`interaction_required` or `consent_required` via the controller.authorizeError
method. Add this check immediately after the existing `prompt=none` and
unauthenticated validation and before the ticket creation logic.
In `@internal/service/oidc_service.go`:
- Around line 618-628: The CreateOIDCSession call is not persisting the AuthTime
from codeEntry, causing refreshed ID tokens to lose the auth_time claim. Add the
AuthTime field to the CreateOIDCSessionParams struct and pass codeEntry.AuthTime
when creating the OIDC session. Then update RefreshAccessToken to retrieve the
stored AuthTime from the OIDC session and pass it to generateIDToken instead of
passing 0, ensuring the auth_time claim is preserved across token refreshes.
🪄 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: 61178aab-f39a-4d47-813d-49d5d0f5193b
📒 Files selected for processing (6)
frontend/src/lib/hooks/screen-params.tsfrontend/src/pages/authorize-page.tsxfrontend/src/pages/login-page.tsxinternal/controller/oidc_controller.gointernal/model/context.gointernal/service/oidc_service.go
dosubot
Bot
added
size:L
Contributor
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 `@internal/service/oidc_service.go`:
- Around line 949-965: The GetPrompt function does not validate the OIDC Core
spec requirement that prompt=none cannot be combined with other prompt values.
Add validation logic within the GetPrompt function to detect when the input
contains "none" alongside other prompts and reject this invalid combination.
After splitting the prompt string, check if both "none" and other supported
prompts are present in the list, and if so, return an error indicator or
empty/invalid state rather than proceeding to return the first supported prompt.
This ensures requests like "prompt=none login" are properly rejected as
invalid_request instead of being accepted.
🪄 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: 21f44d18-3908-4622-a031-980dc2766de5
📒 Files selected for processing (5)
frontend/src/lib/hooks/screen-params.tsfrontend/src/pages/authorize-page.tsxfrontend/src/pages/login-page.tsxinternal/controller/oidc_controller.gointernal/service/oidc_service.go
Comment on lines
+949
to
+965
|
|
||
| // Return the first prompt in the list of prompts, or an empty string if no prompt is specified | ||
| func (service *OIDCService) GetPrompt(prompt string) OIDCPrompt { | ||
| if prompt == "" { | ||
| return "" | ||
| } | ||
|
|
||
| prompts := strings.Split(prompt, " ") | ||
|
|
||
| for _, p := range prompts { | ||
| if slices.Contains(SupportedPrompts, p) { | ||
| return OIDCPrompt(p) | ||
| } | ||
| } | ||
|
|
||
| return "" | ||
| } |
Contributor
There was a problem hiding this comment.
Missing validation for prompt=none combined with other values.
Per OIDC Core spec section 3.1.2.1, prompt=none "MUST NOT be present with any other value." The current implementation returns the first supported prompt without validating this constraint. A request like prompt=none login would be accepted (returning "none"), but should be rejected as invalid_request.
Consider adding validation logic:
Suggested fix
func (service *OIDCService) GetPrompt(prompt string) OIDCPrompt {
if prompt == "" {
return ""
}
prompts := strings.Split(prompt, " ")
+ // OIDC spec: prompt=none MUST NOT be combined with other values
+ if slices.Contains(prompts, string(OIDCPromptNone)) && len(prompts) > 1 {
+ return "" // Or return a special error indicator
+ }
+
for _, p := range prompts {
if slices.Contains(SupportedPrompts, p) {
return OIDCPrompt(p)
}
}
return ""
}Alternatively, return an error from this function, or add a separate ValidatePrompt function that the controller calls to reject invalid combinations with an invalid_request error response.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Return the first prompt in the list of prompts, or an empty string if no prompt is specified | |
| func (service *OIDCService) GetPrompt(prompt string) OIDCPrompt { | |
| if prompt == "" { | |
| return "" | |
| } | |
| prompts := strings.Split(prompt, " ") | |
| for _, p := range prompts { | |
| if slices.Contains(SupportedPrompts, p) { | |
| return OIDCPrompt(p) | |
| } | |
| } | |
| return "" | |
| } | |
| // Return the first prompt in the list of prompts, or an empty string if no prompt is specified | |
| func (service *OIDCService) GetPrompt(prompt string) OIDCPrompt { | |
| if prompt == "" { | |
| return "" | |
| } | |
| prompts := strings.Split(prompt, " ") | |
| // OIDC spec: prompt=none MUST NOT be combined with other values | |
| if slices.Contains(prompts, string(OIDCPromptNone)) && len(prompts) > 1 { | |
| return "" // Or return a special error indicator | |
| } | |
| for _, p := range prompts { | |
| if slices.Contains(SupportedPrompts, p) { | |
| return OIDCPrompt(p) | |
| } | |
| } | |
| return "" | |
| } |
🤖 Prompt for 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.
In `@internal/service/oidc_service.go` around lines 949 - 965, The GetPrompt
function does not validate the OIDC Core spec requirement that prompt=none
cannot be combined with other prompt values. Add validation logic within the
GetPrompt function to detect when the input contains "none" alongside other
prompts and reject this invalid combination. After splitting the prompt string,
check if both "none" and other supported prompts are present in the list, and if
so, return an error indicator or empty/invalid state rather than proceeding to
return the first supported prompt. This ensures requests like "prompt=none
login" are properly rejected as invalid_request instead of being accepted.
Rycochet
approved these changes
Jun 19, 2026
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 by CodeRabbit
New Features
promptparameter (none/login) and propagated it through URL parameters, UI flow, and backend authorization.auth_time.Improvements
prompt=noneauthorization now returns a standardized “login required” error when the user can’t be silently authenticated.prompt=noneis combined with other prompt values, the request is rejected as invalid.