Skip to content

Commit

Permalink
Fix regression with OIDC discovery url but no tokenUrl (#3280)
Browse files Browse the repository at this point in the history
* Fix OIDC discovery update

Issue #3271

* Add tests

* sonar
  • Loading branch information
strehle authored Feb 11, 2025
1 parent ba3b9ca commit f9e2fbc
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,10 @@ Authentication oidcPasswordGrant(Authentication authentication, final IdentityPr
if (clientSecret == null && config.getJwtClientAuthentication() == null && config.getAuthMethod() == null) {
throw new ProviderConfigurationException("External OpenID Connect provider configuration is missing relyingPartySecret, jwtClientAuthentication or authMethod.");
}
if (tokenUrl == null) {
externalOAuthAuthenticationManager.fetchMetadataAndUpdateDefinition(config);
tokenUrl = Optional.ofNullable(config.getTokenUrl()).orElseThrow(() -> new ProviderConfigurationException("External OpenID Connect metadata is missing after discovery update."));
}
String calcAuthMethod = ClientAuthentication.getCalculatedMethod(config.getAuthMethod(), clientSecret != null, config.getJwtClientAuthentication() != null);
String userName = authentication.getPrincipal() instanceof String pStr ? pStr : null;
if (userName == null || authentication.getCredentials() == null || !(authentication.getCredentials() instanceof String)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -808,6 +808,14 @@ public KeyInfoService getKeyInfoService() {
return keyInfoService;
}

public void fetchMetadataAndUpdateDefinition(OIDCIdentityProviderDefinition definition) {
try {
oidcMetadataFetcher.fetchMetadataAndUpdateDefinition(definition);
} catch (OidcMetadataFetchingException e) {
logger.warn("OidcMetadataFetchingException", e);
}
}

protected static class AuthenticationData {

private Map<String, Object> claims;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,33 @@ void oidcPasswordGrantProviderNoRelyingPartyCredentials() {
}
}

@Test
void oidcPasswordGrantProviderFailedInOidcMetadataUpdate() {
IdentityProvider localIdp = mock(IdentityProvider.class);
OIDCIdentityProviderDefinition localIdpConfig = mock(OIDCIdentityProviderDefinition.class);
when(localIdp.getOriginKey()).thenReturn("oidcprovider");
when(localIdp.getType()).thenReturn(OriginKeys.OIDC10);
when(localIdp.isActive()).thenReturn(true);
when(localIdp.getConfig()).thenReturn(localIdpConfig);
when(localIdpConfig.isPasswordGrantEnabled()).thenReturn(true);
when(localIdpConfig.getRelyingPartyId()).thenReturn("oidcprovider");
when(localIdpConfig.getRelyingPartySecret()).thenReturn("");

when(identityProviderProvisioning.retrieveActive("uaa")).thenReturn(Arrays.asList(uaaProvider, ldapProvider, localIdp));
when(identityProviderProvisioning.retrieveByOrigin("oidcprovider", "uaa")).thenReturn(localIdp);
UaaLoginHint loginHint = mock(UaaLoginHint.class);
when(loginHint.getOrigin()).thenReturn("oidcprovider");
Authentication auth = mock(Authentication.class);
when(zoneAwareAuthzAuthenticationManager.extractLoginHint(auth)).thenReturn(loginHint);

try {
instance.authenticate(auth);
fail("");
} catch (ProviderConfigurationException e) {
assertThat(e.getMessage()).isEqualTo("External OpenID Connect metadata is missing after discovery update.");
}
}

@Test
void oidcPasswordGrantProviderJwtClientCredentials() throws ParseException, JOSEException {
// Given
Expand Down Expand Up @@ -450,10 +477,11 @@ void oidcPasswordGrantProviderJwtClientCredentials() throws ParseException, JOSE
}

@Test
void oidcPasswordGrantNoUserCredentials() {
void oidcPasswordGrantNoUserCredentials() throws MalformedURLException {
UaaLoginHint loginHint = mock(UaaLoginHint.class);
when(loginHint.getOrigin()).thenReturn("oidcprovider");
Authentication auth = mock(Authentication.class);
when(idpConfig.getTokenUrl()).thenReturn(null).thenReturn(new URL("http://localhost:8080/uaa/oauth/token"));
when(zoneAwareAuthzAuthenticationManager.extractLoginHint(auth)).thenReturn(loginHint);

try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
import static org.cloudfoundry.identity.uaa.util.UaaMapUtils.entry;
import static org.cloudfoundry.identity.uaa.util.UaaMapUtils.map;
import static org.cloudfoundry.identity.uaa.util.UaaStringUtils.DEFAULT_UAA_URL;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

Expand Down Expand Up @@ -482,4 +483,13 @@ protected <T extends AbstractExternalOAuthIdentityProviderDefinition<T>> String
authManager.getClaimsFromToken(codeToken, idp);
assertThat(codeToken.getIdToken()).isEqualTo(idTokenJwt);
}

@Test
void fetchOidcMetadata() throws OidcMetadataFetchingException {
OIDCIdentityProviderDefinition mockedProviderDefinition = mock(OIDCIdentityProviderDefinition.class);
OidcMetadataFetcher mockedOidcMetadataFetcher = mock(OidcMetadataFetcher.class);
authManager = new ExternalOAuthAuthenticationManager(identityProviderProvisioning, new RestTemplate(), new RestTemplate(), tokenEndpointBuilder, new KeyInfoService(uaaIssuerBaseUrl), mockedOidcMetadataFetcher);
doThrow(new OidcMetadataFetchingException("error")).when(mockedOidcMetadataFetcher).fetchMetadataAndUpdateDefinition(mockedProviderDefinition);
assertThatNoException().isThrownBy(() -> authManager.fetchMetadataAndUpdateDefinition(mockedProviderDefinition));
}
}

0 comments on commit f9e2fbc

Please sign in to comment.