Identity API /manage/2fa ignores IdentityResult from SetTwoFactorEnabledAsync #70053

Summary

The Identity API POST /manage/2fa endpoint ignores the IdentityResult returned by UserManager.SetTwoFactorEnabledAsync. When that update fails (for example with an optimistic concurrency failure), the endpoint continues as though the change was applied and returns a success response.

What is wrong

• SetTwoFactorEnabledAsync returns an IdentityResult that reports store update failures (such as ConcurrencyFailure) rather than throwing. The endpoint awaits it and discards the result.
• The response therefore does not reliably reflect whether two-factor authentication was actually enabled or disabled. Subsequent steps (shared key reset, recovery code generation) also run against a state that may not have been persisted.
• Every other mutating endpoint in the same file checks result.Succeeded and returns CreateValidationProblem(result); this endpoint is inconsistent with that pattern.
• Concurrent updates to the same user record (for example, other operations that update the user row such as failed sign-in bookkeeping) can make the update fail with a concurrency result, so this is reachable in normal operation.

Why it matters (defense in depth)

• Correctness: callers cannot distinguish a persisted change from a rejected one, and a security-relevant setting (2FA enabled/disabled) can end up different from what the client was told.
• Hardening: security-sensitive state transitions should be fail-closed and report failure to the caller so clients can retry or surface an error.

Affected code

• src/Identity/Core/src/IdentityApiEndpointRouteBuilderExtensions.cs:532 - SetTwoFactorEnabledAsync(user, true) result ignored
• src/Identity/Core/src/IdentityApiEndpointRouteBuilderExtensions.cs:536 - SetTwoFactorEnabledAsync(user, false) result ignored
• src/Identity/Core/src/IdentityApiEndpointRouteBuilderExtensions.cs:539-542 - ResetAuthenticatorKeyAsync result in the same handler is also ignored (same class of issue; also again in the "no key yet" branch of the handler)

Recommended fix

• Selected approach: capture the IdentityResult from both SetTwoFactorEnabledAsync calls and, when !Succeeded, return CreateValidationProblem(result), matching the pattern already used by the passkey, password, and email endpoints. The handler's return type already includes ValidationProblem, so there is no public API or response-shape change for existing success paths.
• Alternatives considered:
  • Throwing on failure: rejected; it would surface as a 500 and diverges from the rest of the endpoints.
  • Retrying on concurrency failure inside the endpoint: rejected as larger behavior change; clients can retry on the validation problem.
• Follow-up consideration: apply the same check to ResetAuthenticatorKeyAsync calls in the handler; GenerateNewTwoFactorRecoveryCodesAsync returns a nullable enumerable instead of an IdentityResult, so its failure signal (null) needs a separate decision.
• Compatibility: previously silent failures now return HTTP 400 with an application/problem+json body keyed by the IdentityError code (for example ConcurrencyFailure). Successful requests are unchanged.

Acceptance criteria

• When SetTwoFactorEnabledAsync returns a failed IdentityResult, /manage/2fa returns a validation problem containing the error code, for both the enable and disable paths.
• A functional test in src/Identity/test/Identity.FunctionalTests/MapIdentityApiTests.cs uses a UserManager that returns ConcurrencyFailure from SetTwoFactorEnabledAsync and asserts both paths; the test fails without the fix and passes with it.
• Existing 2FA tests (CanEnableAndLoginWithTwoFactor, CanLoginWithRecoveryCodeAndDisableTwoFactor) continue to pass.
• Decision recorded for ResetAuthenticatorKeyAsync and recovery code generation handling.