Skip to content

Commit ac1bd58

Browse files
committed
PR feedback + a few tests
1 parent e7ccd11 commit ac1bd58

File tree

8 files changed

+250
-189
lines changed

8 files changed

+250
-189
lines changed

src/Identity/Core/src/DefaultPasskeyHandler.cs

Lines changed: 145 additions & 166 deletions
Large diffs are not rendered by default.

src/Identity/Core/src/PasskeyExceptionExtensions.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,5 +147,8 @@ public static PasskeyException NullClientDataJson()
147147

148148
public static PasskeyException InvalidClientDataJsonFormat(JsonException ex)
149149
=> new($"The client data JSON had an invalid format: {ex.Message}", ex);
150+
151+
public static PasskeyException InvalidCredentialPublicKey(Exception ex)
152+
=> new($"The credential public key was invalid.", ex);
150153
}
151154
}

src/Identity/Core/src/Passkeys/AuthenticatorData.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System.Buffers.Binary;
55
using System.Diagnostics;
6+
using System.Diagnostics.CodeAnalysis;
67
using System.Formats.Cbor;
78

89
namespace Microsoft.AspNetCore.Identity;
@@ -68,6 +69,7 @@ internal sealed class AuthenticatorData
6869
/// <summary>
6970
/// Gets whether the authenticator added attested credential data.
7071
/// </summary>
72+
[MemberNotNullWhen(true, nameof(AttestedCredentialData))]
7173
public bool HasAttestedCredentialData => Flags.HasFlag(AuthenticatorDataFlags.HasAttestedCredentialData);
7274

7375
public static AuthenticatorData Parse(ReadOnlyMemory<byte> bytes)

src/Identity/Core/src/Passkeys/BufferSource.cs

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using System.Linq;
5-
using System.Security.Cryptography;
65
using System.Text;
76
using System.Text.Json.Serialization;
87

@@ -78,15 +77,6 @@ public bool Equals(BufferSource? other)
7877
return other is not null && _bytes.Span.SequenceEqual(other._bytes.Span);
7978
}
8079

81-
/// <summary>
82-
/// Performs a fixed-time value-based equality comparison with another <see cref="BufferSource"/> instance
83-
/// using <see cref="CryptographicOperations.FixedTimeEquals"/>.
84-
/// </summary>
85-
public bool FixedTimeEquals(BufferSource? other)
86-
{
87-
return other is not null && CryptographicOperations.FixedTimeEquals(_bytes.Span, other._bytes.Span);
88-
}
89-
9080
/// <inheritdoc/>
9181
public override bool Equals(object? obj)
9282
=> obj is BufferSource other && Equals(other);

src/Identity/Core/src/Passkeys/CredentialPublicKey.cs

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ internal sealed class CredentialPublicKey
1616

1717
public COSEAlgorithmIdentifier Alg => _alg;
1818

19-
public CredentialPublicKey(ReadOnlyMemory<byte> bytes)
19+
private CredentialPublicKey(ReadOnlyMemory<byte> bytes)
2020
{
2121
var reader = Ctap2CborReader.Create(bytes);
2222

@@ -38,7 +38,30 @@ public CredentialPublicKey(ReadOnlyMemory<byte> bytes)
3838
}
3939

4040
var keyLength = bytes.Length - reader.BytesRemaining;
41-
_bytes = bytes.Slice(0, keyLength);
41+
_bytes = bytes[..keyLength];
42+
}
43+
44+
public static CredentialPublicKey Decode(ReadOnlyMemory<byte> bytes)
45+
{
46+
try
47+
{
48+
return new CredentialPublicKey(bytes);
49+
}
50+
catch (PasskeyException)
51+
{
52+
throw;
53+
}
54+
catch (Exception ex)
55+
{
56+
throw PasskeyException.InvalidCredentialPublicKey(ex);
57+
}
58+
}
59+
60+
public static CredentialPublicKey Decode(ReadOnlyMemory<byte> bytes, out int bytesRead)
61+
{
62+
var key = Decode(bytes);
63+
bytesRead = key._bytes.Length;
64+
return key;
4265
}
4366

4467
public bool Verify(ReadOnlySpan<byte> data, ReadOnlySpan<byte> signature)
@@ -187,13 +210,6 @@ private static HashAlgorithmName HashAlgFromCOSEAlg(COSEAlgorithmIdentifier alg)
187210
};
188211
}
189212

190-
public static CredentialPublicKey Decode(ReadOnlyMemory<byte> cpk, out int bytesRead)
191-
{
192-
var key = new CredentialPublicKey(cpk);
193-
bytesRead = key._bytes.Length;
194-
return key;
195-
}
196-
197213
public ReadOnlyMemory<byte> AsMemory() => _bytes;
198214

199215
public byte[] ToArray() => _bytes.ToArray();

src/Identity/Core/src/SignInManager.cs

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -537,7 +537,7 @@ private void ThrowIfNoPasskeyHandler()
537537
}
538538

539539
/// <summary>
540-
/// Attempts to sign in the user with a passkey.
540+
/// Performs a passkey assertion and attempts to sign in the user.
541541
/// </summary>
542542
/// <param name="credentialJson">The credentials obtained by JSON-serializing the result of the <c>navigator.credentials.get()</c> JavaScript function.</param>
543543
/// <param name="options">The original passkey request options provided to the browser.</param>
@@ -555,6 +555,8 @@ public virtual async Task<SignInResult> PasskeySignInAsync(string credentialJson
555555
return SignInResult.Failed;
556556
}
557557

558+
// After a successful assertion, we need to update the passkey so that it has the latest
559+
// sign count and authenticator data.
558560
var setPasskeyResult = await UserManager.SetPasskeyAsync(assertionResult.User, assertionResult.Passkey);
559561
if (!setPasskeyResult.Succeeded)
560562
{
@@ -568,6 +570,10 @@ public virtual async Task<SignInResult> PasskeySignInAsync(string credentialJson
568570
/// Generates a <see cref="PasskeyCreationOptions"/> and stores it in the current <see cref="HttpContext"/> for later retrieval.
569571
/// </summary>
570572
/// <param name="creationArgs">Args for configuring the <see cref="PasskeyCreationOptions"/>.</param>
573+
/// <remarks>
574+
/// The returned options should be passed to the <c>navigator.credentials.create()</c> JavaScript function.
575+
/// The credentials returned from that function can then be passed to the <see cref="PerformPasskeyAttestationAsync"/>.
576+
/// </remarks>
571577
/// <returns>
572578
/// A task object representing the asynchronous operation containing the <see cref="PasskeyCreationOptions"/>.
573579
/// </returns>
@@ -593,6 +599,10 @@ public virtual async Task<PasskeyCreationOptions> ConfigurePasskeyCreationOption
593599
/// Generates a <see cref="PasskeyCreationOptions"/> to create a new passkey for a user.
594600
/// </summary>
595601
/// <param name="creationArgs">Args for configuring the <see cref="PasskeyCreationOptions"/>.</param>
602+
/// <remarks>
603+
/// The returned options should be passed to the <c>navigator.credentials.create()</c> JavaScript function.
604+
/// The credentials returned from that function can then be passed to the <see cref="PerformPasskeyAttestationAsync"/>.
605+
/// </remarks>
596606
/// <returns>
597607
/// A task object representing the asynchronous operation containing the <see cref="PasskeyCreationOptions"/>.
598608
/// </returns>
@@ -653,6 +663,11 @@ async Task<PublicKeyCredentialDescriptor[]> GetExcludeCredentialsAsync()
653663
/// Generates a <see cref="PasskeyRequestOptions"/> and stores it in the current <see cref="HttpContext"/> for later retrieval.
654664
/// </summary>
655665
/// <param name="requestArgs">Args for configuring the <see cref="PasskeyRequestOptions"/>.</param>
666+
/// <remarks>
667+
/// The returned options should be passed to the <c>navigator.credentials.get()</c> JavaScript function.
668+
/// The credentials returned from that function can then be passed to the <see cref="PasskeySignInAsync"/> or
669+
/// <see cref="PerformPasskeyAssertionAsync"/> methods.
670+
/// </remarks>
656671
/// <returns>
657672
/// A task object representing the asynchronous operation containing the <see cref="PasskeyRequestOptions"/>.
658673
/// </returns>
@@ -680,6 +695,10 @@ public virtual async Task<PasskeyRequestOptions> ConfigurePasskeyRequestOptionsA
680695
/// Generates a <see cref="PasskeyRequestOptions"/> to request an existing passkey for a user.
681696
/// </summary>
682697
/// <param name="requestArgs">Args for configuring the <see cref="PasskeyRequestOptions"/>.</param>
698+
/// <remarks>
699+
/// The returned options should be passed to the <c>navigator.credentials.get()</c> JavaScript function.
700+
/// The credentials returned from that function can then be passed to the <see cref="PerformPasskeyAssertionAsync"/> method.
701+
/// </remarks>
683702
/// <returns>
684703
/// A task object representing the asynchronous operation containing the <see cref="PasskeyRequestOptions"/>.
685704
/// </returns>

src/Identity/test/Identity.Test/IdentityOptionsTest.cs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,12 @@ public void VerifyDefaultOptions()
3232
Assert.Equal(ClaimTypes.Name, options.ClaimsIdentity.UserNameClaimType);
3333
Assert.Equal(ClaimTypes.NameIdentifier, options.ClaimsIdentity.UserIdClaimType);
3434
Assert.Equal("AspNet.Identity.SecurityStamp", options.ClaimsIdentity.SecurityStampClaimType);
35+
36+
Assert.Equal(TimeSpan.FromMinutes(1), options.Passkey.Timeout);
37+
Assert.Equal(16, options.Passkey.ChallengeSize);
38+
Assert.True(options.Passkey.AllowCurrentOrigin);
39+
Assert.Equal(PasskeyOptions.CredentialBackupPolicy.Allowed, options.Passkey.BackupEligibleCredentialPolicy);
40+
Assert.Equal(PasskeyOptions.CredentialBackupPolicy.Allowed, options.Passkey.BackedUpCredentialPolicy);
3541
}
3642

3743
[Fact]
@@ -89,5 +95,4 @@ public void CanConfigureCookieOptions()
8995
Assert.Equal("c", options.Get(IdentityConstants.TwoFactorRememberMeScheme).Cookie.Name);
9096
Assert.Equal("d", options.Get(IdentityConstants.TwoFactorUserIdScheme).Cookie.Name);
9197
}
92-
9398
}

src/Identity/test/Identity.Test/SignInManagerTest.cs

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,13 @@ private static Mock<UserManager<PocoUser>> SetupUserManager(PocoUser user)
9797
return manager;
9898
}
9999

100-
private static SignInManager<PocoUser> SetupSignInManager(UserManager<PocoUser> manager, HttpContext context, ILogger logger = null, IdentityOptions identityOptions = null, IAuthenticationSchemeProvider schemeProvider = null)
100+
private static SignInManager<PocoUser> SetupSignInManager(
101+
UserManager<PocoUser> manager,
102+
HttpContext context,
103+
ILogger logger = null,
104+
IdentityOptions identityOptions = null,
105+
IAuthenticationSchemeProvider schemeProvider = null,
106+
IPasskeyHandler<PocoUser> passkeyHandler = null)
101107
{
102108
var contextAccessor = new Mock<IHttpContextAccessor>();
103109
contextAccessor.Setup(a => a.HttpContext).Returns(context);
@@ -107,7 +113,16 @@ private static SignInManager<PocoUser> SetupSignInManager(UserManager<PocoUser>
107113
options.Setup(a => a.Value).Returns(identityOptions);
108114
var claimsFactory = new UserClaimsPrincipalFactory<PocoUser, PocoRole>(manager, roleManager.Object, options.Object);
109115
schemeProvider = schemeProvider ?? new MockSchemeProvider();
110-
var sm = new SignInManager<PocoUser>(manager, contextAccessor.Object, claimsFactory, options.Object, null, schemeProvider, new DefaultUserConfirmation<PocoUser>());
116+
passkeyHandler = passkeyHandler ?? Mock.Of<IPasskeyHandler<PocoUser>>();
117+
var sm = new SignInManager<PocoUser>(
118+
manager,
119+
contextAccessor.Object,
120+
claimsFactory,
121+
options.Object,
122+
null,
123+
schemeProvider,
124+
new DefaultUserConfirmation<PocoUser>(),
125+
passkeyHandler);
111126
sm.Logger = logger ?? NullLogger<SignInManager<PocoUser>>.Instance;
112127
return sm;
113128
}
@@ -339,6 +354,38 @@ public async Task ExternalSignInRequiresVerificationIfNotBypassed(bool bypass)
339354
auth.Verify();
340355
}
341356

357+
[Fact]
358+
public async Task CanPasskeySignIn()
359+
{
360+
// Setup
361+
var user = new PocoUser { UserName = "Foo" };
362+
var passkey = new UserPasskeyInfo(null, null, null, default, 0, null, false, false, false, null, null);
363+
var assertionResult = PasskeyAssertionResult.Success(passkey, user);
364+
var passkeyHandler = new Mock<IPasskeyHandler<PocoUser>>();
365+
passkeyHandler
366+
.Setup(h => h.PerformAssertionAsync(It.IsAny<PasskeyAssertionContext<PocoUser>>()))
367+
.Returns(Task.FromResult(assertionResult));
368+
var manager = SetupUserManager(user);
369+
manager
370+
.Setup(m => m.SetPasskeyAsync(user, passkey))
371+
.Returns(Task.FromResult(IdentityResult.Success))
372+
.Verifiable();
373+
var context = new DefaultHttpContext();
374+
var auth = MockAuth(context);
375+
SetupSignIn(context, auth, user.Id, isPersistent: false, loginProvider: null);
376+
var helper = SetupSignInManager(manager.Object, context, passkeyHandler: passkeyHandler.Object);
377+
378+
// Act
379+
var passkeyRequestOptions = new PasskeyRequestOptions(userId: user.Id, "<some-options>");
380+
var signInResult = await helper.PasskeySignInAsync(credentialJson: "<some-passkey>", passkeyRequestOptions);
381+
382+
// Assert
383+
Assert.True(assertionResult.Succeeded);
384+
Assert.Same(SignInResult.Success, signInResult);
385+
manager.Verify();
386+
auth.Verify();
387+
}
388+
342389
private class GoodTokenProvider : AuthenticatorTokenProvider<PocoUser>
343390
{
344391
public override Task<bool> ValidateAsync(string purpose, string token, UserManager<PocoUser> manager, PocoUser user)

0 commit comments

Comments
 (0)