Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jan 12, 2026

Fix PasskeySignInAsync to enforce email/phone confirmation and lockout checks

  • You've read the Contributor Guide and Code of Conduct.
  • You've included unit or integration tests for your change, where applicable.
  • You've included inline docs for your change, where applicable.
  • There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

Fix PasskeySignInAsync bypassing RequireConfirmedEmail, RequireConfirmedPhoneNumber, and lockout

Description

PasskeySignInAsync() was not calling PreSignInCheck() before signing in users, allowing authentication to succeed even when:

  • RequireConfirmedEmail = true and email is unconfirmed
  • RequireConfirmedPhoneNumber = true and phone is unconfirmed
  • User is locked out

This is inconsistent with PasswordSignInAsync() and other sign-in methods that properly enforce these requirements.

Changes:

  • SignInManager.PasskeySignInCoreAsync: Added PreSignInCheck() call after successful passkey assertion, before updating passkey metadata. Returns SignInResult.NotAllowed or SignInResult.LockedOut when requirements aren't met.

  • Tests: Added CanRequireConfirmedEmailForPasskeySignIn, CanRequireConfirmedPhoneNumberForPasskeySignIn, and PasskeySignInReturnsLockedOutWhenLockedOut to verify enforcement.

Original prompt

Problem

SignInManager<TUser>.PasskeySignInAsync() does not call PreSignInCheck() before signing in the user, which means it bypasses sign-in requirements like RequireConfirmedEmail, RequireConfirmedPhoneNumber, and lockout checks. This is inconsistent with PasswordSignInAsync() which correctly enforces these requirements.

Reference Issue: #65020

Current Behavior

Looking at PasskeySignInCoreAsync in src/Identity/Core/src/SignInManager.cs:

private async Task<SignInResult> PasskeySignInCoreAsync(string credentialJson)
{
    ArgumentException.ThrowIfNullOrEmpty(credentialJson);

    var assertionResult = await PerformPasskeyAssertionAsync(credentialJson);
    if (!assertionResult.Succeeded)
    {
        return SignInResult.Failed;
    }

    // After a successful assertion, we need to update the passkey so that it has the latest
    // sign count and authenticator data.
    var setPasskeyResult = await UserManager.AddOrUpdatePasskeyAsync(assertionResult.User, assertionResult.Passkey);
    if (!setPasskeyResult.Succeeded)
    {
        return SignInResult.Failed;
    }

    return await SignInOrTwoFactorAsync(assertionResult.User, isPersistent: false, bypassTwoFactor: true);
}

The method directly proceeds to SignInOrTwoFactorAsync without calling PreSignInCheck().

Expected Behavior

PasskeySignInAsync() should call PreSignInCheck() after successful passkey assertion (but before signing in) to enforce:

  • RequireConfirmedEmail - returns SignInResult.NotAllowed if email is not confirmed
  • RequireConfirmedPhoneNumber - returns SignInResult.NotAllowed if phone is not confirmed
  • User lockout - returns SignInResult.LockedOut if user is locked out

Required Changes

1. Modify PasskeySignInCoreAsync in src/Identity/Core/src/SignInManager.cs

Add a call to PreSignInCheck() after successful passkey assertion, similar to how TwoFactorAuthenticatorSignInCoreAsync and TwoFactorSignInCoreAsync do it:

private async Task<SignInResult> PasskeySignInCoreAsync(string credentialJson)
{
    ArgumentException.ThrowIfNullOrEmpty(credentialJson);

    var assertionResult = await PerformPasskeyAssertionAsync(credentialJson);
    if (!assertionResult.Succeeded)
    {
        return SignInResult.Failed;
    }

    // Check pre-sign-in requirements (email confirmation, phone confirmation, lockout)
    var error = await PreSignInCheck(assertionResult.User);
    if (error != null)
    {
        return error;
    }

    // After a successful assertion, we need to update the passkey so that it has the latest
    // sign count and authenticator data.
    var setPasskeyResult = await UserManager.AddOrUpdatePasskeyAsync(assertionResult.User, assertionResult.Passkey);
    if (!setPasskeyResult.Succeeded)
    {
        return SignInResult.Failed;
    }

    return await SignInOrTwoFactorAsync(assertionResult.User, isPersistent: false, bypassTwoFactor: true);
}

2. Add unit tests in src/Identity/test/Identity.Test/SignInManagerTest.cs

Add the following test methods that mirror the existing password sign-in tests:

Test 1: CanRequireConfirmedEmailForPasskeySignIn

Similar to CanRequireConfirmedEmailForPasswordSignIn, this test should verify that when RequireConfirmedEmail = true:

  • If email is NOT confirmed → PasskeySignInAsync returns SignInResult.NotAllowed
  • If email IS confirmed → PasskeySignInAsync returns SignInResult.Success

Test 2: CanRequireConfirmedPhoneNumberForPasskeySignIn

Similar to CanRequireConfirmedPhoneNumberForPasswordSignIn, this test should verify that when RequireConfirmedPhoneNumber = true:

  • If phone is NOT confirmed → PasskeySignInAsync returns SignInResult.NotAllowed
  • If phone IS confirmed → PasskeySignInAsync returns SignInResult.Success

Test 3: PasskeySignInReturnsLockedOutWhenLockedOut

Similar to PasswordSignInReturnsLockedOutWhenLockedOut, this test should verify that:

  • If user is locked out → PasskeySignInAsync returns SignInResult.LockedOut

For reference, the existing CanPasskeySignIn test in the same file shows how to set up passkey sign-in tests with mocked IPasskeyHandler. Use similar setup patterns for the new tests.

This pull request was created from Copilot chat.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: MackinnonBuck <10456961+MackinnonBuck@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix missing pre-signin checks in PasskeySignInAsync Fix PasskeySignInAsync to enforce email/phone confirmation and lockout checks Jan 12, 2026
Copilot AI requested a review from MackinnonBuck January 12, 2026 19:28
@MackinnonBuck MackinnonBuck marked this pull request as ready for review January 15, 2026 22:28
Copilot AI review requested due to automatic review settings January 15, 2026 22:28
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a security vulnerability where PasskeySignInAsync() was bypassing important sign-in requirements (email confirmation, phone confirmation, and lockout checks) that are properly enforced by other sign-in methods like PasswordSignInAsync().

Changes:

  • Added PreSignInCheck() call in PasskeySignInCoreAsync to enforce email/phone confirmation and lockout requirements before signing in
  • Added three comprehensive unit tests to verify the fix works correctly for all three scenarios (email confirmation, phone confirmation, and lockout)

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/Identity/Core/src/SignInManager.cs Added PreSignInCheck() call after successful passkey assertion to enforce sign-in requirements
src/Identity/test/Identity.Test/SignInManagerTest.cs Added three tests to verify email confirmation, phone confirmation, and lockout checks work with passkey sign-in

@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Jan 23, 2026
@MackinnonBuck
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@dotnet-policy-service dotnet-policy-service bot removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Jan 27, 2026
@halter73
Copy link
Member

I assume we're going to backport this to release/10.0?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants