fix: hold profile and group names unique in the database - #826
Merged
blaipr merged 1 commit intoAug 18, 2026
Merged
Conversation
`UserProfile` and `UserGroup` each refuse a name that is already taken — `UPPER(:name) = UPPER(name)` in `checkDuplicatedOnAdd()` — and neither table had a unique index at all. That check is a SELECT followed by an INSERT, so two requests arriving together both find nothing and both insert, with nothing underneath to stop the second: two groups called Admins, and no way to tell which one a permission refers to. A plain unique index is the application's rule exactly, because `name` collates utf8mb4_unicode_ci — 'Admins' and 'ADMINS' collide, which is what the UPPER() comparison was asking for. Confirmed against a server, with the whole schema reloaded to check nothing else objects. This completes the sweep the User login started. Every `checkDuplicated*` in the repositories now has an index behind it: Category, Client and Tag by hash, PublicLink by hash and by account, AuthToken by (token, actionId) — a composite because that is its rule — User by login, and these two by name. The schema test covers all five through one data provider, so a table added later with a PHP-only uniqueness rule is caught by adding a row to it. Schema only, as in #806 and #825: which of two identically named groups is the real one, and what becomes of the permissions pointing at the other, is an administrator's decision rather than a migration's.
blaipr
deleted the
fix/a-profile-and-group-name-are-unique-in-the-database
branch
August 18, 2026 23:06
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 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.
UserProfileandUserGroupeach refuse a name that is already taken:Neither table had a unique index at all. That check is a SELECT followed by an INSERT, so two requests arriving together both find nothing and both insert, and there was nothing underneath to stop the second — two groups called
Admins, with no way to tell which one a permission refers to.A plain unique index is the rule exactly
namecollatesutf8mb4_unicode_ci, so the index is case-insensitive —'Admins'and'ADMINS'collide, which is what theUPPER()comparison was asking for. Verified against a server rather than assumed, with the whole ofdbstructure.sqlreloaded to check nothing else objects:Distinct names are unaffected, and the fixtures hold no duplicates by either spelling.
This completes the sweep
Every
checkDuplicated*in the repositories now has an index behind it:Category,Client,TagClientfixed in #806)PublicLinkuk_PublicLink_01,_02AuthToken(token, actionId)— composite is its ruleUserUserProfile,UserGroupSchemaEnforcesIdentityTestnow covers all five through one data provider, so a table added later with a PHP-only uniqueness rule is caught by adding a row to it rather than by an incident. Removing either new key fails it with Only a unique index stops the second one.Schema only
As in #806 and #825: which of two identically named groups is the real one, and what becomes of the permissions pointing at the other, is an administrator's decision rather than a migration's. A fresh install gets the constraint.
3979 unit tests pass; the integration suite is running against a freshly reloaded schema so the new constraints are exercised rather than merely declared.