-
Notifications
You must be signed in to change notification settings - Fork 71
Adds option to support IdP that require PKCE for confidential clients. #69
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
Conversation
|
Currently ContainerProxy depends on Spring Security 5.5.5. Spring Security 5.7.1 comes with support for PKCE for confidential clients, how to enable is described in the documentation. There will still be need for a configuration setting and updating the resolver to enable PKCE for confidential clients. https://docs.spring.io/spring-security/reference/reactive/oauth2/client/authorization-grants.html#_initiating_the_authorization_request (scroll down to tip box)
When migrating to Spring Security 5.7.1 the AlwaysPkceAuthorizationRequestResolver.java in this patch will not be required, but maybe the DelegatedOAuth2AuthorizationRequestResolver.java (previously named FixedDefaultOAuth2AuthorizationRequestResolver.java) can to be updated to enable PKCE if the proxy.openid.pkce-always is true. Modification with Spring Security >5.7.1 mockup: |
|
Hi Thanks for opening this PR! This is valuable improvement for our openid integration and was on our wishlist as well. |
|
Thank you. I tried to upgrade to Spring Boot 2.7.0 and ContainerProxy seems to work with the original patch in this PR. I tried afterwards to change to the native PKCE for confidential clients and it built without problem, no syntax errors in the IDE. Unfortunately I ran into some issues when I tried to run ShinyProxy, so this needs some investigation. (Note that I renamed FixedDefaultOAuth2AuthorizationRequestResolver.java and using DelegatedOAuth2AuthorizationRequestResolver.java from the patch in the snippet above.) |
|
Did you also changed the spring boot version in the shinyproxy pom? https://github.com/openanalytics/shinyproxy/blob/master/pom.xml#L22 Otherwise, I'll have a look at the issue later. |
|
Thank you for the feedback. Yes I tried to update spring boot in shinyproxy as well. This was the error (some weird characters in git bash on windows here apparently): UPDATE: I could log in after setting spring.main.allow-circular-references to true as suggested in the log output. |
|
I've updated the PR, significantly smaller commit than original solution.
PR doesn't add Spring Boot 2.7.0 dependency, so I guess it can get it when it gets rebased on a later ContainerProxy version. |
…nts to "with-pkce".
Our client needs to use an Identity Provider which require the client to support PKCE for confidential clients. This is a feature that is on the Spring Security 5.7 milestone, but currently the solution is apparently to add a custom authorization request resolver.
This PR adds support for PKCE for confidential clients as described on the Okta Developer blog. This feature is enabled by setting proxy.openid.pkce-always=true. By default the opt in authorization request resolver will be ignored, and the DefaultOAuth2AuthorizationRequestResolver will be used as before.
For more information please see the following links.
Micah Silverman at Okta Developer Blog: https://developer.okta.com/blog/2020/01/23/pkce-oauth2-spring-boot
Spring Security 5.7 milestone: spring-projects/spring-security#6548