feat(group): Sanitize group names and ids on creation#56222
Merged
Conversation
artonge
requested review from
ArtificialOwl,
CarlSchwan,
icewind1991 and
yemkareems
and removed request for
a team
artonge
added
enhancement
3. to review
artonge
force-pushed
the
artonge/feat/sanitize_groupname
branch
from
fda3b6b to
21b5f96
Compare
solracsf
reviewed
Nov 5, 2025
It does not make sense to allow group name with weird white space sequence going forward. Same for group ids, in which we do not really want white space. Signed-off-by: Louis Chmn <louis@chmn.me>
artonge
force-pushed
the
artonge/feat/sanitize_groupname
branch
from
21b5f96 to
9890117
Compare
tcitworld
reviewed
Nov 5, 2025
| return mb_strlen($displayName) > 64 | ||
| ? hash('sha256', $displayName) | ||
| : $displayName; | ||
| $displayNameWithoutWhitespace = preg_replace('/\s+/', '_', $displayName); |
Member
There was a problem hiding this comment.
Maybe the opportunity to be even more restrictive on the GID sanitation. Should it contain special Unicode characters, quotes, accentuation, emoji?
Member
There was a problem hiding this comment.
Just to be clear, this is only for new groups. We still have to support all those other things on already existing groups
nickvergessen
approved these changes
Nov 6, 2025
come-nc
approved these changes
Nov 6, 2025
Member
|
This is a breaking behavioral change. At the very least it must be added to critical changes. For instance it breaks SAML integration tests, and there might be users who automate group provisioning and may run into it as well. But I would recommend to revert it. Logic has to be kept in place, foreign backends are not effected. This causes more problems than it solves. |
Merged
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
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes 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.
It does not make sense to allow group name with weird white space sequence going forward.
Same for group ids, in which we do not really want white space.