From 926d1197d332974818939c1abbf7cd203864805b Mon Sep 17 00:00:00 2001 From: strehle Date: Tue, 11 Jul 2023 08:11:44 +0200 Subject: [PATCH] Feature: Allow sending static key/value pairs to the configured external 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 --- ...lOAuthIdentityProviderConfigValidator.java | 39 +++++++++++++------ .../oauth/OauthIDPWrapperFactoryBean.java | 7 +--- .../ExternalOAuthAuthenticationManagerIT.java | 1 + ...thIdentityProviderConfigValidatorTest.java | 24 ++++++++++++ 4 files changed, 55 insertions(+), 16 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthIdentityProviderConfigValidator.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthIdentityProviderConfigValidator.java index facd699a1be..1cb359a66dc 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthIdentityProviderConfigValidator.java +++ b/server/src/main/java/org/cloudfoundry/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 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 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/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/OauthIDPWrapperFactoryBean.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/OauthIDPWrapperFactoryBean.java index 12f3cafddaa..bd244669358 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/OauthIDPWrapperFactoryBean.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/OauthIDPWrapperFactoryBean.java @@ -39,9 +39,6 @@ public class OauthIDPWrapperFactoryBean { private Map oauthIdpDefinitions = new HashMap<>(); private List providers = new LinkedList<>(); - private static final Set 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 definitions) { if (definitions != null) { @@ -184,8 +181,8 @@ private static Map parseAdditionalParameters(Map } 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/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManagerIT.java b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManagerIT.java index 887a9e8eab6..53a86c95fbf 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManagerIT.java +++ b/server/src/test/java/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 discoveryContent = new HashMap(); discoveryContent.put("authorization_endpoint", authUrl.toString()); discoveryContent.put("token_endpoint", tokenUrl.toString()); diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthIdentityProviderConfigValidatorTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthIdentityProviderConfigValidatorTest.java index a01e7e28fc3..24a1ee9973f 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthIdentityProviderConfigValidatorTest.java +++ b/server/src/test/java/org/cloudfoundry/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); + } }