feat: enforce domain-layer authorization on privileged store operations (#69)#75
Merged
Merged
Conversation
The stores performed privileged mutations with no authorization of their own — the only gate was the page-level [Authorize]. A forgotten attribute, a permissive host fallback policy, or any caller reaching the store directly was a privilege-escalation path (self-assign Administrator → takeover). This is #41's tracked root cause. Add defense-in-depth authorization in the domain layer via a new IIdentityAuthorizer: - DB-authoritative admin check (queries the live projection, not claims), failing closed for an unidentified caller. - Guard the operations that are never legitimately anonymous: * UserStore.AddToRoleAsync / RemoveFromRoleAsync — require Administrator (throw IdentityAuthorizationException). * UserStore.DeleteAsync — require Administrator or account ownership. * UserStore.RestoreAsync and all RoleStore mutations — require Administrator (return IdentityResult.Failed "NotAuthorized"). - BeginSystemScope() — an explicit, ambient escape hatch so trusted server-side code (seeding, bootstrap) can bypass the checks. CreateAsync and UpdateAsync are intentionally left unguarded: ASP.NET Identity drives them through anonymous flows (registration, password reset, email confirmation, lockout), where the authorization is the token at the UI layer. First-run root creation appends the admin role via direct events (not the guarded methods), so bootstrap needs no pre-existing admin. Adds UserStoreAuthorizationTests (deny non-admin/anonymous, allow admin/self, system-scope bypass); existing persistence tests run under a permissive authorizer. Updates THREAT-MODEL.md.
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.
Fixes #69 (the domain-layer half of #41).
Problem
UserStore/RoleStoreperformed privileged mutations (assign role, delete, restore, role management) with no authorization of their own — the only gate was the page-level[Authorize]. A single forgotten attribute, a permissive hostFallbackPolicy, or any caller reaching a store directly became a privilege-escalation path (self-assignAdministrator→ full takeover). This is #41's tracked root cause, previously closed without a domain-layer fix.Approach
New
IIdentityAuthorizerprovides a defense-in-depth check independent of the UI:UserRoleAssignment/Roleprojection (not claims; per [architecture] Default-deny authorization + defense-in-depth in the domain layer #41 "the acting identity from claims proves identity, never authority"), and fails closed for an unidentified (Guid.Empty) caller.UserStore.AddToRoleAsync/RemoveFromRoleAsync→ requireAdministrator(throwIdentityAuthorizationException; these returnTask). This is the headline escalation vector.UserStore.DeleteAsync→ requireAdministratoror account ownership.UserStore.RestoreAsyncand allRoleStoremutations → requireAdministrator(returnIdentityResult.Failed/NotAuthorized).BeginSystemScope()— an explicit, ambient (AsyncLocal) escape hatch so trusted server-side code (seeding, background provisioning, alternative bootstrap) can bypass the checks deliberately.Why
CreateAsync/UpdateAsyncare not guardedASP.NET Identity legitimately drives them through anonymous flows — registration, password reset, email confirmation, failed-login lockout — with no authenticated principal. Their real authorization is the reset/confirmation token at the UI layer; a role/ownership check in the domain layer would break those flows. This is documented in
THREAT-MODEL.md, and the stale-UpdateAsync-overwrite concern remains tracked in #70.Why bootstrap doesn't deadlock
First-run root creation runs through
CreateAsync, which appends theRoleAssigned/RoleCreatedevents directly (not via the guarded methods), so the first admin is created without needing a pre-existing admin.Tests
UserStoreAuthorizationTests(9 cases): non-admin and anonymous actors are denied (role ops throw; delete/restore returnNotAuthorized); admin and self are allowed;BeginSystemScope()bypasses.RemoveFromRole_RootUserAdministrator_IsRefusedstill reaches the root invariant.Out of scope (documented)
Page-level default-deny (
FallbackPolicy/AuthorizeRouteView) remains a host obligation — this PR is the domain-layer defense-in-depth half.