Repository navigation
Reject group names containing invisible Unicode characters - #54719
siddhant-pardeshi wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Unconditional name validation incorrectly returns HTTP 400 when moving existing groups with unchanged legacy names, so both endpoints need to restrict validation to group creation.
1 open issue
What changed in this PR
This PR addresses invisible Unicode characters in realm group names that can silently break UMA group-path matching. It adds validation to Admin REST API create and rename operations.
Changes:
- Adds a shared detector for Unicode format (
Cf) characters. - Rejects affected group names with HTTP 400, while allowing unchanged legacy names on updates.
- Adds five tests covering rejection, visible special characters, and legacy-name updates.
| File | Description |
|---|---|
tests/base/src/test/java/org/keycloak/tests/admin/group/GroupTest.java |
Adds validation and compatibility tests. |
services/src/main/java/org/keycloak/services/validation/Validation.java |
Adds code-point-based format-character detection. |
services/src/main/java/org/keycloak/services/resources/admin/GroupsResource.java |
Validates names in the top-level group endpoint. |
services/src/main/java/org/keycloak/services/resources/admin/GroupResource.java |
Validates names during renaming and subgroup creation. |
Review effort: Balanced
Group names containing invisible or formatting-only Unicode characters (Unicode general category Cf) such as zero-width space (U+200B), word joiner (U+2060), soft hyphen (U+00AD), byte order mark (U+FEFF) or bidi controls are now rejected on group creation, subgroup creation and rename via the Admin REST API with a 400 error. Such names were accepted silently but break exact string matching: they are invisible in the Admin UI and API, yet significant for UMA group-path comparison, causing authorization failures with no diagnostic hint (most commonly after copy-pasting names from word processors or web editors). Updates that leave the name of an existing group unchanged are still allowed, so realms with legacy names are not affected. Names with visible special characters (accents, CJK, emoji) remain valid. Closes keycloak#54532 Signed-off-by: siddhant-pardeshi <siddhantpardeshi307@gmail.com>
|
Good catch, thank you — confirmed and fixed. The move branches ( The validation now runs only when a new group is created ( |
0f92d13 to
ad62e3c
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The validation rejects legitimate joiners in Persian names and emoji, causing previously valid internationalized group names to receive 400 responses; targeted or contextual validation with regression tests is needed.
1 open issue
1 resolved since last review
Review effort: Balanced
| if (s == null) { | ||
| return false; | ||
| } | ||
| return s.codePoints().anyMatch(cp -> Character.getType(cp) == Character.FORMAT); |
|
Thank you for the review — this contains two different points, one factual correction and one legitimate design trade-off: On emoji: visible characters are not affected. Emoji are Unicode category On script-essential joiners (e.g. ZWNJ, U+200C, used in Persian orthography): this is a fair trade-off to raise. ZWNJ is category |
Closes #54532
This follows the proposal I posted on the issue (comment) — happy to adjust the approach per review, including normalization instead of rejection if preferred.
What
Group names submitted via the Admin REST API are now checked for invisible, formatting-only Unicode characters (Unicode general category
Cf): zero-width space (U+200B), word joiner (U+2060), soft hyphen (U+00AD), byte order mark (U+FEFF), bidi controls, etc. Requests that create or rename a group with such a name are rejected with400 Bad Requestand the messageGroup name contains invisible Unicode characters.Why
As described in #54532, such characters are accepted today without any warning. They are invisible in the Admin UI and API responses, but they are significant for exact string comparison — so a group that looks identical to another silently breaks UMA group-path matching (exact byte comparison), producing
403 access_deniedwith no diagnostic hint. The most common source is copy-pasting names from Word/Excel/web editors, which frequently carry U+2060 or U+200B.Where
Validation.containsInvisibleCharacters(...)— checks every code point againstCharacter.FORMAT(Unicode categoryCf)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)Backward compatibility / trade-offs
updatingLegacyGroupWithInvisibleCharactersInNameSucceeds).Cfis rejected (covered bycreateGroupWithVisibleSpecialCharactersNameSucceeds).Testing
GroupTest:createGroupWithInvisibleUnicodeNameShouldFail— word joiner, zero-width space, soft hyphen on the create pathrenamingGroupToInvisibleUnicodeNameShouldFail— rename pathcreateSubGroupWithInvisibleUnicodeNameShouldFail— subgroup pathcreateGroupWithVisibleSpecialCharactersNameSucceeds— no over-blocking of visible charactersupdatingLegacyGroupWithInvisibleCharactersInNameSucceeds— legacy names can still be managedmainand pass with this change../mvnw install -DskipTests— BUILD SUCCESS;./mvnw -f tests/base/pom.xml -Dtest=GroupTest test— Tests run: 39, Failures: 0, Errors: 0, Skipped: 0 (34 pre-existing + 5 new).AI disclosure
Code changes were prepared with the assistance of an AI coding agent; the author reviewed, built and tested all changes locally and takes full responsibility for the contribution (per the project's AI contribution guidelines).