From 2c203c7511744e0a411ec31a8424285b1539d28d Mon Sep 17 00:00:00 2001 From: d036670 Date: Thu, 9 May 2024 19:36:05 +0200 Subject: [PATCH 01/19] WIP: idp secret --- ...actExternalOAuthIdentityProviderDefinition.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) 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 fc243ceffe9..48a3ee5df04 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 @@ -16,6 +16,7 @@ import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.annotation.JsonProperty; import lombok.Getter; import java.lang.reflect.ParameterizedType; @@ -51,6 +52,9 @@ public enum OAuthGroupMappingMode { private OAuthGroupMappingMode groupMappingMode; private boolean pkce = true; private boolean performRpInitiatedLogout = true; + @JsonInclude(JsonInclude.Include.NON_NULL) + @JsonProperty(access = JsonProperty.Access.READ_ONLY) + private String auth_method; public T setAuthUrl(URL authUrl) { this.authUrl = authUrl; @@ -149,6 +153,14 @@ public void setPerformRpInitiatedLogout(boolean performRpInitiatedLogout) { this.performRpInitiatedLogout = performRpInitiatedLogout; } + public String getAuth_method() { + return this.auth_method; + } + + public void setAuth_method(final String auth_method) { + this.auth_method = auth_method; + } + @JsonIgnore public Class getParameterizedClass() { ParameterizedType parameterizedType = @@ -183,6 +195,7 @@ public boolean equals(Object o) { if (!Objects.equals(groupMappingMode, that.groupMappingMode)) return false; if (pkce != that.pkce) return false; if (performRpInitiatedLogout != that.performRpInitiatedLogout) return false; + if (!Objects.equals(auth_method, that.auth_method)) return false; return Objects.equals(responseType, that.responseType); } @@ -208,6 +221,7 @@ public int hashCode() { result = 31 * result + (responseType != null ? responseType.hashCode() : 0); result = 31 * result + (pkce ? 1 : 0); result = 31 * result + (performRpInitiatedLogout ? 1 : 0); + result = 31 * result + (auth_method != null ? auth_method.hashCode() : 0); return result; } } From 189cf06988ad8e18f7557c4a9464c5ff37e8d9c0 Mon Sep 17 00:00:00 2001 From: d036670 Date: Fri, 10 May 2024 11:54:04 +0200 Subject: [PATCH 02/19] feature: delete secret on existing IdP - allow to delete a relyingPartySecret on IdP - Filter IdP list by origin - Return the auth_method to show current configured client authentication method --- .../uaa/account/OpenIdConfiguration.java | 3 +- .../uaa/constants/ClientAuthentication.java | 18 ++++ .../uaa/oauth/token/TokenConstants.java | 6 +- ...ternalOAuthIdentityProviderDefinition.java | 16 ++-- .../provider/IdentityProviderEndpoints.java | 61 ++++++++++++- .../IdentityProviderEndpointsTest.java | 87 +++++++++++++++++-- 6 files changed, 168 insertions(+), 23 deletions(-) create mode 100644 model/src/main/java/org/cloudfoundry/identity/uaa/constants/ClientAuthentication.java diff --git a/model/src/main/java/org/cloudfoundry/identity/uaa/account/OpenIdConfiguration.java b/model/src/main/java/org/cloudfoundry/identity/uaa/account/OpenIdConfiguration.java index 2fcfe27364e..2beec448029 100644 --- a/model/src/main/java/org/cloudfoundry/identity/uaa/account/OpenIdConfiguration.java +++ b/model/src/main/java/org/cloudfoundry/identity/uaa/account/OpenIdConfiguration.java @@ -3,6 +3,7 @@ import com.fasterxml.jackson.annotation.JsonProperty; import lombok.Data; import lombok.NoArgsConstructor; +import org.cloudfoundry.identity.uaa.constants.ClientAuthentication; @Data @NoArgsConstructor @@ -18,7 +19,7 @@ public class OpenIdConfiguration { private String tokenUrl; @JsonProperty("token_endpoint_auth_methods_supported") - private String[] tokenAMR = new String[]{"client_secret_basic", "client_secret_post", "private_key_jwt"}; + private String[] tokenAMR = new String[]{ClientAuthentication.CLIENT_SECRET_BASIC, ClientAuthentication.CLIENT_SECRET_POST, ClientAuthentication.PRIVATE_KEY_JWT}; @JsonProperty("token_endpoint_auth_signing_alg_values_supported") private String[] tokenEndpointAuthSigningValues = new String[]{"RS256", "HS256"}; diff --git a/model/src/main/java/org/cloudfoundry/identity/uaa/constants/ClientAuthentication.java b/model/src/main/java/org/cloudfoundry/identity/uaa/constants/ClientAuthentication.java new file mode 100644 index 00000000000..b847eb58a29 --- /dev/null +++ b/model/src/main/java/org/cloudfoundry/identity/uaa/constants/ClientAuthentication.java @@ -0,0 +1,18 @@ +package org.cloudfoundry.identity.uaa.constants; + +/** + * ClientAuthentication constants are defined in OIDC core and discovery standard, e.g. https://openid.net/specs/openid-connect-registration-1_0.html + * OIDC possible values are: client_secret_post, client_secret_basic, client_secret_jwt, private_key_jwt, and none + * UAA knows only: client_secret_post, client_secret_basic, private_key_jwt, and none + * + * Planned: tls_client_auth + */ +public final class ClientAuthentication { + + private ClientAuthentication() {} + + public static final String CLIENT_SECRET_BASIC = "client_secret_basic"; + public static final String CLIENT_SECRET_POST = "client_secret_post"; + public static final String PRIVATE_KEY_JWT = "private_key_jwt"; + public static final String NONE = "none"; +} diff --git a/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/token/TokenConstants.java b/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/token/TokenConstants.java index 3b1a8d93cdb..338a954fbcf 100644 --- a/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/token/TokenConstants.java +++ b/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/token/TokenConstants.java @@ -15,6 +15,8 @@ package org.cloudfoundry.identity.uaa.oauth.token; +import org.cloudfoundry.identity.uaa.constants.ClientAuthentication; + import java.util.Arrays; import java.util.List; import java.util.stream.Collectors; @@ -62,9 +64,9 @@ static public List getStringValues() { public static final String GRANT_TYPE_AUTHORIZATION_CODE = "authorization_code"; public static final String GRANT_TYPE_IMPLICIT = "implicit"; - public static final String CLIENT_AUTH_NONE = "none"; + public static final String CLIENT_AUTH_NONE = ClientAuthentication.NONE; public static final String CLIENT_AUTH_EMPTY = "empty"; - public static final String CLIENT_AUTH_PRIVATE_KEY_JWT = "private_key_jwt"; + public static final String CLIENT_AUTH_PRIVATE_KEY_JWT = ClientAuthentication.PRIVATE_KEY_JWT; public static final String ID_TOKEN_HINT_PROMPT = "prompt"; public static final String ID_TOKEN_HINT_PROMPT_NONE = "none"; 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 48a3ee5df04..2ec15c53037 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,8 +53,8 @@ public enum OAuthGroupMappingMode { private boolean pkce = true; private boolean performRpInitiatedLogout = true; @JsonInclude(JsonInclude.Include.NON_NULL) - @JsonProperty(access = JsonProperty.Access.READ_ONLY) - private String auth_method; + @JsonProperty(value = "auth_method", access = JsonProperty.Access.READ_ONLY) + private String authMethod; public T setAuthUrl(URL authUrl) { this.authUrl = authUrl; @@ -153,12 +153,12 @@ public void setPerformRpInitiatedLogout(boolean performRpInitiatedLogout) { this.performRpInitiatedLogout = performRpInitiatedLogout; } - public String getAuth_method() { - return this.auth_method; + public String getAuthMethod() { + return this.authMethod; } - public void setAuth_method(final String auth_method) { - this.auth_method = auth_method; + public void setAuthMethod(final String authMethod) { + this.authMethod = authMethod; } @JsonIgnore @@ -195,7 +195,7 @@ public boolean equals(Object o) { if (!Objects.equals(groupMappingMode, that.groupMappingMode)) return false; if (pkce != that.pkce) return false; if (performRpInitiatedLogout != that.performRpInitiatedLogout) return false; - if (!Objects.equals(auth_method, that.auth_method)) return false; + if (!Objects.equals(authMethod, that.authMethod)) return false; return Objects.equals(responseType, that.responseType); } @@ -221,7 +221,7 @@ public int hashCode() { result = 31 * result + (responseType != null ? responseType.hashCode() : 0); result = 31 * result + (pkce ? 1 : 0); result = 31 * result + (performRpInitiatedLogout ? 1 : 0); - result = 31 * result + (auth_method != null ? auth_method.hashCode() : 0); + result = 31 * result + (authMethod != null ? authMethod.hashCode() : 0); return result; } } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java index 77d42fb8ab0..6d3df271208 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java @@ -43,11 +43,13 @@ import org.cloudfoundry.identity.uaa.audit.event.EntityDeletedEvent; import org.cloudfoundry.identity.uaa.authentication.manager.DynamicLdapAuthenticationManager; import org.cloudfoundry.identity.uaa.authentication.manager.LdapLoginAuthenticationManager; +import org.cloudfoundry.identity.uaa.constants.ClientAuthentication; import org.cloudfoundry.identity.uaa.provider.saml.SamlIdentityProviderConfigurator; import org.cloudfoundry.identity.uaa.scim.ScimGroupExternalMembershipManager; import org.cloudfoundry.identity.uaa.scim.ScimGroupProvisioning; import org.cloudfoundry.identity.uaa.util.JsonUtils; import org.cloudfoundry.identity.uaa.util.ObjectUtils; +import org.cloudfoundry.identity.uaa.util.UaaStringUtils; import org.cloudfoundry.identity.uaa.zone.beans.IdentityZoneManager; import org.opensaml.saml2.metadata.provider.MetadataProviderException; import org.slf4j.Logger; @@ -67,6 +69,7 @@ import org.springframework.transaction.PlatformTransactionManager; import org.springframework.transaction.annotation.Transactional; import org.springframework.transaction.support.TransactionTemplate; +import org.springframework.web.bind.annotation.DeleteMapping; import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.RequestBody; @@ -168,6 +171,7 @@ public ResponseEntity createIdentityProvider(@RequestBody Iden return new ResponseEntity<>(body, UNPROCESSABLE_ENTITY); } createdIdp.setSerializeConfigRaw(rawConfig); + setAuthMethod(createdIdp); redactSensitiveData(createdIdp); return new ResponseEntity<>(createdIdp, CREATED); @@ -194,6 +198,7 @@ public ResponseEntity deleteIdentityProvider(@PathVariable Str // delete the IdP existing.setSerializeConfigRaw(rawConfig); publisher.publishEvent(new EntityDeletedEvent<>(existing, authentication, identityZoneId)); + setAuthMethod(existing); redactSensitiveData(existing); // delete the alias IdP if present @@ -224,6 +229,7 @@ public ResponseEntity updateIdentityProvider(@PathVariable Str IdentityProvider existing = identityProviderProvisioning.retrieve(id, zoneId); body.setId(id); body.setIdentityZoneId(zoneId); + setAuthMethod(existing); patchSensitiveData(id, body); try { configValidator.validate(body); @@ -275,6 +281,7 @@ public ResponseEntity updateIdentityProvider(@PathVariable Str return new ResponseEntity<>(body, UNPROCESSABLE_ENTITY); } updatedIdp.setSerializeConfigRaw(rawConfig); + setAuthMethod(updatedIdp); redactSensitiveData(updatedIdp); return new ResponseEntity<>(updatedIdp, OK); @@ -308,12 +315,40 @@ public ResponseEntity updateIdentityProviderStatus(@Path return new ResponseEntity<>(body, OK); } + @DeleteMapping(value = "{id}/secret") + public ResponseEntity> deleteSecret(@PathVariable String id) { + String zoneId = identityZoneManager.getCurrentIdentityZoneId(); + IdentityProvider existing = identityProviderProvisioning.retrieve(id, zoneId); + if((OIDC10.equals(existing.getType()) || OAUTH20.equals(existing.getType())) + && existing.getConfig() instanceof AbstractExternalOAuthIdentityProviderDefinition idpConfiguration) { + idpConfiguration.setRelyingPartySecret(null); + identityProviderProvisioning.update(existing, zoneId); + setAuthMethod(existing); + redactSensitiveData(existing); + logger.info("Secret deleted for Identity Provider: {}", existing.getId()); + return new ResponseEntity<>(existing, OK); + } else { + logger.debug("Invalid operation. This operation is only supported on external OAuth/OIDC IDP"); + return new ResponseEntity<>(UNPROCESSABLE_ENTITY); + } + } + @RequestMapping(method = GET) - public ResponseEntity> retrieveIdentityProviders(@RequestParam(value = "active_only", required = false) String activeOnly, @RequestParam(required = false, defaultValue = "false") boolean rawConfig) { + public ResponseEntity> retrieveIdentityProviders( + @RequestParam(value = "active_only", required = false) String activeOnly, + @RequestParam(required = false, defaultValue = "false") boolean rawConfig, + @RequestParam(required = false, defaultValue = "") String origin) + { boolean retrieveActiveOnly = Boolean.parseBoolean(activeOnly); - List identityProviderList = identityProviderProvisioning.retrieveAll(retrieveActiveOnly, identityZoneManager.getCurrentIdentityZoneId()); - for(IdentityProvider idp : identityProviderList) { + List identityProviderList; + if (UaaStringUtils.isNotEmpty(origin)) { + identityProviderList = List.of(identityProviderProvisioning.retrieveByOrigin(origin, identityZoneManager.getCurrentIdentityZoneId())); + } else { + identityProviderList = identityProviderProvisioning.retrieveAll(retrieveActiveOnly, identityZoneManager.getCurrentIdentityZoneId()); + } + for(IdentityProvider idp : identityProviderList) { idp.setSerializeConfigRaw(rawConfig); + setAuthMethod(idp); redactSensitiveData(idp); } return new ResponseEntity<>(identityProviderList, OK); @@ -323,6 +358,7 @@ public ResponseEntity> retrieveIdentityProviders(@Request public ResponseEntity retrieveIdentityProvider(@PathVariable String id, @RequestParam(required = false, defaultValue = "false") boolean rawConfig) { IdentityProvider identityProvider = identityProviderProvisioning.retrieve(id, identityZoneManager.getCurrentIdentityZoneId()); identityProvider.setSerializeConfigRaw(rawConfig); + setAuthMethod(identityProvider); redactSensitiveData(identityProvider); return new ResponseEntity<>(identityProvider, OK); } @@ -470,4 +506,23 @@ protected void redactSensitiveData(IdentityProvider provider) { } } + protected void setAuthMethod(IdentityProvider provider) { + if (provider.getConfig() instanceof AbstractExternalOAuthIdentityProviderDefinition abstractExternalOAuthIdentityProviderDefinition) { + if (abstractExternalOAuthIdentityProviderDefinition.getRelyingPartySecret() == null) { + if (abstractExternalOAuthIdentityProviderDefinition instanceof OIDCIdentityProviderDefinition oidcIdentityProviderDefinition && + oidcIdentityProviderDefinition.getJwtClientAuthentication() != null) { + abstractExternalOAuthIdentityProviderDefinition.setAuthMethod(ClientAuthentication.PRIVATE_KEY_JWT); + } else { + abstractExternalOAuthIdentityProviderDefinition.setAuthMethod(ClientAuthentication.NONE); + } + } else { + if (abstractExternalOAuthIdentityProviderDefinition.isClientAuthInBody()) { + abstractExternalOAuthIdentityProviderDefinition.setAuthMethod(ClientAuthentication.CLIENT_SECRET_POST); + } else { + abstractExternalOAuthIdentityProviderDefinition.setAuthMethod(ClientAuthentication.CLIENT_SECRET_BASIC); + } + } + } + } + } diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpointsTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpointsTest.java index 776067616e7..32f6d081ae2 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpointsTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpointsTest.java @@ -24,6 +24,7 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.springframework.http.HttpStatus.UNPROCESSABLE_ENTITY; import java.net.MalformedURLException; import java.net.URL; @@ -40,6 +41,7 @@ import org.assertj.core.api.Assertions; import org.cloudfoundry.identity.uaa.alias.EntityAliasFailedException; import org.cloudfoundry.identity.uaa.audit.event.EntityDeletedEvent; +import org.cloudfoundry.identity.uaa.constants.ClientAuthentication; import org.cloudfoundry.identity.uaa.constants.OriginKeys; import org.cloudfoundry.identity.uaa.extensions.PollutionPreventionExtension; import org.cloudfoundry.identity.uaa.zone.IdentityZone; @@ -288,7 +290,7 @@ void patch_bind_password_non_ldap() { void retrieve_all_providers_redacts_data() { when(mockIdentityProviderProvisioning.retrieveAll(anyBoolean(), anyString())) .thenReturn(Arrays.asList(getLdapDefinition(), getExternalOAuthProvider())); - ResponseEntity> ldapList = identityProviderEndpoints.retrieveIdentityProviders("false", true); + ResponseEntity> ldapList = identityProviderEndpoints.retrieveIdentityProviders("false", true, ""); assertNotNull(ldapList); assertNotNull(ldapList.getBody()); assertEquals(2, ldapList.getBody().size()); @@ -305,6 +307,43 @@ void retrieve_all_providers_redacts_data() { assertNull(oauth.getConfig().getRelyingPartySecret()); } + @Test + void retrieve_by_origin_providers_redacts_data() { + when(mockIdentityProviderProvisioning.retrieveByOrigin(anyString(), anyString())) + .thenReturn(getExternalOAuthProvider()); + ResponseEntity> puppyList = identityProviderEndpoints.retrieveIdentityProviders("false", true, "puppy"); + assertNotNull(puppyList); + assertNotNull(puppyList.getBody()); + assertEquals(1, puppyList.getBody().size()); + IdentityProvider oidc = puppyList.getBody().get(0); + assertNotNull(oidc); + assertNotNull(oidc.getConfig()); + assertTrue(oidc.getConfig() instanceof AbstractExternalOAuthIdentityProviderDefinition); + assertNull(oidc.getConfig().getRelyingPartySecret()); + assertEquals(ClientAuthentication.CLIENT_SECRET_BASIC, oidc.getConfig().getAuthMethod()); + } + + @Test + void delete_secret_and_retrieve_by_origin_providers_redacts_data() { + when(mockIdentityProviderProvisioning.retrieve("puppyId", "uaa")).thenReturn(getExternalOAuthProvider()); + ResponseEntity> oidcBody = identityProviderEndpoints.deleteSecret("puppyId"); + IdentityProvider oidc = oidcBody.getBody(); + assertNotNull(oidc); + assertNotNull(oidc.getConfig()); + assertTrue(oidc.getConfig() instanceof AbstractExternalOAuthIdentityProviderDefinition); + assertNull(((AbstractExternalOAuthIdentityProviderDefinition) oidc.getConfig()).getRelyingPartySecret()); + assertEquals(ClientAuthentication.NONE, ((AbstractExternalOAuthIdentityProviderDefinition) oidc.getConfig()).getAuthMethod()); + } + + @Test + void delete_secret_on_ldap_fails() { + when(mockIdentityProviderProvisioning.retrieve("puppyId", "uaa")).thenReturn(getLdapDefinition()); + ResponseEntity> oidcBody = identityProviderEndpoints.deleteSecret("puppyId"); + IdentityProvider oidc = oidcBody.getBody(); + assertEquals(UNPROCESSABLE_ENTITY, oidcBody.getStatusCode()); + assertNull(oidc); + } + @Test void update_ldap_provider_patches_password() throws Exception { IdentityProvider provider = retrieve_ldap_provider_by_id("id"); @@ -434,7 +473,7 @@ void shouldRespondWith422_WhenAliasPropertiesAreNotValid() throws MetadataProvid true ); - Assertions.assertThat(response.getStatusCode()).isEqualTo(HttpStatus.UNPROCESSABLE_ENTITY); + Assertions.assertThat(response.getStatusCode()).isEqualTo(UNPROCESSABLE_ENTITY); } @ParameterizedTest @@ -550,7 +589,7 @@ void shouldRespondWith422_WhenAliasPropertiesAreNotValid() throws MetadataProvid true ); - Assertions.assertThat(response.getStatusCode()).isEqualTo(HttpStatus.UNPROCESSABLE_ENTITY); + Assertions.assertThat(response.getStatusCode()).isEqualTo(UNPROCESSABLE_ENTITY); } @ParameterizedTest @@ -685,7 +724,7 @@ void testDeleteIdpWithAlias_AliasFeatureDisabled() { ); // deletion should be rejected - assertThat(response.getStatusCode()).isEqualTo(HttpStatus.UNPROCESSABLE_ENTITY); + assertThat(response.getStatusCode()).isEqualTo(UNPROCESSABLE_ENTITY); } private Pair, IdentityProvider> arrangeIdpWithAliasExists(final String zone1Id, final String zone2Id) { @@ -759,7 +798,7 @@ void create_oauth_provider_removes_password() throws Exception { void testPatchIdentityProviderStatusInvalidPayload() { IdentityProviderStatus identityProviderStatus = new IdentityProviderStatus(); ResponseEntity responseEntity = identityProviderEndpoints.updateIdentityProviderStatus("123", identityProviderStatus); - assertEquals(HttpStatus.UNPROCESSABLE_ENTITY, responseEntity.getStatusCode()); + assertEquals(UNPROCESSABLE_ENTITY, responseEntity.getStatusCode()); } @Test @@ -772,7 +811,7 @@ void testPatchIdentityProviderStatusInvalidIDP() { notUAAIDP.setConfig(new SamlIdentityProviderDefinition()); when(mockIdentityProviderProvisioning.retrieve(anyString(), eq(zoneId))).thenReturn(notUAAIDP); ResponseEntity responseEntity = identityProviderEndpoints.updateIdentityProviderStatus("123", identityProviderStatus); - assertEquals(HttpStatus.UNPROCESSABLE_ENTITY, responseEntity.getStatusCode()); + assertEquals(UNPROCESSABLE_ENTITY, responseEntity.getStatusCode()); } @Test @@ -785,7 +824,7 @@ void testPatchIdentityProviderStatusWithNoIDPDefinition() { invalidIDP.setType(OriginKeys.UAA); when(mockIdentityProviderProvisioning.retrieve(anyString(), eq(zoneId))).thenReturn(invalidIDP); ResponseEntity responseEntity = identityProviderEndpoints.updateIdentityProviderStatus("123", identityProviderStatus); - assertEquals(HttpStatus.UNPROCESSABLE_ENTITY, responseEntity.getStatusCode()); + assertEquals(UNPROCESSABLE_ENTITY, responseEntity.getStatusCode()); } @Test @@ -798,7 +837,7 @@ void testPatchIdentityProviderStatusWithNoPasswordPolicy() { invalidIDP.setConfig(new UaaIdentityProviderDefinition(null, null)); when(mockIdentityProviderProvisioning.retrieve(anyString(), eq(zoneId))).thenReturn(invalidIDP); ResponseEntity responseEntity = identityProviderEndpoints.updateIdentityProviderStatus("123", identityProviderStatus); - assertEquals(HttpStatus.UNPROCESSABLE_ENTITY, responseEntity.getStatusCode()); + assertEquals(UNPROCESSABLE_ENTITY, responseEntity.getStatusCode()); } @Test @@ -845,7 +884,7 @@ void testDeleteIdentityProviderNotExisting() { ResponseEntity deleteResponse = identityProviderEndpoints.deleteIdentityProvider( identityProviderIdentifier, false); - assertEquals(HttpStatus.UNPROCESSABLE_ENTITY, + assertEquals(UNPROCESSABLE_ENTITY, deleteResponse.getStatusCode()); } @@ -891,6 +930,36 @@ void testDeleteIdentityProviderResponseNotContainingBindPassword() { .getBody().getConfig()).getBindPassword()); } + @Test + void set_auth_client_secret() { + for (String type : Arrays.asList(OIDC10, OAUTH20)) { + IdentityProvider provider = getExternalOAuthProvider(); + AbstractExternalOAuthIdentityProviderDefinition def = provider.getConfig(); + AbstractExternalOAuthIdentityProviderDefinition spy = Mockito.spy(def); + provider.setConfig(spy); + provider.setType(type); + // standard secret usage + when(spy.getRelyingPartySecret()).thenReturn("secret"); + identityProviderEndpoints.setAuthMethod(provider); + assertEquals(ClientAuthentication.CLIENT_SECRET_BASIC, provider.getConfig().getAuthMethod()); + // use secrets in body + when(spy.isClientAuthInBody()).thenReturn(true); + identityProviderEndpoints.setAuthMethod(provider); + assertEquals(ClientAuthentication.CLIENT_SECRET_POST, provider.getConfig().getAuthMethod()); + // no secret usage but treat it as public client + when(spy.getRelyingPartySecret()).thenReturn(null); + identityProviderEndpoints.setAuthMethod(provider); + assertEquals(ClientAuthentication.NONE, provider.getConfig().getAuthMethod()); + // private_key_jwt in OIDC case + if (OIDC10.equals(type)) { + OIDCIdentityProviderDefinition oidcSpy = (OIDCIdentityProviderDefinition) spy; + when(oidcSpy.getJwtClientAuthentication()).thenReturn(Boolean.TRUE); + identityProviderEndpoints.setAuthMethod(provider); + assertEquals(ClientAuthentication.PRIVATE_KEY_JWT, provider.getConfig().getAuthMethod()); + } + } + } + private void arrangeAliasEntitiesEnabled(final boolean enabled) { ReflectionTestUtils.setField(identityProviderEndpoints, "aliasEntitiesEnabled", enabled); } From e896c0b7337ff6c3bb3b9c9b8169b5230b6e69f1 Mon Sep 17 00:00:00 2001 From: d036670 Date: Fri, 10 May 2024 17:30:04 +0200 Subject: [PATCH 03/19] Documentation --- .../source/index.html.md.erb | 63 ++++++++++++++++++ .../IdentityProviderEndpointDocs.java | 65 ++++++++++++++++++- 2 files changed, 126 insertions(+), 2 deletions(-) diff --git a/uaa/slateCustomizations/source/index.html.md.erb b/uaa/slateCustomizations/source/index.html.md.erb index 490ae2f633f..dad26056e38 100644 --- a/uaa/slateCustomizations/source/index.html.md.erb +++ b/uaa/slateCustomizations/source/index.html.md.erb @@ -1115,6 +1115,31 @@ _Error Codes_ |------------|-----------------------------------------------------------------------| | 403 | Forbidden - Insufficient scope | +## Retrieve with Filtering + +<%= render('IdentityProviderEndpointDocs/getFilteredIdentityProviders/curl-request.md') %> +<%= render('IdentityProviderEndpointDocs/getFilteredIdentityProviders/http-request.md') %> +<%= render('IdentityProviderEndpointDocs/getFilteredIdentityProviders/http-response.md') %> + +_Request Headers_ + +<%= render('IdentityProviderEndpointDocs/getFilteredIdentityProviders/request-headers.md') %> + +_Request Parameters_ + +<%= render('IdentityProviderEndpointDocs/getFilteredIdentityProviders/request-parameters.md') %> + +_Response Fields_ + +<%= render('IdentityProviderEndpointDocs/getFilteredIdentityProviders/response-fields.md') %> + +_Error Codes_ + +| Error Code | Description | +|------------|-----------------------------------------------------------------------| +| 403 | Forbidden - Insufficient scope | + + ## Retrieve <%= render('IdentityProviderEndpointDocs/getIdentityProvider/curl-request.md') %> @@ -1237,6 +1262,44 @@ _Error Codes_ | 422 | Unprocessable Entity - Invalid config | +## Delete Secret + + +
+Delete a secret from the OAuth2 / OIDC IdP configuration. +
+ +<%= render('IdentityProviderEndpointDocs/deleteIdentityProviderSecret/curl-request.md') %> +<%= render('IdentityProviderEndpointDocs/deleteIdentityProviderSecret/http-request.md') %> +<%= render('IdentityProviderEndpointDocs/deleteIdentityProviderSecret/http-response.md') %> + +_Path Parameters_ + +<%= render('IdentityProviderEndpointDocs/deleteIdentityProviderSecret/path-parameters.md') %> + +_Request Headers_ + +<%= render('IdentityProviderEndpointDocs/deleteIdentityProviderSecret/request-headers.md') %> + + + +_Request and Response Fields_ + +<%= render('IdentityProviderEndpointDocs/deleteIdentityProviderSecret/request-fields.md') %> + +_Error Codes_ + +| Error Code | Description | +|------------|-----------------------------------------------------------------------| +| 403 | Forbidden - Insufficient scope | +| 422 | Unprocessable Entity - Invalid config | + # Users Users can be queried, created and updated via the `/Users` endpoint. 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 24a27cd2f39..5bbfe7a9051 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 @@ -106,7 +106,6 @@ import org.springframework.restdocs.payload.FieldDescriptor; import org.springframework.restdocs.snippet.Attributes; import org.springframework.restdocs.snippet.Snippet; -import org.cloudfoundry.identity.uaa.client.UaaClientDetails; import org.springframework.test.web.servlet.ResultActions; class IdentityProviderEndpointDocs extends EndpointDocs { @@ -591,6 +590,7 @@ void createOAuthIdentityProvider() throws Exception { 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"), fieldWithPath("config.linkText").optional(null).type(STRING).description("Text to use for the login link to the provider"), + fieldWithPath("config.auth_method").optional(null).type(STRING).description("UAA 77.10.0 Client authentication method. Possible strings are: client_secret_basic, client_secret_post, private_key_jwt, none."), fieldWithPath("config.relyingPartyId").required().type(STRING).description("The client ID which is registered with the external OAuth provider for use by the UAA"), fieldWithPath("config.skipSslValidation").optional(null).type(BOOLEAN).description("A flag controlling whether SSL validation should be skipped when communicating with the external OAuth server"), fieldWithPath("config.scopes").optional(null).type(ARRAY).description("What scopes to request on a call to the external OAuth provider"), @@ -626,7 +626,7 @@ void createOAuthIdentityProvider() throws Exception { IDENTITY_ZONE_ID, CREATED, LAST_MODIFIED, - fieldWithPath("config.externalGroupsWhitelist").optional(null).type(ARRAY).description("Not currently used.") + fieldWithPath("config.externalGroupsWhitelist").optional(null).type(ARRAY).description("Not currently used."), }, ALIAS_FIELDS_GET ) @@ -688,6 +688,7 @@ void createOidcIdentityProvider() throws Exception { 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"), + fieldWithPath("config.auth_method").optional(null).type(STRING).description("UAA 77.10.0 Client authentication method. Possible strings are: client_secret_basic, client_secret_post, private_key_jwt, none."), fieldWithPath("config.relyingPartyId").required().type(STRING).description("The client ID which is registered with the external OAuth provider for use by the UAA"), fieldWithPath("config.skipSslValidation").optional(null).type(BOOLEAN).description("A flag controlling whether SSL validation should be skipped when communicating with the external OAuth server"), fieldWithPath("config.scopes").optional(null).type(ARRAY).description("What scopes to request on a call to the external OAuth/OpenID provider. For example, can provide " + @@ -959,6 +960,47 @@ void getAllIdentityProviders() throws Exception { responseFields)); } + @Test + void getFilteredIdentityProviders() throws Exception { + Snippet responseFields = responseFields( + fieldWithPath("[].type").description("Type of the identity provider."), + fieldWithPath("[].originKey").description("Unique identifier for the identity provider."), + fieldWithPath("[].name").description(NAME_DESC), + fieldWithPath("[].config").description(CONFIG_DESCRIPTION), + fieldWithPath("[]." + FIELD_ALIAS_ID).description(ALIAS_ID_DESC).attributes(key("constraints").value("Optional")).optional().type(STRING), + fieldWithPath("[]." + FIELD_ALIAS_ZID).description(ALIAS_ZID_DESC).attributes(key("constraints").value("Optional")).optional().type(STRING), + + fieldWithPath("[].version").description(VERSION_DESC), + fieldWithPath("[].active").description(ACTIVE_DESC), + + fieldWithPath("[].id").description(ID_DESC), + fieldWithPath("[].identityZoneId").description(IDENTITY_ZONE_ID_DESC), + fieldWithPath("[].created").description(CREATED_DESC), + fieldWithPath("[].last_modified").description(LAST_MODIFIED_DESC) + ); + + mockMvc.perform(get("/identity-providers") + .param("rawConfig", "false") + .param("active_only", "false") + .param("origin", "my-oauth2-provider") + .header("Authorization", "Bearer " + adminToken) + .contentType(APPLICATION_JSON)) + .andExpect(status().isOk()) + .andDo(document("{ClassName}/{methodName}", + preprocessResponse(prettyPrint()), + requestHeaders( + headerWithName("Authorization").description("Bearer token containing `zones..admin` or `zones..idps.read` or `uaa.admin` or `idps.read` (only in the same zone that you are a user of)"), + headerWithName("X-Identity-Zone-Id").description("May include this header to administer another zone if using `zones..admin` or `zones..idps.read` or `uaa.admin` scope against the default UAA zone.").optional(), + IDENTITY_ZONE_SUBDOMAIN_HEADER + ), + requestParameters( + parameterWithName("rawConfig").optional("false").type(BOOLEAN).description("Flag indicating whether the response should use raw, unescaped JSON for the `config` field of the IDP, rather than the default behavior of encoding the JSON as a string."), + parameterWithName("active_only").optional("false").type(BOOLEAN).description("Flag indicating whether only active IdPs should be returned or all."), + parameterWithName("origin").optional(null).type(STRING).description("UAA 77.10.0 Return only IdPs with specific origin.") + ), + responseFields)); + } + @Test void getIdentityProvider() throws Exception { IdentityProvider identityProvider = JsonUtils.readValue(mockMvc.perform(post("/identity-providers") @@ -1091,6 +1133,25 @@ void patchIdentityProviderStatus() throws Exception { } + @Test + void createOAuthIdentityProviderThenDeleteSecret() throws Exception { + IdentityProvider identityProvider = identityProviderProvisioning.retrieveByOrigin("my-oauth2-provider", IdentityZoneHolder.get().getId()); + + mockMvc.perform(delete("/identity-providers/{id}/secret", identityProvider.getId()) + .header("Authorization", "Bearer " + adminToken)) + .andExpect(status().isOk()) + .andDo(document("{ClassName}/{methodName}", + preprocessResponse(prettyPrint()), + pathParameters(parameterWithName("id").description(ID_DESC) + ), + requestHeaders( + headerWithName("Authorization").description("Bearer token containing `zones..admin` or `uaa.admin` or `idps.write` (only in the same zone that you are a user of)"), + IDENTITY_ZONE_ID_HEADER, + IDENTITY_ZONE_SUBDOMAIN_HEADER + ), + responseFields(getCommonProviderFieldsAnyType()))); + } + @Test void deleteIdentityProvider() throws Exception { IdentityProvider identityProvider = JsonUtils.readValue(mockMvc.perform(post("/identity-providers") From b35b9edcf7a24e056c0642bf40a764d1e0bbdc43 Mon Sep 17 00:00:00 2001 From: d036670 Date: Fri, 10 May 2024 18:00:45 +0200 Subject: [PATCH 04/19] fix names --- uaa/slateCustomizations/source/index.html.md.erb | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/uaa/slateCustomizations/source/index.html.md.erb b/uaa/slateCustomizations/source/index.html.md.erb index dad26056e38..294491df73f 100644 --- a/uaa/slateCustomizations/source/index.html.md.erb +++ b/uaa/slateCustomizations/source/index.html.md.erb @@ -1271,27 +1271,29 @@ _Error Codes_ Delete a secret from the OAuth2 / OIDC IdP configuration.
-<%= render('IdentityProviderEndpointDocs/deleteIdentityProviderSecret/curl-request.md') %> -<%= render('IdentityProviderEndpointDocs/deleteIdentityProviderSecret/http-request.md') %> -<%= render('IdentityProviderEndpointDocs/deleteIdentityProviderSecret/http-response.md') %> +<%= render('IdentityProviderEndpointDocs/createOAuthIdentityProviderThenDeleteSecret/curl-request.md') %> +<%= render('IdentityProviderEndpointDocs/createOAuthIdentityProviderThenDeleteSecret/http-request.md') %> +<%= render('IdentityProviderEndpointDocs/createOAuthIdentityProviderThenDeleteSecret/http-response.md') %> _Path Parameters_ -<%= render('IdentityProviderEndpointDocs/deleteIdentityProviderSecret/path-parameters.md') %> +<%= render('IdentityProviderEndpointDocs/createOAuthIdentityProviderThenDeleteSecret/path-parameters.md') %> _Request Headers_ -<%= render('IdentityProviderEndpointDocs/deleteIdentityProviderSecret/request-headers.md') %> +<%= render('IdentityProviderEndpointDocs/createOAuthIdentityProviderThenDeleteSecret/request-headers.md') %> _Request and Response Fields_ -<%= render('IdentityProviderEndpointDocs/deleteIdentityProviderSecret/request-fields.md') %> +<%= render('IdentityProviderEndpointDocs/createOAuthIdentityProviderThenDeleteSecret/response-fields.md') %> _Error Codes_ From 75542a1e0174dadc8e39fa2c24af877b87e585bb Mon Sep 17 00:00:00 2001 From: d036670 Date: Fri, 10 May 2024 20:02:53 +0200 Subject: [PATCH 05/19] sonar --- .../identity/uaa/provider/IdentityProviderTest.java | 2 ++ .../uaa/provider/OIDCIdentityProviderDefinitionTests.java | 1 + .../identity/uaa/provider/IdentityProviderEndpoints.java | 6 +++--- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/model/src/test/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderTest.java b/model/src/test/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderTest.java index 52f5aadfa48..2d68ccc47c9 100644 --- a/model/src/test/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderTest.java +++ b/model/src/test/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderTest.java @@ -58,6 +58,7 @@ void testEqualsAndHashCode() { idp1.setIdentityZoneId(UAA); final OIDCIdentityProviderDefinition config1 = new OIDCIdentityProviderDefinition(); config1.setIssuer("issuer"); + config1.setAuthMethod("none"); idp1.setConfig(config1); final IdentityProvider idp2 = new IdentityProvider<>(); @@ -70,6 +71,7 @@ void testEqualsAndHashCode() { idp2.setIdentityZoneId(UAA); final OIDCIdentityProviderDefinition config2 = new OIDCIdentityProviderDefinition(); config2.setIssuer("issuer"); + config2.setAuthMethod("none"); idp2.setConfig(config2); idp2.setCreated(idp1.getCreated()); diff --git a/model/src/test/java/org/cloudfoundry/identity/uaa/provider/OIDCIdentityProviderDefinitionTests.java b/model/src/test/java/org/cloudfoundry/identity/uaa/provider/OIDCIdentityProviderDefinitionTests.java index ff4f0f6cf44..436b6a1ef3c 100644 --- a/model/src/test/java/org/cloudfoundry/identity/uaa/provider/OIDCIdentityProviderDefinitionTests.java +++ b/model/src/test/java/org/cloudfoundry/identity/uaa/provider/OIDCIdentityProviderDefinitionTests.java @@ -80,5 +80,6 @@ public void serialize_jwtClientAuthentication() { String json = JsonUtils.writeValueAsString(def); def = JsonUtils.readValue(json, OIDCIdentityProviderDefinition.class); assertEquals(settings, def.getJwtClientAuthentication()); + assertNull(def.getAuthMethod()); } } \ No newline at end of file diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java index 6d3df271208..6f6c15fa0a6 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java @@ -316,9 +316,9 @@ public ResponseEntity updateIdentityProviderStatus(@Path } @DeleteMapping(value = "{id}/secret") - public ResponseEntity> deleteSecret(@PathVariable String id) { + public ResponseEntity deleteSecret(@PathVariable String id) { String zoneId = identityZoneManager.getCurrentIdentityZoneId(); - IdentityProvider existing = identityProviderProvisioning.retrieve(id, zoneId); + IdentityProvider existing = identityProviderProvisioning.retrieve(id, zoneId); if((OIDC10.equals(existing.getType()) || OAUTH20.equals(existing.getType())) && existing.getConfig() instanceof AbstractExternalOAuthIdentityProviderDefinition idpConfiguration) { idpConfiguration.setRelyingPartySecret(null); @@ -326,7 +326,7 @@ public ResponseEntity> deleteSecret(@PathVariable String id) setAuthMethod(existing); redactSensitiveData(existing); logger.info("Secret deleted for Identity Provider: {}", existing.getId()); - return new ResponseEntity<>(existing, OK); + return new ResponseEntity<>(existing, OK); } else { logger.debug("Invalid operation. This operation is only supported on external OAuth/OIDC IDP"); return new ResponseEntity<>(UNPROCESSABLE_ENTITY); From 0b3a6728678af8957e581323beb30e13a3142122 Mon Sep 17 00:00:00 2001 From: d036670 Date: Fri, 10 May 2024 20:32:25 +0200 Subject: [PATCH 06/19] sonar --- .../identity/uaa/provider/IdentityProviderEndpointsTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpointsTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpointsTest.java index 32f6d081ae2..f2d758cf830 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpointsTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpointsTest.java @@ -326,7 +326,7 @@ void retrieve_by_origin_providers_redacts_data() { @Test void delete_secret_and_retrieve_by_origin_providers_redacts_data() { when(mockIdentityProviderProvisioning.retrieve("puppyId", "uaa")).thenReturn(getExternalOAuthProvider()); - ResponseEntity> oidcBody = identityProviderEndpoints.deleteSecret("puppyId"); + ResponseEntity oidcBody = identityProviderEndpoints.deleteSecret("puppyId"); IdentityProvider oidc = oidcBody.getBody(); assertNotNull(oidc); assertNotNull(oidc.getConfig()); @@ -338,7 +338,7 @@ void delete_secret_and_retrieve_by_origin_providers_redacts_data() { @Test void delete_secret_on_ldap_fails() { when(mockIdentityProviderProvisioning.retrieve("puppyId", "uaa")).thenReturn(getLdapDefinition()); - ResponseEntity> oidcBody = identityProviderEndpoints.deleteSecret("puppyId"); + ResponseEntity oidcBody = identityProviderEndpoints.deleteSecret("puppyId"); IdentityProvider oidc = oidcBody.getBody(); assertEquals(UNPROCESSABLE_ENTITY, oidcBody.getStatusCode()); assertNull(oidc); From cd32e6149374461b837a21daf02af151ab1361f5 Mon Sep 17 00:00:00 2001 From: d036670 Date: Mon, 13 May 2024 10:31:14 +0200 Subject: [PATCH 07/19] Add patch call to change a secret from an external IdP --- .../IdentityProviderSecretChange.java | 17 +++++++ .../provider/IdentityProviderEndpoints.java | 27 +++++++++- .../IdentityProviderEndpointsTest.java | 49 +++++++++++++++++++ .../source/index.html.md.erb | 40 ++++++++++++++- .../IdentityProviderEndpointDocs.java | 36 +++++++++++++- 5 files changed, 164 insertions(+), 5 deletions(-) create mode 100644 model/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderSecretChange.java diff --git a/model/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderSecretChange.java b/model/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderSecretChange.java new file mode 100644 index 00000000000..74d8cb6a64b --- /dev/null +++ b/model/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderSecretChange.java @@ -0,0 +1,17 @@ +package org.cloudfoundry.identity.uaa.provider; + +import com.fasterxml.jackson.annotation.JsonInclude; + +@JsonInclude(JsonInclude.Include.NON_NULL) +public class IdentityProviderSecretChange { + + private String secret; + + public String getSecret() { + return this.secret; + } + + public void setSecret(final String secret) { + this.secret = secret; + } +} diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java index 6f6c15fa0a6..b477d75e31e 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java @@ -71,6 +71,7 @@ import org.springframework.transaction.support.TransactionTemplate; import org.springframework.web.bind.annotation.DeleteMapping; import org.springframework.web.bind.annotation.ExceptionHandler; +import org.springframework.web.bind.annotation.PatchMapping; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestMapping; @@ -328,11 +329,35 @@ public ResponseEntity deleteSecret(@PathVariable String id) { logger.info("Secret deleted for Identity Provider: {}", existing.getId()); return new ResponseEntity<>(existing, OK); } else { - logger.debug("Invalid operation. This operation is only supported on external OAuth/OIDC IDP"); + logger.debug("Invalid operation. This operation is only supported on external IDP of type OAuth/OIDC"); return new ResponseEntity<>(UNPROCESSABLE_ENTITY); } } + @PatchMapping(value = "{id}/secret") + public ResponseEntity changeSecret(@PathVariable String id, @RequestBody IdentityProviderSecretChange secretChange) { + String zoneId = identityZoneManager.getCurrentIdentityZoneId(); + IdentityProvider existing = identityProviderProvisioning.retrieve(id, zoneId); + if(UaaStringUtils.isNullOrEmpty(secretChange.getSecret())) { + logger.debug("Invalid payload. The property secret needs to be set"); + return new ResponseEntity<>(UNPROCESSABLE_ENTITY); + } + if((OIDC10.equals(existing.getType()) || OAUTH20.equals(existing.getType())) + && existing.getConfig() instanceof AbstractExternalOAuthIdentityProviderDefinition idpConfiguration) { + idpConfiguration.setRelyingPartySecret(secretChange.getSecret()); + } else if(LDAP.equals(existing.getType()) && existing.getConfig() instanceof LdapIdentityProviderDefinition ldapIdentityProviderDefinition) { + ldapIdentityProviderDefinition.setBindPassword(secretChange.getSecret()); + } else { + logger.debug("Invalid operation. This operation is only supported on external IDP of type LDAP/OAuth/OIDC"); + return new ResponseEntity<>(UNPROCESSABLE_ENTITY); + } + identityProviderProvisioning.update(existing, zoneId); + setAuthMethod(existing); + redactSensitiveData(existing); + logger.info("Secret changed for Identity Provider: {}", existing.getId()); + return new ResponseEntity<>(existing, OK); + } + @RequestMapping(method = GET) public ResponseEntity> retrieveIdentityProviders( @RequestParam(value = "active_only", required = false) String activeOnly, diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpointsTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpointsTest.java index f2d758cf830..7ead40f9084 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpointsTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpointsTest.java @@ -344,6 +344,55 @@ void delete_secret_on_ldap_fails() { assertNull(oidc); } + @Test + void change_bindPassword_and_retrieve_by_origin_providers_redacts_data() { + IdentityProvider idp = getLdapDefinition(); + when(mockIdentityProviderProvisioning.retrieve("puppyId", "uaa")).thenReturn(idp); + IdentityProviderSecretChange identityProviderSecretChange = new IdentityProviderSecretChange(); + identityProviderSecretChange.setSecret("newSecret"); + ResponseEntity oidcBody = identityProviderEndpoints.changeSecret("puppyId", identityProviderSecretChange); + IdentityProvider oidc = oidcBody.getBody(); + assertNotNull(oidc); + assertNotNull(oidc.getConfig()); + assertTrue(oidc.getConfig() instanceof LdapIdentityProviderDefinition); + assertNull(((LdapIdentityProviderDefinition) oidc.getConfig()).getBindPassword()); + } + + @Test + void change_secret_and_retrieve_by_origin_providers_redacts_data() { + IdentityProvider idp = getExternalOAuthProvider(); + when(mockIdentityProviderProvisioning.retrieve("puppyId", "uaa")).thenReturn(idp); + IdentityProviderSecretChange identityProviderSecretChange = new IdentityProviderSecretChange(); + identityProviderSecretChange.setSecret("newSecret"); + ResponseEntity oidcBody = identityProviderEndpoints.changeSecret("puppyId", identityProviderSecretChange); + IdentityProvider oidc = oidcBody.getBody(); + assertNotNull(oidc); + assertNotNull(oidc.getConfig()); + assertTrue(oidc.getConfig() instanceof AbstractExternalOAuthIdentityProviderDefinition); + assertNull(((AbstractExternalOAuthIdentityProviderDefinition) oidc.getConfig()).getRelyingPartySecret()); + assertEquals(ClientAuthentication.CLIENT_SECRET_BASIC, ((AbstractExternalOAuthIdentityProviderDefinition) oidc.getConfig()).getAuthMethod()); + } + + @Test + void change_secret_on_uaafails() { + IdentityProvider identityProvider = new IdentityProvider<>(); + identityProvider.setConfig(new SamlIdentityProviderDefinition()); + identityProvider.setName("my saml provider"); + identityProvider.setIdentityZoneId(OriginKeys.UAA); + identityProvider.setType(OriginKeys.SAML); + IdentityProviderSecretChange identityProviderSecretChange = new IdentityProviderSecretChange(); + when(mockIdentityProviderProvisioning.retrieve("puppyId", "uaa")).thenReturn(identityProvider); + ResponseEntity oidcBody = identityProviderEndpoints.changeSecret("puppyId", identityProviderSecretChange); + IdentityProvider oidc = oidcBody.getBody(); + assertEquals(UNPROCESSABLE_ENTITY, oidcBody.getStatusCode()); + assertNull(oidc); + identityProviderSecretChange.setSecret("newSecret"); + oidcBody = identityProviderEndpoints.changeSecret("puppyId", identityProviderSecretChange); + oidc = oidcBody.getBody(); + assertEquals(UNPROCESSABLE_ENTITY, oidcBody.getStatusCode()); + assertNull(oidc); + } + @Test void update_ldap_provider_patches_password() throws Exception { IdentityProvider provider = retrieve_ldap_provider_by_id("id"); diff --git a/uaa/slateCustomizations/source/index.html.md.erb b/uaa/slateCustomizations/source/index.html.md.erb index 294491df73f..632f61d5291 100644 --- a/uaa/slateCustomizations/source/index.html.md.erb +++ b/uaa/slateCustomizations/source/index.html.md.erb @@ -1261,6 +1261,42 @@ _Error Codes_ | 403 | Forbidden - Insufficient scope | | 422 | Unprocessable Entity - Invalid config | +## Change Secret + + +
+Change a relyingPartySecret in the OAuth2 / OIDC IdP configuration or bindPassword in case of LDAP. +
+ +<%= render('IdentityProviderEndpointDocs/createOAuthIdentityProviderThenChangeSecret/curl-request.md') %> +<%= render('IdentityProviderEndpointDocs/createOAuthIdentityProviderThenChangeSecret/http-request.md') %> +<%= render('IdentityProviderEndpointDocs/createOAuthIdentityProviderThenChangeSecret/http-response.md') %> + +_Path Parameters_ + +<%= render('IdentityProviderEndpointDocs/createOAuthIdentityProviderThenChangeSecret/path-parameters.md') %> + +_Request Headers_ + +<%= render('IdentityProviderEndpointDocs/createOAuthIdentityProviderThenChangeSecret/request-headers.md') %> + + + +_Request and Response Fields_ + +<%= render('IdentityProviderEndpointDocs/createOAuthIdentityProviderThenChangeSecret/response-fields.md') %> + +_Error Codes_ + +| Error Code | Description | +|------------|-----------------------------------------------------------------------| +| 403 | Forbidden - Insufficient scope | +| 422 | Unprocessable Entity - Invalid config | ## Delete Secret @@ -1268,7 +1304,7 @@ _Error Codes_ Added in UAA 77.10.0
-Delete a secret from the OAuth2 / OIDC IdP configuration. +Delete a secret from the OAuth2 / OIDC IdP configuration only, because these providers support usages without a secret.
<%= render('IdentityProviderEndpointDocs/createOAuthIdentityProviderThenDeleteSecret/curl-request.md') %> @@ -1288,7 +1324,7 @@ _Request Headers_ For a standard IdP the result of auth_method will be none, because the removal of the secret lead to public flow.
If jwtClientAuthentication section is configured, then after this call, the result of auth_method is private_key_jwt.
- If you want set again a secret or change the current secret, please use the update call and set relyingPartySecret.
+ If you want set again a secret or change the current secret, please use patch call, e.g. Change Secret
_Request and Response Fields_ 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 5bbfe7a9051..48abab062e3 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 @@ -77,6 +77,7 @@ import org.cloudfoundry.identity.uaa.provider.AbstractExternalOAuthIdentityProviderDefinition; import org.cloudfoundry.identity.uaa.provider.IdentityProvider; import org.cloudfoundry.identity.uaa.provider.IdentityProviderProvisioning; +import org.cloudfoundry.identity.uaa.provider.IdentityProviderSecretChange; import org.cloudfoundry.identity.uaa.provider.IdentityProviderStatus; import org.cloudfoundry.identity.uaa.provider.JdbcIdentityProviderProvisioning; import org.cloudfoundry.identity.uaa.provider.LdapIdentityProviderDefinition; @@ -1133,14 +1134,45 @@ void patchIdentityProviderStatus() throws Exception { } + @Test + void createOAuthIdentityProviderThenChangeSecret() throws Exception { + IdentityProvider identityProvider = identityProviderProvisioning.retrieveByOrigin("my-oauth2-provider", IdentityZoneHolder.get().getId()); + + IdentityProviderSecretChange identityProviderSecretChange = new IdentityProviderSecretChange(); + identityProviderSecretChange.setSecret("newSecret" + new AlphanumericRandomValueStringGenerator(10).generate()); + + FieldDescriptor[] idempotentFields = new FieldDescriptor[]{ + fieldWithPath("secret").required().description("Set new secret and/or bind password, depending on provided IdP type.") + }; + + Snippet requestFields = requestFields(idempotentFields); + + mockMvc.perform(patch("/identity-providers/{id}/secret", identityProvider.getId()) + .header("Authorization", "Bearer " + adminToken) + .contentType(APPLICATION_JSON) + .content(serializeExcludingProperties(identityProviderSecretChange))) + .andExpect(status().isOk()) + .andDo(document("{ClassName}/{methodName}", + preprocessResponse(prettyPrint()), + pathParameters(parameterWithName("id").description(ID_DESC) + ), + requestHeaders( + headerWithName("Authorization").description("Bearer token containing `zones..admin` or `uaa.admin` or `idps.write` (only in the same zone that you are a user of)"), + IDENTITY_ZONE_ID_HEADER, + IDENTITY_ZONE_SUBDOMAIN_HEADER + ), + requestFields, + responseFields(getCommonProviderFieldsAnyType()))); + } + @Test void createOAuthIdentityProviderThenDeleteSecret() throws Exception { IdentityProvider identityProvider = identityProviderProvisioning.retrieveByOrigin("my-oauth2-provider", IdentityZoneHolder.get().getId()); mockMvc.perform(delete("/identity-providers/{id}/secret", identityProvider.getId()) .header("Authorization", "Bearer " + adminToken)) - .andExpect(status().isOk()) - .andDo(document("{ClassName}/{methodName}", + .andExpect(status().isOk()) + .andDo(document("{ClassName}/{methodName}", preprocessResponse(prettyPrint()), pathParameters(parameterWithName("id").description(ID_DESC) ), From a89ce12514d81c05974c8d78f6cc456661491596 Mon Sep 17 00:00:00 2001 From: d036670 Date: Tue, 14 May 2024 12:18:51 +0200 Subject: [PATCH 08/19] Alias handling --- .../provider/IdentityProviderEndpoints.java | 24 ++++++++++++------- .../IdentityProviderEndpointsTest.java | 5 +++- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java index b477d75e31e..d55f082b26e 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java @@ -257,6 +257,11 @@ public ResponseEntity updateIdentityProvider(@PathVariable Str body.setConfig(definition); } + return persistIdentityProviderChange(body, rawConfig, zoneId, existing); + } + + private ResponseEntity persistIdentityProviderChange(IdentityProvider body, boolean rawConfig, String zoneId, + IdentityProvider existing) { final IdentityProvider updatedIdp; try { updatedIdp = transactionTemplate.execute(txStatus -> { @@ -321,13 +326,11 @@ public ResponseEntity deleteSecret(@PathVariable String id) { String zoneId = identityZoneManager.getCurrentIdentityZoneId(); IdentityProvider existing = identityProviderProvisioning.retrieve(id, zoneId); if((OIDC10.equals(existing.getType()) || OAUTH20.equals(existing.getType())) - && existing.getConfig() instanceof AbstractExternalOAuthIdentityProviderDefinition idpConfiguration) { - idpConfiguration.setRelyingPartySecret(null); - identityProviderProvisioning.update(existing, zoneId); - setAuthMethod(existing); - redactSensitiveData(existing); - logger.info("Secret deleted for Identity Provider: {}", existing.getId()); - return new ResponseEntity<>(existing, OK); + && existing.getConfig() instanceof AbstractExternalOAuthIdentityProviderDefinition) { + IdentityProvider updated = existing; + ((AbstractExternalOAuthIdentityProviderDefinition) updated.getConfig()).setRelyingPartySecret(null); + logger.info("Delete secret for Identity Provider: {}", existing.getId()); + return persistIdentityProviderChange(updated, false, zoneId, existing); } else { logger.debug("Invalid operation. This operation is only supported on external IDP of type OAuth/OIDC"); return new ResponseEntity<>(UNPROCESSABLE_ENTITY); @@ -343,8 +346,11 @@ public ResponseEntity changeSecret(@PathVariable String id, @R return new ResponseEntity<>(UNPROCESSABLE_ENTITY); } if((OIDC10.equals(existing.getType()) || OAUTH20.equals(existing.getType())) - && existing.getConfig() instanceof AbstractExternalOAuthIdentityProviderDefinition idpConfiguration) { - idpConfiguration.setRelyingPartySecret(secretChange.getSecret()); + && existing.getConfig() instanceof AbstractExternalOAuthIdentityProviderDefinition) { + IdentityProvider updated = existing; + ((AbstractExternalOAuthIdentityProviderDefinition) updated.getConfig()).setRelyingPartySecret(secretChange.getSecret()); + logger.info("Change secret for Identity Provider: {}", existing.getId()); + return persistIdentityProviderChange(updated, false, zoneId, existing); } else if(LDAP.equals(existing.getType()) && existing.getConfig() instanceof LdapIdentityProviderDefinition ldapIdentityProviderDefinition) { ldapIdentityProviderDefinition.setBindPassword(secretChange.getSecret()); } else { diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpointsTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpointsTest.java index 7ead40f9084..9f2ebde049e 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpointsTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpointsTest.java @@ -325,7 +325,9 @@ void retrieve_by_origin_providers_redacts_data() { @Test void delete_secret_and_retrieve_by_origin_providers_redacts_data() { - when(mockIdentityProviderProvisioning.retrieve("puppyId", "uaa")).thenReturn(getExternalOAuthProvider()); + IdentityProvider idp = getExternalOAuthProvider(); + when(mockIdpAliasHandler.ensureConsistencyOfAliasEntity(null, idp)).thenReturn(idp); + when(mockIdentityProviderProvisioning.retrieve("puppyId", "uaa")).thenReturn(idp); ResponseEntity oidcBody = identityProviderEndpoints.deleteSecret("puppyId"); IdentityProvider oidc = oidcBody.getBody(); assertNotNull(oidc); @@ -361,6 +363,7 @@ void change_bindPassword_and_retrieve_by_origin_providers_redacts_data() { @Test void change_secret_and_retrieve_by_origin_providers_redacts_data() { IdentityProvider idp = getExternalOAuthProvider(); + when(mockIdpAliasHandler.ensureConsistencyOfAliasEntity(null, idp)).thenReturn(idp); when(mockIdentityProviderProvisioning.retrieve("puppyId", "uaa")).thenReturn(idp); IdentityProviderSecretChange identityProviderSecretChange = new IdentityProviderSecretChange(); identityProviderSecretChange.setSecret("newSecret"); From ede8431990e674b5b645b96149ffefc0158d0203 Mon Sep 17 00:00:00 2001 From: d036670 Date: Tue, 14 May 2024 13:33:59 +0200 Subject: [PATCH 09/19] Sonar refactorings See https://sonarcloud.io/code?id=cloudfoundry-identity-parent&selected=cloudfoundry-identity-parent%3Aserver%2Fsrc%2Fmain%2Fjava%2Forg%2Fcloudfoundry%2Fidentity%2Fuaa%2Fprovider%2FIdentityProviderEndpoints.java --- .../provider/IdentityProviderEndpoints.java | 145 +++++++----------- 1 file changed, 57 insertions(+), 88 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java index 77d42fb8ab0..b726ba098c7 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java @@ -27,11 +27,6 @@ import static org.springframework.http.HttpStatus.OK; import static org.springframework.http.HttpStatus.UNPROCESSABLE_ENTITY; import static org.springframework.util.StringUtils.hasText; -import static org.springframework.web.bind.annotation.RequestMethod.DELETE; -import static org.springframework.web.bind.annotation.RequestMethod.GET; -import static org.springframework.web.bind.annotation.RequestMethod.PATCH; -import static org.springframework.web.bind.annotation.RequestMethod.POST; -import static org.springframework.web.bind.annotation.RequestMethod.PUT; import java.io.PrintWriter; import java.io.StringWriter; @@ -50,8 +45,9 @@ import org.cloudfoundry.identity.uaa.util.ObjectUtils; import org.cloudfoundry.identity.uaa.zone.beans.IdentityZoneManager; import org.opensaml.saml2.metadata.provider.MetadataProviderException; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.beans.factory.annotation.Value; import org.springframework.context.ApplicationEventPublisher; @@ -67,8 +63,13 @@ import org.springframework.transaction.PlatformTransactionManager; import org.springframework.transaction.annotation.Transactional; import org.springframework.transaction.support.TransactionTemplate; +import org.springframework.web.bind.annotation.DeleteMapping; import org.springframework.web.bind.annotation.ExceptionHandler; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.PatchMapping; import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.PostMapping; +import org.springframework.web.bind.annotation.PutMapping; import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestParam; @@ -78,7 +79,7 @@ @RestController public class IdentityProviderEndpoints implements ApplicationEventPublisherAware { - protected static Logger logger = LoggerFactory.getLogger(IdentityProviderEndpoints.class); + protected static Logger logger = LogManager.getLogger(); @Value("${login.aliasEntitiesEnabled:false}") private boolean aliasEntitiesEnabled; @@ -119,7 +120,7 @@ public IdentityProviderEndpoints( this.idpAliasHandler = idpAliasHandler; } - @RequestMapping(method = POST) + @PostMapping() public ResponseEntity createIdentityProvider(@RequestBody IdentityProvider body, @RequestParam(required = false, defaultValue = "false") boolean rawConfig) throws MetadataProviderException{ body.setSerializeConfigRaw(rawConfig); String zoneId = identityZoneManager.getCurrentIdentityZoneId(); @@ -127,7 +128,8 @@ public ResponseEntity createIdentityProvider(@RequestBody Iden try { configValidator.validate(body); } catch (IllegalArgumentException e) { - logger.debug("IdentityProvider[origin="+body.getOriginKey()+"; zone="+body.getIdentityZoneId()+"] - Configuration validation error.", e); + logger.log(Level.DEBUG, () -> String.format("IdentityProvider[origin=%s; zone=%s] - Configuration validation error.", + body.getOriginKey(), body.getIdentityZoneId()), e); return new ResponseEntity<>(body, UNPROCESSABLE_ENTITY); } if (SAML.equals(body.getType())) { @@ -143,37 +145,10 @@ public ResponseEntity createIdentityProvider(@RequestBody Iden } // persist IdP and create alias if necessary - final IdentityProvider createdIdp; - try { - createdIdp = transactionTemplate.execute(txStatus -> { - final IdentityProvider createdOriginalIdp = identityProviderProvisioning.create(body, zoneId); - return idpAliasHandler.ensureConsistencyOfAliasEntity(createdOriginalIdp, null); - }); - } catch (final IdpAlreadyExistsException e) { - return new ResponseEntity<>(body, CONFLICT); - } catch (final EntityAliasFailedException e) { - logger.warn("Could not create alias for {}", e.getMessage()); - final HttpStatus responseCode = Optional.ofNullable(HttpStatus.resolve(e.getHttpStatus())).orElse(INTERNAL_SERVER_ERROR); - return new ResponseEntity<>(body, responseCode); - } catch (final Exception e) { - logger.warn("Unable to create IdentityProvider[origin=" + body.getOriginKey() + "; zone=" + body.getIdentityZoneId() + "]", e); - return new ResponseEntity<>(body, INTERNAL_SERVER_ERROR); - } - if (createdIdp == null) { - logger.warn( - "IdentityProvider[origin={}; zone={}] - Transaction creating IdP (and alias IdP, if applicable) was not successful, but no exception was thrown.", - getCleanedUserControlString(body.getOriginKey()), - getCleanedUserControlString(body.getIdentityZoneId()) - ); - return new ResponseEntity<>(body, UNPROCESSABLE_ENTITY); - } - createdIdp.setSerializeConfigRaw(rawConfig); - redactSensitiveData(createdIdp); - - return new ResponseEntity<>(createdIdp, CREATED); + return persistIdentityProviderChange(body, rawConfig, zoneId, null, CREATED); } - @RequestMapping(value = "{id}", method = DELETE) + @DeleteMapping(value = "{id}") @Transactional public ResponseEntity deleteIdentityProvider(@PathVariable String id, @RequestParam(required = false, defaultValue = "false") boolean rawConfig) { String identityZoneId = identityZoneManager.getCurrentIdentityZoneId(); @@ -217,7 +192,7 @@ public ResponseEntity deleteIdentityProvider(@PathVariable Str return new ResponseEntity<>(existing, OK); } - @RequestMapping(value = "{id}", method = PUT) + @PutMapping(value = "{id}") public ResponseEntity updateIdentityProvider(@PathVariable String id, @RequestBody IdentityProvider body, @RequestParam(required = false, defaultValue = "false") boolean rawConfig) throws MetadataProviderException { body.setSerializeConfigRaw(rawConfig); String zoneId = identityZoneManager.getCurrentIdentityZoneId(); @@ -228,16 +203,17 @@ public ResponseEntity updateIdentityProvider(@PathVariable Str try { configValidator.validate(body); } catch (IllegalArgumentException e) { - logger.debug("IdentityProvider[origin="+body.getOriginKey()+"; zone="+body.getIdentityZoneId()+"] - Configuration validation error for update.", e); + logger.log(Level.DEBUG, () -> String.format("IdentityProvider[origin=%s; zone=%s] - Configuration validation error for update.", + body.getOriginKey(), body.getIdentityZoneId()), e); return new ResponseEntity<>(body, UNPROCESSABLE_ENTITY); } if (!idpAliasHandler.aliasPropertiesAreValid(body, existing)) { - logger.warn( - "IdentityProvider[origin={}; zone={}] - Alias ID and/or ZID changed during update of IdP with alias.", + logger.log(Level.WARN, () -> String.format( + "IdentityProvider[origin=%s; zone=%s] - Alias ID and/or ZID changed during update of IdP with alias.", getCleanedUserControlString(body.getOriginKey()), getCleanedUserControlString(body.getIdentityZoneId()) - ); + )); return new ResponseEntity<>(body, UNPROCESSABLE_ENTITY); } @@ -250,37 +226,42 @@ public ResponseEntity updateIdentityProvider(@PathVariable Str body.setConfig(definition); } + return persistIdentityProviderChange(body, rawConfig, zoneId, existing, OK); + } + + private ResponseEntity persistIdentityProviderChange(IdentityProvider body, boolean rawConfig, String zoneId, + IdentityProvider existing, HttpStatus status) { final IdentityProvider updatedIdp; try { updatedIdp = transactionTemplate.execute(txStatus -> { - final IdentityProvider updatedOriginalIdp = identityProviderProvisioning.update(body, zoneId); + final IdentityProvider updatedOriginalIdp = status == CREATED ? identityProviderProvisioning.create(body, zoneId) : identityProviderProvisioning.update(body, zoneId); return idpAliasHandler.ensureConsistencyOfAliasEntity(updatedOriginalIdp, existing); }); } catch (final IdpAlreadyExistsException e) { return new ResponseEntity<>(body, CONFLICT); } catch (final EntityAliasFailedException e) { - logger.warn("Could not create alias for {}", e.getMessage()); + logger.log(Level.WARN, () -> String.format("Could not create alias for %s", e.getMessage())); final HttpStatus responseCode = Optional.ofNullable(HttpStatus.resolve(e.getHttpStatus())).orElse(INTERNAL_SERVER_ERROR); return new ResponseEntity<>(body, responseCode); } catch (final Exception e) { - logger.warn("Unable to update IdentityProvider[origin=" + body.getOriginKey() + "; zone=" + body.getIdentityZoneId() + "]", e); + logger.log(Level.WARN, () -> String.format("Unable to %s IdentityProvider[origin=%s; zone=%s]", + status == CREATED ? "create" : "update", body.getOriginKey(), body.getIdentityZoneId()), e); return new ResponseEntity<>(body, INTERNAL_SERVER_ERROR); } if (updatedIdp == null) { - logger.warn( - "IdentityProvider[origin={}; zone={}] - Transaction updating IdP (and alias IdP, if applicable) was not successful, but no exception was thrown.", - getCleanedUserControlString(body.getOriginKey()), - getCleanedUserControlString(body.getIdentityZoneId()) - ); + logger.log(Level.WARN, () -> String.format("IdentityProvider[origin=%s; zone=%s] - Transaction %s IdP (and alias IdP, if applicable) was not successful, but no exception was thrown.", + getCleanedUserControlString(body.getOriginKey()), + getCleanedUserControlString(body.getIdentityZoneId()), + status == CREATED ? "creating" : "updating")); return new ResponseEntity<>(body, UNPROCESSABLE_ENTITY); } updatedIdp.setSerializeConfigRaw(rawConfig); redactSensitiveData(updatedIdp); - return new ResponseEntity<>(updatedIdp, OK); + return new ResponseEntity<>(updatedIdp, status); } - @RequestMapping (value = "{id}/status", method = PATCH) + @PatchMapping(value = "{id}/status") public ResponseEntity updateIdentityProviderStatus(@PathVariable String id, @RequestBody IdentityProviderStatus body) { String zoneId = identityZoneManager.getCurrentIdentityZoneId(); IdentityProvider existing = identityProviderProvisioning.retrieve(id, zoneId); @@ -299,7 +280,7 @@ public ResponseEntity updateIdentityProviderStatus(@Path } uaaIdentityProviderDefinition.getPasswordPolicy().setPasswordNewerThan(new Date(System.currentTimeMillis())); identityProviderProvisioning.update(existing, zoneId); - logger.info("PasswordChangeRequired property set for Identity Provider: " + existing.getId()); + logger.info("PasswordChangeRequired property set for Identity Provider: {}", existing.getId()); /* since this operation is only allowed for IdPs of type "UAA" and aliases are not supported for "UAA" IdPs, * we do not need to propagate the changes to an alias IdP here. */ @@ -308,7 +289,7 @@ public ResponseEntity updateIdentityProviderStatus(@Path return new ResponseEntity<>(body, OK); } - @RequestMapping(method = GET) + @GetMapping() public ResponseEntity> retrieveIdentityProviders(@RequestParam(value = "active_only", required = false) String activeOnly, @RequestParam(required = false, defaultValue = "false") boolean rawConfig) { boolean retrieveActiveOnly = Boolean.parseBoolean(activeOnly); List identityProviderList = identityProviderProvisioning.retrieveAll(retrieveActiveOnly, identityZoneManager.getCurrentIdentityZoneId()); @@ -319,7 +300,7 @@ public ResponseEntity> retrieveIdentityProviders(@Request return new ResponseEntity<>(identityProviderList, OK); } - @RequestMapping(value = "{id}", method = GET) + @GetMapping(value = "{id}") public ResponseEntity retrieveIdentityProvider(@PathVariable String id, @RequestParam(required = false, defaultValue = "false") boolean rawConfig) { IdentityProvider identityProvider = identityProviderProvisioning.retrieve(id, identityZoneManager.getCurrentIdentityZoneId()); identityProvider.setSerializeConfigRaw(rawConfig); @@ -327,7 +308,7 @@ public ResponseEntity retrieveIdentityProvider(@PathVariable S return new ResponseEntity<>(identityProvider, OK); } - @RequestMapping(value = "test", method = POST) + @PostMapping(value = "test") public ResponseEntity testIdentityProvider(@RequestBody IdentityProviderValidationRequest body) { String exception = "ok"; HttpStatus status = OK; @@ -406,32 +387,23 @@ protected void patchSensitiveData(String id, IdentityProvider provider) { } switch (provider.getType()) { case LDAP: { - if (provider.getConfig() instanceof LdapIdentityProviderDefinition) { - LdapIdentityProviderDefinition definition = (LdapIdentityProviderDefinition) provider.getConfig(); - if (definition.getBindPassword() == null) { - IdentityProvider existing = identityProviderProvisioning.retrieve(id, zoneId); - if (existing!=null && - existing.getConfig()!=null && - existing.getConfig() instanceof LdapIdentityProviderDefinition) { - LdapIdentityProviderDefinition existingDefinition = (LdapIdentityProviderDefinition)existing.getConfig(); - definition.setBindPassword(existingDefinition.getBindPassword()); - } + if (provider.getConfig() instanceof LdapIdentityProviderDefinition definition && definition.getBindPassword() == null) { + IdentityProvider existing = identityProviderProvisioning.retrieve(id, zoneId); + if (existing!=null && + existing.getConfig()!=null && + existing.getConfig() instanceof LdapIdentityProviderDefinition existingDefinition) { + definition.setBindPassword(existingDefinition.getBindPassword()); } } break; } - case OAUTH20 : - case OIDC10 : { - if (provider.getConfig() instanceof AbstractExternalOAuthIdentityProviderDefinition) { - AbstractExternalOAuthIdentityProviderDefinition definition = (AbstractExternalOAuthIdentityProviderDefinition) provider.getConfig(); - if (definition.getRelyingPartySecret() == null) { - IdentityProvider existing = identityProviderProvisioning.retrieve(id, zoneId); - if (existing!=null && - existing.getConfig()!=null && - existing.getConfig() instanceof AbstractExternalOAuthIdentityProviderDefinition) { - AbstractExternalOAuthIdentityProviderDefinition existingDefinition = (AbstractExternalOAuthIdentityProviderDefinition)existing.getConfig(); - definition.setRelyingPartySecret(existingDefinition.getRelyingPartySecret()); - } + case OAUTH20, OIDC10 : { + if (provider.getConfig() instanceof AbstractExternalOAuthIdentityProviderDefinition definition && definition.getRelyingPartySecret() == null) { + IdentityProvider existing = identityProviderProvisioning.retrieve(id, zoneId); + if (existing!=null && + existing.getConfig()!=null && + existing.getConfig() instanceof AbstractExternalOAuthIdentityProviderDefinition existingDefinition) { + definition.setRelyingPartySecret(existingDefinition.getRelyingPartySecret()); } } break; @@ -448,18 +420,15 @@ protected void redactSensitiveData(IdentityProvider provider) { } switch (provider.getType()) { case LDAP: { - if (provider.getConfig() instanceof LdapIdentityProviderDefinition) { - logger.debug("Removing bind password from LDAP provider id:"+provider.getId()); - LdapIdentityProviderDefinition definition = (LdapIdentityProviderDefinition) provider.getConfig(); + if (provider.getConfig() instanceof LdapIdentityProviderDefinition definition) { + logger.log(Level.DEBUG, () -> String.format("Removing bind password from LDAP provider id: %s", provider.getId())); definition.setBindPassword(null); } break; } - case OAUTH20 : - case OIDC10 : { - if (provider.getConfig() instanceof AbstractExternalOAuthIdentityProviderDefinition) { - logger.debug("Removing relying secret from OAuth/OIDC provider id:"+provider.getId()); - AbstractExternalOAuthIdentityProviderDefinition definition = (AbstractExternalOAuthIdentityProviderDefinition) provider.getConfig(); + case OAUTH20, OIDC10 : { + if (provider.getConfig() instanceof AbstractExternalOAuthIdentityProviderDefinition definition) { + logger.log(Level.DEBUG, () -> String.format("Removing relying secret from OAuth/OIDC provider id: %s", provider.getId())); definition.setRelyingPartySecret(null); } break; From c7774ea751a60b4a261bba5f5865babdab4e9403 Mon Sep 17 00:00:00 2001 From: d036670 Date: Tue, 14 May 2024 14:38:07 +0200 Subject: [PATCH 10/19] remove dead code --- .../provider/IdentityProviderEndpoints.java | 48 ------------------- .../IdentityProviderEndpointDocs.java | 1 - 2 files changed, 49 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java index b726ba098c7..178e50acba2 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java @@ -19,24 +19,19 @@ import static org.cloudfoundry.identity.uaa.constants.OriginKeys.SAML; import static org.cloudfoundry.identity.uaa.constants.OriginKeys.UAA; import static org.cloudfoundry.identity.uaa.util.UaaStringUtils.getCleanedUserControlString; -import static org.springframework.http.HttpStatus.BAD_REQUEST; import static org.springframework.http.HttpStatus.CONFLICT; import static org.springframework.http.HttpStatus.CREATED; -import static org.springframework.http.HttpStatus.EXPECTATION_FAILED; import static org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR; import static org.springframework.http.HttpStatus.OK; import static org.springframework.http.HttpStatus.UNPROCESSABLE_ENTITY; import static org.springframework.util.StringUtils.hasText; -import java.io.PrintWriter; -import java.io.StringWriter; import java.util.Date; import java.util.List; import java.util.Optional; import org.cloudfoundry.identity.uaa.alias.EntityAliasFailedException; import org.cloudfoundry.identity.uaa.audit.event.EntityDeletedEvent; -import org.cloudfoundry.identity.uaa.authentication.manager.DynamicLdapAuthenticationManager; import org.cloudfoundry.identity.uaa.authentication.manager.LdapLoginAuthenticationManager; import org.cloudfoundry.identity.uaa.provider.saml.SamlIdentityProviderConfigurator; import org.cloudfoundry.identity.uaa.scim.ScimGroupExternalMembershipManager; @@ -55,8 +50,6 @@ import org.springframework.dao.EmptyResultDataAccessException; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; -import org.springframework.security.authentication.BadCredentialsException; -import org.springframework.security.authentication.InternalAuthenticationServiceException; import org.springframework.security.core.Authentication; import org.springframework.security.core.AuthenticationException; import org.springframework.security.core.context.SecurityContextHolder; @@ -308,40 +301,6 @@ public ResponseEntity retrieveIdentityProvider(@PathVariable S return new ResponseEntity<>(identityProvider, OK); } - @PostMapping(value = "test") - public ResponseEntity testIdentityProvider(@RequestBody IdentityProviderValidationRequest body) { - String exception = "ok"; - HttpStatus status = OK; - //create the LDAP IDP - DynamicLdapAuthenticationManager manager = new DynamicLdapAuthenticationManager( - ObjectUtils.castInstance(body.getProvider().getConfig(),LdapIdentityProviderDefinition.class), - scimGroupExternalMembershipManager, - scimGroupProvisioning, - noOpManager - ); - try { - //attempt authentication - Authentication result = manager.authenticate(body.getCredentials()); - if ((result == null) || (result != null && !result.isAuthenticated())) { - status = EXPECTATION_FAILED; - } - } catch (BadCredentialsException x) { - status = EXPECTATION_FAILED; - exception = "bad credentials"; - } catch (InternalAuthenticationServiceException x) { - status = BAD_REQUEST; - exception = getExceptionString(x); - } catch (Exception x) { - logger.error("Identity provider validation failed.", x); - status = INTERNAL_SERVER_ERROR; - exception = "check server logs"; - }finally { - //destroy IDP - manager.destroy(); - } - //return results - return new ResponseEntity<>(JsonUtils.writeValueAsString(exception), status); - } @ExceptionHandler(MetadataProviderException.class) public ResponseEntity handleMetadataProviderException(MetadataProviderException e) { @@ -362,13 +321,6 @@ public ResponseEntity handleProviderNotFoundException() { return new ResponseEntity<>("Provider not found.", HttpStatus.NOT_FOUND); } - - protected String getExceptionString(Exception x) { - StringWriter writer = new StringWriter(); - x.printStackTrace(new PrintWriter(writer)); - return writer.getBuffer().toString(); - } - protected static class NoOpLdapLoginAuthenticationManager extends LdapLoginAuthenticationManager { public NoOpLdapLoginAuthenticationManager() { super(null); 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 24a27cd2f39..0139e741443 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 @@ -106,7 +106,6 @@ import org.springframework.restdocs.payload.FieldDescriptor; import org.springframework.restdocs.snippet.Attributes; import org.springframework.restdocs.snippet.Snippet; -import org.cloudfoundry.identity.uaa.client.UaaClientDetails; import org.springframework.test.web.servlet.ResultActions; class IdentityProviderEndpointDocs extends EndpointDocs { From d1e706eb342fb1c10b65ea523caa1aa5d6cd4452 Mon Sep 17 00:00:00 2001 From: d036670 Date: Tue, 14 May 2024 18:10:05 +0200 Subject: [PATCH 11/19] more sonar smell fixes --- .../provider/IdentityProviderEndpoints.java | 28 +------ .../IdentityProviderEndpointsTest.java | 78 +++++++++++++++++++ .../identity/uaa/mock/EndpointDocs.java | 4 + .../IdentityProviderEndpointDocs.java | 7 +- 4 files changed, 87 insertions(+), 30 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java index 178e50acba2..4a1d54e923f 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java @@ -32,10 +32,7 @@ import org.cloudfoundry.identity.uaa.alias.EntityAliasFailedException; import org.cloudfoundry.identity.uaa.audit.event.EntityDeletedEvent; -import org.cloudfoundry.identity.uaa.authentication.manager.LdapLoginAuthenticationManager; import org.cloudfoundry.identity.uaa.provider.saml.SamlIdentityProviderConfigurator; -import org.cloudfoundry.identity.uaa.scim.ScimGroupExternalMembershipManager; -import org.cloudfoundry.identity.uaa.scim.ScimGroupProvisioning; import org.cloudfoundry.identity.uaa.util.JsonUtils; import org.cloudfoundry.identity.uaa.util.ObjectUtils; import org.cloudfoundry.identity.uaa.zone.beans.IdentityZoneManager; @@ -51,7 +48,6 @@ import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.security.core.Authentication; -import org.springframework.security.core.AuthenticationException; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.transaction.PlatformTransactionManager; import org.springframework.transaction.annotation.Transactional; @@ -77,9 +73,6 @@ public class IdentityProviderEndpoints implements ApplicationEventPublisherAware @Value("${login.aliasEntitiesEnabled:false}") private boolean aliasEntitiesEnabled; private final IdentityProviderProvisioning identityProviderProvisioning; - private final ScimGroupExternalMembershipManager scimGroupExternalMembershipManager; - private final ScimGroupProvisioning scimGroupProvisioning; - private final NoOpLdapLoginAuthenticationManager noOpManager = new NoOpLdapLoginAuthenticationManager(); private final SamlIdentityProviderConfigurator samlConfigurator; private final IdentityProviderConfigValidator configValidator; private final IdentityZoneManager identityZoneManager; @@ -95,8 +88,6 @@ public void setApplicationEventPublisher(ApplicationEventPublisher applicationEv public IdentityProviderEndpoints( final @Qualifier("identityProviderProvisioning") IdentityProviderProvisioning identityProviderProvisioning, - final @Qualifier("externalGroupMembershipManager") ScimGroupExternalMembershipManager scimGroupExternalMembershipManager, - final @Qualifier("scimGroupProvisioning") ScimGroupProvisioning scimGroupProvisioning, final @Qualifier("metaDataProviders") SamlIdentityProviderConfigurator samlConfigurator, final @Qualifier("identityProviderConfigValidator") IdentityProviderConfigValidator configValidator, final IdentityZoneManager identityZoneManager, @@ -104,8 +95,6 @@ public IdentityProviderEndpoints( final IdentityProviderAliasHandler idpAliasHandler ) { this.identityProviderProvisioning = identityProviderProvisioning; - this.scimGroupExternalMembershipManager = scimGroupExternalMembershipManager; - this.scimGroupProvisioning = scimGroupProvisioning; this.samlConfigurator = samlConfigurator; this.configValidator = configValidator; this.identityZoneManager = identityZoneManager; @@ -121,8 +110,7 @@ public ResponseEntity createIdentityProvider(@RequestBody Iden try { configValidator.validate(body); } catch (IllegalArgumentException e) { - logger.log(Level.DEBUG, () -> String.format("IdentityProvider[origin=%s; zone=%s] - Configuration validation error.", - body.getOriginKey(), body.getIdentityZoneId()), e); + logger.log(Level.DEBUG, () -> String.format("IdentityProvider[origin=%s; zone=%s] - Configuration validation error.", body.getOriginKey(), body.getIdentityZoneId()), e); return new ResponseEntity<>(body, UNPROCESSABLE_ENTITY); } if (SAML.equals(body.getType())) { @@ -196,8 +184,7 @@ public ResponseEntity updateIdentityProvider(@PathVariable Str try { configValidator.validate(body); } catch (IllegalArgumentException e) { - logger.log(Level.DEBUG, () -> String.format("IdentityProvider[origin=%s; zone=%s] - Configuration validation error for update.", - body.getOriginKey(), body.getIdentityZoneId()), e); + logger.log(Level.DEBUG, () -> String.format("IdentityProvider[origin=%s; zone=%s] - Configuration validation error for update.", body.getOriginKey(), body.getIdentityZoneId()), e); return new ResponseEntity<>(body, UNPROCESSABLE_ENTITY); } @@ -321,17 +308,6 @@ public ResponseEntity handleProviderNotFoundException() { return new ResponseEntity<>("Provider not found.", HttpStatus.NOT_FOUND); } - protected static class NoOpLdapLoginAuthenticationManager extends LdapLoginAuthenticationManager { - public NoOpLdapLoginAuthenticationManager() { - super(null); - } - - @Override - public Authentication authenticate(Authentication request) throws AuthenticationException { - return request; - } - } - protected void patchSensitiveData(String id, IdentityProvider provider) { String zoneId = identityZoneManager.getCurrentIdentityZoneId(); if (provider.getConfig() == null) { diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpointsTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpointsTest.java index 776067616e7..56525061697 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpointsTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpointsTest.java @@ -4,6 +4,7 @@ import static org.cloudfoundry.identity.uaa.constants.OriginKeys.LDAP; import static org.cloudfoundry.identity.uaa.constants.OriginKeys.OAUTH20; import static org.cloudfoundry.identity.uaa.constants.OriginKeys.OIDC10; +import static org.cloudfoundry.identity.uaa.constants.OriginKeys.SAML; import static org.cloudfoundry.identity.uaa.constants.OriginKeys.UAA; import static org.cloudfoundry.identity.uaa.constants.OriginKeys.UNKNOWN; import static org.cloudfoundry.identity.uaa.provider.ExternalIdentityProviderDefinition.USER_NAME_ATTRIBUTE_NAME; @@ -17,6 +18,7 @@ import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -24,6 +26,7 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.springframework.http.HttpStatus.UNPROCESSABLE_ENTITY; import java.net.MalformedURLException; import java.net.URL; @@ -42,6 +45,7 @@ import org.cloudfoundry.identity.uaa.audit.event.EntityDeletedEvent; import org.cloudfoundry.identity.uaa.constants.OriginKeys; import org.cloudfoundry.identity.uaa.extensions.PollutionPreventionExtension; +import org.cloudfoundry.identity.uaa.provider.saml.SamlIdentityProviderConfigurator; import org.cloudfoundry.identity.uaa.zone.IdentityZone; import org.cloudfoundry.identity.uaa.zone.IdentityZoneProvisioning; import org.cloudfoundry.identity.uaa.zone.beans.IdentityZoneManager; @@ -86,6 +90,9 @@ class IdentityProviderEndpointsTest { @Mock private IdentityProviderAliasHandler mockIdpAliasHandler; + @Mock + SamlIdentityProviderConfigurator samlConfigurator; + @InjectMocks private IdentityProviderEndpoints identityProviderEndpoints; @@ -356,6 +363,77 @@ void update_ldap_provider_takes_new_password() throws Exception { assertNull(((LdapIdentityProviderDefinition) response.getBody().getConfig()).getBindPassword()); } + @Test + void update_saml_provider_validator_failed() throws Exception { + IdentityProvider provider = new IdentityProvider<>(); + String zoneId = IdentityZone.getUaaZoneId(); + provider.setId("id"); + provider.setType(SAML); + provider.setIdentityZoneId(zoneId); + provider.setOriginKey("originKey"); + SamlIdentityProviderDefinition samlConfig = new SamlIdentityProviderDefinition(); + provider.setConfig(samlConfig); + doThrow(new IllegalArgumentException("error")).when(mockIdentityProviderConfigValidationDelegator).validate(any()); + when(mockIdentityProviderProvisioning.retrieve(any(), eq(zoneId))).thenReturn(provider); + ResponseEntity response = identityProviderEndpoints.updateIdentityProvider(provider.getId(), provider, true); + assertNotNull(response); + assertEquals(UNPROCESSABLE_ENTITY, response.getStatusCode()); + verify(mockPlatformTransactionManager, never()).getTransaction(any()); + verify(mockIdpAliasHandler, never()).ensureConsistencyOfAliasEntity(any(), any()); + } + + @Test + void update_saml_provider_alias_failed() throws Exception { + IdentityProvider provider = new IdentityProvider<>(); + String zoneId = IdentityZone.getUaaZoneId(); + provider.setId("id"); + provider.setType(SAML); + provider.setIdentityZoneId(zoneId); + provider.setOriginKey("originKey"); + SamlIdentityProviderDefinition samlConfig = new SamlIdentityProviderDefinition(); + provider.setConfig(samlConfig); + when(mockIdentityProviderProvisioning.retrieve(any(), eq(zoneId))).thenReturn(provider); + ResponseEntity response = identityProviderEndpoints.updateIdentityProvider(provider.getId(), provider, true); + assertNotNull(response); + assertEquals(UNPROCESSABLE_ENTITY, response.getStatusCode()); + verify(mockPlatformTransactionManager).getTransaction(any()); + verify(mockIdpAliasHandler, times(1)).ensureConsistencyOfAliasEntity(any(), any()); + } + + @Test + void create_saml_provider_validator_failed() throws Exception { + IdentityProvider provider = new IdentityProvider<>(); + String zoneId = IdentityZone.getUaaZoneId(); + provider.setId("id"); + provider.setType(SAML); + provider.setIdentityZoneId(zoneId); + provider.setOriginKey("originKey"); + SamlIdentityProviderDefinition samlConfig = new SamlIdentityProviderDefinition(); + provider.setConfig(samlConfig); + doThrow(new IllegalArgumentException("error")).when(mockIdentityProviderConfigValidationDelegator).validate(any()); + ResponseEntity response = identityProviderEndpoints.createIdentityProvider(provider, true); + assertNotNull(response); + assertEquals(UNPROCESSABLE_ENTITY, response.getStatusCode()); + verify(mockIdpAliasHandler, never()).aliasPropertiesAreValid(provider, null); + } + + @Test + void create_saml_provider_alias_failed() throws Exception { + IdentityProvider provider = new IdentityProvider<>(); + String zoneId = IdentityZone.getUaaZoneId(); + provider.setId("id"); + provider.setType(SAML); + provider.setIdentityZoneId(zoneId); + provider.setOriginKey("originKey"); + SamlIdentityProviderDefinition samlConfig = new SamlIdentityProviderDefinition(); + provider.setConfig(samlConfig); + ResponseEntity response = identityProviderEndpoints.createIdentityProvider(provider, true); + assertNotNull(response); + assertEquals(UNPROCESSABLE_ENTITY, response.getStatusCode()); + verify(mockPlatformTransactionManager).getTransaction(any()); + verify(mockIdpAliasHandler, times(1)).ensureConsistencyOfAliasEntity(any(), any()); + } + @Test void create_ldap_provider_removes_password() throws Exception { String zoneId = IdentityZone.getUaaZoneId(); diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/EndpointDocs.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/EndpointDocs.java index f29e9f1a5fb..7500037ab8e 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/EndpointDocs.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/EndpointDocs.java @@ -3,6 +3,7 @@ import org.cloudfoundry.identity.uaa.DefaultTestContext; import org.cloudfoundry.identity.uaa.test.JUnitRestDocumentationExtension; import org.cloudfoundry.identity.uaa.test.TestClient; +import org.cloudfoundry.identity.uaa.zone.beans.IdentityZoneManager; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.extension.ExtendWith; import org.springframework.beans.factory.annotation.Autowired; @@ -22,6 +23,9 @@ public class EndpointDocs { @Autowired protected WebApplicationContext webApplicationContext; + @Autowired + protected IdentityZoneManager identityZoneManager; + protected MockMvc mockMvc; protected TestClient testClient; 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 0139e741443..c317d43f973 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 @@ -93,7 +93,6 @@ import org.cloudfoundry.identity.uaa.util.AlphanumericRandomValueStringGenerator; import org.cloudfoundry.identity.uaa.util.JsonUtils; import org.cloudfoundry.identity.uaa.zone.IdentityZone; -import org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder; import org.cloudfoundry.identity.uaa.zone.IdentityZoneSwitchingFilter; import org.cloudfoundry.identity.uaa.zone.MultitenancyFixture; import org.junit.jupiter.api.AfterAll; @@ -869,7 +868,7 @@ void createLDAPProvider(IdentityProvider identit MockMvcUtils.createOtherIdentityZoneAndReturnResult(new AlphanumericRandomValueStringGenerator(8).generate().toLowerCase(), mockMvc, webApplicationContext, - admin, IdentityZoneHolder.getCurrentZoneId()); + admin, identityZoneManager.getCurrentIdentityZoneId()); Snippet requestFields = requestFields(fields); @@ -990,7 +989,7 @@ void getIdentityProvider() throws Exception { @Test void updateIdentityProvider() throws Exception { - IdentityProvider identityProvider = identityProviderProvisioning.retrieveByOrigin(OriginKeys.UAA, IdentityZoneHolder.get().getId()); + IdentityProvider identityProvider = identityProviderProvisioning.retrieveByOrigin(OriginKeys.UAA, identityZoneManager.getCurrentIdentityZoneId()); UaaIdentityProviderDefinition config = new UaaIdentityProviderDefinition(); config.setLockoutPolicy(new LockoutPolicy(8, 8, 8)); @@ -1057,7 +1056,7 @@ void updateIdentityProvider() throws Exception { @Test void patchIdentityProviderStatus() throws Exception { - IdentityProvider identityProvider = identityProviderProvisioning.retrieveByOrigin(OriginKeys.UAA, IdentityZoneHolder.get().getId()); + IdentityProvider identityProvider = identityProviderProvisioning.retrieveByOrigin(OriginKeys.UAA, identityZoneManager.getCurrentIdentityZoneId()); identityProvider.setConfig(new UaaIdentityProviderDefinition(new PasswordPolicy(0, 20, 0, 0, 0, 0, 0), null)); identityProviderProvisioning.update(identityProvider, identityProvider.getIdentityZoneId()); IdentityProviderStatus identityProviderStatus = new IdentityProviderStatus(); From 672b8d33802cd6e2441dfd6d068cad5b51b4db18 Mon Sep 17 00:00:00 2001 From: d036670 Date: Tue, 14 May 2024 18:42:43 +0200 Subject: [PATCH 12/19] no log change --- .../provider/IdentityProviderEndpoints.java | 38 +++++++++---------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java index 4a1d54e923f..282c28a4b22 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java @@ -37,9 +37,8 @@ import org.cloudfoundry.identity.uaa.util.ObjectUtils; import org.cloudfoundry.identity.uaa.zone.beans.IdentityZoneManager; import org.opensaml.saml2.metadata.provider.MetadataProviderException; -import org.apache.logging.log4j.Level; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.beans.factory.annotation.Value; import org.springframework.context.ApplicationEventPublisher; @@ -68,7 +67,7 @@ @RestController public class IdentityProviderEndpoints implements ApplicationEventPublisherAware { - protected static Logger logger = LogManager.getLogger(); + protected static Logger logger = LoggerFactory.getLogger(IdentityProviderEndpoints.class); @Value("${login.aliasEntitiesEnabled:false}") private boolean aliasEntitiesEnabled; @@ -110,7 +109,7 @@ public ResponseEntity createIdentityProvider(@RequestBody Iden try { configValidator.validate(body); } catch (IllegalArgumentException e) { - logger.log(Level.DEBUG, () -> String.format("IdentityProvider[origin=%s; zone=%s] - Configuration validation error.", body.getOriginKey(), body.getIdentityZoneId()), e); + logger.debug(String.format("IdentityProvider[origin=%s; zone=%s] - Configuration validation error.", body.getOriginKey(), body.getIdentityZoneId()), e); return new ResponseEntity<>(body, UNPROCESSABLE_ENTITY); } if (SAML.equals(body.getType())) { @@ -184,16 +183,15 @@ public ResponseEntity updateIdentityProvider(@PathVariable Str try { configValidator.validate(body); } catch (IllegalArgumentException e) { - logger.log(Level.DEBUG, () -> String.format("IdentityProvider[origin=%s; zone=%s] - Configuration validation error for update.", body.getOriginKey(), body.getIdentityZoneId()), e); + logger.debug(String.format("IdentityProvider[origin=%s; zone=%s] - Configuration validation error for update.", body.getOriginKey(), body.getIdentityZoneId()), e); return new ResponseEntity<>(body, UNPROCESSABLE_ENTITY); } if (!idpAliasHandler.aliasPropertiesAreValid(body, existing)) { - logger.log(Level.WARN, () -> String.format( - "IdentityProvider[origin=%s; zone=%s] - Alias ID and/or ZID changed during update of IdP with alias.", - getCleanedUserControlString(body.getOriginKey()), - getCleanedUserControlString(body.getIdentityZoneId()) - )); + if (logger.isWarnEnabled()) { + logger.warn("IdentityProvider[origin={}; zone={}] - Alias ID and/or ZID changed during update of IdP with alias.", + getCleanedUserControlString(body.getOriginKey()), getCleanedUserControlString(body.getIdentityZoneId())); + } return new ResponseEntity<>(body, UNPROCESSABLE_ENTITY); } @@ -220,19 +218,21 @@ private ResponseEntity persistIdentityProviderChange(IdentityP } catch (final IdpAlreadyExistsException e) { return new ResponseEntity<>(body, CONFLICT); } catch (final EntityAliasFailedException e) { - logger.log(Level.WARN, () -> String.format("Could not create alias for %s", e.getMessage())); + logger.warn(String.format("Could not create alias for %s", e.getMessage())); final HttpStatus responseCode = Optional.ofNullable(HttpStatus.resolve(e.getHttpStatus())).orElse(INTERNAL_SERVER_ERROR); return new ResponseEntity<>(body, responseCode); } catch (final Exception e) { - logger.log(Level.WARN, () -> String.format("Unable to %s IdentityProvider[origin=%s; zone=%s]", + logger.warn(String.format("Unable to %s IdentityProvider[origin=%s; zone=%s]", status == CREATED ? "create" : "update", body.getOriginKey(), body.getIdentityZoneId()), e); return new ResponseEntity<>(body, INTERNAL_SERVER_ERROR); } if (updatedIdp == null) { - logger.log(Level.WARN, () -> String.format("IdentityProvider[origin=%s; zone=%s] - Transaction %s IdP (and alias IdP, if applicable) was not successful, but no exception was thrown.", - getCleanedUserControlString(body.getOriginKey()), - getCleanedUserControlString(body.getIdentityZoneId()), - status == CREATED ? "creating" : "updating")); + if (logger.isWarnEnabled()) { + logger.warn( + "IdentityProvider[origin={}; zone={}] - Transaction {} IdP (and alias IdP, if applicable) was not successful, but no exception was thrown.", + getCleanedUserControlString(body.getOriginKey()), getCleanedUserControlString(body.getIdentityZoneId()), + status == CREATED ? "creating" : "updating"); + } return new ResponseEntity<>(body, UNPROCESSABLE_ENTITY); } updatedIdp.setSerializeConfigRaw(rawConfig); @@ -349,14 +349,14 @@ protected void redactSensitiveData(IdentityProvider provider) { switch (provider.getType()) { case LDAP: { if (provider.getConfig() instanceof LdapIdentityProviderDefinition definition) { - logger.log(Level.DEBUG, () -> String.format("Removing bind password from LDAP provider id: %s", provider.getId())); + logger.debug("Removing bind password from LDAP provider id: {}", provider.getId()); definition.setBindPassword(null); } break; } case OAUTH20, OIDC10 : { if (provider.getConfig() instanceof AbstractExternalOAuthIdentityProviderDefinition definition) { - logger.log(Level.DEBUG, () -> String.format("Removing relying secret from OAuth/OIDC provider id: %s", provider.getId())); + logger.debug("Removing relying secret from OAuth/OIDC provider id: {}", provider.getId()); definition.setRelyingPartySecret(null); } break; From 771bc91bc72c302984f08993de5c7ae24fcb50c1 Mon Sep 17 00:00:00 2001 From: d036670 Date: Tue, 14 May 2024 19:46:07 +0200 Subject: [PATCH 13/19] revert and allow test endpoint again --- .../provider/IdentityProviderEndpoints.java | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java index 282c28a4b22..a5840a78b54 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java @@ -19,20 +19,29 @@ import static org.cloudfoundry.identity.uaa.constants.OriginKeys.SAML; import static org.cloudfoundry.identity.uaa.constants.OriginKeys.UAA; import static org.cloudfoundry.identity.uaa.util.UaaStringUtils.getCleanedUserControlString; +import static org.springframework.http.HttpStatus.BAD_REQUEST; import static org.springframework.http.HttpStatus.CONFLICT; import static org.springframework.http.HttpStatus.CREATED; +import static org.springframework.http.HttpStatus.EXPECTATION_FAILED; import static org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR; import static org.springframework.http.HttpStatus.OK; import static org.springframework.http.HttpStatus.UNPROCESSABLE_ENTITY; import static org.springframework.util.StringUtils.hasText; +import static org.springframework.web.bind.annotation.RequestMethod.POST; +import java.io.PrintWriter; +import java.io.StringWriter; import java.util.Date; import java.util.List; import java.util.Optional; import org.cloudfoundry.identity.uaa.alias.EntityAliasFailedException; import org.cloudfoundry.identity.uaa.audit.event.EntityDeletedEvent; +import org.cloudfoundry.identity.uaa.authentication.manager.DynamicLdapAuthenticationManager; +import org.cloudfoundry.identity.uaa.authentication.manager.LdapLoginAuthenticationManager; import org.cloudfoundry.identity.uaa.provider.saml.SamlIdentityProviderConfigurator; +import org.cloudfoundry.identity.uaa.scim.ScimGroupExternalMembershipManager; +import org.cloudfoundry.identity.uaa.scim.ScimGroupProvisioning; import org.cloudfoundry.identity.uaa.util.JsonUtils; import org.cloudfoundry.identity.uaa.util.ObjectUtils; import org.cloudfoundry.identity.uaa.zone.beans.IdentityZoneManager; @@ -46,7 +55,10 @@ import org.springframework.dao.EmptyResultDataAccessException; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; +import org.springframework.security.authentication.BadCredentialsException; +import org.springframework.security.authentication.InternalAuthenticationServiceException; import org.springframework.security.core.Authentication; +import org.springframework.security.core.AuthenticationException; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.transaction.PlatformTransactionManager; import org.springframework.transaction.annotation.Transactional; @@ -72,6 +84,9 @@ public class IdentityProviderEndpoints implements ApplicationEventPublisherAware @Value("${login.aliasEntitiesEnabled:false}") private boolean aliasEntitiesEnabled; private final IdentityProviderProvisioning identityProviderProvisioning; + private final ScimGroupExternalMembershipManager scimGroupExternalMembershipManager; + private final ScimGroupProvisioning scimGroupProvisioning; + private final NoOpLdapLoginAuthenticationManager noOpManager = new NoOpLdapLoginAuthenticationManager(); private final SamlIdentityProviderConfigurator samlConfigurator; private final IdentityProviderConfigValidator configValidator; private final IdentityZoneManager identityZoneManager; @@ -87,6 +102,8 @@ public void setApplicationEventPublisher(ApplicationEventPublisher applicationEv public IdentityProviderEndpoints( final @Qualifier("identityProviderProvisioning") IdentityProviderProvisioning identityProviderProvisioning, + final @Qualifier("externalGroupMembershipManager") ScimGroupExternalMembershipManager scimGroupExternalMembershipManager, + final @Qualifier("scimGroupProvisioning") ScimGroupProvisioning scimGroupProvisioning, final @Qualifier("metaDataProviders") SamlIdentityProviderConfigurator samlConfigurator, final @Qualifier("identityProviderConfigValidator") IdentityProviderConfigValidator configValidator, final IdentityZoneManager identityZoneManager, @@ -94,6 +111,8 @@ public IdentityProviderEndpoints( final IdentityProviderAliasHandler idpAliasHandler ) { this.identityProviderProvisioning = identityProviderProvisioning; + this.scimGroupExternalMembershipManager = scimGroupExternalMembershipManager; + this.scimGroupProvisioning = scimGroupProvisioning; this.samlConfigurator = samlConfigurator; this.configValidator = configValidator; this.identityZoneManager = identityZoneManager; @@ -288,6 +307,40 @@ public ResponseEntity retrieveIdentityProvider(@PathVariable S return new ResponseEntity<>(identityProvider, OK); } + @RequestMapping(value = "test", method = POST) + public ResponseEntity testIdentityProvider(@RequestBody IdentityProviderValidationRequest body) { + String exception = "ok"; + HttpStatus status = OK; + //create the LDAP IDP + DynamicLdapAuthenticationManager manager = new DynamicLdapAuthenticationManager( + ObjectUtils.castInstance(body.getProvider().getConfig(),LdapIdentityProviderDefinition.class), + scimGroupExternalMembershipManager, + scimGroupProvisioning, + noOpManager + ); + try { + //attempt authentication + Authentication result = manager.authenticate(body.getCredentials()); + if ((result == null) || (result != null && !result.isAuthenticated())) { + status = EXPECTATION_FAILED; + } + } catch (BadCredentialsException x) { + status = EXPECTATION_FAILED; + exception = "bad credentials"; + } catch (InternalAuthenticationServiceException x) { + status = BAD_REQUEST; + exception = getExceptionString(x); + } catch (Exception x) { + logger.error("Identity provider validation failed.", x); + status = INTERNAL_SERVER_ERROR; + exception = "check server logs"; + }finally { + //destroy IDP + manager.destroy(); + } + //return results + return new ResponseEntity<>(JsonUtils.writeValueAsString(exception), status); + } @ExceptionHandler(MetadataProviderException.class) public ResponseEntity handleMetadataProviderException(MetadataProviderException e) { @@ -308,6 +361,24 @@ public ResponseEntity handleProviderNotFoundException() { return new ResponseEntity<>("Provider not found.", HttpStatus.NOT_FOUND); } + + protected String getExceptionString(Exception x) { + StringWriter writer = new StringWriter(); + x.printStackTrace(new PrintWriter(writer)); + return writer.getBuffer().toString(); + } + + protected static class NoOpLdapLoginAuthenticationManager extends LdapLoginAuthenticationManager { + public NoOpLdapLoginAuthenticationManager() { + super(null); + } + + @Override + public Authentication authenticate(Authentication request) throws AuthenticationException { + return request; + } + } + protected void patchSensitiveData(String id, IdentityProvider provider) { String zoneId = identityZoneManager.getCurrentIdentityZoneId(); if (provider.getConfig() == null) { From 2e2f04d89c555f1adea8f614aa8c4b4d77b4db0a Mon Sep 17 00:00:00 2001 From: d036670 Date: Tue, 14 May 2024 20:13:09 +0200 Subject: [PATCH 14/19] warnings --- .../uaa/provider/IdentityProviderEndpointsTest.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpointsTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpointsTest.java index 56525061697..b7a8bfaeb4d 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpointsTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpointsTest.java @@ -168,7 +168,7 @@ IdentityProvider getLdapDefinition() { } @Test - void retrieve_oauth_provider_by_id_redacts_password() throws Exception { + void retrieve_oauth_provider_by_id_redacts_password() { retrieve_oauth_provider_by_id("", OriginKeys.OAUTH20); retrieve_oauth_provider_by_id("", OriginKeys.OIDC10); } @@ -188,7 +188,7 @@ IdentityProvider retrieve_oauth_provider_by_id(S } @Test - void retrieve_ldap_provider_by_id_redacts_password() throws Exception { + void retrieve_ldap_provider_by_id_redacts_password() { retrieve_ldap_provider_by_id(""); } @@ -208,7 +208,7 @@ IdentityProvider retrieve_ldap_provider_by_id(St void remove_bind_password() { remove_sensitive_data(() -> getLdapDefinition(), LDAP, - (spy) -> verify((LdapIdentityProviderDefinition) spy, times(1)).setBindPassword(isNull())); + spy -> verify((LdapIdentityProviderDefinition) spy, times(1)).setBindPassword(isNull())); } @Test @@ -216,7 +216,7 @@ void remove_client_secret() { for (String type : Arrays.asList(OIDC10, OAUTH20)) { remove_sensitive_data(() -> getExternalOAuthProvider(), type, - (spy) -> verify((AbstractExternalOAuthIdentityProviderDefinition) spy, times(1)).setRelyingPartySecret(isNull())); + spy -> verify((AbstractExternalOAuthIdentityProviderDefinition) spy, times(1)).setRelyingPartySecret(isNull())); } } @@ -716,7 +716,6 @@ void testDeleteIdpWithAlias() { void testDeleteIdpWithAlias_DanglingReference() { final String idpId = UUID.randomUUID().toString(); final String aliasIdpId = UUID.randomUUID().toString(); - final String customZoneId = UUID.randomUUID().toString(); final IdentityProvider idp = new IdentityProvider<>(); idp.setType(OIDC10); @@ -753,7 +752,6 @@ void testDeleteIdpWithAlias_AliasFeatureDisabled() { identityProviderEndpoints.setApplicationEventPublisher(mockEventPublisher); // arrange IdP with alias exists - final String customZoneId = UUID.randomUUID().toString(); final Pair, IdentityProvider> idpAndAlias = arrangeIdpWithAliasExists(UAA, customZoneId); final IdentityProvider idp = idpAndAlias.getLeft(); From 441c13dbb47164fb537712f27f595df0f42b283a Mon Sep 17 00:00:00 2001 From: d036670 Date: Thu, 23 May 2024 20:12:38 +0200 Subject: [PATCH 15/19] rebase --- .../uaa/provider/IdentityProviderEndpoints.java | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java index ef717c3d806..5f012aaf4a0 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java @@ -170,6 +170,7 @@ public ResponseEntity deleteIdentityProvider(@PathVariable Str // delete the IdP existing.setSerializeConfigRaw(rawConfig); publisher.publishEvent(new EntityDeletedEvent<>(existing, authentication, identityZoneId)); + setAuthMethod(existing); redactSensitiveData(existing); // delete the alias IdP if present @@ -296,6 +297,7 @@ public ResponseEntity> retrieveIdentityProviders(@Request List identityProviderList = identityProviderProvisioning.retrieveAll(retrieveActiveOnly, identityZoneManager.getCurrentIdentityZoneId()); for(IdentityProvider idp : identityProviderList) { idp.setSerializeConfigRaw(rawConfig); + setAuthMethod(idp); redactSensitiveData(idp); } return new ResponseEntity<>(identityProviderList, OK); @@ -305,6 +307,7 @@ public ResponseEntity> retrieveIdentityProviders(@Request public ResponseEntity retrieveIdentityProvider(@PathVariable String id, @RequestParam(required = false, defaultValue = "false") boolean rawConfig) { IdentityProvider identityProvider = identityProviderProvisioning.retrieve(id, identityZoneManager.getCurrentIdentityZoneId()); identityProvider.setSerializeConfigRaw(rawConfig); + setAuthMethod(identityProvider); redactSensitiveData(identityProvider); return new ResponseEntity<>(identityProvider, OK); } @@ -399,13 +402,13 @@ protected void patchSensitiveData(String id, IdentityProvider provider) { break; } case OAUTH20, OIDC10 : { - if (provider.getConfig() instanceof AbstractExternalOAuthIdentityProviderDefinition definition && definition.getRelyingPartySecret() == null) { + if (provider.getConfig() instanceof AbstractExternalOAuthIdentityProviderDefinition definition && + definition.getRelyingPartySecret() == null && + secretNeeded(definition)) { IdentityProvider existing = identityProviderProvisioning.retrieve(id, zoneId); if (existing!=null && existing.getConfig()!=null && - existing.getConfig() instanceof AbstractExternalOAuthIdentityProviderDefinition existingDefinition) { - existing.getConfig() instanceof AbstractExternalOAuthIdentityProviderDefinition existingDefinition && - secretNeeded(definition)) { + existing.getConfig() instanceof AbstractExternalOAuthIdentityProviderDefinition existingDefinition) { definition.setRelyingPartySecret(existingDefinition.getRelyingPartySecret()); } } @@ -450,4 +453,10 @@ protected boolean secretNeeded(AbstractExternalOAuthIdentityProviderDefinition a return needSecret; } + protected void setAuthMethod(IdentityProvider provider) { + if (provider.getConfig() instanceof AbstractExternalOAuthIdentityProviderDefinition definition) { + definition.setAuthMethod(ExternalOAuthIdentityProviderConfigValidator.getAuthMethod(definition)); + } + } + } From d7a1c357e790e9260544f30bd72c15a7748d313a Mon Sep 17 00:00:00 2001 From: d036670 Date: Fri, 24 May 2024 14:20:06 +0200 Subject: [PATCH 16/19] rebase --- .../identity/uaa/provider/IdentityProviderEndpoints.java | 1 + 1 file changed, 1 insertion(+) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java index 5f012aaf4a0..ec127fcee47 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java @@ -258,6 +258,7 @@ private ResponseEntity persistIdentityProviderChange(IdentityP return new ResponseEntity<>(body, UNPROCESSABLE_ENTITY); } updatedIdp.setSerializeConfigRaw(rawConfig); + setAuthMethod(updatedIdp); redactSensitiveData(updatedIdp); return new ResponseEntity<>(updatedIdp, status); From ae4a539927c39a155303b1535c1846da41c43393 Mon Sep 17 00:00:00 2001 From: d036670 Date: Fri, 24 May 2024 14:20:20 +0200 Subject: [PATCH 17/19] test setup --- .../providers/IdentityProviderEndpointsAliasMockMvcTests.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/providers/IdentityProviderEndpointsAliasMockMvcTests.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/providers/IdentityProviderEndpointsAliasMockMvcTests.java index 31477a1077c..884da26abea 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/providers/IdentityProviderEndpointsAliasMockMvcTests.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/providers/IdentityProviderEndpointsAliasMockMvcTests.java @@ -25,6 +25,7 @@ import java.util.stream.Stream; import org.cloudfoundry.identity.uaa.DefaultTestContext; +import org.cloudfoundry.identity.uaa.constants.ClientAuthentication; import org.cloudfoundry.identity.uaa.mock.util.MockMvcUtils; import org.cloudfoundry.identity.uaa.oauth.token.Claims; import org.cloudfoundry.identity.uaa.oauth.token.TokenConstants; @@ -1434,6 +1435,7 @@ private static AbstractIdentityProviderDefinition buildIdpDefinition(final Strin switch (type) { case OIDC10: final OIDCIdentityProviderDefinition definition = new OIDCIdentityProviderDefinition(); + definition.setAuthMethod(ClientAuthentication.CLIENT_SECRET_BASIC); try { return definition .setAuthUrl(new URL("https://www.example.com/oauth/authorize")) From 73f0c831bc9fe6667fb7d87dc66e8dc39ae78394 Mon Sep 17 00:00:00 2001 From: d036670 Date: Fri, 24 May 2024 16:43:45 +0200 Subject: [PATCH 18/19] rebase --- .../IdentityProviderSecretChange.java | 17 ----- .../provider/IdentityProviderEndpoints.java | 2 - .../IdentityProviderEndpointsTest.java | 73 ------------------ .../source/index.html.md.erb | 76 ------------------- .../IdentityProviderEndpointDocs.java | 55 +------------- 5 files changed, 1 insertion(+), 222 deletions(-) delete mode 100644 model/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderSecretChange.java diff --git a/model/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderSecretChange.java b/model/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderSecretChange.java deleted file mode 100644 index 74d8cb6a64b..00000000000 --- a/model/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderSecretChange.java +++ /dev/null @@ -1,17 +0,0 @@ -package org.cloudfoundry.identity.uaa.provider; - -import com.fasterxml.jackson.annotation.JsonInclude; - -@JsonInclude(JsonInclude.Include.NON_NULL) -public class IdentityProviderSecretChange { - - private String secret; - - public String getSecret() { - return this.secret; - } - - public void setSecret(final String secret) { - this.secret = secret; - } -} diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java index b3d3fcb1549..8fdafe406c8 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java @@ -70,7 +70,6 @@ import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.PatchMapping; -import org.springframework.web.bind.annotation.PatchMapping; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.PostMapping; import org.springframework.web.bind.annotation.PutMapping; @@ -411,7 +410,6 @@ protected void patchSensitiveData(String id, IdentityProvider provider) { definition.setBindPassword(existingDefinition.getBindPassword()); } } - } break; } case OAUTH20, OIDC10 : { diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpointsTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpointsTest.java index d77813d8fc1..cd70f259025 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpointsTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpointsTest.java @@ -329,79 +329,6 @@ void retrieve_by_origin_providers_redacts_data() { assertEquals(ClientAuthentication.CLIENT_SECRET_BASIC, oidc.getConfig().getAuthMethod()); } - @Test - void delete_secret_and_retrieve_by_origin_providers_redacts_data() { - IdentityProvider idp = getExternalOAuthProvider(); - when(mockIdpAliasHandler.ensureConsistencyOfAliasEntity(null, idp)).thenReturn(idp); - when(mockIdentityProviderProvisioning.retrieve("puppyId", "uaa")).thenReturn(idp); - ResponseEntity oidcBody = identityProviderEndpoints.deleteSecret("puppyId"); - IdentityProvider oidc = oidcBody.getBody(); - assertNotNull(oidc); - assertNotNull(oidc.getConfig()); - assertTrue(oidc.getConfig() instanceof AbstractExternalOAuthIdentityProviderDefinition); - assertNull(((AbstractExternalOAuthIdentityProviderDefinition) oidc.getConfig()).getRelyingPartySecret()); - assertEquals(ClientAuthentication.NONE, ((AbstractExternalOAuthIdentityProviderDefinition) oidc.getConfig()).getAuthMethod()); - } - - @Test - void delete_secret_on_ldap_fails() { - when(mockIdentityProviderProvisioning.retrieve("puppyId", "uaa")).thenReturn(getLdapDefinition()); - ResponseEntity oidcBody = identityProviderEndpoints.deleteSecret("puppyId"); - IdentityProvider oidc = oidcBody.getBody(); - assertEquals(UNPROCESSABLE_ENTITY, oidcBody.getStatusCode()); - assertNull(oidc); - } - - @Test - void change_bindPassword_and_retrieve_by_origin_providers_redacts_data() { - IdentityProvider idp = getLdapDefinition(); - when(mockIdentityProviderProvisioning.retrieve("puppyId", "uaa")).thenReturn(idp); - IdentityProviderSecretChange identityProviderSecretChange = new IdentityProviderSecretChange(); - identityProviderSecretChange.setSecret("newSecret"); - ResponseEntity oidcBody = identityProviderEndpoints.changeSecret("puppyId", identityProviderSecretChange); - IdentityProvider oidc = oidcBody.getBody(); - assertNotNull(oidc); - assertNotNull(oidc.getConfig()); - assertTrue(oidc.getConfig() instanceof LdapIdentityProviderDefinition); - assertNull(((LdapIdentityProviderDefinition) oidc.getConfig()).getBindPassword()); - } - - @Test - void change_secret_and_retrieve_by_origin_providers_redacts_data() { - IdentityProvider idp = getExternalOAuthProvider(); - when(mockIdpAliasHandler.ensureConsistencyOfAliasEntity(null, idp)).thenReturn(idp); - when(mockIdentityProviderProvisioning.retrieve("puppyId", "uaa")).thenReturn(idp); - IdentityProviderSecretChange identityProviderSecretChange = new IdentityProviderSecretChange(); - identityProviderSecretChange.setSecret("newSecret"); - ResponseEntity oidcBody = identityProviderEndpoints.changeSecret("puppyId", identityProviderSecretChange); - IdentityProvider oidc = oidcBody.getBody(); - assertNotNull(oidc); - assertNotNull(oidc.getConfig()); - assertTrue(oidc.getConfig() instanceof AbstractExternalOAuthIdentityProviderDefinition); - assertNull(((AbstractExternalOAuthIdentityProviderDefinition) oidc.getConfig()).getRelyingPartySecret()); - assertEquals(ClientAuthentication.CLIENT_SECRET_BASIC, ((AbstractExternalOAuthIdentityProviderDefinition) oidc.getConfig()).getAuthMethod()); - } - - @Test - void change_secret_on_uaafails() { - IdentityProvider identityProvider = new IdentityProvider<>(); - identityProvider.setConfig(new SamlIdentityProviderDefinition()); - identityProvider.setName("my saml provider"); - identityProvider.setIdentityZoneId(OriginKeys.UAA); - identityProvider.setType(OriginKeys.SAML); - IdentityProviderSecretChange identityProviderSecretChange = new IdentityProviderSecretChange(); - when(mockIdentityProviderProvisioning.retrieve("puppyId", "uaa")).thenReturn(identityProvider); - ResponseEntity oidcBody = identityProviderEndpoints.changeSecret("puppyId", identityProviderSecretChange); - IdentityProvider oidc = oidcBody.getBody(); - assertEquals(UNPROCESSABLE_ENTITY, oidcBody.getStatusCode()); - assertNull(oidc); - identityProviderSecretChange.setSecret("newSecret"); - oidcBody = identityProviderEndpoints.changeSecret("puppyId", identityProviderSecretChange); - oidc = oidcBody.getBody(); - assertEquals(UNPROCESSABLE_ENTITY, oidcBody.getStatusCode()); - assertNull(oidc); - } - @Test void update_ldap_provider_patches_password() throws Exception { IdentityProvider provider = retrieve_ldap_provider_by_id("id"); diff --git a/uaa/slateCustomizations/source/index.html.md.erb b/uaa/slateCustomizations/source/index.html.md.erb index 632f61d5291..792a2ce97b3 100644 --- a/uaa/slateCustomizations/source/index.html.md.erb +++ b/uaa/slateCustomizations/source/index.html.md.erb @@ -1261,82 +1261,6 @@ _Error Codes_ | 403 | Forbidden - Insufficient scope | | 422 | Unprocessable Entity - Invalid config | -## Change Secret - - -
-Change a relyingPartySecret in the OAuth2 / OIDC IdP configuration or bindPassword in case of LDAP. -
- -<%= render('IdentityProviderEndpointDocs/createOAuthIdentityProviderThenChangeSecret/curl-request.md') %> -<%= render('IdentityProviderEndpointDocs/createOAuthIdentityProviderThenChangeSecret/http-request.md') %> -<%= render('IdentityProviderEndpointDocs/createOAuthIdentityProviderThenChangeSecret/http-response.md') %> - -_Path Parameters_ - -<%= render('IdentityProviderEndpointDocs/createOAuthIdentityProviderThenChangeSecret/path-parameters.md') %> - -_Request Headers_ - -<%= render('IdentityProviderEndpointDocs/createOAuthIdentityProviderThenChangeSecret/request-headers.md') %> - - - -_Request and Response Fields_ - -<%= render('IdentityProviderEndpointDocs/createOAuthIdentityProviderThenChangeSecret/response-fields.md') %> - -_Error Codes_ - -| Error Code | Description | -|------------|-----------------------------------------------------------------------| -| 403 | Forbidden - Insufficient scope | -| 422 | Unprocessable Entity - Invalid config | - -## Delete Secret - - -
-Delete a secret from the OAuth2 / OIDC IdP configuration only, because these providers support usages without a secret. -
- -<%= render('IdentityProviderEndpointDocs/createOAuthIdentityProviderThenDeleteSecret/curl-request.md') %> -<%= render('IdentityProviderEndpointDocs/createOAuthIdentityProviderThenDeleteSecret/http-request.md') %> -<%= render('IdentityProviderEndpointDocs/createOAuthIdentityProviderThenDeleteSecret/http-response.md') %> - -_Path Parameters_ - -<%= render('IdentityProviderEndpointDocs/createOAuthIdentityProviderThenDeleteSecret/path-parameters.md') %> - -_Request Headers_ - -<%= render('IdentityProviderEndpointDocs/createOAuthIdentityProviderThenDeleteSecret/request-headers.md') %> - - - -_Request and Response Fields_ - -<%= render('IdentityProviderEndpointDocs/createOAuthIdentityProviderThenDeleteSecret/response-fields.md') %> - -_Error Codes_ - -| Error Code | Description | -|------------|-----------------------------------------------------------------------| -| 403 | Forbidden - Insufficient scope | -| 422 | Unprocessable Entity - Invalid config | # Users 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 ec58a149fa0..b9441103546 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 @@ -77,7 +77,6 @@ import org.cloudfoundry.identity.uaa.provider.AbstractExternalOAuthIdentityProviderDefinition; import org.cloudfoundry.identity.uaa.provider.IdentityProvider; import org.cloudfoundry.identity.uaa.provider.IdentityProviderProvisioning; -import org.cloudfoundry.identity.uaa.provider.IdentityProviderSecretChange; import org.cloudfoundry.identity.uaa.provider.IdentityProviderStatus; import org.cloudfoundry.identity.uaa.provider.JdbcIdentityProviderProvisioning; import org.cloudfoundry.identity.uaa.provider.LdapIdentityProviderDefinition; @@ -590,7 +589,6 @@ void createOAuthIdentityProvider() throws Exception { 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"), fieldWithPath("config.linkText").optional(null).type(STRING).description("Text to use for the login link to the provider"), - fieldWithPath("config.auth_method").optional(null).type(STRING).description("UAA 77.10.0 Client authentication method. Possible strings are: client_secret_basic, client_secret_post, private_key_jwt, none."), fieldWithPath("config.relyingPartyId").required().type(STRING).description("The client ID which is registered with the external OAuth provider for use by the UAA"), fieldWithPath("config.skipSslValidation").optional(null).type(BOOLEAN).description("A flag controlling whether SSL validation should be skipped when communicating with the external OAuth server"), fieldWithPath("config.scopes").optional(null).type(ARRAY).description("What scopes to request on a call to the external OAuth provider"), @@ -635,7 +633,7 @@ void createOAuthIdentityProvider() throws Exception { IDENTITY_ZONE_ID, CREATED, LAST_MODIFIED, - fieldWithPath("config.externalGroupsWhitelist").optional(null).type(ARRAY).description("Not currently used."), + fieldWithPath("config.externalGroupsWhitelist").optional(null).type(ARRAY).description("Not currently used.") }, ALIAS_FIELDS_GET ) @@ -697,7 +695,6 @@ void createOidcIdentityProvider() throws Exception { 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"), - fieldWithPath("config.auth_method").optional(null).type(STRING).description("UAA 77.10.0 Client authentication method. Possible strings are: client_secret_basic, client_secret_post, private_key_jwt, none."), fieldWithPath("config.relyingPartyId").required().type(STRING).description("The client ID which is registered with the external OAuth provider for use by the UAA"), fieldWithPath("config.skipSslValidation").optional(null).type(BOOLEAN).description("A flag controlling whether SSL validation should be skipped when communicating with the external OAuth server"), fieldWithPath("config.scopes").optional(null).type(ARRAY).description("What scopes to request on a call to the external OAuth/OpenID provider. For example, can provide " + @@ -1143,56 +1140,6 @@ void patchIdentityProviderStatus() throws Exception { } - @Test - void createOAuthIdentityProviderThenChangeSecret() throws Exception { - IdentityProvider identityProvider = identityProviderProvisioning.retrieveByOrigin("my-oauth2-provider", IdentityZoneHolder.get().getId()); - - IdentityProviderSecretChange identityProviderSecretChange = new IdentityProviderSecretChange(); - identityProviderSecretChange.setSecret("newSecret" + new AlphanumericRandomValueStringGenerator(10).generate()); - - FieldDescriptor[] idempotentFields = new FieldDescriptor[]{ - fieldWithPath("secret").required().description("Set new secret and/or bind password, depending on provided IdP type.") - }; - - Snippet requestFields = requestFields(idempotentFields); - - mockMvc.perform(patch("/identity-providers/{id}/secret", identityProvider.getId()) - .header("Authorization", "Bearer " + adminToken) - .contentType(APPLICATION_JSON) - .content(serializeExcludingProperties(identityProviderSecretChange))) - .andExpect(status().isOk()) - .andDo(document("{ClassName}/{methodName}", - preprocessResponse(prettyPrint()), - pathParameters(parameterWithName("id").description(ID_DESC) - ), - requestHeaders( - headerWithName("Authorization").description("Bearer token containing `zones..admin` or `uaa.admin` or `idps.write` (only in the same zone that you are a user of)"), - IDENTITY_ZONE_ID_HEADER, - IDENTITY_ZONE_SUBDOMAIN_HEADER - ), - requestFields, - responseFields(getCommonProviderFieldsAnyType()))); - } - - @Test - void createOAuthIdentityProviderThenDeleteSecret() throws Exception { - IdentityProvider identityProvider = identityProviderProvisioning.retrieveByOrigin("my-oauth2-provider", IdentityZoneHolder.get().getId()); - - mockMvc.perform(delete("/identity-providers/{id}/secret", identityProvider.getId()) - .header("Authorization", "Bearer " + adminToken)) - .andExpect(status().isOk()) - .andDo(document("{ClassName}/{methodName}", - preprocessResponse(prettyPrint()), - pathParameters(parameterWithName("id").description(ID_DESC) - ), - requestHeaders( - headerWithName("Authorization").description("Bearer token containing `zones..admin` or `uaa.admin` or `idps.write` (only in the same zone that you are a user of)"), - IDENTITY_ZONE_ID_HEADER, - IDENTITY_ZONE_SUBDOMAIN_HEADER - ), - responseFields(getCommonProviderFieldsAnyType()))); - } - @Test void deleteIdentityProvider() throws Exception { IdentityProvider identityProvider = JsonUtils.readValue(mockMvc.perform(post("/identity-providers") From 59e35d57e1a45409693d8c0504fe186fed0354d3 Mon Sep 17 00:00:00 2001 From: d036670 Date: Fri, 31 May 2024 19:38:01 +0200 Subject: [PATCH 19/19] review --- .../identity/uaa/provider/IdentityProviderEndpoints.java | 6 +++--- .../uaa/mock/providers/IdentityProviderEndpointDocs.java | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java index 8fdafe406c8..3e57f54642e 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderEndpoints.java @@ -297,12 +297,12 @@ public ResponseEntity updateIdentityProviderStatus(@Path public ResponseEntity> retrieveIdentityProviders( @RequestParam(value = "active_only", required = false) String activeOnly, @RequestParam(required = false, defaultValue = "false") boolean rawConfig, - @RequestParam(required = false, defaultValue = "") String origin) + @RequestParam(required = false, defaultValue = "") String originKey) { boolean retrieveActiveOnly = Boolean.parseBoolean(activeOnly); List identityProviderList; - if (UaaStringUtils.isNotEmpty(origin)) { - identityProviderList = List.of(identityProviderProvisioning.retrieveByOrigin(origin, identityZoneManager.getCurrentIdentityZoneId())); + if (UaaStringUtils.isNotEmpty(originKey)) { + identityProviderList = List.of(identityProviderProvisioning.retrieveByOrigin(originKey, identityZoneManager.getCurrentIdentityZoneId())); } else { identityProviderList = identityProviderProvisioning.retrieveAll(retrieveActiveOnly, identityZoneManager.getCurrentIdentityZoneId()); } 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 b9441103546..100b2ae51e9 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 @@ -989,7 +989,7 @@ void getFilteredIdentityProviders() throws Exception { mockMvc.perform(get("/identity-providers") .param("rawConfig", "false") .param("active_only", "false") - .param("origin", "my-oauth2-provider") + .param("originKey", "my-oauth2-provider") .header("Authorization", "Bearer " + adminToken) .contentType(APPLICATION_JSON)) .andExpect(status().isOk()) @@ -1003,7 +1003,7 @@ void getFilteredIdentityProviders() throws Exception { requestParameters( parameterWithName("rawConfig").optional("false").type(BOOLEAN).description("Flag indicating whether the response should use raw, unescaped JSON for the `config` field of the IDP, rather than the default behavior of encoding the JSON as a string."), parameterWithName("active_only").optional("false").type(BOOLEAN).description("Flag indicating whether only active IdPs should be returned or all."), - parameterWithName("origin").optional(null).type(STRING).description("UAA 77.10.0 Return only IdPs with specific origin.") + parameterWithName("originKey").optional(null).type(STRING).description("UAA 77.10.0 Return only IdPs with specific origin.") ), responseFields)); }