From 216b774f2ac8aa5f4343e2d4d291f2140a145a67 Mon Sep 17 00:00:00 2001 From: d036670 Date: Thu, 6 Jun 2024 10:07:40 +0200 Subject: [PATCH] fix: Support authMethod=none in proxy password grant * Fix definition of authMethod in uaa.yml, which was not taken over * Check for authMethod=none and allow to omit authentication with password grant There are some OIDC IdPs which support password grant without secret, therefore offer this in proxy mode. --- .../PasswordGrantAuthenticationManager.java | 13 +++++--- .../ExternalOAuthAuthenticationManager.java | 4 ++- .../oauth/OauthIDPWrapperFactoryBean.java | 8 +++++ ...asswordGrantAuthenticationManagerTest.java | 8 +++++ ...tityProviderDefinitionFactoryBeanTest.java | 32 +++++++++++++++++++ 5 files changed, 60 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/manager/PasswordGrantAuthenticationManager.java b/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/manager/PasswordGrantAuthenticationManager.java index 6c3ea54be99..1dc5aeaa508 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/manager/PasswordGrantAuthenticationManager.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/authentication/manager/PasswordGrantAuthenticationManager.java @@ -1,10 +1,12 @@ package org.cloudfoundry.identity.uaa.authentication.manager; import org.apache.commons.lang3.ObjectUtils; +import org.cloudfoundry.identity.uaa.authentication.AbstractClientParametersAuthenticationFilter; import org.cloudfoundry.identity.uaa.authentication.ProviderConfigurationException; import org.cloudfoundry.identity.uaa.authentication.UaaAuthenticationDetails; import org.cloudfoundry.identity.uaa.authentication.UaaLoginHint; import org.cloudfoundry.identity.uaa.authentication.event.IdentityProviderAuthenticationFailureEvent; +import org.cloudfoundry.identity.uaa.constants.ClientAuthentication; import org.cloudfoundry.identity.uaa.constants.OriginKeys; import org.cloudfoundry.identity.uaa.impl.config.RestTemplateConfig; import org.cloudfoundry.identity.uaa.login.Prompt; @@ -132,9 +134,10 @@ Authentication oidcPasswordGrant(Authentication authentication, OIDCIdentityProv if (clientId == null) { throw new ProviderConfigurationException("External OpenID Connect provider configuration is missing relyingPartyId."); } - if (clientSecret == null && config.getJwtClientAuthentication() == null) { - throw new ProviderConfigurationException("External OpenID Connect provider configuration is missing relyingPartySecret or jwtclientAuthentication."); + if (clientSecret == null && config.getJwtClientAuthentication() == null && config.getAuthMethod() == null) { + throw new ProviderConfigurationException("External OpenID Connect provider configuration is missing relyingPartySecret, jwtclientAuthentication or authMethod."); } + String calcAuthMethod = ClientAuthentication.getCalculatedMethod(config.getAuthMethod(), config.getRelyingPartySecret() != null, config.getJwtClientAuthentication() != null); String userName = authentication.getPrincipal() instanceof String ? (String)authentication.getPrincipal() : null; if (userName == null || authentication.getCredentials() == null || !(authentication.getCredentials() instanceof String)) { throw new BadCredentialsException("Request is missing username or password."); @@ -152,12 +155,14 @@ Authentication oidcPasswordGrant(Authentication authentication, OIDCIdentityProv headers.setContentType(MediaType.APPLICATION_FORM_URLENCODED); MultiValueMap params = new LinkedMultiValueMap<>(); - if (clientSecret == null) { + if (ClientAuthentication.PRIVATE_KEY_JWT.equals(calcAuthMethod)) { params = new JwtClientAuthentication(externalOAuthAuthenticationManager.getKeyInfoService()) .getClientAuthenticationParameters(params, config); - } else { + } else if (ClientAuthentication.secretNeeded(calcAuthMethod)){ String auth = clientId + ":" + clientSecret; headers.add("Authorization", "Basic " + Base64Utils.encodeToString(auth.getBytes())); + } else { + params.add(AbstractClientParametersAuthenticationFilter.CLIENT_ID, clientId); } if (config.isSetForwardHeader() && authentication.getDetails() != null &&authentication.getDetails() instanceof UaaAuthenticationDetails) { UaaAuthenticationDetails details = (UaaAuthenticationDetails) authentication.getDetails(); diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManager.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManager.java index fabc4a17cc3..9158f590325 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManager.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManager.java @@ -23,6 +23,7 @@ import org.cloudfoundry.identity.uaa.authentication.manager.ExternalGroupAuthorizationEvent; import org.cloudfoundry.identity.uaa.authentication.manager.ExternalLoginAuthenticationManager; import org.cloudfoundry.identity.uaa.authentication.manager.InvitedUserAuthenticatedEvent; +import org.cloudfoundry.identity.uaa.constants.ClientAuthentication; import org.cloudfoundry.identity.uaa.oauth.KeyInfo; import org.cloudfoundry.identity.uaa.oauth.KeyInfoService; import org.cloudfoundry.identity.uaa.oauth.TokenEndpointBuilder; @@ -712,7 +713,8 @@ private String getTokenFromCode(ExternalOAuthCodeToken codeToken, AbstractExtern // https://docs.spring.io/spring-security/site/docs/5.3.1.RELEASE/reference/html5/#initiating-the-authorization-request if (config.getRelyingPartySecret() == null) { // no secret but jwtClientAuthentication - if (config instanceof OIDCIdentityProviderDefinition && ((OIDCIdentityProviderDefinition) config).getJwtClientAuthentication() != null) { + if (config instanceof OIDCIdentityProviderDefinition oidcDefinition && ClientAuthentication.PRIVATE_KEY_JWT.equals( + ClientAuthentication.getCalculatedMethod(config.getAuthMethod(), false, oidcDefinition.getJwtClientAuthentication() != null))) { body = new JwtClientAuthentication(keyInfoService) .getClientAuthenticationParameters(body, (OIDCIdentityProviderDefinition) config); } 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 046c99fcf3e..a8e386f279e 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 @@ -12,6 +12,7 @@ *******************************************************************************/ package org.cloudfoundry.identity.uaa.provider.oauth; +import org.cloudfoundry.identity.uaa.constants.ClientAuthentication; import org.cloudfoundry.identity.uaa.constants.OriginKeys; import org.cloudfoundry.identity.uaa.login.Prompt; import org.cloudfoundry.identity.uaa.provider.AbstractExternalOAuthIdentityProviderDefinition; @@ -174,6 +175,13 @@ protected void setCommonProperties(Map idpDefinitionMap, Abstrac if (idpDefinitionMap.get("performRpInitiatedLogout") instanceof Boolean) { idpDefinition.setPerformRpInitiatedLogout((boolean)idpDefinitionMap.get("performRpInitiatedLogout")); } + if (idpDefinitionMap.get("authMethod") instanceof String definedAuthMethod) { + if (ClientAuthentication.isMethodSupported(definedAuthMethod)) { + idpDefinition.setAuthMethod(definedAuthMethod); + } else { + throw new IllegalArgumentException("Invalid IdP authentication method"); + } + } } private static Map parseAdditionalParameters(Map idpDefinitionMap) { diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/authentication/manager/PasswordGrantAuthenticationManagerTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/authentication/manager/PasswordGrantAuthenticationManagerTest.java index 02d6f03317d..5777f1050b5 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/authentication/manager/PasswordGrantAuthenticationManagerTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/authentication/manager/PasswordGrantAuthenticationManagerTest.java @@ -876,6 +876,14 @@ void oidcPasswordGrant_credentialsMustBeString() { assertThrows(BadCredentialsException.class, () -> instance.oidcPasswordGrant(authentication, config)); } + @Test + void oidcPasswordGrant_requireAuthenticationStatement() { + UsernamePasswordAuthenticationToken authentication = new UsernamePasswordAuthenticationToken("user", new Object()); + OIDCIdentityProviderDefinition config = new OIDCIdentityProviderDefinition() + .setRelyingPartyId("client-id"); + assertThrows(ProviderConfigurationException.class, () -> instance.oidcPasswordGrant(authentication, config)); + } + private void mockKeyInfoService() throws JOSEException { KeyInfoService keyInfoService = mock(KeyInfoService.class); KeyInfo keyInfo = mock(KeyInfo.class); diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/OauthIdentityProviderDefinitionFactoryBeanTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/OauthIdentityProviderDefinitionFactoryBeanTest.java index 559433b59bd..c4d51dfa992 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/OauthIdentityProviderDefinitionFactoryBeanTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/OauthIdentityProviderDefinitionFactoryBeanTest.java @@ -15,6 +15,7 @@ package org.cloudfoundry.identity.uaa.provider.oauth; +import org.cloudfoundry.identity.uaa.constants.ClientAuthentication; import org.cloudfoundry.identity.uaa.constants.OriginKeys; import org.cloudfoundry.identity.uaa.provider.OIDCIdentityProviderDefinition; import org.junit.Before; @@ -33,6 +34,7 @@ import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; public class OauthIdentityProviderDefinitionFactoryBeanTest { @@ -267,4 +269,34 @@ public void testPerformRpInitiatedLogoutFalse() { factoryBean.setCommonProperties(idpDefinitionMap, providerDefinition); assertFalse(providerDefinition.isPerformRpInitiatedLogout()); } + + @Test + public void testAuthMethodNotSet() { + factoryBean.setCommonProperties(idpDefinitionMap, providerDefinition); + assertNull(providerDefinition.getAuthMethod()); + assertEquals(ClientAuthentication.CLIENT_SECRET_BASIC, ClientAuthentication.getCalculatedMethod(providerDefinition.getAuthMethod(), providerDefinition.getRelyingPartySecret() != null, providerDefinition.getJwtClientAuthentication() != null)); + } + + @Test + public void testAuthMethodSetInvalidValue() { + idpDefinitionMap.put("authMethod", "empty"); + assertThrows(IllegalArgumentException.class, () -> factoryBean.setCommonProperties(idpDefinitionMap, providerDefinition)); + } + + @Test + public void testAuthMethodSet() { + // given: 2 similar entry because of issue #2752 + idpDefinitionMap.put("jwtclientAuthentication", true); + idpDefinitionMap.put("authMethod", "none"); + idpDefinitionMap.put("type", OriginKeys.OIDC10); + Map definitions = new HashMap<>(); + definitions.put("new.idp", idpDefinitionMap); + // when: load beans from uaa.yml + factoryBean = new OauthIDPWrapperFactoryBean(definitions); + factoryBean.setCommonProperties(idpDefinitionMap, providerDefinition); + // then + assertTrue(factoryBean.getProviders().get(0).getProvider().getConfig() instanceof OIDCIdentityProviderDefinition); + assertNotNull(((OIDCIdentityProviderDefinition) factoryBean.getProviders().get(0).getProvider().getConfig()).getJwtClientAuthentication()); + assertEquals("none", (((OIDCIdentityProviderDefinition) factoryBean.getProviders().get(0).getProvider().getConfig()).getAuthMethod())); + } }