diff --git a/server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java b/server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java index 537588175ef..f6c17587b06 100755 --- a/server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java +++ b/server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java @@ -81,7 +81,9 @@ import org.keycloak.models.GroupModel; import org.keycloak.models.IdentityProviderMapperModel; import org.keycloak.models.IdentityProviderModel; import org.keycloak.models.KeycloakSession; +import org.keycloak.models.ModelDuplicateException; import org.keycloak.models.ModelException; +import org.keycloak.models.ModelValidationException; import org.keycloak.models.OrganizationDomainModel; import org.keycloak.models.OrganizationModel; import org.keycloak.models.ProtocolMapperModel; @@ -630,7 +632,11 @@ public class RepresentationToModel { existingProtocolMappers.remove(protocolNameKey); } else { - resource.addProtocolMapper(toModel(protocolMapperRepresentation)); + try { + resource.addProtocolMapper(toModel(protocolMapperRepresentation)); + } catch (ModelDuplicateException e) { + throw new ModelValidationException("Cannot add protocol mapper '%s'. %s".formatted(protocolMapperRepresentation.getName(), e.getMessage())); + } } } diff --git a/services/src/main/java/org/keycloak/services/resources/admin/ClientResource.java b/services/src/main/java/org/keycloak/services/resources/admin/ClientResource.java index 636c243ee0b..402782fd66e 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/ClientResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/ClientResource.java @@ -53,6 +53,7 @@ import org.keycloak.models.ClientSecretConstants; import org.keycloak.models.Constants; import org.keycloak.models.KeycloakSession; import org.keycloak.models.ModelDuplicateException; +import org.keycloak.models.ModelValidationException; import org.keycloak.models.RealmModel; import org.keycloak.models.UserCredentialModel; import org.keycloak.models.UserModel; @@ -183,6 +184,8 @@ public class ClientResource { throw ErrorResponse.error(cte.getMessage(), cte.getParameters(), Response.Status.BAD_REQUEST); } catch (ClientPolicyException cpe) { throw new ErrorResponseException(cpe.getError(), cpe.getErrorDetail(), Response.Status.BAD_REQUEST); + } catch (ModelValidationException e) { + throw new ErrorResponseException(Errors.INVALID_INPUT, e.getMessage(), Response.Status.BAD_REQUEST); } } @@ -426,7 +429,7 @@ public class ClientResource { } validateClientScopeAssignment(session, clientScope, defaultScope, realm); - + client.addClientScope(clientScope, defaultScope); adminEvent.operation(OperationType.CREATE).resource(ResourceType.CLIENT_SCOPE_CLIENT_MAPPING).resourcePath(session.getContext().getUri()).success(); diff --git a/services/src/main/java/org/keycloak/services/resources/admin/ClientsResource.java b/services/src/main/java/org/keycloak/services/resources/admin/ClientsResource.java index 8e99a9a8773..d00be5b699d 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/ClientsResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/ClientsResource.java @@ -246,7 +246,7 @@ public class ClientsResource { } catch (ClientPolicyException cpe) { throw new ErrorResponseException(cpe.getError(), cpe.getErrorDetail(), Response.Status.BAD_REQUEST); } catch (ModelValidationException e) { - throw new ErrorResponseException("validation error", e.getMessage(), Response.Status.BAD_REQUEST); + throw new ErrorResponseException(Errors.INVALID_INPUT, e.getMessage(), Response.Status.BAD_REQUEST); } catch (ClientTypeException cte) { throw ErrorResponse.error(cte.getMessage(), cte.getParameters(), Response.Status.BAD_REQUEST); diff --git a/tests/base/src/test/java/org/keycloak/tests/admin/ClientTest.java b/tests/base/src/test/java/org/keycloak/tests/admin/ClientTest.java index abffe6565a7..7c8ebc63588 100644 --- a/tests/base/src/test/java/org/keycloak/tests/admin/ClientTest.java +++ b/tests/base/src/test/java/org/keycloak/tests/admin/ClientTest.java @@ -351,9 +351,15 @@ public class ClientTest { } rep.setRedirectUris(null); - if (expectedRootUrlError != null) rep.setRootUrl(testUrl); - if (expectedBaseUrlError != null) rep.setBaseUrl(testUrl); - if (expectedRedirectUrisError != null) rep.setRedirectUris(List.of(testUrl)); + if (expectedRootUrlError != null) { + rep.setRootUrl(testUrl); + } + if (expectedBaseUrlError != null) { + rep.setBaseUrl(testUrl); + } + if (expectedRedirectUrisError != null) { + rep.setRedirectUris(List.of(testUrl)); + } createOrUpdateClientExpectingValidationErrors(rep, create, expectedRootUrlError, expectedBaseUrlError, expectedRedirectUrisError); rep.setRootUrl(null); @@ -969,27 +975,54 @@ public class ClientTest { ClientRepresentation storedClient = managedRealm.admin().clients().get(client.getId()).toRepresentation(); assertClient(client, storedClient); + + // try adding a duplicate protocol mapper + protocolMappers.add(barMapper); + newClient.setProtocolMappers(protocolMappers); + + createOrUpdateClientExpectingValidationErrors(newClient, false, "Cannot add protocol mapper 'bar'. Protocol mapper name must be unique per protocol"); } public static void assertClient(ClientRepresentation client, ClientRepresentation storedClient) { - if (client.getClientId() != null) Assert.assertEquals(client.getClientId(), storedClient.getClientId()); - if (client.getName() != null) Assert.assertEquals(client.getName(), storedClient.getName()); - if (client.isEnabled() != null) Assert.assertEquals(client.isEnabled(), storedClient.isEnabled()); - if (client.isAlwaysDisplayInConsole() != null) + if (client.getClientId() != null) { + Assert.assertEquals(client.getClientId(), storedClient.getClientId()); + } + if (client.getName() != null) { + Assert.assertEquals(client.getName(), storedClient.getName()); + } + if (client.isEnabled() != null) { + Assert.assertEquals(client.isEnabled(), storedClient.isEnabled()); + } + if (client.isAlwaysDisplayInConsole() != null) { Assert.assertEquals(client.isAlwaysDisplayInConsole(), storedClient.isAlwaysDisplayInConsole()); - if (client.isBearerOnly() != null) Assert.assertEquals(client.isBearerOnly(), storedClient.isBearerOnly()); - if (client.isPublicClient() != null) + } + if (client.isBearerOnly() != null) { + Assert.assertEquals(client.isBearerOnly(), storedClient.isBearerOnly()); + } + if (client.isPublicClient() != null) { Assert.assertEquals(client.isPublicClient(), storedClient.isPublicClient()); - if (client.isFullScopeAllowed() != null) + } + if (client.isFullScopeAllowed() != null) { Assert.assertEquals(client.isFullScopeAllowed(), storedClient.isFullScopeAllowed()); - if (client.getRootUrl() != null) Assert.assertEquals(client.getRootUrl(), storedClient.getRootUrl()); - if (client.getAdminUrl() != null) Assert.assertEquals(client.getAdminUrl(), storedClient.getAdminUrl()); - if (client.getBaseUrl() != null) Assert.assertEquals(client.getBaseUrl(), storedClient.getBaseUrl()); - if (client.isSurrogateAuthRequired() != null) + } + if (client.getRootUrl() != null) { + Assert.assertEquals(client.getRootUrl(), storedClient.getRootUrl()); + } + if (client.getAdminUrl() != null) { + Assert.assertEquals(client.getAdminUrl(), storedClient.getAdminUrl()); + } + if (client.getBaseUrl() != null) { + Assert.assertEquals(client.getBaseUrl(), storedClient.getBaseUrl()); + } + if (client.isSurrogateAuthRequired() != null) { Assert.assertEquals(client.isSurrogateAuthRequired(), storedClient.isSurrogateAuthRequired()); - if (client.getClientAuthenticatorType() != null) + } + if (client.getClientAuthenticatorType() != null) { Assert.assertEquals(client.getClientAuthenticatorType(), storedClient.getClientAuthenticatorType()); - if (client.getSecret() != null) Assert.assertEquals(client.getSecret(), storedClient.getSecret()); + } + if (client.getSecret() != null) { + Assert.assertEquals(client.getSecret(), storedClient.getSecret()); + } if (client.getNotBefore() != null) { Assertions.assertEquals(client.getNotBefore(), storedClient.getNotBefore());