Skip to content

Commit

Permalink
Refactored OIDC metadata fetcher
Browse files Browse the repository at this point in the history
[#157639907] https://www.pivotaltracker.com/story/show/157639907

Signed-off-by: Bruce Ricard <bricard@pivotal.io>
Co-authored-by: Bruce Ricard <bricard@pivotal.io>
  • Loading branch information
Jaskanwal Pawar and bruce-ricard committed Oct 17, 2018
1 parent 80ff310 commit cab25a9
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 20 deletions.
Expand Up @@ -9,37 +9,41 @@
import java.io.IOException; import java.io.IOException;
import java.net.URL; import java.net.URL;


public class OidcMetadataDiscoverer { public class OidcMetadataFetcher {
private final UrlContentCache contentCache; private final UrlContentCache contentCache;
private final RestTemplate trustingRestTemplate; private final RestTemplate trustingRestTemplate;
private final RestTemplate nonTrustingRestTemplate; private final RestTemplate nonTrustingRestTemplate;


public OidcMetadataDiscoverer(UrlContentCache contentCache, public OidcMetadataFetcher(UrlContentCache contentCache,
RestTemplate trustingRestTemplate, RestTemplate trustingRestTemplate,
RestTemplate nonTrustingRestTemplate RestTemplate nonTrustingRestTemplate
) { ) {
this.contentCache = contentCache; this.contentCache = contentCache;
this.trustingRestTemplate = trustingRestTemplate; this.trustingRestTemplate = trustingRestTemplate;
this.nonTrustingRestTemplate = nonTrustingRestTemplate; this.nonTrustingRestTemplate = nonTrustingRestTemplate;
} }


public void performDiscoveryAndUpdateDefinition(OIDCIdentityProviderDefinition definition) throws IOException { public void fetchMetadataAndUpdateDefinition(OIDCIdentityProviderDefinition definition) throws OidcMetadataFetchingException {
if (shouldPerformDiscovery(definition)) { if (shouldFetchMetadata(definition)) {
OidcMetadata oidcMetadata = OidcMetadata oidcMetadata =
performDiscovery(definition.getDiscoveryUrl(), definition.isSkipSslValidation()); fetchMetadata(definition.getDiscoveryUrl(), definition.isSkipSslValidation());


updateIdpDefinition(definition, oidcMetadata); updateIdpDefinition(definition, oidcMetadata);
} }
} }


private OidcMetadata performDiscovery(URL discoveryUrl, boolean shouldDoSslValidation) throws IOException { private OidcMetadata fetchMetadata(URL discoveryUrl, boolean shouldDoSslValidation) throws OidcMetadataFetchingException {
byte[] rawContents; byte[] rawContents;
if (shouldDoSslValidation) { if (shouldDoSslValidation) {
rawContents = contentCache.getUrlContent(discoveryUrl.toString(), trustingRestTemplate); rawContents = contentCache.getUrlContent(discoveryUrl.toString(), trustingRestTemplate);
} else { } else {
rawContents = contentCache.getUrlContent(discoveryUrl.toString(), nonTrustingRestTemplate); rawContents = contentCache.getUrlContent(discoveryUrl.toString(), nonTrustingRestTemplate);
} }
return new ObjectMapper().readValue(rawContents, OidcMetadata.class); try {
return new ObjectMapper().readValue(rawContents, OidcMetadata.class);
} catch (IOException e) {
throw new OidcMetadataFetchingException(e);
}
} }


private void updateIdpDefinition(OIDCIdentityProviderDefinition definition, OidcMetadata oidcMetadata) { private void updateIdpDefinition(OIDCIdentityProviderDefinition definition, OidcMetadata oidcMetadata) {
Expand All @@ -50,7 +54,7 @@ private void updateIdpDefinition(OIDCIdentityProviderDefinition definition, Oidc
definition.setIssuer(oidcMetadata.getIssuer()); definition.setIssuer(oidcMetadata.getIssuer());
} }


private boolean shouldPerformDiscovery(OIDCIdentityProviderDefinition definition) { private boolean shouldFetchMetadata(OIDCIdentityProviderDefinition definition) {
return definition.getDiscoveryUrl() != null && !StringUtils.isBlank(definition.getDiscoveryUrl().toString()); return definition.getDiscoveryUrl() != null && !StringUtils.isBlank(definition.getDiscoveryUrl().toString());
} }
} }
@@ -0,0 +1,15 @@
package org.cloudfoundry.identity.uaa.provider.oauth;

public class OidcMetadataFetchingException extends Exception {
public OidcMetadataFetchingException(String message) {
super(message);
}

public OidcMetadataFetchingException(String message, Throwable cause) {
super(message, cause);
}

public OidcMetadataFetchingException(Throwable cause) {
super(cause);
}
}
@@ -1,5 +1,6 @@
package org.cloudfoundry.identity.uaa.provider.oauth; package org.cloudfoundry.identity.uaa.provider.oauth;


import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.ObjectMapper;
import org.cloudfoundry.identity.uaa.cache.ExpiringUrlCache; import org.cloudfoundry.identity.uaa.cache.ExpiringUrlCache;
import org.cloudfoundry.identity.uaa.cache.UrlContentCache; import org.cloudfoundry.identity.uaa.cache.UrlContentCache;
Expand All @@ -11,6 +12,7 @@
import org.springframework.web.client.RestTemplate; import org.springframework.web.client.RestTemplate;


import java.io.IOException; import java.io.IOException;
import java.net.MalformedURLException;
import java.net.URI; import java.net.URI;
import java.net.URL; import java.net.URL;
import java.time.Duration; import java.time.Duration;
Expand All @@ -22,8 +24,8 @@
import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when; import static org.mockito.Mockito.when;


class OidcMetadataDiscovererTest { class OidcMetadataFetcherTest {
private OidcMetadataDiscoverer metadataDiscoverer; private OidcMetadataFetcher metadataDiscoverer;
private UrlContentCache urlContentCache; private UrlContentCache urlContentCache;
private RestTemplate restTemplate; private RestTemplate restTemplate;


Expand All @@ -36,16 +38,16 @@ void setUp() {
urlContentCache = new ExpiringUrlCache(Duration.ofMinutes(2), new TimeServiceImpl(), 2); urlContentCache = new ExpiringUrlCache(Duration.ofMinutes(2), new TimeServiceImpl(), 2);
restTemplate = mock(RestTemplate.class, Answers.RETURNS_DEEP_STUBS); restTemplate = mock(RestTemplate.class, Answers.RETURNS_DEEP_STUBS);


metadataDiscoverer = new OidcMetadataDiscoverer(urlContentCache, restTemplate, restTemplate); metadataDiscoverer = new OidcMetadataFetcher(urlContentCache, restTemplate, restTemplate);
} }


@Test @Test
public void withoutDiscoveryUrl_shouldNotPerformDiscovery() throws IOException { public void withoutDiscoveryUrl_shouldNotPerformDiscovery() throws OidcMetadataFetchingException, MalformedURLException {
definition = new OIDCIdentityProviderDefinition(); definition = new OIDCIdentityProviderDefinition();
definition.setAuthUrl(new URL("http://not.updated")); definition.setAuthUrl(new URL("http://not.updated"));
definition.setTokenUrl(new URL("http://not.updated")); definition.setTokenUrl(new URL("http://not.updated"));


metadataDiscoverer.performDiscoveryAndUpdateDefinition(definition); metadataDiscoverer.fetchMetadataAndUpdateDefinition(definition);


assertThat(definition, is(notNullValue())); assertThat(definition, is(notNullValue()));
assertThat(definition.getDiscoveryUrl(), nullValue()); assertThat(definition.getDiscoveryUrl(), nullValue());
Expand All @@ -54,15 +56,15 @@ public void withoutDiscoveryUrl_shouldNotPerformDiscovery() throws IOException {
} }


@Test @Test
public void withDiscoveryUrl_shouldPerformDiscovery() throws IOException { public void withDiscoveryUrl_shouldPerformDiscovery() throws OidcMetadataFetchingException, MalformedURLException, JsonProcessingException {
definition = new OIDCIdentityProviderDefinition(); definition = new OIDCIdentityProviderDefinition();
definition.setAuthUrl(new URL("http://should.be.updated")); definition.setAuthUrl(new URL("http://should.be.updated"));
definition.setTokenUrl(new URL("http://should.be.updated")); definition.setTokenUrl(new URL("http://should.be.updated"));
definition.setDiscoveryUrl(new URL("http://discovery.com")); definition.setDiscoveryUrl(new URL("http://discovery.com"));
when(restTemplate.getForObject(any(URI.class), eq(byte[].class))) when(restTemplate.getForObject(any(URI.class), eq(byte[].class)))
.thenReturn(objectMapper.writeValueAsBytes(buildOidcMetadata())); .thenReturn(objectMapper.writeValueAsBytes(buildOidcMetadata()));


metadataDiscoverer.performDiscoveryAndUpdateDefinition(definition); metadataDiscoverer.fetchMetadataAndUpdateDefinition(definition);


assertThat(definition, is(notNullValue())); assertThat(definition, is(notNullValue()));
assertThat(definition.getAuthUrl().toString(), is("http://authz.endpoint")); assertThat(definition.getAuthUrl().toString(), is("http://authz.endpoint"));
Expand All @@ -73,7 +75,7 @@ public void withDiscoveryUrl_shouldPerformDiscovery() throws IOException {
} }


@Test @Test
public void withDiscoveryUrl_usesCache() throws IOException { public void withDiscoveryUrl_usesCache() throws OidcMetadataFetchingException, MalformedURLException, JsonProcessingException {
definition = new OIDCIdentityProviderDefinition(); definition = new OIDCIdentityProviderDefinition();
definition.setAuthUrl(new URL("http://should.be.updated")); definition.setAuthUrl(new URL("http://should.be.updated"));
definition.setTokenUrl(new URL("http://should.be.updated")); definition.setTokenUrl(new URL("http://should.be.updated"));
Expand All @@ -84,8 +86,8 @@ public void withDiscoveryUrl_usesCache() throws IOException {
.thenReturn(objectMapper.writeValueAsBytes(buildOidcMetadata())) .thenReturn(objectMapper.writeValueAsBytes(buildOidcMetadata()))
.thenThrow(new RuntimeException("shouldn't have been called more than once")); .thenThrow(new RuntimeException("shouldn't have been called more than once"));


metadataDiscoverer.performDiscoveryAndUpdateDefinition(definition); metadataDiscoverer.fetchMetadataAndUpdateDefinition(definition);
metadataDiscoverer.performDiscoveryAndUpdateDefinition(definition); metadataDiscoverer.fetchMetadataAndUpdateDefinition(definition);
} }


private OidcMetadata buildOidcMetadata() { private OidcMetadata buildOidcMetadata() {
Expand Down

0 comments on commit cab25a9

Please sign in to comment.