fix(auth): harden MFA recovery codes and enrollment state by mathrmartins · Pull Request #686 · gameguild-gg/gameguild

mathrmartins · 2026-10-05T02:15:31Z

Summary

Restore recorded MFA recovery criteria: default ten unique 12-character codes, salted PBKDF2-HMAC-SHA256 at 600,000 iterations, versioned issued-set metadata, constant-time comparison and legacy SHA-256 verification.
Compare original MFA security state on updates; reload conflicts before bounded retries. Commit consumption before reporting success. Stale attempts cannot restore consumed/regenerated codes; five concurrent failures accumulate into the 15-minute lockout.
Reject backup verification during pending enrollment. Record successful TOTP setup completion and a fixed enrollment deadline that failed attempts cannot extend.
Preserve numeric v1 status contracts and represent unknown legacy history with an additive flag. Preserve original issued count after consumption/configuration changes; regenerate the typed client.
Add migration, 23 authentication cases, nine real PostgreSQL/HTTP cases and three client cases; retain independent acceptance boundaries in docs/architecture/mfa-recovery-reconciliation.md.

Related to #223, #243 and #285. These issues remain open: configured provider email delivery and real authenticator QR acceptance are still required. This PR is an implementation increment, not a full issue-closure claim.

Local validation

Configured Release solution build: exit 0, zero warnings/errors.
Authentication 2,083; authorization 1,667; SharedKernel 1,371; selected API architecture/security/bearer 106; PostgreSQL/HTTP recovery nine; client contract three — all passed. Repeated/superseded runs are excluded.
Baseline reproduced four unit failures and real PostgreSQL double consumption before the fix.
Source/API/integration authentication assemblies have identical SHA-256; EF reports no pending model changes.
OpenAPI retains 1,296 paths/1,654 schemas, with only two response-schema changes. Client regeneration/type checking and git diff --check pass.
Actual HTTP recovery uses a fixture principal. Separate bearer tests exercise the real registration. Configured external email delivery/production deployment are not certified.
Interrupted builds/export, one invalid fixture assertion and the superseded broad API unit run are recorded and excluded.

Exact-head GitHub checks pending.

codacy-production · 2026-10-05T02:17:56Z

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 69 complexity · 0 duplication

Metric Results

Complexity 69

Duplication 0

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer}
_{TIP This summary will be updated as you push new changes.}

Copilot

Copilot review overview

🟡 Changes recommended

Anonymous verification can trigger multiple expensive PBKDF2 derivations per request, creating a CPU-exhaustion path.

Review effort: Balanced
Findings: 1 · 1

Open (2)

Invalid backup-code verification enables CPU exhaustion · New
IndependentTotp uses inconsistent timestamps during verification · New

What changed in this PR

Hardens MFA recovery-code storage, concurrency, enrollment expiry, API compatibility, and validation coverage while retaining legacy support.

Changes:

Adds versioned salted PBKDF2 recovery codes and issuance metadata.
Adds optimistic concurrency and fixed TOTP enrollment deadlines.
Regenerates clients and expands unit/integration contract coverage.

File	Description
`packages/infrastructure/client/tests/validation/mfa-recovery-contract.test.ts`	Tests additive status compatibility.
`packages/infrastructure/client/src/generated/types.gen.ts`	Adds recovery metadata types.
`packages/infrastructure/client/src/generated/.metadata.json`	Updates generation metadata.
`docs/architecture/mfa-recovery-reconciliation.md`	Records scope and evidence.
`apps/api/tests/GameGuild.SharedKernel.UnitTests/Configuration/ConfigurationOptionsTests.cs`	Updates default-length assertion.
`apps/api/tests/GameGuild.SharedKernel.UnitTests/Configuration/ConfigurationCoreOptionsTests.cs`	Updates core default assertion.
`apps/api/tests/GameGuild.Identity.Authentication.UnitTests/Services/TotpRecoverySecurityTests.cs`	Adds TOTP security tests.
`apps/api/tests/GameGuild.Identity.Authentication.UnitTests/Services/BackupCodeRecoverySecurityTests.cs`	Adds recovery-code tests.
`apps/api/tests/GameGuild.API.IntegrationTests/MfaRecoveryPostgreSqlTests.cs`	Tests PostgreSQL concurrency and HTTP contracts.
`apps/api/Source/Modules/GameGuild.SharedKernel/Configuration/ApplicationLayer/MfaOptions.cs`	Changes default code length.
`apps/api/Source/Modules/GameGuild.Identity.Authentication/Services/TotpMfaService.cs`	Persists enrollment state and expiry.
`apps/api/Source/Modules/GameGuild.Identity.Authentication/Services/MfaService.cs`	Propagates cancellation.
`apps/api/Source/Modules/GameGuild.Identity.Authentication/Services/MfaAttemptTrackingService.cs`	Retries concurrent failure updates.
`apps/api/Source/Modules/GameGuild.Identity.Authentication/Services/BackupCodeSet.cs`	Encodes issued-set metadata.
`apps/api/Source/Modules/GameGuild.Identity.Authentication/Services/BackupCodeMfaService.cs`	Implements hardened code handling.
`apps/api/Source/Modules/GameGuild.Identity.Authentication/Repositories/UserMfaConfigurationRepository.cs`	Reloads concurrency conflicts.
`apps/api/Source/Modules/GameGuild.Identity.Authentication/Entities/UserMfaConfiguration.cs`	Adds setup expiration state.
`apps/api/Source/Modules/GameGuild.Identity.Authentication/DTOs/MfaConfigurationResponse.cs`	Exposes issued count.
`apps/api/Source/Modules/GameGuild.Identity.Authentication/Data/Configurations/UserMfaConfigurationConfiguration.cs`	Configures concurrency tokens.
`apps/api/Source/Modules/GameGuild.Identity.Authentication/Controllers/MfaController.cs`	Preserves numeric status contracts.
`apps/api/Source/Modules/GameGuild.Identity.Authentication/AUTHORIZATION_ARCHITECTURE.md`	Documents recovery invariants.
`apps/api/Source/GameGuild.API/Database/Migrations/ApplicationDbContextModelSnapshot.cs`	Updates the EF model snapshot.
`apps/api/Source/GameGuild.API/Database/Migrations/20261005013448_MfaRecoveryConcurrency.cs`	Adds setup-expiry column.
`apps/api/Source/GameGuild.API/Database/Migrations/20261005013448_MfaRecoveryConcurrency.Designer.cs`	Defines migration target model.
`apps/api/Source/GameGuild.API/appsettings.json`	Configures 12-character codes.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+                foreach (var hashedCode in hashedCodes)
+                {
+                    if (await VerifyBackupCodeHashAsync(backupCode, hashedCode, cancellationToken))


+            if (bits >= 8) { bits -= 8; key.Add((byte)(buffer >> bits)); }
+        }
+        var counter = new byte[8];
+        BinaryPrimitives.WriteInt64BigEndian(counter, DateTimeOffset.UtcNow.ToUnixTimeSeconds() / 30 + offset);


mathrmartins · 2026-10-05T03:11:50Z

Exact head 11b41bf063181ab821368351be7afe3cff98c53a passed all four selected CI suites: 151 API integration, 1,049 API unit, 2,083 authentication and 1,371 SharedKernel cases (4,654). The job reached its 20-minute limit during the final OpenAPI test-project build; its aggregate gate is cancelled and is not counted as passed. The preceding head completed the same sequence in about 19 minutes. Logs are retained in local evidence.

Commit 10e1df87c50d4f6a8280b42b6ffcdfff450b2429 extends only the API job allowance to 30 minutes. All tests, warning requirements, final HTTP verification and aggregate gate remain enabled. The workflow and evidence-document change does not alter the already-tested MFA implementation. Matching-head checks have started again. The prior new head passed Codacy, all four CodeQL analyses, Web, OpenAPI/client consistency, migration and policy; those historical results are not substituted for this new head's gates.

Fresh local Release API architecture/security/JWT selection passed 106/106. A preceding Debug repetition crashed after 75 cases and is retained/excluded, with no additional coverage claimed. #223/#243/#285 remain OPEN with their remaining functional acceptance; this PR does not close them automatically.

mathrmartins · 2026-10-05T03:35:16Z

MFA recovery increment merged — original issue remains OPEN

PR #686 is MERGED into develop at 8e1266698a0998364ab39a34078e0fd16eaa17c8 (2026-10-05T03:31:03Z). Accepted head: 10e1df87c50d4f6a8280b42b6ffcdfff450b2429.

All applicable matching-head checks passed: API, Web, clean migration/model compatibility, OpenAPI/client consistency, repository policy, tracked-artifact checks, PR Required Gate, Codacy and four CodeQL analyses. CI run executed 151 API integration, 1,049 API unit, 2,083 authentication and 1,371 SharedKernel cases, all passing (4,654 principal executions), followed by 14 passing repeated OpenAPI HTTP cases. Both Release builds had zero warnings/errors. Nonapplicable Testing Lab/Economy/Emception jobs were skipped.

Local evidence remains 5,239 distinct cases, including 35 new cases, with the full warning/error-clean solution build, real migrated PostgreSQL concurrency/replay/actor/lockout checks, legacy compatibility and regenerated client contracts. Repeated, superseded, interrupted and crashed attempts are excluded; the previous head's 20-minute API cancellation is retained as failure history. The API allowance was extended to 30 minutes without dropping a test or gate.

Implementation and boundary map: full original issuance counts for new sets, explicit unknown legacy history, 12-character/600,000-iteration backup criteria, guarded concurrent single use, pending-enrollment rejection and fixed new-enrollment deadline/completion state.

#223, #243 and #285 remain OPEN. Configured external recovery-email delivery and a real authenticator QR round trip remain unverified. #243 retains its native child. Existing anonymous MFA verification returns no access/refresh tokens; complete sign-in challenge/session acceptance is not certified by this increment. No automatic closure or production-delivery claim is made.

The primary develop checkout is synchronized with all 55 unrelated local files preserved byte-for-byte and by status. The integrated branch was removed locally/remotely. Two checkouts are reused and no stashes remain; password hashing/policy work under #254/#255 continues in the isolated checkout.

…codes-20261004 fix(auth): harden MFA recovery codes and enrollment state

fix(auth): enforce safe MFA recovery and enrollment state

38c81a5

Copilot AI balanced review requested due to automatic review settings October 5, 2026 02:15

Copilot started reviewing on behalf of mathrmartins October 5, 2026 02:16 View session

This was referenced Oct 5, 2026

[Enhancement]: Backup Codes & Recovery Mechanisms #223

Closed

[Enhancement]: MfaService - TOTP & Backup Codes Management #243

Closed

Copilot AI reviewed Oct 5, 2026

View reviewed changes

mathrmartins added 2 commits October 4, 2026 23:43

fix(auth): make MFA retry bounds explicit and generate test keys

11b41bf

ci: allow measured API verification time without skipping gates

10e1df8

mathrmartins merged commit 8e12666 into develop Oct 5, 2026
22 checks passed

mathrmartins deleted the feature/issue-223-recovery-codes-20261004 branch October 5, 2026 03:31

mathrmartins mentioned this pull request Oct 5, 2026

[Enhancement]: TOTP MFA (QR Provisioning) #285

Closed

This was referenced Oct 5, 2026

fix(auth): preserve full password input and reconcile password policy #687

Merged

[Enhancement]: PasswordService - Password Hashing & Verification #254

Closed

[Enhancement]: Password Strength & Complexity Rules #255

Closed

tolstenko mentioned this pull request Oct 5, 2026

Merge develop into main: auth hardening and audit trails #696

Merged

tolstenko pushed a commit that referenced this pull request Oct 10, 2026

Merge pull request #686 from gameguild-gg/feature/issue-223-recovery-…

50b0c95

…codes-20261004 fix(auth): harden MFA recovery codes and enrollment state

Navigation Menu

Repository navigation

fix(auth): harden MFA recovery codes and enrollment state - #686

fix(auth): harden MFA recovery codes and enrollment state#686
mathrmartins merged 3 commits into
developfrom
feature/issue-223-recovery-codes-20261004

mathrmartins commented Oct 5, 2026

Uh oh!

codacy-production Bot commented Oct 5, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

mathrmartins commented Oct 5, 2026

Uh oh!

Uh oh!

mathrmartins commented Oct 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

mathrmartins commented Oct 5, 2026

Summary

Local validation

Uh oh!

codacy-production Bot commented Oct 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Up to standards ✅

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

mathrmartins commented Oct 5, 2026

Uh oh!

Uh oh!

mathrmartins commented Oct 5, 2026

MFA recovery increment merged — original issue remains OPEN

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codacy-production Bot commented Oct 5, 2026 •

edited

Loading