fix(identity): make the system-role permission sync remove revoked claims

[marcelo-maciel]

Closes #1417. Docs: fullstackhero/docs#266.

Problem

RolePermissionSyncer only ever added claims. On every start it inserted the permissions missing from the built-in Basic and Admin roles and never removed one, so a permission that stopped being IsBasic, or that a module stopped registering, stayed on those roles in every tenant that already existed. Every user of those tenants kept a permission the code no longer granted. An operator could not fix it either: RoleService.UpdatePermissionsAsync refuses permission edits on system roles, so the only way out was a manual DELETE on RoleClaims plus a permission-cache flush. Narrowing what every user can do is a security change, and today it silently did not apply to existing tenants.

Fix

The sync is now authoritative for the two framework-managed roles. For each role it loads the role's claims of type permission and reconciles them with the role's target set from the catalog: PermissionConstants.Basic for Basic, PermissionConstants.Admin for Admin, and Admin plus PermissionConstants.Root for the root tenant's Admin, the same targets the sync already used for additions. Missing permissions are inserted as before, and permission claims outside the target set are removed, in the same SaveChangesAsync. A permission claim with a null value grants nothing and is removed too. Claims of any other type on those roles are never read or touched, and custom roles are still left alone. Because Basic and Admin are locked against manual edits, there is no operator change the removal can lose.

The permission cache is invalidated on removals the same way it already was on additions: SyncRoleAsync now returns the number of claims it added plus the number it removed, and SyncAsync keeps calling RemoveByTagAsync(CacheKeys.Tags.Permissions) whenever that total is above zero. This matters because the sync runs at API start while the distributed HybridCache entries (Redis, one hour) and other instances can still be warm.

The class summaries of RolePermissionSyncer and RolePermissionSyncHostedService described the sync as add-only and are updated. Nothing in src/BuildingBlocks changes, and nothing new is configured.

Logging

Each removed claim is logged at Warning, one entry per claim, with a message template: Removed permission claim '{Permission}' from '{Role}' for tenant '{Tenant}': the permission catalog no longer grants it. A mistaken change to the permission constants is visible in the logs instead of silent. The existing Information line for added claims is unchanged and is now only written when something was added.

Tests

In Catalog/RolePermissionSyncerTests:

• SyncAsync_Should_Remove_Permission_Claims_The_Catalog_No_Longer_Grants seeds, in the root tenant, an unregistered Permissions.Retired.View on both Basic and Admin, an Admin-only catalog permission (Permissions.Roles.Delete) on Basic, which is what narrowing IsBasic leaves behind, and a claim of another type on Basic. After SyncAsync, Basic holds exactly the IsBasic permissions, Admin lost the retired claim but kept Permissions.Roles.Delete, and the non-permission claim is still there. The seeded rows are deleted in a finally so a failure cannot leak into the rest of the collection.
• SyncAsync_Should_Keep_Root_Permissions_On_Root_Tenant_Admin runs the sync twice on the root tenant and checks after each pass that every IsRoot permission is still on its Admin. Two passes are needed because a sync that adds and removes against different targets flips those claims on each run.

In Roles/PermissionCacheInvalidationTests, RolePermissionSync_Should_EvictCachedPermissions_When_It_RevokesAClaim gives a fresh user the Basic role, seeds a retired permission on Basic, warms the user's cache through GET /api/v1/identity/permissions (the permission is present), runs the sync, and asserts the next read no longer contains it.

Mutation gate, run against RolePermissionSyncerTests and PermissionCacheInvalidationTests (9 tests), restoring the syncer with git checkout HEAD and checking git diff --exit-code after each mutation:

Syncer	Result
Reverted to main (additive only)	2 failed (the removal test and the cache test), 7 passed
Removal target drops root permissions, add target keeps them	1 failed (the root test), 8 passed
Root tenant never recognised	1 failed (the root test), 8 passed
Cache gate counts only additions	1 failed (the cache test), 8 passed
This branch	9 passed, 0 failed

The root test passes on main too, as it should: it guards against over-deletion, not the bug.

Runs on this branch (.NET 10):

Project	Result
Architecture.Tests	55 passed, 0 failed
Identity.Tests	319 passed, 0 failed
Integration.Middleware.Tests	5 passed, 0 failed
Integration.Tests, Identity-related (Roles, Users, Authorization, Authentication, Groups, Sessions, Impersonation, RolePermissionSyncer)	186 passed, 0 failed

The test harness only provisions the root tenant, so the non-root Admin case (Root permissions must not be granted there, and would now be removed if present) is covered by the same code path but not by a dedicated test.

Behaviour change for operators: turning IsBasic off for a permission, or removing a permission from a module, now revokes it from Basic (or Admin) in every existing tenant at the next API start, logged at Warning per claim.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[iammukeshm]

Solid change, and the mutation table is thorough. I checked the main risk, the sync stripping Admin permissions by mistake. Within the kit it's safe: every module registers its permissions in ConfigureServices, so the catalog is complete before the sync runs; the DbMigrator removes all BackgroundServices; and root Admin keeps Root because add and remove use the same target set.

Because the sync now deletes rows in every tenant, based only on which modules the running process loaded, I'd like these guards in this PR, not after:

1. Guard against a partial catalog. Skip removal when a role's target set is empty, and add a switch such as Identity:PermissionSync:RemoveRevoked (default true). A consumer's second host (a worker, say) that registers IdentityModule with only some of the other modules would otherwise revoke the missing modules' permissions from Admin in every tenant on startup. Before this PR that setup was harmless.
2. Two instances starting at once. Both try to delete the same rows. The second SaveChangesAsync throws DbUpdateConcurrencyException (expected 1 row, got 0), which the per-tenant catch logs as an Error. The end state is correct because the first instance's write succeeded, so please catch that exception in SyncRoleAsync and log it at Debug/Information instead of raising a false alarm.
3. Test hygiene. In PermissionCacheInvalidationTests, wrap the seeded RetiredCacheProbe claim in try/finally. If the pre-check fails, it currently stays on root's shared Basic role and leaks into other tests.
4. Docs (#266). Document the switch from (1), and add one line to the upgrade note: during a rolling deploy, an old-version instance that restarts re-adds a revoked claim until the next new-version start.

Happy to merge as soon as these are in.