Adds option to support IdP that require PKCE for confidential clients.

[janhh]

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: https://github.com/spring-projects/spring-security/issues/6548

[janhh]

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)

If the OAuth 2.0 Provider supports PKCE for Confidential Clients, you may (optionally) configure it using DefaultServerOAuth2AuthorizationRequestResolver.setAuthorizationRequestCustomizer(OAuth2AuthorizationRequestCustomizers.withPkce()).

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:

    private final DefaultOAuth2AuthorizationRequestResolver delegate;

    public DelegatedOAuth2AuthorizationRequestResolver(ClientRegistrationRepository clientRegistrationRepository, String authorizationRequestBaseUri, boolean pkceAlways) {
        delegate = new DefaultOAuth2AuthorizationRequestResolver(clientRegistrationRepository, authorizationRequestBaseUri);
        if (pkceAlways) {
            //delegate.setAuthorizationRequestCustomizer(OAuth2AuthorizationRequestCustomizers.withPkce());
                       DefaultServerOAuth2AuthorizationRequestResolver.setAuthorizationRequestCustomizer(OAuth2AuthorizationRequestCustomizers.withPkce())
        }
       }

[LEDfan]

Hi

Thanks for opening this PR! This is valuable improvement for our openid integration and was on our wishlist as well. For the next release of ContainerProxy/ShinyProxy we will again update Spring Boot. Currently, I believe we will upgrade to at least spring boot 2.7, if we don't encounter any issue with it. So, I'll make sure to use the native solution of Spring Boot.

[janhh]

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.

    private final DefaultOAuth2AuthorizationRequestResolver delegate;

    public DelegatedOAuth2AuthorizationRequestResolver(ClientRegistrationRepository clientRegistrationRepository, String authorizationRequestBaseUri, boolean pkceAlways) {
        delegate = new DefaultOAuth2AuthorizationRequestResolver(clientRegistrationRepository, authorizationRequestBaseUri);
        if (pkceAlways) {
            delegate.setAuthorizationRequestCustomizer(OAuth2AuthorizationRequestCustomizers.withPkce());
        }
    }

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.)

[LEDfan]

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.

[janhh]

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):

2022-06-17 11:41:42.317  INFO 30196 --- [           main] ConditionEvaluationReportLoggingListener : 

Error starting ApplicationContext. To display the conditions report re-run your application with 'debug' enabled.
2022-06-17 11:41:42.339 ERROR 30196 --- [           main] o.s.b.d.LoggingFailureAnalysisReporter   : 

***************************
APPLICATION FAILED TO START
***************************

Description:

The dependencies of some of the beans in the application context form a cycle:

ΓöîΓöÇΓöÇΓöÇΓöÇΓöÇΓöÉ
|  containerProxyApplication (field private eu.openanalytics.containerproxy.util.ProxyMappingManager eu.openanalytics.containerproxy.ContainerProxyApplication.mappingManager)
Γåæ     Γåô
|  proxyMappingManager (field private eu.openanalytics.containerproxy.service.hearbeat.HeartbeatService eu.openanalytics.containerproxy.util.ProxyMappingManager.heartbeatService)
ΓööΓöÇΓöÇΓöÇΓöÇΓöÇΓöÿ

Action:

Relying upon circular references is discouraged and they are prohibited by default. Update your application to remove the dependency cycle between beans. As a last resort, it may be possible to break the cycle automatically by setting spring.main.allow-circular-references to true.

UPDATE: I could log in after setting spring.main.allow-circular-references to true as suggested in the log output.

[janhh]

I've updated the PR, significantly smaller commit than original solution.

1. Renamed boolean configuration proxy.openid.pkce-always to proxy.openid.pkce-confidential-clients. Maybe configuration option could also be named proxy.openid.with-pkce, I am not sure what is most suitable.
2. Reverted rename of DelegatedOAuth2AuthorizationRequestResolver.java back to FixedDefaultOAuth2AuthorizationRequestResolver.java
3. Refactored FixedDefaultOAuth2AuthorizationRequestResolver.java a bit, implemented native "withPkce" solution.
4. Deleted AlwaysPkceAuthorizationRequestResolver.java.

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.

[LEDfan]

Hi @janhh , thanks again for the contribution! This is now part of ShinyProxy 3.0.0 which we just released: https://github.com/openanalytics/containerproxy/commit/b9343d1d5177c08233d426cd31bf64bc124e826d