Skip to content
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

feature: Allow sending static key/value pairs to the configured IdP #2397

Merged
merged 16 commits into from
Jul 17, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,13 @@
import org.cloudfoundry.identity.uaa.login.Prompt;

import java.net.URL;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;

import static java.util.Collections.emptyMap;

@JsonIgnoreProperties(ignoreUnknown = true)
public class OIDCIdentityProviderDefinition extends AbstractExternalOAuthIdentityProviderDefinition<OIDCIdentityProviderDefinition>
Expand All @@ -31,6 +35,8 @@ public class OIDCIdentityProviderDefinition extends AbstractExternalOAuthIdentit
private List<Prompt> prompts = null;
@JsonInclude(JsonInclude.Include.NON_NULL)
private Object jwtClientAuthentication;
@JsonInclude(JsonInclude.Include.NON_NULL)
private Map<String, String> additionalAuthzParameters = null;

public URL getDiscoveryUrl() {
return discoveryUrl;
Expand Down Expand Up @@ -74,6 +80,14 @@ public void setJwtClientAuthentication(final Object jwtClientAuthentication) {
this.jwtClientAuthentication = jwtClientAuthentication;
}

public Map<String, String> getAdditionalAuthzParameters() {
return this.additionalAuthzParameters != null ? Collections.unmodifiableMap(this.additionalAuthzParameters) : null;
}

public void setAdditionalAuthzParameters(final Map<String, String> additonalAuthzParameters) {
this.additionalAuthzParameters = new HashMap<>(additonalAuthzParameters!=null?additonalAuthzParameters: emptyMap());
}

@Override
public Object clone() throws CloneNotSupportedException {
return super.clone();
Expand All @@ -90,6 +104,7 @@ public boolean equals(Object o) {
if (this.passwordGrantEnabled != that.passwordGrantEnabled) return false;
if (this.setForwardHeader != that.setForwardHeader) return false;
if (this.jwtClientAuthentication != that.jwtClientAuthentication) return false;
if (this.additionalAuthzParameters != that.additionalAuthzParameters) return false;
strehle marked this conversation as resolved.
Show resolved Hide resolved
return Objects.equals(discoveryUrl, that.discoveryUrl);

}
Expand All @@ -101,6 +116,7 @@ public int hashCode() {
result = 31 * result + (passwordGrantEnabled ? 1 : 0);
result = 31 * result + (setForwardHeader ? 1 : 0);
result = 31 * result + (jwtClientAuthentication != null ? jwtClientAuthentication.hashCode() : 0);
result = 31 * result + (additionalAuthzParameters != null ? additionalAuthzParameters.hashCode() : 0);
return result;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;

import static org.junit.Assert.assertTrue;

public class OIDCIdentityProviderDefinitionTests {

private final String defaultJson = "{\"emailDomain\":null,\"additionalConfiguration\":null,\"providerDescription\":null,\"externalGroupsWhitelist\":[],\"attributeMappings\":{},\"addShadowUserOnLogin\":true,\"storeCustomAttributes\":false,\"authUrl\":null,\"tokenUrl\":null,\"tokenKeyUrl\":null,\"tokenKey\":null,\"linkText\":null,\"showLinkText\":true,\"skipSslValidation\":false,\"relyingPartyId\":null,\"relyingPartySecret\":null,\"scopes\":null,\"issuer\":null,\"responseType\":\"code\",\"userInfoUrl\":null,\"jwtClientAuthentication\":false}";
private final String defaultJson = "{\"emailDomain\":null,\"additionalConfiguration\":null,\"providerDescription\":null,\"externalGroupsWhitelist\":[],\"attributeMappings\":{},\"addShadowUserOnLogin\":true,\"storeCustomAttributes\":false,\"authUrl\":null,\"tokenUrl\":null,\"tokenKeyUrl\":null,\"tokenKey\":null,\"linkText\":null,\"showLinkText\":true,\"skipSslValidation\":false,\"relyingPartyId\":null,\"relyingPartySecret\":null,\"scopes\":null,\"issuer\":null,\"responseType\":\"code\",\"userInfoUrl\":null,\"jwtClientAuthentication\":false,\"additonalAuthzParameters\":{\"token_format\":\"jwt\"}}";
strehle marked this conversation as resolved.
Show resolved Hide resolved
strehle marked this conversation as resolved.
Show resolved Hide resolved
String url = "https://accounts.google.com/.well-known/openid-configuration";

@Test
Expand All @@ -46,6 +46,14 @@ public void serialize_discovery_url() throws MalformedURLException {
assertEquals(url, def.getDiscoveryUrl().toString());
}

@Test
public void testSerializableObjectCalls() throws CloneNotSupportedException {
OIDCIdentityProviderDefinition def = JsonUtils.readValue(defaultJson, OIDCIdentityProviderDefinition.class);
OIDCIdentityProviderDefinition def2 = (OIDCIdentityProviderDefinition) def.clone();
assertTrue(def.equals(def2));
assertEquals(def.hashCode(), def2.hashCode());
strehle marked this conversation as resolved.
Show resolved Hide resolved
}

@Test
public void serialize_prompts() {
OIDCIdentityProviderDefinition def = JsonUtils.readValue(defaultJson, OIDCIdentityProviderDefinition.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -690,6 +690,15 @@ private String getTokenFromCode(ExternalOAuthCodeToken codeToken, AbstractExtern
logger.debug("Adding new client_id and client_secret for token exchange");
body.add("client_id", config.getRelyingPartyId());

if (config instanceof OIDCIdentityProviderDefinition) {
OIDCIdentityProviderDefinition oidcIdentityProviderDefinition = (OIDCIdentityProviderDefinition) config;
if (oidcIdentityProviderDefinition.getAdditionalAuthzParameters() != null){
for (Map.Entry<String, String> entry : oidcIdentityProviderDefinition.getAdditionalAuthzParameters().entrySet()) {
body.add(entry.getKey(), entry.getValue());
}
}
}

HttpHeaders headers = new HttpHeaders();

// no client-secret, switch to PKCE
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,12 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.stream.Collectors;

import static java.util.Collections.emptyList;
import static java.util.Collections.emptyMap;
import static java.util.Optional.ofNullable;
import static org.cloudfoundry.identity.uaa.constants.OriginKeys.OAUTH20;
import static org.cloudfoundry.identity.uaa.constants.OriginKeys.OIDC10;
Expand Down Expand Up @@ -92,6 +95,9 @@ public String getIdpAuthenticationUrl(
if (OIDCIdentityProviderDefinition.class.equals(definition.getParameterizedClass())) {
var nonceGenerator = new RandomValueStringGenerator(12);
uriBuilder.queryParam("nonce", nonceGenerator.generate());

Map<String, String> additionalParameters = ofNullable(((OIDCIdentityProviderDefinition) definition).getAdditionalAuthzParameters()).orElse(emptyMap());
additionalParameters.keySet().stream().filter(Objects::nonNull).forEach(e -> uriBuilder.queryParam(e, additionalParameters.get(e)));
strehle marked this conversation as resolved.
Show resolved Hide resolved
}

return uriBuilder.build().toUriString();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@
import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Set;

import static org.cloudfoundry.identity.uaa.constants.OriginKeys.OAUTH20;
import static org.cloudfoundry.identity.uaa.constants.OriginKeys.OIDC10;
Expand All @@ -37,6 +39,9 @@
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) {
Expand Down Expand Up @@ -142,8 +147,13 @@ protected void setCommonProperties(Map<String, Object> idpDefinitionMap, Abstrac
}
String discoveryUrl = (String) idpDefinitionMap.get("discoveryUrl");
try {
if (hasText(discoveryUrl) && idpDefinition instanceof OIDCIdentityProviderDefinition) {
((OIDCIdentityProviderDefinition) idpDefinition).setDiscoveryUrl(new URL(discoveryUrl));
OIDCIdentityProviderDefinition oidcIdentityProviderDefinition = null;
if (idpDefinition instanceof OIDCIdentityProviderDefinition) {
oidcIdentityProviderDefinition = (OIDCIdentityProviderDefinition) idpDefinition;
oidcIdentityProviderDefinition.setAdditionalAuthzParameters(parseAdditionalParameters(idpDefinitionMap));
}
if (hasText(discoveryUrl) && oidcIdentityProviderDefinition != null) {
oidcIdentityProviderDefinition.setDiscoveryUrl(new URL(discoveryUrl));
strehle marked this conversation as resolved.
Show resolved Hide resolved
} else {
idpDefinition.setAuthUrl(new URL((String) idpDefinitionMap.get("authUrl")));
idpDefinition.setTokenKeyUrl(idpDefinitionMap.get("tokenKeyUrl") == null ? null : new URL((String) idpDefinitionMap.get("tokenKeyUrl")));
Expand All @@ -159,6 +169,29 @@ protected void setCommonProperties(Map<String, Object> idpDefinitionMap, Abstrac
}
}

private static Map<String, String> parseAdditionalParameters(Map<String, Object> idpDefinitionMap) {
Map<String, Object> additionalParameters = (Map<String, Object>) idpDefinitionMap.get("additionalAuthzParameters");
if (additionalParameters != null) {
Map<String,String> additionalQueryParameters = new HashMap<>();
for (Map.Entry<String, Object> entry : additionalParameters.entrySet()) {
String keyEntry = entry.getKey().toLowerCase(Locale.ROOT);
String value = null;
if (entry.getValue() instanceof Integer) {
value = String.valueOf(entry.getValue());
} else if (entry.getValue() instanceof String) {
value = (String) entry.getValue();
}
// accept only custom parameters
if (value == null || oauthParameters.contains(keyEntry)) {
strehle marked this conversation as resolved.
Show resolved Hide resolved
continue;
}
additionalQueryParameters.put(entry.getKey(), value);
}
return additionalQueryParameters;
}
return null;
}

/* parse with null check because default should be null */
private AbstractExternalOAuthIdentityProviderDefinition.OAuthGroupMappingMode parseExternalGroupMappingMode(Object mode) {
if (mode instanceof String) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,7 @@ void setUp() throws Exception {
.setRelyingPartySecret("identitysecret")
.setUserInfoUrl(new URL("http://localhost/userinfo"))
.setTokenKey(PUBLIC_KEY);
config.setAdditionalAuthzParameters(Map.of("token_format", "jwt"));
strehle marked this conversation as resolved.
Show resolved Hide resolved
config.setExternalGroupsWhitelist(
Collections.singletonList(
"*"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ public void setup() throws Exception {
entry(GROUP_ATTRIBUTE_NAME, "roles")
);
oidcConfig.setAttributeMappings(externalGroupMapping);
oidcConfig.setAdditionalAuthzParameters(Map.of("token_format", "jwt"));
strehle marked this conversation as resolved.
Show resolved Hide resolved
provider.setConfig(oidcConfig);
when(identityProviderProvisioning.retrieveByOrigin(origin, zoneId)).thenReturn(provider);
uaaIssuerBaseUrl = "http://uaa.example.com";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ void setup() throws MalformedURLException {
config.setRelyingPartySecret("identitysecret");
config.setResponseType("id_token");
config.setScopes(List.of("openid", "cloud_controller.read"));
config.setAdditionalAuthzParameters(Map.of("token_format", "jwt"));
strehle marked this conversation as resolved.
Show resolved Hide resolved

oidcProvider = new IdentityProvider<>();
oidcProvider.setType(OIDC10);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

import java.util.HashMap;
import java.util.Map;
import java.util.Set;

import static org.cloudfoundry.identity.uaa.provider.ExternalIdentityProviderDefinition.GROUP_ATTRIBUTE_NAME;
import static org.cloudfoundry.identity.uaa.provider.ExternalIdentityProviderDefinition.STORE_CUSTOM_ATTRIBUTES_NAME;
Expand Down Expand Up @@ -165,4 +166,40 @@ public void jwtClientAuthenticationWithCustomSetting() {
assertNotNull(((OIDCIdentityProviderDefinition) factoryBean.getProviders().get(0).getProvider().getConfig()).getJwtClientAuthentication());
assertEquals("issuer", (((Map<String, String>)((OIDCIdentityProviderDefinition) factoryBean.getProviders().get(0).getProvider().getConfig()).getJwtClientAuthentication()).get("iss")));
}
}

@Test
public void testAdditionalParametersInConfig() {
Map<String, Object> additionalMap = new HashMap<>();
Map<String, Map> definitions = new HashMap<>();
additionalMap.put("token_format", "jwt");
additionalMap.put("expires", 0);
additionalMap.put("code", 12345678);
additionalMap.put("client_id", "id");
additionalMap.put("complex", Set.of("1", "2"));
additionalMap.put("null", null);
additionalMap.put("empty", "");
idpDefinitionMap.put("additionalAuthzParameters", additionalMap);
idpDefinitionMap.put("type", OriginKeys.OIDC10);
definitions.put("test", idpDefinitionMap);
factoryBean = new OauthIDPWrapperFactoryBean(definitions);
factoryBean.setCommonProperties(idpDefinitionMap, providerDefinition);
assertTrue(factoryBean.getProviders().get(0).getProvider().getConfig() instanceof OIDCIdentityProviderDefinition);
Map<String, String> receivedParameters = ((OIDCIdentityProviderDefinition) factoryBean.getProviders().get(0).getProvider().getConfig()).getAdditionalAuthzParameters();
assertEquals(3, receivedParameters.size());
assertEquals("jwt", receivedParameters.get("token_format"));
assertEquals("0", receivedParameters.get("expires"));
assertEquals("", receivedParameters.get("empty"));
}

@Test
public void testNoAdditionalParametersInConfig() {
Map<String, Map> definitions = new HashMap<>();
idpDefinitionMap.put("type", OriginKeys.OIDC10);
definitions.put("test", idpDefinitionMap);
factoryBean = new OauthIDPWrapperFactoryBean(definitions);
factoryBean.setCommonProperties(idpDefinitionMap, providerDefinition);
assertTrue(factoryBean.getProviders().get(0).getProvider().getConfig() instanceof OIDCIdentityProviderDefinition);
Map<String, String> receivedParameters = ((OIDCIdentityProviderDefinition) factoryBean.getProviders().get(0).getProvider().getConfig()).getAdditionalAuthzParameters();
assertEquals(0, receivedParameters.size());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -561,6 +561,7 @@ void createOidcIdentityProvider() throws Exception {
fieldWithPath("config.setForwardHeader").optional(false).type(BOOLEAN).description("Only effective if Password Grant enabled. Set X-Forward-For header in Password Grant request to this identity provider."),
fieldWithPath("config.jwtClientAuthentication").optional(null).type(OBJECT).description("<small><mark>UAA 76.5.0</mark></small> Only effective if relyingPartySecret is not set or null. Creates private_key_jwt client authentication according to OIDC or OAuth2 (RFC 7523) standard. For OIDC, set true. For OAuth2, define custom elements `iss` and `aud`."),
fieldWithPath("config.attributeMappings.user_name").optional("sub").type(STRING).description("Map `user_name` to the attribute for user name in the provider assertion or token. The default for OpenID Connect is `sub`."),
fieldWithPath("config.additionalAuthzParameters").optional(null).type(OBJECT).description("<small><mark>UAA 76.17.0</mark></small>Map of key, value pair to be added as custom request parameters during the authorization code flow to the IdP. Example of UAA custom parameter is token_format. Other (unknown) parameters should be ignored."),
fieldWithPath("config.prompts[]").optional(null).type(ARRAY).description("List of fields that users are prompted on to the OIDC provider through the password grant flow. Defaults to username, password, and passcode. Any additional prompts beyond username, password, and passcode will be forwarded on to the OIDC provider."),
fieldWithPath("config.prompts[].name").optional(null).type(STRING).description("Name of field"),
fieldWithPath("config.prompts[].type").optional(null).type(STRING).description("What kind of field this is (e.g. text or password)"),
Expand Down