Skip to content

fix: hold profile and group names unique in the database - #826

Merged
blaipr merged 1 commit into
mainfrom
fix/a-profile-and-group-name-are-unique-in-the-database
Aug 18, 2026
Merged

fix: hold profile and group names unique in the database#826
blaipr merged 1 commit into
mainfrom
fix/a-profile-and-group-name-are-unique-in-the-database

Conversation

@blaipr

@blaipr blaipr commented Aug 18, 2026

Copy link
Copy Markdown
Member

UserProfile and UserGroup each refuse a name that is already taken:

->where('UPPER(:name) = UPPER(name)', ['name' => $userGroup->getName()])

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

name collates utf8mb4_unicode_ci, so the index is case-insensitive — 'Admins' and 'ADMINS' collide, which is what the UPPER() comparison was asking for. Verified against a server rather than assumed, with the whole of dbstructure.sql reloaded to check nothing else objects:

INSERT INTO UserGroup (name) VALUES ('Admins');
INSERT INTO UserGroup (name) VALUES ('ADMINS');
  -> ERROR 1062 Duplicate entry 'ADMINS' for key 'uk_UserGroup_01'

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:

table the application's rule index
Category, Client, Tag hash of the name already unique (Client fixed in #806)
PublicLink hash, one link per account uk_PublicLink_01, _02
AuthToken token per action (token, actionId) — composite is its rule
User login fixed in #825
UserProfile, UserGroup name, case-insensitive this change

SchemaEnforcesIdentityTest now 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.

`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
blaipr merged commit 161bbfb into main Aug 18, 2026
8 checks passed
@blaipr
blaipr deleted the fix/a-profile-and-group-name-are-unique-in-the-database branch August 18, 2026 23:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant