You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
RoleClaims has no unique constraint on (RoleId, ClaimType, ClaimValue): concurrent permission syncs insert duplicate rows #1435
identity.RoleClaims has no uniqueness on (RoleId, ClaimType, ClaimValue). Its only index is on RoleId (IdentityDbContextModelSnapshot, entity FshRoleClaim), so nothing in the database stops the same permission from being stored twice on the same role.
Both writers that grant permissions to the built-in roles follow a read-then-insert pattern without a lock:
RolePermissionSyncer.SyncRoleAsync loads the role's permission claims, computes the missing ones and inserts them.
IdentityDbInitializer.AssignPermissionsToRoleAsync does the same during seeding.
When two API instances start at the same time after a release that registers a new permission, both read the role before either one commits, both see the permission as missing, and both insert it. The result is a duplicate row, with no exception and no log line that would point at it. The same applies when the seeder and the startup sync overlap on a fresh tenant.
Impact
Authorization is not affected today: the effective permission set is built as a set, the sync reads existing claims into a HashSet, and when a permission is later revoked the sync deletes every matching row. The cost is silent data drift in RoleClaims, which grows by one row per permission per concurrent start, and any future code that counts claims or relies on one row per grant would be wrong.
This came up in the review of #1426 (point 2, two instances starting at once). That PR handles the delete side of the race (DbUpdateConcurrencyException is caught and logged at Information); the insert side needs a schema change, so it was left out of that PR.
Proposed fix
A migration that first removes existing duplicates (keeping the lowest Id per (RoleId, ClaimType, ClaimValue)) and then adds a unique index on (RoleId, ClaimType, ClaimValue). ClaimValue is nullable, and PostgreSQL treats NULLs as distinct in a unique index, so the null-valued claims the sync already removes do not block the index.
The two writers treat a unique violation on that index (23505) the same way the sync treats a lost delete race: another writer already inserted the row, so the end state is correct and it is logged at Information, not as an error.
An integration test that inserts the same permission twice on a role and asserts the second insert is rejected, plus a test that the sync survives the conflict.
Expand-then-contract is not needed here, because the index only rejects rows that should never have existed; but the dedupe step must run before the index is created, inside the same migration.
Activity
fix(identity): enforce one row per role claim grant (fullstackhero#1458)