Validate name of client scope (#12571)

Closes #12553
This commit is contained in:
Lex Cao 2022-06-27 18:26:18 +08:00 committed by GitHub
parent c058983655
commit f8a7c8e160
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 53 additions and 3 deletions

View file

@ -63,6 +63,7 @@ public class ClientScopeResource {
protected ClientScopeModel clientScope; protected ClientScopeModel clientScope;
protected KeycloakSession session; protected KeycloakSession session;
protected static Pattern dynamicScreenPattern = Pattern.compile("[^\\s\\*]*\\*{1}[^\\s\\*]*"); protected static Pattern dynamicScreenPattern = Pattern.compile("[^\\s\\*]*\\*{1}[^\\s\\*]*");
protected final static Pattern scopeNamePattern = Pattern.compile("[\\x21\\x23-\\x5B\\x5D-\\x7E]+");
public ClientScopeResource(RealmModel realm, AdminPermissionEvaluator auth, ClientScopeModel clientScope, KeycloakSession session, AdminEventBuilder adminEvent) { public ClientScopeResource(RealmModel realm, AdminPermissionEvaluator auth, ClientScopeModel clientScope, KeycloakSession session, AdminEventBuilder adminEvent) {
this.realm = realm; this.realm = realm;
@ -187,12 +188,21 @@ public class ClientScopeResource {
} }
} }
public static void validateClientScopeName(String name) throws ErrorResponseException {
if (!scopeNamePattern.matcher(name).matches()) {
String message = String.format("Unexpected name \"%s\" for ClientScope", name);
throw new ErrorResponseException(ErrorResponse.error(message, Response.Status.BAD_REQUEST));
}
}
/** /**
* Makes sure that an update that makes a Client Scope Dynamic is rejected if the Client Scope is assigned to a client * Makes sure that an update that makes a Client Scope Dynamic is rejected if the Client Scope is assigned to a client
* as a default scope. * as a default scope.
* @param rep the {@link ClientScopeRepresentation} with the changes from the frontend. * @param rep the {@link ClientScopeRepresentation} with the changes from the frontend.
*/ */
public void validateDynamicScopeUpdate(ClientScopeRepresentation rep) { public void validateDynamicScopeUpdate(ClientScopeRepresentation rep) {
validateClientScopeName(rep.getName());
// Only check this if the representation has been sent to make it dynamic // Only check this if the representation has been sent to make it dynamic
if (rep.getAttributes() != null && rep.getAttributes().getOrDefault(ClientScopeModel.IS_DYNAMIC_SCOPE, "false").equalsIgnoreCase("true")) { if (rep.getAttributes() != null && rep.getAttributes().getOrDefault(ClientScopeModel.IS_DYNAMIC_SCOPE, "false").equalsIgnoreCase("true")) {
Optional<String> scopeModelOpt = realm.getClientsStream() Optional<String> scopeModelOpt = realm.getClientsStream()

View file

@ -18,7 +18,6 @@ package org.keycloak.services.resources.admin;
import org.jboss.logging.Logger; import org.jboss.logging.Logger;
import org.jboss.resteasy.annotations.cache.NoCache; import org.jboss.resteasy.annotations.cache.NoCache;
import javax.ws.rs.NotFoundException;
import org.jboss.resteasy.spi.ResteasyProviderFactory; import org.jboss.resteasy.spi.ResteasyProviderFactory;
import org.keycloak.events.admin.OperationType; import org.keycloak.events.admin.OperationType;
import org.keycloak.events.admin.ResourceType; import org.keycloak.events.admin.ResourceType;
@ -34,6 +33,7 @@ import org.keycloak.services.resources.admin.permissions.AdminPermissionEvaluato
import javax.ws.rs.Consumes; import javax.ws.rs.Consumes;
import javax.ws.rs.GET; import javax.ws.rs.GET;
import javax.ws.rs.NotFoundException;
import javax.ws.rs.POST; import javax.ws.rs.POST;
import javax.ws.rs.Path; import javax.ws.rs.Path;
import javax.ws.rs.PathParam; import javax.ws.rs.PathParam;
@ -94,6 +94,7 @@ public class ClientScopesResource {
@NoCache @NoCache
public Response createClientScope(ClientScopeRepresentation rep) { public Response createClientScope(ClientScopeRepresentation rep) {
auth.clients().requireManageClientScopes(); auth.clients().requireManageClientScopes();
ClientScopeResource.validateClientScopeName(rep.getName());
ClientScopeResource.validateDynamicClientScope(rep); ClientScopeResource.validateDynamicClientScope(rep);
try { try {
ClientScopeModel clientModel = RepresentationToModel.createClientScope(session, realm, rep); ClientScopeModel clientModel = RepresentationToModel.createClientScope(session, realm, rep);

View file

@ -17,7 +17,6 @@
package org.keycloak.testsuite.admin.client; package org.keycloak.testsuite.admin.client;
import org.hamcrest.MatcherAssert;
import org.junit.Assert; import org.junit.Assert;
import org.junit.Test; import org.junit.Test;
import org.keycloak.admin.client.resource.ClientResource; import org.keycloak.admin.client.resource.ClientResource;
@ -76,6 +75,46 @@ import static org.keycloak.testsuite.Assert.assertNames;
*/ */
public class ClientScopeTest extends AbstractClientTest { public class ClientScopeTest extends AbstractClientTest {
@Test
public void testAddFailureWithInvalidScopeName() {
ClientScopeRepresentation scopeRep = new ClientScopeRepresentation();
scopeRep.setName("マルチバイト");
ErrorRepresentation error;
try (Response response = clientScopes().create(scopeRep)) {
Assert.assertEquals(400, response.getStatus());
error = response.readEntity(ErrorRepresentation.class);
}
Assert.assertEquals("Unexpected name \"マルチバイト\" for ClientScope", error.getErrorMessage());
}
@Test
public void testUpdateFailureWithInvalidScopeName() {
// Creating first
ClientScopeRepresentation scopeRep = new ClientScopeRepresentation();
scopeRep.setName("scope1");
String scope1Id = createClientScope(scopeRep);
// Assert created
scopeRep = clientScopes().get(scope1Id).toRepresentation();
Assert.assertEquals("scope1", scopeRep.getName());
// Test updating
scopeRep.setName("マルチバイト");
try {
clientScopes().get(scope1Id).update(scopeRep);
} catch (ClientErrorException e) {
ErrorRepresentation error;
try (Response response = e.getResponse()) {
Assert.assertEquals(400, response.getStatus());
error = response.readEntity(ErrorRepresentation.class);
}
Assert.assertEquals("Unexpected name \"マルチバイト\" for ClientScope", error.getErrorMessage());
}
removeClientScope(scope1Id);
}
@Test @Test
public void testAddDuplicatedClientScope() { public void testAddDuplicatedClientScope() {
ClientScopeRepresentation scopeRep = new ClientScopeRepresentation(); ClientScopeRepresentation scopeRep = new ClientScopeRepresentation();
@ -359,7 +398,7 @@ public class ClientScopeTest extends AbstractClientTest {
assertNames(scopesResource.clientLevel(roleContainerClientUuid).listEffective(), "client-composite", assertNames(scopesResource.clientLevel(roleContainerClientUuid).listEffective(), "client-composite",
"client-child"); "client-child");
} }
// KEYCLOAK-2809 // KEYCLOAK-2809
@Test @Test
public void testRemoveScopedRole() { public void testRemoveScopedRole() {