Feature: Allow sending static key/value pairs to the configured exter…

…nal IdP for authorization code flow Add parameter additionalAuthzParameters (additional authorization parameters) to requests for auth code flow. The usage is: OIDC IdPs have additional parameters, e.g. UAA itself has token_format to get special features. The new parameter allows to customize the IdP with own parameters
cloudfoundry · Jul 11, 2023 · 926d119 · 926d119
1 parent 49f4543
commit 926d119
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 16 deletions.
diff --git a/...loudfoundry/identity/uaa/provider/oauth/ExternalOAuthIdentityProviderConfigValidator.java b/...loudfoundry/identity/uaa/provider/oauth/ExternalOAuthIdentityProviderConfigValidator.java
@@ -1,19 +1,24 @@
 package org.cloudfoundry.identity.uaa.provider.oauth;
 
-import org.cloudfoundry.identity.uaa.provider.AbstractIdentityProviderDefinition;
 import org.cloudfoundry.identity.uaa.provider.AbstractExternalOAuthIdentityProviderDefinition;
+import org.cloudfoundry.identity.uaa.provider.AbstractIdentityProviderDefinition;
 import org.cloudfoundry.identity.uaa.provider.BaseIdentityProviderValidator;
 import org.cloudfoundry.identity.uaa.provider.OIDCIdentityProviderDefinition;
 import org.springframework.stereotype.Component;
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Set;
 
 import static org.springframework.util.StringUtils.hasText;
 
 @Component
 public class ExternalOAuthIdentityProviderConfigValidator extends BaseIdentityProviderValidator {
 
+    private static final Set<String> oAuthStandardParameters = Set.of("redirect_uri", "code", "client_id", "client_secret", "response_type",
+        "grant_type", "code_verifier", "client_assertion", "client_assertion_type", "code_challenge", "code_challenge_method", "nonce", "state",
+        "scope", "assertion", "subject_token", "actor_token", "username", "password");
+
     @Override
     public void validate(AbstractIdentityProviderDefinition definition) {
         if (definition == null) {
@@ -26,19 +31,27 @@ public void validate(AbstractIdentityProviderDefinition definition) {
         AbstractExternalOAuthIdentityProviderDefinition def = (AbstractExternalOAuthIdentityProviderDefinition) definition;
 
         List<String> errors = new ArrayList<>();
-        if (def instanceof OIDCIdentityProviderDefinition && ((OIDCIdentityProviderDefinition) definition).getDiscoveryUrl() != null) {
-            //we don't require auth/token url or keys/key url
-        } else {
-            if (def.getAuthUrl() == null) {
-                errors.add("Authorization URL must be a valid URL");
-            }
+        if (def instanceof OIDCIdentityProviderDefinition) {
+            OIDCIdentityProviderDefinition oidcIdentityProviderDefinition = (OIDCIdentityProviderDefinition) def;
+            if (oidcIdentityProviderDefinition.getDiscoveryUrl() != null) {
+                //we don't require auth/token url or keys/key url
+            } else {
+                if (def.getAuthUrl() == null) {
+                    errors.add("Authorization URL must be a valid URL");
+                }
 
-            if (def.getTokenUrl() == null) {
-                errors.add("Token URL must be a valid URL");
+                if (def.getTokenUrl() == null) {
+                    errors.add("Token URL must be a valid URL");
+                }
+
+                if (!hasText(def.getTokenKey()) && def.getTokenKeyUrl() == null) {
+                    errors.add("Either token key or token key URL must be specified");
+                }
             }
 
-            if (!hasText(def.getTokenKey()) && def.getTokenKeyUrl() == null) {
-                errors.add("Either token key or token key URL must be specified");
+            if (oidcIdentityProviderDefinition.getAdditionalAuthzParameters() != null &&
+                oAuthStandardParameters.stream().anyMatch(oidcIdentityProviderDefinition.getAdditionalAuthzParameters().keySet()::contains)) {
+                errors.add("No OAuth standard parameters allowed in section additionalAuthzParameters");
             }
         }
 
@@ -56,4 +69,8 @@ public void validate(AbstractIdentityProviderDefinition definition) {
             throw new IllegalArgumentException("Invalid config for Identity Provider " + errorMessages);
         }
     }
+
+    protected static boolean isOAuthStandardParameter(String value) {
+        return oAuthStandardParameters.contains(value);
+    }
 }
diff --git a/...rc/main/java/org/cloudfoundry/identity/uaa/provider/oauth/OauthIDPWrapperFactoryBean.java b/...rc/main/java/org/cloudfoundry/identity/uaa/provider/oauth/OauthIDPWrapperFactoryBean.java
@@ -39,9 +39,6 @@
 public class OauthIDPWrapperFactoryBean {
     private Map<String, AbstractExternalOAuthIdentityProviderDefinition> oauthIdpDefinitions = new HashMap<>();
     private List<IdentityProviderWrapper> providers = new LinkedList<>();
-    private static final Set<String> oauthParameters = Set.of("redirect_uri", "code", "client_id", "client_secret", "response_type", "grant_type",
-        "code_verifier", "client_assertion", "client_assertion_type", "code_challenge", "code_challenge_method", "nonce", "state", "scope",
-        "assertion", "subject_token", "actor_token", "username", "password");
 
     public OauthIDPWrapperFactoryBean(Map<String, Map> definitions) {
         if (definitions != null) {
@@ -184,8 +181,8 @@ private static Map<String, String> parseAdditionalParameters(Map<String, Object>
                 } else if (entry.getValue() instanceof String) {
                     value = (String) entry.getValue();
                 }
-                // accept only custom parameters
-                if (value == null || oauthParameters.contains(keyEntry)) {
+                // accept only custom parameters, filter out standard parameters
+                if (value == null || ExternalOAuthIdentityProviderConfigValidator.isOAuthStandardParameter(keyEntry)) {
                     continue;
                 }
                 additionalQueryParameters.put(entry.getKey(), value);

diff --git a/...va/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManagerIT.java b/...va/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManagerIT.java
@@ -570,6 +570,7 @@ void discoveryURL_is_used() throws MalformedURLException {
         config.setAuthUrl(null);
         config.setTokenUrl(null);
         config.setDiscoveryUrl(new URL("http://some.discovery.url"));
+
         Map<String, Object> discoveryContent = new HashMap();
         discoveryContent.put("authorization_endpoint", authUrl.toString());
         discoveryContent.put("token_endpoint", tokenUrl.toString());

diff --git a/...foundry/identity/uaa/provider/oauth/ExternalOAuthIdentityProviderConfigValidatorTest.java b/...foundry/identity/uaa/provider/oauth/ExternalOAuthIdentityProviderConfigValidatorTest.java
@@ -8,6 +8,8 @@
 
 import java.net.MalformedURLException;
 import java.net.URL;
+import java.util.Collections;
+import java.util.Map;
 
 public class ExternalOAuthIdentityProviderConfigValidatorTest {
     private AbstractExternalOAuthIdentityProviderDefinition definition;
@@ -102,4 +104,26 @@ public void tokenKeyUrl_orTokenKeyMustBeSpecified() {
         definition.setTokenKeyUrl(null);
         validator.validate(definition);
     }
+
+    @Test
+    public void testAdditionalParametersAdd() {
+        OIDCIdentityProviderDefinition oidcIdentityProviderDefinition = (OIDCIdentityProviderDefinition) definition;
+        // nothing
+        oidcIdentityProviderDefinition.setAdditionalAuthzParameters(null);
+        validator.validate(definition);
+        // empty
+        oidcIdentityProviderDefinition.setAdditionalAuthzParameters(Collections.emptyMap());
+        validator.validate(definition);
+        // list
+        oidcIdentityProviderDefinition.setAdditionalAuthzParameters(Map.of("token_format", "jwt", "token_key", "any"));
+        validator.validate(definition);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testAdditionalParametersError() {
+        OIDCIdentityProviderDefinition oidcIdentityProviderDefinition = (OIDCIdentityProviderDefinition) definition;
+        // one standard parameter, should fail
+        oidcIdentityProviderDefinition.setAdditionalAuthzParameters(Map.of("token_format", "jwt", "code", "1234"));
+        validator.validate(definition);
+    }
 }