Add client ID length validation (#35725)

Closes #35723

Signed-off-by: Rungsikorn Rungsikavarnich <rungsikorn@me.com>
This commit is contained in:
Rungsikorn Rungsikavanich 2024-12-19 17:19:59 +07:00 committed by GitHub
parent c81246e2d0
commit 41696b964b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 26 additions and 1 deletions

View File

@ -62,6 +62,7 @@ import org.keycloak.models.GroupProvider;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.ModelDuplicateException;
import org.keycloak.models.ModelException;
import org.keycloak.models.ModelValidationException;
import org.keycloak.models.RealmModel;
import org.keycloak.models.RealmProvider;
import org.keycloak.models.RoleContainerModel;
@ -786,6 +787,8 @@ public class JpaRealmProvider implements RealmProvider, ClientProvider, ClientSc
if (id == null) {
id = KeycloakModelUtils.generateId();
} else if (id.length() > ClientEntity.ID_MAX_LENGTH){
throw new ModelValidationException("Client ID must not exceed 36 characters");
}
if (clientId == null) {

View File

@ -60,8 +60,10 @@ import java.util.Set;
})
public class ClientEntity {
public static final int ID_MAX_LENGTH = 36;
@Id
@Column(name="ID", length = 36)
@Column(name="ID", length = ID_MAX_LENGTH)
@Access(AccessType.PROPERTY) // we do this because relationships often fetch id, but not entity. This avoids an extra SQL
private String id;
@Nationalized

View File

@ -32,6 +32,7 @@ import org.keycloak.events.admin.ResourceType;
import org.keycloak.models.ClientModel;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.ModelDuplicateException;
import org.keycloak.models.ModelValidationException;
import org.keycloak.models.RealmModel;
import org.keycloak.models.UserModel;
import org.keycloak.models.utils.ModelToRepresentation;
@ -223,6 +224,8 @@ public class ClientsResource {
throw ErrorResponse.exists("Client " + rep.getClientId() + " already exists");
} 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);
}
catch (ClientTypeException cte) {
throw ErrorResponse.error(cte.getMessage(), cte.getParameters(), Response.Status.BAD_REQUEST);

View File

@ -170,6 +170,23 @@ public class ClientTest extends AbstractAdminTest {
Assert.assertNames(realm.clients().findAll(), "account", "account-console", "realm-management", "security-admin-console", "broker", "my-app", Constants.ADMIN_CLI_CLIENT_ID);
}
@Test
public void testInvalidLengthClientIdValidation() {
ClientRepresentation rep = new ClientRepresentation();
rep.setId("test-long-invalid-client-id-validation-400-bad-request");
rep.setClientId("invalid-client-id-app");
rep.setDescription("invalid-client-id-app description");
rep.setEnabled(true);
rep.setPublicClient(true);
try (Response response = realm.clients().create(rep)) {
if (response.getStatus() != 400) {
response.bufferEntity();
String body = response.readEntity(String.class);
fail("expect 400 Bad request response code but receive: " + response.getStatus() + "\n" + body);
}
}
}
@Test
public void testInvalidUrlClientValidation() {
testClientUriValidation("Root URL is not a valid URL",