PKCE support in IDP (OIDC) proxy authorization flow (cloudfoundry#1606)

* [WIP] add PKCE support in IDP proxy authorization flow * [WIP] add PKCE support in IDP proxy authorization flow * minor refactoring * cleanup * add documentation and integration guides
GESoftware-CF · Aug 5, 2021 · c70b9f6 · c70b9f6
1 parent 6b1ac15
commit c70b9f6
Show file tree

Hide file tree

Showing 12 changed files with 175 additions and 14 deletions.
diff --git a/docs/okta-public-oidc-provider.md b/docs/okta-public-oidc-provider.md
@@ -0,0 +1,33 @@
+# Registering Okta as external, public OIDC provider in UAA
+
+Okta can be setup as an [OIDC provider](https://developer.okta.com/docs/guides/add-an-external-idp/openidconnect/configure-idp-in-okta/) for UAA login.
+In order to prevent storing a client secret in UAA configuration and all of it's successor problems like secret rotation and so on, register the
+external OIDC provider with a public client.
+
+1. Create an OIDC application and set it with [PKCE public](https://developer.okta.com/blog/2019/08/22/okta-authjs-pkce#use-pkce-to-make-your-apps-more-secure).
+   Register the "Redirect URIs" in the application section "OpenID Connect Configuration"
+
+   Add following URI in list field:
+   `http://{UAA_HOST}/login/callback/{origin}`. [Additional documentation for achieving this can be found here](https://developer.okta.com/docs/guides/implement-auth-code-pkce/overview/).
+
+2. Copy client id.
+
+3. Minimal OIDC configuration needs to be added in login.ym.
+   Read configuration refer to 'https://<your-tenant>.okta.com/.well-known/openid-configuration' for discoveryUrl and issuer
+
+        login:
+          oauth:
+            providers:
+              okta.public:
+                type: oidc1.0
+                discoveryUrl: https://trailaccount.okta.com/.well-known/openid-configuration
+                issuer: https://trailaccount.okta.com
+                scopes:
+                  - openid
+                linkText: Login with Okta-Public
+                showLinkText: true
+                relyingPartyId: 0iak4aiaC4HV39L6g123
+
+4. Ensure that the scope `openid` is included in the`scopes` property.
+
+5. Restart UAA. You will see `Login with Okta-Public` link on your login page.
diff --git a/docs/sap-public-oidc-provider.md b/docs/sap-public-oidc-provider.md
@@ -0,0 +1,36 @@
+# Registering SAP IAS as external, public OIDC provider in UAA
+
+SAP IAS can be setup as an [OIDC provider](https://help.sap.com/viewer/6d6d63354d1242d185ab4830fc04feb1/Cloud/en-US/a789c9c8c0f5439da8c30b5d9e43bece.htm) for UAA login.
+In order to prevent storing a client secret in UAA configuration and all of it's successor problems like secret rotation and so on, register the
+external OIDC provider with a public client.
+
+1. Create an OIDC application and set it with [type public](https://help.sap.com/viewer/6d6d63354d1242d185ab4830fc04feb1/Cloud/en-US/a721157cd40544eb9bad40085cf8ec15.html).
+   Register the "Redirect URIs" in the application section "OpenID Connect Configuration"
+
+   Add following URI in list field:
+   `http://{UAA_HOST}/login/callback/{origin}`. [Additional documentation for achieving this can be found here](https://help.sap.com/viewer/6d6d63354d1242d185ab4830fc04feb1/Cloud/en-US/1ae324ee3b2d4a728650eb022d5fd910.html).
+
+2. Copy client id.
+
+3. Minimal OIDC configuration needs to be added in login.ym.
+   Read configuration refer to '[https://<tenant ID>.accounts.ondemand.com/.well-known/openid-configuration](https://help.sap.com/viewer/6d6d63354d1242d185ab4830fc04feb1/Cloud/en-US/c297516bae4547eb82eeed80fea2b937.html)' for discoveryUrl and issuer
+
+        login:
+          oauth:
+            providers:
+              ias.public:
+                type: oidc1.0
+                discoveryUrl: https://trailaccount.accounts.ondemand.com/.well-known/openid-configuration
+                issuer: https://trailaccount.accounts.ondemand.com
+                scopes:
+                  - openid
+                  - email
+                  - profile
+                linkText: Login with IAS-Public
+                showLinkText: true
+                relyingPartyId: 3feb7ecb-d106-4432-b335-aca2689ad123
+
+4. Ensure that the scope `openid`, `email` and `profile` is included in the`scopes` property. Then UAA shadow user (if addShadowUserOnLogin=true) is created
+   with all properties. 
+
+5. Restart UAA. You will see `Login with IAS-Public` link on your login page.
diff --git a/...er/src/main/java/org/cloudfoundry/identity/uaa/oauth/pkce/verifiers/S256PkceVerifier.java b/...er/src/main/java/org/cloudfoundry/identity/uaa/oauth/pkce/verifiers/S256PkceVerifier.java
@@ -28,18 +28,22 @@ public boolean verify(String codeVerifier, String codeChallenge) {
         if (codeVerifier == null || codeChallenge == null) {
             return false;
         }
+        return codeChallenge.contentEquals(compute(codeVerifier));
+    }
+
+    public String compute(String codeVerifier) {
         try {
             byte[] bytes = codeVerifier.getBytes("US-ASCII");
             MessageDigest md = MessageDigest.getInstance("SHA-256");
             md.update(bytes, 0, bytes.length);
             byte[] digest = md.digest();
-            return codeChallenge.contentEquals(Base64.encodeBase64URLSafeString(digest));
+            return Base64.encodeBase64URLSafeString(digest);
         } catch (UnsupportedEncodingException e) {
             logger.debug(e.getMessage(),e);
         } catch (NoSuchAlgorithmException e) {
-        	logger.debug(e.getMessage(),e);
+            logger.debug(e.getMessage(),e);
         }
-        return false;
+        return null;
     }
 
     @Override

diff --git a/...java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManager.java b/...java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManager.java
@@ -64,7 +64,7 @@
 import org.springframework.web.client.RestTemplate;
 import org.springframework.web.context.request.RequestAttributes;
 import org.springframework.web.context.request.RequestContextHolder;
-import org.springframework.web.servlet.support.RequestContextUtils;
+import org.springframework.web.context.request.ServletRequestAttributes;
 
 import java.net.URI;
 import java.net.URISyntaxException;
@@ -640,11 +640,18 @@ private String getTokenFromCode(ExternalOAuthCodeToken codeToken, AbstractExtern
 
         HttpHeaders headers = new HttpHeaders();
 
-        if(config.isClientAuthInBody()) {
-            body.add("client_secret", config.getRelyingPartySecret());
+        // no client-secret, switch to PKCE and treat client as public, same logic is implemented in spring security
+        // https://docs.spring.io/spring-security/site/docs/5.3.1.RELEASE/reference/html5/#initiating-the-authorization-request
+        if (config.getRelyingPartySecret() == null) {
+            // if session is expired or other issues in retrieven code_verifier, then flow fails with 401, which is expected
+            body.add("code_verifier", getSessionValue(SessionUtils.codeVerifierParameterAttributeKeyForIdp(codeToken.getOrigin())));
         } else {
-            String clientAuthHeader = getClientAuthHeader(config);
-            headers.add("Authorization", clientAuthHeader);
+            if (config.isClientAuthInBody()) {
+                body.add("client_secret", config.getRelyingPartySecret());
+            } else {
+                String clientAuthHeader = getClientAuthHeader(config);
+                headers.add("Authorization", clientAuthHeader);
+            }
         }
         headers.add("Accept", "application/json");
 
@@ -672,6 +679,16 @@ private String getTokenFromCode(ExternalOAuthCodeToken codeToken, AbstractExtern
         return responseEntity.getBody().get(getTokenFieldName(config));
     }
 
+    private String getSessionValue(String value) {
+        try {
+            ServletRequestAttributes attr = (ServletRequestAttributes) RequestContextHolder.currentRequestAttributes();
+            return (String) SessionUtils.getStateParam(attr.getRequest().getSession(false), value);
+        } catch (Exception e) {
+            logger.warn("Exception", e);
+            return (String)"";
+        }
+    }
+
     private String getClientAuthHeader(AbstractExternalOAuthIdentityProviderDefinition config) {
         String clientAuth = new String(Base64.encodeBase64((config.getRelyingPartyId() + ":" + config.getRelyingPartySecret()).getBytes()));
         return "Basic " + clientAuth;

diff --git a/...loudfoundry/identity/uaa/provider/oauth/ExternalOAuthIdentityProviderConfigValidator.java b/...loudfoundry/identity/uaa/provider/oauth/ExternalOAuthIdentityProviderConfigValidator.java
@@ -46,10 +46,6 @@ public void validate(AbstractIdentityProviderDefinition definition) {
             errors.add("Relying Party Id must be the client-id for the UAA that is registered with the external IDP");
         }
 
-        if (!hasText(def.getRelyingPartySecret()) && !def.getResponseType().contains("token")) {
-            errors.add("Relying Party Secret must be the client-secret for the UAA that is registered with the external IDP");
-        }
-
         if (def.isShowLinkText() && !hasText(def.getLinkText())) {
             errors.add("Link Text must be specified because showLinkText is true");
         }

diff --git a/.../java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthProviderConfigurator.java b/.../java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthProviderConfigurator.java
@@ -1,5 +1,6 @@
 package org.cloudfoundry.identity.uaa.provider.oauth;
 
+import org.cloudfoundry.identity.uaa.oauth.pkce.verifiers.S256PkceVerifier;
 import org.cloudfoundry.identity.uaa.provider.AbstractExternalOAuthIdentityProviderDefinition;
 import org.cloudfoundry.identity.uaa.provider.IdentityProvider;
 import org.cloudfoundry.identity.uaa.provider.IdentityProviderProvisioning;
@@ -73,6 +74,17 @@ public String getIdpAuthenticationUrl(
                 .queryParam("redirect_uri", callbackUrl)
                 .queryParam("state", state);
 
+        // no client-secret, switch to PKCE and treat client as public, same logic is implemented in spring security
+        // https://docs.spring.io/spring-security/site/docs/5.3.1.RELEASE/reference/html5/#initiating-the-authorization-request
+        if (definition.getRelyingPartySecret() == null) {
+            var pkceVerifier = new S256PkceVerifier();
+            var codeVerifier = generateCodeVerifier();
+            var codeChallenge = pkceVerifier.compute(codeVerifier);
+            SessionUtils.setStateParam(request.getSession(), SessionUtils.codeVerifierParameterAttributeKeyForIdp(idpOriginKey), codeVerifier);
+            uriBuilder.queryParam("code_challenge", codeChallenge);
+            uriBuilder.queryParam("code_challenge_method", pkceVerifier.getCodeChallengeMethod());
+        }
+
         if (!CollectionUtils.isEmpty(definition.getScopes())) {
             uriBuilder.queryParam("scope", URLEncoder.encode(String.join(" ", definition.getScopes()), StandardCharsets.UTF_8));
         }
@@ -89,6 +101,10 @@ private String generateStateParam() {
         return uaaRandomStringUtil.getSecureRandom(10);
     }
 
+    private String generateCodeVerifier() {
+        return uaaRandomStringUtil.getSecureRandom(128);
+    }
+
     private String getCallbackUrlForIdp(String idpOriginKey, String uaaBaseUrl) {
         return URLEncoder.encode(uaaBaseUrl + "/login/callback/" + idpOriginKey, StandardCharsets.UTF_8);
     }

diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/util/SessionUtils.java b/server/src/main/java/org/cloudfoundry/identity/uaa/util/SessionUtils.java
@@ -24,6 +24,7 @@ public final class SessionUtils {
     public static final String SPRING_SECURITY_CONTEXT = "SPRING_SECURITY_CONTEXT";
 
     private static final String EXTERNAL_OAUTH_STATE_ATTRIBUTE_PREFIX = "external-oauth-state-";
+    private static final String EXTERNAL_OAUTH_CODE_VERIFIER_ATTRIBUTE_PREFIX = "external-oauth-verifier-";
 
     private SessionUtils() {}
 
@@ -80,4 +81,8 @@ public static AuthenticationException getAuthenticationException(HttpSession ses
     public static String stateParameterAttributeKeyForIdp(String idpOriginKey) {
         return EXTERNAL_OAUTH_STATE_ATTRIBUTE_PREFIX + idpOriginKey;
     }
+
+    public static String codeVerifierParameterAttributeKeyForIdp(String idpOriginKey) {
+        return EXTERNAL_OAUTH_CODE_VERIFIER_ATTRIBUTE_PREFIX + idpOriginKey;
+    }
 }
diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpointTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/login/LoginInfoEndpointTests.java
@@ -425,6 +425,7 @@ void discoverIdentityProviderCarriesUsername() throws MalformedURLException {
         when(idpConfig.getAuthUrl()).thenReturn(new URL("https://example.com/oauth/authorize"));
         when(idpConfig.getResponseType()).thenReturn("code");
         when(idpConfig.getRelyingPartyId()).thenReturn("clientid");
+        when(idpConfig.getRelyingPartySecret()).thenReturn("clientSecret");
         when(idpConfig.getUserPropagationParameter()).thenReturn("username");
         when(idp.getConfig()).thenReturn(idpConfig);
         when(mockIdentityProviderProvisioning.retrieveActive("uaa")).thenReturn(Collections.singletonList(idp));
@@ -904,6 +905,7 @@ void oauth_provider_links_shown() throws Exception {
 
         definition.setAuthUrl(new URL("http://auth.url"));
         definition.setTokenUrl(new URL("http://token.url"));
+        definition.setRelyingPartySecret("client-secret");
 
         IdentityProvider<AbstractExternalOAuthIdentityProviderDefinition> identityProvider = MultitenancyFixture.identityProvider("oauth-idp-alias", "uaa");
         identityProvider.setConfig(definition);
@@ -1054,6 +1056,7 @@ void loginHintEmailDomain() throws Exception {
         AbstractExternalOAuthIdentityProviderDefinition mockOidcConfig = mock(OIDCIdentityProviderDefinition.class);
         when(mockOidcConfig.getAuthUrl()).thenReturn(new URL("http://localhost:8080/uaa"));
         when(mockOidcConfig.getRelyingPartyId()).thenReturn("client-id");
+        when(mockOidcConfig.getRelyingPartySecret()).thenReturn("client-secret");
         when(mockOidcConfig.getResponseType()).thenReturn("token");
         when(mockOidcConfig.getEmailDomain()).thenReturn(singletonList("example.com"));
         when(mockProvider.getConfig()).thenReturn(mockOidcConfig);
@@ -1890,6 +1893,7 @@ private static IdentityProvider createOIDCIdentityProvider(String originKey) thr
         oidcIdentityProvider.setType(OriginKeys.OIDC10);
         OIDCIdentityProviderDefinition definition = new OIDCIdentityProviderDefinition();
         definition.setAuthUrl(new URL("https://" + originKey + ".com"));
+        definition.setRelyingPartySecret("client-secret");
         oidcIdentityProvider.setConfig(definition);
 
         return oidcIdentityProvider;
@@ -1919,6 +1923,7 @@ private static void mockOidcProvider(IdentityProviderProvisioning mockIdentityPr
         AbstractExternalOAuthIdentityProviderDefinition mockOidcConfig = mock(OIDCIdentityProviderDefinition.class);
         when(mockOidcConfig.getAuthUrl()).thenReturn(new URL("http://localhost:8080/uaa"));
         when(mockOidcConfig.getRelyingPartyId()).thenReturn("client-id");
+        when(mockOidcConfig.getRelyingPartySecret()).thenReturn("client-secret");
         when(mockOidcConfig.getResponseType()).thenReturn("token");
         when(mockProvider.getConfig()).thenReturn(mockOidcConfig);
         when(mockOidcConfig.isShowLinkText()).thenReturn(true);
@@ -1933,6 +1938,7 @@ private static void mockLoginHintProvider(ExternalOAuthProviderConfigurator mock
         AbstractExternalOAuthIdentityProviderDefinition mockOidcConfig = mock(OIDCIdentityProviderDefinition.class);
         when(mockOidcConfig.getAuthUrl()).thenReturn(new URL("http://localhost:8080/uaa"));
         when(mockOidcConfig.getRelyingPartyId()).thenReturn("client-id");
+        when(mockOidcConfig.getRelyingPartySecret()).thenReturn("client-secret");
         when(mockOidcConfig.getResponseType()).thenReturn("token");
         when(mockProvider.getConfig()).thenReturn(mockOidcConfig);
         when(mockOidcConfig.isShowLinkText()).thenReturn(true);

diff --git a/...va/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManagerIT.java b/...va/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManagerIT.java
@@ -37,6 +37,7 @@
 import org.cloudfoundry.identity.uaa.user.UaaUserPrototype;
 import org.cloudfoundry.identity.uaa.user.UserInfo;
 import org.cloudfoundry.identity.uaa.util.JsonUtils;
+import org.cloudfoundry.identity.uaa.util.SessionUtils;
 import org.cloudfoundry.identity.uaa.util.TimeServiceImpl;
 import org.cloudfoundry.identity.uaa.util.UaaRandomStringUtil;
 import org.cloudfoundry.identity.uaa.util.UaaTokenUtils;
@@ -608,6 +609,27 @@ void clientAuthInBody_is_used() {
         mockUaaServer.verify();
     }
 
+    @Test
+    void pkceClientAuthInBody_is_used() {
+        config.setClientAuthInBody(true);
+        mockUaaServer.expect(requestTo(config.getTokenUrl().toString()))
+            .andExpect(request -> assertThat("Check Auth header not present", request.getHeaders().get("Authorization"), nullValue()))
+            .andExpect(content().string(containsString("client_id=" + config.getRelyingPartyId())))
+            .andRespond(withStatus(OK).contentType(APPLICATION_JSON).body(getIdTokenResponse()));
+        IdentityProvider<AbstractExternalOAuthIdentityProviderDefinition> identityProvider = getProvider();
+        when(provisioning.retrieveByOrigin(eq(ORIGIN), anyString())).thenReturn(identityProvider);
+
+        config.setRelyingPartySecret(null);
+        RequestAttributes attributes = new ServletRequestAttributes(new MockHttpServletRequest());
+        attributes.setAttribute(SessionUtils.codeVerifierParameterAttributeKeyForIdp("uaa"), "code_verifier", RequestAttributes.SCOPE_SESSION);
+        RequestContextHolder.setRequestAttributes(attributes);
+
+        Map<String, Object> idToken = externalOAuthAuthenticationManager.getClaimsFromToken(xCodeToken, config);
+        assertNotNull(idToken);
+
+        mockUaaServer.verify();
+    }
+
     @Test
     void idToken_In_Redirect_Should_Use_it() {
         mockToken();

diff --git a/...foundry/identity/uaa/provider/oauth/ExternalOAuthIdentityProviderConfigValidatorTest.java b/...foundry/identity/uaa/provider/oauth/ExternalOAuthIdentityProviderConfigValidatorTest.java
@@ -59,7 +59,7 @@ public void configWithNullRelyingPartyId_ThrowsException() {
         validator.validate(definition);
     }
 
-    @Test(expected = IllegalArgumentException.class)
+    @Test
     public void configWithNullRelyingPartySecret_ThrowsException() {
         definition.setRelyingPartySecret(null);
         validator = new ExternalOAuthIdentityProviderConfigValidator();

diff --git a/.../org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthProviderConfiguratorTests.java b/.../org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthProviderConfiguratorTests.java
@@ -40,6 +40,7 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.Mockito.anyString;
 import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.doThrow;
@@ -85,6 +86,7 @@ void setup() throws MalformedURLException {
             def.setTokenKeyUrl(new URL("http://oidc10.random-made-up-url.com/token_keys"));
             def.setScopes(Arrays.asList("openid", "password.write"));
             def.setRelyingPartyId("clientId");
+            def.setRelyingPartySecret("clientSecret");
         }
         oidc.setResponseType("id_token code");
         oauth.setResponseType("code");
@@ -235,6 +237,30 @@ void getIdpAuthenticationUrl_doesNotIncludeNonceOnOAuth() {
         assertThat(queryParams, not(hasKey("nonce")));
     }
 
+    @Test
+    void getIdpAuthenticationUrl_includesPkceOnPublicOIDC() {
+        oidc.setRelyingPartySecret(null); // public client means no secret
+        when(mockUaaRandomStringUtil.getSecureRandom(anyInt())).thenReturn("01234567890123456789012345678901234567890123456789");
+        String authzUri = configurator.getIdpAuthenticationUrl(oidc, "alias", mockHttpServletRequest);
+
+        Map<String, String> queryParams =
+            UriComponentsBuilder.fromUriString(authzUri).build().getQueryParams().toSingleValueMap();
+        assertThat(queryParams, hasKey("code_challenge"));
+        assertThat(queryParams, hasKey("code_challenge_method"));
+    }
+
+    @Test
+    void getIdpAuthenticationUrl_includesPkceOnPublicOAuth() {
+        oauth.setRelyingPartySecret(null); // public client means no secret
+        when(mockUaaRandomStringUtil.getSecureRandom(anyInt())).thenReturn("01234567890123456789012345678901234567890123456789");
+        String authzUri = configurator.getIdpAuthenticationUrl(oauth, "alias", mockHttpServletRequest);
+
+        Map<String, String> queryParams =
+            UriComponentsBuilder.fromUriString(authzUri).build().getQueryParams().toSingleValueMap();
+        assertThat(queryParams, hasKey("code_challenge"));
+        assertThat(queryParams, hasKey("code_challenge_method"));
+    }
+
     @Test
     void getIdpAuthenticationUrl_withOnlyDiscoveryUrlForOIDCProvider() throws MalformedURLException, OidcMetadataFetchingException {
         String discoveryUrl = "https://accounts.google.com/.well-known/openid-configuration";

diff --git a/.../test/java/org/cloudfoundry/identity/uaa/mock/providers/IdentityProviderEndpointDocs.java b/.../test/java/org/cloudfoundry/identity/uaa/mock/providers/IdentityProviderEndpointDocs.java
@@ -136,7 +136,7 @@ class IdentityProviderEndpointDocs extends EndpointDocs {
             ATTRIBUTE_MAPPING_CUSTOM_ATTRIBUTES_DEPARTMENT
     };
 
-    private FieldDescriptor relyingPartySecret = fieldWithPath("config.relyingPartySecret").required().type(STRING).description("The client secret of the relying party at the external OAuth provider");
+    private FieldDescriptor relyingPartySecret = fieldWithPath("config.relyingPartySecret").optional().type(STRING).description("The client secret of the relying party at the external OAuth provider. If not set, the external OAuth client is treated as public client, then the flow is protected with [PKCE](https://tools.ietf.org/html/rfc7636) using code challenge method `S256`.");
 
     private static InMemoryLdapServer ldapContainer;