Skip to content

Commit

Permalink
fix: Support authMethod=none in proxy password grant (#2918)
Browse files Browse the repository at this point in the history
* 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.

* test added

* rebase

* Error message fixed.

* Add assert to check for the expected error message
  • Loading branch information
strehle committed Jun 11, 2024
1 parent 2a45920 commit a4d9a34
Show file tree
Hide file tree
Showing 5 changed files with 80 additions and 5 deletions.
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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(), clientSecret != 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.");
Expand All @@ -152,12 +155,14 @@ Authentication oidcPasswordGrant(Authentication authentication, OIDCIdentityProv
headers.setContentType(MediaType.APPLICATION_FORM_URLENCODED);
MultiValueMap<String, String> 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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -177,6 +178,13 @@ protected void setCommonProperties(Map<String, Object> idpDefinitionMap, Abstrac
if (idpDefinitionMap.get("cacheJwks") instanceof Boolean) {
idpDefinition.setCacheJwks((boolean)idpDefinitionMap.get("cacheJwks"));
}
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<String, String> parseAdditionalParameters(Map<String, Object> idpDefinitionMap) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@
import org.springframework.web.client.HttpClientErrorException;
import org.springframework.web.client.RestTemplate;

import java.net.MalformedURLException;
import java.net.URI;
import java.net.URL;
import java.text.ParseException;
import java.util.ArrayList;
Expand Down Expand Up @@ -876,6 +878,32 @@ void oidcPasswordGrant_credentialsMustBeString() {
assertThrows(BadCredentialsException.class, () -> instance.oidcPasswordGrant(authentication, config));
}

@Test
void oidcPasswordGrant_credentialsMustBeStringButNoSecretNeeded() throws MalformedURLException {
RestTemplate restTemplate = mock(RestTemplate.class);
ResponseEntity responseEntity = mock(ResponseEntity.class);
UsernamePasswordAuthenticationToken authentication = new UsernamePasswordAuthenticationToken("user", "");
OIDCIdentityProviderDefinition config = new OIDCIdentityProviderDefinition()
.setRelyingPartyId("client-id").setTokenUrl(URI.create("http://localhost:8080/uaa/oauth/token").toURL());
config.setAuthMethod("none");
OIDCIdentityProviderDefinition spyConfig = spy(config);
when(restTemplateConfig.nonTrustingRestTemplate()).thenReturn(restTemplate);
when(restTemplate.exchange(anyString(), any(HttpMethod.class), any(HttpEntity.class), any(ParameterizedTypeReference.class))).thenReturn(responseEntity);
when(responseEntity.hasBody()).thenReturn(true);
when(responseEntity.getBody()).thenReturn(Map.of("id_token", "dummy"));
assertNull(instance.oidcPasswordGrant(authentication, spyConfig));
verify(spyConfig, atLeast(2)).getAuthMethod();
}

@Test
void oidcPasswordGrant_requireAuthenticationStatement() {
UsernamePasswordAuthenticationToken authentication = new UsernamePasswordAuthenticationToken("user", new Object());
OIDCIdentityProviderDefinition config = new OIDCIdentityProviderDefinition()
.setRelyingPartyId("client-id");
Exception exception = assertThrows(ProviderConfigurationException.class, () -> instance.oidcPasswordGrant(authentication, config));
assertEquals("External OpenID Connect provider configuration is missing relyingPartySecret, jwtClientAuthentication or authMethod.", exception.getMessage());
}

private void mockKeyInfoService() throws JOSEException {
KeyInfoService keyInfoService = mock(KeyInfoService.class);
KeyInfo keyInfo = mock(KeyInfo.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand Down Expand Up @@ -268,4 +270,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: jwtclientAuthentication, but overrule it with authMethod=none
idpDefinitionMap.put("jwtclientAuthentication", true);
idpDefinitionMap.put("authMethod", "none");
idpDefinitionMap.put("type", OriginKeys.OIDC10);
Map<String, 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()));
}
}

0 comments on commit a4d9a34

Please sign in to comment.