From 9be8d9c3c6d97fc19d136a60a52b6747fa2fdd70 Mon Sep 17 00:00:00 2001 From: d036670 Date: Mon, 13 May 2024 10:59:23 +0200 Subject: [PATCH 1/3] fix: allow to delete a relyingPartySecret on IdP Extract the delete change into an own PR, because this missing method is really a bug / issue in UAA. A UAA operator cannot remove a secret currently, but there are situation where this is needed, e.g. public flows or change to private_key_jwt. --- .../provider/IdentityProviderEndpoints.java | 18 +++++++++ .../IdentityProviderEndpointsTest.java | 21 ++++++++++ .../source/index.html.md.erb | 39 +++++++++++++++++++ .../IdentityProviderEndpointDocs.java | 20 +++++++++- 4 files changed, 97 insertions(+), 1 deletion(-) 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..d33373e57ac 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 @@ -67,6 +67,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; @@ -308,6 +309,23 @@ 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); + 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 IDP of type OAuth/OIDC"); + 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) { boolean retrieveActiveOnly = Boolean.parseBoolean(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 776067616e7..385c322e200 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; @@ -305,6 +306,26 @@ void retrieve_all_providers_redacts_data() { assertNull(oauth.getConfig().getRelyingPartySecret()); } + @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()); + } + + @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"); diff --git a/uaa/slateCustomizations/source/index.html.md.erb b/uaa/slateCustomizations/source/index.html.md.erb index 490ae2f633f..3592399902c 100644 --- a/uaa/slateCustomizations/source/index.html.md.erb +++ b/uaa/slateCustomizations/source/index.html.md.erb @@ -1236,6 +1236,45 @@ _Error Codes_ | 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 24a27cd2f39..7d3c9de8148 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 { @@ -1091,6 +1090,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 88441bdcf7f815392300f2e89f46ee2d3ec62479 Mon Sep 17 00:00:00 2001 From: d036670 Date: Tue, 14 May 2024 10:56:04 +0200 Subject: [PATCH 2/3] alternative fix: allow to change or delete a relyingPartySecret on IdP Alternative to PR: https://github.com/cloudfoundry/uaa/pull/2882 and PR: https://github.com/cloudfoundry/uaa/pull/2885 Only one call in backend, but more input on client side. --- .../provider/IdentityProviderEndpoints.java | 32 ++++++++--- .../IdentityProviderSecretChangeRequest.java | 45 ++++++++++++++++ .../IdentityProviderEndpointsTest.java | 54 ++++++++++++++++--- .../source/index.html.md.erb | 29 +++++----- .../IdentityProviderEndpointDocs.java | 27 +++++++--- 5 files changed, 152 insertions(+), 35 deletions(-) create mode 100644 server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderSecretChangeRequest.java 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 d33373e57ac..28626cf20c0 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,6 +37,7 @@ import java.io.StringWriter; import java.util.Date; import java.util.List; +import java.util.Locale; import java.util.Optional; import org.cloudfoundry.identity.uaa.alias.EntityAliasFailedException; @@ -48,6 +49,7 @@ 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,9 +69,9 @@ 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.PutMapping; import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestParam; @@ -309,21 +311,35 @@ public ResponseEntity updateIdentityProviderStatus(@Path return new ResponseEntity<>(body, OK); } - @DeleteMapping(value = "{id}/secret") - public ResponseEntity deleteSecret(@PathVariable String id) { + @PutMapping(value = "{id}/secret") + public ResponseEntity changeSecret(@PathVariable String id, @RequestBody IdentityProviderSecretChangeRequest secretChangeRequest) { String zoneId = identityZoneManager.getCurrentIdentityZoneId(); IdentityProvider existing = identityProviderProvisioning.retrieve(id, zoneId); + if(secretChangeRequest.getChangeMode() == IdentityProviderSecretChangeRequest.ChangeMode.UPDATE + && UaaStringUtils.isNullOrEmpty(secretChangeRequest.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(null); - identityProviderProvisioning.update(existing, zoneId); - redactSensitiveData(existing); - logger.info("Secret deleted for Identity Provider: {}", existing.getId()); + idpConfiguration.setRelyingPartySecret(secretChangeRequest.getSecret()); return new ResponseEntity<>(existing, OK); + } else if(LDAP.equals(existing.getType()) + && secretChangeRequest.getChangeMode() == IdentityProviderSecretChangeRequest.ChangeMode.UPDATE + && existing.getConfig() instanceof LdapIdentityProviderDefinition ldapIdentityProviderDefinition) { + ldapIdentityProviderDefinition.setBindPassword(secretChangeRequest.getSecret()); } else { - logger.debug("Invalid operation. This operation is only supported on external IDP of type OAuth/OIDC"); + if (secretChangeRequest.getChangeMode() == IdentityProviderSecretChangeRequest.ChangeMode.DELETE) { + logger.info("Invalid operation. This operation is only supported on external IDP of type OAuth/OIDC"); + } else { + logger.debug("Invalid operation. This operation has an invalid status"); + } return new ResponseEntity<>(UNPROCESSABLE_ENTITY); } + logger.info("Secret {} for Identity Provider: {}", secretChangeRequest.getChangeMode().toString().toLowerCase(Locale.ROOT), existing.getId()); + identityProviderProvisioning.update(existing, zoneId); + redactSensitiveData(existing); + return new ResponseEntity<>(existing, OK); } @RequestMapping(method = GET) diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderSecretChangeRequest.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderSecretChangeRequest.java new file mode 100644 index 00000000000..9d577bec8ee --- /dev/null +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/IdentityProviderSecretChangeRequest.java @@ -0,0 +1,45 @@ +package org.cloudfoundry.identity.uaa.provider; + +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.annotation.JsonInclude; + +import static org.cloudfoundry.identity.uaa.provider.IdentityProviderSecretChangeRequest.ChangeMode.DELETE; +import static org.cloudfoundry.identity.uaa.provider.IdentityProviderSecretChangeRequest.ChangeMode.UPDATE; + +@JsonInclude(JsonInclude.Include.NON_NULL) +@JsonIgnoreProperties(ignoreUnknown = true) +public class IdentityProviderSecretChangeRequest { + + public enum ChangeMode { + UPDATE, + DELETE + } + + private String secret; + private ChangeMode changeMode; + + public IdentityProviderSecretChangeRequest() { + changeMode = DELETE; + } + + public IdentityProviderSecretChangeRequest(String secret) { + changeMode = UPDATE; + this.secret = secret; + } + + public String getSecret() { + return secret; + } + + public void setSecret(String secret) { + this.secret = secret; + } + + public ChangeMode getChangeMode() { + return changeMode; + } + + public void setChangeMode(ChangeMode changeMode) { + this.changeMode = changeMode; + } +} 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 385c322e200..40d2046112b 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 @@ -307,21 +307,61 @@ void retrieve_all_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"); + void change_bindPassword_and_retrieve_by_origin_providers_redacts_data() { + IdentityProvider idp = getLdapDefinition(); + when(mockIdentityProviderProvisioning.retrieve("puppyId", "uaa")).thenReturn(idp); + IdentityProviderSecretChangeRequest identityProviderSecretChangeRequest = new IdentityProviderSecretChangeRequest("newSecret"); + ResponseEntity oidcBody = identityProviderEndpoints.changeSecret("puppyId", identityProviderSecretChangeRequest); + 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); + IdentityProviderSecretChangeRequest identityProviderSecretChangeRequest = new IdentityProviderSecretChangeRequest(); + identityProviderSecretChangeRequest.setSecret("newSecret"); + ResponseEntity oidcBody = identityProviderEndpoints.changeSecret("puppyId", identityProviderSecretChangeRequest); IdentityProvider oidc = oidcBody.getBody(); assertNotNull(oidc); assertNotNull(oidc.getConfig()); assertTrue(oidc.getConfig() instanceof AbstractExternalOAuthIdentityProviderDefinition); - assertNull(((AbstractExternalOAuthIdentityProviderDefinition) oidc.getConfig()).getRelyingPartySecret()); + assertNotNull(((AbstractExternalOAuthIdentityProviderDefinition) oidc.getConfig()).getRelyingPartySecret()); } @Test - void delete_secret_on_ldap_fails() { - when(mockIdentityProviderProvisioning.retrieve("puppyId", "uaa")).thenReturn(getLdapDefinition()); - ResponseEntity oidcBody = identityProviderEndpoints.deleteSecret("puppyId"); + void change_secret_invalid() { + IdentityProvider idp = getExternalOAuthProvider(); + when(mockIdentityProviderProvisioning.retrieve("puppyId", "uaa")).thenReturn(idp); + IdentityProviderSecretChangeRequest identityProviderSecretChangeRequest = new IdentityProviderSecretChangeRequest(""); + ResponseEntity oidcBody = identityProviderEndpoints.changeSecret("puppyId", identityProviderSecretChangeRequest); + // secret must be set + assertEquals(UNPROCESSABLE_ENTITY, oidcBody.getStatusCode()); + } + + @Test + void change_secret_on_uaafails() { + IdentityProvider identityProvider = new IdentityProvider<>(); + identityProvider.setConfig(new SamlIdentityProviderDefinition()); + identityProvider.setName("my saml provider"); + identityProvider.setIdentityZoneId(UAA); + identityProvider.setType(OriginKeys.SAML); + IdentityProviderSecretChangeRequest identityProviderSecretChangeRequest = new IdentityProviderSecretChangeRequest(); + when(mockIdentityProviderProvisioning.retrieve("puppyId", "uaa")).thenReturn(identityProvider); + ResponseEntity oidcBody = identityProviderEndpoints.changeSecret("puppyId", identityProviderSecretChangeRequest); IdentityProvider oidc = oidcBody.getBody(); + // because not allowed for SAML IdP + assertEquals(UNPROCESSABLE_ENTITY, oidcBody.getStatusCode()); + assertNull(oidc); + // because not possible for SAML IdP + identityProviderSecretChangeRequest.setSecret("newSecret"); + identityProviderSecretChangeRequest.setChangeMode(IdentityProviderSecretChangeRequest.ChangeMode.UPDATE); + oidcBody = identityProviderEndpoints.changeSecret("puppyId", identityProviderSecretChangeRequest); + oidc = oidcBody.getBody(); assertEquals(UNPROCESSABLE_ENTITY, oidcBody.getStatusCode()); assertNull(oidc); } diff --git a/uaa/slateCustomizations/source/index.html.md.erb b/uaa/slateCustomizations/source/index.html.md.erb index 3592399902c..bc9ba6ee41c 100644 --- a/uaa/slateCustomizations/source/index.html.md.erb +++ b/uaa/slateCustomizations/source/index.html.md.erb @@ -1236,38 +1236,39 @@ _Error Codes_ | 403 | Forbidden - Insufficient scope | | 422 | Unprocessable Entity - Invalid config | -## Delete Secret +## Change Secret
-Delete a secret from the OAuth2 / OIDC IdP configuration only, because these providers support usages without a secret. +Change or delete a secret. Change is supported for LDAP, OAuth2 and OIDC IdP configuration. +Delete is supported for 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') %> +<%= render('IdentityProviderEndpointDocs/changeIdpSecret/curl-request.md') %> +<%= render('IdentityProviderEndpointDocs/changeIdpSecret/http-request.md') %> +<%= render('IdentityProviderEndpointDocs/changeIdpSecret/http-response.md') %> _Path Parameters_ -<%= render('IdentityProviderEndpointDocs/createOAuthIdentityProviderThenDeleteSecret/path-parameters.md') %> +<%= render('IdentityProviderEndpointDocs/changeIdpSecret/path-parameters.md') %> _Request Headers_ -<%= render('IdentityProviderEndpointDocs/createOAuthIdentityProviderThenDeleteSecret/request-headers.md') %> +<%= render('IdentityProviderEndpointDocs/changeIdpSecret/request-headers.md') %> -_Request and Response Fields_ +_Request Fields_ + +<%= render('IdentityProviderEndpointDocs/changeIdpSecret/request-fields.md') %> + +_Response Fields_ -<%= render('IdentityProviderEndpointDocs/createOAuthIdentityProviderThenDeleteSecret/response-fields.md') %> +<%= render('IdentityProviderEndpointDocs/changeIdpSecret/response-fields.md') %> _Error Codes_ 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 7d3c9de8148..8597973071f 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 @@ -20,6 +20,8 @@ import static org.cloudfoundry.identity.uaa.provider.IdentityProvider.FIELD_ALIAS_ID; import static org.cloudfoundry.identity.uaa.provider.IdentityProvider.FIELD_ALIAS_ZID; import static org.cloudfoundry.identity.uaa.provider.IdentityProvider.FIELD_IDENTITY_ZONE_ID; +import static org.cloudfoundry.identity.uaa.provider.IdentityProviderSecretChangeRequest.ChangeMode.DELETE; +import static org.cloudfoundry.identity.uaa.provider.IdentityProviderSecretChangeRequest.ChangeMode.UPDATE; import static org.cloudfoundry.identity.uaa.provider.LdapIdentityProviderDefinition.MAIL; import static org.cloudfoundry.identity.uaa.provider.SamlIdentityProviderDefinition.EMAIL_ATTRIBUTE_NAME; import static org.cloudfoundry.identity.uaa.provider.SamlIdentityProviderDefinition.EMAIL_VERIFIED_ATTRIBUTE_NAME; @@ -77,6 +79,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.IdentityProviderSecretChangeRequest; import org.cloudfoundry.identity.uaa.provider.IdentityProviderStatus; import org.cloudfoundry.identity.uaa.provider.JdbcIdentityProviderProvisioning; import org.cloudfoundry.identity.uaa.provider.LdapIdentityProviderDefinition; @@ -1091,21 +1094,33 @@ void patchIdentityProviderStatus() throws Exception { } @Test - void createOAuthIdentityProviderThenDeleteSecret() throws Exception { + void changeIdpSecret() 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}", + IdentityProviderSecretChangeRequest identityProviderSecretChange = new IdentityProviderSecretChangeRequest(); + + final FieldDescriptor[] secretChangeFields = new FieldDescriptor[]{ + fieldWithPath("secret").constrained("Optional if changeMode is `"+DELETE+"`. Required otherwise.").type(STRING).description("The new secret and/or bindPassword for the external IdP."), + fieldWithPath("changeMode").constrained("Required, `"+DELETE+"` or `"+UPDATE+"`.").type(STRING).description("If the change mode is set to `"+DELETE+"`, the existing secret will be deleted to support other OAuth methods. If set to `"+UPDATE+"` requires a secret.") + }; + + mockMvc.perform(put("/identity-providers/{id}/secret", identityProvider.getId()) + .header("Authorization", "Bearer " + adminToken) + .contentType(APPLICATION_JSON) + .accept(APPLICATION_JSON) + .content(serializeExcludingProperties(identityProviderSecretChange))) + .andExpect(status().isOk()) + .andDo(document("{ClassName}/{methodName}", + preprocessRequest(prettyPrint()), preprocessResponse(prettyPrint()), - pathParameters(parameterWithName("id").description(ID_DESC) + pathParameters(parameterWithName("id").required().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(secretChangeFields), responseFields(getCommonProviderFieldsAnyType()))); } From 7e62f696da0b4edc7c5f98cefe201857ef214180 Mon Sep 17 00:00:00 2001 From: d036670 Date: Tue, 14 May 2024 11:41:15 +0200 Subject: [PATCH 3/3] Alias handling --- .../provider/IdentityProviderEndpoints.java | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 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 28626cf20c0..d7b5960708d 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 @@ -231,16 +231,14 @@ 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.debug("IdentityProvider[origin=" + body.getOriginKey() + "; zone=" + body.getIdentityZoneId() + + "] - Configuration validation error for update.", 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.", - getCleanedUserControlString(body.getOriginKey()), - getCleanedUserControlString(body.getIdentityZoneId()) - ); + 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); } @@ -252,7 +250,11 @@ public ResponseEntity updateIdentityProvider(@PathVariable Str samlConfigurator.validateSamlIdentityProviderDefinition(definition); 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,9 +323,10 @@ 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(secretChangeRequest.getSecret()); - return new ResponseEntity<>(existing, OK); + && existing.getConfig() instanceof AbstractExternalOAuthIdentityProviderDefinition) { + IdentityProvider updated = existing; + ((AbstractExternalOAuthIdentityProviderDefinition) updated.getConfig()).setRelyingPartySecret(secretChangeRequest.getSecret()); + return persistIdentityProviderChange(updated, false, zoneId, existing); } else if(LDAP.equals(existing.getType()) && secretChangeRequest.getChangeMode() == IdentityProviderSecretChangeRequest.ChangeMode.UPDATE && existing.getConfig() instanceof LdapIdentityProviderDefinition ldapIdentityProviderDefinition) {