docs(auth): design for refusing login to resigned accounts (#8072)

[renemadsen]

Design only — no code. Closes nothing yet; implementation follows as four gated PRs.

Why

Resigned is a visibility flag, not an access flag. It gates ~15 pickers, lists and reports, and is read by zero authentication or authorisation code (grep -rni resigned over eFormAPI.Web, eFormApi.BasePn and eform-angular-frontend-base → 0 hits).

Measured on tenant 1012: all four resigned workers still hold a working login — password set, no lockout — and a live device Unit. One resigned seven weeks ago.

The same change closes the account-enumeration oracle: unknown account, wrong password and refused account all return one identical message, "You have entered an invalid username or password". Today they return three distinguishable hard-coded strings, one of which echoes the submitted username back.

Approach

Give the login account its own state (EformUser.IsActive), written by the resign action — rather than resolving a login to an SDK Worker by email at authentication time. There is no FK between Users and the SDK; the join is an email string across two databases that FixUserToWorkerLinks() rewrites by name match on every host boot (Startup.cs:492-519). Putting an auth decision on that join means a resigned person gets in whenever the match drifts, and an active person is locked out whenever a name collides.

Fact-checked

The spec's claims and line references were checked against the code before this PR. Corrections folded in:

• EformUser is declared in eFormApi.BasePn, not in this repo's base — only the migration belongs in eform-angular-frontend-base
• there is a second resign path (TimeSettingService.UpdateAssignedSite in the time-planning plugin) that never touches a login
• UpdateDeviceUser's skipLoginWork rule sets user = null for non-plugin-managed accounts, which would have silently skipped exactly the admin logins worth disabling
• SecurityGroupBackfillService is a one-time marker-gated sweep, so it is hardening rather than the live hole an earlier draft described
• the resource key already exists with different wording; the time-planning gRPC path cannot reach the host's SharedResource at all
• EF Core's sentinel behaviour for a non-nullable bool default, which fails silently in the insecure direction if only half-applied

Rollout

Four PRs, each gated on the previous package being released and restorable — no step starts against an unreleased package, because it could not be tested. Deploy is host-before-plugins: plugins share BasePn types across load contexts, so a version skew is a runtime MissingMethodException, not a build error.

Ending already-issued sessions is out of scope and tracked in #8071 — blocking login does not end a 24h token that /api/auth/token/refresh will roll forward indefinitely.

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

Unresolved critical design and rollout safeguards remain.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Design-only proposal for refusing login to resigned accounts and unifying authentication failures.

Changes:

• Defines EformUser.IsActive, migration, and backfill strategy.
• Covers login, refresh, Google Auth, resignation paths, and tests.
• Documents rollout sequencing, risks, and live-session follow-up.

File summaries

File

Findings

docs/superpowers/specs/2026-09-17-resigned-account-login-refusal-design.md

Critical: Configure EF Core’s sentinel and test explicit false inserts (3 votes); preserve independent support disables (2); enforce unambiguous one-to-one bulk matches (1); avoid restart-dependent enforcement (2); gate refusal until backfill is complete (1). Moderate: Use the generic response for lockouts (3); route all normal authentication branches through the shared key (2); expand authentication and Google Auth tests (2). Nit: Preserve localized resource translations (1).

Review details

Suppressed comments (2)

docs/superpowers/specs/2026-09-17-resigned-account-login-refusal-design.md:182

• Plugin startup runs from UseEFormPlugins, while this host calls FixUserToWorkerLinks() afterwards (Startup.cs:396-404). If the backfill joins by worker email as described, it can run against the stale link before the host repairs it and leave a resigned account active. Define/run the backfill after that repair or use a stable, independent match.

It runs in the **BackendConfiguration plugin's startup**. Two existing sites are nearby but
they are in *different methods*: the `SecurityGroupBackfillService` invocation
(`EformBackendConfigurationPlugin.cs:903`, inside the try at `:899-908`) and the
resigned-flag reconciliation (`:701-726`, inside `SeedEForms` at `:252`). Put the backfill
next to the reconciliation — that is where worker state is already reconciled across the
two databases and where the SDK context is to hand. (`eFormAPI.Web` can also see
`Worker.Resigned`: it already builds an SDK context at `Startup.cs:498-500`.)

docs/superpowers/specs/2026-09-17-resigned-account-login-refusal-design.md:152

• The resource entries are localized today—for example, SharedResource.da.resx:326-328 is Danish—so “update that key's text to the wording above” is ambiguous. Replacing all 25 values with the English sentence would regress supported locales; preserve per-locale translations while reusing the key for REST unknown, wrong-password, and inactive results, or explicitly choose an invariant English contract.

The key **already exists**: `UserNameOrPasswordIncorrect`, in all 25 `SharedResource*.resx`
files, EN *"Username or password is incorrect"* (`SharedResource.resx:384-386`), DA
*"Brugernavn eller adgangskode er forkert"* (`SharedResource.da.resx:326-328`), already
used by `GetGoogleAuthenticator` (`AuthService.cs:415, :431`). Update that key's text to
the wording above rather than adding a near-duplicate — which also changes the

• Files reviewed: 1/1 changed files
• Comments generated: 8
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

Critical and moderate design gaps remain in authentication coverage, synchronization, reconciliation, and validation.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (14)

Previously missed (1) — in code that hasn't changed since the last review.

docs/superpowers/specs/2026-09-17-resigned-account-login-refusal-design.md:298

• §5 says this backfill is a one-off and a no-op on every subsequent boot, so it cannot heal a later resignation write that fails after deployment. In that failure mode the warning only records an active login for a resigned worker until manual repair. Make reconciliation run on every boot/retry failed writes, or define an explicit operational repair path.

docs/superpowers/specs/2026-09-17-resigned-account-login-refusal-design.md:74

• HasDefaultValue(true) does not make true the EF sentinel for a non-nullable bool; EF uses the CLR default (false) unless a sentinel is explicitly configured. With the initializer, true is written explicitly, while without it false is omitted and the database default applies. The current explanation reverses this behavior and could lead the implementation astray.

mapping declares `HasDefaultValue(true)`, and that is what makes `true` this property's
sentinel; EF Core omits a sentinel-valued column from the INSERT so the store default
applies. Without the initializer a freshly constructed `EformUser` holds `false`, EF Core
writes it explicitly, and **every new account is created disabled** — fail-closed, a
lockout of all new users. Verified against EF Core 10.0.12, the pinned version.

docs/superpowers/specs/2026-09-17-resigned-account-login-refusal-design.md:128

• The boot-time fallback does not enforce the stated requirement at resignation time: until the next host restart, the account remains IsActive = true and can still log in. Require Path B to update the host account synchronously (or fail/raise an operationally visible error) instead of accepting an indefinite enforcement delay.

So the flag must be written in **both** paths, or a resignation made through time-planning
leaves `IsActive = true` with nothing logged. Path B is in a different plugin, which may
not have `UserManager` to hand — **verify this before implementing PR 4**; if it does not,
the fallback is to write `IsActive` in the boot-time reconciliation at `:701-726`, which
already sees both paths, accepting that it only takes effect at the next restart.

docs/superpowers/specs/2026-09-17-resigned-account-login-refusal-design.md:131

• Because IsActive is described as an independent support disable flag, assigning it unconditionally from resigned can re-enable a manually disabled account whenever UpdateDeviceUser runs with resigned = false. Restrict this assignment to explicit resign/un-resign transitions or model the disable reason separately.

The write is **unconditional** (`IsActive = !resigned`), not `if (resigned)`, so that
un-resigning restores access through the same code.

docs/superpowers/specs/2026-09-17-resigned-account-login-refusal-design.md:190

• A distinct lockout response still leaks account existence: an attacker can submit wrong passwords until CheckPasswordSignInAsync(..., lockoutOnFailure: true) reports lockout, distinguishing a real username from an unknown one. That leaves the enumeration oracle in place despite §4's generic contract; keep lockout details server-side and return the same generic response externally.

Lockout keeps its own distinct message: it is a temporary, self-resolving state that users
need to understand, and it already reveals nothing that the generic message hides.

docs/superpowers/specs/2026-09-17-resigned-account-login-refusal-design.md:250

• The requirement is three-way equivalence, but this test only compares an inactive account with an unknown account. A regression could still leave the wrong-password response distinguishable, so include that response in the byte-identical assertion.

- a login whose account has `IsActive = false` is refused, and the response is
  **byte-identical** to the unknown-account response — this is the requirement, so it is
  asserted directly rather than by checking the message alone

docs/superpowers/specs/2026-09-17-resigned-account-login-refusal-design.md:251

• The read-path table includes the anonymous google-auth-key credential check, but the host test list has no inactive-account case for it. Without that test, this separate path can continue issuing Google-authenticator setup data for a disabled account while the normal login test passes.

- `RefreshToken` refuses for an inactive account

docs/superpowers/specs/2026-09-17-resigned-account-login-refusal-design.md:265

• The time-planning test list covers its login path but omits the separate gRPC RefreshToken path listed in §3. Add an inactive-account refresh test so this endpoint cannot continue minting tokens after the account is disabled.

**The time-planning plugin's test project:**
- the gRPC login refuses an inactive account with the same message as the REST path
- resigning through `TimeSettingService.UpdateAssignedSite` also disables the login

docs/superpowers/specs/2026-09-17-resigned-account-login-refusal-design.md:211

• Path B initially changes AssignedSite.Resigned and only copies it to Worker.Resigned during the BC boot reconciliation. A one-off backfill placed before that copy will miss these users, and the “no-op on every subsequent boot” rule makes the miss permanent. Specify that the backfill runs after reconciliation, or make it retry/idempotently reconcile on later boots.

next to the reconciliation — that is where worker state is already reconciled across the
two databases and where the SDK context is to hand. (`eFormAPI.Web` can also see
`Worker.Resigned`: it already builds an SDK context at `Startup.cs:498-500`.)

It logs every account it disables plus a total, and is a no-op on every subsequent boot.

docs/superpowers/specs/2026-09-17-resigned-account-login-refusal-design.md:180

• UserNameOrPasswordIncorrect is localized in all resource files (for example, the German and Danish values in SharedResource.de.resx and SharedResource.da.resx). Replacing every value with the English sentence would regress non-English login and Google-auth errors. Keep per-locale translations and define “identical” as the same key/message for a given locale, or explicitly document and approve the intentional English-only regression.

files, EN *"Username or password is incorrect"* (`SharedResource.resx:384-386`), DA
*"Brugernavn eller adgangskode er forkert"* (`SharedResource.da.resx:326-328`), already
used by `GetGoogleAuthenticator` (`AuthService.cs:415, :431`). Update that key's text to
the wording above rather than adding a near-duplicate — which also changes the
`google-auth-key` response, consistently and for the better.

docs/superpowers/specs/2026-09-17-resigned-account-login-refusal-design.md:253

• The test plan omits the schema/default behavior called out as security-critical: a freshly constructed EformUser must persist IsActive = true, and the migration must leave existing rows true. A false scaffolded default or initializer/mapping mismatch can disable every new or existing account while the listed login tests still pass. Add migration and user-creation assertions in the owning package/test project.

  **byte-identical** to the unknown-account response — this is the requirement, so it is
  asserted directly rather than by checking the message alone
- `RefreshToken` refuses for an inactive account
- the backfill disables a pre-existing resigned worker's login and is idempotent on a
  second run

docs/superpowers/specs/2026-09-17-resigned-account-login-refusal-design.md:152

• Require this check before CheckPasswordSignInAsync in both REST and Google-auth flows. That call currently has lockout-on-failure enabled; checking IsActive afterward can mutate lockout state for disabled accounts and return the distinct lockout message, breaking the generic error contract.

Refuse when `IsActive == false`, returning the shared failure result (§4):

docs/superpowers/specs/2026-09-17-resigned-account-login-refusal-design.md:251

• The plan covers REST login/refresh and time-planning login, but not the independent POST /api/auth/google-auth-key path or time-planning RefreshToken, both explicitly listed as separate read surfaces above. Add inactive-account assertions for both, including the same response contract; otherwise the second credential oracle and alternate refresh implementation can regress undetected.

**`eFormAPI.Web.Integration.Tests`** (host):
- a login whose account has `IsActive = false` is refused, and the response is
  **byte-identical** to the unknown-account response — this is the requirement, so it is
  asserted directly rather than by checking the message alone
- `RefreshToken` refuses for an inactive account

docs/superpowers/specs/2026-09-17-resigned-account-login-refusal-design.md:49

• The claim that email matching is used only once on an admin-present write path is contradicted by §5, where an unattended startup backfill uses a worker/login email match to set IsActive = false. That is exposed to the same drift and collision concerns described above; specify a safe reconciliation rule or revise the rationale and rollout safeguards.

The email match is still used, but only on the **write** path — once, with an admin
present, at the moment of resigning — where a miss is visible and recoverable.

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Lite