From d91e22edba174392a4d2a8c811843ddb2dd78ba6 Mon Sep 17 00:00:00 2001 From: Kyle Spearrin Date: Wed, 26 Apr 2023 11:12:30 -0400 Subject: [PATCH 01/50] support for fido2 auth --- .../Auth/Controllers/WebAuthnController.cs | 108 ++++++++++++++++++ src/Core/Auth/Entities/WebAuthnCredential.cs | 29 +++++ .../Auth/Repositories/IWebAuthnRepository.cs | 9 ++ .../Controllers/AccountsController.cs | 27 +++++ src/Identity/IdentityServer/ApiClient.cs | 2 +- .../IdentityServer/ExtensionGrantValidator.cs | 100 ++++++++++++++++ .../Utilities/ServiceCollectionExtensions.cs | 3 +- .../WebAuthnCredentialRepository.cs | 34 ++++++ .../Auth/Models/WebAuthnCredential.cs | 17 +++ .../WebAuthnCredentialRepository.cs | 26 +++++ .../Repositories/DatabaseContext.cs | 4 + src/Sql/Sql.sqlproj | 12 +- .../WebAuthnCredential_Create.sql | 45 ++++++++ .../WebAuthnCredential_DeleteById.sql | 12 ++ .../WebAuthnCredential_ReadById.sql | 13 +++ .../WebAuthnCredential_ReadByUserId.sql | 13 +++ .../WebAuthnCredential_Update.sql | 32 ++++++ src/Sql/dbo/Tables/WebAuthnCredential.sql | 21 ++++ src/Sql/dbo/Views/WebAuthnCredentialView.sql | 6 + 19 files changed, 508 insertions(+), 5 deletions(-) create mode 100644 src/Api/Auth/Controllers/WebAuthnController.cs create mode 100644 src/Core/Auth/Entities/WebAuthnCredential.cs create mode 100644 src/Core/Auth/Repositories/IWebAuthnRepository.cs create mode 100644 src/Identity/IdentityServer/ExtensionGrantValidator.cs create mode 100644 src/Infrastructure.Dapper/Auth/Repositories/WebAuthnCredentialRepository.cs create mode 100644 src/Infrastructure.EntityFramework/Auth/Models/WebAuthnCredential.cs create mode 100644 src/Infrastructure.EntityFramework/Auth/Repositories/WebAuthnCredentialRepository.cs create mode 100644 src/Sql/dbo/Stored Procedures/WebAuthnCredential_Create.sql create mode 100644 src/Sql/dbo/Stored Procedures/WebAuthnCredential_DeleteById.sql create mode 100644 src/Sql/dbo/Stored Procedures/WebAuthnCredential_ReadById.sql create mode 100644 src/Sql/dbo/Stored Procedures/WebAuthnCredential_ReadByUserId.sql create mode 100644 src/Sql/dbo/Stored Procedures/WebAuthnCredential_Update.sql create mode 100644 src/Sql/dbo/Tables/WebAuthnCredential.sql create mode 100644 src/Sql/dbo/Views/WebAuthnCredentialView.sql diff --git a/src/Api/Auth/Controllers/WebAuthnController.cs b/src/Api/Auth/Controllers/WebAuthnController.cs new file mode 100644 index 000000000000..9c9025ff610a --- /dev/null +++ b/src/Api/Auth/Controllers/WebAuthnController.cs @@ -0,0 +1,108 @@ +using Bit.Api.Auth.Models.Request; +using Bit.Api.Auth.Models.Request.Accounts; +using Bit.Api.Auth.Models.Response.TwoFactor; +using Bit.Api.Models.Request; +using Bit.Api.Models.Response; +using Bit.Api.Vault.Models.Response; +using Bit.Core.Auth.Enums; +using Bit.Core.Auth.LoginFeatures.PasswordlessLogin.Interfaces; +using Bit.Core.Auth.Utilities; +using Bit.Core.Context; +using Bit.Core.Entities; +using Bit.Core.Exceptions; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Bit.Core.Settings; +using Bit.Core.Utilities; +using Fido2NetLib; +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Identity; +using Microsoft.AspNetCore.Mvc; + +namespace Bit.Api.Auth.Controllers; + +[Route("webauthn")] +[Authorize("Web")] +public class WebAuthnController : Controller +{ + private readonly IUserService _userService; + private readonly IOrganizationRepository _organizationRepository; + private readonly IOrganizationService _organizationService; + private readonly GlobalSettings _globalSettings; + private readonly UserManager _userManager; + private readonly ICurrentContext _currentContext; + private readonly IVerifyAuthRequestCommand _verifyAuthRequestCommand; + + public WebAuthnController( + IUserService userService, + IOrganizationRepository organizationRepository, + IOrganizationService organizationService, + GlobalSettings globalSettings, + UserManager userManager, + ICurrentContext currentContext, + IVerifyAuthRequestCommand verifyAuthRequestCommand) + { + _userService = userService; + _organizationRepository = organizationRepository; + _organizationService = organizationService; + _globalSettings = globalSettings; + _userManager = userManager; + _currentContext = currentContext; + _verifyAuthRequestCommand = verifyAuthRequestCommand; + } + + [HttpGet("")] + public async Task> Get() + { + var user = await _userService.GetUserByPrincipalAsync(User); + if (user == null) + { + throw new UnauthorizedAccessException(); + } + return new ListResponseModel(new List { }); + } + + [HttpPost("options")] + [ApiExplorerSettings(IgnoreApi = true)] // Disable Swagger due to CredentialCreateOptions not converting properly + public async Task PostOptions([FromBody] SecretVerificationRequestModel model) + { + var user = await _userService.GetUserByPrincipalAsync(User); + if (user == null) + { + throw new UnauthorizedAccessException(); + } + + var reg = await _userService.StartWebAuthnRegistrationAsync(user); + return reg; + } + + [HttpPost("")] + public async Task Post([FromBody] TwoFactorWebAuthnRequestModel model) + { + var user = await _userService.GetUserByPrincipalAsync(User); + if (user == null) + { + throw new UnauthorizedAccessException(); + } + + var success = await _userService.CompleteWebAuthRegistrationAsync( + user, model.Id.Value, model.Name, model.DeviceResponse); + if (!success) + { + throw new BadRequestException("Unable to complete WebAuthn registration."); + } + var response = new TwoFactorWebAuthnResponseModel(user); + return response; + } + + [HttpDelete("{id}")] + [HttpPost("{id}/delete")] + public async Task Delete(string id) + { + var user = await _userService.GetUserByPrincipalAsync(User); + if (user == null) + { + throw new UnauthorizedAccessException(); + } + } +} diff --git a/src/Core/Auth/Entities/WebAuthnCredential.cs b/src/Core/Auth/Entities/WebAuthnCredential.cs new file mode 100644 index 000000000000..a1195b82ae74 --- /dev/null +++ b/src/Core/Auth/Entities/WebAuthnCredential.cs @@ -0,0 +1,29 @@ +using System.ComponentModel.DataAnnotations; +using Bit.Core.Entities; +using Bit.Core.Utilities; + +namespace Bit.Core.Auth.Entities; + +public class WebAuthnCredential : ITableObject +{ + public Guid Id { get; set; } + public Guid UserId { get; set; } + [MaxLength(50)] + public string Name { get; set; } + [MaxLength(256)] + public string PublicKey { get; set; } + [MaxLength(256)] + public string DescriptorId { get; set; } + public int Counter { get; set; } + [MaxLength(20)] + public string Type { get; set; } + public Guid AaGuid { get; set; } + public string UserKey { get; set; } + public DateTime CreationDate { get; internal set; } = DateTime.UtcNow; + public DateTime RevisionDate { get; internal set; } = DateTime.UtcNow; + + public void SetNewId() + { + Id = CoreHelpers.GenerateComb(); + } +} diff --git a/src/Core/Auth/Repositories/IWebAuthnRepository.cs b/src/Core/Auth/Repositories/IWebAuthnRepository.cs new file mode 100644 index 000000000000..751c49429af4 --- /dev/null +++ b/src/Core/Auth/Repositories/IWebAuthnRepository.cs @@ -0,0 +1,9 @@ +using Bit.Core.Auth.Entities; +using Bit.Core.Repositories; + +namespace Bit.Core.Auth.Repositories; + +public interface IWebAuthnRepository : IRepository +{ + Task> GetManyByUserIdAsync(Guid userId); +} diff --git a/src/Identity/Controllers/AccountsController.cs b/src/Identity/Controllers/AccountsController.cs index 40451727440c..4e89d984fc8e 100644 --- a/src/Identity/Controllers/AccountsController.cs +++ b/src/Identity/Controllers/AccountsController.cs @@ -71,4 +71,31 @@ public async Task PostPrelogin([FromBody] PreloginRequest } return new PreloginResponseModel(kdfInformation); } + + [HttpPost("webauthn")] + public async Task PostWebAuthn([FromBody] PreloginRequestModel model) + { + var kdfInformation = await _userRepository.GetKdfInformationByEmailAsync(model.Email); + if (kdfInformation == null) + { + kdfInformation = new UserKdfInformation + { + Kdf = KdfType.PBKDF2_SHA256, + KdfIterations = 100000, + }; + } + return new PreloginResponseModel(kdfInformation); + } + + [HttpPost("webauthn-assertion-options")] + public async Task PostWebAuthnAssertionOptions([FromBody] PreloginRequestModel model) + { + + } + + [HttpPost("webauthn-assertion")] + public async Task PostWebAuthnAssertion([FromBody] PreloginRequestModel model) + { + + } } diff --git a/src/Identity/IdentityServer/ApiClient.cs b/src/Identity/IdentityServer/ApiClient.cs index 8d2a294bec9c..ef94d60b4647 100644 --- a/src/Identity/IdentityServer/ApiClient.cs +++ b/src/Identity/IdentityServer/ApiClient.cs @@ -13,7 +13,7 @@ public ApiClient( string[] scopes = null) { ClientId = id; - AllowedGrantTypes = new[] { GrantType.ResourceOwnerPassword, GrantType.AuthorizationCode }; + AllowedGrantTypes = new[] { GrantType.ResourceOwnerPassword, GrantType.AuthorizationCode, "extension" }; RefreshTokenExpiration = TokenExpiration.Sliding; RefreshTokenUsage = TokenUsage.ReUse; SlidingRefreshTokenLifetime = 86400 * refreshTokenSlidingDays; diff --git a/src/Identity/IdentityServer/ExtensionGrantValidator.cs b/src/Identity/IdentityServer/ExtensionGrantValidator.cs new file mode 100644 index 000000000000..51462e8263f1 --- /dev/null +++ b/src/Identity/IdentityServer/ExtensionGrantValidator.cs @@ -0,0 +1,100 @@ +using System.Security.Claims; +using IdentityServer4.Models; +using IdentityServer4.Validation; +using Microsoft.AspNetCore.Identity; +using Bit.Core.Auth.Identity; +using Bit.Core.Context; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Bit.Core.Entities; +using Bit.Core.Settings; + +namespace Bit.Identity.IdentityServer; + +public class ExtensionGrantValidator : BaseRequestValidator, IExtensionGrantValidator +{ + private UserManager _userManager; + + public ExtensionGrantValidator( + UserManager userManager, + IDeviceRepository deviceRepository, + IDeviceService deviceService, + IUserService userService, + IEventService eventService, + IOrganizationDuoWebTokenProvider organizationDuoWebTokenProvider, + IOrganizationRepository organizationRepository, + IOrganizationUserRepository organizationUserRepository, + IApplicationCacheService applicationCacheService, + IMailService mailService, + ILogger logger, + ICurrentContext currentContext, + GlobalSettings globalSettings, + IPolicyRepository policyRepository, + IUserRepository userRepository, + IPolicyService policyService) + : base(userManager, deviceRepository, deviceService, userService, eventService, + organizationDuoWebTokenProvider, organizationRepository, organizationUserRepository, + applicationCacheService, mailService, logger, currentContext, globalSettings, policyRepository, + userRepository, policyService) + { + _userManager = userManager; + } + + public string GrantType => "extension"; + + public async Task ValidateAsync(ExtensionGrantValidationContext context) + { + var email = context.Request.Raw.Get("email"); + if (string.IsNullOrWhiteSpace(email)) + { + context.Result = new GrantValidationResult(TokenRequestErrors.InvalidGrant); + return; + } + + var user = await _userManager.FindByEmailAsync(email.ToLowerInvariant()); + var validatorContext = new CustomValidatorRequestContext + { + User = user, + KnownDevice = await KnownDeviceAsync(user, context.Request) + }; + + await ValidateAsync(context, context.Request, validatorContext); + } + + protected override Task ValidateContextAsync(ExtensionGrantValidationContext context, + CustomValidatorRequestContext validatorContext) + { + var email = context.Request.Raw.Get("email"); + return Task.FromResult(!string.IsNullOrWhiteSpace(email) && validatorContext.User != null); + } + + protected override Task SetSuccessResult(ExtensionGrantValidationContext context, User user, + List claims, Dictionary customResponse) + { + context.Result = new GrantValidationResult(user.Id.ToString(), "Application", + identityProvider: "bitwarden", + claims: claims.Count > 0 ? claims : null, + customResponse: customResponse); + return Task.CompletedTask; + } + + protected override void SetTwoFactorResult(ExtensionGrantValidationContext context, + Dictionary customResponse) + { + context.Result = new GrantValidationResult(TokenRequestErrors.InvalidGrant, "Two factor required.", + customResponse); + } + + protected override void SetSsoResult(ExtensionGrantValidationContext context, + Dictionary customResponse) + { + context.Result = new GrantValidationResult(TokenRequestErrors.InvalidGrant, "Sso authentication required.", + customResponse); + } + + protected override void SetErrorResult(ExtensionGrantValidationContext context, + Dictionary customResponse) + { + context.Result = new GrantValidationResult(TokenRequestErrors.InvalidGrant, customResponse: customResponse); + } +} diff --git a/src/Identity/Utilities/ServiceCollectionExtensions.cs b/src/Identity/Utilities/ServiceCollectionExtensions.cs index 068e12bd4007..87f9616546b8 100644 --- a/src/Identity/Utilities/ServiceCollectionExtensions.cs +++ b/src/Identity/Utilities/ServiceCollectionExtensions.cs @@ -44,7 +44,8 @@ public static IIdentityServerBuilder AddCustomIdentityServerServices(this IServi .AddResourceOwnerValidator() .AddPersistedGrantStore() .AddClientStore() - .AddIdentityServerCertificate(env, globalSettings); + .AddIdentityServerCertificate(env, globalSettings) + .AddExtensionGrantValidator(); services.AddTransient(); return identityServerBuilder; diff --git a/src/Infrastructure.Dapper/Auth/Repositories/WebAuthnCredentialRepository.cs b/src/Infrastructure.Dapper/Auth/Repositories/WebAuthnCredentialRepository.cs new file mode 100644 index 000000000000..72e15119fb0b --- /dev/null +++ b/src/Infrastructure.Dapper/Auth/Repositories/WebAuthnCredentialRepository.cs @@ -0,0 +1,34 @@ +using System.Data; +using Bit.Core.Auth.Entities; +using Bit.Core.Auth.Repositories; +using Bit.Core.Settings; +using Bit.Infrastructure.Dapper.Repositories; +using Dapper; +using Microsoft.Data.SqlClient; + + +namespace Bit.Infrastructure.Dapper.Auth.Repositories; + +public class WebAuthnCredentialRepository : Repository, IWebAuthnRepository +{ + public WebAuthnCredentialRepository(GlobalSettings globalSettings) + : this(globalSettings.SqlServer.ConnectionString, globalSettings.SqlServer.ReadOnlyConnectionString) + { } + + public WebAuthnCredentialRepository(string connectionString, string readOnlyConnectionString) + : base(connectionString, readOnlyConnectionString) + { } + + public async Task> GetManyByUserIdAsync(Guid userId) + { + using (var connection = new SqlConnection(ConnectionString)) + { + var results = await connection.QueryAsync( + $"[{Schema}].[{Table}_ReadByUserId]", + new { UserId = userId }, + commandType: CommandType.StoredProcedure); + + return results.ToList(); + } + } +} diff --git a/src/Infrastructure.EntityFramework/Auth/Models/WebAuthnCredential.cs b/src/Infrastructure.EntityFramework/Auth/Models/WebAuthnCredential.cs new file mode 100644 index 000000000000..696fad79215b --- /dev/null +++ b/src/Infrastructure.EntityFramework/Auth/Models/WebAuthnCredential.cs @@ -0,0 +1,17 @@ +using AutoMapper; +using Bit.Infrastructure.EntityFramework.Models; + +namespace Bit.Infrastructure.EntityFramework.Auth.Models; + +public class WebAuthnCredential : Core.Auth.Entities.WebAuthnCredential +{ + public virtual User User { get; set; } +} + +public class WebAuthnCredentialMapperProfile : Profile +{ + public WebAuthnCredentialMapperProfile() + { + CreateMap().ReverseMap(); + } +} diff --git a/src/Infrastructure.EntityFramework/Auth/Repositories/WebAuthnCredentialRepository.cs b/src/Infrastructure.EntityFramework/Auth/Repositories/WebAuthnCredentialRepository.cs new file mode 100644 index 000000000000..9d2cf81e7b4e --- /dev/null +++ b/src/Infrastructure.EntityFramework/Auth/Repositories/WebAuthnCredentialRepository.cs @@ -0,0 +1,26 @@ +using AutoMapper; +using Bit.Core.Auth.Repositories; +using Bit.Infrastructure.EntityFramework.Auth.Models; +using Bit.Infrastructure.EntityFramework.Repositories; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; + +namespace Bit.Infrastructure.EntityFramework.Auth.Repositories; + +public class WebAuthnCredentialRepository : Repository, IWebAuthnRepository +{ + public WebAuthnCredentialRepository(IServiceScopeFactory serviceScopeFactory, IMapper mapper) + : base(serviceScopeFactory, mapper, (context) => context.WebAuthnCredentials) + { } + + public async Task> GetManyByUserIdAsync(Guid userId) + { + using (var scope = ServiceScopeFactory.CreateScope()) + { + var dbContext = GetDatabaseContext(scope); + var query = dbContext.WebAuthnCredentials.Where(d => d.UserId == userId); + var creds = await query.ToListAsync(); + return Mapper.Map>(creds); + } + } +} diff --git a/src/Infrastructure.EntityFramework/Repositories/DatabaseContext.cs b/src/Infrastructure.EntityFramework/Repositories/DatabaseContext.cs index 428b2eed588a..eba974a9ecf9 100644 --- a/src/Infrastructure.EntityFramework/Repositories/DatabaseContext.cs +++ b/src/Infrastructure.EntityFramework/Repositories/DatabaseContext.cs @@ -59,6 +59,7 @@ public DatabaseContext(DbContextOptions options) public DbSet Users { get; set; } public DbSet AuthRequests { get; set; } public DbSet OrganizationDomains { get; set; } + public DbSet WebAuthnCredentials { get; set; } protected override void OnModelCreating(ModelBuilder builder) { @@ -99,6 +100,7 @@ protected override void OnModelCreating(ModelBuilder builder) var eOrganizationConnection = builder.Entity(); var eAuthRequest = builder.Entity(); var eOrganizationDomain = builder.Entity(); + var aWebAuthnCredential = builder.Entity(); eCipher.Property(c => c.Id).ValueGeneratedNever(); eCollection.Property(c => c.Id).ValueGeneratedNever(); @@ -121,6 +123,7 @@ protected override void OnModelCreating(ModelBuilder builder) eOrganizationConnection.Property(c => c.Id).ValueGeneratedNever(); eAuthRequest.Property(ar => ar.Id).ValueGeneratedNever(); eOrganizationDomain.Property(ar => ar.Id).ValueGeneratedNever(); + aWebAuthnCredential.Property(ar => ar.Id).ValueGeneratedNever(); eCollectionCipher.HasKey(cc => new { cc.CollectionId, cc.CipherId }); eCollectionUser.HasKey(cu => new { cu.CollectionId, cu.OrganizationUserId }); @@ -173,6 +176,7 @@ protected override void OnModelCreating(ModelBuilder builder) eOrganizationConnection.ToTable(nameof(OrganizationConnection)); eAuthRequest.ToTable(nameof(AuthRequest)); eOrganizationDomain.ToTable(nameof(OrganizationDomain)); + aWebAuthnCredential.ToTable(nameof(WebAuthnCredential)); ConfigureDateTimeUtcQueries(builder); } diff --git a/src/Sql/Sql.sqlproj b/src/Sql/Sql.sqlproj index bdc16b8969a1..d092e4d549e9 100644 --- a/src/Sql/Sql.sqlproj +++ b/src/Sql/Sql.sqlproj @@ -1,6 +1,5 @@  - + Debug @@ -480,6 +479,13 @@ + + + + + + + @@ -497,4 +503,4 @@ - + \ No newline at end of file diff --git a/src/Sql/dbo/Stored Procedures/WebAuthnCredential_Create.sql b/src/Sql/dbo/Stored Procedures/WebAuthnCredential_Create.sql new file mode 100644 index 000000000000..e4a3816e1d05 --- /dev/null +++ b/src/Sql/dbo/Stored Procedures/WebAuthnCredential_Create.sql @@ -0,0 +1,45 @@ +CREATE PROCEDURE [dbo].[WebAuthnCredential_Create] + @Id UNIQUEIDENTIFIER OUTPUT, + @UserId UNIQUEIDENTIFIER, + @Name NVARCHAR(50), + @PublicKey VARCHAR (256), + @DescriptorId VARCHAR(256), + @Counter INT, + @Type VARCHAR(20), + @AaGuid UNIQUEIDENTIFIER, + @UserKey VARCHAR (MAX), + @CreationDate DATETIME2(7), + @RevisionDate DATETIME2(7) +AS +BEGIN + SET NOCOUNT ON + + INSERT INTO [dbo].[WebAuthnCredential] + ( + [Id], + [UserId], + [Name], + [PublicKey], + [DescriptorId], + [Counter], + [Type], + [AaGuid], + [UserKey], + [CreationDate], + [RevisionDate] + ) + VALUES + ( + @Id, + @UserId, + @Name, + @PublicKey, + @DescriptorId, + @Counter, + @Type, + @AaGuid, + @UserKey, + @CreationDate, + @RevisionDate + ) +END diff --git a/src/Sql/dbo/Stored Procedures/WebAuthnCredential_DeleteById.sql b/src/Sql/dbo/Stored Procedures/WebAuthnCredential_DeleteById.sql new file mode 100644 index 000000000000..cb3be12dca10 --- /dev/null +++ b/src/Sql/dbo/Stored Procedures/WebAuthnCredential_DeleteById.sql @@ -0,0 +1,12 @@ +CREATE PROCEDURE [dbo].[WebAuthnCredential_DeleteById] + @Id UNIQUEIDENTIFIER +AS +BEGIN + SET NOCOUNT ON + + DELETE + FROM + [dbo].[WebAuthnCredential] + WHERE + [Id] = @Id +END \ No newline at end of file diff --git a/src/Sql/dbo/Stored Procedures/WebAuthnCredential_ReadById.sql b/src/Sql/dbo/Stored Procedures/WebAuthnCredential_ReadById.sql new file mode 100644 index 000000000000..f960fecf9b45 --- /dev/null +++ b/src/Sql/dbo/Stored Procedures/WebAuthnCredential_ReadById.sql @@ -0,0 +1,13 @@ +CREATE PROCEDURE [dbo].[WebAuthnCredential_ReadById] + @Id UNIQUEIDENTIFIER +AS +BEGIN + SET NOCOUNT ON + + SELECT + * + FROM + [dbo].[WebAuthnCredentialView] + WHERE + [Id] = @Id +END \ No newline at end of file diff --git a/src/Sql/dbo/Stored Procedures/WebAuthnCredential_ReadByUserId.sql b/src/Sql/dbo/Stored Procedures/WebAuthnCredential_ReadByUserId.sql new file mode 100644 index 000000000000..001f2fe0b949 --- /dev/null +++ b/src/Sql/dbo/Stored Procedures/WebAuthnCredential_ReadByUserId.sql @@ -0,0 +1,13 @@ +CREATE PROCEDURE [dbo].[WebAuthnCredential_ReadByUserId] + @UserId UNIQUEIDENTIFIER +AS +BEGIN + SET NOCOUNT ON + + SELECT + * + FROM + [dbo].[WebAuthnCredentialView] + WHERE + [UserId] = @UserId +END \ No newline at end of file diff --git a/src/Sql/dbo/Stored Procedures/WebAuthnCredential_Update.sql b/src/Sql/dbo/Stored Procedures/WebAuthnCredential_Update.sql new file mode 100644 index 000000000000..060d312fd5f2 --- /dev/null +++ b/src/Sql/dbo/Stored Procedures/WebAuthnCredential_Update.sql @@ -0,0 +1,32 @@ +CREATE PROCEDURE [dbo].[WebAuthnCredential_Update] + @Id UNIQUEIDENTIFIER, + @UserId UNIQUEIDENTIFIER, + @Name NVARCHAR(50), + @PublicKey VARCHAR (256), + @DescriptorId VARCHAR(256), + @Counter INT, + @Type VARCHAR(20), + @AaGuid UNIQUEIDENTIFIER, + @UserKey VARCHAR (MAX), + @CreationDate DATETIME2(7), + @RevisionDate DATETIME2(7) +AS +BEGIN + SET NOCOUNT ON + + UPDATE + [dbo].[WebAuthnCredential] + SET + [UserId] = @UserId, + [Name] = @Name, + [PublicKey] = @PublicKey, + [DescriptorId] = @DescriptorId, + [Counter] = @Counter, + [Type] = @Type, + [AaGuid] = @AaGuid, + [UserKey] = @UserKey, + [CreationDate] = @CreationDate, + [RevisionDate] = @RevisionDate + WHERE + [Id] = @Id +END \ No newline at end of file diff --git a/src/Sql/dbo/Tables/WebAuthnCredential.sql b/src/Sql/dbo/Tables/WebAuthnCredential.sql new file mode 100644 index 000000000000..670869b85b0f --- /dev/null +++ b/src/Sql/dbo/Tables/WebAuthnCredential.sql @@ -0,0 +1,21 @@ +CREATE TABLE [dbo].[WebAuthnCredential] ( + [Id] UNIQUEIDENTIFIER NOT NULL, + [UserId] UNIQUEIDENTIFIER NOT NULL, + [Name] NVARCHAR (50) NOT NULL, + [PublicKey] VARCHAR (256) NOT NULL, + [DescriptorId] VARCHAR (256) NOT NULL, + [Counter] INT NOT NULL, + [Type] VARCHAR (20) NULL, + [AaGuid] UNIQUEIDENTIFIER NOT NULL, + [UserKey] VARCHAR (MAX) NULL, + [CreationDate] DATETIME2 (7) NOT NULL, + [RevisionDate] DATETIME2 (7) NOT NULL, + CONSTRAINT [PK_WebAuthnCredential] PRIMARY KEY CLUSTERED ([Id] ASC), + CONSTRAINT [FK_WebAuthnCredential_User] FOREIGN KEY ([UserId]) REFERENCES [dbo].[User] ([Id]) +); + + +GO +CREATE NONCLUSTERED INDEX [IX_WebAuthnCredential_UserId] + ON [dbo].[WebAuthnCredential]([UserId] ASC); + diff --git a/src/Sql/dbo/Views/WebAuthnCredentialView.sql b/src/Sql/dbo/Views/WebAuthnCredentialView.sql new file mode 100644 index 000000000000..69b92eff2359 --- /dev/null +++ b/src/Sql/dbo/Views/WebAuthnCredentialView.sql @@ -0,0 +1,6 @@ +CREATE VIEW [dbo].[WebAuthnCredentialView] +AS +SELECT + * +FROM + [dbo].[WebAuthnCredential] From 9dc24de4550d8f5837a8a66c259209bfdbef32e6 Mon Sep 17 00:00:00 2001 From: Kyle Spearrin Date: Fri, 28 Apr 2023 16:16:52 -0400 Subject: [PATCH 02/50] stub out registration implementations --- .../Auth/Controllers/WebAuthnController.cs | 42 ++---------- src/Core/Services/IUserService.cs | 2 + .../Services/Implementations/UserService.cs | 66 ++++++++++++++++++- 3 files changed, 74 insertions(+), 36 deletions(-) diff --git a/src/Api/Auth/Controllers/WebAuthnController.cs b/src/Api/Auth/Controllers/WebAuthnController.cs index 9c9025ff610a..860d5d218ce2 100644 --- a/src/Api/Auth/Controllers/WebAuthnController.cs +++ b/src/Api/Auth/Controllers/WebAuthnController.cs @@ -1,22 +1,10 @@ using Bit.Api.Auth.Models.Request; -using Bit.Api.Auth.Models.Request.Accounts; using Bit.Api.Auth.Models.Response.TwoFactor; -using Bit.Api.Models.Request; using Bit.Api.Models.Response; -using Bit.Api.Vault.Models.Response; -using Bit.Core.Auth.Enums; -using Bit.Core.Auth.LoginFeatures.PasswordlessLogin.Interfaces; -using Bit.Core.Auth.Utilities; -using Bit.Core.Context; -using Bit.Core.Entities; using Bit.Core.Exceptions; -using Bit.Core.Repositories; using Bit.Core.Services; -using Bit.Core.Settings; -using Bit.Core.Utilities; using Fido2NetLib; using Microsoft.AspNetCore.Authorization; -using Microsoft.AspNetCore.Identity; using Microsoft.AspNetCore.Mvc; namespace Bit.Api.Auth.Controllers; @@ -26,32 +14,15 @@ namespace Bit.Api.Auth.Controllers; public class WebAuthnController : Controller { private readonly IUserService _userService; - private readonly IOrganizationRepository _organizationRepository; - private readonly IOrganizationService _organizationService; - private readonly GlobalSettings _globalSettings; - private readonly UserManager _userManager; - private readonly ICurrentContext _currentContext; - private readonly IVerifyAuthRequestCommand _verifyAuthRequestCommand; public WebAuthnController( - IUserService userService, - IOrganizationRepository organizationRepository, - IOrganizationService organizationService, - GlobalSettings globalSettings, - UserManager userManager, - ICurrentContext currentContext, - IVerifyAuthRequestCommand verifyAuthRequestCommand) + IUserService userService) { _userService = userService; - _organizationRepository = organizationRepository; - _organizationService = organizationService; - _globalSettings = globalSettings; - _userManager = userManager; - _currentContext = currentContext; - _verifyAuthRequestCommand = verifyAuthRequestCommand; } [HttpGet("")] + // TODO: Create proper models for this call public async Task> Get() { var user = await _userService.GetUserByPrincipalAsync(User); @@ -64,7 +35,7 @@ public async Task> Get() [HttpPost("options")] [ApiExplorerSettings(IgnoreApi = true)] // Disable Swagger due to CredentialCreateOptions not converting properly - public async Task PostOptions([FromBody] SecretVerificationRequestModel model) + public async Task PostOptions() { var user = await _userService.GetUserByPrincipalAsync(User); if (user == null) @@ -72,11 +43,12 @@ public async Task PostOptions([FromBody] SecretVerifica throw new UnauthorizedAccessException(); } - var reg = await _userService.StartWebAuthnRegistrationAsync(user); + var reg = await _userService.StartWebAuthnLoginRegistrationAsync(user); return reg; } [HttpPost("")] + // TODO: Create proper models for this call public async Task Post([FromBody] TwoFactorWebAuthnRequestModel model) { var user = await _userService.GetUserByPrincipalAsync(User); @@ -85,8 +57,7 @@ public async Task Post([FromBody] TwoFactorWebAu throw new UnauthorizedAccessException(); } - var success = await _userService.CompleteWebAuthRegistrationAsync( - user, model.Id.Value, model.Name, model.DeviceResponse); + var success = await _userService.CompleteWebAuthLoginRegistrationAsync(user, model.Name, model.DeviceResponse); if (!success) { throw new BadRequestException("Unable to complete WebAuthn registration."); @@ -104,5 +75,6 @@ public async Task Delete(string id) { throw new UnauthorizedAccessException(); } + // TODO: Delete } } diff --git a/src/Core/Services/IUserService.cs b/src/Core/Services/IUserService.cs index d0c078d40645..df8993d45a9f 100644 --- a/src/Core/Services/IUserService.cs +++ b/src/Core/Services/IUserService.cs @@ -27,6 +27,8 @@ public interface IUserService Task StartWebAuthnRegistrationAsync(User user); Task DeleteWebAuthnKeyAsync(User user, int id); Task CompleteWebAuthRegistrationAsync(User user, int value, string name, AuthenticatorAttestationRawResponse attestationResponse); + Task StartWebAuthnLoginRegistrationAsync(User user); + Task CompleteWebAuthLoginRegistrationAsync(User user, string name, AuthenticatorAttestationRawResponse attestationResponse); Task SendEmailVerificationAsync(User user); Task ConfirmEmailAsync(User user, string token); Task InitiateEmailChangeAsync(User user, string newEmail); diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index 8aa2dd85c13c..932611415a13 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -1,7 +1,9 @@ using System.Security.Claims; using System.Text.Json; +using Bit.Core.Auth.Entities; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Models; +using Bit.Core.Auth.Repositories; using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Enums; @@ -54,6 +56,7 @@ public class UserService : UserManager, IUserService, IDisposable private readonly IOrganizationService _organizationService; private readonly IProviderUserRepository _providerUserRepository; private readonly IStripeSyncService _stripeSyncService; + private readonly IWebAuthnRepository _webAuthnRepository; public UserService( IUserRepository userRepository, @@ -83,7 +86,8 @@ public UserService( IGlobalSettings globalSettings, IOrganizationService organizationService, IProviderUserRepository providerUserRepository, - IStripeSyncService stripeSyncService) + IStripeSyncService stripeSyncService, + IWebAuthnRepository webAuthnRepository) : base( store, optionsAccessor, @@ -119,6 +123,7 @@ public UserService( _organizationService = organizationService; _providerUserRepository = providerUserRepository; _stripeSyncService = stripeSyncService; + _webAuthnRepository = webAuthnRepository; } public Guid? GetProperUserId(ClaimsPrincipal principal) @@ -499,6 +504,65 @@ public async Task DeleteWebAuthnKeyAsync(User user, int id) return true; } + public async Task StartWebAuthnLoginRegistrationAsync(User user) + { + var fidoUser = new Fido2User + { + DisplayName = user.Name, + Name = user.Email, + Id = user.Id.ToByteArray(), + }; + + // Get existing keys to exclude + var existingKeys = await _webAuthnRepository.GetManyByUserIdAsync(user.Id); + var excludeCredentials = existingKeys + .Select(k => new PublicKeyCredentialDescriptor(CoreHelpers.Base64UrlDecode(k.DescriptorId))) + .ToList(); + + var authenticatorSelection = new AuthenticatorSelection + { + AuthenticatorAttachment = null, + RequireResidentKey = false, + UserVerification = UserVerificationRequirement.Preferred + }; + + // TODO: PRF + var extensions = new AuthenticationExtensionsClientInputs { }; + + var options = _fido2.RequestNewCredential(fidoUser, excludeCredentials, authenticatorSelection, + AttestationConveyancePreference.None, extensions); + + // TODO: temp save options to user record somehow + + return options; + } + + public async Task CompleteWebAuthLoginRegistrationAsync(User user, string name, + AuthenticatorAttestationRawResponse attestationResponse) + { + // TODO: Get options from user record somehow, then clear them + var options = CredentialCreateOptions.FromJson(""); + + // TODO: Callback to ensure credential ID is unique. Do we care? I don't think so. + IsCredentialIdUniqueToUserAsyncDelegate callback = (args, cancellationToken) => Task.FromResult(true); + + var success = await _fido2.MakeNewCredentialAsync(attestationResponse, options, callback); + + var credential = new WebAuthnCredential + { + Name = name, + DescriptorId = CoreHelpers.Base64UrlEncode(success.Result.CredentialId), + PublicKey = CoreHelpers.Base64UrlEncode(success.Result.PublicKey), + Type = success.Result.CredType, + AaGuid = success.Result.Aaguid, + Counter = (int)success.Result.Counter, + UserId = user.Id + }; + + await _webAuthnRepository.CreateAsync(credential); + return true; + } + public async Task SendEmailVerificationAsync(User user) { if (user.EmailVerified) From f33570e3ad831e18367f504b8467de4fa38f0d23 Mon Sep 17 00:00:00 2001 From: Kyle Spearrin Date: Mon, 1 May 2023 10:17:08 -0400 Subject: [PATCH 03/50] stub out assertion steps and token issuance --- src/Core/Services/IUserService.cs | 2 + .../Services/Implementations/UserService.cs | 61 +++++++++++++++++++ .../Controllers/AccountsController.cs | 42 +++++++------ 3 files changed, 88 insertions(+), 17 deletions(-) diff --git a/src/Core/Services/IUserService.cs b/src/Core/Services/IUserService.cs index df8993d45a9f..49a8c9a7b8c5 100644 --- a/src/Core/Services/IUserService.cs +++ b/src/Core/Services/IUserService.cs @@ -29,6 +29,8 @@ public interface IUserService Task CompleteWebAuthRegistrationAsync(User user, int value, string name, AuthenticatorAttestationRawResponse attestationResponse); Task StartWebAuthnLoginRegistrationAsync(User user); Task CompleteWebAuthLoginRegistrationAsync(User user, string name, AuthenticatorAttestationRawResponse attestationResponse); + Task StartWebAuthnLoginAssertionAsync(User user); + Task CompleteWebAuthLoginAssertionAsync(AuthenticatorAssertionRawResponse assertionResponse, User user); Task SendEmailVerificationAsync(User user); Task ConfirmEmailAsync(User user, string token); Task InitiateEmailChangeAsync(User user, string newEmail); diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index 932611415a13..807ee428c170 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -24,6 +24,8 @@ using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; +using Microsoft.Identity.Client.Platforms.Features.DesktopOs.Kerberos; +using static IdentityServer4.Models.IdentityResources; using File = System.IO.File; namespace Bit.Core.Services; @@ -563,6 +565,65 @@ public async Task CompleteWebAuthLoginRegistrationAsync(User user, string return true; } + public async Task StartWebAuthnLoginAssertionAsync(User user) + { + var provider = user.GetTwoFactorProvider(TwoFactorProviderType.WebAuthn); + var existingKeys = await _webAuthnRepository.GetManyByUserIdAsync(user.Id); + var existingCredentials = existingKeys + .Select(k => new PublicKeyCredentialDescriptor(CoreHelpers.Base64UrlDecode(k.DescriptorId))) + .ToList(); + + if (existingCredentials.Count == 0) + { + return null; + } + + // TODO: PRF? + var exts = new AuthenticationExtensionsClientInputs + { + UserVerificationMethod = true + }; + var options = _fido2.GetAssertionOptions(existingCredentials, UserVerificationRequirement.Preferred, exts); + + // TODO: temp save options to user record somehow + + return options; + } + + public async Task CompleteWebAuthLoginAssertionAsync(AuthenticatorAssertionRawResponse assertionResponse, User user) + { + // TODO: Get options from user record somehow, then clear them + var options = AssertionOptions.FromJson(""); + + var userCredentials = await _webAuthnRepository.GetManyByUserIdAsync(user.Id); + var assertionId = CoreHelpers.Base64UrlEncode(assertionResponse.Id); + var credential = userCredentials.FirstOrDefault(c => c.DescriptorId == assertionId); + if (credential == null) + { + return null; + } + + // TODO: Callback to ensure credential ID is unique. Do we care? I don't think so. + IsUserHandleOwnerOfCredentialIdAsync callback = (args, cancellationToken) => Task.FromResult(true); + var credentialPublicKey = CoreHelpers.Base64UrlDecode(credential.PublicKey); + var assertionVerificationResult = await _fido2.MakeAssertionAsync( + assertionResponse, options, credentialPublicKey, (uint)credential.Counter, callback); + + // Update SignatureCounter + credential.Counter = (int)assertionVerificationResult.Counter; + await _webAuthnRepository.ReplaceAsync(credential); + + if (assertionVerificationResult.Status == "ok") + { + var token = await base.GenerateUserTokenAsync(user, TokenOptions.DefaultEmailProvider, "webAuthnLogin"); + return token; + } + else + { + return null; + } + } + public async Task SendEmailVerificationAsync(User user) { if (user.EmailVerified) diff --git a/src/Identity/Controllers/AccountsController.cs b/src/Identity/Controllers/AccountsController.cs index 4e89d984fc8e..55184e3c0c2c 100644 --- a/src/Identity/Controllers/AccountsController.cs +++ b/src/Identity/Controllers/AccountsController.cs @@ -1,4 +1,6 @@ -using Bit.Core.Auth.Models.Api.Request.Accounts; +using Bit.Core.Auth.Enums; +using System.Text.Json; +using Bit.Core.Auth.Models.Api.Request.Accounts; using Bit.Core.Auth.Models.Api.Response.Accounts; using Bit.Core.Auth.Services; using Bit.Core.Auth.Utilities; @@ -8,7 +10,9 @@ using Bit.Core.Repositories; using Bit.Core.Services; using Bit.SharedWeb.Utilities; +using Fido2NetLib; using Microsoft.AspNetCore.Mvc; +using Bit.Identity.IdentityServer; namespace Bit.Identity.Controllers; @@ -72,30 +76,34 @@ public async Task PostPrelogin([FromBody] PreloginRequest return new PreloginResponseModel(kdfInformation); } - [HttpPost("webauthn")] - public async Task PostWebAuthn([FromBody] PreloginRequestModel model) + [HttpPost("webauthn-assertion-options")] + [ApiExplorerSettings(IgnoreApi = true)] // Disable Swagger due to CredentialCreateOptions not converting properly + // TODO: Create proper models for this call + public async Task PostWebAuthnAssertionOptions([FromBody] PreloginRequestModel model) { - var kdfInformation = await _userRepository.GetKdfInformationByEmailAsync(model.Email); - if (kdfInformation == null) + var user = await _userRepository.GetByEmailAsync(model.Email); + if (user == null) { - kdfInformation = new UserKdfInformation - { - Kdf = KdfType.PBKDF2_SHA256, - KdfIterations = 100000, - }; + // TODO: return something? possible enumeration attacks with this response + return new AssertionOptions(); } - return new PreloginResponseModel(kdfInformation); - } - - [HttpPost("webauthn-assertion-options")] - public async Task PostWebAuthnAssertionOptions([FromBody] PreloginRequestModel model) - { + var options = await _userService.StartWebAuthnLoginAssertionAsync(user); + return options; } [HttpPost("webauthn-assertion")] - public async Task PostWebAuthnAssertion([FromBody] PreloginRequestModel model) + // TODO: Create proper models for this call + public async Task PostWebAuthnAssertion([FromBody] PreloginRequestModel model) { + var user = await _userRepository.GetByEmailAsync(model.Email); + if (user == null) + { + // TODO: proper response here? + throw new BadRequestException(); + } + var token = await _userService.CompleteWebAuthLoginAssertionAsync(null, user); + return token; } } From bd01206332bdccbf60df32fe3292c7660f023364 Mon Sep 17 00:00:00 2001 From: Kyle Spearrin Date: Mon, 1 May 2023 10:24:52 -0400 Subject: [PATCH 04/50] verify token --- .../Services/Implementations/UserService.cs | 2 +- .../IdentityServer/ExtensionGrantValidator.cs | 18 ++++++++++++++---- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index 807ee428c170..8652f13adf68 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -615,7 +615,7 @@ public async Task CompleteWebAuthLoginAssertionAsync(AuthenticatorAssert if (assertionVerificationResult.Status == "ok") { - var token = await base.GenerateUserTokenAsync(user, TokenOptions.DefaultEmailProvider, "webAuthnLogin"); + var token = await base.GenerateUserTokenAsync(user, TokenOptions.DefaultAuthenticatorProvider, "webAuthnLogin"); return token; } else diff --git a/src/Identity/IdentityServer/ExtensionGrantValidator.cs b/src/Identity/IdentityServer/ExtensionGrantValidator.cs index 51462e8263f1..6684a8014132 100644 --- a/src/Identity/IdentityServer/ExtensionGrantValidator.cs +++ b/src/Identity/IdentityServer/ExtensionGrantValidator.cs @@ -45,7 +45,9 @@ public ExtensionGrantValidator( public async Task ValidateAsync(ExtensionGrantValidationContext context) { var email = context.Request.Raw.Get("email"); - if (string.IsNullOrWhiteSpace(email)) + var token = context.Request.Raw.Get("token"); + var type = context.Request.Raw.Get("type"); + if (string.IsNullOrWhiteSpace(email) || string.IsNullOrWhiteSpace(token) || string.IsNullOrWhiteSpace(type)) { context.Result = new GrantValidationResult(TokenRequestErrors.InvalidGrant); return; @@ -61,11 +63,19 @@ public async Task ValidateAsync(ExtensionGrantValidationContext context) await ValidateAsync(context, context.Request, validatorContext); } - protected override Task ValidateContextAsync(ExtensionGrantValidationContext context, + protected override async Task ValidateContextAsync(ExtensionGrantValidationContext context, CustomValidatorRequestContext validatorContext) { - var email = context.Request.Raw.Get("email"); - return Task.FromResult(!string.IsNullOrWhiteSpace(email) && validatorContext.User != null); + var token = context.Request.Raw.Get("token"); + var type = context.Request.Raw.Get("type"); + if (validatorContext.User == null || string.IsNullOrWhiteSpace(token) || string.IsNullOrWhiteSpace(type)) + { + return false; + } + var purpose = type == "webAuthn" ? "webAuthnLogin" : null; + var verified = await _userManager.VerifyUserTokenAsync(validatorContext.User, + TokenOptions.DefaultAuthenticatorProvider, purpose, token); + return verified; } protected override Task SetSuccessResult(ExtensionGrantValidationContext context, User user, From 0abba03d0fb496e4d00bf679aebadc739b6b47e7 Mon Sep 17 00:00:00 2001 From: Kyle Spearrin Date: Mon, 1 May 2023 10:52:01 -0400 Subject: [PATCH 05/50] webauthn tokenable --- .../Tokenables/WebAuthnLoginTokenable.cs | 44 +++++++++++++++++++ .../Services/Implementations/UserService.cs | 9 +++- .../IdentityServer/ExtensionGrantValidator.cs | 12 +++-- .../Utilities/ServiceCollectionExtensions.cs | 6 +++ 4 files changed, 65 insertions(+), 6 deletions(-) create mode 100644 src/Core/Auth/Models/Business/Tokenables/WebAuthnLoginTokenable.cs diff --git a/src/Core/Auth/Models/Business/Tokenables/WebAuthnLoginTokenable.cs b/src/Core/Auth/Models/Business/Tokenables/WebAuthnLoginTokenable.cs new file mode 100644 index 000000000000..f57d40a6273a --- /dev/null +++ b/src/Core/Auth/Models/Business/Tokenables/WebAuthnLoginTokenable.cs @@ -0,0 +1,44 @@ +using System.Text.Json.Serialization; +using Bit.Core.Entities; +using Bit.Core.Tokens; + +namespace Bit.Core.Auth.Models.Business.Tokenables; + +public class WebAuthnLoginTokenable : ExpiringTokenable +{ + private const double _tokenLifetimeInHours = (double)1 / 60; // 1 minute + public const string ClearTextPrefix = "BWWebAuthnLogin_"; + public const string DataProtectorPurpose = "WebAuthnLoginDataProtector"; + public const string TokenIdentifier = "WebAuthnLoginToken"; + + public string Identifier { get; set; } = TokenIdentifier; + public Guid Id { get; set; } + public string Email { get; set; } + + [JsonConstructor] + public WebAuthnLoginTokenable() + { + ExpirationDate = DateTime.UtcNow.AddHours(_tokenLifetimeInHours); + ExpirationDate = DateTime.UtcNow.AddHours(_tokenLifetimeInHours); + } + + public WebAuthnLoginTokenable(User user) : this() + { + Id = user?.Id ?? default; + Email = user?.Email; + } + + public bool TokenIsValid(User user) + { + if (Id == default || Email == default || user == null) + { + return false; + } + + return Id == user.Id && + Email.Equals(user.Email, StringComparison.InvariantCultureIgnoreCase); + } + + // Validates deserialized + protected override bool TokenIsValid() => Identifier == TokenIdentifier && Id != default && !string.IsNullOrWhiteSpace(Email); +} diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index 8652f13adf68..33e05e35e097 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -3,6 +3,7 @@ using Bit.Core.Auth.Entities; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Models; +using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Auth.Repositories; using Bit.Core.Context; using Bit.Core.Entities; @@ -11,6 +12,7 @@ using Bit.Core.Models.Business; using Bit.Core.Repositories; using Bit.Core.Settings; +using Bit.Core.Tokens; using Bit.Core.Tools.Entities; using Bit.Core.Tools.Enums; using Bit.Core.Tools.Models.Business; @@ -59,6 +61,7 @@ public class UserService : UserManager, IUserService, IDisposable private readonly IProviderUserRepository _providerUserRepository; private readonly IStripeSyncService _stripeSyncService; private readonly IWebAuthnRepository _webAuthnRepository; + private readonly IDataProtectorTokenFactory _webAuthnLoginTokenizer; public UserService( IUserRepository userRepository, @@ -89,7 +92,8 @@ public UserService( IOrganizationService organizationService, IProviderUserRepository providerUserRepository, IStripeSyncService stripeSyncService, - IWebAuthnRepository webAuthnRepository) + IWebAuthnRepository webAuthnRepository, + IDataProtectorTokenFactory webAuthnLoginTokenizer) : base( store, optionsAccessor, @@ -126,6 +130,7 @@ public UserService( _providerUserRepository = providerUserRepository; _stripeSyncService = stripeSyncService; _webAuthnRepository = webAuthnRepository; + _webAuthnLoginTokenizer = webAuthnLoginTokenizer; } public Guid? GetProperUserId(ClaimsPrincipal principal) @@ -615,7 +620,7 @@ public async Task CompleteWebAuthLoginAssertionAsync(AuthenticatorAssert if (assertionVerificationResult.Status == "ok") { - var token = await base.GenerateUserTokenAsync(user, TokenOptions.DefaultAuthenticatorProvider, "webAuthnLogin"); + var token = _webAuthnLoginTokenizer.Protect(new WebAuthnLoginTokenable(user)); return token; } else diff --git a/src/Identity/IdentityServer/ExtensionGrantValidator.cs b/src/Identity/IdentityServer/ExtensionGrantValidator.cs index 6684a8014132..c476e6eb19f8 100644 --- a/src/Identity/IdentityServer/ExtensionGrantValidator.cs +++ b/src/Identity/IdentityServer/ExtensionGrantValidator.cs @@ -8,12 +8,15 @@ using Bit.Core.Services; using Bit.Core.Entities; using Bit.Core.Settings; +using Bit.Core.Tokens; +using Bit.Core.Auth.Models.Business.Tokenables; namespace Bit.Identity.IdentityServer; public class ExtensionGrantValidator : BaseRequestValidator, IExtensionGrantValidator { private UserManager _userManager; + private readonly IDataProtectorTokenFactory _webAuthnLoginTokenizer; public ExtensionGrantValidator( UserManager userManager, @@ -31,13 +34,15 @@ public ExtensionGrantValidator( GlobalSettings globalSettings, IPolicyRepository policyRepository, IUserRepository userRepository, - IPolicyService policyService) + IPolicyService policyService, + IDataProtectorTokenFactory webAuthnLoginTokenizer) : base(userManager, deviceRepository, deviceService, userService, eventService, organizationDuoWebTokenProvider, organizationRepository, organizationUserRepository, applicationCacheService, mailService, logger, currentContext, globalSettings, policyRepository, userRepository, policyService) { _userManager = userManager; + _webAuthnLoginTokenizer = webAuthnLoginTokenizer; } public string GrantType => "extension"; @@ -72,9 +77,8 @@ protected override async Task ValidateContextAsync(ExtensionGrantValidatio { return false; } - var purpose = type == "webAuthn" ? "webAuthnLogin" : null; - var verified = await _userManager.VerifyUserTokenAsync(validatorContext.User, - TokenOptions.DefaultAuthenticatorProvider, purpose, token); + var verified = _webAuthnLoginTokenizer.TryUnprotect(token, out var tokenData) && + tokenData.Valid && tokenData.TokenIsValid(validatorContext.User); return verified; } diff --git a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs index fbea8faa9c01..60fb7f4c0226 100644 --- a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs +++ b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs @@ -157,6 +157,12 @@ public static void AddTokenizers(this IServiceCollection services) SsoTokenable.DataProtectorPurpose, serviceProvider.GetDataProtectionProvider(), serviceProvider.GetRequiredService>>())); + services.AddSingleton>(serviceProvider => + new DataProtectorTokenFactory( + WebAuthnLoginTokenable.ClearTextPrefix, + WebAuthnLoginTokenable.DataProtectorPurpose, + serviceProvider.GetDataProtectionProvider(), + serviceProvider.GetRequiredService>>())); } public static void AddDefaultServices(this IServiceCollection services, GlobalSettings globalSettings) From 6d4aeff8388c6f5d85481378a6fc0d28d10b118d Mon Sep 17 00:00:00 2001 From: Kyle Spearrin Date: Mon, 1 May 2023 10:54:42 -0400 Subject: [PATCH 06/50] remove duplicate expiration set --- .../Auth/Models/Business/Tokenables/WebAuthnLoginTokenable.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Core/Auth/Models/Business/Tokenables/WebAuthnLoginTokenable.cs b/src/Core/Auth/Models/Business/Tokenables/WebAuthnLoginTokenable.cs index f57d40a6273a..b27b1fb35538 100644 --- a/src/Core/Auth/Models/Business/Tokenables/WebAuthnLoginTokenable.cs +++ b/src/Core/Auth/Models/Business/Tokenables/WebAuthnLoginTokenable.cs @@ -19,7 +19,6 @@ public class WebAuthnLoginTokenable : ExpiringTokenable public WebAuthnLoginTokenable() { ExpirationDate = DateTime.UtcNow.AddHours(_tokenLifetimeInHours); - ExpirationDate = DateTime.UtcNow.AddHours(_tokenLifetimeInHours); } public WebAuthnLoginTokenable(User user) : this() From 74e56eeac0d22e98029ea686a0172ba858103e39 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Mon, 8 May 2023 09:23:48 +0200 Subject: [PATCH 07/50] [PM-2014] chore: rename `IWebAuthnRespository` to `IWebAuthnCredentialRepository` --- ...itory.cs => IWebAuthnCredentialRepository.cs} | 2 +- src/Core/Services/Implementations/UserService.cs | 16 ++++++++-------- .../Repositories/WebAuthnCredentialRepository.cs | 2 +- .../Repositories/WebAuthnCredentialRepository.cs | 2 +- 4 files changed, 11 insertions(+), 11 deletions(-) rename src/Core/Auth/Repositories/{IWebAuthnRepository.cs => IWebAuthnCredentialRepository.cs} (67%) diff --git a/src/Core/Auth/Repositories/IWebAuthnRepository.cs b/src/Core/Auth/Repositories/IWebAuthnCredentialRepository.cs similarity index 67% rename from src/Core/Auth/Repositories/IWebAuthnRepository.cs rename to src/Core/Auth/Repositories/IWebAuthnCredentialRepository.cs index 751c49429af4..1758ea90e8ee 100644 --- a/src/Core/Auth/Repositories/IWebAuthnRepository.cs +++ b/src/Core/Auth/Repositories/IWebAuthnCredentialRepository.cs @@ -3,7 +3,7 @@ namespace Bit.Core.Auth.Repositories; -public interface IWebAuthnRepository : IRepository +public interface IWebAuthnCredentialRepository : IRepository { Task> GetManyByUserIdAsync(Guid userId); } diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index 33e05e35e097..4f14bc4ad272 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -60,7 +60,7 @@ public class UserService : UserManager, IUserService, IDisposable private readonly IOrganizationService _organizationService; private readonly IProviderUserRepository _providerUserRepository; private readonly IStripeSyncService _stripeSyncService; - private readonly IWebAuthnRepository _webAuthnRepository; + private readonly IWebAuthnCredentialRepository _webAuthnCredentialRepository; private readonly IDataProtectorTokenFactory _webAuthnLoginTokenizer; public UserService( @@ -92,7 +92,7 @@ public UserService( IOrganizationService organizationService, IProviderUserRepository providerUserRepository, IStripeSyncService stripeSyncService, - IWebAuthnRepository webAuthnRepository, + IWebAuthnCredentialRepository webAuthnRepository, IDataProtectorTokenFactory webAuthnLoginTokenizer) : base( store, @@ -129,7 +129,7 @@ public UserService( _organizationService = organizationService; _providerUserRepository = providerUserRepository; _stripeSyncService = stripeSyncService; - _webAuthnRepository = webAuthnRepository; + _webAuthnCredentialRepository = webAuthnRepository; _webAuthnLoginTokenizer = webAuthnLoginTokenizer; } @@ -521,7 +521,7 @@ public async Task StartWebAuthnLoginRegistrationAsync(U }; // Get existing keys to exclude - var existingKeys = await _webAuthnRepository.GetManyByUserIdAsync(user.Id); + var existingKeys = await _webAuthnCredentialRepository.GetManyByUserIdAsync(user.Id); var excludeCredentials = existingKeys .Select(k => new PublicKeyCredentialDescriptor(CoreHelpers.Base64UrlDecode(k.DescriptorId))) .ToList(); @@ -566,14 +566,14 @@ public async Task CompleteWebAuthLoginRegistrationAsync(User user, string UserId = user.Id }; - await _webAuthnRepository.CreateAsync(credential); + await _webAuthnCredentialRepository.CreateAsync(credential); return true; } public async Task StartWebAuthnLoginAssertionAsync(User user) { var provider = user.GetTwoFactorProvider(TwoFactorProviderType.WebAuthn); - var existingKeys = await _webAuthnRepository.GetManyByUserIdAsync(user.Id); + var existingKeys = await _webAuthnCredentialRepository.GetManyByUserIdAsync(user.Id); var existingCredentials = existingKeys .Select(k => new PublicKeyCredentialDescriptor(CoreHelpers.Base64UrlDecode(k.DescriptorId))) .ToList(); @@ -600,7 +600,7 @@ public async Task CompleteWebAuthLoginAssertionAsync(AuthenticatorAssert // TODO: Get options from user record somehow, then clear them var options = AssertionOptions.FromJson(""); - var userCredentials = await _webAuthnRepository.GetManyByUserIdAsync(user.Id); + var userCredentials = await _webAuthnCredentialRepository.GetManyByUserIdAsync(user.Id); var assertionId = CoreHelpers.Base64UrlEncode(assertionResponse.Id); var credential = userCredentials.FirstOrDefault(c => c.DescriptorId == assertionId); if (credential == null) @@ -616,7 +616,7 @@ public async Task CompleteWebAuthLoginAssertionAsync(AuthenticatorAssert // Update SignatureCounter credential.Counter = (int)assertionVerificationResult.Counter; - await _webAuthnRepository.ReplaceAsync(credential); + await _webAuthnCredentialRepository.ReplaceAsync(credential); if (assertionVerificationResult.Status == "ok") { diff --git a/src/Infrastructure.Dapper/Auth/Repositories/WebAuthnCredentialRepository.cs b/src/Infrastructure.Dapper/Auth/Repositories/WebAuthnCredentialRepository.cs index 72e15119fb0b..943ae2f09956 100644 --- a/src/Infrastructure.Dapper/Auth/Repositories/WebAuthnCredentialRepository.cs +++ b/src/Infrastructure.Dapper/Auth/Repositories/WebAuthnCredentialRepository.cs @@ -9,7 +9,7 @@ namespace Bit.Infrastructure.Dapper.Auth.Repositories; -public class WebAuthnCredentialRepository : Repository, IWebAuthnRepository +public class WebAuthnCredentialRepository : Repository, IWebAuthnCredentialRepository { public WebAuthnCredentialRepository(GlobalSettings globalSettings) : this(globalSettings.SqlServer.ConnectionString, globalSettings.SqlServer.ReadOnlyConnectionString) diff --git a/src/Infrastructure.EntityFramework/Auth/Repositories/WebAuthnCredentialRepository.cs b/src/Infrastructure.EntityFramework/Auth/Repositories/WebAuthnCredentialRepository.cs index 9d2cf81e7b4e..c86d7bd20aad 100644 --- a/src/Infrastructure.EntityFramework/Auth/Repositories/WebAuthnCredentialRepository.cs +++ b/src/Infrastructure.EntityFramework/Auth/Repositories/WebAuthnCredentialRepository.cs @@ -7,7 +7,7 @@ namespace Bit.Infrastructure.EntityFramework.Auth.Repositories; -public class WebAuthnCredentialRepository : Repository, IWebAuthnRepository +public class WebAuthnCredentialRepository : Repository, IWebAuthnCredentialRepository { public WebAuthnCredentialRepository(IServiceScopeFactory serviceScopeFactory, IMapper mapper) : base(serviceScopeFactory, mapper, (context) => context.WebAuthnCredentials) From cdf531dd7190f2bbdeaa9dd9bb1cc56bb3e58f7b Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Mon, 8 May 2023 09:24:50 +0200 Subject: [PATCH 08/50] [PM-2014] fix: add missing service registration --- src/Infrastructure.Dapper/DapperServiceCollectionExtensions.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Infrastructure.Dapper/DapperServiceCollectionExtensions.cs b/src/Infrastructure.Dapper/DapperServiceCollectionExtensions.cs index d7082184d36e..3a2233f4cb61 100644 --- a/src/Infrastructure.Dapper/DapperServiceCollectionExtensions.cs +++ b/src/Infrastructure.Dapper/DapperServiceCollectionExtensions.cs @@ -45,6 +45,7 @@ public static void AddDapperRepositories(this IServiceCollection services, bool services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); + services.AddSingleton(); if (selfHosted) { From 00d92ed031eefc9ca990787145ea1dd29b321714 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Mon, 8 May 2023 13:24:04 +0200 Subject: [PATCH 09/50] [PM-2014] feat: add user verification when fetching options --- .../Auth/Controllers/WebAuthnController.cs | 34 +++++++++++++++---- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/src/Api/Auth/Controllers/WebAuthnController.cs b/src/Api/Auth/Controllers/WebAuthnController.cs index 860d5d218ce2..ca28c714c44e 100644 --- a/src/Api/Auth/Controllers/WebAuthnController.cs +++ b/src/Api/Auth/Controllers/WebAuthnController.cs @@ -1,6 +1,8 @@ using Bit.Api.Auth.Models.Request; +using Bit.Api.Auth.Models.Request.Accounts; using Bit.Api.Auth.Models.Response.TwoFactor; using Bit.Api.Models.Response; +using Bit.Core.Entities; using Bit.Core.Exceptions; using Bit.Core.Services; using Fido2NetLib; @@ -35,14 +37,9 @@ public async Task> Get() [HttpPost("options")] [ApiExplorerSettings(IgnoreApi = true)] // Disable Swagger due to CredentialCreateOptions not converting properly - public async Task PostOptions() + public async Task PostOptions([FromBody] SecretVerificationRequestModel model) { - var user = await _userService.GetUserByPrincipalAsync(User); - if (user == null) - { - throw new UnauthorizedAccessException(); - } - + var user = await CheckAsync(model); var reg = await _userService.StartWebAuthnLoginRegistrationAsync(user); return reg; } @@ -77,4 +74,27 @@ public async Task Delete(string id) } // TODO: Delete } + + private async Task CheckAsync(SecretVerificationRequestModel model) + { + var user = await _userService.GetUserByPrincipalAsync(User); + if (user == null) + { + throw new UnauthorizedAccessException(); + } + + if (!await _userService.VerifySecretAsync(user, model.Secret)) + { + await Task.Delay(2000); + throw new BadRequestException(string.Empty, "User verification failed."); + } + + // TODO: Is premium requried? + //if (premium && !(await _userService.CanAccessPremium(user))) + //{ + // throw new BadRequestException("Premium status is required."); + //} + + return user; + } } From f8fd97ed00ffa4bb776b41bd6895603c1932bf83 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Mon, 8 May 2023 13:40:43 +0200 Subject: [PATCH 10/50] [PM-2014] feat: create migration script for mssql --- ...2023-05-08-00_WebAuthnLoginCredentials.sql | 152 ++++++++++++++++++ 1 file changed, 152 insertions(+) create mode 100644 util/Migrator/DbScripts/2023-05-08-00_WebAuthnLoginCredentials.sql diff --git a/util/Migrator/DbScripts/2023-05-08-00_WebAuthnLoginCredentials.sql b/util/Migrator/DbScripts/2023-05-08-00_WebAuthnLoginCredentials.sql new file mode 100644 index 000000000000..a0fec82e1247 --- /dev/null +++ b/util/Migrator/DbScripts/2023-05-08-00_WebAuthnLoginCredentials.sql @@ -0,0 +1,152 @@ +CREATE TABLE [dbo].[WebAuthnCredential] ( + [Id] UNIQUEIDENTIFIER NOT NULL, + [UserId] UNIQUEIDENTIFIER NOT NULL, + [Name] NVARCHAR (50) NOT NULL, + [PublicKey] VARCHAR (256) NOT NULL, + [DescriptorId] VARCHAR (256) NOT NULL, + [Counter] INT NOT NULL, + [Type] VARCHAR (20) NULL, + [AaGuid] UNIQUEIDENTIFIER NOT NULL, + [UserKey] VARCHAR (MAX) NULL, + [CreationDate] DATETIME2 (7) NOT NULL, + [RevisionDate] DATETIME2 (7) NOT NULL, + CONSTRAINT [PK_WebAuthnCredential] PRIMARY KEY CLUSTERED ([Id] ASC), + CONSTRAINT [FK_WebAuthnCredential_User] FOREIGN KEY ([UserId]) REFERENCES [dbo].[User] ([Id]) +); + +GO +CREATE NONCLUSTERED INDEX [IX_WebAuthnCredential_UserId] + ON [dbo].[WebAuthnCredential]([UserId] ASC); + +GO +CREATE VIEW [dbo].[WebAuthnCredentialView] +AS +SELECT + * +FROM + [dbo].[WebAuthnCredential] + +GO +CREATE PROCEDURE [dbo].[WebAuthnCredential_Create] + @Id UNIQUEIDENTIFIER OUTPUT, + @UserId UNIQUEIDENTIFIER, + @Name NVARCHAR(50), + @PublicKey VARCHAR (256), + @DescriptorId VARCHAR(256), + @Counter INT, + @Type VARCHAR(20), + @AaGuid UNIQUEIDENTIFIER, + @UserKey VARCHAR (MAX), + @CreationDate DATETIME2(7), + @RevisionDate DATETIME2(7) +AS +BEGIN + SET NOCOUNT ON + + INSERT INTO [dbo].[WebAuthnCredential] + ( + [Id], + [UserId], + [Name], + [PublicKey], + [DescriptorId], + [Counter], + [Type], + [AaGuid], + [UserKey], + [CreationDate], + [RevisionDate] + ) + VALUES + ( + @Id, + @UserId, + @Name, + @PublicKey, + @DescriptorId, + @Counter, + @Type, + @AaGuid, + @UserKey, + @CreationDate, + @RevisionDate + ) +END + +GO +CREATE PROCEDURE [dbo].[WebAuthnCredential_DeleteById] + @Id UNIQUEIDENTIFIER +AS +BEGIN + SET NOCOUNT ON + + DELETE + FROM + [dbo].[WebAuthnCredential] + WHERE + [Id] = @Id +END + +GO +CREATE PROCEDURE [dbo].[WebAuthnCredential_ReadById] + @Id UNIQUEIDENTIFIER +AS +BEGIN + SET NOCOUNT ON + + SELECT + * + FROM + [dbo].[WebAuthnCredentialView] + WHERE + [Id] = @Id +END + +GO +CREATE PROCEDURE [dbo].[WebAuthnCredential_ReadByUserId] + @UserId UNIQUEIDENTIFIER +AS +BEGIN + SET NOCOUNT ON + + SELECT + * + FROM + [dbo].[WebAuthnCredentialView] + WHERE + [UserId] = @UserId +END + +GO +CREATE PROCEDURE [dbo].[WebAuthnCredential_Update] + @Id UNIQUEIDENTIFIER, + @UserId UNIQUEIDENTIFIER, + @Name NVARCHAR(50), + @PublicKey VARCHAR (256), + @DescriptorId VARCHAR(256), + @Counter INT, + @Type VARCHAR(20), + @AaGuid UNIQUEIDENTIFIER, + @UserKey VARCHAR (MAX), + @CreationDate DATETIME2(7), + @RevisionDate DATETIME2(7) +AS +BEGIN + SET NOCOUNT ON + + UPDATE + [dbo].[WebAuthnCredential] + SET + [UserId] = @UserId, + [Name] = @Name, + [PublicKey] = @PublicKey, + [DescriptorId] = @DescriptorId, + [Counter] = @Counter, + [Type] = @Type, + [AaGuid] = @AaGuid, + [UserKey] = @UserKey, + [CreationDate] = @CreationDate, + [RevisionDate] = @RevisionDate + WHERE + [Id] = @Id +END \ No newline at end of file From f8ee4fa6f56369d60d7f8195730f964d1ba0988e Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Mon, 8 May 2023 13:43:39 +0200 Subject: [PATCH 11/50] [PM-2014] chore: append to todo comment --- src/Core/Services/Implementations/UserService.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index 4f14bc4ad272..d0f6cbdcc447 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -533,7 +533,8 @@ public async Task StartWebAuthnLoginRegistrationAsync(U UserVerification = UserVerificationRequirement.Preferred }; - // TODO: PRF + // TODO: PRF, maybe extension logic should be moved to client since it's gonna need to encrypt + // key using PRF output var extensions = new AuthenticationExtensionsClientInputs { }; var options = _fido2.RequestNewCredential(fidoUser, excludeCredentials, authenticatorSelection, From ebec5cb2f514ecd6a95dd69348b82fb7228dc739 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Tue, 9 May 2023 10:41:58 +0200 Subject: [PATCH 12/50] [PM-2014] feat: add support for creation token --- .../Auth/Controllers/WebAuthnController.cs | 54 +++++++++++-------- .../WebAuthnCredentialRequestModel.cs | 12 +++++ ...thnCredentialCreateOptionsResponseModel.cs | 9 ++++ ...ebAuthnCredentialCreateOptionsTokenable.cs | 44 +++++++++++++++ .../Utilities/ServiceCollectionExtensions.cs | 6 +++ 5 files changed, 102 insertions(+), 23 deletions(-) create mode 100644 src/Api/Auth/Models/Request/WebAuthn/WebAuthnCredentialRequestModel.cs create mode 100644 src/Api/Auth/Models/Response/WebAuthn/WebAuthnCredentialCreateOptionsResponseModel.cs create mode 100644 src/Core/Auth/Models/Business/Tokenables/WebAuthnCredentialCreateOptionsTokenable.cs diff --git a/src/Api/Auth/Controllers/WebAuthnController.cs b/src/Api/Auth/Controllers/WebAuthnController.cs index ca28c714c44e..859b81349dde 100644 --- a/src/Api/Auth/Controllers/WebAuthnController.cs +++ b/src/Api/Auth/Controllers/WebAuthnController.cs @@ -1,11 +1,12 @@ -using Bit.Api.Auth.Models.Request; -using Bit.Api.Auth.Models.Request.Accounts; +using Bit.Api.Auth.Models.Request.Accounts; +using Bit.Api.Auth.Models.Request.Webauthn; using Bit.Api.Auth.Models.Response.TwoFactor; +using Bit.Api.Auth.Models.Response.WebAuthn; using Bit.Api.Models.Response; -using Bit.Core.Entities; +using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Exceptions; using Bit.Core.Services; -using Fido2NetLib; +using Bit.Core.Tokens; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; @@ -16,11 +17,14 @@ namespace Bit.Api.Auth.Controllers; public class WebAuthnController : Controller { private readonly IUserService _userService; + private readonly IDataProtectorTokenFactory _createOptionsDataProtector; public WebAuthnController( - IUserService userService) + IUserService userService, + IDataProtectorTokenFactory createOptionsDataProtector) { _userService = userService; + _createOptionsDataProtector = createOptionsDataProtector; } [HttpGet("")] @@ -37,30 +41,34 @@ public async Task> Get() [HttpPost("options")] [ApiExplorerSettings(IgnoreApi = true)] // Disable Swagger due to CredentialCreateOptions not converting properly - public async Task PostOptions([FromBody] SecretVerificationRequestModel model) + public async Task PostOptions([FromBody] SecretVerificationRequestModel model) { var user = await CheckAsync(model); - var reg = await _userService.StartWebAuthnLoginRegistrationAsync(user); - return reg; + var options = await _userService.StartWebAuthnLoginRegistrationAsync(user); + + var tokenable = new WebAuthnCredentialCreateOptionsTokenable(user, options); + var token = _createOptionsDataProtector.Protect(tokenable); + + return new WebAuthnCredentialCreateOptionsResponseModel + { + Options = options, + Token = token, + }; } [HttpPost("")] // TODO: Create proper models for this call - public async Task Post([FromBody] TwoFactorWebAuthnRequestModel model) + public async Task Post([FromBody] WebAuthnCredentialRequestModel model) { - var user = await _userService.GetUserByPrincipalAsync(User); - if (user == null) - { - throw new UnauthorizedAccessException(); - } - - var success = await _userService.CompleteWebAuthLoginRegistrationAsync(user, model.Name, model.DeviceResponse); - if (!success) - { - throw new BadRequestException("Unable to complete WebAuthn registration."); - } - var response = new TwoFactorWebAuthnResponseModel(user); - return response; + return null; + //var user = await CheckAsync(model); + //var success = await _userService.CompleteWebAuthLoginRegistrationAsync(user, model.Name, model.DeviceResponse); + //if (!success) + //{ + // throw new BadRequestException("Unable to complete WebAuthn registration."); + //} + //var response = new TwoFactorWebAuthnResponseModel(user); + //return response; } [HttpDelete("{id}")] @@ -75,7 +83,7 @@ public async Task Delete(string id) // TODO: Delete } - private async Task CheckAsync(SecretVerificationRequestModel model) + private async Task CheckAsync(SecretVerificationRequestModel model) { var user = await _userService.GetUserByPrincipalAsync(User); if (user == null) diff --git a/src/Api/Auth/Models/Request/WebAuthn/WebAuthnCredentialRequestModel.cs b/src/Api/Auth/Models/Request/WebAuthn/WebAuthnCredentialRequestModel.cs new file mode 100644 index 000000000000..1d184a2734c8 --- /dev/null +++ b/src/Api/Auth/Models/Request/WebAuthn/WebAuthnCredentialRequestModel.cs @@ -0,0 +1,12 @@ +using Fido2NetLib; +using System.ComponentModel.DataAnnotations; + +namespace Bit.Api.Auth.Models.Request.Webauthn; + +public class WebAuthnCredentialRequestModel +{ + [Required] + public AuthenticatorAttestationRawResponse DeviceResponse { get; set; } + public string Name { get; set; } +} + diff --git a/src/Api/Auth/Models/Response/WebAuthn/WebAuthnCredentialCreateOptionsResponseModel.cs b/src/Api/Auth/Models/Response/WebAuthn/WebAuthnCredentialCreateOptionsResponseModel.cs new file mode 100644 index 000000000000..0889ba3dcba0 --- /dev/null +++ b/src/Api/Auth/Models/Response/WebAuthn/WebAuthnCredentialCreateOptionsResponseModel.cs @@ -0,0 +1,9 @@ +using Fido2NetLib; + +namespace Bit.Api.Auth.Models.Response.WebAuthn; + +public class WebAuthnCredentialCreateOptionsResponseModel +{ + public CredentialCreateOptions Options { get; set; } + public string Token { get; set; } +} diff --git a/src/Core/Auth/Models/Business/Tokenables/WebAuthnCredentialCreateOptionsTokenable.cs b/src/Core/Auth/Models/Business/Tokenables/WebAuthnCredentialCreateOptionsTokenable.cs new file mode 100644 index 000000000000..765828b5a900 --- /dev/null +++ b/src/Core/Auth/Models/Business/Tokenables/WebAuthnCredentialCreateOptionsTokenable.cs @@ -0,0 +1,44 @@ +using System.Text.Json.Serialization; +using Bit.Core.Entities; +using Bit.Core.Tokens; +using Fido2NetLib; + +namespace Bit.Core.Auth.Models.Business.Tokenables; + +public class WebAuthnCredentialCreateOptionsTokenable : ExpiringTokenable +{ + // 7 minutes = max webauthn timeout (6 minutes) + slack for miscellaneous delays + private const double _tokenLifetimeInHours = (double)7 / 60; + public const string ClearTextPrefix = "BWWebAuthnCredentialCreateOptions_"; + public const string DataProtectorPurpose = "WebAuthnCredentialCreateDataProtector"; + public const string TokenIdentifier = "WebAuthnCredentialCreateOptionsToken"; + + public string Identifier { get; set; } = TokenIdentifier; + public Guid UserId { get; set; } + public CredentialCreateOptions Options { get; set; } + + [JsonConstructor] + public WebAuthnCredentialCreateOptionsTokenable() + { + ExpirationDate = DateTime.UtcNow.AddHours(_tokenLifetimeInHours); + } + + public WebAuthnCredentialCreateOptionsTokenable(User user, CredentialCreateOptions options) : this() + { + UserId = user?.Id ?? default; + Options = options; + } + + public bool TokenIsValid(User user) + { + if (UserId == default || user == default) + { + return false; + } + + return UserId == user.Id; + } + + protected override bool TokenIsValid() => Identifier == TokenIdentifier && UserId != default && Options != default; +} + diff --git a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs index 60fb7f4c0226..6b0497e6cd6d 100644 --- a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs +++ b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs @@ -163,6 +163,12 @@ public static void AddTokenizers(this IServiceCollection services) WebAuthnLoginTokenable.DataProtectorPurpose, serviceProvider.GetDataProtectionProvider(), serviceProvider.GetRequiredService>>())); + services.AddSingleton>(serviceProvider => + new DataProtectorTokenFactory( + WebAuthnCredentialCreateOptionsTokenable.ClearTextPrefix, + WebAuthnCredentialCreateOptionsTokenable.DataProtectorPurpose, + serviceProvider.GetDataProtectionProvider(), + serviceProvider.GetRequiredService>>())); } public static void AddDefaultServices(this IServiceCollection services, GlobalSettings globalSettings) From 88dce908b79066f8754943b94784924d05c72b1d Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Tue, 9 May 2023 14:16:56 +0200 Subject: [PATCH 13/50] [PM-2014] feat: implement credential saving --- .../Auth/Controllers/WebAuthnController.cs | 28 ++++++++++++------- .../WebAuthnCredentialRequestModel.cs | 5 ++++ src/Core/Services/IUserService.cs | 2 +- .../Services/Implementations/UserService.cs | 4 +-- 4 files changed, 25 insertions(+), 14 deletions(-) diff --git a/src/Api/Auth/Controllers/WebAuthnController.cs b/src/Api/Auth/Controllers/WebAuthnController.cs index 859b81349dde..ab33c5df743a 100644 --- a/src/Api/Auth/Controllers/WebAuthnController.cs +++ b/src/Api/Auth/Controllers/WebAuthnController.cs @@ -1,4 +1,5 @@ -using Bit.Api.Auth.Models.Request.Accounts; +using Amazon.Runtime.Credentials.Internal; +using Bit.Api.Auth.Models.Request.Accounts; using Bit.Api.Auth.Models.Request.Webauthn; using Bit.Api.Auth.Models.Response.TwoFactor; using Bit.Api.Auth.Models.Response.WebAuthn; @@ -8,6 +9,7 @@ using Bit.Core.Services; using Bit.Core.Tokens; using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.DataProtection; using Microsoft.AspNetCore.Mvc; namespace Bit.Api.Auth.Controllers; @@ -60,15 +62,21 @@ public async Task PostOptions([Fro // TODO: Create proper models for this call public async Task Post([FromBody] WebAuthnCredentialRequestModel model) { - return null; - //var user = await CheckAsync(model); - //var success = await _userService.CompleteWebAuthLoginRegistrationAsync(user, model.Name, model.DeviceResponse); - //if (!success) - //{ - // throw new BadRequestException("Unable to complete WebAuthn registration."); - //} - //var response = new TwoFactorWebAuthnResponseModel(user); - //return response; + var user = await _userService.GetUserByPrincipalAsync(User); + + var tokenable = _createOptionsDataProtector.Unprotect(model.Token); + if (!tokenable.TokenIsValid(user)) + { + throw new BadRequestException("The token associated with your request is expired. A valid token is required to continue."); + } + + var success = await _userService.CompleteWebAuthLoginRegistrationAsync(user, model.Name, tokenable.Options, model.DeviceResponse); + if (!success) + { + throw new BadRequestException("Unable to complete WebAuthn registration."); + } + var response = new TwoFactorWebAuthnResponseModel(user); + return response; } [HttpDelete("{id}")] diff --git a/src/Api/Auth/Models/Request/WebAuthn/WebAuthnCredentialRequestModel.cs b/src/Api/Auth/Models/Request/WebAuthn/WebAuthnCredentialRequestModel.cs index 1d184a2734c8..f55a963966a1 100644 --- a/src/Api/Auth/Models/Request/WebAuthn/WebAuthnCredentialRequestModel.cs +++ b/src/Api/Auth/Models/Request/WebAuthn/WebAuthnCredentialRequestModel.cs @@ -7,6 +7,11 @@ public class WebAuthnCredentialRequestModel { [Required] public AuthenticatorAttestationRawResponse DeviceResponse { get; set; } + + [Required] public string Name { get; set; } + + [Required] + public string Token { get; set; } } diff --git a/src/Core/Services/IUserService.cs b/src/Core/Services/IUserService.cs index 49a8c9a7b8c5..e27668946638 100644 --- a/src/Core/Services/IUserService.cs +++ b/src/Core/Services/IUserService.cs @@ -28,7 +28,7 @@ public interface IUserService Task DeleteWebAuthnKeyAsync(User user, int id); Task CompleteWebAuthRegistrationAsync(User user, int value, string name, AuthenticatorAttestationRawResponse attestationResponse); Task StartWebAuthnLoginRegistrationAsync(User user); - Task CompleteWebAuthLoginRegistrationAsync(User user, string name, AuthenticatorAttestationRawResponse attestationResponse); + Task CompleteWebAuthLoginRegistrationAsync(User user, string name, CredentialCreateOptions options, AuthenticatorAttestationRawResponse attestationResponse); Task StartWebAuthnLoginAssertionAsync(User user); Task CompleteWebAuthLoginAssertionAsync(AuthenticatorAssertionRawResponse assertionResponse, User user); Task SendEmailVerificationAsync(User user); diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index d0f6cbdcc447..a4ec45e7cd0e 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -546,11 +546,9 @@ public async Task StartWebAuthnLoginRegistrationAsync(U } public async Task CompleteWebAuthLoginRegistrationAsync(User user, string name, + CredentialCreateOptions options, AuthenticatorAttestationRawResponse attestationResponse) { - // TODO: Get options from user record somehow, then clear them - var options = CredentialCreateOptions.FromJson(""); - // TODO: Callback to ensure credential ID is unique. Do we care? I don't think so. IsCredentialIdUniqueToUserAsyncDelegate callback = (args, cancellationToken) => Task.FromResult(true); From 556b75809cad9c722b4bf1e7cc95ee09ec69fee1 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Tue, 9 May 2023 14:22:36 +0200 Subject: [PATCH 14/50] [PM-2014] chore: add resident key TODO comment --- .../WebAuthnCredentialCreateOptionsResponseModel.cs | 11 +++++++++-- src/Core/Services/Implementations/UserService.cs | 2 +- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/Api/Auth/Models/Response/WebAuthn/WebAuthnCredentialCreateOptionsResponseModel.cs b/src/Api/Auth/Models/Response/WebAuthn/WebAuthnCredentialCreateOptionsResponseModel.cs index 0889ba3dcba0..3d57dd2f1c85 100644 --- a/src/Api/Auth/Models/Response/WebAuthn/WebAuthnCredentialCreateOptionsResponseModel.cs +++ b/src/Api/Auth/Models/Response/WebAuthn/WebAuthnCredentialCreateOptionsResponseModel.cs @@ -1,9 +1,16 @@ -using Fido2NetLib; +using Bit.Core.Models.Api; +using Fido2NetLib; namespace Bit.Api.Auth.Models.Response.WebAuthn; -public class WebAuthnCredentialCreateOptionsResponseModel +public class WebAuthnCredentialCreateOptionsResponseModel : ResponseModel { + private const string ResponseObj = "webauthnCredentialCreateOptions"; + public CredentialCreateOptions Options { get; set; } public string Token { get; set; } + + public WebAuthnCredentialCreateOptionsResponseModel() : base(ResponseObj) + { + } } diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index a4ec45e7cd0e..7a0696910a3c 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -529,7 +529,7 @@ public async Task StartWebAuthnLoginRegistrationAsync(U var authenticatorSelection = new AuthenticatorSelection { AuthenticatorAttachment = null, - RequireResidentKey = false, + RequireResidentKey = false, // TODO: This is using the old residentKey selection variant, we need to update our lib so that we can set this to preferred UserVerification = UserVerificationRequirement.Preferred }; From ee32d5ac010f33787732b5f44225fb6663ccf578 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Wed, 10 May 2023 12:53:50 +0200 Subject: [PATCH 15/50] [PM-2014] feat: implement passkey listing --- .../Auth/Controllers/WebAuthnController.cs | 11 ++++++++-- .../WebAuthnCredentialResponseModel.cs | 20 +++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) create mode 100644 src/Api/Auth/Models/Response/WebAuthn/WebAuthnCredentialResponseModel.cs diff --git a/src/Api/Auth/Controllers/WebAuthnController.cs b/src/Api/Auth/Controllers/WebAuthnController.cs index ab33c5df743a..fe789f58c8df 100644 --- a/src/Api/Auth/Controllers/WebAuthnController.cs +++ b/src/Api/Auth/Controllers/WebAuthnController.cs @@ -5,6 +5,7 @@ using Bit.Api.Auth.Models.Response.WebAuthn; using Bit.Api.Models.Response; using Bit.Core.Auth.Models.Business.Tokenables; +using Bit.Core.Auth.Repositories; using Bit.Core.Exceptions; using Bit.Core.Services; using Bit.Core.Tokens; @@ -19,26 +20,32 @@ namespace Bit.Api.Auth.Controllers; public class WebAuthnController : Controller { private readonly IUserService _userService; + private readonly IWebAuthnCredentialRepository _credentialRepository; private readonly IDataProtectorTokenFactory _createOptionsDataProtector; public WebAuthnController( IUserService userService, + IWebAuthnCredentialRepository credentialRepository, IDataProtectorTokenFactory createOptionsDataProtector) { _userService = userService; + _credentialRepository = credentialRepository; _createOptionsDataProtector = createOptionsDataProtector; } [HttpGet("")] // TODO: Create proper models for this call - public async Task> Get() + public async Task> Get() { var user = await _userService.GetUserByPrincipalAsync(User); if (user == null) { throw new UnauthorizedAccessException(); } - return new ListResponseModel(new List { }); + + var credentials = await _credentialRepository.GetManyByUserIdAsync(user.Id); + + return new ListResponseModel(credentials.Select(c => new WebAuthnCredentialResponseModel(c))); } [HttpPost("options")] diff --git a/src/Api/Auth/Models/Response/WebAuthn/WebAuthnCredentialResponseModel.cs b/src/Api/Auth/Models/Response/WebAuthn/WebAuthnCredentialResponseModel.cs new file mode 100644 index 000000000000..55f79222dba0 --- /dev/null +++ b/src/Api/Auth/Models/Response/WebAuthn/WebAuthnCredentialResponseModel.cs @@ -0,0 +1,20 @@ +using Bit.Core.Auth.Entities; +using Bit.Core.Models.Api; + +namespace Bit.Api.Auth.Models.Response.WebAuthn; + +public class WebAuthnCredentialResponseModel : ResponseModel +{ + private const string ResponseObj = "webauthnCredential"; + + public string Id { get; set; } + public string Name { get; set; } + public bool PrfSupport { get; set; } + + public WebAuthnCredentialResponseModel(WebAuthnCredential credential) : base(ResponseObj) + { + Id = credential.Id.ToString(); + Name = credential.Name; + PrfSupport = false; + } +} From 73b7a001f91f5c99a8c7a4f33afa9e37edeec2af Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Wed, 10 May 2023 15:10:26 +0200 Subject: [PATCH 16/50] [PM-2014] feat: implement deletion without user verification --- src/Api/Auth/Controllers/WebAuthnController.cs | 9 ++++++++- .../Repositories/IWebAuthnCredentialRepository.cs | 1 + .../Repositories/WebAuthnCredentialRepository.cs | 13 +++++++++++++ .../Repositories/WebAuthnCredentialRepository.cs | 10 ++++++++++ 4 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/Api/Auth/Controllers/WebAuthnController.cs b/src/Api/Auth/Controllers/WebAuthnController.cs index fe789f58c8df..ed7734fceb1f 100644 --- a/src/Api/Auth/Controllers/WebAuthnController.cs +++ b/src/Api/Auth/Controllers/WebAuthnController.cs @@ -95,7 +95,14 @@ public async Task Delete(string id) { throw new UnauthorizedAccessException(); } - // TODO: Delete + + var credential = await _credentialRepository.GetByIdAsync(new Guid(id), user.Id); + if (credential == null) + { + throw new NotFoundException("Credential not found."); + } + + await _credentialRepository.DeleteAsync(credential); } private async Task CheckAsync(SecretVerificationRequestModel model) diff --git a/src/Core/Auth/Repositories/IWebAuthnCredentialRepository.cs b/src/Core/Auth/Repositories/IWebAuthnCredentialRepository.cs index 1758ea90e8ee..7a052df6883e 100644 --- a/src/Core/Auth/Repositories/IWebAuthnCredentialRepository.cs +++ b/src/Core/Auth/Repositories/IWebAuthnCredentialRepository.cs @@ -5,5 +5,6 @@ namespace Bit.Core.Auth.Repositories; public interface IWebAuthnCredentialRepository : IRepository { + Task GetByIdAsync(Guid id, Guid userId); Task> GetManyByUserIdAsync(Guid userId); } diff --git a/src/Infrastructure.Dapper/Auth/Repositories/WebAuthnCredentialRepository.cs b/src/Infrastructure.Dapper/Auth/Repositories/WebAuthnCredentialRepository.cs index 943ae2f09956..502569136f2a 100644 --- a/src/Infrastructure.Dapper/Auth/Repositories/WebAuthnCredentialRepository.cs +++ b/src/Infrastructure.Dapper/Auth/Repositories/WebAuthnCredentialRepository.cs @@ -19,6 +19,19 @@ public WebAuthnCredentialRepository(string connectionString, string readOnlyConn : base(connectionString, readOnlyConnectionString) { } + public async Task GetByIdAsync(Guid id, Guid userId) + { + using (var connection = new SqlConnection(ConnectionString)) + { + var results = await connection.QueryAsync( + $"[{Schema}].[{Table}_ReadByIdUserId]", + new { Id = id, UserId = userId }, + commandType: CommandType.StoredProcedure); + + return results.FirstOrDefault(); + } + } + public async Task> GetManyByUserIdAsync(Guid userId) { using (var connection = new SqlConnection(ConnectionString)) diff --git a/src/Infrastructure.EntityFramework/Auth/Repositories/WebAuthnCredentialRepository.cs b/src/Infrastructure.EntityFramework/Auth/Repositories/WebAuthnCredentialRepository.cs index c86d7bd20aad..e2840ea2dec4 100644 --- a/src/Infrastructure.EntityFramework/Auth/Repositories/WebAuthnCredentialRepository.cs +++ b/src/Infrastructure.EntityFramework/Auth/Repositories/WebAuthnCredentialRepository.cs @@ -13,6 +13,16 @@ public WebAuthnCredentialRepository(IServiceScopeFactory serviceScopeFactory, IM : base(serviceScopeFactory, mapper, (context) => context.WebAuthnCredentials) { } + public async Task GetByIdAsync(Guid id, Guid userId) { + using (var scope = ServiceScopeFactory.CreateScope()) + { + var dbContext = GetDatabaseContext(scope); + var query = dbContext.WebAuthnCredentials.Where(d => d.Id == id && d.UserId == userId); + var cred = await query.FirstOrDefaultAsync(); + return Mapper.Map(cred); + } + } + public async Task> GetManyByUserIdAsync(Guid userId) { using (var scope = ServiceScopeFactory.CreateScope()) From f2c6b03d512747ace0c8a8173455ab5840802ce3 Mon Sep 17 00:00:00 2001 From: Kyle Spearrin Date: Wed, 10 May 2023 11:48:18 -0400 Subject: [PATCH 17/50] revert sqlproj changes --- src/Sql/Sql.sqlproj | 466 +------------------------------------------- 1 file changed, 2 insertions(+), 464 deletions(-) diff --git a/src/Sql/Sql.sqlproj b/src/Sql/Sql.sqlproj index c0bb0ff74829..09f8eceddb3f 100644 --- a/src/Sql/Sql.sqlproj +++ b/src/Sql/Sql.sqlproj @@ -1,6 +1,5 @@  - - + Sql @@ -8,468 +7,7 @@ Microsoft.Data.Tools.Schema.Sql.SqlAzureV12DatabaseSchemaProvider 1033,CI - - bin\Release\ - $(MSBuildProjectName).sql - False - pdbonly - true - false - true - prompt - 4 - 71502 - - - bin\Debug\ - $(MSBuildProjectName).sql - false - true - full - false - true - true - prompt - 4 - 71502 - - - 12.0 - - True - 12.0 - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - \ No newline at end of file + From 26da1b208b9de278b00c59657a04b75fba064c73 Mon Sep 17 00:00:00 2001 From: Kyle Spearrin Date: Wed, 10 May 2023 11:50:08 -0400 Subject: [PATCH 18/50] update sqlproj target framework --- src/Sql/Sql.sqlproj | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Sql/Sql.sqlproj b/src/Sql/Sql.sqlproj index 09f8eceddb3f..b53ba1aeb361 100644 --- a/src/Sql/Sql.sqlproj +++ b/src/Sql/Sql.sqlproj @@ -6,8 +6,11 @@ {58554e52-fdec-4832-aff9-302b01e08dca} Microsoft.Data.Tools.Schema.Sql.SqlAzureV12DatabaseSchemaProvider 1033,CI + True + v4.7.2 + - + - + \ No newline at end of file From ec0c93f5d60b9aa0e86674581923027677c8489f Mon Sep 17 00:00:00 2001 From: Kyle Spearrin Date: Wed, 10 May 2023 11:55:56 -0400 Subject: [PATCH 19/50] update new validator signature --- src/Identity/IdentityServer/ExtensionGrantValidator.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Identity/IdentityServer/ExtensionGrantValidator.cs b/src/Identity/IdentityServer/ExtensionGrantValidator.cs index c476e6eb19f8..2b13b0cfef05 100644 --- a/src/Identity/IdentityServer/ExtensionGrantValidator.cs +++ b/src/Identity/IdentityServer/ExtensionGrantValidator.cs @@ -35,11 +35,12 @@ public ExtensionGrantValidator( IPolicyRepository policyRepository, IUserRepository userRepository, IPolicyService policyService, + IDataProtectorTokenFactory tokenDataFactory, IDataProtectorTokenFactory webAuthnLoginTokenizer) : base(userManager, deviceRepository, deviceService, userService, eventService, organizationDuoWebTokenProvider, organizationRepository, organizationUserRepository, applicationCacheService, mailService, logger, currentContext, globalSettings, policyRepository, - userRepository, policyService) + userRepository, policyService, tokenDataFactory) { _userManager = userManager; _webAuthnLoginTokenizer = webAuthnLoginTokenizer; From 258afae003250f329ec1d6d2901180b98d8f6426 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Thu, 11 May 2023 10:22:46 +0200 Subject: [PATCH 20/50] [PM-2014] feat: add user verification to delete --- src/Api/Auth/Controllers/WebAuthnController.cs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/Api/Auth/Controllers/WebAuthnController.cs b/src/Api/Auth/Controllers/WebAuthnController.cs index ed7734fceb1f..5013356b48f5 100644 --- a/src/Api/Auth/Controllers/WebAuthnController.cs +++ b/src/Api/Auth/Controllers/WebAuthnController.cs @@ -86,16 +86,10 @@ public async Task Post([FromBody] WebAuthnCreden return response; } - [HttpDelete("{id}")] [HttpPost("{id}/delete")] - public async Task Delete(string id) + public async Task Delete(string id, [FromBody] SecretVerificationRequestModel model) { - var user = await _userService.GetUserByPrincipalAsync(User); - if (user == null) - { - throw new UnauthorizedAccessException(); - } - + var user = await CheckAsync(model); var credential = await _credentialRepository.GetByIdAsync(new Guid(id), user.Id); if (credential == null) { From 6aff50a1401c6ed2043fa0747b7dc79371eff894 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Thu, 11 May 2023 14:12:30 +0200 Subject: [PATCH 21/50] [PM-2014] feat: implement passkey limit --- src/Core/Services/Implementations/UserService.cs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index 7a0696910a3c..1a6517a5ce52 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -533,15 +533,11 @@ public async Task StartWebAuthnLoginRegistrationAsync(U UserVerification = UserVerificationRequirement.Preferred }; - // TODO: PRF, maybe extension logic should be moved to client since it's gonna need to encrypt - // key using PRF output var extensions = new AuthenticationExtensionsClientInputs { }; var options = _fido2.RequestNewCredential(fidoUser, excludeCredentials, authenticatorSelection, AttestationConveyancePreference.None, extensions); - // TODO: temp save options to user record somehow - return options; } @@ -549,8 +545,14 @@ public async Task CompleteWebAuthLoginRegistrationAsync(User user, string CredentialCreateOptions options, AuthenticatorAttestationRawResponse attestationResponse) { - // TODO: Callback to ensure credential ID is unique. Do we care? I don't think so. - IsCredentialIdUniqueToUserAsyncDelegate callback = (args, cancellationToken) => Task.FromResult(true); + var existingCredentials = await _webAuthnCredentialRepository.GetManyByUserIdAsync(user.Id); + if (existingCredentials.Count >= 5) + { + return false; + } + + var existingCredentialIds = existingCredentials.Select(c => c.DescriptorId); + IsCredentialIdUniqueToUserAsyncDelegate callback = (args, cancellationToken) => Task.FromResult(!existingCredentialIds.Contains(CoreHelpers.Base64UrlEncode(args.CredentialId))); var success = await _fido2.MakeNewCredentialAsync(attestationResponse, options, callback); From e8ab5d72721bab4af681387f9ec3081d657f3fe1 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Thu, 11 May 2023 14:13:15 +0200 Subject: [PATCH 22/50] [PM-2014] chore: clean up todo comments --- src/Api/Auth/Controllers/WebAuthnController.cs | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/Api/Auth/Controllers/WebAuthnController.cs b/src/Api/Auth/Controllers/WebAuthnController.cs index 5013356b48f5..935c0e3443a0 100644 --- a/src/Api/Auth/Controllers/WebAuthnController.cs +++ b/src/Api/Auth/Controllers/WebAuthnController.cs @@ -1,7 +1,5 @@ -using Amazon.Runtime.Credentials.Internal; -using Bit.Api.Auth.Models.Request.Accounts; +using Bit.Api.Auth.Models.Request.Accounts; using Bit.Api.Auth.Models.Request.Webauthn; -using Bit.Api.Auth.Models.Response.TwoFactor; using Bit.Api.Auth.Models.Response.WebAuthn; using Bit.Api.Models.Response; using Bit.Core.Auth.Models.Business.Tokenables; @@ -10,7 +8,6 @@ using Bit.Core.Services; using Bit.Core.Tokens; using Microsoft.AspNetCore.Authorization; -using Microsoft.AspNetCore.DataProtection; using Microsoft.AspNetCore.Mvc; namespace Bit.Api.Auth.Controllers; @@ -34,7 +31,6 @@ public WebAuthnController( } [HttpGet("")] - // TODO: Create proper models for this call public async Task> Get() { var user = await _userService.GetUserByPrincipalAsync(User); @@ -66,8 +62,7 @@ public async Task PostOptions([Fro } [HttpPost("")] - // TODO: Create proper models for this call - public async Task Post([FromBody] WebAuthnCredentialRequestModel model) + public async Task Post([FromBody] WebAuthnCredentialRequestModel model) { var user = await _userService.GetUserByPrincipalAsync(User); @@ -82,8 +77,6 @@ public async Task Post([FromBody] WebAuthnCreden { throw new BadRequestException("Unable to complete WebAuthn registration."); } - var response = new TwoFactorWebAuthnResponseModel(user); - return response; } [HttpPost("{id}/delete")] From 85db96c75e4228857375fd7c11e22896a665c3a0 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Thu, 11 May 2023 14:13:45 +0200 Subject: [PATCH 23/50] [PM-2014] fix: add missing sql scripts Missed staging them when commiting --- .../WebAuthnCredential_ReadByIdUserId.sql | 16 ++++++++++++++++ .../2023-05-08-00_WebAuthnLoginCredentials.sql | 18 ++++++++++++++++++ 2 files changed, 34 insertions(+) create mode 100644 src/Sql/dbo/Stored Procedures/WebAuthnCredential_ReadByIdUserId.sql diff --git a/src/Sql/dbo/Stored Procedures/WebAuthnCredential_ReadByIdUserId.sql b/src/Sql/dbo/Stored Procedures/WebAuthnCredential_ReadByIdUserId.sql new file mode 100644 index 000000000000..d0565ff05309 --- /dev/null +++ b/src/Sql/dbo/Stored Procedures/WebAuthnCredential_ReadByIdUserId.sql @@ -0,0 +1,16 @@ +CREATE PROCEDURE [dbo].[WebAuthnCredential_ReadByIdUserId] + @Id UNIQUEIDENTIFIER, + @UserId UNIQUEIDENTIFIER +AS +BEGIN + SET NOCOUNT ON + + SELECT + * + FROM + [dbo].[WebAuthnCredentialView] + WHERE + [Id] = @Id + AND + [UserId] = @UserId +END \ No newline at end of file diff --git a/util/Migrator/DbScripts/2023-05-08-00_WebAuthnLoginCredentials.sql b/util/Migrator/DbScripts/2023-05-08-00_WebAuthnLoginCredentials.sql index a0fec82e1247..ef3cefdec81a 100644 --- a/util/Migrator/DbScripts/2023-05-08-00_WebAuthnLoginCredentials.sql +++ b/util/Migrator/DbScripts/2023-05-08-00_WebAuthnLoginCredentials.sql @@ -149,4 +149,22 @@ BEGIN [RevisionDate] = @RevisionDate WHERE [Id] = @Id +END + +GO +CREATE PROCEDURE [dbo].[WebAuthnCredential_ReadByIdUserId] + @Id UNIQUEIDENTIFIER, + @UserId UNIQUEIDENTIFIER +AS +BEGIN + SET NOCOUNT ON + + SELECT + * + FROM + [dbo].[WebAuthnCredentialView] + WHERE + [Id] = @Id + AND + [UserId] = @UserId END \ No newline at end of file From 934dd82333220740c1686e3f91bf93869cf77bd7 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Mon, 15 May 2023 10:46:31 +0200 Subject: [PATCH 24/50] [PM-2014] feat: include options response model in swagger docs --- src/Api/Auth/Controllers/WebAuthnController.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Api/Auth/Controllers/WebAuthnController.cs b/src/Api/Auth/Controllers/WebAuthnController.cs index 935c0e3443a0..78a65bdc906d 100644 --- a/src/Api/Auth/Controllers/WebAuthnController.cs +++ b/src/Api/Auth/Controllers/WebAuthnController.cs @@ -45,7 +45,6 @@ public async Task> Get() } [HttpPost("options")] - [ApiExplorerSettings(IgnoreApi = true)] // Disable Swagger due to CredentialCreateOptions not converting properly public async Task PostOptions([FromBody] SecretVerificationRequestModel model) { var user = await CheckAsync(model); From 74d8f2e77a0283e8146e98722121b4b5e99684c5 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Mon, 15 May 2023 10:48:05 +0200 Subject: [PATCH 25/50] [PM-2014] chore: move properties after ctor --- .../WebAuthnCredentialCreateOptionsResponseModel.cs | 6 +++--- .../Response/WebAuthn/WebAuthnCredentialResponseModel.cs | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Api/Auth/Models/Response/WebAuthn/WebAuthnCredentialCreateOptionsResponseModel.cs b/src/Api/Auth/Models/Response/WebAuthn/WebAuthnCredentialCreateOptionsResponseModel.cs index 3d57dd2f1c85..d521bdac960b 100644 --- a/src/Api/Auth/Models/Response/WebAuthn/WebAuthnCredentialCreateOptionsResponseModel.cs +++ b/src/Api/Auth/Models/Response/WebAuthn/WebAuthnCredentialCreateOptionsResponseModel.cs @@ -7,10 +7,10 @@ public class WebAuthnCredentialCreateOptionsResponseModel : ResponseModel { private const string ResponseObj = "webauthnCredentialCreateOptions"; - public CredentialCreateOptions Options { get; set; } - public string Token { get; set; } - public WebAuthnCredentialCreateOptionsResponseModel() : base(ResponseObj) { } + + public CredentialCreateOptions Options { get; set; } + public string Token { get; set; } } diff --git a/src/Api/Auth/Models/Response/WebAuthn/WebAuthnCredentialResponseModel.cs b/src/Api/Auth/Models/Response/WebAuthn/WebAuthnCredentialResponseModel.cs index 55f79222dba0..0e358c751d32 100644 --- a/src/Api/Auth/Models/Response/WebAuthn/WebAuthnCredentialResponseModel.cs +++ b/src/Api/Auth/Models/Response/WebAuthn/WebAuthnCredentialResponseModel.cs @@ -7,14 +7,14 @@ public class WebAuthnCredentialResponseModel : ResponseModel { private const string ResponseObj = "webauthnCredential"; - public string Id { get; set; } - public string Name { get; set; } - public bool PrfSupport { get; set; } - public WebAuthnCredentialResponseModel(WebAuthnCredential credential) : base(ResponseObj) { Id = credential.Id.ToString(); Name = credential.Name; PrfSupport = false; } + + public string Id { get; set; } + public string Name { get; set; } + public bool PrfSupport { get; set; } } From 40e56ef4fdaa5af32a99e8dca228b0e4f22fadf1 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Mon, 15 May 2023 11:14:07 +0200 Subject: [PATCH 26/50] [PM-2014] feat: use `Guid` directly as input paramter --- src/Api/Auth/Controllers/WebAuthnController.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Api/Auth/Controllers/WebAuthnController.cs b/src/Api/Auth/Controllers/WebAuthnController.cs index 78a65bdc906d..bf8d6a0ece82 100644 --- a/src/Api/Auth/Controllers/WebAuthnController.cs +++ b/src/Api/Auth/Controllers/WebAuthnController.cs @@ -79,10 +79,10 @@ public async Task Post([FromBody] WebAuthnCredentialRequestModel model) } [HttpPost("{id}/delete")] - public async Task Delete(string id, [FromBody] SecretVerificationRequestModel model) + public async Task Delete(Guid id, [FromBody] SecretVerificationRequestModel model) { var user = await CheckAsync(model); - var credential = await _credentialRepository.GetByIdAsync(new Guid(id), user.Id); + var credential = await _credentialRepository.GetByIdAsync(id, user.Id); if (credential == null) { throw new NotFoundException("Credential not found."); From e718989880168a97924392f1ab52bc89eeaac4f5 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Mon, 15 May 2023 11:22:35 +0200 Subject: [PATCH 27/50] [PM-2014] feat: use nullable guid in token --- .../WebAuthnCredentialCreateOptionsTokenable.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Core/Auth/Models/Business/Tokenables/WebAuthnCredentialCreateOptionsTokenable.cs b/src/Core/Auth/Models/Business/Tokenables/WebAuthnCredentialCreateOptionsTokenable.cs index 765828b5a900..97e70c122c4f 100644 --- a/src/Core/Auth/Models/Business/Tokenables/WebAuthnCredentialCreateOptionsTokenable.cs +++ b/src/Core/Auth/Models/Business/Tokenables/WebAuthnCredentialCreateOptionsTokenable.cs @@ -14,7 +14,7 @@ public class WebAuthnCredentialCreateOptionsTokenable : ExpiringTokenable public const string TokenIdentifier = "WebAuthnCredentialCreateOptionsToken"; public string Identifier { get; set; } = TokenIdentifier; - public Guid UserId { get; set; } + public Guid? UserId { get; set; } public CredentialCreateOptions Options { get; set; } [JsonConstructor] @@ -25,13 +25,13 @@ public WebAuthnCredentialCreateOptionsTokenable() public WebAuthnCredentialCreateOptionsTokenable(User user, CredentialCreateOptions options) : this() { - UserId = user?.Id ?? default; + UserId = user?.Id; Options = options; } public bool TokenIsValid(User user) { - if (UserId == default || user == default) + if (UserId == null || user == default) { return false; } @@ -39,6 +39,6 @@ public bool TokenIsValid(User user) return UserId == user.Id; } - protected override bool TokenIsValid() => Identifier == TokenIdentifier && UserId != default && Options != default; + protected override bool TokenIsValid() => Identifier == TokenIdentifier && UserId != null && Options != default; } From f17ea48e8591ad54a57eb0f4de8040712d76469d Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Mon, 15 May 2023 11:25:57 +0200 Subject: [PATCH 28/50] [PM-2014] chore: add new-line --- .../dbo/Stored Procedures/WebAuthnCredential_ReadByIdUserId.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Sql/dbo/Stored Procedures/WebAuthnCredential_ReadByIdUserId.sql b/src/Sql/dbo/Stored Procedures/WebAuthnCredential_ReadByIdUserId.sql index d0565ff05309..8b0f1d19f99f 100644 --- a/src/Sql/dbo/Stored Procedures/WebAuthnCredential_ReadByIdUserId.sql +++ b/src/Sql/dbo/Stored Procedures/WebAuthnCredential_ReadByIdUserId.sql @@ -13,4 +13,4 @@ BEGIN [Id] = @Id AND [UserId] = @UserId -END \ No newline at end of file +END From 75edd13ee8fed3b77f86a5755738c51eaaa6394c Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Mon, 15 May 2023 13:53:13 +0200 Subject: [PATCH 29/50] [PM-2014] feat: add support for feature flag --- src/Core/Constants.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index 420edb403e31..b83db77a4df9 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -30,6 +30,7 @@ public static class FeatureFlagKeys public const string SecretsManager = "secrets-manager"; public const string DisplayEuEnvironment = "display-eu-environment"; public const string DisplayLowKdfIterationWarning = "display-kdf-iteration-warning"; + public const string PasswordlessLogin = "passwordless-login"; public static List GetAllKeys() { From c7ee1027fda1ac0ef67a76ab0a412079bf227043 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Mon, 15 May 2023 15:30:11 +0200 Subject: [PATCH 30/50] [PM-2014] feat: start adding controller tests --- .../Auth/Controllers/WebAuthnController.cs | 21 +++--- test/Api.Test/Api.Test.csproj | 8 +++ .../Controllers/WebAuthnControllerTests.cs | 68 +++++++++++++++++++ 3 files changed, 86 insertions(+), 11 deletions(-) create mode 100644 test/Api.Test/Auth/Controllers/WebAuthnControllerTests.cs diff --git a/src/Api/Auth/Controllers/WebAuthnController.cs b/src/Api/Auth/Controllers/WebAuthnController.cs index bf8d6a0ece82..bf2e3d53d3b0 100644 --- a/src/Api/Auth/Controllers/WebAuthnController.cs +++ b/src/Api/Auth/Controllers/WebAuthnController.cs @@ -33,12 +33,7 @@ public WebAuthnController( [HttpGet("")] public async Task> Get() { - var user = await _userService.GetUserByPrincipalAsync(User); - if (user == null) - { - throw new UnauthorizedAccessException(); - } - + var user = await GetUser(); var credentials = await _credentialRepository.GetManyByUserIdAsync(user.Id); return new ListResponseModel(credentials.Select(c => new WebAuthnCredentialResponseModel(c))); @@ -47,7 +42,7 @@ public async Task> Get() [HttpPost("options")] public async Task PostOptions([FromBody] SecretVerificationRequestModel model) { - var user = await CheckAsync(model); + var user = await VerifyUser(model); var options = await _userService.StartWebAuthnLoginRegistrationAsync(user); var tokenable = new WebAuthnCredentialCreateOptionsTokenable(user, options); @@ -63,8 +58,7 @@ public async Task PostOptions([Fro [HttpPost("")] public async Task Post([FromBody] WebAuthnCredentialRequestModel model) { - var user = await _userService.GetUserByPrincipalAsync(User); - + var user = await GetUser(); var tokenable = _createOptionsDataProtector.Unprotect(model.Token); if (!tokenable.TokenIsValid(user)) { @@ -81,7 +75,7 @@ public async Task Post([FromBody] WebAuthnCredentialRequestModel model) [HttpPost("{id}/delete")] public async Task Delete(Guid id, [FromBody] SecretVerificationRequestModel model) { - var user = await CheckAsync(model); + var user = await VerifyUser(model); var credential = await _credentialRepository.GetByIdAsync(id, user.Id); if (credential == null) { @@ -91,14 +85,19 @@ public async Task Delete(Guid id, [FromBody] SecretVerificationRequestModel mode await _credentialRepository.DeleteAsync(credential); } - private async Task CheckAsync(SecretVerificationRequestModel model) + private async Task GetUser() { var user = await _userService.GetUserByPrincipalAsync(User); if (user == null) { throw new UnauthorizedAccessException(); } + return user; + } + private async Task VerifyUser(SecretVerificationRequestModel model) + { + var user = await GetUser(); if (!await _userService.VerifySecretAsync(user, model.Secret)) { await Task.Delay(2000); diff --git a/test/Api.Test/Api.Test.csproj b/test/Api.Test/Api.Test.csproj index 564e90c225d6..cf5c3b86b0a4 100644 --- a/test/Api.Test/Api.Test.csproj +++ b/test/Api.Test/Api.Test.csproj @@ -26,4 +26,12 @@ + + + + + + + + diff --git a/test/Api.Test/Auth/Controllers/WebAuthnControllerTests.cs b/test/Api.Test/Auth/Controllers/WebAuthnControllerTests.cs new file mode 100644 index 000000000000..01901c6da13f --- /dev/null +++ b/test/Api.Test/Auth/Controllers/WebAuthnControllerTests.cs @@ -0,0 +1,68 @@ +using Bit.Api.Auth.Controllers; +using Bit.Api.Auth.Models.Request.Accounts; +using Bit.Api.Auth.Models.Request.Webauthn; +using Bit.Core.Services; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using NSubstitute.ReturnsExtensions; +using Xunit; + +namespace Bit.Api.Test.Auth.Controllers; + +[ControllerCustomize(typeof(WebAuthnController))] +[SutProviderCustomize] +public class WebAuthnControllerTests +{ + [Theory, BitAutoData] + public async Task Get_UserNotFound_ThrowsUnauthorizedAccessException(SutProvider sutProvider) + { + // Arrange + sutProvider.GetDependency().GetUserByPrincipalAsync(default).ReturnsNullForAnyArgs(); + + // Act + var result = () => sutProvider.Sut.Get(); + + // Assert + await Assert.ThrowsAsync(result); + } + + [Theory, BitAutoData] + public async Task PostOptions_UserNotFound_ThrowsUnauthorizedAccessException(SecretVerificationRequestModel requestModel, SutProvider sutProvider) + { + // Arrange + sutProvider.GetDependency().GetUserByPrincipalAsync(default).ReturnsNullForAnyArgs(); + + // Act + var result = () => sutProvider.Sut.PostOptions(requestModel); + + // Assert + await Assert.ThrowsAsync(result); + } + + [Theory, BitAutoData] + public async Task Post_UserNotFound_ThrowsUnauthorizedAccessException(WebAuthnCredentialRequestModel requestModel, SutProvider sutProvider) + { + // Arrange + sutProvider.GetDependency().GetUserByPrincipalAsync(default).ReturnsNullForAnyArgs(); + + // Act + var result = () => sutProvider.Sut.Post(requestModel); + + // Assert + await Assert.ThrowsAsync(result); + } + + [Theory, BitAutoData] + public async Task Delete_UserNotFound_ThrowsUnauthorizedAccessException(Guid credentialId, SecretVerificationRequestModel requestModel, SutProvider sutProvider) + { + // Arrange + sutProvider.GetDependency().GetUserByPrincipalAsync(default).ReturnsNullForAnyArgs(); + + // Act + var result = () => sutProvider.Sut.Delete(credentialId, requestModel); + + // Assert + await Assert.ThrowsAsync(result); + } +} + From 590713f4567a7f98a8073f39ef3618bd1adc5c78 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Mon, 15 May 2023 16:03:59 +0200 Subject: [PATCH 31/50] [PM-2014] feat: add user verification test --- .../Controllers/WebAuthnControllerTests.cs | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/test/Api.Test/Auth/Controllers/WebAuthnControllerTests.cs b/test/Api.Test/Auth/Controllers/WebAuthnControllerTests.cs index 01901c6da13f..54cadb16255d 100644 --- a/test/Api.Test/Auth/Controllers/WebAuthnControllerTests.cs +++ b/test/Api.Test/Auth/Controllers/WebAuthnControllerTests.cs @@ -1,9 +1,12 @@ using Bit.Api.Auth.Controllers; using Bit.Api.Auth.Models.Request.Accounts; using Bit.Api.Auth.Models.Request.Webauthn; +using Bit.Core.Entities; +using Bit.Core.Exceptions; using Bit.Core.Services; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; +using NSubstitute; using NSubstitute.ReturnsExtensions; using Xunit; @@ -39,6 +42,20 @@ public async Task PostOptions_UserNotFound_ThrowsUnauthorizedAccessException(Sec await Assert.ThrowsAsync(result); } + [Theory, BitAutoData] + public async Task PostOptions_UserVerificationFailed_ThrowsBadRequestException(SecretVerificationRequestModel requestModel, User user, SutProvider sutProvider) + { + // Arrange + sutProvider.GetDependency().GetUserByPrincipalAsync(default).ReturnsForAnyArgs(user); + sutProvider.GetDependency().VerifySecretAsync(user, default).Returns(false); + + // Act + var result = () => sutProvider.Sut.PostOptions(requestModel); + + // Assert + await Assert.ThrowsAsync(result); + } + [Theory, BitAutoData] public async Task Post_UserNotFound_ThrowsUnauthorizedAccessException(WebAuthnCredentialRequestModel requestModel, SutProvider sutProvider) { @@ -64,5 +81,19 @@ public async Task Delete_UserNotFound_ThrowsUnauthorizedAccessException(Guid cre // Assert await Assert.ThrowsAsync(result); } + + [Theory, BitAutoData] + public async Task Delete_UserVerificationFailed_ThrowsBadRequestException(Guid credentialId, SecretVerificationRequestModel requestModel, User user, SutProvider sutProvider) + { + // Arrange + sutProvider.GetDependency().GetUserByPrincipalAsync(default).ReturnsForAnyArgs(user); + sutProvider.GetDependency().VerifySecretAsync(user, default).Returns(false); + + // Act + var result = () => sutProvider.Sut.Delete(credentialId, requestModel); + + // Assert + await Assert.ThrowsAsync(result); + } } From 0a9544c9e7b0bcc6bae8fc5e2dc131caf3b458ba Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Mon, 15 May 2023 16:35:28 +0200 Subject: [PATCH 32/50] [PM-2014] feat: add controller tests for token interaction --- .../Controllers/WebAuthnControllerTests.cs | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/test/Api.Test/Auth/Controllers/WebAuthnControllerTests.cs b/test/Api.Test/Auth/Controllers/WebAuthnControllerTests.cs index 54cadb16255d..c26579dfaa8c 100644 --- a/test/Api.Test/Auth/Controllers/WebAuthnControllerTests.cs +++ b/test/Api.Test/Auth/Controllers/WebAuthnControllerTests.cs @@ -1,11 +1,15 @@ using Bit.Api.Auth.Controllers; using Bit.Api.Auth.Models.Request.Accounts; using Bit.Api.Auth.Models.Request.Webauthn; +using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Entities; using Bit.Core.Exceptions; using Bit.Core.Services; +using Bit.Core.Tokens; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; +using Fido2NetLib; +using Microsoft.Identity.Client.Platforms.Features.DesktopOs.Kerberos; using NSubstitute; using NSubstitute.ReturnsExtensions; using Xunit; @@ -69,6 +73,47 @@ public async Task Post_UserNotFound_ThrowsUnauthorizedAccessException(WebAuthnCr await Assert.ThrowsAsync(result); } + [Theory, BitAutoData] + public async Task Post_ExpiredToken_ThrowsBadRequestException(WebAuthnCredentialRequestModel requestModel, CredentialCreateOptions createOptions, User user, SutProvider sutProvider) + { + // Arrange + var token = new WebAuthnCredentialCreateOptionsTokenable(user, createOptions); + sutProvider.GetDependency() + .GetUserByPrincipalAsync(default) + .ReturnsForAnyArgs(user); + sutProvider.GetDependency>() + .Unprotect(requestModel.Token) + .Returns(token); + + // Act + var result = () => sutProvider.Sut.Post(requestModel); + + // Assert + await Assert.ThrowsAsync(result); + } + + [Theory, BitAutoData] + public async Task Post_ValidInput_Returns(WebAuthnCredentialRequestModel requestModel, CredentialCreateOptions createOptions, User user, SutProvider sutProvider) + { + // Arrange + var token = new WebAuthnCredentialCreateOptionsTokenable(user, createOptions); + sutProvider.GetDependency() + .GetUserByPrincipalAsync(default) + .ReturnsForAnyArgs(user); + sutProvider.GetDependency() + .CompleteWebAuthLoginRegistrationAsync(user, requestModel.Name, createOptions, Arg.Any()) + .Returns(true); + sutProvider.GetDependency>() + .Unprotect(requestModel.Token) + .Returns(token); + + // Act + await sutProvider.Sut.Post(requestModel); + + // Assert + // Nothing to assert since return is void + } + [Theory, BitAutoData] public async Task Delete_UserNotFound_ThrowsUnauthorizedAccessException(Guid credentialId, SecretVerificationRequestModel requestModel, SutProvider sutProvider) { From 204a63fccb72c79d79159c60bfcc300b348b679e Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Mon, 15 May 2023 16:53:02 +0200 Subject: [PATCH 33/50] [PM-2014] feat: add tokenable tests --- ...ebAuthnCredentialCreateOptionsTokenable.cs | 2 +- ...hnCredentialCreateOptionsTokenableTests.cs | 81 +++++++++++++++++++ 2 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 test/Core.Test/Auth/Models/Business/Tokenables/WebAuthnCredentialCreateOptionsTokenableTests.cs diff --git a/src/Core/Auth/Models/Business/Tokenables/WebAuthnCredentialCreateOptionsTokenable.cs b/src/Core/Auth/Models/Business/Tokenables/WebAuthnCredentialCreateOptionsTokenable.cs index 97e70c122c4f..ea42681bf78d 100644 --- a/src/Core/Auth/Models/Business/Tokenables/WebAuthnCredentialCreateOptionsTokenable.cs +++ b/src/Core/Auth/Models/Business/Tokenables/WebAuthnCredentialCreateOptionsTokenable.cs @@ -31,7 +31,7 @@ public WebAuthnCredentialCreateOptionsTokenable(User user, CredentialCreateOptio public bool TokenIsValid(User user) { - if (UserId == null || user == default) + if (!Valid || user == default) { return false; } diff --git a/test/Core.Test/Auth/Models/Business/Tokenables/WebAuthnCredentialCreateOptionsTokenableTests.cs b/test/Core.Test/Auth/Models/Business/Tokenables/WebAuthnCredentialCreateOptionsTokenableTests.cs new file mode 100644 index 000000000000..c16f5c910013 --- /dev/null +++ b/test/Core.Test/Auth/Models/Business/Tokenables/WebAuthnCredentialCreateOptionsTokenableTests.cs @@ -0,0 +1,81 @@ +using Bit.Core.Auth.Models.Business.Tokenables; +using Bit.Core.Entities; +using Bit.Test.Common.AutoFixture.Attributes; +using Fido2NetLib; +using Xunit; + +namespace Bit.Core.Test.Auth.Models.Business.Tokenables; + +public class WebAuthnCredentialCreateOptionsTokenableTests +{ + [Theory, BitAutoData] + public void Valid_TokenWithoutUser_ReturnsFalse(CredentialCreateOptions createOptions) + { + var token = new WebAuthnCredentialCreateOptionsTokenable(null, createOptions); + + var isValid = token.Valid; + + Assert.False(isValid); + } + + [Theory, BitAutoData] + public void Valid_TokenWithoutOptions_ReturnsFalse(User user) + { + var token = new WebAuthnCredentialCreateOptionsTokenable(user, null); + + var isValid = token.Valid; + + Assert.False(isValid); + } + + [Theory, BitAutoData] + public void Valid_NewlyCreatedToken_ReturnsTrue(User user, CredentialCreateOptions createOptions) + { + var token = new WebAuthnCredentialCreateOptionsTokenable(user, createOptions); + + var isValid = token.Valid; + + Assert.True(isValid); + } + + [Theory, BitAutoData] + public void ValidIsValid_TokenWithoutUser_ReturnsFalse(User user, CredentialCreateOptions createOptions) + { + var token = new WebAuthnCredentialCreateOptionsTokenable(null, createOptions); + + var isValid = token.TokenIsValid(user); + + Assert.False(isValid); + } + + [Theory, BitAutoData] + public void ValidIsValid_TokenWithoutOptions_ReturnsFalse(User user) + { + var token = new WebAuthnCredentialCreateOptionsTokenable(user, null); + + var isValid = token.TokenIsValid(user); + + Assert.False(isValid); + } + + [Theory, BitAutoData] + public void ValidIsValid_NonMatchingUsers_ReturnsFalse(User user1, User user2, CredentialCreateOptions createOptions) + { + var token = new WebAuthnCredentialCreateOptionsTokenable(user1, createOptions); + + var isValid = token.TokenIsValid(user2); + + Assert.False(isValid); + } + + [Theory, BitAutoData] + public void ValidIsValid_SameUser_ReturnsTrue(User user, CredentialCreateOptions createOptions) + { + var token = new WebAuthnCredentialCreateOptionsTokenable(user, createOptions); + + var isValid = token.TokenIsValid(user); + + Assert.True(isValid); + } +} + From 97048ea5d08cfcb309d38def127f39213c5875ab Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Tue, 16 May 2023 10:41:22 +0200 Subject: [PATCH 34/50] [PM-2014] chore: clean up commented premium check --- src/Api/Auth/Controllers/WebAuthnController.cs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/Api/Auth/Controllers/WebAuthnController.cs b/src/Api/Auth/Controllers/WebAuthnController.cs index bf2e3d53d3b0..6a56b56e2abc 100644 --- a/src/Api/Auth/Controllers/WebAuthnController.cs +++ b/src/Api/Auth/Controllers/WebAuthnController.cs @@ -104,12 +104,6 @@ public async Task Delete(Guid id, [FromBody] SecretVerificationRequestModel mode throw new BadRequestException(string.Empty, "User verification failed."); } - // TODO: Is premium requried? - //if (premium && !(await _userService.CanAccessPremium(user))) - //{ - // throw new BadRequestException("Premium status is required."); - //} - return user; } } From 84cd8c72155b0ace1c27f3430ce505352e90a4dd Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Tue, 16 May 2023 11:39:41 +0200 Subject: [PATCH 35/50] [PM-2014] feat: add user service test for credential limit --- test/Core.Test/Services/UserServiceTests.cs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/test/Core.Test/Services/UserServiceTests.cs b/test/Core.Test/Services/UserServiceTests.cs index 9cdda95de8e4..8aea2b26b082 100644 --- a/test/Core.Test/Services/UserServiceTests.cs +++ b/test/Core.Test/Services/UserServiceTests.cs @@ -1,6 +1,9 @@ using System.Text.Json; +using AutoFixture; +using Bit.Core.Auth.Entities; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Models; +using Bit.Core.Auth.Repositories; using Bit.Core.Entities; using Bit.Core.Models.Business; using Bit.Core.Models.Data.Organizations; @@ -9,6 +12,7 @@ using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; using Bit.Test.Common.Helpers; +using Fido2NetLib; using Microsoft.AspNetCore.Identity; using NSubstitute; using NSubstitute.ReceivedExtensions; @@ -170,4 +174,19 @@ public async void HasPremiumFromOrganization_Returns_True_If_Org_Eligible(SutPro Assert.True(await sutProvider.Sut.HasPremiumFromOrganization(user)); } + + [Theory, BitAutoData] + public async void CompleteWebAuthLoginRegistrationAsync_ExceedsExistingCredentialsLimit_ReturnsFalse(SutProvider sutProvider, User user, CredentialCreateOptions options, AuthenticatorAttestationRawResponse response, Generator credentialGenerator) + { + // Arrange + var existingCredentials = credentialGenerator.Take(5).ToList(); + sutProvider.GetDependency().GetManyByUserIdAsync(user.Id).Returns(existingCredentials); + + // Act + var result = await sutProvider.Sut.CompleteWebAuthLoginRegistrationAsync(user, "name", options, response); + + // Assert + Assert.False(result); + sutProvider.GetDependency().DidNotReceive(); + } } From 158b20d91e6996bbc4cb76cb9d792e36d74a2406 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Tue, 16 May 2023 18:09:51 +0200 Subject: [PATCH 36/50] [PM-2014] fix: run `dotnet format` --- .../Request/WebAuthn/WebAuthnCredentialRequestModel.cs | 4 ++-- src/Core/Services/Implementations/UserService.cs | 2 -- src/Identity/Controllers/AccountsController.cs | 5 +---- src/Identity/IdentityServer/ExtensionGrantValidator.cs | 10 +++++----- .../Auth/Repositories/WebAuthnCredentialRepository.cs | 3 ++- .../Auth/Controllers/WebAuthnControllerTests.cs | 1 - 6 files changed, 10 insertions(+), 15 deletions(-) diff --git a/src/Api/Auth/Models/Request/WebAuthn/WebAuthnCredentialRequestModel.cs b/src/Api/Auth/Models/Request/WebAuthn/WebAuthnCredentialRequestModel.cs index f55a963966a1..8f16fe7f5065 100644 --- a/src/Api/Auth/Models/Request/WebAuthn/WebAuthnCredentialRequestModel.cs +++ b/src/Api/Auth/Models/Request/WebAuthn/WebAuthnCredentialRequestModel.cs @@ -1,5 +1,5 @@ -using Fido2NetLib; -using System.ComponentModel.DataAnnotations; +using System.ComponentModel.DataAnnotations; +using Fido2NetLib; namespace Bit.Api.Auth.Models.Request.Webauthn; diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index 1a6517a5ce52..c9062792468e 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -26,8 +26,6 @@ using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; -using Microsoft.Identity.Client.Platforms.Features.DesktopOs.Kerberos; -using static IdentityServer4.Models.IdentityResources; using File = System.IO.File; namespace Bit.Core.Services; diff --git a/src/Identity/Controllers/AccountsController.cs b/src/Identity/Controllers/AccountsController.cs index 55184e3c0c2c..de9aa682fe78 100644 --- a/src/Identity/Controllers/AccountsController.cs +++ b/src/Identity/Controllers/AccountsController.cs @@ -1,6 +1,4 @@ -using Bit.Core.Auth.Enums; -using System.Text.Json; -using Bit.Core.Auth.Models.Api.Request.Accounts; +using Bit.Core.Auth.Models.Api.Request.Accounts; using Bit.Core.Auth.Models.Api.Response.Accounts; using Bit.Core.Auth.Services; using Bit.Core.Auth.Utilities; @@ -12,7 +10,6 @@ using Bit.SharedWeb.Utilities; using Fido2NetLib; using Microsoft.AspNetCore.Mvc; -using Bit.Identity.IdentityServer; namespace Bit.Identity.Controllers; diff --git a/src/Identity/IdentityServer/ExtensionGrantValidator.cs b/src/Identity/IdentityServer/ExtensionGrantValidator.cs index 2b13b0cfef05..b7b2aa819226 100644 --- a/src/Identity/IdentityServer/ExtensionGrantValidator.cs +++ b/src/Identity/IdentityServer/ExtensionGrantValidator.cs @@ -1,15 +1,15 @@ using System.Security.Claims; -using IdentityServer4.Models; -using IdentityServer4.Validation; -using Microsoft.AspNetCore.Identity; using Bit.Core.Auth.Identity; +using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Context; +using Bit.Core.Entities; using Bit.Core.Repositories; using Bit.Core.Services; -using Bit.Core.Entities; using Bit.Core.Settings; using Bit.Core.Tokens; -using Bit.Core.Auth.Models.Business.Tokenables; +using IdentityServer4.Models; +using IdentityServer4.Validation; +using Microsoft.AspNetCore.Identity; namespace Bit.Identity.IdentityServer; diff --git a/src/Infrastructure.EntityFramework/Auth/Repositories/WebAuthnCredentialRepository.cs b/src/Infrastructure.EntityFramework/Auth/Repositories/WebAuthnCredentialRepository.cs index e2840ea2dec4..68f14243c41e 100644 --- a/src/Infrastructure.EntityFramework/Auth/Repositories/WebAuthnCredentialRepository.cs +++ b/src/Infrastructure.EntityFramework/Auth/Repositories/WebAuthnCredentialRepository.cs @@ -13,7 +13,8 @@ public WebAuthnCredentialRepository(IServiceScopeFactory serviceScopeFactory, IM : base(serviceScopeFactory, mapper, (context) => context.WebAuthnCredentials) { } - public async Task GetByIdAsync(Guid id, Guid userId) { + public async Task GetByIdAsync(Guid id, Guid userId) + { using (var scope = ServiceScopeFactory.CreateScope()) { var dbContext = GetDatabaseContext(scope); diff --git a/test/Api.Test/Auth/Controllers/WebAuthnControllerTests.cs b/test/Api.Test/Auth/Controllers/WebAuthnControllerTests.cs index c26579dfaa8c..32f2d5d49177 100644 --- a/test/Api.Test/Auth/Controllers/WebAuthnControllerTests.cs +++ b/test/Api.Test/Auth/Controllers/WebAuthnControllerTests.cs @@ -9,7 +9,6 @@ using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; using Fido2NetLib; -using Microsoft.Identity.Client.Platforms.Features.DesktopOs.Kerberos; using NSubstitute; using NSubstitute.ReturnsExtensions; using Xunit; From 1205e9fe07c9cb43c88a39fd17faf9a9f62eeee2 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Wed, 17 May 2023 08:28:27 +0200 Subject: [PATCH 37/50] [PM-2014] chore: remove trailing comma --- src/Api/Auth/Controllers/WebAuthnController.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Api/Auth/Controllers/WebAuthnController.cs b/src/Api/Auth/Controllers/WebAuthnController.cs index 6a56b56e2abc..feff28f67f46 100644 --- a/src/Api/Auth/Controllers/WebAuthnController.cs +++ b/src/Api/Auth/Controllers/WebAuthnController.cs @@ -51,7 +51,7 @@ public async Task PostOptions([Fro return new WebAuthnCredentialCreateOptionsResponseModel { Options = options, - Token = token, + Token = token }; } From 2f644646a8671a75ab54453819bc42dbf9230b4f Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Wed, 17 May 2023 08:45:25 +0200 Subject: [PATCH 38/50] [PM-2014] chore: add `Async` suffix --- src/Api/Auth/Controllers/WebAuthnController.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Api/Auth/Controllers/WebAuthnController.cs b/src/Api/Auth/Controllers/WebAuthnController.cs index feff28f67f46..d96a9c97d50f 100644 --- a/src/Api/Auth/Controllers/WebAuthnController.cs +++ b/src/Api/Auth/Controllers/WebAuthnController.cs @@ -33,7 +33,7 @@ public WebAuthnController( [HttpGet("")] public async Task> Get() { - var user = await GetUser(); + var user = await GetUserAsync(); var credentials = await _credentialRepository.GetManyByUserIdAsync(user.Id); return new ListResponseModel(credentials.Select(c => new WebAuthnCredentialResponseModel(c))); @@ -42,7 +42,7 @@ public async Task> Get() [HttpPost("options")] public async Task PostOptions([FromBody] SecretVerificationRequestModel model) { - var user = await VerifyUser(model); + var user = await VerifyUserAsync(model); var options = await _userService.StartWebAuthnLoginRegistrationAsync(user); var tokenable = new WebAuthnCredentialCreateOptionsTokenable(user, options); @@ -58,7 +58,7 @@ public async Task PostOptions([Fro [HttpPost("")] public async Task Post([FromBody] WebAuthnCredentialRequestModel model) { - var user = await GetUser(); + var user = await GetUserAsync(); var tokenable = _createOptionsDataProtector.Unprotect(model.Token); if (!tokenable.TokenIsValid(user)) { @@ -75,7 +75,7 @@ public async Task Post([FromBody] WebAuthnCredentialRequestModel model) [HttpPost("{id}/delete")] public async Task Delete(Guid id, [FromBody] SecretVerificationRequestModel model) { - var user = await VerifyUser(model); + var user = await VerifyUserAsync(model); var credential = await _credentialRepository.GetByIdAsync(id, user.Id); if (credential == null) { @@ -85,7 +85,7 @@ public async Task Delete(Guid id, [FromBody] SecretVerificationRequestModel mode await _credentialRepository.DeleteAsync(credential); } - private async Task GetUser() + private async Task GetUserAsync() { var user = await _userService.GetUserByPrincipalAsync(User); if (user == null) @@ -95,9 +95,9 @@ public async Task Delete(Guid id, [FromBody] SecretVerificationRequestModel mode return user; } - private async Task VerifyUser(SecretVerificationRequestModel model) + private async Task VerifyUserAsync(SecretVerificationRequestModel model) { - var user = await GetUser(); + var user = await GetUserAsync(); if (!await _userService.VerifySecretAsync(user, model.Secret)) { await Task.Delay(2000); From 91ff3e5d9abefaa13d4887588a1dfb7c4ba2857d Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Wed, 17 May 2023 08:46:55 +0200 Subject: [PATCH 39/50] [PM-2014] chore: move delay to constant --- src/Api/Auth/Controllers/WebAuthnController.cs | 3 ++- src/Core/Constants.cs | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Api/Auth/Controllers/WebAuthnController.cs b/src/Api/Auth/Controllers/WebAuthnController.cs index d96a9c97d50f..26f56ec34964 100644 --- a/src/Api/Auth/Controllers/WebAuthnController.cs +++ b/src/Api/Auth/Controllers/WebAuthnController.cs @@ -7,6 +7,7 @@ using Bit.Core.Exceptions; using Bit.Core.Services; using Bit.Core.Tokens; +using Bit.Core; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; @@ -100,7 +101,7 @@ public async Task Delete(Guid id, [FromBody] SecretVerificationRequestModel mode var user = await GetUserAsync(); if (!await _userService.VerifySecretAsync(user, model.Secret)) { - await Task.Delay(2000); + await Task.Delay(Constants.FailedSecretVerificationDelay); throw new BadRequestException(string.Empty, "User verification failed."); } diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index b83db77a4df9..b508e7f2b37e 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -5,6 +5,7 @@ namespace Bit.Core; public static class Constants { public const int BypassFiltersEventId = 12482444; + public const int FailedSecretVerificationDelay = 2000; // File size limits - give 1 MB extra for cushion. // Note: if request size limits are changed, 'client_max_body_size' From edc48403ca08d651021689510928fab37c509e29 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Wed, 17 May 2023 08:52:26 +0200 Subject: [PATCH 40/50] [PM-2014] chore: change `default` to `null` --- .../Tokenables/WebAuthnCredentialCreateOptionsTokenable.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Core/Auth/Models/Business/Tokenables/WebAuthnCredentialCreateOptionsTokenable.cs b/src/Core/Auth/Models/Business/Tokenables/WebAuthnCredentialCreateOptionsTokenable.cs index ea42681bf78d..e64edace4558 100644 --- a/src/Core/Auth/Models/Business/Tokenables/WebAuthnCredentialCreateOptionsTokenable.cs +++ b/src/Core/Auth/Models/Business/Tokenables/WebAuthnCredentialCreateOptionsTokenable.cs @@ -31,7 +31,7 @@ public WebAuthnCredentialCreateOptionsTokenable(User user, CredentialCreateOptio public bool TokenIsValid(User user) { - if (!Valid || user == default) + if (!Valid || user == null) { return false; } @@ -39,6 +39,6 @@ public bool TokenIsValid(User user) return UserId == user.Id; } - protected override bool TokenIsValid() => Identifier == TokenIdentifier && UserId != null && Options != default; + protected override bool TokenIsValid() => Identifier == TokenIdentifier && UserId != null && Options != null; } From c33249dd9755f1740e936ed06212ffb51dd3171e Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Wed, 17 May 2023 08:56:02 +0200 Subject: [PATCH 41/50] [PM-2014] chore: remove autogenerated weirdness --- test/Api.Test/Api.Test.csproj | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/Api.Test/Api.Test.csproj b/test/Api.Test/Api.Test.csproj index cf5c3b86b0a4..8862d800f544 100644 --- a/test/Api.Test/Api.Test.csproj +++ b/test/Api.Test/Api.Test.csproj @@ -26,10 +26,6 @@ - - - - From 02336857c01ebb294c49d9fd327ececfa460fe0c Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Wed, 17 May 2023 14:00:45 +0200 Subject: [PATCH 42/50] [PM-2241] feat: add prf support to database --- .../WebAuthnCredential_Create.sql | 11 +- .../WebAuthnCredential_Update.sql | 6 + src/Sql/dbo/Tables/WebAuthnCredential.sql | 3 + .../DbScripts/2023-05-17-00_PrfSupport.sql | 122 ++++++++++++++++++ 4 files changed, 141 insertions(+), 1 deletion(-) create mode 100644 util/Migrator/DbScripts/2023-05-17-00_PrfSupport.sql diff --git a/src/Sql/dbo/Stored Procedures/WebAuthnCredential_Create.sql b/src/Sql/dbo/Stored Procedures/WebAuthnCredential_Create.sql index e4a3816e1d05..ce69d4094024 100644 --- a/src/Sql/dbo/Stored Procedures/WebAuthnCredential_Create.sql +++ b/src/Sql/dbo/Stored Procedures/WebAuthnCredential_Create.sql @@ -8,6 +8,9 @@ @Type VARCHAR(20), @AaGuid UNIQUEIDENTIFIER, @UserKey VARCHAR (MAX), + @PrfPublicKey VARCHAR (MAX), + @PrfPrivateKey VARCHAR (MAX), + @SupportsPrf BIT, @CreationDate DATETIME2(7), @RevisionDate DATETIME2(7) AS @@ -25,6 +28,9 @@ BEGIN [Type], [AaGuid], [UserKey], + [PrfPublicKey], + [PrfPrivateKey], + [SupportsPrf], [CreationDate], [RevisionDate] ) @@ -39,7 +45,10 @@ BEGIN @Type, @AaGuid, @UserKey, + @PrfPublicKey, + @PrfPrivateKey, + @SupportsPrf, @CreationDate, @RevisionDate ) -END +END \ No newline at end of file diff --git a/src/Sql/dbo/Stored Procedures/WebAuthnCredential_Update.sql b/src/Sql/dbo/Stored Procedures/WebAuthnCredential_Update.sql index 060d312fd5f2..230ddd0d6c02 100644 --- a/src/Sql/dbo/Stored Procedures/WebAuthnCredential_Update.sql +++ b/src/Sql/dbo/Stored Procedures/WebAuthnCredential_Update.sql @@ -8,6 +8,9 @@ @Type VARCHAR(20), @AaGuid UNIQUEIDENTIFIER, @UserKey VARCHAR (MAX), + @PrfPublicKey VARCHAR (MAX), + @PrfPrivateKey VARCHAR (MAX), + @SupportsPrf BIT, @CreationDate DATETIME2(7), @RevisionDate DATETIME2(7) AS @@ -25,6 +28,9 @@ BEGIN [Type] = @Type, [AaGuid] = @AaGuid, [UserKey] = @UserKey, + [PrfPublicKey] = @PrfPublicKey, + [PrfPrivateKey] = @PrfPrivateKey, + [SupportsPrf] = @SupportsPrf, [CreationDate] = @CreationDate, [RevisionDate] = @RevisionDate WHERE diff --git a/src/Sql/dbo/Tables/WebAuthnCredential.sql b/src/Sql/dbo/Tables/WebAuthnCredential.sql index 670869b85b0f..c92b842266b4 100644 --- a/src/Sql/dbo/Tables/WebAuthnCredential.sql +++ b/src/Sql/dbo/Tables/WebAuthnCredential.sql @@ -8,6 +8,9 @@ [Type] VARCHAR (20) NULL, [AaGuid] UNIQUEIDENTIFIER NOT NULL, [UserKey] VARCHAR (MAX) NULL, + [PrfPublicKey] VARCHAR (MAX) NULL, + [PrfPrivateKey] VARCHAR (MAX) NULL, + [SupportsPrf] BIT NOT NULL, [CreationDate] DATETIME2 (7) NOT NULL, [RevisionDate] DATETIME2 (7) NOT NULL, CONSTRAINT [PK_WebAuthnCredential] PRIMARY KEY CLUSTERED ([Id] ASC), diff --git a/util/Migrator/DbScripts/2023-05-17-00_PrfSupport.sql b/util/Migrator/DbScripts/2023-05-17-00_PrfSupport.sql new file mode 100644 index 000000000000..9b35b61dee4e --- /dev/null +++ b/util/Migrator/DbScripts/2023-05-17-00_PrfSupport.sql @@ -0,0 +1,122 @@ +IF COL_LENGTH('[dbo].[WebAuthnCredential]', 'PrfPublicKey') IS NULL +BEGIN + ALTER TABLE + [dbo].[WebAuthnCredential] + ADD + [PrfPublicKey] VARCHAR (MAX) NULL +END +GO + +IF COL_LENGTH('[dbo].[WebAuthnCredential]', 'PrfPrivateKey') IS NULL +BEGIN + ALTER TABLE + [dbo].[WebAuthnCredential] + ADD + [PrfPrivateKey] VARCHAR (MAX) NULL +END +GO + +IF COL_LENGTH('[dbo].[WebAuthnCredential]', 'SupportsPrf') IS NULL +BEGIN + ALTER TABLE + [dbo].[WebAuthnCredential] + ADD + [SupportsPrf] BIT NOT NULL DEFAULT 0 +END +GO + +GO +CREATE OR ALTER PROCEDURE [dbo].[WebAuthnCredential_Create] + @Id UNIQUEIDENTIFIER OUTPUT, + @UserId UNIQUEIDENTIFIER, + @Name NVARCHAR(50), + @PublicKey VARCHAR (256), + @DescriptorId VARCHAR(256), + @Counter INT, + @Type VARCHAR(20), + @AaGuid UNIQUEIDENTIFIER, + @UserKey VARCHAR (MAX), + @PrfPublicKey VARCHAR (MAX), + @PrfPrivateKey VARCHAR (MAX), + @SupportsPrf BIT, + @CreationDate DATETIME2(7), + @RevisionDate DATETIME2(7) +AS +BEGIN + SET NOCOUNT ON + + INSERT INTO [dbo].[WebAuthnCredential] + ( + [Id], + [UserId], + [Name], + [PublicKey], + [DescriptorId], + [Counter], + [Type], + [AaGuid], + [UserKey], + [PrfPublicKey], + [PrfPrivateKey], + [SupportsPrf], + [CreationDate], + [RevisionDate] + ) + VALUES + ( + @Id, + @UserId, + @Name, + @PublicKey, + @DescriptorId, + @Counter, + @Type, + @AaGuid, + @UserKey, + @PrfPublicKey, + @PrfPrivateKey, + @SupportsPrf, + @CreationDate, + @RevisionDate + ) +END + +GO +CREATE OR ALTER PROCEDURE [dbo].[WebAuthnCredential_Update] + @Id UNIQUEIDENTIFIER, + @UserId UNIQUEIDENTIFIER, + @Name NVARCHAR(50), + @PublicKey VARCHAR (256), + @DescriptorId VARCHAR(256), + @Counter INT, + @Type VARCHAR(20), + @AaGuid UNIQUEIDENTIFIER, + @UserKey VARCHAR (MAX), + @PrfPublicKey VARCHAR (MAX), + @PrfPrivateKey VARCHAR (MAX), + @SupportsPrf BIT, + @CreationDate DATETIME2(7), + @RevisionDate DATETIME2(7) +AS +BEGIN + SET NOCOUNT ON + + UPDATE + [dbo].[WebAuthnCredential] + SET + [UserId] = @UserId, + [Name] = @Name, + [PublicKey] = @PublicKey, + [DescriptorId] = @DescriptorId, + [Counter] = @Counter, + [Type] = @Type, + [AaGuid] = @AaGuid, + [UserKey] = @UserKey, + [PrfPublicKey] = @PrfPublicKey, + [PrfPrivateKey] = @PrfPrivateKey, + [SupportsPrf] = @SupportsPrf, + [CreationDate] = @CreationDate, + [RevisionDate] = @RevisionDate + WHERE + [Id] = @Id +END From 1f1cbd57a49b2689d478d116a8ab4b10b8adcaf8 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Wed, 17 May 2023 14:57:55 +0200 Subject: [PATCH 43/50] [PM-2241] feat: add support for saving prf --- .../Auth/Controllers/WebAuthnController.cs | 2 +- .../WebAuthnCredentialRequestModel.cs | 5 +++++ .../WebAuthnCredentialResponseModel.cs | 5 +++-- src/Core/Auth/Entities/WebAuthnCredential.cs | 19 +++++++++++++++++++ src/Core/Auth/Enums/WebAuthnPrfStatus.cs | 9 +++++++++ src/Core/Services/IUserService.cs | 2 +- .../Services/Implementations/UserService.cs | 8 ++++++-- .../DbScripts/2023-05-17-00_PrfSupport.sql | 13 +++++++++++++ 8 files changed, 57 insertions(+), 6 deletions(-) create mode 100644 src/Core/Auth/Enums/WebAuthnPrfStatus.cs diff --git a/src/Api/Auth/Controllers/WebAuthnController.cs b/src/Api/Auth/Controllers/WebAuthnController.cs index 26f56ec34964..f18e0c236a2c 100644 --- a/src/Api/Auth/Controllers/WebAuthnController.cs +++ b/src/Api/Auth/Controllers/WebAuthnController.cs @@ -66,7 +66,7 @@ public async Task Post([FromBody] WebAuthnCredentialRequestModel model) throw new BadRequestException("The token associated with your request is expired. A valid token is required to continue."); } - var success = await _userService.CompleteWebAuthLoginRegistrationAsync(user, model.Name, tokenable.Options, model.DeviceResponse); + var success = await _userService.CompleteWebAuthLoginRegistrationAsync(user, model.Name, model.UserKey, model.PrfPublicKey, model.PrfPrivateKey, model.SupportsPrf, tokenable.Options, model.DeviceResponse); if (!success) { throw new BadRequestException("Unable to complete WebAuthn registration."); diff --git a/src/Api/Auth/Models/Request/WebAuthn/WebAuthnCredentialRequestModel.cs b/src/Api/Auth/Models/Request/WebAuthn/WebAuthnCredentialRequestModel.cs index 8f16fe7f5065..44eb4e975c11 100644 --- a/src/Api/Auth/Models/Request/WebAuthn/WebAuthnCredentialRequestModel.cs +++ b/src/Api/Auth/Models/Request/WebAuthn/WebAuthnCredentialRequestModel.cs @@ -13,5 +13,10 @@ public class WebAuthnCredentialRequestModel [Required] public string Token { get; set; } + + public string UserKey { get; set; } + public string PrfPublicKey { get; set; } + public string PrfPrivateKey { get; set; } + public bool SupportsPrf { get; set; } } diff --git a/src/Api/Auth/Models/Response/WebAuthn/WebAuthnCredentialResponseModel.cs b/src/Api/Auth/Models/Response/WebAuthn/WebAuthnCredentialResponseModel.cs index 0e358c751d32..01cf2559a6e5 100644 --- a/src/Api/Auth/Models/Response/WebAuthn/WebAuthnCredentialResponseModel.cs +++ b/src/Api/Auth/Models/Response/WebAuthn/WebAuthnCredentialResponseModel.cs @@ -1,4 +1,5 @@ using Bit.Core.Auth.Entities; +using Bit.Core.Auth.Enums; using Bit.Core.Models.Api; namespace Bit.Api.Auth.Models.Response.WebAuthn; @@ -11,10 +12,10 @@ public WebAuthnCredentialResponseModel(WebAuthnCredential credential) : base(Res { Id = credential.Id.ToString(); Name = credential.Name; - PrfSupport = false; + PrfStatus = credential.GetPrfStatus(); } public string Id { get; set; } public string Name { get; set; } - public bool PrfSupport { get; set; } + public WebAuthnPrfStatus PrfStatus { get; set; } } diff --git a/src/Core/Auth/Entities/WebAuthnCredential.cs b/src/Core/Auth/Entities/WebAuthnCredential.cs index a1195b82ae74..820977d54e9a 100644 --- a/src/Core/Auth/Entities/WebAuthnCredential.cs +++ b/src/Core/Auth/Entities/WebAuthnCredential.cs @@ -1,4 +1,5 @@ using System.ComponentModel.DataAnnotations; +using Bit.Core.Auth.Enums; using Bit.Core.Entities; using Bit.Core.Utilities; @@ -19,6 +20,9 @@ public class WebAuthnCredential : ITableObject public string Type { get; set; } public Guid AaGuid { get; set; } public string UserKey { get; set; } + public string PrfPublicKey { get; set; } + public string PrfPrivateKey { get; set; } + public bool SupportsPrf { get; set; } public DateTime CreationDate { get; internal set; } = DateTime.UtcNow; public DateTime RevisionDate { get; internal set; } = DateTime.UtcNow; @@ -26,4 +30,19 @@ public void SetNewId() { Id = CoreHelpers.GenerateComb(); } + + public WebAuthnPrfStatus GetPrfStatus() + { + if (SupportsPrf && PrfPublicKey != null && PrfPrivateKey != null) + { + return WebAuthnPrfStatus.Enabled; + } + else if (SupportsPrf) + { + return WebAuthnPrfStatus.Supported; + } + + return WebAuthnPrfStatus.Unsupported; + } + } diff --git a/src/Core/Auth/Enums/WebAuthnPrfStatus.cs b/src/Core/Auth/Enums/WebAuthnPrfStatus.cs new file mode 100644 index 000000000000..52bb899d74d2 --- /dev/null +++ b/src/Core/Auth/Enums/WebAuthnPrfStatus.cs @@ -0,0 +1,9 @@ +namespace Bit.Core.Auth.Enums; + +public enum WebAuthnPrfStatus +{ + Enabled = 0, + Supported = 1, + Unsupported = 2 +} + diff --git a/src/Core/Services/IUserService.cs b/src/Core/Services/IUserService.cs index e27668946638..d4c82053184a 100644 --- a/src/Core/Services/IUserService.cs +++ b/src/Core/Services/IUserService.cs @@ -28,7 +28,7 @@ public interface IUserService Task DeleteWebAuthnKeyAsync(User user, int id); Task CompleteWebAuthRegistrationAsync(User user, int value, string name, AuthenticatorAttestationRawResponse attestationResponse); Task StartWebAuthnLoginRegistrationAsync(User user); - Task CompleteWebAuthLoginRegistrationAsync(User user, string name, CredentialCreateOptions options, AuthenticatorAttestationRawResponse attestationResponse); + Task CompleteWebAuthLoginRegistrationAsync(User user, string name, string userKey, string prfPublicKey, string prfPrivateKey, bool supportsPrf, CredentialCreateOptions options, AuthenticatorAttestationRawResponse attestationResponse); Task StartWebAuthnLoginAssertionAsync(User user); Task CompleteWebAuthLoginAssertionAsync(AuthenticatorAssertionRawResponse assertionResponse, User user); Task SendEmailVerificationAsync(User user); diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index c9062792468e..eb6a19c96d07 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -539,7 +539,7 @@ public async Task StartWebAuthnLoginRegistrationAsync(U return options; } - public async Task CompleteWebAuthLoginRegistrationAsync(User user, string name, + public async Task CompleteWebAuthLoginRegistrationAsync(User user, string name, string userKey, string prfPublicKey, string prfPrivateKey, bool supportsPrf, CredentialCreateOptions options, AuthenticatorAttestationRawResponse attestationResponse) { @@ -562,7 +562,11 @@ public async Task CompleteWebAuthLoginRegistrationAsync(User user, string Type = success.Result.CredType, AaGuid = success.Result.Aaguid, Counter = (int)success.Result.Counter, - UserId = user.Id + UserId = user.Id, + UserKey = userKey, + SupportsPrf = supportsPrf, + PrfPublicKey = prfPublicKey, + PrfPrivateKey = prfPrivateKey }; await _webAuthnCredentialRepository.CreateAsync(credential); diff --git a/util/Migrator/DbScripts/2023-05-17-00_PrfSupport.sql b/util/Migrator/DbScripts/2023-05-17-00_PrfSupport.sql index 9b35b61dee4e..37fbe093cae1 100644 --- a/util/Migrator/DbScripts/2023-05-17-00_PrfSupport.sql +++ b/util/Migrator/DbScripts/2023-05-17-00_PrfSupport.sql @@ -25,6 +25,19 @@ BEGIN END GO +IF OBJECT_ID('[dbo].[WebAuthnCredentialView]') IS NOT NULL +BEGIN + DROP VIEW [dbo].[WebAuthnCredentialView] +END +GO + +CREATE VIEW [dbo].[WebAuthnCredentialView] +AS +SELECT + * +FROM + [dbo].[WebAuthnCredential] + GO CREATE OR ALTER PROCEDURE [dbo].[WebAuthnCredential_Create] @Id UNIQUEIDENTIFIER OUTPUT, From 7d3582d5f17cf4de8e60c0b68cbda40d63aab417 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Thu, 18 May 2023 15:20:15 +0200 Subject: [PATCH 44/50] [PM-2023] feat: implement assert options fetching --- ...WebauthnCredentialAssertionRequestModel.cs | 11 +++++++ ...ebauthnCredentialAssertionResponseModel.cs | 17 ++++++++++ .../Services/Implementations/UserService.cs | 32 +++++++++++-------- .../Controllers/AccountsController.cs | 22 +++++++------ 4 files changed, 60 insertions(+), 22 deletions(-) create mode 100644 src/Core/Auth/Models/Api/Request/Accounts/WebauthnCredentialAssertionRequestModel.cs create mode 100644 src/Core/Auth/Models/Api/Response/Accounts/WebauthnCredentialAssertionResponseModel.cs diff --git a/src/Core/Auth/Models/Api/Request/Accounts/WebauthnCredentialAssertionRequestModel.cs b/src/Core/Auth/Models/Api/Request/Accounts/WebauthnCredentialAssertionRequestModel.cs new file mode 100644 index 000000000000..f9dd9c107e70 --- /dev/null +++ b/src/Core/Auth/Models/Api/Request/Accounts/WebauthnCredentialAssertionRequestModel.cs @@ -0,0 +1,11 @@ +using System.ComponentModel.DataAnnotations; + +namespace Bit.Core.Auth.Models.Api.Request.Accounts; + +public class WebauthnCredentialAssertionRequestModel +{ + [EmailAddress] + [StringLength(256)] + public string Email { get; set; } +} + diff --git a/src/Core/Auth/Models/Api/Response/Accounts/WebauthnCredentialAssertionResponseModel.cs b/src/Core/Auth/Models/Api/Response/Accounts/WebauthnCredentialAssertionResponseModel.cs new file mode 100644 index 000000000000..9ec5609fec4e --- /dev/null +++ b/src/Core/Auth/Models/Api/Response/Accounts/WebauthnCredentialAssertionResponseModel.cs @@ -0,0 +1,17 @@ +using Bit.Core.Models.Api; +using Fido2NetLib; + +namespace Bit.Core.Auth.Models.Api.Response.Accounts; + +public class WebAuthnCredentialAssertionOptionsResponseModel : ResponseModel +{ + private const string ResponseObj = "webauthnCredentialAssertionOptions"; + + public WebAuthnCredentialAssertionOptionsResponseModel() : base(ResponseObj) + { + } + + public AssertionOptions Options { get; set; } + public string Token { get; set; } +} + diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index eb6a19c96d07..608da1e0b63e 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -575,22 +575,28 @@ public async Task CompleteWebAuthLoginRegistrationAsync(User user, string public async Task StartWebAuthnLoginAssertionAsync(User user) { - var provider = user.GetTwoFactorProvider(TwoFactorProviderType.WebAuthn); - var existingKeys = await _webAuthnCredentialRepository.GetManyByUserIdAsync(user.Id); - var existingCredentials = existingKeys - .Select(k => new PublicKeyCredentialDescriptor(CoreHelpers.Base64UrlDecode(k.DescriptorId))) - .ToList(); - - if (existingCredentials.Count == 0) + if (user != null) { - return null; + throw new NotImplementedException(); } - // TODO: PRF? - var exts = new AuthenticationExtensionsClientInputs - { - UserVerificationMethod = true - }; + //var extensions = new AuthenticationExtensionsClientInputs { }; + + //var options = _fido2.RequestNewCredential(fidoUser, excludeCredentials, authenticatorSelection, + // AttestationConveyancePreference.None, extensions); + //var provider = user.GetTwoFactorProvider(TwoFactorProviderType.WebAuthn); + //var existingKeys = await _webAuthnCredentialRepository.GetManyByUserIdAsync(user.Id); + //var existingCredentials = existingKeys + // .Select(k => new PublicKeyCredentialDescriptor(CoreHelpers.Base64UrlDecode(k.DescriptorId))) + // .ToList(); + var existingCredentials = new List(); + + //if (existingCredentials.Count == 0) + //{ + // return null; + //} + + var exts = new AuthenticationExtensionsClientInputs(); var options = _fido2.GetAssertionOptions(existingCredentials, UserVerificationRequirement.Preferred, exts); // TODO: temp save options to user record somehow diff --git a/src/Identity/Controllers/AccountsController.cs b/src/Identity/Controllers/AccountsController.cs index de9aa682fe78..e5a77759dc97 100644 --- a/src/Identity/Controllers/AccountsController.cs +++ b/src/Identity/Controllers/AccountsController.cs @@ -76,17 +76,21 @@ public async Task PostPrelogin([FromBody] PreloginRequest [HttpPost("webauthn-assertion-options")] [ApiExplorerSettings(IgnoreApi = true)] // Disable Swagger due to CredentialCreateOptions not converting properly // TODO: Create proper models for this call - public async Task PostWebAuthnAssertionOptions([FromBody] PreloginRequestModel model) + public async Task PostWebAuthnAssertionOptions([FromBody] WebauthnCredentialAssertionRequestModel model) { - var user = await _userRepository.GetByEmailAsync(model.Email); - if (user == null) - { - // TODO: return something? possible enumeration attacks with this response - return new AssertionOptions(); - } + //var user = await _userRepository.GetByEmailAsync(model.Email); + //if (user == null) + //{ + // // TODO: return something? possible enumeration attacks with this response + // return new AssertionOptions(); + //} - var options = await _userService.StartWebAuthnLoginAssertionAsync(user); - return options; + var options = await _userService.StartWebAuthnLoginAssertionAsync(null); + return new WebAuthnCredentialAssertionOptionsResponseModel + { + Options = options, + Token = "NotImplemented" + }; } [HttpPost("webauthn-assertion")] From 8754e0a8d29bcb46c03bbf74959f98ff671616e0 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Mon, 22 May 2023 10:29:45 +0200 Subject: [PATCH 45/50] [PM-2023] feat: add support for assertion token --- ...uthnCredentialAssertionOptionsTokenable.cs | 44 +++++++++++++++++++ .../Controllers/AccountsController.cs | 14 ++++-- .../Utilities/ServiceCollectionExtensions.cs | 8 +++- 3 files changed, 62 insertions(+), 4 deletions(-) create mode 100644 src/Core/Auth/Models/Business/Tokenables/WebAuthnCredentialAssertionOptionsTokenable.cs diff --git a/src/Core/Auth/Models/Business/Tokenables/WebAuthnCredentialAssertionOptionsTokenable.cs b/src/Core/Auth/Models/Business/Tokenables/WebAuthnCredentialAssertionOptionsTokenable.cs new file mode 100644 index 000000000000..4cc8751e528c --- /dev/null +++ b/src/Core/Auth/Models/Business/Tokenables/WebAuthnCredentialAssertionOptionsTokenable.cs @@ -0,0 +1,44 @@ +using System.Text.Json.Serialization; +using Bit.Core.Entities; +using Bit.Core.Tokens; +using Fido2NetLib; + +namespace Bit.Core.Auth.Models.Business.Tokenables; + +public class WebAuthnCredentialAssertionOptionsTokenable : ExpiringTokenable +{ + // 7 minutes = max webauthn timeout (6 minutes) + slack for miscellaneous delays + private const double _tokenLifetimeInHours = (double)7 / 60; + public const string ClearTextPrefix = "BWWebAuthnCredentialAssertionOptions_"; + public const string DataProtectorPurpose = "WebAuthnCredentialAssertionDataProtector"; + public const string TokenIdentifier = "WebAuthnCredentialAssertionOptionsToken"; + + public string Identifier { get; set; } = TokenIdentifier; + public Guid? UserId { get; set; } + public AssertionOptions Options { get; set; } + + [JsonConstructor] + public WebAuthnCredentialAssertionOptionsTokenable() + { + ExpirationDate = DateTime.UtcNow.AddHours(_tokenLifetimeInHours); + } + + public WebAuthnCredentialAssertionOptionsTokenable(User user, AssertionOptions options) : this() + { + UserId = user?.Id; + Options = options; + } + + public bool TokenIsValid(User user) + { + if (!Valid || user == null) + { + return false; + } + + return UserId == user.Id; + } + + protected override bool TokenIsValid() => Identifier == TokenIdentifier && UserId != null && Options != null; +} + diff --git a/src/Identity/Controllers/AccountsController.cs b/src/Identity/Controllers/AccountsController.cs index e5a77759dc97..d02ed609a01f 100644 --- a/src/Identity/Controllers/AccountsController.cs +++ b/src/Identity/Controllers/AccountsController.cs @@ -1,5 +1,6 @@ using Bit.Core.Auth.Models.Api.Request.Accounts; using Bit.Core.Auth.Models.Api.Response.Accounts; +using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Auth.Services; using Bit.Core.Auth.Utilities; using Bit.Core.Enums; @@ -7,6 +8,7 @@ using Bit.Core.Models.Data; using Bit.Core.Repositories; using Bit.Core.Services; +using Bit.Core.Tokens; using Bit.SharedWeb.Utilities; using Fido2NetLib; using Microsoft.AspNetCore.Mvc; @@ -21,17 +23,20 @@ public class AccountsController : Controller private readonly IUserRepository _userRepository; private readonly IUserService _userService; private readonly ICaptchaValidationService _captchaValidationService; + private readonly IDataProtectorTokenFactory _assertionOptionsDataProtector; public AccountsController( ILogger logger, IUserRepository userRepository, IUserService userService, - ICaptchaValidationService captchaValidationService) + ICaptchaValidationService captchaValidationService, + IDataProtectorTokenFactory assertionOptionsDataProtector) { _logger = logger; _userRepository = userRepository; _userService = userService; _captchaValidationService = captchaValidationService; + _assertionOptionsDataProtector = assertionOptionsDataProtector; } // Moved from API, If you modify this endpoint, please update API as well. Self hosted installs still use the API endpoints. @@ -84,12 +89,15 @@ public async Task PostWebAuthnA // // TODO: return something? possible enumeration attacks with this response // return new AssertionOptions(); //} - var options = await _userService.StartWebAuthnLoginAssertionAsync(null); + + var tokenable = new WebAuthnCredentialAssertionOptionsTokenable(null, options); + var token = _assertionOptionsDataProtector.Protect(tokenable); + return new WebAuthnCredentialAssertionOptionsResponseModel { Options = options, - Token = "NotImplemented" + Token = token }; } diff --git a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs index 595b24ef1396..33ce8e9928f4 100644 --- a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs +++ b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs @@ -171,7 +171,13 @@ public static void AddTokenizers(this IServiceCollection services) WebAuthnCredentialCreateOptionsTokenable.DataProtectorPurpose, serviceProvider.GetDataProtectionProvider(), serviceProvider.GetRequiredService>>())); - services.AddSingleton>(serviceProvider => + services.AddSingleton>(serviceProvider => + new DataProtectorTokenFactory( + WebAuthnCredentialAssertionOptionsTokenable.ClearTextPrefix, + WebAuthnCredentialAssertionOptionsTokenable.DataProtectorPurpose, + serviceProvider.GetDataProtectionProvider(), + serviceProvider.GetRequiredService>>())); + services.AddSingleton>(serviceProvider => new DataProtectorTokenFactory( SsoEmail2faSessionTokenable.ClearTextPrefix, SsoEmail2faSessionTokenable.DataProtectorPurpose, From 4e8fd85e9ba4965d79321959d3fe4e1e0a980ed7 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Mon, 22 May 2023 13:44:09 +0200 Subject: [PATCH 46/50] [PM-2023] feat: implement working assertion --- .../WebAuthnCredentialRequestModel.cs | 4 +++- ...CredentialAssertionOptionsRequestModel.cs} | 2 +- ...ebauthnCredentialAssertionRequestModel .cs | 14 +++++++++++ ...CredentialAssertionOptionsResponseModel.cs | 17 ++++++++++++++ ...ebauthnCredentialAssertionResponseModel.cs | 8 +++---- src/Core/Services/IUserService.cs | 2 +- .../Services/Implementations/UserService.cs | 20 ++++++++-------- .../Controllers/AccountsController.cs | 23 ++++++++++++------- 8 files changed, 63 insertions(+), 27 deletions(-) rename src/Core/Auth/Models/Api/Request/Accounts/{WebauthnCredentialAssertionRequestModel.cs => WebauthnCredentialAssertionOptionsRequestModel.cs} (75%) create mode 100644 src/Core/Auth/Models/Api/Request/Accounts/WebauthnCredentialAssertionRequestModel .cs create mode 100644 src/Core/Auth/Models/Api/Response/Accounts/WebauthnCredentialAssertionOptionsResponseModel.cs diff --git a/src/Api/Auth/Models/Request/WebAuthn/WebAuthnCredentialRequestModel.cs b/src/Api/Auth/Models/Request/WebAuthn/WebAuthnCredentialRequestModel.cs index 44eb4e975c11..4624dd2253aa 100644 --- a/src/Api/Auth/Models/Request/WebAuthn/WebAuthnCredentialRequestModel.cs +++ b/src/Api/Auth/Models/Request/WebAuthn/WebAuthnCredentialRequestModel.cs @@ -14,9 +14,11 @@ public class WebAuthnCredentialRequestModel [Required] public string Token { get; set; } + [Required] + public bool SupportsPrf { get; set; } + public string UserKey { get; set; } public string PrfPublicKey { get; set; } public string PrfPrivateKey { get; set; } - public bool SupportsPrf { get; set; } } diff --git a/src/Core/Auth/Models/Api/Request/Accounts/WebauthnCredentialAssertionRequestModel.cs b/src/Core/Auth/Models/Api/Request/Accounts/WebauthnCredentialAssertionOptionsRequestModel.cs similarity index 75% rename from src/Core/Auth/Models/Api/Request/Accounts/WebauthnCredentialAssertionRequestModel.cs rename to src/Core/Auth/Models/Api/Request/Accounts/WebauthnCredentialAssertionOptionsRequestModel.cs index f9dd9c107e70..08b7c9ab732a 100644 --- a/src/Core/Auth/Models/Api/Request/Accounts/WebauthnCredentialAssertionRequestModel.cs +++ b/src/Core/Auth/Models/Api/Request/Accounts/WebauthnCredentialAssertionOptionsRequestModel.cs @@ -2,7 +2,7 @@ namespace Bit.Core.Auth.Models.Api.Request.Accounts; -public class WebauthnCredentialAssertionRequestModel +public class WebauthnCredentialAssertionOptionsRequestModel { [EmailAddress] [StringLength(256)] diff --git a/src/Core/Auth/Models/Api/Request/Accounts/WebauthnCredentialAssertionRequestModel .cs b/src/Core/Auth/Models/Api/Request/Accounts/WebauthnCredentialAssertionRequestModel .cs new file mode 100644 index 000000000000..cf83d47cf52b --- /dev/null +++ b/src/Core/Auth/Models/Api/Request/Accounts/WebauthnCredentialAssertionRequestModel .cs @@ -0,0 +1,14 @@ +using System.ComponentModel.DataAnnotations; +using Fido2NetLib; + +namespace Bit.Core.Auth.Models.Api.Request.Accounts; + +public class WebauthnCredentialAssertionRequestModel +{ + [Required] + public AuthenticatorAssertionRawResponse DeviceResponse { get; set; } + + [Required] + public string Token { get; set; } +} + diff --git a/src/Core/Auth/Models/Api/Response/Accounts/WebauthnCredentialAssertionOptionsResponseModel.cs b/src/Core/Auth/Models/Api/Response/Accounts/WebauthnCredentialAssertionOptionsResponseModel.cs new file mode 100644 index 000000000000..9ec5609fec4e --- /dev/null +++ b/src/Core/Auth/Models/Api/Response/Accounts/WebauthnCredentialAssertionOptionsResponseModel.cs @@ -0,0 +1,17 @@ +using Bit.Core.Models.Api; +using Fido2NetLib; + +namespace Bit.Core.Auth.Models.Api.Response.Accounts; + +public class WebAuthnCredentialAssertionOptionsResponseModel : ResponseModel +{ + private const string ResponseObj = "webauthnCredentialAssertionOptions"; + + public WebAuthnCredentialAssertionOptionsResponseModel() : base(ResponseObj) + { + } + + public AssertionOptions Options { get; set; } + public string Token { get; set; } +} + diff --git a/src/Core/Auth/Models/Api/Response/Accounts/WebauthnCredentialAssertionResponseModel.cs b/src/Core/Auth/Models/Api/Response/Accounts/WebauthnCredentialAssertionResponseModel.cs index 9ec5609fec4e..bbf9b7c4233d 100644 --- a/src/Core/Auth/Models/Api/Response/Accounts/WebauthnCredentialAssertionResponseModel.cs +++ b/src/Core/Auth/Models/Api/Response/Accounts/WebauthnCredentialAssertionResponseModel.cs @@ -1,17 +1,15 @@ using Bit.Core.Models.Api; -using Fido2NetLib; namespace Bit.Core.Auth.Models.Api.Response.Accounts; -public class WebAuthnCredentialAssertionOptionsResponseModel : ResponseModel +public class WebAuthnCredentialAssertionResponseModel : ResponseModel { - private const string ResponseObj = "webauthnCredentialAssertionOptions"; + private const string ResponseObj = "webauthnCredentialAssertion"; - public WebAuthnCredentialAssertionOptionsResponseModel() : base(ResponseObj) + public WebAuthnCredentialAssertionResponseModel() : base(ResponseObj) { } - public AssertionOptions Options { get; set; } public string Token { get; set; } } diff --git a/src/Core/Services/IUserService.cs b/src/Core/Services/IUserService.cs index d4c82053184a..b9fe880b3265 100644 --- a/src/Core/Services/IUserService.cs +++ b/src/Core/Services/IUserService.cs @@ -30,7 +30,7 @@ public interface IUserService Task StartWebAuthnLoginRegistrationAsync(User user); Task CompleteWebAuthLoginRegistrationAsync(User user, string name, string userKey, string prfPublicKey, string prfPrivateKey, bool supportsPrf, CredentialCreateOptions options, AuthenticatorAttestationRawResponse attestationResponse); Task StartWebAuthnLoginAssertionAsync(User user); - Task CompleteWebAuthLoginAssertionAsync(AuthenticatorAssertionRawResponse assertionResponse, User user); + Task CompleteWebAuthLoginAssertionAsync(Guid? nonDiscoverableUserId, AssertionOptions options, AuthenticatorAssertionRawResponse deviceResponse); Task SendEmailVerificationAsync(User user); Task ConfirmEmailAsync(User user, string token); Task InitiateEmailChangeAsync(User user, string newEmail); diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index 608da1e0b63e..5194b98ee020 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -527,7 +527,7 @@ public async Task StartWebAuthnLoginRegistrationAsync(U var authenticatorSelection = new AuthenticatorSelection { AuthenticatorAttachment = null, - RequireResidentKey = false, // TODO: This is using the old residentKey selection variant, we need to update our lib so that we can set this to preferred + RequireResidentKey = true, // TODO: This is using the old residentKey selection variant, we need to update our lib so that we can set this to preferred UserVerification = UserVerificationRequirement.Preferred }; @@ -604,24 +604,21 @@ public async Task StartWebAuthnLoginAssertionAsync(User user) return options; } - public async Task CompleteWebAuthLoginAssertionAsync(AuthenticatorAssertionRawResponse assertionResponse, User user) + public async Task CompleteWebAuthLoginAssertionAsync(Guid? nonDiscoverableUserId, AssertionOptions options, AuthenticatorAssertionRawResponse deviceResponse) { - // TODO: Get options from user record somehow, then clear them - var options = AssertionOptions.FromJson(""); - - var userCredentials = await _webAuthnCredentialRepository.GetManyByUserIdAsync(user.Id); - var assertionId = CoreHelpers.Base64UrlEncode(assertionResponse.Id); - var credential = userCredentials.FirstOrDefault(c => c.DescriptorId == assertionId); + var userId = nonDiscoverableUserId ?? new Guid(deviceResponse.Response.UserHandle); + var userCredentials = await _webAuthnCredentialRepository.GetManyByUserIdAsync(userId); + var credentialId = CoreHelpers.Base64UrlEncode(deviceResponse.Id); + var credential = userCredentials.FirstOrDefault(c => c.DescriptorId == credentialId); if (credential == null) { return null; } - // TODO: Callback to ensure credential ID is unique. Do we care? I don't think so. - IsUserHandleOwnerOfCredentialIdAsync callback = (args, cancellationToken) => Task.FromResult(true); + IsUserHandleOwnerOfCredentialIdAsync callback = (args, cancellationToken) => Task.FromResult(credential.UserId == userId); var credentialPublicKey = CoreHelpers.Base64UrlDecode(credential.PublicKey); var assertionVerificationResult = await _fido2.MakeAssertionAsync( - assertionResponse, options, credentialPublicKey, (uint)credential.Counter, callback); + deviceResponse, options, credentialPublicKey, (uint)credential.Counter, callback); // Update SignatureCounter credential.Counter = (int)assertionVerificationResult.Counter; @@ -629,6 +626,7 @@ public async Task CompleteWebAuthLoginAssertionAsync(AuthenticatorAssert if (assertionVerificationResult.Status == "ok") { + var user = await GetUserByIdAsync(userId); var token = _webAuthnLoginTokenizer.Protect(new WebAuthnLoginTokenable(user)); return token; } diff --git a/src/Identity/Controllers/AccountsController.cs b/src/Identity/Controllers/AccountsController.cs index d02ed609a01f..b59daaf47481 100644 --- a/src/Identity/Controllers/AccountsController.cs +++ b/src/Identity/Controllers/AccountsController.cs @@ -81,7 +81,7 @@ public async Task PostPrelogin([FromBody] PreloginRequest [HttpPost("webauthn-assertion-options")] [ApiExplorerSettings(IgnoreApi = true)] // Disable Swagger due to CredentialCreateOptions not converting properly // TODO: Create proper models for this call - public async Task PostWebAuthnAssertionOptions([FromBody] WebauthnCredentialAssertionRequestModel model) + public async Task PostWebAuthnAssertionOptions([FromBody] WebauthnCredentialAssertionOptionsRequestModel model) { //var user = await _userRepository.GetByEmailAsync(model.Email); //if (user == null) @@ -103,16 +103,23 @@ public async Task PostWebAuthnA [HttpPost("webauthn-assertion")] // TODO: Create proper models for this call - public async Task PostWebAuthnAssertion([FromBody] PreloginRequestModel model) + public async Task PostWebAuthnAssertion([FromBody] WebauthnCredentialAssertionRequestModel model) { - var user = await _userRepository.GetByEmailAsync(model.Email); - if (user == null) + + //var user = await _userRepository.GetByEmailAsync(model.Email); + //if (user == null) + //{ + // // TODO: proper response here? + // throw new BadRequestException(); + //} + var optionsToken = _assertionOptionsDataProtector.Unprotect(model.Token); + var loginToken = await _userService.CompleteWebAuthLoginAssertionAsync(optionsToken.UserId, optionsToken.Options, model.DeviceResponse); + + if (loginToken == null) { - // TODO: proper response here? - throw new BadRequestException(); + throw new UnauthorizedAccessException(); } - var token = await _userService.CompleteWebAuthLoginAssertionAsync(null, user); - return token; + return new WebAuthnCredentialAssertionResponseModel { Token = loginToken }; } } From 9b1ac4a3a2721d6bd8e007f5f2acb558e0ab6ebd Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Mon, 22 May 2023 16:48:35 +0200 Subject: [PATCH 47/50] [PM-2023] feat: return prf keys to allow unlocking --- .../Tokenables/WebAuthnLoginTokenable.cs | 5 +++- .../Services/Implementations/UserService.cs | 2 +- .../IdentityServer/ExtensionGrantValidator.cs | 30 ++++++++++++++----- 3 files changed, 28 insertions(+), 9 deletions(-) diff --git a/src/Core/Auth/Models/Business/Tokenables/WebAuthnLoginTokenable.cs b/src/Core/Auth/Models/Business/Tokenables/WebAuthnLoginTokenable.cs index b27b1fb35538..f4bc71679b72 100644 --- a/src/Core/Auth/Models/Business/Tokenables/WebAuthnLoginTokenable.cs +++ b/src/Core/Auth/Models/Business/Tokenables/WebAuthnLoginTokenable.cs @@ -1,6 +1,7 @@ using System.Text.Json.Serialization; using Bit.Core.Entities; using Bit.Core.Tokens; +using Bit.Core.Auth.Entities; namespace Bit.Core.Auth.Models.Business.Tokenables; @@ -14,6 +15,7 @@ public class WebAuthnLoginTokenable : ExpiringTokenable public string Identifier { get; set; } = TokenIdentifier; public Guid Id { get; set; } public string Email { get; set; } + public WebAuthnCredential Credential { get; set; } [JsonConstructor] public WebAuthnLoginTokenable() @@ -21,10 +23,11 @@ public WebAuthnLoginTokenable() ExpirationDate = DateTime.UtcNow.AddHours(_tokenLifetimeInHours); } - public WebAuthnLoginTokenable(User user) : this() + public WebAuthnLoginTokenable(User user, WebAuthnCredential credential) : this() { Id = user?.Id ?? default; Email = user?.Email; + Credential = credential; } public bool TokenIsValid(User user) diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index 5194b98ee020..2a0021f879be 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -627,7 +627,7 @@ public async Task CompleteWebAuthLoginAssertionAsync(Guid? nonDiscoverab if (assertionVerificationResult.Status == "ok") { var user = await GetUserByIdAsync(userId); - var token = _webAuthnLoginTokenizer.Protect(new WebAuthnLoginTokenable(user)); + var token = _webAuthnLoginTokenizer.Protect(new WebAuthnLoginTokenable(user, credential)); return token; } else diff --git a/src/Identity/IdentityServer/ExtensionGrantValidator.cs b/src/Identity/IdentityServer/ExtensionGrantValidator.cs index b7b2aa819226..dd03517444ad 100644 --- a/src/Identity/IdentityServer/ExtensionGrantValidator.cs +++ b/src/Identity/IdentityServer/ExtensionGrantValidator.cs @@ -1,6 +1,7 @@ using System.Security.Claims; using Bit.Core.Auth.Identity; using Bit.Core.Auth.Models.Business.Tokenables; +using Bit.Core.Auth.Repositories; using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Repositories; @@ -17,6 +18,7 @@ public class ExtensionGrantValidator : BaseRequestValidator _userManager; private readonly IDataProtectorTokenFactory _webAuthnLoginTokenizer; + private readonly IWebAuthnCredentialRepository _webAuthnCredentialRepository; public ExtensionGrantValidator( UserManager userManager, @@ -35,6 +37,7 @@ public ExtensionGrantValidator( IPolicyRepository policyRepository, IUserRepository userRepository, IPolicyService policyService, + IWebAuthnCredentialRepository webAuthnCredentialRepository, IDataProtectorTokenFactory tokenDataFactory, IDataProtectorTokenFactory webAuthnLoginTokenizer) : base(userManager, deviceRepository, deviceService, userService, eventService, @@ -44,22 +47,27 @@ public ExtensionGrantValidator( { _userManager = userManager; _webAuthnLoginTokenizer = webAuthnLoginTokenizer; + _webAuthnCredentialRepository = webAuthnCredentialRepository; } public string GrantType => "extension"; public async Task ValidateAsync(ExtensionGrantValidationContext context) { - var email = context.Request.Raw.Get("email"); + //var email = context.Request.Raw.Get("email"); var token = context.Request.Raw.Get("token"); - var type = context.Request.Raw.Get("type"); - if (string.IsNullOrWhiteSpace(email) || string.IsNullOrWhiteSpace(token) || string.IsNullOrWhiteSpace(type)) + //var type = context.Request.Raw.Get("type"); + //if (string.IsNullOrWhiteSpace(email) || string.IsNullOrWhiteSpace(token) || string.IsNullOrWhiteSpace(type)) + if (string.IsNullOrWhiteSpace(token)) { context.Result = new GrantValidationResult(TokenRequestErrors.InvalidGrant); return; } - var user = await _userManager.FindByEmailAsync(email.ToLowerInvariant()); + var verified = _webAuthnLoginTokenizer.TryUnprotect(token, out var tokenData); + + //var user = await _userManager.FindByEmailAsync(email.ToLowerInvariant()); + var user = await _userManager.FindByIdAsync(tokenData.Id.ToString()); var validatorContext = new CustomValidatorRequestContext { User = user, @@ -73,8 +81,7 @@ protected override async Task ValidateContextAsync(ExtensionGrantValidatio CustomValidatorRequestContext validatorContext) { var token = context.Request.Raw.Get("token"); - var type = context.Request.Raw.Get("type"); - if (validatorContext.User == null || string.IsNullOrWhiteSpace(token) || string.IsNullOrWhiteSpace(type)) + if (validatorContext.User == null || string.IsNullOrWhiteSpace(token)) { return false; } @@ -86,10 +93,19 @@ protected override async Task ValidateContextAsync(ExtensionGrantValidatio protected override Task SetSuccessResult(ExtensionGrantValidationContext context, User user, List claims, Dictionary customResponse) { + var token = context.Request.Raw.Get("token"); + var tokenData = _webAuthnLoginTokenizer.Unprotect(token); + + var extendedCustomResponse = new Dictionary(customResponse); + + extendedCustomResponse["PrfPublicKey"] = tokenData.Credential.PrfPublicKey; + extendedCustomResponse["PrfPrivateKey"] = tokenData.Credential.PrfPrivateKey; + extendedCustomResponse["UserKey"] = tokenData.Credential.UserKey; + context.Result = new GrantValidationResult(user.Id.ToString(), "Application", identityProvider: "bitwarden", claims: claims.Count > 0 ? claims : null, - customResponse: customResponse); + customResponse: extendedCustomResponse); return Task.CompletedTask; } From 5d11d066ec0f93a290f5f5fbf56762bef1cf5d0d Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Tue, 23 May 2023 15:02:39 +0200 Subject: [PATCH 48/50] [PM-2023] feat: webauthn login with email --- .../Services/Implementations/UserService.cs | 32 +++++++------------ .../Controllers/AccountsController.cs | 22 ++++++++----- 2 files changed, 25 insertions(+), 29 deletions(-) diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index 2a0021f879be..e109732ae51f 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -527,7 +527,7 @@ public async Task StartWebAuthnLoginRegistrationAsync(U var authenticatorSelection = new AuthenticatorSelection { AuthenticatorAttachment = null, - RequireResidentKey = true, // TODO: This is using the old residentKey selection variant, we need to update our lib so that we can set this to preferred + RequireResidentKey = false, // TODO: This is using the old residentKey selection variant, we need to update our lib so that we can set this to preferred UserVerification = UserVerificationRequirement.Preferred }; @@ -575,32 +575,22 @@ public async Task CompleteWebAuthLoginRegistrationAsync(User user, string public async Task StartWebAuthnLoginAssertionAsync(User user) { + List existingCredentials; + if (user != null) { - throw new NotImplementedException(); + var existingKeys = await _webAuthnCredentialRepository.GetManyByUserIdAsync(user.Id); + existingCredentials = existingKeys + .Select(k => new PublicKeyCredentialDescriptor(CoreHelpers.Base64UrlDecode(k.DescriptorId))) + .ToList(); + } + else + { + existingCredentials = new List(); } - - //var extensions = new AuthenticationExtensionsClientInputs { }; - - //var options = _fido2.RequestNewCredential(fidoUser, excludeCredentials, authenticatorSelection, - // AttestationConveyancePreference.None, extensions); - //var provider = user.GetTwoFactorProvider(TwoFactorProviderType.WebAuthn); - //var existingKeys = await _webAuthnCredentialRepository.GetManyByUserIdAsync(user.Id); - //var existingCredentials = existingKeys - // .Select(k => new PublicKeyCredentialDescriptor(CoreHelpers.Base64UrlDecode(k.DescriptorId))) - // .ToList(); - var existingCredentials = new List(); - - //if (existingCredentials.Count == 0) - //{ - // return null; - //} var exts = new AuthenticationExtensionsClientInputs(); var options = _fido2.GetAssertionOptions(existingCredentials, UserVerificationRequirement.Preferred, exts); - - // TODO: temp save options to user record somehow - return options; } diff --git a/src/Identity/Controllers/AccountsController.cs b/src/Identity/Controllers/AccountsController.cs index b59daaf47481..7a6c240aed4c 100644 --- a/src/Identity/Controllers/AccountsController.cs +++ b/src/Identity/Controllers/AccountsController.cs @@ -3,6 +3,7 @@ using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Auth.Services; using Bit.Core.Auth.Utilities; +using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.Models.Data; @@ -83,15 +84,20 @@ public async Task PostPrelogin([FromBody] PreloginRequest // TODO: Create proper models for this call public async Task PostWebAuthnAssertionOptions([FromBody] WebauthnCredentialAssertionOptionsRequestModel model) { - //var user = await _userRepository.GetByEmailAsync(model.Email); - //if (user == null) - //{ - // // TODO: return something? possible enumeration attacks with this response - // return new AssertionOptions(); - //} - var options = await _userService.StartWebAuthnLoginAssertionAsync(null); + User user = null; + if (model.Email != null) + { + user = await _userRepository.GetByEmailAsync(model.Email); + if (user == null) + { + // TODO: return something? possible enumeration attacks with this response + throw new UnauthorizedAccessException(); + } + } + + var options = await _userService.StartWebAuthnLoginAssertionAsync(user); - var tokenable = new WebAuthnCredentialAssertionOptionsTokenable(null, options); + var tokenable = new WebAuthnCredentialAssertionOptionsTokenable(user, options); var token = _assertionOptionsDataProtector.Protect(tokenable); return new WebAuthnCredentialAssertionOptionsResponseModel From cd5635f967889d5f104d47e978e180bcf33e24f3 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Fri, 26 May 2023 15:01:29 +0200 Subject: [PATCH 49/50] [PM-2014] Passkey registration (#2915) * [PM-2014] chore: rename `IWebAuthnRespository` to `IWebAuthnCredentialRepository` * [PM-2014] fix: add missing service registration * [PM-2014] feat: add user verification when fetching options * [PM-2014] feat: create migration script for mssql * [PM-2014] chore: append to todo comment * [PM-2014] feat: add support for creation token * [PM-2014] feat: implement credential saving * [PM-2014] chore: add resident key TODO comment * [PM-2014] feat: implement passkey listing * [PM-2014] feat: implement deletion without user verification * [PM-2014] feat: add user verification to delete * [PM-2014] feat: implement passkey limit * [PM-2014] chore: clean up todo comments * [PM-2014] fix: add missing sql scripts Missed staging them when commiting * [PM-2014] feat: include options response model in swagger docs * [PM-2014] chore: move properties after ctor * [PM-2014] feat: use `Guid` directly as input paramter * [PM-2014] feat: use nullable guid in token * [PM-2014] chore: add new-line * [PM-2014] feat: add support for feature flag * [PM-2014] feat: start adding controller tests * [PM-2014] feat: add user verification test * [PM-2014] feat: add controller tests for token interaction * [PM-2014] feat: add tokenable tests * [PM-2014] chore: clean up commented premium check * [PM-2014] feat: add user service test for credential limit * [PM-2014] fix: run `dotnet format` * [PM-2014] chore: remove trailing comma * [PM-2014] chore: add `Async` suffix * [PM-2014] chore: move delay to constant * [PM-2014] chore: change `default` to `null` * [PM-2014] chore: remove autogenerated weirdness * [PM-2014] fix: lint --- .../Auth/Controllers/WebAuthnController.cs | 94 ++++++---- .../WebAuthnCredentialRequestModel.cs | 17 ++ ...thnCredentialCreateOptionsResponseModel.cs | 16 ++ .../WebAuthnCredentialResponseModel.cs | 20 +++ ...ebAuthnCredentialCreateOptionsTokenable.cs | 44 +++++ ...ry.cs => IWebAuthnCredentialRepository.cs} | 3 +- src/Core/Constants.cs | 2 + src/Core/Services/IUserService.cs | 2 +- .../Services/Implementations/UserService.cs | 35 ++-- .../Controllers/AccountsController.cs | 5 +- .../IdentityServer/ExtensionGrantValidator.cs | 10 +- .../WebAuthnCredentialRepository.cs | 15 +- .../DapperServiceCollectionExtensions.cs | 1 + .../WebAuthnCredentialRepository.cs | 13 +- .../Utilities/ServiceCollectionExtensions.cs | 6 + .../WebAuthnCredential_ReadByIdUserId.sql | 16 ++ test/Api.Test/Api.Test.csproj | 4 + .../Controllers/WebAuthnControllerTests.cs | 143 +++++++++++++++ ...hnCredentialCreateOptionsTokenableTests.cs | 81 +++++++++ test/Core.Test/Services/UserServiceTests.cs | 19 ++ ...2023-05-08-00_WebAuthnLoginCredentials.sql | 170 ++++++++++++++++++ 21 files changed, 653 insertions(+), 63 deletions(-) create mode 100644 src/Api/Auth/Models/Request/WebAuthn/WebAuthnCredentialRequestModel.cs create mode 100644 src/Api/Auth/Models/Response/WebAuthn/WebAuthnCredentialCreateOptionsResponseModel.cs create mode 100644 src/Api/Auth/Models/Response/WebAuthn/WebAuthnCredentialResponseModel.cs create mode 100644 src/Core/Auth/Models/Business/Tokenables/WebAuthnCredentialCreateOptionsTokenable.cs rename src/Core/Auth/Repositories/{IWebAuthnRepository.cs => IWebAuthnCredentialRepository.cs} (54%) create mode 100644 src/Sql/dbo/Stored Procedures/WebAuthnCredential_ReadByIdUserId.sql create mode 100644 test/Api.Test/Auth/Controllers/WebAuthnControllerTests.cs create mode 100644 test/Core.Test/Auth/Models/Business/Tokenables/WebAuthnCredentialCreateOptionsTokenableTests.cs create mode 100644 util/Migrator/DbScripts/2023-05-08-00_WebAuthnLoginCredentials.sql diff --git a/src/Api/Auth/Controllers/WebAuthnController.cs b/src/Api/Auth/Controllers/WebAuthnController.cs index 860d5d218ce2..a1f3071fad34 100644 --- a/src/Api/Auth/Controllers/WebAuthnController.cs +++ b/src/Api/Auth/Controllers/WebAuthnController.cs @@ -1,9 +1,13 @@ -using Bit.Api.Auth.Models.Request; -using Bit.Api.Auth.Models.Response.TwoFactor; +using Bit.Api.Auth.Models.Request.Accounts; +using Bit.Api.Auth.Models.Request.Webauthn; +using Bit.Api.Auth.Models.Response.WebAuthn; using Bit.Api.Models.Response; +using Bit.Core; +using Bit.Core.Auth.Models.Business.Tokenables; +using Bit.Core.Auth.Repositories; using Bit.Core.Exceptions; using Bit.Core.Services; -using Fido2NetLib; +using Bit.Core.Tokens; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; @@ -14,67 +18,93 @@ namespace Bit.Api.Auth.Controllers; public class WebAuthnController : Controller { private readonly IUserService _userService; + private readonly IWebAuthnCredentialRepository _credentialRepository; + private readonly IDataProtectorTokenFactory _createOptionsDataProtector; public WebAuthnController( - IUserService userService) + IUserService userService, + IWebAuthnCredentialRepository credentialRepository, + IDataProtectorTokenFactory createOptionsDataProtector) { _userService = userService; + _credentialRepository = credentialRepository; + _createOptionsDataProtector = createOptionsDataProtector; } [HttpGet("")] - // TODO: Create proper models for this call - public async Task> Get() + public async Task> Get() { - var user = await _userService.GetUserByPrincipalAsync(User); - if (user == null) - { - throw new UnauthorizedAccessException(); - } - return new ListResponseModel(new List { }); + var user = await GetUserAsync(); + var credentials = await _credentialRepository.GetManyByUserIdAsync(user.Id); + + return new ListResponseModel(credentials.Select(c => new WebAuthnCredentialResponseModel(c))); } [HttpPost("options")] - [ApiExplorerSettings(IgnoreApi = true)] // Disable Swagger due to CredentialCreateOptions not converting properly - public async Task PostOptions() + public async Task PostOptions([FromBody] SecretVerificationRequestModel model) { - var user = await _userService.GetUserByPrincipalAsync(User); - if (user == null) - { - throw new UnauthorizedAccessException(); - } + var user = await VerifyUserAsync(model); + var options = await _userService.StartWebAuthnLoginRegistrationAsync(user); - var reg = await _userService.StartWebAuthnLoginRegistrationAsync(user); - return reg; + var tokenable = new WebAuthnCredentialCreateOptionsTokenable(user, options); + var token = _createOptionsDataProtector.Protect(tokenable); + + return new WebAuthnCredentialCreateOptionsResponseModel + { + Options = options, + Token = token + }; } [HttpPost("")] - // TODO: Create proper models for this call - public async Task Post([FromBody] TwoFactorWebAuthnRequestModel model) + public async Task Post([FromBody] WebAuthnCredentialRequestModel model) { - var user = await _userService.GetUserByPrincipalAsync(User); - if (user == null) + var user = await GetUserAsync(); + var tokenable = _createOptionsDataProtector.Unprotect(model.Token); + if (!tokenable.TokenIsValid(user)) { - throw new UnauthorizedAccessException(); + throw new BadRequestException("The token associated with your request is expired. A valid token is required to continue."); } - var success = await _userService.CompleteWebAuthLoginRegistrationAsync(user, model.Name, model.DeviceResponse); + var success = await _userService.CompleteWebAuthLoginRegistrationAsync(user, model.Name, tokenable.Options, model.DeviceResponse); if (!success) { throw new BadRequestException("Unable to complete WebAuthn registration."); } - var response = new TwoFactorWebAuthnResponseModel(user); - return response; } - [HttpDelete("{id}")] [HttpPost("{id}/delete")] - public async Task Delete(string id) + public async Task Delete(Guid id, [FromBody] SecretVerificationRequestModel model) + { + var user = await VerifyUserAsync(model); + var credential = await _credentialRepository.GetByIdAsync(id, user.Id); + if (credential == null) + { + throw new NotFoundException("Credential not found."); + } + + await _credentialRepository.DeleteAsync(credential); + } + + private async Task GetUserAsync() { var user = await _userService.GetUserByPrincipalAsync(User); if (user == null) { throw new UnauthorizedAccessException(); } - // TODO: Delete + return user; + } + + private async Task VerifyUserAsync(SecretVerificationRequestModel model) + { + var user = await GetUserAsync(); + if (!await _userService.VerifySecretAsync(user, model.Secret)) + { + await Task.Delay(Constants.FailedSecretVerificationDelay); + throw new BadRequestException(string.Empty, "User verification failed."); + } + + return user; } } diff --git a/src/Api/Auth/Models/Request/WebAuthn/WebAuthnCredentialRequestModel.cs b/src/Api/Auth/Models/Request/WebAuthn/WebAuthnCredentialRequestModel.cs new file mode 100644 index 000000000000..8f16fe7f5065 --- /dev/null +++ b/src/Api/Auth/Models/Request/WebAuthn/WebAuthnCredentialRequestModel.cs @@ -0,0 +1,17 @@ +using System.ComponentModel.DataAnnotations; +using Fido2NetLib; + +namespace Bit.Api.Auth.Models.Request.Webauthn; + +public class WebAuthnCredentialRequestModel +{ + [Required] + public AuthenticatorAttestationRawResponse DeviceResponse { get; set; } + + [Required] + public string Name { get; set; } + + [Required] + public string Token { get; set; } +} + diff --git a/src/Api/Auth/Models/Response/WebAuthn/WebAuthnCredentialCreateOptionsResponseModel.cs b/src/Api/Auth/Models/Response/WebAuthn/WebAuthnCredentialCreateOptionsResponseModel.cs new file mode 100644 index 000000000000..d521bdac960b --- /dev/null +++ b/src/Api/Auth/Models/Response/WebAuthn/WebAuthnCredentialCreateOptionsResponseModel.cs @@ -0,0 +1,16 @@ +using Bit.Core.Models.Api; +using Fido2NetLib; + +namespace Bit.Api.Auth.Models.Response.WebAuthn; + +public class WebAuthnCredentialCreateOptionsResponseModel : ResponseModel +{ + private const string ResponseObj = "webauthnCredentialCreateOptions"; + + public WebAuthnCredentialCreateOptionsResponseModel() : base(ResponseObj) + { + } + + public CredentialCreateOptions Options { get; set; } + public string Token { get; set; } +} diff --git a/src/Api/Auth/Models/Response/WebAuthn/WebAuthnCredentialResponseModel.cs b/src/Api/Auth/Models/Response/WebAuthn/WebAuthnCredentialResponseModel.cs new file mode 100644 index 000000000000..0e358c751d32 --- /dev/null +++ b/src/Api/Auth/Models/Response/WebAuthn/WebAuthnCredentialResponseModel.cs @@ -0,0 +1,20 @@ +using Bit.Core.Auth.Entities; +using Bit.Core.Models.Api; + +namespace Bit.Api.Auth.Models.Response.WebAuthn; + +public class WebAuthnCredentialResponseModel : ResponseModel +{ + private const string ResponseObj = "webauthnCredential"; + + public WebAuthnCredentialResponseModel(WebAuthnCredential credential) : base(ResponseObj) + { + Id = credential.Id.ToString(); + Name = credential.Name; + PrfSupport = false; + } + + public string Id { get; set; } + public string Name { get; set; } + public bool PrfSupport { get; set; } +} diff --git a/src/Core/Auth/Models/Business/Tokenables/WebAuthnCredentialCreateOptionsTokenable.cs b/src/Core/Auth/Models/Business/Tokenables/WebAuthnCredentialCreateOptionsTokenable.cs new file mode 100644 index 000000000000..e64edace4558 --- /dev/null +++ b/src/Core/Auth/Models/Business/Tokenables/WebAuthnCredentialCreateOptionsTokenable.cs @@ -0,0 +1,44 @@ +using System.Text.Json.Serialization; +using Bit.Core.Entities; +using Bit.Core.Tokens; +using Fido2NetLib; + +namespace Bit.Core.Auth.Models.Business.Tokenables; + +public class WebAuthnCredentialCreateOptionsTokenable : ExpiringTokenable +{ + // 7 minutes = max webauthn timeout (6 minutes) + slack for miscellaneous delays + private const double _tokenLifetimeInHours = (double)7 / 60; + public const string ClearTextPrefix = "BWWebAuthnCredentialCreateOptions_"; + public const string DataProtectorPurpose = "WebAuthnCredentialCreateDataProtector"; + public const string TokenIdentifier = "WebAuthnCredentialCreateOptionsToken"; + + public string Identifier { get; set; } = TokenIdentifier; + public Guid? UserId { get; set; } + public CredentialCreateOptions Options { get; set; } + + [JsonConstructor] + public WebAuthnCredentialCreateOptionsTokenable() + { + ExpirationDate = DateTime.UtcNow.AddHours(_tokenLifetimeInHours); + } + + public WebAuthnCredentialCreateOptionsTokenable(User user, CredentialCreateOptions options) : this() + { + UserId = user?.Id; + Options = options; + } + + public bool TokenIsValid(User user) + { + if (!Valid || user == null) + { + return false; + } + + return UserId == user.Id; + } + + protected override bool TokenIsValid() => Identifier == TokenIdentifier && UserId != null && Options != null; +} + diff --git a/src/Core/Auth/Repositories/IWebAuthnRepository.cs b/src/Core/Auth/Repositories/IWebAuthnCredentialRepository.cs similarity index 54% rename from src/Core/Auth/Repositories/IWebAuthnRepository.cs rename to src/Core/Auth/Repositories/IWebAuthnCredentialRepository.cs index 751c49429af4..7a052df6883e 100644 --- a/src/Core/Auth/Repositories/IWebAuthnRepository.cs +++ b/src/Core/Auth/Repositories/IWebAuthnCredentialRepository.cs @@ -3,7 +3,8 @@ namespace Bit.Core.Auth.Repositories; -public interface IWebAuthnRepository : IRepository +public interface IWebAuthnCredentialRepository : IRepository { + Task GetByIdAsync(Guid id, Guid userId); Task> GetManyByUserIdAsync(Guid userId); } diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index 8ff378d00cb7..8c3464adccc0 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -5,6 +5,7 @@ namespace Bit.Core; public static class Constants { public const int BypassFiltersEventId = 12482444; + public const int FailedSecretVerificationDelay = 2000; // File size limits - give 1 MB extra for cushion. // Note: if request size limits are changed, 'client_max_body_size' @@ -35,6 +36,7 @@ public static class FeatureFlagKeys { public const string DisplayEuEnvironment = "display-eu-environment"; public const string DisplayLowKdfIterationWarning = "display-kdf-iteration-warning"; + public const string PasswordlessLogin = "passwordless-login"; public const string TrustedDeviceEncryption = "trusted-device-encryption"; public static List GetAllKeys() diff --git a/src/Core/Services/IUserService.cs b/src/Core/Services/IUserService.cs index 49a8c9a7b8c5..e27668946638 100644 --- a/src/Core/Services/IUserService.cs +++ b/src/Core/Services/IUserService.cs @@ -28,7 +28,7 @@ public interface IUserService Task DeleteWebAuthnKeyAsync(User user, int id); Task CompleteWebAuthRegistrationAsync(User user, int value, string name, AuthenticatorAttestationRawResponse attestationResponse); Task StartWebAuthnLoginRegistrationAsync(User user); - Task CompleteWebAuthLoginRegistrationAsync(User user, string name, AuthenticatorAttestationRawResponse attestationResponse); + Task CompleteWebAuthLoginRegistrationAsync(User user, string name, CredentialCreateOptions options, AuthenticatorAttestationRawResponse attestationResponse); Task StartWebAuthnLoginAssertionAsync(User user); Task CompleteWebAuthLoginAssertionAsync(AuthenticatorAssertionRawResponse assertionResponse, User user); Task SendEmailVerificationAsync(User user); diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index 949b4b137e18..346b2a605203 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -26,8 +26,6 @@ using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; -using Microsoft.Identity.Client.Platforms.Features.DesktopOs.Kerberos; -using static IdentityServer4.Models.IdentityResources; using File = System.IO.File; namespace Bit.Core.Services; @@ -61,7 +59,7 @@ public class UserService : UserManager, IUserService, IDisposable private readonly IOrganizationService _organizationService; private readonly IProviderUserRepository _providerUserRepository; private readonly IStripeSyncService _stripeSyncService; - private readonly IWebAuthnRepository _webAuthnRepository; + private readonly IWebAuthnCredentialRepository _webAuthnCredentialRepository; private readonly IDataProtectorTokenFactory _webAuthnLoginTokenizer; public UserService( @@ -94,7 +92,7 @@ public UserService( IOrganizationService organizationService, IProviderUserRepository providerUserRepository, IStripeSyncService stripeSyncService, - IWebAuthnRepository webAuthnRepository, + IWebAuthnCredentialRepository webAuthnRepository, IDataProtectorTokenFactory webAuthnLoginTokenizer) : base( store, @@ -132,7 +130,7 @@ public UserService( _organizationService = organizationService; _providerUserRepository = providerUserRepository; _stripeSyncService = stripeSyncService; - _webAuthnRepository = webAuthnRepository; + _webAuthnCredentialRepository = webAuthnRepository; _webAuthnLoginTokenizer = webAuthnLoginTokenizer; } @@ -524,7 +522,7 @@ public async Task StartWebAuthnLoginRegistrationAsync(U }; // Get existing keys to exclude - var existingKeys = await _webAuthnRepository.GetManyByUserIdAsync(user.Id); + var existingKeys = await _webAuthnCredentialRepository.GetManyByUserIdAsync(user.Id); var excludeCredentials = existingKeys .Select(k => new PublicKeyCredentialDescriptor(CoreHelpers.Base64UrlDecode(k.DescriptorId))) .ToList(); @@ -532,29 +530,30 @@ public async Task StartWebAuthnLoginRegistrationAsync(U var authenticatorSelection = new AuthenticatorSelection { AuthenticatorAttachment = null, - RequireResidentKey = false, + RequireResidentKey = false, // TODO: This is using the old residentKey selection variant, we need to update our lib so that we can set this to preferred UserVerification = UserVerificationRequirement.Preferred }; - // TODO: PRF var extensions = new AuthenticationExtensionsClientInputs { }; var options = _fido2.RequestNewCredential(fidoUser, excludeCredentials, authenticatorSelection, AttestationConveyancePreference.None, extensions); - // TODO: temp save options to user record somehow - return options; } public async Task CompleteWebAuthLoginRegistrationAsync(User user, string name, + CredentialCreateOptions options, AuthenticatorAttestationRawResponse attestationResponse) { - // TODO: Get options from user record somehow, then clear them - var options = CredentialCreateOptions.FromJson(""); + var existingCredentials = await _webAuthnCredentialRepository.GetManyByUserIdAsync(user.Id); + if (existingCredentials.Count >= 5) + { + return false; + } - // TODO: Callback to ensure credential ID is unique. Do we care? I don't think so. - IsCredentialIdUniqueToUserAsyncDelegate callback = (args, cancellationToken) => Task.FromResult(true); + var existingCredentialIds = existingCredentials.Select(c => c.DescriptorId); + IsCredentialIdUniqueToUserAsyncDelegate callback = (args, cancellationToken) => Task.FromResult(!existingCredentialIds.Contains(CoreHelpers.Base64UrlEncode(args.CredentialId))); var success = await _fido2.MakeNewCredentialAsync(attestationResponse, options, callback); @@ -569,14 +568,14 @@ public async Task CompleteWebAuthLoginRegistrationAsync(User user, string UserId = user.Id }; - await _webAuthnRepository.CreateAsync(credential); + await _webAuthnCredentialRepository.CreateAsync(credential); return true; } public async Task StartWebAuthnLoginAssertionAsync(User user) { var provider = user.GetTwoFactorProvider(TwoFactorProviderType.WebAuthn); - var existingKeys = await _webAuthnRepository.GetManyByUserIdAsync(user.Id); + var existingKeys = await _webAuthnCredentialRepository.GetManyByUserIdAsync(user.Id); var existingCredentials = existingKeys .Select(k => new PublicKeyCredentialDescriptor(CoreHelpers.Base64UrlDecode(k.DescriptorId))) .ToList(); @@ -603,7 +602,7 @@ public async Task CompleteWebAuthLoginAssertionAsync(AuthenticatorAssert // TODO: Get options from user record somehow, then clear them var options = AssertionOptions.FromJson(""); - var userCredentials = await _webAuthnRepository.GetManyByUserIdAsync(user.Id); + var userCredentials = await _webAuthnCredentialRepository.GetManyByUserIdAsync(user.Id); var assertionId = CoreHelpers.Base64UrlEncode(assertionResponse.Id); var credential = userCredentials.FirstOrDefault(c => c.DescriptorId == assertionId); if (credential == null) @@ -619,7 +618,7 @@ public async Task CompleteWebAuthLoginAssertionAsync(AuthenticatorAssert // Update SignatureCounter credential.Counter = (int)assertionVerificationResult.Counter; - await _webAuthnRepository.ReplaceAsync(credential); + await _webAuthnCredentialRepository.ReplaceAsync(credential); if (assertionVerificationResult.Status == "ok") { diff --git a/src/Identity/Controllers/AccountsController.cs b/src/Identity/Controllers/AccountsController.cs index 55184e3c0c2c..de9aa682fe78 100644 --- a/src/Identity/Controllers/AccountsController.cs +++ b/src/Identity/Controllers/AccountsController.cs @@ -1,6 +1,4 @@ -using Bit.Core.Auth.Enums; -using System.Text.Json; -using Bit.Core.Auth.Models.Api.Request.Accounts; +using Bit.Core.Auth.Models.Api.Request.Accounts; using Bit.Core.Auth.Models.Api.Response.Accounts; using Bit.Core.Auth.Services; using Bit.Core.Auth.Utilities; @@ -12,7 +10,6 @@ using Bit.SharedWeb.Utilities; using Fido2NetLib; using Microsoft.AspNetCore.Mvc; -using Bit.Identity.IdentityServer; namespace Bit.Identity.Controllers; diff --git a/src/Identity/IdentityServer/ExtensionGrantValidator.cs b/src/Identity/IdentityServer/ExtensionGrantValidator.cs index 2b13b0cfef05..b7b2aa819226 100644 --- a/src/Identity/IdentityServer/ExtensionGrantValidator.cs +++ b/src/Identity/IdentityServer/ExtensionGrantValidator.cs @@ -1,15 +1,15 @@ using System.Security.Claims; -using IdentityServer4.Models; -using IdentityServer4.Validation; -using Microsoft.AspNetCore.Identity; using Bit.Core.Auth.Identity; +using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Context; +using Bit.Core.Entities; using Bit.Core.Repositories; using Bit.Core.Services; -using Bit.Core.Entities; using Bit.Core.Settings; using Bit.Core.Tokens; -using Bit.Core.Auth.Models.Business.Tokenables; +using IdentityServer4.Models; +using IdentityServer4.Validation; +using Microsoft.AspNetCore.Identity; namespace Bit.Identity.IdentityServer; diff --git a/src/Infrastructure.Dapper/Auth/Repositories/WebAuthnCredentialRepository.cs b/src/Infrastructure.Dapper/Auth/Repositories/WebAuthnCredentialRepository.cs index 72e15119fb0b..502569136f2a 100644 --- a/src/Infrastructure.Dapper/Auth/Repositories/WebAuthnCredentialRepository.cs +++ b/src/Infrastructure.Dapper/Auth/Repositories/WebAuthnCredentialRepository.cs @@ -9,7 +9,7 @@ namespace Bit.Infrastructure.Dapper.Auth.Repositories; -public class WebAuthnCredentialRepository : Repository, IWebAuthnRepository +public class WebAuthnCredentialRepository : Repository, IWebAuthnCredentialRepository { public WebAuthnCredentialRepository(GlobalSettings globalSettings) : this(globalSettings.SqlServer.ConnectionString, globalSettings.SqlServer.ReadOnlyConnectionString) @@ -19,6 +19,19 @@ public WebAuthnCredentialRepository(string connectionString, string readOnlyConn : base(connectionString, readOnlyConnectionString) { } + public async Task GetByIdAsync(Guid id, Guid userId) + { + using (var connection = new SqlConnection(ConnectionString)) + { + var results = await connection.QueryAsync( + $"[{Schema}].[{Table}_ReadByIdUserId]", + new { Id = id, UserId = userId }, + commandType: CommandType.StoredProcedure); + + return results.FirstOrDefault(); + } + } + public async Task> GetManyByUserIdAsync(Guid userId) { using (var connection = new SqlConnection(ConnectionString)) diff --git a/src/Infrastructure.Dapper/DapperServiceCollectionExtensions.cs b/src/Infrastructure.Dapper/DapperServiceCollectionExtensions.cs index d7082184d36e..3a2233f4cb61 100644 --- a/src/Infrastructure.Dapper/DapperServiceCollectionExtensions.cs +++ b/src/Infrastructure.Dapper/DapperServiceCollectionExtensions.cs @@ -45,6 +45,7 @@ public static void AddDapperRepositories(this IServiceCollection services, bool services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); + services.AddSingleton(); if (selfHosted) { diff --git a/src/Infrastructure.EntityFramework/Auth/Repositories/WebAuthnCredentialRepository.cs b/src/Infrastructure.EntityFramework/Auth/Repositories/WebAuthnCredentialRepository.cs index 9d2cf81e7b4e..68f14243c41e 100644 --- a/src/Infrastructure.EntityFramework/Auth/Repositories/WebAuthnCredentialRepository.cs +++ b/src/Infrastructure.EntityFramework/Auth/Repositories/WebAuthnCredentialRepository.cs @@ -7,12 +7,23 @@ namespace Bit.Infrastructure.EntityFramework.Auth.Repositories; -public class WebAuthnCredentialRepository : Repository, IWebAuthnRepository +public class WebAuthnCredentialRepository : Repository, IWebAuthnCredentialRepository { public WebAuthnCredentialRepository(IServiceScopeFactory serviceScopeFactory, IMapper mapper) : base(serviceScopeFactory, mapper, (context) => context.WebAuthnCredentials) { } + public async Task GetByIdAsync(Guid id, Guid userId) + { + using (var scope = ServiceScopeFactory.CreateScope()) + { + var dbContext = GetDatabaseContext(scope); + var query = dbContext.WebAuthnCredentials.Where(d => d.Id == id && d.UserId == userId); + var cred = await query.FirstOrDefaultAsync(); + return Mapper.Map(cred); + } + } + public async Task> GetManyByUserIdAsync(Guid userId) { using (var scope = ServiceScopeFactory.CreateScope()) diff --git a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs index febd9edd17f4..595b24ef1396 100644 --- a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs +++ b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs @@ -165,6 +165,12 @@ public static void AddTokenizers(this IServiceCollection services) WebAuthnLoginTokenable.DataProtectorPurpose, serviceProvider.GetDataProtectionProvider(), serviceProvider.GetRequiredService>>())); + services.AddSingleton>(serviceProvider => + new DataProtectorTokenFactory( + WebAuthnCredentialCreateOptionsTokenable.ClearTextPrefix, + WebAuthnCredentialCreateOptionsTokenable.DataProtectorPurpose, + serviceProvider.GetDataProtectionProvider(), + serviceProvider.GetRequiredService>>())); services.AddSingleton>(serviceProvider => new DataProtectorTokenFactory( SsoEmail2faSessionTokenable.ClearTextPrefix, diff --git a/src/Sql/dbo/Stored Procedures/WebAuthnCredential_ReadByIdUserId.sql b/src/Sql/dbo/Stored Procedures/WebAuthnCredential_ReadByIdUserId.sql new file mode 100644 index 000000000000..8b0f1d19f99f --- /dev/null +++ b/src/Sql/dbo/Stored Procedures/WebAuthnCredential_ReadByIdUserId.sql @@ -0,0 +1,16 @@ +CREATE PROCEDURE [dbo].[WebAuthnCredential_ReadByIdUserId] + @Id UNIQUEIDENTIFIER, + @UserId UNIQUEIDENTIFIER +AS +BEGIN + SET NOCOUNT ON + + SELECT + * + FROM + [dbo].[WebAuthnCredentialView] + WHERE + [Id] = @Id + AND + [UserId] = @UserId +END diff --git a/test/Api.Test/Api.Test.csproj b/test/Api.Test/Api.Test.csproj index b5f6a311c081..d6b31ce9308a 100644 --- a/test/Api.Test/Api.Test.csproj +++ b/test/Api.Test/Api.Test.csproj @@ -26,4 +26,8 @@ + + + + diff --git a/test/Api.Test/Auth/Controllers/WebAuthnControllerTests.cs b/test/Api.Test/Auth/Controllers/WebAuthnControllerTests.cs new file mode 100644 index 000000000000..32f2d5d49177 --- /dev/null +++ b/test/Api.Test/Auth/Controllers/WebAuthnControllerTests.cs @@ -0,0 +1,143 @@ +using Bit.Api.Auth.Controllers; +using Bit.Api.Auth.Models.Request.Accounts; +using Bit.Api.Auth.Models.Request.Webauthn; +using Bit.Core.Auth.Models.Business.Tokenables; +using Bit.Core.Entities; +using Bit.Core.Exceptions; +using Bit.Core.Services; +using Bit.Core.Tokens; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using Fido2NetLib; +using NSubstitute; +using NSubstitute.ReturnsExtensions; +using Xunit; + +namespace Bit.Api.Test.Auth.Controllers; + +[ControllerCustomize(typeof(WebAuthnController))] +[SutProviderCustomize] +public class WebAuthnControllerTests +{ + [Theory, BitAutoData] + public async Task Get_UserNotFound_ThrowsUnauthorizedAccessException(SutProvider sutProvider) + { + // Arrange + sutProvider.GetDependency().GetUserByPrincipalAsync(default).ReturnsNullForAnyArgs(); + + // Act + var result = () => sutProvider.Sut.Get(); + + // Assert + await Assert.ThrowsAsync(result); + } + + [Theory, BitAutoData] + public async Task PostOptions_UserNotFound_ThrowsUnauthorizedAccessException(SecretVerificationRequestModel requestModel, SutProvider sutProvider) + { + // Arrange + sutProvider.GetDependency().GetUserByPrincipalAsync(default).ReturnsNullForAnyArgs(); + + // Act + var result = () => sutProvider.Sut.PostOptions(requestModel); + + // Assert + await Assert.ThrowsAsync(result); + } + + [Theory, BitAutoData] + public async Task PostOptions_UserVerificationFailed_ThrowsBadRequestException(SecretVerificationRequestModel requestModel, User user, SutProvider sutProvider) + { + // Arrange + sutProvider.GetDependency().GetUserByPrincipalAsync(default).ReturnsForAnyArgs(user); + sutProvider.GetDependency().VerifySecretAsync(user, default).Returns(false); + + // Act + var result = () => sutProvider.Sut.PostOptions(requestModel); + + // Assert + await Assert.ThrowsAsync(result); + } + + [Theory, BitAutoData] + public async Task Post_UserNotFound_ThrowsUnauthorizedAccessException(WebAuthnCredentialRequestModel requestModel, SutProvider sutProvider) + { + // Arrange + sutProvider.GetDependency().GetUserByPrincipalAsync(default).ReturnsNullForAnyArgs(); + + // Act + var result = () => sutProvider.Sut.Post(requestModel); + + // Assert + await Assert.ThrowsAsync(result); + } + + [Theory, BitAutoData] + public async Task Post_ExpiredToken_ThrowsBadRequestException(WebAuthnCredentialRequestModel requestModel, CredentialCreateOptions createOptions, User user, SutProvider sutProvider) + { + // Arrange + var token = new WebAuthnCredentialCreateOptionsTokenable(user, createOptions); + sutProvider.GetDependency() + .GetUserByPrincipalAsync(default) + .ReturnsForAnyArgs(user); + sutProvider.GetDependency>() + .Unprotect(requestModel.Token) + .Returns(token); + + // Act + var result = () => sutProvider.Sut.Post(requestModel); + + // Assert + await Assert.ThrowsAsync(result); + } + + [Theory, BitAutoData] + public async Task Post_ValidInput_Returns(WebAuthnCredentialRequestModel requestModel, CredentialCreateOptions createOptions, User user, SutProvider sutProvider) + { + // Arrange + var token = new WebAuthnCredentialCreateOptionsTokenable(user, createOptions); + sutProvider.GetDependency() + .GetUserByPrincipalAsync(default) + .ReturnsForAnyArgs(user); + sutProvider.GetDependency() + .CompleteWebAuthLoginRegistrationAsync(user, requestModel.Name, createOptions, Arg.Any()) + .Returns(true); + sutProvider.GetDependency>() + .Unprotect(requestModel.Token) + .Returns(token); + + // Act + await sutProvider.Sut.Post(requestModel); + + // Assert + // Nothing to assert since return is void + } + + [Theory, BitAutoData] + public async Task Delete_UserNotFound_ThrowsUnauthorizedAccessException(Guid credentialId, SecretVerificationRequestModel requestModel, SutProvider sutProvider) + { + // Arrange + sutProvider.GetDependency().GetUserByPrincipalAsync(default).ReturnsNullForAnyArgs(); + + // Act + var result = () => sutProvider.Sut.Delete(credentialId, requestModel); + + // Assert + await Assert.ThrowsAsync(result); + } + + [Theory, BitAutoData] + public async Task Delete_UserVerificationFailed_ThrowsBadRequestException(Guid credentialId, SecretVerificationRequestModel requestModel, User user, SutProvider sutProvider) + { + // Arrange + sutProvider.GetDependency().GetUserByPrincipalAsync(default).ReturnsForAnyArgs(user); + sutProvider.GetDependency().VerifySecretAsync(user, default).Returns(false); + + // Act + var result = () => sutProvider.Sut.Delete(credentialId, requestModel); + + // Assert + await Assert.ThrowsAsync(result); + } +} + diff --git a/test/Core.Test/Auth/Models/Business/Tokenables/WebAuthnCredentialCreateOptionsTokenableTests.cs b/test/Core.Test/Auth/Models/Business/Tokenables/WebAuthnCredentialCreateOptionsTokenableTests.cs new file mode 100644 index 000000000000..c16f5c910013 --- /dev/null +++ b/test/Core.Test/Auth/Models/Business/Tokenables/WebAuthnCredentialCreateOptionsTokenableTests.cs @@ -0,0 +1,81 @@ +using Bit.Core.Auth.Models.Business.Tokenables; +using Bit.Core.Entities; +using Bit.Test.Common.AutoFixture.Attributes; +using Fido2NetLib; +using Xunit; + +namespace Bit.Core.Test.Auth.Models.Business.Tokenables; + +public class WebAuthnCredentialCreateOptionsTokenableTests +{ + [Theory, BitAutoData] + public void Valid_TokenWithoutUser_ReturnsFalse(CredentialCreateOptions createOptions) + { + var token = new WebAuthnCredentialCreateOptionsTokenable(null, createOptions); + + var isValid = token.Valid; + + Assert.False(isValid); + } + + [Theory, BitAutoData] + public void Valid_TokenWithoutOptions_ReturnsFalse(User user) + { + var token = new WebAuthnCredentialCreateOptionsTokenable(user, null); + + var isValid = token.Valid; + + Assert.False(isValid); + } + + [Theory, BitAutoData] + public void Valid_NewlyCreatedToken_ReturnsTrue(User user, CredentialCreateOptions createOptions) + { + var token = new WebAuthnCredentialCreateOptionsTokenable(user, createOptions); + + var isValid = token.Valid; + + Assert.True(isValid); + } + + [Theory, BitAutoData] + public void ValidIsValid_TokenWithoutUser_ReturnsFalse(User user, CredentialCreateOptions createOptions) + { + var token = new WebAuthnCredentialCreateOptionsTokenable(null, createOptions); + + var isValid = token.TokenIsValid(user); + + Assert.False(isValid); + } + + [Theory, BitAutoData] + public void ValidIsValid_TokenWithoutOptions_ReturnsFalse(User user) + { + var token = new WebAuthnCredentialCreateOptionsTokenable(user, null); + + var isValid = token.TokenIsValid(user); + + Assert.False(isValid); + } + + [Theory, BitAutoData] + public void ValidIsValid_NonMatchingUsers_ReturnsFalse(User user1, User user2, CredentialCreateOptions createOptions) + { + var token = new WebAuthnCredentialCreateOptionsTokenable(user1, createOptions); + + var isValid = token.TokenIsValid(user2); + + Assert.False(isValid); + } + + [Theory, BitAutoData] + public void ValidIsValid_SameUser_ReturnsTrue(User user, CredentialCreateOptions createOptions) + { + var token = new WebAuthnCredentialCreateOptionsTokenable(user, createOptions); + + var isValid = token.TokenIsValid(user); + + Assert.True(isValid); + } +} + diff --git a/test/Core.Test/Services/UserServiceTests.cs b/test/Core.Test/Services/UserServiceTests.cs index 9cdda95de8e4..8aea2b26b082 100644 --- a/test/Core.Test/Services/UserServiceTests.cs +++ b/test/Core.Test/Services/UserServiceTests.cs @@ -1,6 +1,9 @@ using System.Text.Json; +using AutoFixture; +using Bit.Core.Auth.Entities; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Models; +using Bit.Core.Auth.Repositories; using Bit.Core.Entities; using Bit.Core.Models.Business; using Bit.Core.Models.Data.Organizations; @@ -9,6 +12,7 @@ using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; using Bit.Test.Common.Helpers; +using Fido2NetLib; using Microsoft.AspNetCore.Identity; using NSubstitute; using NSubstitute.ReceivedExtensions; @@ -170,4 +174,19 @@ public async void HasPremiumFromOrganization_Returns_True_If_Org_Eligible(SutPro Assert.True(await sutProvider.Sut.HasPremiumFromOrganization(user)); } + + [Theory, BitAutoData] + public async void CompleteWebAuthLoginRegistrationAsync_ExceedsExistingCredentialsLimit_ReturnsFalse(SutProvider sutProvider, User user, CredentialCreateOptions options, AuthenticatorAttestationRawResponse response, Generator credentialGenerator) + { + // Arrange + var existingCredentials = credentialGenerator.Take(5).ToList(); + sutProvider.GetDependency().GetManyByUserIdAsync(user.Id).Returns(existingCredentials); + + // Act + var result = await sutProvider.Sut.CompleteWebAuthLoginRegistrationAsync(user, "name", options, response); + + // Assert + Assert.False(result); + sutProvider.GetDependency().DidNotReceive(); + } } diff --git a/util/Migrator/DbScripts/2023-05-08-00_WebAuthnLoginCredentials.sql b/util/Migrator/DbScripts/2023-05-08-00_WebAuthnLoginCredentials.sql new file mode 100644 index 000000000000..ef3cefdec81a --- /dev/null +++ b/util/Migrator/DbScripts/2023-05-08-00_WebAuthnLoginCredentials.sql @@ -0,0 +1,170 @@ +CREATE TABLE [dbo].[WebAuthnCredential] ( + [Id] UNIQUEIDENTIFIER NOT NULL, + [UserId] UNIQUEIDENTIFIER NOT NULL, + [Name] NVARCHAR (50) NOT NULL, + [PublicKey] VARCHAR (256) NOT NULL, + [DescriptorId] VARCHAR (256) NOT NULL, + [Counter] INT NOT NULL, + [Type] VARCHAR (20) NULL, + [AaGuid] UNIQUEIDENTIFIER NOT NULL, + [UserKey] VARCHAR (MAX) NULL, + [CreationDate] DATETIME2 (7) NOT NULL, + [RevisionDate] DATETIME2 (7) NOT NULL, + CONSTRAINT [PK_WebAuthnCredential] PRIMARY KEY CLUSTERED ([Id] ASC), + CONSTRAINT [FK_WebAuthnCredential_User] FOREIGN KEY ([UserId]) REFERENCES [dbo].[User] ([Id]) +); + +GO +CREATE NONCLUSTERED INDEX [IX_WebAuthnCredential_UserId] + ON [dbo].[WebAuthnCredential]([UserId] ASC); + +GO +CREATE VIEW [dbo].[WebAuthnCredentialView] +AS +SELECT + * +FROM + [dbo].[WebAuthnCredential] + +GO +CREATE PROCEDURE [dbo].[WebAuthnCredential_Create] + @Id UNIQUEIDENTIFIER OUTPUT, + @UserId UNIQUEIDENTIFIER, + @Name NVARCHAR(50), + @PublicKey VARCHAR (256), + @DescriptorId VARCHAR(256), + @Counter INT, + @Type VARCHAR(20), + @AaGuid UNIQUEIDENTIFIER, + @UserKey VARCHAR (MAX), + @CreationDate DATETIME2(7), + @RevisionDate DATETIME2(7) +AS +BEGIN + SET NOCOUNT ON + + INSERT INTO [dbo].[WebAuthnCredential] + ( + [Id], + [UserId], + [Name], + [PublicKey], + [DescriptorId], + [Counter], + [Type], + [AaGuid], + [UserKey], + [CreationDate], + [RevisionDate] + ) + VALUES + ( + @Id, + @UserId, + @Name, + @PublicKey, + @DescriptorId, + @Counter, + @Type, + @AaGuid, + @UserKey, + @CreationDate, + @RevisionDate + ) +END + +GO +CREATE PROCEDURE [dbo].[WebAuthnCredential_DeleteById] + @Id UNIQUEIDENTIFIER +AS +BEGIN + SET NOCOUNT ON + + DELETE + FROM + [dbo].[WebAuthnCredential] + WHERE + [Id] = @Id +END + +GO +CREATE PROCEDURE [dbo].[WebAuthnCredential_ReadById] + @Id UNIQUEIDENTIFIER +AS +BEGIN + SET NOCOUNT ON + + SELECT + * + FROM + [dbo].[WebAuthnCredentialView] + WHERE + [Id] = @Id +END + +GO +CREATE PROCEDURE [dbo].[WebAuthnCredential_ReadByUserId] + @UserId UNIQUEIDENTIFIER +AS +BEGIN + SET NOCOUNT ON + + SELECT + * + FROM + [dbo].[WebAuthnCredentialView] + WHERE + [UserId] = @UserId +END + +GO +CREATE PROCEDURE [dbo].[WebAuthnCredential_Update] + @Id UNIQUEIDENTIFIER, + @UserId UNIQUEIDENTIFIER, + @Name NVARCHAR(50), + @PublicKey VARCHAR (256), + @DescriptorId VARCHAR(256), + @Counter INT, + @Type VARCHAR(20), + @AaGuid UNIQUEIDENTIFIER, + @UserKey VARCHAR (MAX), + @CreationDate DATETIME2(7), + @RevisionDate DATETIME2(7) +AS +BEGIN + SET NOCOUNT ON + + UPDATE + [dbo].[WebAuthnCredential] + SET + [UserId] = @UserId, + [Name] = @Name, + [PublicKey] = @PublicKey, + [DescriptorId] = @DescriptorId, + [Counter] = @Counter, + [Type] = @Type, + [AaGuid] = @AaGuid, + [UserKey] = @UserKey, + [CreationDate] = @CreationDate, + [RevisionDate] = @RevisionDate + WHERE + [Id] = @Id +END + +GO +CREATE PROCEDURE [dbo].[WebAuthnCredential_ReadByIdUserId] + @Id UNIQUEIDENTIFIER, + @UserId UNIQUEIDENTIFIER +AS +BEGIN + SET NOCOUNT ON + + SELECT + * + FROM + [dbo].[WebAuthnCredentialView] + WHERE + [Id] = @Id + AND + [UserId] = @UserId +END \ No newline at end of file From 7b358adf6bb1ace737d7b7891e1e54d96d522d91 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Thu, 24 Aug 2023 14:02:44 +0200 Subject: [PATCH 50/50] [PM-2023] fix: linting --- .../Auth/Models/Business/Tokenables/WebAuthnLoginTokenable.cs | 2 +- src/Identity/Controllers/AccountsController.cs | 1 - test/Core.Test/Services/UserServiceTests.cs | 1 - 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Core/Auth/Models/Business/Tokenables/WebAuthnLoginTokenable.cs b/src/Core/Auth/Models/Business/Tokenables/WebAuthnLoginTokenable.cs index f4bc71679b72..f4dcbb6c7847 100644 --- a/src/Core/Auth/Models/Business/Tokenables/WebAuthnLoginTokenable.cs +++ b/src/Core/Auth/Models/Business/Tokenables/WebAuthnLoginTokenable.cs @@ -1,7 +1,7 @@ using System.Text.Json.Serialization; +using Bit.Core.Auth.Entities; using Bit.Core.Entities; using Bit.Core.Tokens; -using Bit.Core.Auth.Entities; namespace Bit.Core.Auth.Models.Business.Tokenables; diff --git a/src/Identity/Controllers/AccountsController.cs b/src/Identity/Controllers/AccountsController.cs index 7a6c240aed4c..9245b95a6b8c 100644 --- a/src/Identity/Controllers/AccountsController.cs +++ b/src/Identity/Controllers/AccountsController.cs @@ -11,7 +11,6 @@ using Bit.Core.Services; using Bit.Core.Tokens; using Bit.SharedWeb.Utilities; -using Fido2NetLib; using Microsoft.AspNetCore.Mvc; namespace Bit.Identity.Controllers; diff --git a/test/Core.Test/Services/UserServiceTests.cs b/test/Core.Test/Services/UserServiceTests.cs index 693572d01315..0a01752c2b2b 100644 --- a/test/Core.Test/Services/UserServiceTests.cs +++ b/test/Core.Test/Services/UserServiceTests.cs @@ -3,7 +3,6 @@ using Bit.Core.Auth.Entities; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Models; -using Bit.Core.Auth.Repositories; using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Auth.Repositories; using Bit.Core.Context;