From ede8431990e674b5b645b96149ffefc0158d0203 Mon Sep 17 00:00:00 2001 From: d036670 Date: Tue, 14 May 2024 13:33:59 +0200 Subject: [PATCH 1/9] 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 2/9] 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 3/9] 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 4/9] 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 5/9] 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 6/9] 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 7/9] 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 8/9] 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 9/9] 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"))