From 3cea251dabfa7cedede81b6bbdfc5c968f985e70 Mon Sep 17 00:00:00 2001 From: d036670 Date: Thu, 16 May 2024 11:30:20 +0200 Subject: [PATCH] 2nd alternative fix: allow to change or delete a relyingPartySecret on IdP Alternative to PR: #2882, #2887 and PR: #2885 Only one call in backend, but more input on client side. authMethod is used to make this change non-breakable. Because before the update prevented the removal of the relyingPartySecret. No if authMethod is not client_secret_basic or client_secret_post, then the relyingPartySecret can be overwritten with null. --- .../uaa/constants/ClientAuthentication.java | 21 +++ ...ternalOAuthIdentityProviderDefinition.java | 2 - .../IdentityProviderSecretChange.java | 17 --- .../provider/IdentityProviderEndpoints.java | 78 ++--------- ...lOAuthIdentityProviderConfigValidator.java | 9 ++ .../IdentityProviderEndpointsTest.java | 122 +++++------------- ...thIdentityProviderConfigValidatorTest.java | 11 +- .../source/index.html.md.erb | 101 --------------- .../IdentityProviderEndpointDocs.java | 102 ++------------- 9 files changed, 95 insertions(+), 368 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/constants/ClientAuthentication.java b/model/src/main/java/org/cloudfoundry/identity/uaa/constants/ClientAuthentication.java index b847eb58a29..9fec72fee2a 100644 --- a/model/src/main/java/org/cloudfoundry/identity/uaa/constants/ClientAuthentication.java +++ b/model/src/main/java/org/cloudfoundry/identity/uaa/constants/ClientAuthentication.java @@ -1,5 +1,8 @@ package org.cloudfoundry.identity.uaa.constants; +import java.util.List; +import java.util.Optional; + /** * 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 @@ -15,4 +18,22 @@ private ClientAuthentication() {} 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"; + + public static final List UAA_SUPPORTED_METHODS = List.of(CLIENT_SECRET_BASIC, CLIENT_SECRET_POST, NONE, PRIVATE_KEY_JWT); + + public static boolean secretNeeded(String method) { + return method == null || CLIENT_SECRET_POST.equals(method) || CLIENT_SECRET_BASIC.equals(method); + } + + public static boolean isMethodSupported(String method) { + return Optional.ofNullable(method).map(UAA_SUPPORTED_METHODS::contains).orElse(true); + } + + public static boolean isValidMethod(String method, String secret) { + if (isMethodSupported(method) && secretNeeded(method) && secret != null || (method == null && secret == null)) { + return true; + } else { + return false; + } + } } 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 2ec15c53037..0e8b5337175 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,7 +16,6 @@ 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; @@ -53,7 +52,6 @@ public enum OAuthGroupMappingMode { private boolean pkce = true; private boolean performRpInitiatedLogout = true; @JsonInclude(JsonInclude.Include.NON_NULL) - @JsonProperty(value = "auth_method", access = JsonProperty.Access.READ_ONLY) private String authMethod; public T setAuthUrl(URL authUrl) { 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 d55f082b26e..c5ed5bb2069 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 @@ -49,7 +49,6 @@ 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; @@ -69,9 +68,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.PatchMapping; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestMapping; @@ -230,7 +227,6 @@ 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); @@ -257,11 +253,6 @@ 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,63 +312,11 @@ 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) { - 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); - } - } - - @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) { - 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 { - 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, - @RequestParam(required = false, defaultValue = "false") boolean rawConfig, - @RequestParam(required = false, defaultValue = "") String origin) - { + 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; - if (UaaStringUtils.isNotEmpty(origin)) { - identityProviderList = List.of(identityProviderProvisioning.retrieveByOrigin(origin, identityZoneManager.getCurrentIdentityZoneId())); - } else { - identityProviderList = identityProviderProvisioning.retrieveAll(retrieveActiveOnly, identityZoneManager.getCurrentIdentityZoneId()); - } - for(IdentityProvider idp : identityProviderList) { + List identityProviderList = identityProviderProvisioning.retrieveAll(retrieveActiveOnly, identityZoneManager.getCurrentIdentityZoneId()); + for(IdentityProvider idp : identityProviderList) { idp.setSerializeConfigRaw(rawConfig); setAuthMethod(idp); redactSensitiveData(idp); @@ -495,7 +434,8 @@ protected void patchSensitiveData(String id, IdentityProvider provider) { IdentityProvider existing = identityProviderProvisioning.retrieve(id, zoneId); if (existing!=null && existing.getConfig()!=null && - existing.getConfig() instanceof AbstractExternalOAuthIdentityProviderDefinition) { + existing.getConfig() instanceof AbstractExternalOAuthIdentityProviderDefinition && + secretNeeded(definition)) { AbstractExternalOAuthIdentityProviderDefinition existingDefinition = (AbstractExternalOAuthIdentityProviderDefinition)existing.getConfig(); definition.setRelyingPartySecret(existingDefinition.getRelyingPartySecret()); } @@ -537,6 +477,14 @@ protected void redactSensitiveData(IdentityProvider provider) { } } + protected boolean secretNeeded(AbstractExternalOAuthIdentityProviderDefinition abstractExternalOAuthIdentityProviderDefinition) { + boolean needSecret = true; + if (abstractExternalOAuthIdentityProviderDefinition.getAuthMethod() != null) { + return ClientAuthentication.secretNeeded(abstractExternalOAuthIdentityProviderDefinition.getAuthMethod()); + } + return needSecret; + } + protected void setAuthMethod(IdentityProvider provider) { if (provider.getConfig() instanceof AbstractExternalOAuthIdentityProviderDefinition abstractExternalOAuthIdentityProviderDefinition) { if (abstractExternalOAuthIdentityProviderDefinition.getRelyingPartySecret() == null) { diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthIdentityProviderConfigValidator.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthIdentityProviderConfigValidator.java index 0ab99a1a811..504b9967b2c 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthIdentityProviderConfigValidator.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthIdentityProviderConfigValidator.java @@ -1,5 +1,6 @@ package org.cloudfoundry.identity.uaa.provider.oauth; +import org.cloudfoundry.identity.uaa.constants.ClientAuthentication; import org.cloudfoundry.identity.uaa.provider.AbstractExternalOAuthIdentityProviderDefinition; import org.cloudfoundry.identity.uaa.provider.AbstractIdentityProviderDefinition; import org.cloudfoundry.identity.uaa.provider.BaseIdentityProviderValidator; @@ -61,6 +62,14 @@ public void validate(AbstractIdentityProviderDefinition definition) { errors.add("Relying Party Id must be the client-id for the UAA that is registered with the external IDP"); } + if (hasText(def.getAuthMethod()) && !ClientAuthentication.isMethodSupported(def.getAuthMethod())) { + errors.add("Relying Party Authentication Method must be set to either " + String.join(" or ", ClientAuthentication.UAA_SUPPORTED_METHODS)); + } else { + if (!ClientAuthentication.isValidMethod(def.getAuthMethod(), def.getRelyingPartySecret())) { + errors.add("Relying Party Authentication has invalid requirements for the secret."); + } + } + if (def.isShowLinkText() && !hasText(def.getLinkText())) { errors.add("Link Text must be specified because showLinkText is true"); } 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 9f2ebde049e..2d1a9fd2327 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 @@ -290,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()); @@ -307,95 +307,6 @@ 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() { - 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"); @@ -843,6 +754,37 @@ void create_oauth_provider_removes_password() throws Exception { assertNotNull(created.getConfig()); assertTrue(created.getConfig() instanceof AbstractExternalOAuthIdentityProviderDefinition); assertNull(((AbstractExternalOAuthIdentityProviderDefinition) created.getConfig()).getRelyingPartySecret()); + assertEquals(ClientAuthentication.CLIENT_SECRET_BASIC,((AbstractExternalOAuthIdentityProviderDefinition) created.getConfig()).getAuthMethod()); + } + } + + @Test + void create_oauth_provider_set_auth_method_none() throws Exception { + String zoneId = IdentityZone.getUaaZoneId(); + for (String type : Arrays.asList(OIDC10, OAUTH20)) { + IdentityProvider externalOAuthDefinition = getExternalOAuthProvider(); + assertNotNull(externalOAuthDefinition.getConfig().getRelyingPartySecret()); + externalOAuthDefinition.setType(type); + when(mockIdentityProviderProvisioning.create(any(), eq(zoneId))).thenReturn(externalOAuthDefinition); + ResponseEntity response = identityProviderEndpoints.createIdentityProvider(externalOAuthDefinition, true); + IdentityProvider created = response.getBody(); + assertNotNull(created); + assertEquals(type, created.getType()); + assertNotNull(created.getConfig()); + assertTrue(created.getConfig() instanceof AbstractExternalOAuthIdentityProviderDefinition); + assertNull(((AbstractExternalOAuthIdentityProviderDefinition) created.getConfig()).getRelyingPartySecret()); + assertEquals(ClientAuthentication.CLIENT_SECRET_BASIC,((AbstractExternalOAuthIdentityProviderDefinition) created.getConfig()).getAuthMethod()); + externalOAuthDefinition.getConfig().setRelyingPartySecret(null); + externalOAuthDefinition.getConfig().setAuthMethod("none"); + AbstractExternalOAuthIdentityProviderDefinition spy = Mockito.spy(externalOAuthDefinition.getConfig()); + when(mockIdentityProviderProvisioning.retrieve(eq(externalOAuthDefinition.getId()), eq(zoneId))).thenReturn(getExternalOAuthProvider()); + response = identityProviderEndpoints.updateIdentityProvider(created.getId(), externalOAuthDefinition, true); + IdentityProvider upated = response.getBody(); + assertNotNull(upated); + assertEquals(type, upated.getType()); + assertNotNull(upated.getConfig()); + verify(spy, never()).setRelyingPartySecret(eq(getExternalOAuthProvider().getConfig().getRelyingPartySecret())); + assertEquals(ClientAuthentication.NONE,((AbstractExternalOAuthIdentityProviderDefinition) upated.getConfig()).getAuthMethod()); } } diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthIdentityProviderConfigValidatorTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthIdentityProviderConfigValidatorTest.java index 24a1ee9973f..e710fbe8c67 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthIdentityProviderConfigValidatorTest.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthIdentityProviderConfigValidatorTest.java @@ -1,5 +1,6 @@ package org.cloudfoundry.identity.uaa.provider.oauth; +import org.cloudfoundry.identity.uaa.constants.ClientAuthentication; import org.cloudfoundry.identity.uaa.provider.AbstractExternalOAuthIdentityProviderDefinition; import org.cloudfoundry.identity.uaa.provider.BaseIdentityProviderValidator; import org.cloudfoundry.identity.uaa.provider.OIDCIdentityProviderDefinition; @@ -61,9 +62,17 @@ public void configWithNullRelyingPartyId_ThrowsException() { validator.validate(definition); } - @Test + @Test(expected = IllegalArgumentException.class) public void configWithNullRelyingPartySecret_ThrowsException() { definition.setRelyingPartySecret(null); + definition.setAuthMethod(ClientAuthentication.CLIENT_SECRET_BASIC); + validator = new ExternalOAuthIdentityProviderConfigValidator(); + validator.validate(definition); + } + + @Test(expected = IllegalArgumentException.class) + public void configWithInvalidAuthMethod_ThrowsException() { + definition.setAuthMethod("no-sure-about-this"); validator = new ExternalOAuthIdentityProviderConfigValidator(); validator.validate(definition); } diff --git a/uaa/slateCustomizations/source/index.html.md.erb b/uaa/slateCustomizations/source/index.html.md.erb index 632f61d5291..490ae2f633f 100644 --- a/uaa/slateCustomizations/source/index.html.md.erb +++ b/uaa/slateCustomizations/source/index.html.md.erb @@ -1115,31 +1115,6 @@ _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') %> @@ -1261,82 +1236,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 48abab062e3..bf40d962693 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; @@ -605,6 +604,15 @@ void createOAuthIdentityProvider() throws Exception { fieldWithPath("config.userPropagationParameter").optional("username").type(STRING).description("Name of the request parameter that is used to pass a known username when redirecting to this identity provider from the account chooser"), fieldWithPath("config.attributeMappings.user_name").optional("sub").type(STRING).description("Map `user_name` to the attribute for user name in the provider assertion or token. The default for OpenID Connect is `sub`"), fieldWithPath("config.groupMappingMode").optional(AbstractExternalOAuthIdentityProviderDefinition.OAuthGroupMappingMode.EXPLICITLY_MAPPED).type(STRING).description("Either ``EXPLICITLY_MAPPED`` in order to map external claim values to OAuth scopes using the group mappings, or ``AS_SCOPES`` to use claim values names as scopes. You need to define also ``external_groups`` for the mapping in order to use this feature."), + fieldWithPath("config.authMethod").optional("client_secret_basic").type(STRING).description("UAA 77.10.0 Define explicit method to authenticate against identity provider. Supported values are client_secret_basic, client_secret_post, private_key_jwt and none. Remark: In case you switch the method from secret methods towards private_key_jwt or none, your existing relyingPartySecret is removed. If you want switch back, provide again a relyingPartySecret in configuration. Defaults to client_secret_basic."), + fieldWithPath("config.jwtClientAuthentication").optional(null).type(OBJECT).description("UAA 76.5.0 Only effective if relyingPartySecret is not set or null. Creates private_key_jwt client authentication according to OIDC or OAuth2 (RFC 7523) standard. "+ + "
For standard OIDC compliance, set this field to `true`. Alternatively, you can further configure the created JWT for client authentication by setting this parameter to an Object containing sub-parameters, e.g. if your IdP follows OAuth2 standard according to RFC 7523. The supported sub-parameters are" + + "
  • `kid` UAA 76.18.0 Optional custom key from your defined keys, defaults to `activeKeyId` from token policy section
    • " + + "
  • `key` UAA 77.4.0 Optional custom private key, used to generate the client JWT signature, defaults to key from token policy, depending on `kid`
    • " + + "
  • `cert` UAA 77.4.0 Optional custom X509 certificate, related to key, used to generate the client JWT with x5t header, defaults to a cert from token policy or omits x5t header
    • " + + "
  • `iss` Optional custom issuer, see RFC 7523, defaults to `relyingPartyId` for OIDC compliance
    • " + + "
  • `aud` Optional custom audience, see RFC 7523, defaults to `tokenUrl` for OIDC compliance

"+ + "The values in the list can be a reference to another section in uaa yaml, e.g. define for key a reference like ${\"jwt.client.key\"}. This will load the private key from yaml context jwt.client.key. The advantage is, that you can use a single key for many IdP configurations and the key itself is not persistent in the UAA DB.

"), }, attributeMappingFields)); Snippet requestFields = requestFields( @@ -706,6 +714,7 @@ void createOidcIdentityProvider() throws Exception { GROUP_WHITELIST, fieldWithPath("config.passwordGrantEnabled").optional(false).type(BOOLEAN).description("Enable Resource Owner Password Grant flow for this identity provider."), fieldWithPath("config.setForwardHeader").optional(false).type(BOOLEAN).description("Only effective if Password Grant enabled. Set X-Forward-For header in Password Grant request to this identity provider."), + fieldWithPath("config.authMethod").optional("client_secret_basic").type(STRING).description("UAA 77.10.0 Define explicit method to authenticate against identity provider. Supported values are client_secret_basic, client_secret_post, private_key_jwt and none. Remark: In case you switch the method from secret methods towards private_key_jwt or none, your existing relyingPartySecret is removed. If you want switch back, provide again a relyingPartySecret in configuration. Defaults to client_secret_basic."), fieldWithPath("config.jwtClientAuthentication").optional(null).type(OBJECT).description("UAA 76.5.0 Only effective if relyingPartySecret is not set or null. Creates private_key_jwt client authentication according to OIDC or OAuth2 (RFC 7523) standard. "+ "
For standard OIDC compliance, set this field to `true`. Alternatively, you can further configure the created JWT for client authentication by setting this parameter to an Object containing sub-parameters, e.g. if your IdP follows OAuth2 standard according to RFC 7523. The supported sub-parameters are" + "
  • `kid` UAA 76.18.0 Optional custom key from your defined keys, defaults to `activeKeyId` from token policy section
    • " + @@ -961,47 +970,6 @@ 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") @@ -1134,56 +1102,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")