Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Do not evaluate skipApproval on approval page #3086

Merged
merged 1 commit into from
Feb 13, 2024

Conversation

MM53
Copy link
Contributor

@MM53 MM53 commented Aug 25, 2023

Overview

Remove evaluation of skipApproval on approval page

What this PR does / why we need it

The Pull Request #2897 caused a bug when using oauth2.skipApprovalScreen: true together with a client setting approval_prompt=force during the request. The tests validated correctly that the user gets redirected to /approval in this case, but the handler of the approval page contains some code which immediately returns the auth code if oauth2.skipApprovalScreen: true is set and ignores the approval_prompt parameter. Since there is a TODO comment to remove this check after some time, I guess it can be removed now to fix that issue.

Special notes for your reviewer

Does this PR introduce a user-facing change?

Fix a bug that Dex ignores approval_prompt=force when skipping approval screen is enabled as default behavior

Signed-off-by: MM53 <28218664+MM53@users.noreply.github.com>
@MM53 MM53 mentioned this pull request Sep 26, 2023
3 tasks
Copy link
Member

@nabokihms nabokihms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MM53 sorry for the long delay. It does make sense. Thank you for your contribution.

@nabokihms nabokihms merged commit a0cf8b2 into dexidp:master Feb 13, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants