-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[PM-33417] WebAuthn cache #7500
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+744
β14
Merged
Changes from 6 commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
861489a
feat: add webauthn cache provider and tests
ike-kottlowski e2b9314
feat: add new class to controllers and commands. Update commands and β¦
ike-kottlowski 68ea8cb
Merge branch 'main' into auth/pm-33417/webauthn-cache
ike-kottlowski b607d14
fix: remove claude pasta
ike-kottlowski 9e11f78
Merge branch 'main' into auth/pm-33417/webauthn-cache
ike-kottlowski d81e03c
chore: rename cache prefix
ike-kottlowski 4af0daa
Merge branch 'main' into auth/pm-33417/webauthn-cache
ike-kottlowski 2b402d3
test: add integration tests for webauthn login flows.
ike-kottlowski c826140
Merge branch 'auth/pm-33417/webauthn-cache' of https://github.com/bitβ¦
ike-kottlowski b7812f5
test: fix broken test
ike-kottlowski 74a4bd5
Merge branch 'main' into auth/pm-33417/webauthn-cache
ike-kottlowski 935b5c3
Merge branch 'main' into auth/pm-33417/webauthn-cache
ike-kottlowski 5bb2707
Merge branch 'main' into auth/pm-33417/webauthn-cache
ike-kottlowski c802b57
Merge branch 'main' into auth/pm-33417/webauthn-cache
ike-kottlowski 1ee3f60
feat: use static token lifetime
ike-kottlowski 4efa84b
chore: enable nullable for WebauthnLoginAssertionOptions
ike-kottlowski e08e34d
test: update test coverage
ike-kottlowski File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
17 changes: 17 additions & 0 deletions
17
src/Core/Auth/UserFeatures/WebAuthnLogin/IWebAuthnChallengeCacheProvider.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| ο»Ώnamespace Bit.Core.Auth.UserFeatures.WebAuthnLogin; | ||
|
|
||
| /// <summary> | ||
| /// Enforces single-use semantics for WebAuthn assertion challenges per the W3C WebAuthn | ||
| /// specification (Β§13.4.3). Tracks used challenges so that each challenge can only be | ||
| /// validated once. | ||
| /// </summary> | ||
| public interface IWebAuthnChallengeCacheProvider | ||
| { | ||
| /// <summary> | ||
| /// Attempts to mark a challenge as used. Returns <c>true</c> if this is the first use | ||
| /// (challenge was not in cache and has now been saved). Returns <c>false</c> if the | ||
| /// challenge was already marked as used (found in cache). | ||
| /// </summary> | ||
| /// <param name="challenge">The challenge bytes from <see cref="Fido2NetLib.AssertionOptions.Challenge"/>.</param> | ||
| Task<bool> TryMarkChallengeAsUsedAsync(byte[] challenge); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
33 changes: 33 additions & 0 deletions
33
src/Core/Auth/UserFeatures/WebAuthnLogin/Implementations/WebAuthnChallengeCacheProvider.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| ο»Ώusing Bit.Core.Utilities; | ||
| using Microsoft.Extensions.Caching.Distributed; | ||
| using Microsoft.Extensions.DependencyInjection; | ||
|
|
||
| namespace Bit.Core.Auth.UserFeatures.WebAuthnLogin.Implementations; | ||
|
|
||
| internal class WebAuthnChallengeCacheProvider( | ||
| [FromKeyedServices("persistent")] IDistributedCache distributedCache) : IWebAuthnChallengeCacheProvider | ||
| { | ||
| private const string _cacheKeyPrefix = "WebAuthnLoginAssertion_"; | ||
| private static readonly DistributedCacheEntryOptions _cacheOptions = new() | ||
| { | ||
| AbsoluteExpirationRelativeToNow = TimeSpan.FromMinutes(17) | ||
| }; | ||
|
|
||
| private readonly IDistributedCache _distributedCache = distributedCache; | ||
|
|
||
| public async Task<bool> TryMarkChallengeAsUsedAsync(byte[] challenge) | ||
| { | ||
| var cacheKey = BuildCacheKey(challenge); | ||
| var cached = await _distributedCache.GetAsync(cacheKey); | ||
| if (cached != null) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| await _distributedCache.SetAsync(cacheKey, [1], _cacheOptions); | ||
| return true; | ||
| } | ||
|
|
||
| private static string BuildCacheKey(byte[] challenge) | ||
| => $"{_cacheKeyPrefix}{CoreHelpers.Base64UrlEncode(challenge)}"; | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
66 changes: 66 additions & 0 deletions
66
test/Core.Test/Auth/UserFeatures/WebAuthnLogin/WebAuthnChallengeCacheProviderTests.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| ο»Ώusing Bit.Core.Auth.UserFeatures.WebAuthnLogin.Implementations; | ||
| using Bit.Core.Utilities; | ||
| using Bit.Test.Common.AutoFixture; | ||
| using Bit.Test.Common.AutoFixture.Attributes; | ||
| using Microsoft.Extensions.Caching.Distributed; | ||
| using NSubstitute; | ||
| using Xunit; | ||
|
|
||
| namespace Bit.Core.Test.Auth.UserFeatures.WebAuthnLogin; | ||
|
|
||
| [SutProviderCustomize] | ||
| public class WebAuthnChallengeCacheProviderTests | ||
| { | ||
| [Theory, BitAutoData] | ||
| internal async Task TryMarkChallengeAsUsedAsync_FirstUse_SavesAndReturnsTrue( | ||
| SutProvider<WebAuthnChallengeCacheProvider> sutProvider) | ||
| { | ||
| // Arrange | ||
| var challenge = new byte[] { 1, 2, 3, 4, 5, 6, 7, 8 }; | ||
| var expectedKey = $"WebAuthnLoginAssertion_{CoreHelpers.Base64UrlEncode(challenge)}"; | ||
|
|
||
| sutProvider.GetDependency<IDistributedCache>() | ||
| .GetAsync(expectedKey, Arg.Any<CancellationToken>()) | ||
| .Returns((byte[])null); | ||
|
|
||
| // Act | ||
| var result = await sutProvider.Sut.TryMarkChallengeAsUsedAsync(challenge); | ||
|
|
||
| // Assert | ||
| Assert.True(result); | ||
| await sutProvider.GetDependency<IDistributedCache>() | ||
| .Received(1) | ||
| .SetAsync( | ||
| expectedKey, | ||
| Arg.Is<byte[]>(b => b.Length == 1 && b[0] == 1), | ||
| Arg.Is<DistributedCacheEntryOptions>(o => | ||
| o.AbsoluteExpirationRelativeToNow == TimeSpan.FromMinutes(17)), | ||
| Arg.Any<CancellationToken>()); | ||
| } | ||
|
|
||
| [Theory, BitAutoData] | ||
| internal async Task TryMarkChallengeAsUsedAsync_AlreadyUsed_ReturnsFalseAndDoesNotSave( | ||
| SutProvider<WebAuthnChallengeCacheProvider> sutProvider) | ||
| { | ||
| // Arrange | ||
| var challenge = new byte[] { 1, 2, 3, 4, 5, 6, 7, 8 }; | ||
| var expectedKey = $"WebAuthnLoginAssertion_{CoreHelpers.Base64UrlEncode(challenge)}"; | ||
|
|
||
| sutProvider.GetDependency<IDistributedCache>() | ||
| .GetAsync(expectedKey, Arg.Any<CancellationToken>()) | ||
| .Returns(new byte[] { 1 }); | ||
|
|
||
| // Act | ||
| var result = await sutProvider.Sut.TryMarkChallengeAsUsedAsync(challenge); | ||
|
|
||
| // Assert | ||
| Assert.False(result); | ||
| await sutProvider.GetDependency<IDistributedCache>() | ||
| .DidNotReceive() | ||
| .SetAsync( | ||
| Arg.Any<string>(), | ||
| Arg.Any<byte[]>(), | ||
| Arg.Any<DistributedCacheEntryOptions>(), | ||
| Arg.Any<CancellationToken>()); | ||
| } | ||
| } |
115 changes: 115 additions & 0 deletions
115
test/Identity.Test/IdentityServer/RequestValidators/WebAuthnGrantValidatorTests.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,115 @@ | ||
| ο»Ώusing System.Collections.Specialized; | ||
| using Bit.Core.Auth.Entities; | ||
| using Bit.Core.Auth.Enums; | ||
| using Bit.Core.Auth.Models.Business.Tokenables; | ||
| using Bit.Core.Auth.UserFeatures.WebAuthnLogin; | ||
| using Bit.Core.Entities; | ||
| using Bit.Core.Tokens; | ||
| using Bit.Identity.IdentityServer.RequestValidators; | ||
| using Bit.Test.Common.AutoFixture; | ||
| using Bit.Test.Common.AutoFixture.Attributes; | ||
| using Duende.IdentityServer.Validation; | ||
| using Fido2NetLib; | ||
| using NSubstitute; | ||
| using Xunit; | ||
| using AuthFixtures = Bit.Identity.Test.AutoFixture; | ||
|
|
||
| namespace Bit.Identity.Test.IdentityServer.RequestValidators; | ||
|
|
||
| [SutProviderCustomize] | ||
| public class WebAuthnGrantValidatorTests | ||
| { | ||
| private static ExtensionGrantValidationContext CreateContext( | ||
| ValidatedTokenRequest tokenRequest, | ||
| string token = "test-token", | ||
| string deviceResponse = """{"id":"abc","rawId":"abc","type":"public-key","response":{"authenticatorData":"dGVzdA","signature":"dGVzdA","clientDataJSON":"dGVzdA","userHandle":"dGVzdA"}}""") | ||
| { | ||
| tokenRequest.Raw = new NameValueCollection | ||
| { | ||
| { "token", token }, | ||
| { "deviceResponse", deviceResponse } | ||
| }; | ||
|
|
||
| return new ExtensionGrantValidationContext { Request = tokenRequest }; | ||
| } | ||
|
|
||
| [Theory, BitAutoData] | ||
| public async Task ValidateAsync_MissingToken_RejectsWithInvalidGrant( | ||
| [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, | ||
| SutProvider<WebAuthnGrantValidator> sutProvider) | ||
| { | ||
| // Arrange - no token or deviceResponse in raw params | ||
| tokenRequest.Raw = new NameValueCollection(); | ||
| var context = new ExtensionGrantValidationContext { Request = tokenRequest }; | ||
|
|
||
| // Act | ||
| await sutProvider.Sut.ValidateAsync(context); | ||
|
|
||
| // Assert | ||
| Assert.Equal("invalid_grant", context.Result.Error); | ||
| } | ||
|
|
||
| [Theory, BitAutoData] | ||
| public async Task ValidateAsync_InvalidToken_RejectsWithInvalidRequest( | ||
| [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, | ||
| SutProvider<WebAuthnGrantValidator> sutProvider) | ||
| { | ||
| // Arrange | ||
| var context = CreateContext(tokenRequest); | ||
|
|
||
| sutProvider.GetDependency<IDataProtectorTokenFactory<WebAuthnLoginAssertionOptionsTokenable>>() | ||
| .TryUnprotect(Arg.Any<string>(), out Arg.Any<WebAuthnLoginAssertionOptionsTokenable>()) | ||
| .Returns(false); | ||
|
|
||
| // Act | ||
| await sutProvider.Sut.ValidateAsync(context); | ||
|
|
||
| // Assert | ||
| Assert.Equal("invalid_request", context.Result.Error); | ||
| } | ||
|
|
||
| [Theory, BitAutoData] | ||
| public async Task ValidateAsync_ValidToken_CallsAssertCommand( | ||
| [AuthFixtures.ValidatedTokenRequest] ValidatedTokenRequest tokenRequest, | ||
| SutProvider<WebAuthnGrantValidator> sutProvider) | ||
| { | ||
| // Arrange | ||
| var challenge = new byte[] { 1, 2, 3, 4, 5, 6, 7, 8 }; | ||
| var options = new AssertionOptions { Challenge = challenge }; | ||
| var tokenable = new WebAuthnLoginAssertionOptionsTokenable( | ||
| WebAuthnLoginAssertionOptionsScope.Authentication, options); | ||
|
|
||
| var context = CreateContext(tokenRequest); | ||
|
|
||
| sutProvider.GetDependency<IDataProtectorTokenFactory<WebAuthnLoginAssertionOptionsTokenable>>() | ||
| .TryUnprotect(Arg.Any<string>(), out Arg.Any<WebAuthnLoginAssertionOptionsTokenable>()) | ||
| .Returns(x => | ||
| { | ||
| x[1] = tokenable; | ||
| return true; | ||
| }); | ||
|
|
||
| // Mock credential assertion to succeed | ||
| var user = new User { Id = Guid.NewGuid() }; | ||
| var credential = new WebAuthnCredential(); | ||
| sutProvider.GetDependency<IAssertWebAuthnLoginCredentialCommand>() | ||
| .AssertWebAuthnLoginCredential(Arg.Any<AssertionOptions>(), Arg.Any<AuthenticatorAssertionRawResponse>()) | ||
| .Returns((user, credential)); | ||
|
|
||
| // Act - the base validator pipeline may throw due to unmocked dependencies, | ||
| // but our code runs before that. | ||
| try | ||
| { | ||
| await sutProvider.Sut.ValidateAsync(context); | ||
| } | ||
| catch (NullReferenceException) | ||
| { | ||
| // Expected: base validator pipeline has unmocked dependencies | ||
| } | ||
|
|
||
| // Assert - verify the assert command was called | ||
| await sutProvider.GetDependency<IAssertWebAuthnLoginCredentialCommand>() | ||
| .Received(1) | ||
| .AssertWebAuthnLoginCredential(options, Arg.Any<AuthenticatorAssertionRawResponse>()); | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.