From 4de6b1983ff2c58a5a9d9c5a89259dce732a5b11 Mon Sep 17 00:00:00 2001 From: d036670 Date: Thu, 6 Jun 2024 22:27:52 +0200 Subject: [PATCH] Identity provider key caching behavior configurable Issue: In UAA 74.5.x the JWKS is not cached. With newer UAAs there is a cache which is hard coded to 10 min. If you have a fast rotation key, then this could be a problem. Solution: deactivate the cache for such IdPs with cacheJwks=false options --- ...ternalOAuthIdentityProviderDefinition.java | 8 ++++ .../oauth/OauthIDPWrapperFactoryBean.java | 3 ++ .../provider/oauth/OidcMetadataFetcher.java | 41 +++++++++++++---- ...tityProviderDefinitionFactoryBeanTest.java | 1 + .../oauth/OidcMetadataFetcherTest.java | 45 +++++++++++++++++++ .../IdentityProviderEndpointDocs.java | 4 ++ 6 files changed, 94 insertions(+), 8 deletions(-) diff --git a/model/src/main/java/org/cloudfoundry/identity/uaa/provider/AbstractExternalOAuthIdentityProviderDefinition.java b/model/src/main/java/org/cloudfoundry/identity/uaa/provider/AbstractExternalOAuthIdentityProviderDefinition.java index 0e8b5337175..3880eb05cbe 100644 --- a/model/src/main/java/org/cloudfoundry/identity/uaa/provider/AbstractExternalOAuthIdentityProviderDefinition.java +++ b/model/src/main/java/org/cloudfoundry/identity/uaa/provider/AbstractExternalOAuthIdentityProviderDefinition.java @@ -53,6 +53,7 @@ public enum OAuthGroupMappingMode { private boolean performRpInitiatedLogout = true; @JsonInclude(JsonInclude.Include.NON_NULL) private String authMethod; + private boolean cacheJwks = true; public T setAuthUrl(URL authUrl) { this.authUrl = authUrl; @@ -143,6 +144,11 @@ public T setGroupMappingMode(OAuthGroupMappingMode externalGroupMappingMode) { return (T) this; } + public T setCacheJwks(final boolean cacheJwks) { + this.cacheJwks = cacheJwks; + return (T) this; + } + public void setPkce(final boolean pkce) { this.pkce = pkce; } @@ -194,6 +200,7 @@ public boolean equals(Object o) { if (pkce != that.pkce) return false; if (performRpInitiatedLogout != that.performRpInitiatedLogout) return false; if (!Objects.equals(authMethod, that.authMethod)) return false; + if (cacheJwks != that.cacheJwks) return false; return Objects.equals(responseType, that.responseType); } @@ -220,6 +227,7 @@ public int hashCode() { result = 31 * result + (pkce ? 1 : 0); result = 31 * result + (performRpInitiatedLogout ? 1 : 0); result = 31 * result + (authMethod != null ? authMethod.hashCode() : 0); + result = 31 * result + (cacheJwks ? 1 : 0); return result; } } 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..58eec04ebc1 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 @@ -174,6 +174,9 @@ protected void setCommonProperties(Map idpDefinitionMap, Abstrac if (idpDefinitionMap.get("performRpInitiatedLogout") instanceof Boolean) { idpDefinition.setPerformRpInitiatedLogout((boolean)idpDefinitionMap.get("performRpInitiatedLogout")); } + if (idpDefinitionMap.get("cacheJwks") instanceof Boolean) { + idpDefinition.setCacheJwks((boolean)idpDefinitionMap.get("cacheJwks")); + } } private static Map parseAdditionalParameters(Map idpDefinitionMap) { diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/OidcMetadataFetcher.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/OidcMetadataFetcher.java index 2ac641908cd..7344e9e27c1 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/OidcMetadataFetcher.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/OidcMetadataFetcher.java @@ -13,6 +13,8 @@ import org.cloudfoundry.identity.uaa.util.JsonUtils; import org.springframework.http.HttpEntity; import org.springframework.http.HttpMethod; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; import org.springframework.web.client.RestTemplate; @@ -47,13 +49,13 @@ public void fetchMetadataAndUpdateDefinition(OIDCIdentityProviderDefinition defi } } - public JsonWebKeySet fetchWebKeySet(AbstractExternalOAuthIdentityProviderDefinition config) + public JsonWebKeySet fetchWebKeySet(AbstractExternalOAuthIdentityProviderDefinition config) throws OidcMetadataFetchingException { URL tokenKeyUrl = config.getTokenKeyUrl(); if (tokenKeyUrl == null || !org.springframework.util.StringUtils.hasText(tokenKeyUrl.toString())) { return new JsonWebKeySet<>(Collections.emptyList()); } - byte[] rawContents = getJsonBody(tokenKeyUrl.toString(), config.isSkipSslValidation(), getClientAuthHeader(config)); + byte[] rawContents = getJsonBody(tokenKeyUrl.toString(), config.isSkipSslValidation(), config.isCacheJwks(), getClientAuthHeader(config)); if (rawContents == null || rawContents.length == 0) { throw new OidcMetadataFetchingException("Unable to fetch verification keys"); } @@ -68,7 +70,7 @@ public JsonWebKeySet fetchWebKeySet(ClientJwtConfiguration clientJwt if (clientJwtConfiguration.getJwkSet() != null) { return clientJwtConfiguration.getJwkSet(); } else if (clientJwtConfiguration.getJwksUri() != null) { - byte[] rawContents = getJsonBody(clientJwtConfiguration.getJwksUri(), false, null); + byte[] rawContents = getJsonBody(clientJwtConfiguration.getJwksUri(), false, true, null); if (rawContents != null && rawContents.length > 0) { ClientJwtConfiguration clientKeys = ClientJwtConfiguration.parse(null, new String(rawContents, StandardCharsets.UTF_8)); if (clientKeys != null && clientKeys.getJwkSet() != null) { @@ -79,21 +81,44 @@ public JsonWebKeySet fetchWebKeySet(ClientJwtConfiguration clientJwt throw new OidcMetadataFetchingException("Unable to fetch verification keys"); } - private byte[] getJsonBody(String uri, boolean isSkipSslValidation, String authorizationValue) { + private byte[] getJsonBody(String uri, boolean isSkipSslValidation, boolean isCached, String authorizationValue) { MultiValueMap headers = new LinkedMultiValueMap<>(); if (authorizationValue != null) { headers.add("Authorization", authorizationValue); } headers.add("Accept", "application/json"); - HttpEntity tokenKeyRequest = new HttpEntity<>(null, headers); + HttpEntity tokenKeyRequest = new HttpEntity<>(null, headers); + if (isCached) { + return getCachedResponse(uri, isSkipSslValidation, HttpMethod.GET, tokenKeyRequest); + } else { + return getResponse(uri, isSkipSslValidation, HttpMethod.GET, tokenKeyRequest); + } + } + + private byte[] getResponse(String uri, boolean isSkipSslValidation, HttpMethod method, HttpEntity header) { + ResponseEntity responseEntity; + if (isSkipSslValidation) { + responseEntity = trustingRestTemplate.exchange(uri, method, header, byte[].class); + } else { + responseEntity = nonTrustingRestTemplate.exchange(uri, method, header, byte[].class); + } + if (responseEntity.getStatusCode() == HttpStatus.OK) { + return responseEntity.getBody(); + } else { + throw new IllegalArgumentException( + "Unable to fetch content, status:" + responseEntity.getStatusCode().getReasonPhrase()); + } + } + + private byte[] getCachedResponse(String uri, boolean isSkipSslValidation, HttpMethod method, HttpEntity header) { if (isSkipSslValidation) { - return contentCache.getUrlContent(uri, trustingRestTemplate, HttpMethod.GET, tokenKeyRequest); + return contentCache.getUrlContent(uri, trustingRestTemplate, method, header); } else { - return contentCache.getUrlContent(uri, nonTrustingRestTemplate, HttpMethod.GET, tokenKeyRequest); + return contentCache.getUrlContent(uri, nonTrustingRestTemplate, method, header); } } - private String getClientAuthHeader(AbstractExternalOAuthIdentityProviderDefinition config) { + private String getClientAuthHeader(AbstractExternalOAuthIdentityProviderDefinition config) { if (config.getRelyingPartySecret() == null) { return null; } 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..c79a8327cb9 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 @@ -54,6 +54,7 @@ public void setup() { idpDefinitionMap.put("tokenKeyUrl", "http://token-key.url"); idpDefinitionMap.put("logoutUrl", "http://logout.url"); idpDefinitionMap.put("clientAuthInBody", false); + idpDefinitionMap.put("cacheJwks", true); } @Test diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/OidcMetadataFetcherTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/OidcMetadataFetcherTest.java index 3bde8524dfc..c218279ee51 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/OidcMetadataFetcherTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/OidcMetadataFetcherTest.java @@ -13,6 +13,8 @@ import org.mockito.Answers; import org.springframework.http.HttpEntity; import org.springframework.http.HttpMethod; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; import org.springframework.web.client.RestTemplate; import java.net.MalformedURLException; @@ -128,6 +130,49 @@ void shouldPerformTokenKeyUrlUsingCache() throws OidcMetadataFetchingException, any(), any(), any(), any() ); } + + @Test + void shouldPerformTokenKeyUrlNoCacheUsed() throws OidcMetadataFetchingException, MalformedURLException { + definition.setTokenKeyUrl(new URL("http://should.be.updated")); + definition.setSkipSslValidation(false); + definition.setCacheJwks(false); + + ResponseEntity responseEntity = mock(ResponseEntity.class); + when(restTemplate.exchange(anyString(), any(HttpMethod.class), any(HttpEntity.class), any(Class.class))) + .thenReturn(responseEntity); + when(responseEntity.getStatusCode()).thenReturn(HttpStatus.OK); + when(responseEntity.getBody()).thenReturn("{\"keys\":[{\"alg\":\"RS256\",\"e\":\"e\",\"kid\":\"id\",\"kty\":\"RSA\",\"n\":\"n\"}]}".getBytes()); + + metadataDiscoverer.fetchWebKeySet(definition); + definition.setSkipSslValidation(true); + metadataDiscoverer.fetchWebKeySet(definition); + + verify(urlContentCache, times(0)) + .getUrlContent( + any(), any(), any(), any() + ); + verify(restTemplate, times(2)).exchange(anyString(), any(HttpMethod.class), any(HttpEntity.class), any(Class.class)); + } + + @Test + void shouldPerformTokenKeyUrlNoCacheUsedError() throws OidcMetadataFetchingException, MalformedURLException { + definition.setTokenKeyUrl(new URL("http://should.be.updated")); + definition.setSkipSslValidation(false); + definition.setCacheJwks(false); + + ResponseEntity responseEntity = mock(ResponseEntity.class); + when(restTemplate.exchange(anyString(), any(HttpMethod.class), any(HttpEntity.class), any(Class.class))) + .thenReturn(responseEntity); + when(responseEntity.getStatusCode()).thenReturn(HttpStatus.FORBIDDEN); + + assertThrows(IllegalArgumentException.class, () -> metadataDiscoverer.fetchWebKeySet(definition)); + + verify(urlContentCache, times(0)) + .getUrlContent( + any(), any(), any(), any() + ); + verify(restTemplate, times(1)).exchange(anyString(), any(HttpMethod.class), any(HttpEntity.class), any(Class.class)); + } } @Nested diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/providers/IdentityProviderEndpointDocs.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/providers/IdentityProviderEndpointDocs.java index 100b2ae51e9..f5bb255df2c 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/providers/IdentityProviderEndpointDocs.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/providers/IdentityProviderEndpointDocs.java @@ -575,6 +575,7 @@ void createOAuthIdentityProvider() throws Exception { definition.setAttributeMappings(getAttributeMappingMap()); definition.setUserPropagationParameter("username"); definition.setPkce(true); + definition.setCacheJwks(true); definition.setPerformRpInitiatedLogout(true); identityProvider.setConfig(definition); identityProvider.setSerializeConfigRaw(true); @@ -585,6 +586,7 @@ void createOAuthIdentityProvider() throws Exception { fieldWithPath("config.authUrl").required().type(STRING).description("The OAuth 2.0 authorization endpoint URL"), fieldWithPath("config.tokenUrl").required().type(STRING).description("The OAuth 2.0 token endpoint URL"), fieldWithPath("config.tokenKeyUrl").optional(null).type(STRING).description("The URL of the token key endpoint which renders a verification key for validating token signatures"), + fieldWithPath("config.cacheJwks").optional(true).type(BOOLEAN).description("UAA 77.11.0. Option to enable or disabled the cache for the JWKS. The verification key is fetched if only `discoveryUrl` is set or `tokenKeyUrl` in combination with `tokenUrl`.").attributes(new Attributes.Attribute("constraints", "Used only if `discoveryUrl` or `tokenKeyUrl` is set.")), fieldWithPath("config.tokenKey").optional(null).type(STRING).description("A verification key for validating token signatures, set to null if a `tokenKeyUrl` is provided."), fieldWithPath("config.userInfoUrl").optional(null).type(STRING).description("A URL for fetching user info attributes when queried with the obtained token authorization."), fieldWithPath("config.showLinkText").optional(true).type(BOOLEAN).description("A flag controlling whether a link to this provider's login will be shown on the UAA login page"), @@ -674,6 +676,7 @@ void createOidcIdentityProvider() throws Exception { definition.setRelyingPartySecret("secret"); definition.setShowLinkText(false); definition.setPkce(true); + definition.setCacheJwks(true); definition.setPerformRpInitiatedLogout(true); definition.setAttributeMappings(getAttributeMappingMap()); definition.setUserPropagationParameter("username"); @@ -692,6 +695,7 @@ void createOidcIdentityProvider() throws Exception { fieldWithPath("config.authUrl").optional().type(STRING).description("The OIDC 1.0 authorization endpoint URL. This can be left blank if a discovery URL is provided. If both are provided, this property overrides the discovery URL.").attributes(new Attributes.Attribute("constraints", "Required unless `discoveryUrl` is set.")), fieldWithPath("config.tokenUrl").optional().type(STRING).description("The OIDC 1.0 token endpoint URL. This can be left blank if a discovery URL is provided. If both are provided, this property overrides the discovery URL.").attributes(new Attributes.Attribute("constraints", "Required unless `discoveryUrl` is set.")), fieldWithPath("config.tokenKeyUrl").optional(null).type(STRING).description("The URL of the token key endpoint which renders a verification key for validating token signatures. This can be left blank if a discovery URL is provided. If both are provided, this property overrides the discovery URL.").attributes(new Attributes.Attribute("constraints", "Required unless `discoveryUrl` is set.")), + fieldWithPath("config.cacheJwks").optional(true).type(BOOLEAN).description("UAA 77.11.0. Option to enable or disabled the cache for the JWKS. The verification key is fetched if only `discoveryUrl` is set or `tokenKeyUrl` in combination with `tokenUrl`.").attributes(new Attributes.Attribute("constraints", "Used only if `discoveryUrl` or `tokenKeyUrl` is set.")), fieldWithPath("config.tokenKey").optional(null).type(STRING).description("A verification key for validating token signatures. We recommend not setting this as it will not allow for key rotation. This can be left blank if a discovery URL is provided. If both are provided, this property overrides the discovery URL.").attributes(new Attributes.Attribute("constraints", "Required unless `discoveryUrl` is set.")), fieldWithPath("config.showLinkText").optional(true).type(BOOLEAN).description("A flag controlling whether a link to this provider's login will be shown on the UAA login page"), fieldWithPath("config.linkText").optional(null).type(STRING).description("Text to use for the login link to the provider"),