Repository navigation
Conversation
Group names submitted via the Admin REST API could contain control characters (Unicode general category Cc, e.g. NUL) when submitted as JSON escapes. Such names were stored verbatim and round-tripped through the API, which breaks PostgreSQL deployments (NUL bytes are not allowed in text values) and silently breaks exact string matching as the characters are invisible in the Admin UI and propagate into token claims. Group create (top-level and sub-group) and rename requests are now rejected with 400 Bad Request when the name contains a control character. Names of existing (legacy) groups are only validated when actually changed, and moving a group does not validate its name. Closes keycloak#54721
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Both legacy-name tests fail during setup on PostgreSQL because NUL-containing group names are rejected on flush, preventing the update and move checks from running.
1 open issue
What changed in this PR
This PR addresses #54721 by rejecting control characters in group names submitted through the Admin REST API, while preserving updates and moves that leave existing names unchanged.
Changes:
- Adds a shared Unicode category
Ccvalidation helper. - Validates names during group creation, subgroup creation, and renaming.
- Adds six tests covering rejection, visible Unicode names, and legacy-name compatibility.
| File | Description |
|---|---|
tests/base/src/test/java/org/keycloak/tests/admin/group/GroupTest.java |
Adds validation and backward-compatibility tests. |
services/src/main/java/org/keycloak/services/validation/Validation.java |
Adds control-character detection. |
services/src/main/java/org/keycloak/services/resources/admin/GroupsResource.java |
Validates new top-level group names without blocking moves. |
services/src/main/java/org/keycloak/services/resources/admin/GroupResource.java |
Validates renames and new subgroup names. |
Review effort: Balanced
Comment on lines
+464
to
+473
| GroupModel legacyGroup = realm.createGroup("legacy\u0000Group"); | ||
| return legacyGroup.getId(); | ||
| }, String.class); | ||
| managedRealm.cleanup().add(r -> r.groups().group(groupId).remove()); | ||
|
|
||
| GroupRepresentation rep = new GroupRepresentation(); | ||
| rep.setName("legacy\u0000Group"); | ||
| managedRealm.admin().groups().group(groupId).update(rep); | ||
|
|
||
| assertEquals("legacy\u0000Group", managedRealm.admin().groups().group(groupId).toRepresentation().getName()); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no chan
3226
ges were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #54721
What
Group names submitted via the Admin REST API are now checked for control characters (Unicode general category
Cc): NUL (U+0000), the other C0 controls, DEL (U+007F) and the C1 controls (U+0080-U+009F). Requests that create or rename a group with such a name are rejected with400 Bad Requestand the messageGroup name contains control characters.Why
As described in #54721, such characters are accepted today when submitted as JSON escapes (a raw NUL byte is rejected by the JSON parser, but the escaped form passes through). They are stored verbatim and round-trip through the Admin API:
Where
Validation.containsControlCharacters(...)- checks every code point againstCharacter.CONTROL(Unicode categoryCc)POST /admin/realms/{realm}/groups(top-level create - validated only when creating; requests that move an existing group are unaffected)POST /admin/realms/{realm}/groups/{id}/children(subgroup create - same creation-only scope)PUT /admin/realms/{realm}/groups/{id}(rename - checked only when the name actually changes)Note: PR #54719 addresses the same family of issues for invisible formatting characters (category
Cf, tracked in #54532). This PR is independent of it and can be merged in either order; if both are applied, the checks complement each other.Backward compatibility / trade-offs
updatingLegacyGroupWithControlCharactersInNameSucceeds), and moving such a group via the move endpoints is also allowed, since those requests do not apply the name (covered bymoveLegacyGroupWithControlCharactersInNameSucceeds).Ccis rejected (covered bycreateGroupWithVisibleSpecialCharactersNameSucceeds).Testing
GroupTest:createGroupWithControlCharacterNameShouldFail- NUL, DEL and a C1 control on the create path, including the error messagerenamingGroupToControlCharacterNameShouldFail- rename pathcreateSubGroupWithControlCharacterNameShouldFail- subgroup pathcreateGroupWithVisibleSpecialCharactersNameSucceeds- no over-blocking of visible charactersupdatingLegacyGroupWithControlCharactersInNameSucceeds- legacy names can still be managed without renamingmoveLegacyGroupWithControlCharactersInNameSucceeds- move endpoints do not validate unchanged legacy namesGroupTestclass passes locally (40 tests, 0 failures).