upstream/keycloak
1
0
Fork 0
mirror of https://github.com/keycloak/keycloak.git synced 2026-02-18 18:37:54 -05:00
Code Issues Projects Releases Packages Wiki Activity Actions 15

fix: ensuring proper error handling for duplicate protocol mappers

Browse source 
closes: #26946

Signed-off-by: Steve Hawkins <shawkins@redhat.com>
This commit is contained in:
Steven Hawkins 2026-02-13 11:33:01 -05:00 committed by GitHub
parent 404ba76526
commit c28cac9db3
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 61 additions and 19 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java
View file

@ -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()));
}
}
}

5
services/src/main/java/org/keycloak/services/resources/admin/ClientResource.java
View file

@ -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();

2
services/src/main/java/org/keycloak/services/resources/admin/ClientsResource.java
View file

@ -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);

65
tests/base/src/test/java/org/keycloak/tests/admin/ClientTest.java
View file

@ -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());