-
Notifications
You must be signed in to change notification settings - Fork 158
Add require_prompt as an option #492
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
base: develop
Are you sure you want to change the base?
Conversation
@jirian have you considered just using the plugin hooks and custom settings features to add this yourself via a Must Use Plugin? I'm not saying that we won't add this feature, however the plugin is very customizable already to add such a feature without needing to modify the plugin code. |
@timnolte I have now :), but I still feel that since this is a direct part of the OpenID Connect spec, it should be a part of the plugin itself. |
@jirian excellent! I hadn't taken a look at the spec yet to determine whether this was actually part of the spec or possibly something to a specific IDP. When things seem to be specific to an IDP I'm hesitant to incorporate it directly into the plugin. It looks like there are failing checks can you double check and run PHPCS to ensure that your code changes meet standards? I think there is something odd going on with PHPStan, I'll check into that. |
@timnolte I understand that. Just in case, I wrote a must-use plugin too (https://github.com/jirian/oidc-require-prompt-addon), but this really is a direct part of the spec, not a IdP-specific thing. I linted the code using phpcs, it should pass the checks now. |
@jirian not sure what's going on with the static analysis. GitHub Actions & local runs on the current |
@timnolte I'm not sure either, it's reporting the errors on lines that I didn't change at all. I have no clue what is going on. |
All Submissions:
Changes proposed in this Pull Request:
This PR adds a "require_prompt" option in the settings. If enabled, it adds the prompt=login parameter to the authentication URL forcing the IdP to show the login form every time, regardless of whether the user checked "Remember Me".
How to test the changes in this Pull Request:
Other information:
Changelog entry
Adds a "require_prompt" option in the settings. If enabled, it adds the prompt=login parameter to the authentication URL forcing the IdP to show the login form every time, regardless of whether the user checked "Remember Me".