fix: re-check setup completion before creating root admin#60
Merged
Conversation
OnInitializedAsync guarded only the initial render, and the external SetupRedirectMiddleware only blocks fresh GETs of /Setup once an admin exists — neither re-runs for a form submit on an already-open Blazor circuit. A circuit opened during the first-run window could therefore be used to mint a second root administrator after setup completed. Re-assert SetupCompletion.IsCompleteAsync at the top of OnValidSubmit, immediately before creating the user, and fail closed (notify + redirect to /) if setup already happened. This is the defence-in-depth re-check the identity package's SetupRedirectMiddleware explicitly expects the host Setup page to perform. Closes #52
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.
Closes #52 (F3 —
/Setupbootstrap allows root-admin takeover race).What
OnValidSubmitinSetup.razornow re-assertsSetupCompletion.IsCompleteAsyncimmediately before creating the root administrator, and fails closed (info notification + redirect to/) if setup already happened.Why
The completion guard previously ran only in
OnInitializedAsync(initial render). The externalSetupRedirectMiddlewareblocks fresh HTTP GETs of/Setuponce an admin exists, but neither guard re-runs for a form submit on an already-open Blazor Server circuit. A circuit opened during the first-run deployment window could therefore be driven to mint a second root administrator after legitimate setup completed.Re-checking inside the submit closes that window — the create now happens only if no administrator holds the role at submit time.
Why this approach
SetupRedirectMiddlewareinAndreGoepel.Marten.Identitydocuments this exact expectation:SetupCompletion.IsCompleteAsyncreturns true only once a non-deleted user actually holds the Administrator role, so the re-check reflects real completion state, not merely "some data exists."Scope note
This eliminates the practical vectors (retained circuit, re-run after completion). A sub-millisecond race between two simultaneous first-run submits (before any admin exists) is not meaningfully an escalation — there is no admin to protect yet — and is further addressed by the separately-tracked hardening of gating
/Setupbehind a one-time bootstrap secret / loopback.Verification
dotnet build -c Release— 0 errors.dotnet test -c Release— 54/54 passing.Setup.razor(+17).Independent of #59 (password-policy fix), which touches a different region of the same file.