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 f2c6b03d512747ace0c8a8173455ab5840802ce3 Mon Sep 17 00:00:00 2001 From: Kyle Spearrin Date: Wed, 10 May 2023 11:48:18 -0400 Subject: [PATCH 07/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 08/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 09/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 cd5635f967889d5f104d47e978e180bcf33e24f3 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Fri, 26 May 2023 15:01:29 +0200 Subject: [PATCH 10/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 84f7fc8567b040347f3af55c82d02878d19b6826 Mon Sep 17 00:00:00 2001 From: Todd Martin <106564991+trmartin4@users.noreply.github.com> Date: Thu, 21 Sep 2023 09:01:21 -0400 Subject: [PATCH 11/50] Added check for PasswordlessLogin feature flag on new controller and methods. (#3284) * Added check for PasswordlessLogin feature flag on new controller and methods. * fix: build error from missing constructor argument --------- Co-authored-by: Andreas Coroiu --- src/Api/Auth/Controllers/WebAuthnController.cs | 2 ++ src/Identity/Controllers/AccountsController.cs | 6 +++++- src/Identity/IdentityServer/ExtensionGrantValidator.cs | 7 +++++-- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/Api/Auth/Controllers/WebAuthnController.cs b/src/Api/Auth/Controllers/WebAuthnController.cs index a1f3071fad34..b7e9c5bb8b11 100644 --- a/src/Api/Auth/Controllers/WebAuthnController.cs +++ b/src/Api/Auth/Controllers/WebAuthnController.cs @@ -8,6 +8,7 @@ using Bit.Core.Exceptions; using Bit.Core.Services; using Bit.Core.Tokens; +using Bit.Core.Utilities; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; @@ -15,6 +16,7 @@ namespace Bit.Api.Auth.Controllers; [Route("webauthn")] [Authorize("Web")] +[RequireFeature(FeatureFlagKeys.PasswordlessLogin)] public class WebAuthnController : Controller { private readonly IUserService _userService; diff --git a/src/Identity/Controllers/AccountsController.cs b/src/Identity/Controllers/AccountsController.cs index de9aa682fe78..9073884d8c05 100644 --- a/src/Identity/Controllers/AccountsController.cs +++ b/src/Identity/Controllers/AccountsController.cs @@ -1,4 +1,5 @@ -using Bit.Core.Auth.Models.Api.Request.Accounts; +using Bit.Core; +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; @@ -7,6 +8,7 @@ using Bit.Core.Models.Data; using Bit.Core.Repositories; using Bit.Core.Services; +using Bit.Core.Utilities; using Bit.SharedWeb.Utilities; using Fido2NetLib; using Microsoft.AspNetCore.Mvc; @@ -75,6 +77,7 @@ public async Task PostPrelogin([FromBody] PreloginRequest [HttpPost("webauthn-assertion-options")] [ApiExplorerSettings(IgnoreApi = true)] // Disable Swagger due to CredentialCreateOptions not converting properly + [RequireFeature(FeatureFlagKeys.PasswordlessLogin)] // TODO: Create proper models for this call public async Task PostWebAuthnAssertionOptions([FromBody] PreloginRequestModel model) { @@ -90,6 +93,7 @@ public async Task PostWebAuthnAssertionOptions([FromBody] Prel } [HttpPost("webauthn-assertion")] + [RequireFeature(FeatureFlagKeys.PasswordlessLogin)] // TODO: Create proper models for this call public async Task PostWebAuthnAssertion([FromBody] PreloginRequestModel model) { diff --git a/src/Identity/IdentityServer/ExtensionGrantValidator.cs b/src/Identity/IdentityServer/ExtensionGrantValidator.cs index f86b8c0597d7..8d9bbe119e0c 100644 --- a/src/Identity/IdentityServer/ExtensionGrantValidator.cs +++ b/src/Identity/IdentityServer/ExtensionGrantValidator.cs @@ -11,6 +11,7 @@ using IdentityServer4.Models; using IdentityServer4.Validation; using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.Caching.Distributed; namespace Bit.Identity.IdentityServer; @@ -38,11 +39,13 @@ public ExtensionGrantValidator( IPolicyService policyService, IDataProtectorTokenFactory tokenDataFactory, IDataProtectorTokenFactory webAuthnLoginTokenizer, - IFeatureService featureService) + IFeatureService featureService, + IDistributedCache distributedCache + ) : base(userManager, deviceRepository, deviceService, userService, eventService, organizationDuoWebTokenProvider, organizationRepository, organizationUserRepository, applicationCacheService, mailService, logger, currentContext, globalSettings, - userRepository, policyService, tokenDataFactory, featureService, ssoConfigRepository) + userRepository, policyService, tokenDataFactory, featureService, ssoConfigRepository, distributedCache) { _userManager = userManager; _webAuthnLoginTokenizer = webAuthnLoginTokenizer; From d11fc46b5a00a2a23b087a49fd985a10c7c7e38e Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Tue, 10 Oct 2023 13:19:44 +0200 Subject: [PATCH 12/50] [PM-4171] Update DB to support PRF (#3321) * [PM-4171] feat: update database to support PRF * [PM-4171] feat: rename `DescriptorId` to `CredentialId` * [PM-4171] feat: add PRF felds to domain object * [PM-4171] feat: add `SupportsPrf` column * [PM-4171] fix: add missing comma * [PM-4171] fix: add comma --- src/Core/Auth/Entities/WebAuthnCredential.cs | 7 ++- .../Services/Implementations/UserService.cs | 12 ++-- .../WebAuthnCredential_Create.sql | 21 +++++-- .../WebAuthnCredential_Update.sql | 16 +++-- src/Sql/dbo/Tables/WebAuthnCredential.sql | 25 ++++---- ...2023-05-08-00_WebAuthnLoginCredentials.sql | 62 ++++++++++++------- 6 files changed, 91 insertions(+), 52 deletions(-) diff --git a/src/Core/Auth/Entities/WebAuthnCredential.cs b/src/Core/Auth/Entities/WebAuthnCredential.cs index a1195b82ae74..b4b80ff65481 100644 --- a/src/Core/Auth/Entities/WebAuthnCredential.cs +++ b/src/Core/Auth/Entities/WebAuthnCredential.cs @@ -13,12 +13,15 @@ public class WebAuthnCredential : ITableObject [MaxLength(256)] public string PublicKey { get; set; } [MaxLength(256)] - public string DescriptorId { get; set; } + public string CredentialId { 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 string EncryptedUserKey { get; set; } + public string EncryptedPrivateKey { get; set; } + public string EncryptedPublicKey { get; set; } + public bool SupportsPrf { get; set; } public DateTime CreationDate { get; internal set; } = DateTime.UtcNow; public DateTime RevisionDate { get; internal set; } = DateTime.UtcNow; diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index 45049e430bdf..eee255422e07 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -524,13 +524,13 @@ public async Task StartWebAuthnLoginRegistrationAsync(U // Get existing keys to exclude var existingKeys = await _webAuthnCredentialRepository.GetManyByUserIdAsync(user.Id); var excludeCredentials = existingKeys - .Select(k => new PublicKeyCredentialDescriptor(CoreHelpers.Base64UrlDecode(k.DescriptorId))) + .Select(k => new PublicKeyCredentialDescriptor(CoreHelpers.Base64UrlDecode(k.CredentialId))) .ToList(); 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 = 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 }; @@ -552,7 +552,7 @@ public async Task CompleteWebAuthLoginRegistrationAsync(User user, string return false; } - var existingCredentialIds = existingCredentials.Select(c => c.DescriptorId); + var existingCredentialIds = existingCredentials.Select(c => c.CredentialId); IsCredentialIdUniqueToUserAsyncDelegate callback = (args, cancellationToken) => Task.FromResult(!existingCredentialIds.Contains(CoreHelpers.Base64UrlEncode(args.CredentialId))); var success = await _fido2.MakeNewCredentialAsync(attestationResponse, options, callback); @@ -560,7 +560,7 @@ public async Task CompleteWebAuthLoginRegistrationAsync(User user, string var credential = new WebAuthnCredential { Name = name, - DescriptorId = CoreHelpers.Base64UrlEncode(success.Result.CredentialId), + CredentialId = CoreHelpers.Base64UrlEncode(success.Result.CredentialId), PublicKey = CoreHelpers.Base64UrlEncode(success.Result.PublicKey), Type = success.Result.CredType, AaGuid = success.Result.Aaguid, @@ -577,7 +577,7 @@ 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))) + .Select(k => new PublicKeyCredentialDescriptor(CoreHelpers.Base64UrlDecode(k.CredentialId))) .ToList(); if (existingCredentials.Count == 0) @@ -604,7 +604,7 @@ public async Task CompleteWebAuthLoginAssertionAsync(AuthenticatorAssert var userCredentials = await _webAuthnCredentialRepository.GetManyByUserIdAsync(user.Id); var assertionId = CoreHelpers.Base64UrlEncode(assertionResponse.Id); - var credential = userCredentials.FirstOrDefault(c => c.DescriptorId == assertionId); + var credential = userCredentials.FirstOrDefault(c => c.CredentialId == assertionId); if (credential == null) { return null; diff --git a/src/Sql/dbo/Stored Procedures/WebAuthnCredential_Create.sql b/src/Sql/dbo/Stored Procedures/WebAuthnCredential_Create.sql index e4a3816e1d05..b5f45e094a6a 100644 --- a/src/Sql/dbo/Stored Procedures/WebAuthnCredential_Create.sql +++ b/src/Sql/dbo/Stored Procedures/WebAuthnCredential_Create.sql @@ -3,11 +3,14 @@ @UserId UNIQUEIDENTIFIER, @Name NVARCHAR(50), @PublicKey VARCHAR (256), - @DescriptorId VARCHAR(256), + @CredentialId VARCHAR(256), @Counter INT, @Type VARCHAR(20), @AaGuid UNIQUEIDENTIFIER, - @UserKey VARCHAR (MAX), + @EncryptedUserKey VARCHAR (MAX), + @EncryptedPrivateKey VARCHAR (MAX), + @EncryptedPublicKey VARCHAR (MAX), + @SupportsPrf BIT, @CreationDate DATETIME2(7), @RevisionDate DATETIME2(7) AS @@ -20,11 +23,14 @@ BEGIN [UserId], [Name], [PublicKey], - [DescriptorId], + [CredentialId], [Counter], [Type], [AaGuid], - [UserKey], + [EncryptedUserKey], + [EncryptedPrivateKey], + [EncryptedPublicKey], + [SupportsPrf], [CreationDate], [RevisionDate] ) @@ -34,11 +40,14 @@ BEGIN @UserId, @Name, @PublicKey, - @DescriptorId, + @CredentialId, @Counter, @Type, @AaGuid, - @UserKey, + @EncryptedUserKey, + @EncryptedPrivateKey, + @EncryptedPublicKey, + @SupportsPrf, @CreationDate, @RevisionDate ) diff --git a/src/Sql/dbo/Stored Procedures/WebAuthnCredential_Update.sql b/src/Sql/dbo/Stored Procedures/WebAuthnCredential_Update.sql index 060d312fd5f2..5a4da528c8fb 100644 --- a/src/Sql/dbo/Stored Procedures/WebAuthnCredential_Update.sql +++ b/src/Sql/dbo/Stored Procedures/WebAuthnCredential_Update.sql @@ -3,11 +3,14 @@ @UserId UNIQUEIDENTIFIER, @Name NVARCHAR(50), @PublicKey VARCHAR (256), - @DescriptorId VARCHAR(256), + @CredentialId VARCHAR(256), @Counter INT, @Type VARCHAR(20), @AaGuid UNIQUEIDENTIFIER, - @UserKey VARCHAR (MAX), + @EncryptedUserKey VARCHAR (MAX), + @EncryptedPrivateKey VARCHAR (MAX), + @EncryptedPublicKey VARCHAR (MAX), + @SupportsPrf BIT, @CreationDate DATETIME2(7), @RevisionDate DATETIME2(7) AS @@ -20,13 +23,16 @@ BEGIN [UserId] = @UserId, [Name] = @Name, [PublicKey] = @PublicKey, - [DescriptorId] = @DescriptorId, + [CredentialId] = @CredentialId, [Counter] = @Counter, [Type] = @Type, [AaGuid] = @AaGuid, - [UserKey] = @UserKey, + [EncryptedUserKey] = @EncryptedUserKey, + [EncryptedPrivateKey] = @EncryptedPrivateKey, + [EncryptedPublicKey] = @EncryptedPublicKey, + [SupportsPrf] = @SupportsPrf, [CreationDate] = @CreationDate, [RevisionDate] = @RevisionDate WHERE [Id] = @Id -END \ No newline at end of file +END diff --git a/src/Sql/dbo/Tables/WebAuthnCredential.sql b/src/Sql/dbo/Tables/WebAuthnCredential.sql index 670869b85b0f..17828df706ed 100644 --- a/src/Sql/dbo/Tables/WebAuthnCredential.sql +++ b/src/Sql/dbo/Tables/WebAuthnCredential.sql @@ -1,15 +1,18 @@ 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, + [Id] UNIQUEIDENTIFIER NOT NULL, + [UserId] UNIQUEIDENTIFIER NOT NULL, + [Name] NVARCHAR (50) NOT NULL, + [PublicKey] VARCHAR (256) NOT NULL, + [CredentialId] VARCHAR (256) NOT NULL, + [Counter] INT NOT NULL, + [Type] VARCHAR (20) NULL, + [AaGuid] UNIQUEIDENTIFIER NOT NULL, + [EncryptedUserKey] VARCHAR (MAX) NULL, + [EncryptedPrivateKey] VARCHAR (MAX) NULL, + [EncryptedPublicKey] 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), CONSTRAINT [FK_WebAuthnCredential_User] FOREIGN KEY ([UserId]) REFERENCES [dbo].[User] ([Id]) ); diff --git a/util/Migrator/DbScripts/2023-05-08-00_WebAuthnLoginCredentials.sql b/util/Migrator/DbScripts/2023-05-08-00_WebAuthnLoginCredentials.sql index ef3cefdec81a..a88e085dc3db 100644 --- a/util/Migrator/DbScripts/2023-05-08-00_WebAuthnLoginCredentials.sql +++ b/util/Migrator/DbScripts/2023-05-08-00_WebAuthnLoginCredentials.sql @@ -1,15 +1,18 @@ 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, + [Id] UNIQUEIDENTIFIER NOT NULL, + [UserId] UNIQUEIDENTIFIER NOT NULL, + [Name] NVARCHAR (50) NOT NULL, + [PublicKey] VARCHAR (256) NOT NULL, + [CredentialId] VARCHAR (256) NOT NULL, + [Counter] INT NOT NULL, + [Type] VARCHAR (20) NULL, + [AaGuid] UNIQUEIDENTIFIER NOT NULL, + [EncryptedUserKey] VARCHAR (MAX) NULL, + [EncryptedPrivateKey] VARCHAR (MAX) NULL, + [EncryptedPublicKey] 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), CONSTRAINT [FK_WebAuthnCredential_User] FOREIGN KEY ([UserId]) REFERENCES [dbo].[User] ([Id]) ); @@ -32,11 +35,14 @@ CREATE PROCEDURE [dbo].[WebAuthnCredential_Create] @UserId UNIQUEIDENTIFIER, @Name NVARCHAR(50), @PublicKey VARCHAR (256), - @DescriptorId VARCHAR(256), + @CredentialId VARCHAR(256), @Counter INT, @Type VARCHAR(20), @AaGuid UNIQUEIDENTIFIER, - @UserKey VARCHAR (MAX), + @EncryptedUserKey VARCHAR (MAX), + @EncryptedPrivateKey VARCHAR (MAX), + @EncryptedPublicKey VARCHAR (MAX), + @SupportsPrf BIT, @CreationDate DATETIME2(7), @RevisionDate DATETIME2(7) AS @@ -49,11 +55,14 @@ BEGIN [UserId], [Name], [PublicKey], - [DescriptorId], + [CredentialId], [Counter], [Type], [AaGuid], - [UserKey], + [EncryptedUserKey], + [EncryptedPrivateKey], + [EncryptedPublicKey], + [SupportsPrf], [CreationDate], [RevisionDate] ) @@ -63,11 +72,14 @@ BEGIN @UserId, @Name, @PublicKey, - @DescriptorId, + @CredentialId, @Counter, @Type, @AaGuid, - @UserKey, + @EncryptedUserKey, + @EncryptedPrivateKey, + @EncryptedPublicKey, + @SupportsPrf, @CreationDate, @RevisionDate ) @@ -123,11 +135,14 @@ CREATE PROCEDURE [dbo].[WebAuthnCredential_Update] @UserId UNIQUEIDENTIFIER, @Name NVARCHAR(50), @PublicKey VARCHAR (256), - @DescriptorId VARCHAR(256), + @CredentialId VARCHAR(256), @Counter INT, @Type VARCHAR(20), @AaGuid UNIQUEIDENTIFIER, - @UserKey VARCHAR (MAX), + @EncryptedUserKey VARCHAR (MAX), + @EncryptedPrivateKey VARCHAR (MAX), + @EncryptedPublicKey VARCHAR (MAX), + @SupportsPrf BIT, @CreationDate DATETIME2(7), @RevisionDate DATETIME2(7) AS @@ -140,11 +155,14 @@ BEGIN [UserId] = @UserId, [Name] = @Name, [PublicKey] = @PublicKey, - [DescriptorId] = @DescriptorId, + [CredentialId] = @CredentialId, [Counter] = @Counter, [Type] = @Type, [AaGuid] = @AaGuid, - [UserKey] = @UserKey, + [EncryptedUserKey] = @EncryptedUserKey, + [EncryptedPrivateKey] = @EncryptedPrivateKey, + [EncryptedPublicKey] = @EncryptedPublicKey, + [SupportsPrf] = @SupportsPrf, [CreationDate] = @CreationDate, [RevisionDate] = @RevisionDate WHERE @@ -167,4 +185,4 @@ BEGIN [Id] = @Id AND [UserId] = @UserId -END \ No newline at end of file +END From 3bda44727e56a3eb6a3e9557696741373529a33a Mon Sep 17 00:00:00 2001 From: Ike <137194738+ike-kottlowski@users.noreply.github.com> Date: Thu, 12 Oct 2023 13:09:54 -0700 Subject: [PATCH 13/50] [PM-3263] fix identity server tests for passkey registration (#3331) * Added WebAuthnRepo to EF DI * updated config to match current grant types --- .../EntityFrameworkServiceCollectionExtensions.cs | 1 + test/Identity.IntegrationTest/openid-configuration.json | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Infrastructure.EntityFramework/EntityFrameworkServiceCollectionExtensions.cs b/src/Infrastructure.EntityFramework/EntityFrameworkServiceCollectionExtensions.cs index 74716e2077d5..cda0207dcf42 100644 --- a/src/Infrastructure.EntityFramework/EntityFrameworkServiceCollectionExtensions.cs +++ b/src/Infrastructure.EntityFramework/EntityFrameworkServiceCollectionExtensions.cs @@ -82,6 +82,7 @@ public static void AddPasswordManagerEFRepositories(this IServiceCollection serv services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); + services.AddSingleton(); if (selfHosted) { diff --git a/test/Identity.IntegrationTest/openid-configuration.json b/test/Identity.IntegrationTest/openid-configuration.json index 9442330da764..98b0331469a7 100644 --- a/test/Identity.IntegrationTest/openid-configuration.json +++ b/test/Identity.IntegrationTest/openid-configuration.json @@ -38,7 +38,8 @@ "refresh_token", "implicit", "password", - "urn:ietf:params:oauth:grant-type:device_code" + "urn:ietf:params:oauth:grant-type:device_code", + "extension" ], "response_types_supported": [ "code", From 810143513d733a4eb84c33845ec8f54868a3cf24 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Wed, 4 Oct 2023 14:04:45 +0200 Subject: [PATCH 14/50] [PM-4167] feat: add support for `SupportsPrf` --- src/Api/Auth/Controllers/WebAuthnController.cs | 2 +- .../Request/WebAuthn/WebAuthnCredentialRequestModel.cs | 3 +++ .../Response/WebAuthn/WebAuthnCredentialResponseModel.cs | 4 ++-- src/Api/Properties/launchSettings.json | 2 +- src/Core/Services/IUserService.cs | 2 +- src/Core/Services/Implementations/UserService.cs | 5 +++-- test/Core.Test/Services/UserServiceTests.cs | 2 +- 7 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/Api/Auth/Controllers/WebAuthnController.cs b/src/Api/Auth/Controllers/WebAuthnController.cs index b7e9c5bb8b11..94b7ce85b522 100644 --- a/src/Api/Auth/Controllers/WebAuthnController.cs +++ b/src/Api/Auth/Controllers/WebAuthnController.cs @@ -68,7 +68,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.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..fea6df6b0670 100644 --- a/src/Api/Auth/Models/Request/WebAuthn/WebAuthnCredentialRequestModel.cs +++ b/src/Api/Auth/Models/Request/WebAuthn/WebAuthnCredentialRequestModel.cs @@ -13,5 +13,8 @@ public class WebAuthnCredentialRequestModel [Required] public string Token { get; set; } + + [Required] + 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..d4aef567538c 100644 --- a/src/Api/Auth/Models/Response/WebAuthn/WebAuthnCredentialResponseModel.cs +++ b/src/Api/Auth/Models/Response/WebAuthn/WebAuthnCredentialResponseModel.cs @@ -11,10 +11,10 @@ public WebAuthnCredentialResponseModel(WebAuthnCredential credential) : base(Res { Id = credential.Id.ToString(); Name = credential.Name; - PrfSupport = false; + SupportsPrf = credential.SupportsPrf; } public string Id { get; set; } public string Name { get; set; } - public bool PrfSupport { get; set; } + public bool SupportsPrf { get; set; } } diff --git a/src/Api/Properties/launchSettings.json b/src/Api/Properties/launchSettings.json index f40c28ea99d4..b67505b0fc8c 100644 --- a/src/Api/Properties/launchSettings.json +++ b/src/Api/Properties/launchSettings.json @@ -30,4 +30,4 @@ } } } -} +} \ No newline at end of file diff --git a/src/Core/Services/IUserService.cs b/src/Core/Services/IUserService.cs index e27668946638..d30ab8723031 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, 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 eee255422e07..ba66b4ea82de 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -542,7 +542,7 @@ public async Task StartWebAuthnLoginRegistrationAsync(U return options; } - public async Task CompleteWebAuthLoginRegistrationAsync(User user, string name, + public async Task CompleteWebAuthLoginRegistrationAsync(User user, string name, bool supportsPrf, CredentialCreateOptions options, AuthenticatorAttestationRawResponse attestationResponse) { @@ -565,7 +565,8 @@ 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, + SupportsPrf = supportsPrf }; await _webAuthnCredentialRepository.CreateAsync(credential); diff --git a/test/Core.Test/Services/UserServiceTests.cs b/test/Core.Test/Services/UserServiceTests.cs index 2dccea2c8751..833fca77f5ab 100644 --- a/test/Core.Test/Services/UserServiceTests.cs +++ b/test/Core.Test/Services/UserServiceTests.cs @@ -192,7 +192,7 @@ public async void CompleteWebAuthLoginRegistrationAsync_ExceedsExistingCredentia sutProvider.GetDependency().GetManyByUserIdAsync(user.Id).Returns(existingCredentials); // Act - var result = await sutProvider.Sut.CompleteWebAuthLoginRegistrationAsync(user, "name", options, response); + var result = await sutProvider.Sut.CompleteWebAuthLoginRegistrationAsync(user, "name", false, options, response); // Assert Assert.False(result); From 9580f7be81deb5a8f50ef4706b800068af0f1a23 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Thu, 5 Oct 2023 15:38:08 +0200 Subject: [PATCH 15/50] [PM-4167] feat: add `prfStatus` property --- .../WebAuthn/WebAuthnCredentialResponseModel.cs | 5 +++-- src/Core/Auth/Entities/WebAuthnCredential.cs | 16 ++++++++++++++++ src/Core/Auth/Enums/WebAuthnPrfStatus.cs | 8 ++++++++ 3 files changed, 27 insertions(+), 2 deletions(-) create mode 100644 src/Core/Auth/Enums/WebAuthnPrfStatus.cs diff --git a/src/Api/Auth/Models/Response/WebAuthn/WebAuthnCredentialResponseModel.cs b/src/Api/Auth/Models/Response/WebAuthn/WebAuthnCredentialResponseModel.cs index d4aef567538c..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; - SupportsPrf = credential.SupportsPrf; + PrfStatus = credential.GetPrfStatus(); } public string Id { get; set; } public string Name { get; set; } - public bool SupportsPrf { get; set; } + public WebAuthnPrfStatus PrfStatus { get; set; } } diff --git a/src/Core/Auth/Entities/WebAuthnCredential.cs b/src/Core/Auth/Entities/WebAuthnCredential.cs index b4b80ff65481..5cc86fc84e7b 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; @@ -29,4 +30,19 @@ public void SetNewId() { Id = CoreHelpers.GenerateComb(); } + + + public WebAuthnPrfStatus GetPrfStatus() + { + if (SupportsPrf && EncryptedUserKey != null && EncryptedPrivateKey != null && EncryptedPublicKey != 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..4977aacf71ac --- /dev/null +++ b/src/Core/Auth/Enums/WebAuthnPrfStatus.cs @@ -0,0 +1,8 @@ +namespace Bit.Core.Auth.Enums; + +public enum WebAuthnPrfStatus +{ + Enabled = 0, + Supported = 1, + Unsupported = 2 +} From 06b223cd3a1ca0a3cafa2fa731d363eae62c7932 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Fri, 6 Oct 2023 15:09:33 +0200 Subject: [PATCH 16/50] [PM-4167] feat: add support for storing PRF keys --- src/Api/Auth/Controllers/WebAuthnController.cs | 2 +- .../WebAuthn/WebAuthnCredentialRequestModel.cs | 16 ++++++++++++++++ src/Core/Auth/Entities/WebAuthnCredential.cs | 3 +++ src/Core/Services/IUserService.cs | 2 +- src/Core/Services/Implementations/UserService.cs | 6 +++++- 5 files changed, 26 insertions(+), 3 deletions(-) diff --git a/src/Api/Auth/Controllers/WebAuthnController.cs b/src/Api/Auth/Controllers/WebAuthnController.cs index 94b7ce85b522..e4c165b7d856 100644 --- a/src/Api/Auth/Controllers/WebAuthnController.cs +++ b/src/Api/Auth/Controllers/WebAuthnController.cs @@ -68,7 +68,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, model.SupportsPrf, tokenable.Options, model.DeviceResponse); + var success = await _userService.CompleteWebAuthLoginRegistrationAsync(user, model.Name, model.SupportsPrf, model.EncryptedUserKey, model.EncryptedPublicKey, model.EncryptedPrivateKey, 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 fea6df6b0670..7b2abd207d8c 100644 --- a/src/Api/Auth/Models/Request/WebAuthn/WebAuthnCredentialRequestModel.cs +++ b/src/Api/Auth/Models/Request/WebAuthn/WebAuthnCredentialRequestModel.cs @@ -1,4 +1,5 @@ using System.ComponentModel.DataAnnotations; +using Bit.Core.Utilities; using Fido2NetLib; namespace Bit.Api.Auth.Models.Request.Webauthn; @@ -16,5 +17,20 @@ public class WebAuthnCredentialRequestModel [Required] public bool SupportsPrf { get; set; } + + [Required] + [EncryptedString] + [EncryptedStringLength(2000)] + public string EncryptedUserKey { get; set; } + + [Required] + [EncryptedString] + [EncryptedStringLength(2000)] + public string EncryptedPublicKey { get; set; } + + [Required] + [EncryptedString] + [EncryptedStringLength(2000)] + public string EncryptedPrivateKey { get; set; } } diff --git a/src/Core/Auth/Entities/WebAuthnCredential.cs b/src/Core/Auth/Entities/WebAuthnCredential.cs index 5cc86fc84e7b..99dae0da7075 100644 --- a/src/Core/Auth/Entities/WebAuthnCredential.cs +++ b/src/Core/Auth/Entities/WebAuthnCredential.cs @@ -19,8 +19,11 @@ public class WebAuthnCredential : ITableObject [MaxLength(20)] public string Type { get; set; } public Guid AaGuid { get; set; } + [MaxLength(2000)] public string EncryptedUserKey { get; set; } + [MaxLength(2000)] public string EncryptedPrivateKey { get; set; } + [MaxLength(2000)] public string EncryptedPublicKey { get; set; } public bool SupportsPrf { get; set; } public DateTime CreationDate { get; internal set; } = DateTime.UtcNow; diff --git a/src/Core/Services/IUserService.cs b/src/Core/Services/IUserService.cs index d30ab8723031..1883c6e65591 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, bool supportsPrf, CredentialCreateOptions options, AuthenticatorAttestationRawResponse attestationResponse); + Task CompleteWebAuthLoginRegistrationAsync(User user, string name, bool supportsPrf, string encryptedUserKey, string encryptedPublicKey, string encryptedPrivateKey, 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 ba66b4ea82de..5552d9de63f2 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -543,6 +543,7 @@ public async Task StartWebAuthnLoginRegistrationAsync(U } public async Task CompleteWebAuthLoginRegistrationAsync(User user, string name, bool supportsPrf, + string encryptedUserKey, string encryptedPublicKey, string encryptedPrivateKey, CredentialCreateOptions options, AuthenticatorAttestationRawResponse attestationResponse) { @@ -566,7 +567,10 @@ public async Task CompleteWebAuthLoginRegistrationAsync(User user, string AaGuid = success.Result.Aaguid, Counter = (int)success.Result.Counter, UserId = user.Id, - SupportsPrf = supportsPrf + SupportsPrf = supportsPrf, + EncryptedUserKey = encryptedUserKey, + EncryptedPublicKey = encryptedPublicKey, + EncryptedPrivateKey = encryptedPrivateKey }; await _webAuthnCredentialRepository.CreateAsync(credential); From eae9fa62a3d7deeb4bdd330ccb159c71c2c8a231 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Thu, 12 Oct 2023 11:32:20 +0200 Subject: [PATCH 17/50] [PM-4167] fix: allow credentials to be created without encryption support --- .../Models/Request/WebAuthn/WebAuthnCredentialRequestModel.cs | 3 --- src/Api/Properties/launchSettings.json | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/Api/Auth/Models/Request/WebAuthn/WebAuthnCredentialRequestModel.cs b/src/Api/Auth/Models/Request/WebAuthn/WebAuthnCredentialRequestModel.cs index 7b2abd207d8c..43eae3a805c0 100644 --- a/src/Api/Auth/Models/Request/WebAuthn/WebAuthnCredentialRequestModel.cs +++ b/src/Api/Auth/Models/Request/WebAuthn/WebAuthnCredentialRequestModel.cs @@ -18,17 +18,14 @@ public class WebAuthnCredentialRequestModel [Required] public bool SupportsPrf { get; set; } - [Required] [EncryptedString] [EncryptedStringLength(2000)] public string EncryptedUserKey { get; set; } - [Required] [EncryptedString] [EncryptedStringLength(2000)] public string EncryptedPublicKey { get; set; } - [Required] [EncryptedString] [EncryptedStringLength(2000)] public string EncryptedPrivateKey { get; set; } diff --git a/src/Api/Properties/launchSettings.json b/src/Api/Properties/launchSettings.json index b67505b0fc8c..f40c28ea99d4 100644 --- a/src/Api/Properties/launchSettings.json +++ b/src/Api/Properties/launchSettings.json @@ -30,4 +30,4 @@ } } } -} \ No newline at end of file +} From 72420d0049befc1c184ccd3d2d0aa58ac13a374e Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Thu, 12 Oct 2023 14:59:31 +0200 Subject: [PATCH 18/50] [PM-4167] fix: broken test --- test/Core.Test/Services/UserServiceTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Core.Test/Services/UserServiceTests.cs b/test/Core.Test/Services/UserServiceTests.cs index 833fca77f5ab..bca1b1deca53 100644 --- a/test/Core.Test/Services/UserServiceTests.cs +++ b/test/Core.Test/Services/UserServiceTests.cs @@ -192,7 +192,7 @@ public async void CompleteWebAuthLoginRegistrationAsync_ExceedsExistingCredentia sutProvider.GetDependency().GetManyByUserIdAsync(user.Id).Returns(existingCredentials); // Act - var result = await sutProvider.Sut.CompleteWebAuthLoginRegistrationAsync(user, "name", false, options, response); + var result = await sutProvider.Sut.CompleteWebAuthLoginRegistrationAsync(user, "name", false, null, null, null, options, response); // Assert Assert.False(result); From 97cfd1ecd691ef4f6c85e89f72d4b56d91fe99d8 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Thu, 12 Oct 2023 15:03:17 +0200 Subject: [PATCH 19/50] [PM-4167] chore: remove whitespace --- src/Core/Auth/Entities/WebAuthnCredential.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Core/Auth/Entities/WebAuthnCredential.cs b/src/Core/Auth/Entities/WebAuthnCredential.cs index 99dae0da7075..dcde686bd0a9 100644 --- a/src/Core/Auth/Entities/WebAuthnCredential.cs +++ b/src/Core/Auth/Entities/WebAuthnCredential.cs @@ -34,7 +34,6 @@ public void SetNewId() Id = CoreHelpers.GenerateComb(); } - public WebAuthnPrfStatus GetPrfStatus() { if (SupportsPrf && EncryptedUserKey != null && EncryptedPrivateKey != null && EncryptedPublicKey != null) From 083b3421f5f14d1c5bcfb08dabc57288fae838c4 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Fri, 13 Oct 2023 14:03:23 +0200 Subject: [PATCH 20/50] [PM-4167] fix: controller test --- test/Api.Test/Auth/Controllers/WebAuthnControllerTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Api.Test/Auth/Controllers/WebAuthnControllerTests.cs b/test/Api.Test/Auth/Controllers/WebAuthnControllerTests.cs index 32f2d5d49177..01fe69010adb 100644 --- a/test/Api.Test/Auth/Controllers/WebAuthnControllerTests.cs +++ b/test/Api.Test/Auth/Controllers/WebAuthnControllerTests.cs @@ -100,7 +100,7 @@ public async Task Post_ValidInput_Returns(WebAuthnCredentialRequestModel request .GetUserByPrincipalAsync(default) .ReturnsForAnyArgs(user); sutProvider.GetDependency() - .CompleteWebAuthLoginRegistrationAsync(user, requestModel.Name, createOptions, Arg.Any()) + .CompleteWebAuthLoginRegistrationAsync(user, requestModel.Name, requestModel.SupportsPrf, requestModel.EncryptedUserKey, requestModel.EncryptedPublicKey, requestModel.EncryptedPrivateKey, createOptions, Arg.Any()) .Returns(true); sutProvider.GetDependency>() .Unprotect(requestModel.Token) From 69af190ec3edf15f3bbf11518053755a1cfa7698 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Thu, 12 Oct 2023 15:59:36 +0200 Subject: [PATCH 21/50] [PM-3936] [PM-4174] feat: update `UserVerificationRequirement` and `requireResidentKey` --- src/Core/Services/Implementations/UserService.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index 5552d9de63f2..dbe5ece15de5 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -530,8 +530,8 @@ 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 - UserVerification = UserVerificationRequirement.Preferred + RequireResidentKey = true, + UserVerification = UserVerificationRequirement.Required }; var extensions = new AuthenticationExtensionsClientInputs { }; @@ -595,7 +595,7 @@ public async Task StartWebAuthnLoginAssertionAsync(User user) { UserVerificationMethod = true }; - var options = _fido2.GetAssertionOptions(existingCredentials, UserVerificationRequirement.Preferred, exts); + var options = _fido2.GetAssertionOptions(existingCredentials, UserVerificationRequirement.Required, exts); // TODO: temp save options to user record somehow From 120b182d284c006716f3343e556afa2798a50f52 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Fri, 13 Oct 2023 14:21:12 +0200 Subject: [PATCH 22/50] [PM-3936] fix: lint --- src/Core/Services/Implementations/UserService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index dbe5ece15de5..5b088d81e42d 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -530,7 +530,7 @@ public async Task StartWebAuthnLoginRegistrationAsync(U var authenticatorSelection = new AuthenticatorSelection { AuthenticatorAttachment = null, - RequireResidentKey = true, + RequireResidentKey = true, UserVerification = UserVerificationRequirement.Required }; From a28ea69241e886dbdca45b8c4e5f38cce8ca5374 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Tue, 17 Oct 2023 15:57:10 +0200 Subject: [PATCH 23/50] [PM-2032] feat: add assertion options tokenable --- .../WebAuthnLoginAssertionOptionsScope.cs | 7 +++ .../WebAuthnLoginAssertionOptionsTokenable.cs | 44 +++++++++++++ ...uthnLoginAssertionOptionsTokenableTests.cs | 61 +++++++++++++++++++ 3 files changed, 112 insertions(+) create mode 100644 src/Core/Auth/Enums/WebAuthnLoginAssertionOptionsScope.cs create mode 100644 src/Core/Auth/Models/Business/Tokenables/WebAuthnLoginAssertionOptionsTokenable.cs create mode 100644 test/Core.Test/Auth/Models/Business/Tokenables/WebAuthnLoginAssertionOptionsTokenableTests.cs diff --git a/src/Core/Auth/Enums/WebAuthnLoginAssertionOptionsScope.cs b/src/Core/Auth/Enums/WebAuthnLoginAssertionOptionsScope.cs new file mode 100644 index 000000000000..bcafc0e89f02 --- /dev/null +++ b/src/Core/Auth/Enums/WebAuthnLoginAssertionOptionsScope.cs @@ -0,0 +1,7 @@ +namespace Bit.Core.Auth.Enums; + +public enum WebAuthnLoginAssertionOptionsScope +{ + Authentication = 0, + PrfRegistration = 1 +} diff --git a/src/Core/Auth/Models/Business/Tokenables/WebAuthnLoginAssertionOptionsTokenable.cs b/src/Core/Auth/Models/Business/Tokenables/WebAuthnLoginAssertionOptionsTokenable.cs new file mode 100644 index 000000000000..0b37c4bb2e2e --- /dev/null +++ b/src/Core/Auth/Models/Business/Tokenables/WebAuthnLoginAssertionOptionsTokenable.cs @@ -0,0 +1,44 @@ +using System.Text.Json.Serialization; +using Bit.Core.Auth.Enums; +using Bit.Core.Tokens; +using Fido2NetLib; + +namespace Bit.Core.Auth.Models.Business.Tokenables; + +public class WebAuthnLoginAssertionOptionsTokenable : ExpiringTokenable +{ + // 7 minutes = max webauthn timeout (6 minutes) + slack for miscellaneous delays + private const double _tokenLifetimeInHours = (double)7 / 60; + public const string ClearTextPrefix = "BWWebAuthnLoginAssertionOptions_"; + public const string DataProtectorPurpose = "WebAuthnLoginAssetionOptionsDataProtector"; + public const string TokenIdentifier = "WebAuthnLoginAssertionOptionsToken"; + + public string Identifier { get; set; } = TokenIdentifier; + public CredentialCreateOptions Options { get; set; } + public WebAuthnLoginAssertionOptionsScope Scope { get; set; } + + [JsonConstructor] + public WebAuthnLoginAssertionOptionsTokenable() + { + ExpirationDate = DateTime.UtcNow.AddHours(_tokenLifetimeInHours); + } + + public WebAuthnLoginAssertionOptionsTokenable(WebAuthnLoginAssertionOptionsScope scope, CredentialCreateOptions options) : this() + { + Scope = scope; + Options = options; + } + + public bool TokenIsValid(WebAuthnLoginAssertionOptionsScope scope) + { + if (!Valid) + { + return false; + } + + return Scope == scope; + } + + protected override bool TokenIsValid() => Identifier == TokenIdentifier && Options != null; +} + diff --git a/test/Core.Test/Auth/Models/Business/Tokenables/WebAuthnLoginAssertionOptionsTokenableTests.cs b/test/Core.Test/Auth/Models/Business/Tokenables/WebAuthnLoginAssertionOptionsTokenableTests.cs new file mode 100644 index 000000000000..93f9ca32b698 --- /dev/null +++ b/test/Core.Test/Auth/Models/Business/Tokenables/WebAuthnLoginAssertionOptionsTokenableTests.cs @@ -0,0 +1,61 @@ +using Bit.Core.Auth.Enums; +using Bit.Core.Auth.Models.Business.Tokenables; +using Bit.Test.Common.AutoFixture.Attributes; +using Fido2NetLib; +using Xunit; + +namespace Bit.Core.Test.Auth.Models.Business.Tokenables; + +public class WebAuthnLoginAssertionOptionsTokenableTests +{ + [Theory, BitAutoData] + public void Valid_TokenWithoutOptions_ReturnsFalse(WebAuthnLoginAssertionOptionsScope scope) + { + var token = new WebAuthnLoginAssertionOptionsTokenable(scope, null); + + var isValid = token.Valid; + + Assert.False(isValid); + } + + [Theory, BitAutoData] + public void Valid_NewlyCreatedToken_ReturnsTrue(WebAuthnLoginAssertionOptionsScope scope, CredentialCreateOptions createOptions) + { + var token = new WebAuthnLoginAssertionOptionsTokenable(scope, createOptions); + + + var isValid = token.Valid; + + Assert.True(isValid); + } + + [Theory, BitAutoData] + public void ValidIsValid_TokenWithoutOptions_ReturnsFalse(WebAuthnLoginAssertionOptionsScope scope) + { + var token = new WebAuthnLoginAssertionOptionsTokenable(scope, null); + + var isValid = token.TokenIsValid(scope); + + Assert.False(isValid); + } + + [Theory, BitAutoData] + public void ValidIsValid_NonMatchingScope_ReturnsFalse(WebAuthnLoginAssertionOptionsScope scope1, WebAuthnLoginAssertionOptionsScope scope2, CredentialCreateOptions createOptions) + { + var token = new WebAuthnLoginAssertionOptionsTokenable(scope1, createOptions); + + var isValid = token.TokenIsValid(scope2); + + Assert.False(isValid); + } + + [Theory, BitAutoData] + public void ValidIsValid_SameScope_ReturnsTrue(WebAuthnLoginAssertionOptionsScope scope, CredentialCreateOptions createOptions) + { + var token = new WebAuthnLoginAssertionOptionsTokenable(scope, createOptions); + + var isValid = token.TokenIsValid(scope); + + Assert.True(isValid); + } +} From 1ffd97e935b57f3781deb65e9743bed67745f176 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Wed, 18 Oct 2023 10:28:23 +0200 Subject: [PATCH 24/50] [PM-2032] feat: add request and response models --- .../src/Scim/Properties/launchSettings.json | 2 +- .../src/Sso/Properties/launchSettings.json | 6 +-- .../Properties/launchSettings.json | 2 +- src/Admin/Properties/launchSettings.json | 2 +- src/Api/Properties/launchSettings.json | 2 +- src/Billing/Properties/launchSettings.json | 2 +- ...bAuthnLoginAssertionOptionsRequestModel.cs | 10 ++++ ...AuthnLoginAssertionOptionsResponseModel.cs | 18 +++++++ src/Events/Properties/launchSettings.json | 2 +- .../Properties/launchSettings.json | 2 +- src/Icons/Properties/launchSettings.json | 2 +- .../Controllers/AccountsController.cs | 51 ++++++++++--------- src/Identity/Properties/launchSettings.json | 2 +- .../Properties/launchSettings.json | 2 +- .../Properties/launchSettings.json | 2 +- .../Properties/launchSettings.json | 2 +- util/Server/Properties/launchSettings.json | 2 +- 17 files changed, 71 insertions(+), 40 deletions(-) create mode 100644 src/Core/Auth/Models/Api/Request/Accounts/WebAuthnLoginAssertionOptionsRequestModel.cs create mode 100644 src/Core/Auth/Models/Api/Response/Accounts/WebAuthnLoginAssertionOptionsResponseModel.cs diff --git a/bitwarden_license/src/Scim/Properties/launchSettings.json b/bitwarden_license/src/Scim/Properties/launchSettings.json index aa77a519810a..a6add7c52311 100644 --- a/bitwarden_license/src/Scim/Properties/launchSettings.json +++ b/bitwarden_license/src/Scim/Properties/launchSettings.json @@ -24,4 +24,4 @@ } } } -} +} \ No newline at end of file diff --git a/bitwarden_license/src/Sso/Properties/launchSettings.json b/bitwarden_license/src/Sso/Properties/launchSettings.json index ce32b2110566..561dcd84c71f 100644 --- a/bitwarden_license/src/Sso/Properties/launchSettings.json +++ b/bitwarden_license/src/Sso/Properties/launchSettings.json @@ -3,7 +3,7 @@ "windowsAuthentication": false, "anonymousAuthentication": true, "iisExpress": { - "applicationUrl": "http://localhost:51822", + "applicationUrl": "http://0.0.0.0:51822", "sslPort": 0 } }, @@ -16,7 +16,7 @@ }, "Sso": { "commandName": "Project", - "applicationUrl": "http://localhost:51822", + "applicationUrl": "http://0.0.0.0:51822", "environmentVariables": { "ASPNETCORE_ENVIRONMENT": "Development" } @@ -30,4 +30,4 @@ } } } -} +} \ No newline at end of file diff --git a/bitwarden_license/test/Scim.IntegrationTest/Properties/launchSettings.json b/bitwarden_license/test/Scim.IntegrationTest/Properties/launchSettings.json index 485fc8885a52..f3895db525a1 100644 --- a/bitwarden_license/test/Scim.IntegrationTest/Properties/launchSettings.json +++ b/bitwarden_license/test/Scim.IntegrationTest/Properties/launchSettings.json @@ -13,4 +13,4 @@ } } } -} +} \ No newline at end of file diff --git a/src/Admin/Properties/launchSettings.json b/src/Admin/Properties/launchSettings.json index 16e75d4388e0..8e3857c915c1 100644 --- a/src/Admin/Properties/launchSettings.json +++ b/src/Admin/Properties/launchSettings.json @@ -30,4 +30,4 @@ } } } -} +} \ No newline at end of file diff --git a/src/Api/Properties/launchSettings.json b/src/Api/Properties/launchSettings.json index f40c28ea99d4..b67505b0fc8c 100644 --- a/src/Api/Properties/launchSettings.json +++ b/src/Api/Properties/launchSettings.json @@ -30,4 +30,4 @@ } } } -} +} \ No newline at end of file diff --git a/src/Billing/Properties/launchSettings.json b/src/Billing/Properties/launchSettings.json index 5b3fd376476d..b4497ceca279 100644 --- a/src/Billing/Properties/launchSettings.json +++ b/src/Billing/Properties/launchSettings.json @@ -24,4 +24,4 @@ } } } -} +} \ No newline at end of file diff --git a/src/Core/Auth/Models/Api/Request/Accounts/WebAuthnLoginAssertionOptionsRequestModel.cs b/src/Core/Auth/Models/Api/Request/Accounts/WebAuthnLoginAssertionOptionsRequestModel.cs new file mode 100644 index 000000000000..4d4ce9c0f1dc --- /dev/null +++ b/src/Core/Auth/Models/Api/Request/Accounts/WebAuthnLoginAssertionOptionsRequestModel.cs @@ -0,0 +1,10 @@ +namespace Bit.Core.Auth.Models.Api.Request.Accounts; + +public class WebAuthnLoginAssertionOptionsRequestModel +{ + // Requried to add support for non-discoverable logins + // [EmailAddress] + // [StringLength(256)] + // public string Email { get; set; } +} + diff --git a/src/Core/Auth/Models/Api/Response/Accounts/WebAuthnLoginAssertionOptionsResponseModel.cs b/src/Core/Auth/Models/Api/Response/Accounts/WebAuthnLoginAssertionOptionsResponseModel.cs new file mode 100644 index 000000000000..6a0641246be8 --- /dev/null +++ b/src/Core/Auth/Models/Api/Response/Accounts/WebAuthnLoginAssertionOptionsResponseModel.cs @@ -0,0 +1,18 @@ + +using Bit.Core.Models.Api; +using Fido2NetLib; + +namespace Bit.Core.Auth.Models.Api.Response.Accounts; + +public class WebAuthnLoginAssertionOptionsResponseModel : ResponseModel +{ + private const string ResponseObj = "webAuthnLoginAssertionOptions"; + + public WebAuthnLoginAssertionOptionsResponseModel() : base(ResponseObj) + { + } + + public AssertionOptions Options { get; set; } + public string Token { get; set; } +} + diff --git a/src/Events/Properties/launchSettings.json b/src/Events/Properties/launchSettings.json index a743b51c5696..0e2ceda6c90e 100644 --- a/src/Events/Properties/launchSettings.json +++ b/src/Events/Properties/launchSettings.json @@ -30,4 +30,4 @@ } } } -} +} \ No newline at end of file diff --git a/src/EventsProcessor/Properties/launchSettings.json b/src/EventsProcessor/Properties/launchSettings.json index f772b225dfd8..835fe5c55b26 100644 --- a/src/EventsProcessor/Properties/launchSettings.json +++ b/src/EventsProcessor/Properties/launchSettings.json @@ -22,4 +22,4 @@ } } } -} +} \ No newline at end of file diff --git a/src/Icons/Properties/launchSettings.json b/src/Icons/Properties/launchSettings.json index 9588da8c5f4f..c6cc79cced1e 100644 --- a/src/Icons/Properties/launchSettings.json +++ b/src/Icons/Properties/launchSettings.json @@ -24,4 +24,4 @@ } } } -} +} \ No newline at end of file diff --git a/src/Identity/Controllers/AccountsController.cs b/src/Identity/Controllers/AccountsController.cs index 9073884d8c05..40ec0500cee0 100644 --- a/src/Identity/Controllers/AccountsController.cs +++ b/src/Identity/Controllers/AccountsController.cs @@ -75,36 +75,39 @@ public async Task PostPrelogin([FromBody] PreloginRequest return new PreloginResponseModel(kdfInformation); } - [HttpPost("webauthn-assertion-options")] + [HttpPost("webauthn/assertion-options")] [ApiExplorerSettings(IgnoreApi = true)] // Disable Swagger due to CredentialCreateOptions not converting properly [RequireFeature(FeatureFlagKeys.PasswordlessLogin)] + // TODO: Add tests // TODO: Create proper models for this call - public async Task PostWebAuthnAssertionOptions([FromBody] PreloginRequestModel model) + public async Task PostWebAuthnLoginAssertionOptions(WebAuthnLoginAssertionOptionsRequestModel 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(user); + //return options; + return null; } - [HttpPost("webauthn-assertion")] - [RequireFeature(FeatureFlagKeys.PasswordlessLogin)] - // 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(); - } + // TODO: Remove, this will move to the grant validator + //[HttpPost("webauthn-assertion")] + //[RequireFeature(FeatureFlagKeys.PasswordlessLogin)] + //// 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; - } + // var token = await _userService.CompleteWebAuthLoginAssertionAsync(null, user); + // return token; + //} } diff --git a/src/Identity/Properties/launchSettings.json b/src/Identity/Properties/launchSettings.json index 8dcc422bcdb9..de4db21fbbd9 100644 --- a/src/Identity/Properties/launchSettings.json +++ b/src/Identity/Properties/launchSettings.json @@ -30,4 +30,4 @@ } } } -} +} \ No newline at end of file diff --git a/src/Notifications/Properties/launchSettings.json b/src/Notifications/Properties/launchSettings.json index adff79ae353c..74adbc567d86 100644 --- a/src/Notifications/Properties/launchSettings.json +++ b/src/Notifications/Properties/launchSettings.json @@ -30,4 +30,4 @@ } } } -} +} \ No newline at end of file diff --git a/test/Api.IntegrationTest/Properties/launchSettings.json b/test/Api.IntegrationTest/Properties/launchSettings.json index 04fd816afe74..57c702253115 100644 --- a/test/Api.IntegrationTest/Properties/launchSettings.json +++ b/test/Api.IntegrationTest/Properties/launchSettings.json @@ -13,4 +13,4 @@ } } } -} +} \ No newline at end of file diff --git a/test/Identity.IntegrationTest/Properties/launchSettings.json b/test/Identity.IntegrationTest/Properties/launchSettings.json index 43e4ad659f4b..46f4892b335e 100644 --- a/test/Identity.IntegrationTest/Properties/launchSettings.json +++ b/test/Identity.IntegrationTest/Properties/launchSettings.json @@ -8,4 +8,4 @@ } } } -} +} \ No newline at end of file diff --git a/util/Server/Properties/launchSettings.json b/util/Server/Properties/launchSettings.json index 22aba45f0a71..0943477d82fd 100644 --- a/util/Server/Properties/launchSettings.json +++ b/util/Server/Properties/launchSettings.json @@ -8,4 +8,4 @@ } } } -} +} \ No newline at end of file From 0f44dda900fbd5b5fc53761d22d6ffcd138fe9a7 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Wed, 18 Oct 2023 14:07:33 +0200 Subject: [PATCH 25/50] [PM-2032] feat: implement `assertion-options` identity endpoint --- .../WebAuthnLoginAssertionOptionsTokenable.cs | 4 ++-- src/Core/Services/IUserService.cs | 2 +- .../Services/Implementations/UserService.cs | 24 +++---------------- .../Controllers/AccountsController.cs | 22 +++++++++++++---- .../Utilities/ServiceCollectionExtensions.cs | 6 +++++ ...uthnLoginAssertionOptionsTokenableTests.cs | 6 ++--- 6 files changed, 33 insertions(+), 31 deletions(-) diff --git a/src/Core/Auth/Models/Business/Tokenables/WebAuthnLoginAssertionOptionsTokenable.cs b/src/Core/Auth/Models/Business/Tokenables/WebAuthnLoginAssertionOptionsTokenable.cs index 0b37c4bb2e2e..e3aebd01c06c 100644 --- a/src/Core/Auth/Models/Business/Tokenables/WebAuthnLoginAssertionOptionsTokenable.cs +++ b/src/Core/Auth/Models/Business/Tokenables/WebAuthnLoginAssertionOptionsTokenable.cs @@ -14,7 +14,7 @@ public class WebAuthnLoginAssertionOptionsTokenable : ExpiringTokenable public const string TokenIdentifier = "WebAuthnLoginAssertionOptionsToken"; public string Identifier { get; set; } = TokenIdentifier; - public CredentialCreateOptions Options { get; set; } + public AssertionOptions Options { get; set; } public WebAuthnLoginAssertionOptionsScope Scope { get; set; } [JsonConstructor] @@ -23,7 +23,7 @@ public WebAuthnLoginAssertionOptionsTokenable() ExpirationDate = DateTime.UtcNow.AddHours(_tokenLifetimeInHours); } - public WebAuthnLoginAssertionOptionsTokenable(WebAuthnLoginAssertionOptionsScope scope, CredentialCreateOptions options) : this() + public WebAuthnLoginAssertionOptionsTokenable(WebAuthnLoginAssertionOptionsScope scope, AssertionOptions options) : this() { Scope = scope; Options = options; diff --git a/src/Core/Services/IUserService.cs b/src/Core/Services/IUserService.cs index 1883c6e65591..7dfe95cc38ed 100644 --- a/src/Core/Services/IUserService.cs +++ b/src/Core/Services/IUserService.cs @@ -29,7 +29,7 @@ public interface IUserService Task CompleteWebAuthRegistrationAsync(User user, int value, string name, AuthenticatorAttestationRawResponse attestationResponse); Task StartWebAuthnLoginRegistrationAsync(User user); Task CompleteWebAuthLoginRegistrationAsync(User user, string name, bool supportsPrf, string encryptedUserKey, string encryptedPublicKey, string encryptedPrivateKey, CredentialCreateOptions options, AuthenticatorAttestationRawResponse attestationResponse); - Task StartWebAuthnLoginAssertionAsync(User user); + Task StartWebAuthnLoginAssertionAsync(); Task CompleteWebAuthLoginAssertionAsync(AuthenticatorAssertionRawResponse assertionResponse, User user); Task SendEmailVerificationAsync(User user); Task ConfirmEmailAsync(User user, string token); diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index 5b088d81e42d..1e2cddd57224 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -577,29 +577,11 @@ public async Task CompleteWebAuthLoginRegistrationAsync(User user, string return true; } - public async Task StartWebAuthnLoginAssertionAsync(User user) + public Task StartWebAuthnLoginAssertionAsync() { - var provider = user.GetTwoFactorProvider(TwoFactorProviderType.WebAuthn); - var existingKeys = await _webAuthnCredentialRepository.GetManyByUserIdAsync(user.Id); - var existingCredentials = existingKeys - .Select(k => new PublicKeyCredentialDescriptor(CoreHelpers.Base64UrlDecode(k.CredentialId))) - .ToList(); + var options = _fido2.GetAssertionOptions(Enumerable.Empty(), UserVerificationRequirement.Required); - if (existingCredentials.Count == 0) - { - return null; - } - - // TODO: PRF? - var exts = new AuthenticationExtensionsClientInputs - { - UserVerificationMethod = true - }; - var options = _fido2.GetAssertionOptions(existingCredentials, UserVerificationRequirement.Required, exts); - - // TODO: temp save options to user record somehow - - return options; + return Task.FromResult(options); } public async Task CompleteWebAuthLoginAssertionAsync(AuthenticatorAssertionRawResponse assertionResponse, User user) diff --git a/src/Identity/Controllers/AccountsController.cs b/src/Identity/Controllers/AccountsController.cs index 40ec0500cee0..5d2f77399643 100644 --- a/src/Identity/Controllers/AccountsController.cs +++ b/src/Identity/Controllers/AccountsController.cs @@ -1,6 +1,8 @@ using Bit.Core; +using Bit.Core.Auth.Enums; 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; @@ -8,6 +10,7 @@ using Bit.Core.Models.Data; using Bit.Core.Repositories; using Bit.Core.Services; +using Bit.Core.Tokens; using Bit.Core.Utilities; using Bit.SharedWeb.Utilities; using Fido2NetLib; @@ -23,17 +26,21 @@ 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. @@ -89,9 +96,16 @@ public async Task PostWebAuthnLoginA // return new AssertionOptions(); //} - //var options = await _userService.StartWebAuthnLoginAssertionAsync(user); - //return options; - return null; + var options = await _userService.StartWebAuthnLoginAssertionAsync(); + + var tokenable = new WebAuthnLoginAssertionOptionsTokenable(WebAuthnLoginAssertionOptionsScope.Authentication, options); + var token = _assertionOptionsDataProtector.Protect(tokenable); + + return new WebAuthnLoginAssertionOptionsResponseModel + { + Options = options, + Token = token + }; } // TODO: Remove, this will move to the grant validator diff --git a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs index f3005e7759f8..0fd3a67c9824 100644 --- a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs +++ b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs @@ -174,6 +174,12 @@ public static void AddTokenizers(this IServiceCollection services) WebAuthnCredentialCreateOptionsTokenable.DataProtectorPurpose, serviceProvider.GetDataProtectionProvider(), serviceProvider.GetRequiredService>>())); + services.AddSingleton>(serviceProvider => + new DataProtectorTokenFactory( + WebAuthnLoginAssertionOptionsTokenable.ClearTextPrefix, + WebAuthnLoginAssertionOptionsTokenable.DataProtectorPurpose, + serviceProvider.GetDataProtectionProvider(), + serviceProvider.GetRequiredService>>())); services.AddSingleton>(serviceProvider => new DataProtectorTokenFactory( SsoEmail2faSessionTokenable.ClearTextPrefix, diff --git a/test/Core.Test/Auth/Models/Business/Tokenables/WebAuthnLoginAssertionOptionsTokenableTests.cs b/test/Core.Test/Auth/Models/Business/Tokenables/WebAuthnLoginAssertionOptionsTokenableTests.cs index 93f9ca32b698..3978fef0f4ed 100644 --- a/test/Core.Test/Auth/Models/Business/Tokenables/WebAuthnLoginAssertionOptionsTokenableTests.cs +++ b/test/Core.Test/Auth/Models/Business/Tokenables/WebAuthnLoginAssertionOptionsTokenableTests.cs @@ -19,7 +19,7 @@ public void Valid_TokenWithoutOptions_ReturnsFalse(WebAuthnLoginAssertionOptions } [Theory, BitAutoData] - public void Valid_NewlyCreatedToken_ReturnsTrue(WebAuthnLoginAssertionOptionsScope scope, CredentialCreateOptions createOptions) + public void Valid_NewlyCreatedToken_ReturnsTrue(WebAuthnLoginAssertionOptionsScope scope, AssertionOptions createOptions) { var token = new WebAuthnLoginAssertionOptionsTokenable(scope, createOptions); @@ -40,7 +40,7 @@ public void ValidIsValid_TokenWithoutOptions_ReturnsFalse(WebAuthnLoginAssertion } [Theory, BitAutoData] - public void ValidIsValid_NonMatchingScope_ReturnsFalse(WebAuthnLoginAssertionOptionsScope scope1, WebAuthnLoginAssertionOptionsScope scope2, CredentialCreateOptions createOptions) + public void ValidIsValid_NonMatchingScope_ReturnsFalse(WebAuthnLoginAssertionOptionsScope scope1, WebAuthnLoginAssertionOptionsScope scope2, AssertionOptions createOptions) { var token = new WebAuthnLoginAssertionOptionsTokenable(scope1, createOptions); @@ -50,7 +50,7 @@ public void ValidIsValid_NonMatchingScope_ReturnsFalse(WebAuthnLoginAssertionOpt } [Theory, BitAutoData] - public void ValidIsValid_SameScope_ReturnsTrue(WebAuthnLoginAssertionOptionsScope scope, CredentialCreateOptions createOptions) + public void ValidIsValid_SameScope_ReturnsTrue(WebAuthnLoginAssertionOptionsScope scope, AssertionOptions createOptions) { var token = new WebAuthnLoginAssertionOptionsTokenable(scope, createOptions); From 3a738e3fa33882d5202edb18f4912e511aeda89d Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Wed, 18 Oct 2023 15:54:07 +0200 Subject: [PATCH 26/50] Add LaunchDarkly flag override file to .gitignore (#3357) * Add `src/Identity/flags.json` to .gitignore * Change to cover all OSS projects * Include `bitwarden_license` projects (cherry picked from commit e9be7f11f649bc723220107f49b6824bd7adab8c) --- .gitignore | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 4b798f3b74a8..dd26998ccea9 100644 --- a/.gitignore +++ b/.gitignore @@ -225,4 +225,4 @@ src/Identity/Identity.zip src/Notifications/Notifications.zip bitwarden_license/src/Portal/Portal.zip bitwarden_license/src/Sso/Sso.zip -src/Api/flags.json +**/src/*/flags.json From 70533a2f35c7e3bba2d1f803807de71a32d6a865 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Thu, 19 Oct 2023 16:09:06 +0200 Subject: [PATCH 27/50] [PM-2032] feat: implement authentication with passkey --- src/Core/Services/IUserService.cs | 2 +- .../Services/Implementations/UserService.cs | 9 ++-- src/Identity/IdentityServer/ApiClient.cs | 2 +- .../IdentityServer/BaseRequestValidator.cs | 2 +- .../IdentityServer/ExtensionGrantValidator.cs | 47 +++++++++++-------- 5 files changed, 35 insertions(+), 27 deletions(-) diff --git a/src/Core/Services/IUserService.cs b/src/Core/Services/IUserService.cs index 7dfe95cc38ed..55ce0722ae8b 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, bool supportsPrf, string encryptedUserKey, string encryptedPublicKey, string encryptedPrivateKey, CredentialCreateOptions options, AuthenticatorAttestationRawResponse attestationResponse); Task StartWebAuthnLoginAssertionAsync(); - Task CompleteWebAuthLoginAssertionAsync(AuthenticatorAssertionRawResponse assertionResponse, User user); + Task CompleteWebAuthLoginAssertionAsync(AssertionOptions options, AuthenticatorAssertionRawResponse assertionResponse); 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 1e2cddd57224..2fde3f0a6af5 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -584,10 +584,10 @@ public Task StartWebAuthnLoginAssertionAsync() return Task.FromResult(options); } - public async Task CompleteWebAuthLoginAssertionAsync(AuthenticatorAssertionRawResponse assertionResponse, User user) + public async Task CompleteWebAuthLoginAssertionAsync(AssertionOptions options, AuthenticatorAssertionRawResponse assertionResponse) { - // TODO: Get options from user record somehow, then clear them - var options = AssertionOptions.FromJson(""); + var userId = new Guid(assertionResponse.Response.UserHandle); + var user = await _userRepository.GetByIdAsync(userId); var userCredentials = await _webAuthnCredentialRepository.GetManyByUserIdAsync(user.Id); var assertionId = CoreHelpers.Base64UrlEncode(assertionResponse.Id); @@ -609,8 +609,7 @@ public async Task CompleteWebAuthLoginAssertionAsync(AuthenticatorAssert if (assertionVerificationResult.Status == "ok") { - var token = _webAuthnLoginTokenizer.Protect(new WebAuthnLoginTokenable(user)); - return token; + return user; } else { diff --git a/src/Identity/IdentityServer/ApiClient.cs b/src/Identity/IdentityServer/ApiClient.cs index ef94d60b4647..ddeec9584180 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, "extension" }; + AllowedGrantTypes = new[] { GrantType.ResourceOwnerPassword, GrantType.AuthorizationCode, ExtensionGrantValidator.GrantType }; RefreshTokenExpiration = TokenExpiration.Sliding; RefreshTokenUsage = TokenUsage.ReUse; SlidingRefreshTokenLifetime = 86400 * refreshTokenSlidingDays; diff --git a/src/Identity/IdentityServer/BaseRequestValidator.cs b/src/Identity/IdentityServer/BaseRequestValidator.cs index d52d3064a692..7d6106ecb799 100644 --- a/src/Identity/IdentityServer/BaseRequestValidator.cs +++ b/src/Identity/IdentityServer/BaseRequestValidator.cs @@ -35,7 +35,6 @@ public abstract class BaseRequestValidator where T : class private UserManager _userManager; private readonly IDeviceRepository _deviceRepository; private readonly IDeviceService _deviceService; - private readonly IUserService _userService; private readonly IEventService _eventService; private readonly IOrganizationDuoWebTokenProvider _organizationDuoWebTokenProvider; private readonly IOrganizationRepository _organizationRepository; @@ -53,6 +52,7 @@ public abstract class BaseRequestValidator where T : class protected IPolicyService PolicyService { get; } protected IFeatureService FeatureService { get; } protected ISsoConfigRepository SsoConfigRepository { get; } + protected IUserService _userService { get; } public BaseRequestValidator( UserManager userManager, diff --git a/src/Identity/IdentityServer/ExtensionGrantValidator.cs b/src/Identity/IdentityServer/ExtensionGrantValidator.cs index 8d9bbe119e0c..afe4125c2ea8 100644 --- a/src/Identity/IdentityServer/ExtensionGrantValidator.cs +++ b/src/Identity/IdentityServer/ExtensionGrantValidator.cs @@ -1,4 +1,6 @@ using System.Security.Claims; +using System.Text.Json; +using Bit.Core.Auth.Enums; using Bit.Core.Auth.Identity; using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Auth.Repositories; @@ -8,6 +10,7 @@ using Bit.Core.Services; using Bit.Core.Settings; using Bit.Core.Tokens; +using Fido2NetLib; using IdentityServer4.Models; using IdentityServer4.Validation; using Microsoft.AspNetCore.Identity; @@ -17,8 +20,9 @@ namespace Bit.Identity.IdentityServer; public class ExtensionGrantValidator : BaseRequestValidator, IExtensionGrantValidator { - private UserManager _userManager; - private readonly IDataProtectorTokenFactory _webAuthnLoginTokenizer; + public const string GrantType = "webauthn"; + + private readonly IDataProtectorTokenFactory _assertionOptionsDataProtector; public ExtensionGrantValidator( UserManager userManager, @@ -38,7 +42,7 @@ public ExtensionGrantValidator( IUserRepository userRepository, IPolicyService policyService, IDataProtectorTokenFactory tokenDataFactory, - IDataProtectorTokenFactory webAuthnLoginTokenizer, + IDataProtectorTokenFactory assertionOptionsDataProtector, IFeatureService featureService, IDistributedCache distributedCache ) @@ -47,24 +51,32 @@ IDistributedCache distributedCache applicationCacheService, mailService, logger, currentContext, globalSettings, userRepository, policyService, tokenDataFactory, featureService, ssoConfigRepository, distributedCache) { - _userManager = userManager; - _webAuthnLoginTokenizer = webAuthnLoginTokenizer; + _assertionOptionsDataProtector = assertionOptionsDataProtector; } - public string GrantType => "extension"; + string IExtensionGrantValidator.GrantType => "webauthn"; public async Task ValidateAsync(ExtensionGrantValidationContext context) { - 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 rawToken = context.Request.Raw.Get("token"); + var rawDeviceResponse = context.Request.Raw.Get("deviceResponse"); + if (string.IsNullOrWhiteSpace(rawToken) || string.IsNullOrWhiteSpace(rawDeviceResponse)) { context.Result = new GrantValidationResult(TokenRequestErrors.InvalidGrant); return; } - var user = await _userManager.FindByEmailAsync(email.ToLowerInvariant()); + var verified = _assertionOptionsDataProtector.TryUnprotect(rawToken, out var token) && + token.TokenIsValid(WebAuthnLoginAssertionOptionsScope.Authentication); + var deviceResponse = JsonSerializer.Deserialize(rawDeviceResponse); + + if (!verified) + { + context.Result = new GrantValidationResult(TokenRequestErrors.InvalidRequest); + return; + } + + var user = await _userService.CompleteWebAuthLoginAssertionAsync(token.Options, deviceResponse); var validatorContext = new CustomValidatorRequestContext { User = user, @@ -74,18 +86,15 @@ public async Task ValidateAsync(ExtensionGrantValidationContext context) await ValidateAsync(context, context.Request, validatorContext); } - protected override async Task ValidateContextAsync(ExtensionGrantValidationContext context, + protected override Task ValidateContextAsync(ExtensionGrantValidationContext context, 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) { - return false; + return Task.FromResult(false); } - var verified = _webAuthnLoginTokenizer.TryUnprotect(token, out var tokenData) && - tokenData.Valid && tokenData.TokenIsValid(validatorContext.User); - return verified; + + return Task.FromResult(true); } protected override Task SetSuccessResult(ExtensionGrantValidationContext context, User user, From 4f2ec30caf4c1e9546927636da628e247ace32bf Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Thu, 19 Oct 2023 16:59:08 +0200 Subject: [PATCH 28/50] [PM-2032] chore: rename to `WebAuthnGrantValidator` --- src/Identity/IdentityServer/ApiClient.cs | 2 +- .../{ExtensionGrantValidator.cs => WebAuthnGrantValidator.cs} | 4 ++-- src/Identity/Utilities/ServiceCollectionExtensions.cs | 2 +- test/Identity.IntegrationTest/openid-configuration.json | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) rename src/Identity/IdentityServer/{ExtensionGrantValidator.cs => WebAuthnGrantValidator.cs} (97%) diff --git a/src/Identity/IdentityServer/ApiClient.cs b/src/Identity/IdentityServer/ApiClient.cs index ddeec9584180..7457a8d0e97b 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, ExtensionGrantValidator.GrantType }; + AllowedGrantTypes = new[] { GrantType.ResourceOwnerPassword, GrantType.AuthorizationCode, WebAuthnGrantValidator.GrantType }; RefreshTokenExpiration = TokenExpiration.Sliding; RefreshTokenUsage = TokenUsage.ReUse; SlidingRefreshTokenLifetime = 86400 * refreshTokenSlidingDays; diff --git a/src/Identity/IdentityServer/ExtensionGrantValidator.cs b/src/Identity/IdentityServer/WebAuthnGrantValidator.cs similarity index 97% rename from src/Identity/IdentityServer/ExtensionGrantValidator.cs rename to src/Identity/IdentityServer/WebAuthnGrantValidator.cs index afe4125c2ea8..45cf01942eb8 100644 --- a/src/Identity/IdentityServer/ExtensionGrantValidator.cs +++ b/src/Identity/IdentityServer/WebAuthnGrantValidator.cs @@ -18,13 +18,13 @@ namespace Bit.Identity.IdentityServer; -public class ExtensionGrantValidator : BaseRequestValidator, IExtensionGrantValidator +public class WebAuthnGrantValidator : BaseRequestValidator, IExtensionGrantValidator { public const string GrantType = "webauthn"; private readonly IDataProtectorTokenFactory _assertionOptionsDataProtector; - public ExtensionGrantValidator( + public WebAuthnGrantValidator( UserManager userManager, IDeviceRepository deviceRepository, IDeviceService deviceService, diff --git a/src/Identity/Utilities/ServiceCollectionExtensions.cs b/src/Identity/Utilities/ServiceCollectionExtensions.cs index 87f9616546b8..3a582518ddad 100644 --- a/src/Identity/Utilities/ServiceCollectionExtensions.cs +++ b/src/Identity/Utilities/ServiceCollectionExtensions.cs @@ -45,7 +45,7 @@ public static IIdentityServerBuilder AddCustomIdentityServerServices(this IServi .AddPersistedGrantStore() .AddClientStore() .AddIdentityServerCertificate(env, globalSettings) - .AddExtensionGrantValidator(); + .AddExtensionGrantValidator(); services.AddTransient(); return identityServerBuilder; diff --git a/test/Identity.IntegrationTest/openid-configuration.json b/test/Identity.IntegrationTest/openid-configuration.json index 98b0331469a7..6897a828fb16 100644 --- a/test/Identity.IntegrationTest/openid-configuration.json +++ b/test/Identity.IntegrationTest/openid-configuration.json @@ -39,7 +39,7 @@ "implicit", "password", "urn:ietf:params:oauth:grant-type:device_code", - "extension" + "webauthn" ], "response_types_supported": [ "code", From 17ff52da43f0be0415f18ff605aef32d6d5cabb1 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Fri, 27 Oct 2023 09:37:20 +0200 Subject: [PATCH 29/50] [PM-2032] fix: add missing subsitute --- test/Identity.Test/Controllers/AccountsControllerTests.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/Identity.Test/Controllers/AccountsControllerTests.cs b/test/Identity.Test/Controllers/AccountsControllerTests.cs index 32473593dc2b..3d94fd54ba09 100644 --- a/test/Identity.Test/Controllers/AccountsControllerTests.cs +++ b/test/Identity.Test/Controllers/AccountsControllerTests.cs @@ -1,4 +1,5 @@ using Bit.Core.Auth.Models.Api.Request.Accounts; +using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Auth.Services; using Bit.Core.Entities; using Bit.Core.Enums; @@ -6,6 +7,7 @@ using Bit.Core.Models.Data; using Bit.Core.Repositories; using Bit.Core.Services; +using Bit.Core.Tokens; using Bit.Identity.Controllers; using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.Logging; @@ -22,6 +24,7 @@ public class AccountsControllerTests : IDisposable private readonly IUserRepository _userRepository; private readonly IUserService _userService; private readonly ICaptchaValidationService _captchaValidationService; + private readonly IDataProtectorTokenFactory _assertionOptionsDataProtector; public AccountsControllerTests() { @@ -29,11 +32,13 @@ public AccountsControllerTests() _userRepository = Substitute.For(); _userService = Substitute.For(); _captchaValidationService = Substitute.For(); + _assertionOptionsDataProtector = Substitute.For>(); _sut = new AccountsController( _logger, _userRepository, _userService, - _captchaValidationService + _captchaValidationService, + _assertionOptionsDataProtector ); } From 15a5e063f33b90b7594595e63d2300b9b7e970a4 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Fri, 27 Oct 2023 09:50:33 +0200 Subject: [PATCH 30/50] [PM-2032] feat: start adding builder --- .../UserDecryptionOptionsBuilder.cs | 117 ++++++++++++++++++ .../UserDecryptionOptionsBuilderTests.cs | 36 ++++++ 2 files changed, 153 insertions(+) create mode 100644 src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs create mode 100644 test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs diff --git a/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs b/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs new file mode 100644 index 000000000000..41e6e2953931 --- /dev/null +++ b/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs @@ -0,0 +1,117 @@ +using Bit.Core; +using Bit.Core.Auth.Entities; +using Bit.Core.Auth.Enums; +using Bit.Core.Auth.Models.Api.Response; +using Bit.Core.Context; +using Bit.Core.Entities; +using Bit.Core.Services; +using Bit.Identity.Utilities; +using System.Security.Claims; + +namespace Bit.Identity.IdentityServer; + +/// +/// Used to create a list of all possible ways the newly authenticated user can decrypt their vault contents +/// +public class UserDecryptionOptionsBuilder +{ + private readonly UserDecryptionOptions _options = new UserDecryptionOptions(); + + public UserDecryptionOptionsBuilder() + { + } + + public UserDecryptionOptionsBuilder ForUser(User user) + { + _options.HasMasterPassword = user.HasMasterPassword(); + return this; + } + + public UserDecryptionOptions Build() + { + return _options; + } + + //private async Task CreateUserDecryptionOptionsAsync(User user, Device device, ClaimsPrincipal subject) + //{ + // var ssoConfiguration = await GetSsoConfigurationDataAsync(subject); + + // var userDecryptionOption = new UserDecryptionOptions + // { + // HasMasterPassword = !string.IsNullOrEmpty(user.MasterPassword) + // }; + + // var ssoConfigurationData = ssoConfiguration?.GetData(); + + // if (ssoConfigurationData is { MemberDecryptionType: MemberDecryptionType.KeyConnector } && !string.IsNullOrEmpty(ssoConfigurationData.KeyConnectorUrl)) + // { + // // KeyConnector makes it mutually exclusive + // userDecryptionOption.KeyConnectorOption = new KeyConnectorUserDecryptionOption(ssoConfigurationData.KeyConnectorUrl); + // return userDecryptionOption; + // } + + // // Only add the trusted device specific option when the flag is turned on + // if (FeatureService.IsEnabled(FeatureFlagKeys.TrustedDeviceEncryption, CurrentContext) && ssoConfigurationData is { MemberDecryptionType: MemberDecryptionType.TrustedDeviceEncryption }) + // { + // string? encryptedPrivateKey = null; + // string? encryptedUserKey = null; + // if (device.IsTrusted()) + // { + // encryptedPrivateKey = device.EncryptedPrivateKey; + // encryptedUserKey = device.EncryptedUserKey; + // } + + // var allDevices = await _deviceRepository.GetManyByUserIdAsync(user.Id); + // // Checks if the current user has any devices that are capable of approving login with device requests except for + // // their current device. + // // NOTE: this doesn't check for if the users have configured the devices to be capable of approving requests as that is a client side setting. + // var hasLoginApprovingDevice = allDevices + // .Where(d => d.Identifier != device.Identifier && LoginApprovingDeviceTypes.Types.Contains(d.Type)) + // .Any(); + + // // Determine if user has manage reset password permission as post sso logic requires it for forcing users with this permission to set a MP + // var hasManageResetPasswordPermission = false; + + // // when a user is being created via JIT provisioning, they will not have any orgs so we can't assume we will have orgs here + // if (CurrentContext.Organizations.Any(o => o.Id == ssoConfiguration!.OrganizationId)) + // { + // // TDE requires single org so grabbing first org & id is fine. + // hasManageResetPasswordPermission = await CurrentContext.ManageResetPassword(ssoConfiguration!.OrganizationId); + // } + + // // If sso configuration data is not null then I know for sure that ssoConfiguration isn't null + // var organizationUser = await _organizationUserRepository.GetByOrganizationAsync(ssoConfiguration!.OrganizationId, user.Id); + + // // They are only able to be approved by an admin if they have enrolled is reset password + // var hasAdminApproval = !string.IsNullOrEmpty(organizationUser.ResetPasswordKey); + + // // TrustedDeviceEncryption only exists for SSO, but if that ever changes this value won't always be true + // userDecryptionOption.TrustedDeviceOption = new TrustedDeviceUserDecryptionOption( + // hasAdminApproval, + // hasLoginApprovingDevice, + // hasManageResetPasswordPermission, + // encryptedPrivateKey, + // encryptedUserKey); + // } + + // return userDecryptionOption; + //} + + //private async Task GetSsoConfigurationDataAsync(ClaimsPrincipal subject) + //{ + // var organizationClaim = subject?.FindFirstValue("organizationId"); + + // if (organizationClaim == null || !Guid.TryParse(organizationClaim, out var organizationId)) + // { + // return null; + // } + + // var ssoConfig = await SsoConfigRepository.GetByOrganizationIdAsync(organizationId); + // if (ssoConfig == null) + // { + // return null; + // } + + // return ssoConfig; + //} +} \ No newline at end of file diff --git a/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs b/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs new file mode 100644 index 000000000000..87708728aa92 --- /dev/null +++ b/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs @@ -0,0 +1,36 @@ +using Bit.Core.Entities; +using Bit.Identity.IdentityServer; +using Bit.Test.Common.AutoFixture.Attributes; +using Xunit; + +namespace Bit.Identity.Test.IdentityServer; + +public class UserDecryptionOptionsBuilderTests +{ + private readonly UserDecryptionOptionsBuilder _builder; + + public UserDecryptionOptionsBuilderTests() + { + _builder = new UserDecryptionOptionsBuilder(); + } + + [Theory, BitAutoData] + public void ForUser_WhenUserHasMasterPassword_ShouldReturnMasterPasswordOption(User user) + { + user.MasterPassword = "password"; + + var result = _builder.ForUser(user).Build(); + + Assert.True(result.HasMasterPassword); + } + + [Theory, BitAutoData] + public void ForUser_WhenUserDoesNotHaveMasterPassword_ShouldNotReturnMasterPasswordOption(User user) + { + user.MasterPassword = null; + + var result = _builder.ForUser(user).Build(); + + Assert.False(result.HasMasterPassword); + } +} From 367f58deee111d8cd8307ee58fb086876cb7fea3 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Fri, 27 Oct 2023 10:15:42 +0200 Subject: [PATCH 31/50] [PM-2032] feat: add support for KeyConnector --- .../UserDecryptionOptionsBuilder.cs | 10 ++++++++++ .../UserDecryptionOptionsBuilderTests.cs | 17 ++++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs b/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs index 41e6e2953931..69294a2bcfb2 100644 --- a/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs +++ b/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs @@ -27,6 +27,16 @@ public UserDecryptionOptionsBuilder ForUser(User user) return this; } + public UserDecryptionOptionsBuilder WithSso(SsoConfig ssoConfig) + { + var ssoConfigurationData = ssoConfig.GetData(); + if (ssoConfigurationData is { MemberDecryptionType: MemberDecryptionType.KeyConnector } && !string.IsNullOrEmpty(ssoConfigurationData.KeyConnectorUrl)) + { + _options.KeyConnectorOption = new KeyConnectorUserDecryptionOption(ssoConfigurationData.KeyConnectorUrl); + } + return this; + } + public UserDecryptionOptions Build() { return _options; diff --git a/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs b/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs index 87708728aa92..ad7fc95fe7d1 100644 --- a/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs +++ b/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs @@ -1,4 +1,7 @@ -using Bit.Core.Entities; +using Bit.Core.Auth.Entities; +using Bit.Core.Auth.Enums; +using Bit.Core.Auth.Models.Data; +using Bit.Core.Entities; using Bit.Identity.IdentityServer; using Bit.Test.Common.AutoFixture.Attributes; using Xunit; @@ -33,4 +36,16 @@ public void ForUser_WhenUserDoesNotHaveMasterPassword_ShouldNotReturnMasterPassw Assert.False(result.HasMasterPassword); } + + [Theory, BitAutoData] + public void WithSso_WhenConfigHasKeyConnector_ShouldReturnKeyConnectorOptions(SsoConfig ssoConfig, SsoConfigurationData configurationData) + { + ssoConfig.Data = configurationData.Serialize(); + configurationData.MemberDecryptionType = MemberDecryptionType.KeyConnector; + + var result = _builder.WithSso(ssoConfig).Build(); + + Assert.NotNull(result.KeyConnectorOption); + Assert.Equal(configurationData.KeyConnectorUrl, result.KeyConnectorOption?.KeyConnectorUrl); + } } From f60131207702963466d6a87908fd269777dbc044 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Fri, 27 Oct 2023 11:28:35 +0200 Subject: [PATCH 32/50] [PM-2032] feat: add first version of TDE --- .../UserDecryptionOptionsBuilder.cs | 162 +++++++++--------- .../UserDecryptionOptionsBuilderTests.cs | 22 ++- 2 files changed, 98 insertions(+), 86 deletions(-) diff --git a/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs b/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs index 69294a2bcfb2..12196cfe4229 100644 --- a/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs +++ b/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs @@ -6,10 +6,13 @@ using Bit.Core.Entities; using Bit.Core.Services; using Bit.Identity.Utilities; +using Bit.Infrastructure.EntityFramework.Auth.Models; using System.Security.Claims; +using Bit.Core.Auth.Utilities; namespace Bit.Identity.IdentityServer; +#nullable enable /// /// Used to create a list of all possible ways the newly authenticated user can decrypt their vault contents /// @@ -17,6 +20,9 @@ public class UserDecryptionOptionsBuilder { private readonly UserDecryptionOptions _options = new UserDecryptionOptions(); + private Core.Auth.Entities.SsoConfig? _ssoConfig; + private Device? _device; + public UserDecryptionOptionsBuilder() { } @@ -27,101 +33,91 @@ public UserDecryptionOptionsBuilder ForUser(User user) return this; } - public UserDecryptionOptionsBuilder WithSso(SsoConfig ssoConfig) + public UserDecryptionOptionsBuilder WithSso(Core.Auth.Entities.SsoConfig ssoConfig) { var ssoConfigurationData = ssoConfig.GetData(); if (ssoConfigurationData is { MemberDecryptionType: MemberDecryptionType.KeyConnector } && !string.IsNullOrEmpty(ssoConfigurationData.KeyConnectorUrl)) { _options.KeyConnectorOption = new KeyConnectorUserDecryptionOption(ssoConfigurationData.KeyConnectorUrl); } + _ssoConfig = ssoConfig; + return this; + } + + public UserDecryptionOptionsBuilder WithDevice(Device device) + { + _device = device; return this; } public UserDecryptionOptions Build() { + BuildTrustedDeviceEncryption(); + return _options; } - //private async Task CreateUserDecryptionOptionsAsync(User user, Device device, ClaimsPrincipal subject) - //{ - // var ssoConfiguration = await GetSsoConfigurationDataAsync(subject); - - // var userDecryptionOption = new UserDecryptionOptions - // { - // HasMasterPassword = !string.IsNullOrEmpty(user.MasterPassword) - // }; - - // var ssoConfigurationData = ssoConfiguration?.GetData(); - - // if (ssoConfigurationData is { MemberDecryptionType: MemberDecryptionType.KeyConnector } && !string.IsNullOrEmpty(ssoConfigurationData.KeyConnectorUrl)) - // { - // // KeyConnector makes it mutually exclusive - // userDecryptionOption.KeyConnectorOption = new KeyConnectorUserDecryptionOption(ssoConfigurationData.KeyConnectorUrl); - // return userDecryptionOption; - // } - - // // Only add the trusted device specific option when the flag is turned on - // if (FeatureService.IsEnabled(FeatureFlagKeys.TrustedDeviceEncryption, CurrentContext) && ssoConfigurationData is { MemberDecryptionType: MemberDecryptionType.TrustedDeviceEncryption }) - // { - // string? encryptedPrivateKey = null; - // string? encryptedUserKey = null; - // if (device.IsTrusted()) - // { - // encryptedPrivateKey = device.EncryptedPrivateKey; - // encryptedUserKey = device.EncryptedUserKey; - // } - - // var allDevices = await _deviceRepository.GetManyByUserIdAsync(user.Id); - // // Checks if the current user has any devices that are capable of approving login with device requests except for - // // their current device. - // // NOTE: this doesn't check for if the users have configured the devices to be capable of approving requests as that is a client side setting. - // var hasLoginApprovingDevice = allDevices - // .Where(d => d.Identifier != device.Identifier && LoginApprovingDeviceTypes.Types.Contains(d.Type)) - // .Any(); - - // // Determine if user has manage reset password permission as post sso logic requires it for forcing users with this permission to set a MP - // var hasManageResetPasswordPermission = false; - - // // when a user is being created via JIT provisioning, they will not have any orgs so we can't assume we will have orgs here - // if (CurrentContext.Organizations.Any(o => o.Id == ssoConfiguration!.OrganizationId)) - // { - // // TDE requires single org so grabbing first org & id is fine. - // hasManageResetPasswordPermission = await CurrentContext.ManageResetPassword(ssoConfiguration!.OrganizationId); - // } - - // // If sso configuration data is not null then I know for sure that ssoConfiguration isn't null - // var organizationUser = await _organizationUserRepository.GetByOrganizationAsync(ssoConfiguration!.OrganizationId, user.Id); - - // // They are only able to be approved by an admin if they have enrolled is reset password - // var hasAdminApproval = !string.IsNullOrEmpty(organizationUser.ResetPasswordKey); - - // // TrustedDeviceEncryption only exists for SSO, but if that ever changes this value won't always be true - // userDecryptionOption.TrustedDeviceOption = new TrustedDeviceUserDecryptionOption( - // hasAdminApproval, - // hasLoginApprovingDevice, - // hasManageResetPasswordPermission, - // encryptedPrivateKey, - // encryptedUserKey); - // } - - // return userDecryptionOption; - //} - - //private async Task GetSsoConfigurationDataAsync(ClaimsPrincipal subject) - //{ - // var organizationClaim = subject?.FindFirstValue("organizationId"); - - // if (organizationClaim == null || !Guid.TryParse(organizationClaim, out var organizationId)) - // { - // return null; - // } - - // var ssoConfig = await SsoConfigRepository.GetByOrganizationIdAsync(organizationId); - // if (ssoConfig == null) - // { - // return null; - // } - - // return ssoConfig; - //} + private void BuildTrustedDeviceEncryption() + { + if (_device == null || _ssoConfig == null) + { + return; + } + + // TODO: Only add the trusted device specific option when the flag is turned on + + var ssoConfigurationData = _ssoConfig.GetData(); + if (ssoConfigurationData is not { MemberDecryptionType: MemberDecryptionType.TrustedDeviceEncryption }) + { + return; + } + + + string? encryptedPrivateKey = null; + string? encryptedUserKey = null; + if (_device.IsTrusted()) + { + encryptedPrivateKey = _device.EncryptedPrivateKey; + encryptedUserKey = _device.EncryptedUserKey; + } + + _options.TrustedDeviceOption = new TrustedDeviceUserDecryptionOption( + false, + false, + false, + encryptedPrivateKey, + encryptedUserKey); + + //var allDevices = await _deviceRepository.GetManyByUserIdAsync(user.Id); + //// Checks if the current user has any devices that are capable of approving login with device requests except for + //// their current device. + //// NOTE: this doesn't check for if the users have configured the devices to be capable of approving requests as that is a client side setting. + //var hasLoginApprovingDevice = allDevices + // .Where(d => d.Identifier != device.Identifier && LoginApprovingDeviceTypes.Types.Contains(d.Type)) + // .Any(); + + //// Determine if user has manage reset password permission as post sso logic requires it for forcing users with this permission to set a MP + //var hasManageResetPasswordPermission = false; + + //// when a user is being created via JIT provisioning, they will not have any orgs so we can't assume we will have orgs here + //if (CurrentContext.Organizations.Any(o => o.Id == ssoConfiguration!.OrganizationId)) + //{ + // // TDE requires single org so grabbing first org & id is fine. + // hasManageResetPasswordPermission = await CurrentContext.ManageResetPassword(ssoConfiguration!.OrganizationId); + //} + + //// If sso configuration data is not null then I know for sure that ssoConfiguration isn't null + //var organizationUser = await _organizationUserRepository.GetByOrganizationAsync(ssoConfiguration!.OrganizationId, user.Id); + + //// They are only able to be approved by an admin if they have enrolled is reset password + //var hasAdminApproval = !string.IsNullOrEmpty(organizationUser.ResetPasswordKey); + + //// TrustedDeviceEncryption only exists for SSO, but if that ever changes this value won't always be true + //_options.TrustedDeviceOption = new TrustedDeviceUserDecryptionOption( + // hasAdminApproval, + // hasLoginApprovingDevice, + // hasManageResetPasswordPermission, + // encryptedPrivateKey, + // encryptedUserKey); + } } \ No newline at end of file diff --git a/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs b/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs index ad7fc95fe7d1..8637ec24c606 100644 --- a/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs +++ b/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs @@ -38,14 +38,30 @@ public void ForUser_WhenUserDoesNotHaveMasterPassword_ShouldNotReturnMasterPassw } [Theory, BitAutoData] - public void WithSso_WhenConfigHasKeyConnector_ShouldReturnKeyConnectorOptions(SsoConfig ssoConfig, SsoConfigurationData configurationData) + public void WithSso_WhenKeyConnectorIsEnabled_ShouldReturnKeyConnectorOptions(SsoConfig ssoConfig, SsoConfigurationData configurationData) { - ssoConfig.Data = configurationData.Serialize(); configurationData.MemberDecryptionType = MemberDecryptionType.KeyConnector; + ssoConfig.Data = configurationData.Serialize(); var result = _builder.WithSso(ssoConfig).Build(); Assert.NotNull(result.KeyConnectorOption); - Assert.Equal(configurationData.KeyConnectorUrl, result.KeyConnectorOption?.KeyConnectorUrl); + Assert.Equal(configurationData.KeyConnectorUrl, result.KeyConnectorOption!.KeyConnectorUrl); + } + + [Theory, BitAutoData] + public void Build_WhenTrustedDeviceIsEnabledAndDeviceIsTrusted_ShouldReturnTrustedDeviceOptions(SsoConfig ssoConfig, SsoConfigurationData configurationData, Device device) + { + configurationData.MemberDecryptionType = MemberDecryptionType.TrustedDeviceEncryption; + ssoConfig.Data = configurationData.Serialize(); + device.EncryptedPrivateKey = "encryptedPrivateKey"; + device.EncryptedPublicKey = "encryptedPublicKey"; + device.EncryptedUserKey = "encryptedUserKey"; + + var result = _builder.WithSso(ssoConfig).WithDevice(device).Build(); + + Assert.NotNull(result.TrustedDeviceOption); + Assert.Equal(device.EncryptedPrivateKey, result.TrustedDeviceOption!.EncryptedPrivateKey); + Assert.Equal(device.EncryptedUserKey, result.TrustedDeviceOption!.EncryptedUserKey); } } From e79e29578e7523dbcaf4a01f345c89c60baca018 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Fri, 27 Oct 2023 13:38:00 +0200 Subject: [PATCH 33/50] [PM-2032] chore: refactor WithSso --- .../UserDecryptionOptionsBuilder.cs | 25 +++++++++++++------ .../UserDecryptionOptionsBuilderTests.cs | 2 +- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs b/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs index 12196cfe4229..4655cce872ef 100644 --- a/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs +++ b/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs @@ -9,6 +9,7 @@ using Bit.Infrastructure.EntityFramework.Auth.Models; using System.Security.Claims; using Bit.Core.Auth.Utilities; +using Amazon.Util; namespace Bit.Identity.IdentityServer; @@ -35,11 +36,6 @@ public UserDecryptionOptionsBuilder ForUser(User user) public UserDecryptionOptionsBuilder WithSso(Core.Auth.Entities.SsoConfig ssoConfig) { - var ssoConfigurationData = ssoConfig.GetData(); - if (ssoConfigurationData is { MemberDecryptionType: MemberDecryptionType.KeyConnector } && !string.IsNullOrEmpty(ssoConfigurationData.KeyConnectorUrl)) - { - _options.KeyConnectorOption = new KeyConnectorUserDecryptionOption(ssoConfigurationData.KeyConnectorUrl); - } _ssoConfig = ssoConfig; return this; } @@ -52,12 +48,27 @@ public UserDecryptionOptionsBuilder WithDevice(Device device) public UserDecryptionOptions Build() { - BuildTrustedDeviceEncryption(); + BuildKeyConnectorOptions(); + BuildTrustedDeviceOptions(); return _options; } - private void BuildTrustedDeviceEncryption() + private void BuildKeyConnectorOptions() + { + if (_ssoConfig == null) + { + return; + } + + var ssoConfigurationData = _ssoConfig.GetData(); + if (ssoConfigurationData is { MemberDecryptionType: MemberDecryptionType.KeyConnector } && !string.IsNullOrEmpty(ssoConfigurationData.KeyConnectorUrl)) + { + _options.KeyConnectorOption = new KeyConnectorUserDecryptionOption(ssoConfigurationData.KeyConnectorUrl); + } + } + + private void BuildTrustedDeviceOptions() { if (_device == null || _ssoConfig == null) { diff --git a/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs b/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs index 8637ec24c606..2814ebc98b44 100644 --- a/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs +++ b/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs @@ -38,7 +38,7 @@ public void ForUser_WhenUserDoesNotHaveMasterPassword_ShouldNotReturnMasterPassw } [Theory, BitAutoData] - public void WithSso_WhenKeyConnectorIsEnabled_ShouldReturnKeyConnectorOptions(SsoConfig ssoConfig, SsoConfigurationData configurationData) + public void Build_WhenKeyConnectorIsEnabled_ShouldReturnKeyConnectorOptions(SsoConfig ssoConfig, SsoConfigurationData configurationData) { configurationData.MemberDecryptionType = MemberDecryptionType.KeyConnector; ssoConfig.Data = configurationData.Serialize(); From ec6e30acc13d45b0497227617d198a74c9681a8f Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Fri, 27 Oct 2023 13:58:21 +0200 Subject: [PATCH 34/50] [PM-2023] feat: add support for TDE feature flag --- .../UserDecryptionOptionsBuilder.cs | 13 +++++++-- .../UserDecryptionOptionsBuilderTests.cs | 29 +++++++++++++++++-- 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs b/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs index 4655cce872ef..b496edd040a3 100644 --- a/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs +++ b/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs @@ -19,13 +19,20 @@ namespace Bit.Identity.IdentityServer; /// public class UserDecryptionOptionsBuilder { - private readonly UserDecryptionOptions _options = new UserDecryptionOptions(); + private readonly ICurrentContext _currentContext; + private readonly IFeatureService _featureService; + private UserDecryptionOptions _options = new UserDecryptionOptions(); private Core.Auth.Entities.SsoConfig? _ssoConfig; private Device? _device; - public UserDecryptionOptionsBuilder() + public UserDecryptionOptionsBuilder( + ICurrentContext currentContext, + IFeatureService featureService + ) { + _currentContext = currentContext; + _featureService = featureService; } public UserDecryptionOptionsBuilder ForUser(User user) @@ -70,7 +77,7 @@ private void BuildKeyConnectorOptions() private void BuildTrustedDeviceOptions() { - if (_device == null || _ssoConfig == null) + if (_device == null || _ssoConfig == null || !_featureService.IsEnabled(FeatureFlagKeys.TrustedDeviceEncryption, _currentContext)) { return; } diff --git a/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs b/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs index 2814ebc98b44..1ee3b004c8bd 100644 --- a/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs +++ b/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs @@ -1,20 +1,28 @@ -using Bit.Core.Auth.Entities; +using Bit.Core; +using Bit.Core.Auth.Entities; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Models.Data; +using Bit.Core.Context; using Bit.Core.Entities; +using Bit.Core.Services; using Bit.Identity.IdentityServer; using Bit.Test.Common.AutoFixture.Attributes; +using NSubstitute; using Xunit; namespace Bit.Identity.Test.IdentityServer; public class UserDecryptionOptionsBuilderTests { + private readonly ICurrentContext _currentContext; + private readonly IFeatureService _featureService; private readonly UserDecryptionOptionsBuilder _builder; public UserDecryptionOptionsBuilderTests() { - _builder = new UserDecryptionOptionsBuilder(); + _currentContext = Substitute.For(); + _featureService = Substitute.For(); + _builder = new UserDecryptionOptionsBuilder(_currentContext, _featureService); } [Theory, BitAutoData] @@ -52,6 +60,7 @@ public void Build_WhenKeyConnectorIsEnabled_ShouldReturnKeyConnectorOptions(SsoC [Theory, BitAutoData] public void Build_WhenTrustedDeviceIsEnabledAndDeviceIsTrusted_ShouldReturnTrustedDeviceOptions(SsoConfig ssoConfig, SsoConfigurationData configurationData, Device device) { + _featureService.IsEnabled(FeatureFlagKeys.TrustedDeviceEncryption, _currentContext).Returns(true); configurationData.MemberDecryptionType = MemberDecryptionType.TrustedDeviceEncryption; ssoConfig.Data = configurationData.Serialize(); device.EncryptedPrivateKey = "encryptedPrivateKey"; @@ -64,4 +73,20 @@ public void Build_WhenTrustedDeviceIsEnabledAndDeviceIsTrusted_ShouldReturnTrust Assert.Equal(device.EncryptedPrivateKey, result.TrustedDeviceOption!.EncryptedPrivateKey); Assert.Equal(device.EncryptedUserKey, result.TrustedDeviceOption!.EncryptedUserKey); } + + // TODO: Remove when FeatureFlagKeys.TrustedDeviceEncryption is removed + [Theory, BitAutoData] + public void Build_WhenTrustedDeviceIsEnabledButFeatureFlagIsDisabled_ShouldNotReturnTrustedDeviceOptions(SsoConfig ssoConfig, SsoConfigurationData configurationData, Device device) + { + _featureService.IsEnabled(FeatureFlagKeys.TrustedDeviceEncryption, _currentContext).Returns(false); + configurationData.MemberDecryptionType = MemberDecryptionType.TrustedDeviceEncryption; + ssoConfig.Data = configurationData.Serialize(); + device.EncryptedPrivateKey = "encryptedPrivateKey"; + device.EncryptedPublicKey = "encryptedPublicKey"; + device.EncryptedUserKey = "encryptedUserKey"; + + var result = _builder.WithSso(ssoConfig).WithDevice(device).Build(); + + Assert.Null(result.TrustedDeviceOption); + } } From 6c04eee1922d188bb40edcb902406f686b345207 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Fri, 27 Oct 2023 14:27:03 +0200 Subject: [PATCH 35/50] [PM-2023] feat: add support for approving devices --- .../UserDecryptionOptionsBuilder.cs | 42 +++++++----- .../UserDecryptionOptionsBuilderTests.cs | 66 ++++++++++++++----- 2 files changed, 75 insertions(+), 33 deletions(-) diff --git a/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs b/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs index b496edd040a3..29f386bf8676 100644 --- a/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs +++ b/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs @@ -10,6 +10,7 @@ using System.Security.Claims; using Bit.Core.Auth.Utilities; using Amazon.Util; +using Bit.Core.Repositories; namespace Bit.Identity.IdentityServer; @@ -21,23 +22,31 @@ public class UserDecryptionOptionsBuilder { private readonly ICurrentContext _currentContext; private readonly IFeatureService _featureService; + private readonly IDeviceRepository _deviceRepository; + private readonly IOrganizationUserRepository _organizationUserRepository; private UserDecryptionOptions _options = new UserDecryptionOptions(); + private User? _user; private Core.Auth.Entities.SsoConfig? _ssoConfig; private Device? _device; public UserDecryptionOptionsBuilder( ICurrentContext currentContext, - IFeatureService featureService + IFeatureService featureService, + IDeviceRepository deviceRepository, + IOrganizationUserRepository organizationUserRepository ) { _currentContext = currentContext; _featureService = featureService; + _deviceRepository = deviceRepository; + _organizationUserRepository = organizationUserRepository; } public UserDecryptionOptionsBuilder ForUser(User user) { _options.HasMasterPassword = user.HasMasterPassword(); + _user = user; return this; } @@ -53,10 +62,10 @@ public UserDecryptionOptionsBuilder WithDevice(Device device) return this; } - public UserDecryptionOptions Build() + public async Task BuildAsync() { BuildKeyConnectorOptions(); - BuildTrustedDeviceOptions(); + await BuildTrustedDeviceOptions(); return _options; } @@ -75,22 +84,19 @@ private void BuildKeyConnectorOptions() } } - private void BuildTrustedDeviceOptions() + private async Task BuildTrustedDeviceOptions() { if (_device == null || _ssoConfig == null || !_featureService.IsEnabled(FeatureFlagKeys.TrustedDeviceEncryption, _currentContext)) { return; } - // TODO: Only add the trusted device specific option when the flag is turned on - var ssoConfigurationData = _ssoConfig.GetData(); if (ssoConfigurationData is not { MemberDecryptionType: MemberDecryptionType.TrustedDeviceEncryption }) { return; } - string? encryptedPrivateKey = null; string? encryptedUserKey = null; if (_device.IsTrusted()) @@ -99,21 +105,25 @@ private void BuildTrustedDeviceOptions() encryptedUserKey = _device.EncryptedUserKey; } + var hasLoginApprovingDevice = false; + if (_user != null) + { + var allDevices = await _deviceRepository.GetManyByUserIdAsync(_user.Id); + // Checks if the current user has any devices that are capable of approving login with device requests except for + // their current device. + // NOTE: this doesn't check for if the users have configured the devices to be capable of approving requests as that is a client side setting. + hasLoginApprovingDevice = allDevices + .Where(d => d.Identifier != _device.Identifier && LoginApprovingDeviceTypes.Types.Contains(d.Type)) + .Any(); + } + _options.TrustedDeviceOption = new TrustedDeviceUserDecryptionOption( false, - false, + hasLoginApprovingDevice, false, encryptedPrivateKey, encryptedUserKey); - //var allDevices = await _deviceRepository.GetManyByUserIdAsync(user.Id); - //// Checks if the current user has any devices that are capable of approving login with device requests except for - //// their current device. - //// NOTE: this doesn't check for if the users have configured the devices to be capable of approving requests as that is a client side setting. - //var hasLoginApprovingDevice = allDevices - // .Where(d => d.Identifier != device.Identifier && LoginApprovingDeviceTypes.Types.Contains(d.Type)) - // .Any(); - //// Determine if user has manage reset password permission as post sso logic requires it for forcing users with this permission to set a MP //var hasManageResetPasswordPermission = false; diff --git a/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs b/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs index 1ee3b004c8bd..e4d7db8868bd 100644 --- a/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs +++ b/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs @@ -1,11 +1,14 @@ using Bit.Core; using Bit.Core.Auth.Entities; using Bit.Core.Auth.Enums; +using Bit.Core.Auth.Models.Api.Response; using Bit.Core.Auth.Models.Data; using Bit.Core.Context; using Bit.Core.Entities; +using Bit.Core.Repositories; using Bit.Core.Services; using Bit.Identity.IdentityServer; +using Bit.Identity.Utilities; using Bit.Test.Common.AutoFixture.Attributes; using NSubstitute; using Xunit; @@ -16,77 +19,106 @@ public class UserDecryptionOptionsBuilderTests { private readonly ICurrentContext _currentContext; private readonly IFeatureService _featureService; + private readonly IDeviceRepository _deviceRepository; + private readonly IOrganizationUserRepository _organizationUserRepository; private readonly UserDecryptionOptionsBuilder _builder; public UserDecryptionOptionsBuilderTests() { _currentContext = Substitute.For(); _featureService = Substitute.For(); - _builder = new UserDecryptionOptionsBuilder(_currentContext, _featureService); + _deviceRepository = Substitute.For(); + _organizationUserRepository = Substitute.For(); + _builder = new UserDecryptionOptionsBuilder(_currentContext, _featureService, _deviceRepository, _organizationUserRepository); } [Theory, BitAutoData] - public void ForUser_WhenUserHasMasterPassword_ShouldReturnMasterPasswordOption(User user) + public async Task ForUser_WhenUserHasMasterPassword_ShouldReturnMasterPasswordOption(User user) { user.MasterPassword = "password"; - var result = _builder.ForUser(user).Build(); + var result = await _builder.ForUser(user).BuildAsync(); Assert.True(result.HasMasterPassword); } [Theory, BitAutoData] - public void ForUser_WhenUserDoesNotHaveMasterPassword_ShouldNotReturnMasterPasswordOption(User user) + public async Task ForUser_WhenUserDoesNotHaveMasterPassword_ShouldNotReturnMasterPasswordOption(User user) { user.MasterPassword = null; - var result = _builder.ForUser(user).Build(); + var result = await _builder.ForUser(user).BuildAsync(); Assert.False(result.HasMasterPassword); } [Theory, BitAutoData] - public void Build_WhenKeyConnectorIsEnabled_ShouldReturnKeyConnectorOptions(SsoConfig ssoConfig, SsoConfigurationData configurationData) + public async Task Build_WhenKeyConnectorIsEnabled_ShouldReturnKeyConnectorOptions(SsoConfig ssoConfig, SsoConfigurationData configurationData) { configurationData.MemberDecryptionType = MemberDecryptionType.KeyConnector; ssoConfig.Data = configurationData.Serialize(); - var result = _builder.WithSso(ssoConfig).Build(); + var result = await _builder.WithSso(ssoConfig).BuildAsync(); Assert.NotNull(result.KeyConnectorOption); Assert.Equal(configurationData.KeyConnectorUrl, result.KeyConnectorOption!.KeyConnectorUrl); } [Theory, BitAutoData] - public void Build_WhenTrustedDeviceIsEnabledAndDeviceIsTrusted_ShouldReturnTrustedDeviceOptions(SsoConfig ssoConfig, SsoConfigurationData configurationData, Device device) + public async Task Build_WhenTrustedDeviceIsEnabled_ShouldReturnTrustedDeviceOptions(SsoConfig ssoConfig, SsoConfigurationData configurationData, Device device) { _featureService.IsEnabled(FeatureFlagKeys.TrustedDeviceEncryption, _currentContext).Returns(true); configurationData.MemberDecryptionType = MemberDecryptionType.TrustedDeviceEncryption; ssoConfig.Data = configurationData.Serialize(); - device.EncryptedPrivateKey = "encryptedPrivateKey"; - device.EncryptedPublicKey = "encryptedPublicKey"; - device.EncryptedUserKey = "encryptedUserKey"; - var result = _builder.WithSso(ssoConfig).WithDevice(device).Build(); + var result = await _builder.WithSso(ssoConfig).WithDevice(device).BuildAsync(); Assert.NotNull(result.TrustedDeviceOption); - Assert.Equal(device.EncryptedPrivateKey, result.TrustedDeviceOption!.EncryptedPrivateKey); - Assert.Equal(device.EncryptedUserKey, result.TrustedDeviceOption!.EncryptedUserKey); + Assert.False(result.TrustedDeviceOption!.HasAdminApproval); + Assert.False(result.TrustedDeviceOption!.HasLoginApprovingDevice); + Assert.False(result.TrustedDeviceOption!.HasManageResetPasswordPermission); } // TODO: Remove when FeatureFlagKeys.TrustedDeviceEncryption is removed [Theory, BitAutoData] - public void Build_WhenTrustedDeviceIsEnabledButFeatureFlagIsDisabled_ShouldNotReturnTrustedDeviceOptions(SsoConfig ssoConfig, SsoConfigurationData configurationData, Device device) + public async Task Build_WhenTrustedDeviceIsEnabledButFeatureFlagIsDisabled_ShouldNotReturnTrustedDeviceOptions(SsoConfig ssoConfig, SsoConfigurationData configurationData, Device device) { _featureService.IsEnabled(FeatureFlagKeys.TrustedDeviceEncryption, _currentContext).Returns(false); configurationData.MemberDecryptionType = MemberDecryptionType.TrustedDeviceEncryption; ssoConfig.Data = configurationData.Serialize(); + + var result = await _builder.WithSso(ssoConfig).WithDevice(device).BuildAsync(); + + Assert.Null(result.TrustedDeviceOption); + } + + [Theory, BitAutoData] + public async Task Build_WhenDeviceIsTrusted_ShouldReturnKeys(SsoConfig ssoConfig, SsoConfigurationData configurationData, Device device) + { + _featureService.IsEnabled(FeatureFlagKeys.TrustedDeviceEncryption, _currentContext).Returns(true); + configurationData.MemberDecryptionType = MemberDecryptionType.TrustedDeviceEncryption; + ssoConfig.Data = configurationData.Serialize(); device.EncryptedPrivateKey = "encryptedPrivateKey"; device.EncryptedPublicKey = "encryptedPublicKey"; device.EncryptedUserKey = "encryptedUserKey"; - var result = _builder.WithSso(ssoConfig).WithDevice(device).Build(); + var result = await _builder.WithSso(ssoConfig).WithDevice(device).BuildAsync(); - Assert.Null(result.TrustedDeviceOption); + Assert.Equal(device.EncryptedPrivateKey, result.TrustedDeviceOption?.EncryptedPrivateKey); + Assert.Equal(device.EncryptedUserKey, result.TrustedDeviceOption?.EncryptedUserKey); + } + + [Theory, BitAutoData] + public async Task Build_WhenHasLoginApprovingDevice_ShouldApprovingDeviceTrue(SsoConfig ssoConfig, SsoConfigurationData configurationData, User user, Device device, Device approvingDevice) + { + _featureService.IsEnabled(FeatureFlagKeys.TrustedDeviceEncryption, _currentContext).Returns(true); + configurationData.MemberDecryptionType = MemberDecryptionType.TrustedDeviceEncryption; + ssoConfig.Data = configurationData.Serialize(); + approvingDevice.Type = LoginApprovingDeviceTypes.Types.First(); + _deviceRepository.GetManyByUserIdAsync(user.Id).Returns(new Device[] { approvingDevice }); + + var result = await _builder.ForUser(user).WithSso(ssoConfig).WithDevice(device).BuildAsync(); + + Assert.True(result.TrustedDeviceOption?.HasLoginApprovingDevice); } } From fa0bdf62268ea9055c4aa3fe3d1dd5201a4ed020 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Fri, 27 Oct 2023 14:38:29 +0200 Subject: [PATCH 36/50] [PM-2023] feat: add support for hasManageResetPasswordPermission --- .../UserDecryptionOptionsBuilder.cs | 22 +++++++++---------- .../UserDecryptionOptionsBuilderTests.cs | 20 ++++++++++++++++- 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs b/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs index 29f386bf8676..cf034dc4683d 100644 --- a/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs +++ b/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs @@ -117,23 +117,23 @@ private async Task BuildTrustedDeviceOptions() .Any(); } + // Determine if user has manage reset password permission as post sso logic requires it for forcing users with this permission to set a MP + var hasManageResetPasswordPermission = false; + + // when a user is being created via JIT provisioning, they will not have any orgs so we can't assume we will have orgs here + if (_currentContext.Organizations != null && _currentContext.Organizations.Any(o => o.Id == _ssoConfig.OrganizationId)) + { + // TDE requires single org so grabbing first org & id is fine. + hasManageResetPasswordPermission = await _currentContext.ManageResetPassword(_ssoConfig!.OrganizationId); + } + _options.TrustedDeviceOption = new TrustedDeviceUserDecryptionOption( false, hasLoginApprovingDevice, - false, + hasManageResetPasswordPermission, encryptedPrivateKey, encryptedUserKey); - //// Determine if user has manage reset password permission as post sso logic requires it for forcing users with this permission to set a MP - //var hasManageResetPasswordPermission = false; - - //// when a user is being created via JIT provisioning, they will not have any orgs so we can't assume we will have orgs here - //if (CurrentContext.Organizations.Any(o => o.Id == ssoConfiguration!.OrganizationId)) - //{ - // // TDE requires single org so grabbing first org & id is fine. - // hasManageResetPasswordPermission = await CurrentContext.ManageResetPassword(ssoConfiguration!.OrganizationId); - //} - //// If sso configuration data is not null then I know for sure that ssoConfiguration isn't null //var organizationUser = await _organizationUserRepository.GetByOrganizationAsync(ssoConfiguration!.OrganizationId, user.Id); diff --git a/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs b/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs index e4d7db8868bd..7bbce5387319 100644 --- a/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs +++ b/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs @@ -1,7 +1,6 @@ using Bit.Core; using Bit.Core.Auth.Entities; using Bit.Core.Auth.Enums; -using Bit.Core.Auth.Models.Api.Response; using Bit.Core.Auth.Models.Data; using Bit.Core.Context; using Bit.Core.Entities; @@ -121,4 +120,23 @@ public async Task Build_WhenHasLoginApprovingDevice_ShouldApprovingDeviceTrue(Ss Assert.True(result.TrustedDeviceOption?.HasLoginApprovingDevice); } + + [Theory, BitAutoData] + public async Task Build_WhenManageResetPasswordPermissions_ShouldHasManageResetPasswordPermissionTrue( + SsoConfig ssoConfig, + SsoConfigurationData configurationData, + CurrentContextOrganization organization, + Device device) + { + _featureService.IsEnabled(FeatureFlagKeys.TrustedDeviceEncryption, _currentContext).Returns(true); + configurationData.MemberDecryptionType = MemberDecryptionType.TrustedDeviceEncryption; + ssoConfig.Data = configurationData.Serialize(); + ssoConfig.OrganizationId = organization.Id; + _currentContext.Organizations.Returns(new List(new CurrentContextOrganization[] { organization })); + _currentContext.ManageResetPassword(organization.Id).Returns(true); + + var result = await _builder.WithSso(ssoConfig).WithDevice(device).BuildAsync(); + + Assert.True(result.TrustedDeviceOption?.HasManageResetPasswordPermission); + } } From a38f719fcb0d756f3961d14506b7852e78aa7c37 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Fri, 27 Oct 2023 14:50:11 +0200 Subject: [PATCH 37/50] [PM-2032] feat: add support for hasAdminApproval --- .../UserDecryptionOptionsBuilder.cs | 34 ++++++++----------- .../UserDecryptionOptionsBuilderTests.cs | 24 +++++++++++-- 2 files changed, 37 insertions(+), 21 deletions(-) diff --git a/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs b/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs index cf034dc4683d..15ab65390fa2 100644 --- a/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs +++ b/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs @@ -86,7 +86,8 @@ private void BuildKeyConnectorOptions() private async Task BuildTrustedDeviceOptions() { - if (_device == null || _ssoConfig == null || !_featureService.IsEnabled(FeatureFlagKeys.TrustedDeviceEncryption, _currentContext)) + // TrustedDeviceEncryption only exists for SSO, if that changes then these guards should change + if (_ssoConfig == null || !_featureService.IsEnabled(FeatureFlagKeys.TrustedDeviceEncryption, _currentContext)) { return; } @@ -99,14 +100,14 @@ private async Task BuildTrustedDeviceOptions() string? encryptedPrivateKey = null; string? encryptedUserKey = null; - if (_device.IsTrusted()) + if (_device != null && _device.IsTrusted()) { encryptedPrivateKey = _device.EncryptedPrivateKey; encryptedUserKey = _device.EncryptedUserKey; } var hasLoginApprovingDevice = false; - if (_user != null) + if (_device != null && _user != null) { var allDevices = await _deviceRepository.GetManyByUserIdAsync(_user.Id); // Checks if the current user has any devices that are capable of approving login with device requests except for @@ -119,7 +120,6 @@ private async Task BuildTrustedDeviceOptions() // Determine if user has manage reset password permission as post sso logic requires it for forcing users with this permission to set a MP var hasManageResetPasswordPermission = false; - // when a user is being created via JIT provisioning, they will not have any orgs so we can't assume we will have orgs here if (_currentContext.Organizations != null && _currentContext.Organizations.Any(o => o.Id == _ssoConfig.OrganizationId)) { @@ -127,25 +127,21 @@ private async Task BuildTrustedDeviceOptions() hasManageResetPasswordPermission = await _currentContext.ManageResetPassword(_ssoConfig!.OrganizationId); } + var hasAdminApproval = false; + if (_user != null) + { + // If sso configuration data is not null then I know for sure that ssoConfiguration isn't null + var organizationUser = await _organizationUserRepository.GetByOrganizationAsync(_ssoConfig.OrganizationId, _user.Id); + + // They are only able to be approved by an admin if they have enrolled is reset password + hasAdminApproval = organizationUser != null && !string.IsNullOrEmpty(organizationUser.ResetPasswordKey); + } + _options.TrustedDeviceOption = new TrustedDeviceUserDecryptionOption( - false, + hasAdminApproval, hasLoginApprovingDevice, hasManageResetPasswordPermission, encryptedPrivateKey, encryptedUserKey); - - //// If sso configuration data is not null then I know for sure that ssoConfiguration isn't null - //var organizationUser = await _organizationUserRepository.GetByOrganizationAsync(ssoConfiguration!.OrganizationId, user.Id); - - //// They are only able to be approved by an admin if they have enrolled is reset password - //var hasAdminApproval = !string.IsNullOrEmpty(organizationUser.ResetPasswordKey); - - //// TrustedDeviceEncryption only exists for SSO, but if that ever changes this value won't always be true - //_options.TrustedDeviceOption = new TrustedDeviceUserDecryptionOption( - // hasAdminApproval, - // hasLoginApprovingDevice, - // hasManageResetPasswordPermission, - // encryptedPrivateKey, - // encryptedUserKey); } } \ No newline at end of file diff --git a/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs b/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs index 7bbce5387319..65287312d8a5 100644 --- a/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs +++ b/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs @@ -1,4 +1,5 @@ -using Bit.Core; +using Amazon.Runtime.Internal; +using Bit.Core; using Bit.Core.Auth.Entities; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Models.Data; @@ -122,7 +123,7 @@ public async Task Build_WhenHasLoginApprovingDevice_ShouldApprovingDeviceTrue(Ss } [Theory, BitAutoData] - public async Task Build_WhenManageResetPasswordPermissions_ShouldHasManageResetPasswordPermissionTrue( + public async Task Build_WhenManageResetPasswordPermissions_ShouldReturnHasManageResetPasswordPermissionTrue( SsoConfig ssoConfig, SsoConfigurationData configurationData, CurrentContextOrganization organization, @@ -139,4 +140,23 @@ public async Task Build_WhenManageResetPasswordPermissions_ShouldHasManageResetP Assert.True(result.TrustedDeviceOption?.HasManageResetPasswordPermission); } + + [Theory, BitAutoData] + public async Task Build_WhenUserHasEnrolledIntoPasswordReset_ShouldReturnHasAdminApprovalTrue( + SsoConfig ssoConfig, + SsoConfigurationData configurationData, + OrganizationUser organizationUser, + User user, + Device device) + { + _featureService.IsEnabled(FeatureFlagKeys.TrustedDeviceEncryption, _currentContext).Returns(true); + configurationData.MemberDecryptionType = MemberDecryptionType.TrustedDeviceEncryption; + ssoConfig.Data = configurationData.Serialize(); + organizationUser.ResetPasswordKey = "resetPasswordKey"; + _organizationUserRepository.GetByOrganizationAsync(ssoConfig.OrganizationId, user.Id).Returns(organizationUser); + + var result = await _builder.ForUser(user).WithSso(ssoConfig).WithDevice(device).BuildAsync(); + + Assert.True(result.TrustedDeviceOption?.HasAdminApproval); + } } From f6077746290d23cfc66219d87cba728ea98a8677 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Fri, 27 Oct 2023 14:51:32 +0200 Subject: [PATCH 38/50] [PM-2032] chore: don't supply device if not necessary --- .../UserDecryptionOptionsBuilderTests.cs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs b/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs index 65287312d8a5..889572acb034 100644 --- a/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs +++ b/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs @@ -126,8 +126,7 @@ public async Task Build_WhenHasLoginApprovingDevice_ShouldApprovingDeviceTrue(Ss public async Task Build_WhenManageResetPasswordPermissions_ShouldReturnHasManageResetPasswordPermissionTrue( SsoConfig ssoConfig, SsoConfigurationData configurationData, - CurrentContextOrganization organization, - Device device) + CurrentContextOrganization organization) { _featureService.IsEnabled(FeatureFlagKeys.TrustedDeviceEncryption, _currentContext).Returns(true); configurationData.MemberDecryptionType = MemberDecryptionType.TrustedDeviceEncryption; @@ -136,7 +135,7 @@ public async Task Build_WhenManageResetPasswordPermissions_ShouldReturnHasManage _currentContext.Organizations.Returns(new List(new CurrentContextOrganization[] { organization })); _currentContext.ManageResetPassword(organization.Id).Returns(true); - var result = await _builder.WithSso(ssoConfig).WithDevice(device).BuildAsync(); + var result = await _builder.WithSso(ssoConfig).BuildAsync(); Assert.True(result.TrustedDeviceOption?.HasManageResetPasswordPermission); } @@ -146,8 +145,7 @@ public async Task Build_WhenUserHasEnrolledIntoPasswordReset_ShouldReturnHasAdmi SsoConfig ssoConfig, SsoConfigurationData configurationData, OrganizationUser organizationUser, - User user, - Device device) + User user) { _featureService.IsEnabled(FeatureFlagKeys.TrustedDeviceEncryption, _currentContext).Returns(true); configurationData.MemberDecryptionType = MemberDecryptionType.TrustedDeviceEncryption; @@ -155,7 +153,7 @@ public async Task Build_WhenUserHasEnrolledIntoPasswordReset_ShouldReturnHasAdmi organizationUser.ResetPasswordKey = "resetPasswordKey"; _organizationUserRepository.GetByOrganizationAsync(ssoConfig.OrganizationId, user.Id).Returns(organizationUser); - var result = await _builder.ForUser(user).WithSso(ssoConfig).WithDevice(device).BuildAsync(); + var result = await _builder.ForUser(user).WithSso(ssoConfig).BuildAsync(); Assert.True(result.TrustedDeviceOption?.HasAdminApproval); } From 1612e24c1ac5e2e79099e2e47a27e750fdfa70bd Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Fri, 27 Oct 2023 14:52:13 +0200 Subject: [PATCH 39/50] [PM-2032] chore: clean up imports --- src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs | 4 ---- .../IdentityServer/UserDecryptionOptionsBuilderTests.cs | 3 +-- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs b/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs index 15ab65390fa2..aad5946c8a3c 100644 --- a/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs +++ b/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs @@ -1,15 +1,11 @@ using Bit.Core; -using Bit.Core.Auth.Entities; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Models.Api.Response; using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Services; using Bit.Identity.Utilities; -using Bit.Infrastructure.EntityFramework.Auth.Models; -using System.Security.Claims; using Bit.Core.Auth.Utilities; -using Amazon.Util; using Bit.Core.Repositories; namespace Bit.Identity.IdentityServer; diff --git a/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs b/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs index 889572acb034..8b42be2d6aa5 100644 --- a/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs +++ b/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs @@ -1,5 +1,4 @@ -using Amazon.Runtime.Internal; -using Bit.Core; +using Bit.Core; using Bit.Core.Auth.Entities; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Models.Data; From 673f9fd41a83f4e55bd5693738fac09f022c1645 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Fri, 27 Oct 2023 15:10:48 +0200 Subject: [PATCH 40/50] [PM-2023] feat: extract interface --- .../IdentityServer/IUserDecryptionOptionsBuilder.cs | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 src/Identity/IdentityServer/IUserDecryptionOptionsBuilder.cs diff --git a/src/Identity/IdentityServer/IUserDecryptionOptionsBuilder.cs b/src/Identity/IdentityServer/IUserDecryptionOptionsBuilder.cs new file mode 100644 index 000000000000..6078d524dad8 --- /dev/null +++ b/src/Identity/IdentityServer/IUserDecryptionOptionsBuilder.cs @@ -0,0 +1,12 @@ +using Bit.Core.Auth.Entities; +using Bit.Core.Auth.Models.Api.Response; +using Bit.Core.Entities; + +namespace Bit.Identity.IdentityServer; +public interface IUserDecryptionOptionsBuilder +{ + Task BuildAsync(); + IUserDecryptionOptionsBuilder ForUser(User user); + IUserDecryptionOptionsBuilder WithDevice(Device device); + IUserDecryptionOptionsBuilder WithSso(SsoConfig ssoConfig); +} \ No newline at end of file From ed4b85cc0a40f9c1699c9e56fe280289e760ead5 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Fri, 27 Oct 2023 15:12:40 +0200 Subject: [PATCH 41/50] [PM-2023] chore: add clarifying comment --- .../IdentityServer/UserDecryptionOptionsBuilder.cs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs b/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs index aad5946c8a3c..4934f48f48ed 100644 --- a/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs +++ b/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs @@ -13,8 +13,10 @@ namespace Bit.Identity.IdentityServer; #nullable enable /// /// Used to create a list of all possible ways the newly authenticated user can decrypt their vault contents +/// +/// Note: Do not use this as an injected service if you intend to build multiple independent UserDecryptionOptions /// -public class UserDecryptionOptionsBuilder +public class UserDecryptionOptionsBuilder : IUserDecryptionOptionsBuilder { private readonly ICurrentContext _currentContext; private readonly IFeatureService _featureService; @@ -39,20 +41,20 @@ IOrganizationUserRepository organizationUserRepository _organizationUserRepository = organizationUserRepository; } - public UserDecryptionOptionsBuilder ForUser(User user) + public IUserDecryptionOptionsBuilder ForUser(User user) { _options.HasMasterPassword = user.HasMasterPassword(); _user = user; return this; } - public UserDecryptionOptionsBuilder WithSso(Core.Auth.Entities.SsoConfig ssoConfig) + public IUserDecryptionOptionsBuilder WithSso(Core.Auth.Entities.SsoConfig ssoConfig) { _ssoConfig = ssoConfig; return this; } - public UserDecryptionOptionsBuilder WithDevice(Device device) + public IUserDecryptionOptionsBuilder WithDevice(Device device) { _device = device; return this; From 03bb729a793c7325b0abf05b830391f0f73c1c48 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Fri, 27 Oct 2023 15:26:09 +0200 Subject: [PATCH 42/50] [PM-2023] feat: use new builder in production code --- .../IdentityServer/BaseRequestValidator.cs | 74 +++---------------- .../CustomTokenRequestValidator.cs | 5 +- .../ResourceOwnerPasswordValidator.cs | 5 +- .../IdentityServer/WebAuthnGrantValidator.cs | 5 +- .../Utilities/ServiceCollectionExtensions.cs | 1 + 5 files changed, 20 insertions(+), 70 deletions(-) diff --git a/src/Identity/IdentityServer/BaseRequestValidator.cs b/src/Identity/IdentityServer/BaseRequestValidator.cs index 7d6106ecb799..e4f99a479bf7 100644 --- a/src/Identity/IdentityServer/BaseRequestValidator.cs +++ b/src/Identity/IdentityServer/BaseRequestValidator.cs @@ -10,7 +10,6 @@ using Bit.Core.Auth.Models.Api.Response; using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Auth.Repositories; -using Bit.Core.Auth.Utilities; using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Enums; @@ -23,7 +22,6 @@ using Bit.Core.Settings; using Bit.Core.Tokens; using Bit.Core.Utilities; -using Bit.Identity.Utilities; using IdentityServer4.Validation; using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.Caching.Distributed; @@ -47,6 +45,7 @@ public abstract class BaseRequestValidator where T : class private readonly IDataProtectorTokenFactory _tokenDataFactory; private readonly IDistributedCache _distributedCache; private readonly DistributedCacheEntryOptions _cacheEntryOptions; + private readonly IUserDecryptionOptionsBuilder _userDecryptionOptionsBuilder; protected ICurrentContext CurrentContext { get; } protected IPolicyService PolicyService { get; } @@ -73,7 +72,8 @@ public BaseRequestValidator( IDataProtectorTokenFactory tokenDataFactory, IFeatureService featureService, ISsoConfigRepository ssoConfigRepository, - IDistributedCache distributedCache) + IDistributedCache distributedCache, + IUserDecryptionOptionsBuilder userDecryptionOptionsBuilder) { _userManager = userManager; _deviceRepository = deviceRepository; @@ -101,6 +101,7 @@ public BaseRequestValidator( // Email TOTP. AbsoluteExpirationRelativeToNow = new TimeSpan(0, 15, 0) }; + _userDecryptionOptionsBuilder = userDecryptionOptionsBuilder; } protected async Task ValidateAsync(T context, ValidatedTokenRequest request, @@ -612,67 +613,12 @@ private async Task GetMasterPasswordPolicy(Us /// private async Task CreateUserDecryptionOptionsAsync(User user, Device device, ClaimsPrincipal subject) { - var ssoConfiguration = await GetSsoConfigurationDataAsync(subject); - - var userDecryptionOption = new UserDecryptionOptions - { - HasMasterPassword = !string.IsNullOrEmpty(user.MasterPassword) - }; - - var ssoConfigurationData = ssoConfiguration?.GetData(); - - if (ssoConfigurationData is { MemberDecryptionType: MemberDecryptionType.KeyConnector } && !string.IsNullOrEmpty(ssoConfigurationData.KeyConnectorUrl)) - { - // KeyConnector makes it mutually exclusive - userDecryptionOption.KeyConnectorOption = new KeyConnectorUserDecryptionOption(ssoConfigurationData.KeyConnectorUrl); - return userDecryptionOption; - } - - // Only add the trusted device specific option when the flag is turned on - if (FeatureService.IsEnabled(FeatureFlagKeys.TrustedDeviceEncryption, CurrentContext) && ssoConfigurationData is { MemberDecryptionType: MemberDecryptionType.TrustedDeviceEncryption }) - { - string? encryptedPrivateKey = null; - string? encryptedUserKey = null; - if (device.IsTrusted()) - { - encryptedPrivateKey = device.EncryptedPrivateKey; - encryptedUserKey = device.EncryptedUserKey; - } - - var allDevices = await _deviceRepository.GetManyByUserIdAsync(user.Id); - // Checks if the current user has any devices that are capable of approving login with device requests except for - // their current device. - // NOTE: this doesn't check for if the users have configured the devices to be capable of approving requests as that is a client side setting. - var hasLoginApprovingDevice = allDevices - .Where(d => d.Identifier != device.Identifier && LoginApprovingDeviceTypes.Types.Contains(d.Type)) - .Any(); - - // Determine if user has manage reset password permission as post sso logic requires it for forcing users with this permission to set a MP - var hasManageResetPasswordPermission = false; - - // when a user is being created via JIT provisioning, they will not have any orgs so we can't assume we will have orgs here - if (CurrentContext.Organizations.Any(o => o.Id == ssoConfiguration!.OrganizationId)) - { - // TDE requires single org so grabbing first org & id is fine. - hasManageResetPasswordPermission = await CurrentContext.ManageResetPassword(ssoConfiguration!.OrganizationId); - } - - // If sso configuration data is not null then I know for sure that ssoConfiguration isn't null - var organizationUser = await _organizationUserRepository.GetByOrganizationAsync(ssoConfiguration!.OrganizationId, user.Id); - - // They are only able to be approved by an admin if they have enrolled is reset password - var hasAdminApproval = !string.IsNullOrEmpty(organizationUser.ResetPasswordKey); - - // TrustedDeviceEncryption only exists for SSO, but if that ever changes this value won't always be true - userDecryptionOption.TrustedDeviceOption = new TrustedDeviceUserDecryptionOption( - hasAdminApproval, - hasLoginApprovingDevice, - hasManageResetPasswordPermission, - encryptedPrivateKey, - encryptedUserKey); - } - - return userDecryptionOption; + var ssoConfig = await GetSsoConfigurationDataAsync(subject); + return await _userDecryptionOptionsBuilder + .ForUser(user) + .WithDevice(device) + .WithSso(ssoConfig) + .BuildAsync(); } private async Task GetSsoConfigurationDataAsync(ClaimsPrincipal subject) diff --git a/src/Identity/IdentityServer/CustomTokenRequestValidator.cs b/src/Identity/IdentityServer/CustomTokenRequestValidator.cs index a15a7383722b..00f563154ffe 100644 --- a/src/Identity/IdentityServer/CustomTokenRequestValidator.cs +++ b/src/Identity/IdentityServer/CustomTokenRequestValidator.cs @@ -44,12 +44,13 @@ public CustomTokenRequestValidator( IPolicyService policyService, IDataProtectorTokenFactory tokenDataFactory, IFeatureService featureService, - IDistributedCache distributedCache) + IDistributedCache distributedCache, + IUserDecryptionOptionsBuilder userDecryptionOptionsBuilder) : base(userManager, deviceRepository, deviceService, userService, eventService, organizationDuoWebTokenProvider, organizationRepository, organizationUserRepository, applicationCacheService, mailService, logger, currentContext, globalSettings, userRepository, policyService, tokenDataFactory, featureService, ssoConfigRepository, - distributedCache) + distributedCache, userDecryptionOptionsBuilder) { _userManager = userManager; } diff --git a/src/Identity/IdentityServer/ResourceOwnerPasswordValidator.cs b/src/Identity/IdentityServer/ResourceOwnerPasswordValidator.cs index 8b8b0be52dc3..2eb4e770110c 100644 --- a/src/Identity/IdentityServer/ResourceOwnerPasswordValidator.cs +++ b/src/Identity/IdentityServer/ResourceOwnerPasswordValidator.cs @@ -46,11 +46,12 @@ public ResourceOwnerPasswordValidator( IDataProtectorTokenFactory tokenDataFactory, IFeatureService featureService, ISsoConfigRepository ssoConfigRepository, - IDistributedCache distributedCache) + IDistributedCache distributedCache, + IUserDecryptionOptionsBuilder userDecryptionOptionsBuilder) : base(userManager, deviceRepository, deviceService, userService, eventService, organizationDuoWebTokenProvider, organizationRepository, organizationUserRepository, applicationCacheService, mailService, logger, currentContext, globalSettings, userRepository, policyService, - tokenDataFactory, featureService, ssoConfigRepository, distributedCache) + tokenDataFactory, featureService, ssoConfigRepository, distributedCache, userDecryptionOptionsBuilder) { _userManager = userManager; _userService = userService; diff --git a/src/Identity/IdentityServer/WebAuthnGrantValidator.cs b/src/Identity/IdentityServer/WebAuthnGrantValidator.cs index 45cf01942eb8..a4e1e74f68cf 100644 --- a/src/Identity/IdentityServer/WebAuthnGrantValidator.cs +++ b/src/Identity/IdentityServer/WebAuthnGrantValidator.cs @@ -44,12 +44,13 @@ public WebAuthnGrantValidator( IDataProtectorTokenFactory tokenDataFactory, IDataProtectorTokenFactory assertionOptionsDataProtector, IFeatureService featureService, - IDistributedCache distributedCache + IDistributedCache distributedCache, + IUserDecryptionOptionsBuilder userDecryptionOptionsBuilder ) : base(userManager, deviceRepository, deviceService, userService, eventService, organizationDuoWebTokenProvider, organizationRepository, organizationUserRepository, applicationCacheService, mailService, logger, currentContext, globalSettings, - userRepository, policyService, tokenDataFactory, featureService, ssoConfigRepository, distributedCache) + userRepository, policyService, tokenDataFactory, featureService, ssoConfigRepository, distributedCache, userDecryptionOptionsBuilder) { _assertionOptionsDataProtector = assertionOptionsDataProtector; } diff --git a/src/Identity/Utilities/ServiceCollectionExtensions.cs b/src/Identity/Utilities/ServiceCollectionExtensions.cs index 3a582518ddad..53b9c49e9062 100644 --- a/src/Identity/Utilities/ServiceCollectionExtensions.cs +++ b/src/Identity/Utilities/ServiceCollectionExtensions.cs @@ -17,6 +17,7 @@ public static IIdentityServerBuilder AddCustomIdentityServerServices(this IServi services.AddSingleton(); services.AddTransient(); + services.AddTransient(); var issuerUri = new Uri(globalSettings.BaseServiceUri.InternalIdentity); var identityServerBuilder = services From d62937aa324a24c04ecd596c24cba9baedd6244f Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Mon, 30 Oct 2023 10:43:49 +0100 Subject: [PATCH 43/50] [PM-2032] feat: add support for PRF --- .../Api/Response/UserDecryptionOptions.cs | 20 ++++++++++++++ src/Core/Services/IUserService.cs | 3 ++- .../Services/Implementations/UserService.cs | 14 +++++----- .../IdentityServer/BaseRequestValidator.cs | 6 ++--- .../IUserDecryptionOptionsBuilder.cs | 3 ++- .../UserDecryptionOptionsBuilder.cs | 10 +++++++ .../IdentityServer/WebAuthnGrantValidator.cs | 4 ++- .../UserDecryptionOptionsBuilderTests.cs | 26 +++++++++++++++++++ 8 files changed, 72 insertions(+), 14 deletions(-) diff --git a/src/Core/Auth/Models/Api/Response/UserDecryptionOptions.cs b/src/Core/Auth/Models/Api/Response/UserDecryptionOptions.cs index edfcce5a515a..7f97dc131a63 100644 --- a/src/Core/Auth/Models/Api/Response/UserDecryptionOptions.cs +++ b/src/Core/Auth/Models/Api/Response/UserDecryptionOptions.cs @@ -16,6 +16,12 @@ public UserDecryptionOptions() : base("userDecryptionOptions") /// public bool HasMasterPassword { get; set; } + /// + /// Gets or sets the WebAuthn PRF decryption keys. + /// + [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] + public WebAuthnPrfDecryptionOption? WebAuthnPrfOptions { get; set; } + /// /// Gets or sets information regarding this users trusted device decryption setup. /// @@ -29,6 +35,20 @@ public UserDecryptionOptions() : base("userDecryptionOptions") public KeyConnectorUserDecryptionOption? KeyConnectorOption { get; set; } } +public class WebAuthnPrfDecryptionOption +{ + public string EncryptedPrivateKey { get; } + public string EncryptedUserKey { get; } + + public WebAuthnPrfDecryptionOption( + string encryptedPrivateKey, + string encryptedUserKey) + { + EncryptedPrivateKey = encryptedPrivateKey; + EncryptedUserKey = encryptedUserKey; + } +} + public class TrustedDeviceUserDecryptionOption { public bool HasAdminApproval { get; } diff --git a/src/Core/Services/IUserService.cs b/src/Core/Services/IUserService.cs index 55ce0722ae8b..f453b1b6798e 100644 --- a/src/Core/Services/IUserService.cs +++ b/src/Core/Services/IUserService.cs @@ -1,4 +1,5 @@ using System.Security.Claims; +using Bit.Core.Auth.Entities; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Models; using Bit.Core.Entities; @@ -30,7 +31,7 @@ public interface IUserService Task StartWebAuthnLoginRegistrationAsync(User user); Task CompleteWebAuthLoginRegistrationAsync(User user, string name, bool supportsPrf, string encryptedUserKey, string encryptedPublicKey, string encryptedPrivateKey, CredentialCreateOptions options, AuthenticatorAttestationRawResponse attestationResponse); Task StartWebAuthnLoginAssertionAsync(); - Task CompleteWebAuthLoginAssertionAsync(AssertionOptions options, AuthenticatorAssertionRawResponse assertionResponse); + Task<(User, WebAuthnCredential)> CompleteWebAuthLoginAssertionAsync(AssertionOptions options, AuthenticatorAssertionRawResponse assertionResponse); 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 2fde3f0a6af5..e0759e8f808d 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -584,7 +584,7 @@ public Task StartWebAuthnLoginAssertionAsync() return Task.FromResult(options); } - public async Task CompleteWebAuthLoginAssertionAsync(AssertionOptions options, AuthenticatorAssertionRawResponse assertionResponse) + public async Task<(User, WebAuthnCredential)> CompleteWebAuthLoginAssertionAsync(AssertionOptions options, AuthenticatorAssertionRawResponse assertionResponse) { var userId = new Guid(assertionResponse.Response.UserHandle); var user = await _userRepository.GetByIdAsync(userId); @@ -594,7 +594,7 @@ public async Task CompleteWebAuthLoginAssertionAsync(AssertionOptions opti var credential = userCredentials.FirstOrDefault(c => c.CredentialId == assertionId); if (credential == null) { - return null; + throw new BadRequestException("Invalid credential."); } // TODO: Callback to ensure credential ID is unique. Do we care? I don't think so. @@ -607,14 +607,12 @@ public async Task CompleteWebAuthLoginAssertionAsync(AssertionOptions opti credential.Counter = (int)assertionVerificationResult.Counter; await _webAuthnCredentialRepository.ReplaceAsync(credential); - if (assertionVerificationResult.Status == "ok") - { - return user; - } - else + if (assertionVerificationResult.Status != "ok") { - return null; + throw new BadRequestException("Invalid credential."); } + + return (user, credential); } public async Task SendEmailVerificationAsync(User user) diff --git a/src/Identity/IdentityServer/BaseRequestValidator.cs b/src/Identity/IdentityServer/BaseRequestValidator.cs index e4f99a479bf7..57ab8ea8c7ff 100644 --- a/src/Identity/IdentityServer/BaseRequestValidator.cs +++ b/src/Identity/IdentityServer/BaseRequestValidator.cs @@ -45,13 +45,13 @@ public abstract class BaseRequestValidator where T : class private readonly IDataProtectorTokenFactory _tokenDataFactory; private readonly IDistributedCache _distributedCache; private readonly DistributedCacheEntryOptions _cacheEntryOptions; - private readonly IUserDecryptionOptionsBuilder _userDecryptionOptionsBuilder; protected ICurrentContext CurrentContext { get; } protected IPolicyService PolicyService { get; } protected IFeatureService FeatureService { get; } protected ISsoConfigRepository SsoConfigRepository { get; } protected IUserService _userService { get; } + protected IUserDecryptionOptionsBuilder UserDecryptionOptionsBuilder { get; } public BaseRequestValidator( UserManager userManager, @@ -101,7 +101,7 @@ public BaseRequestValidator( // Email TOTP. AbsoluteExpirationRelativeToNow = new TimeSpan(0, 15, 0) }; - _userDecryptionOptionsBuilder = userDecryptionOptionsBuilder; + UserDecryptionOptionsBuilder = userDecryptionOptionsBuilder; } protected async Task ValidateAsync(T context, ValidatedTokenRequest request, @@ -614,7 +614,7 @@ private async Task GetMasterPasswordPolicy(Us private async Task CreateUserDecryptionOptionsAsync(User user, Device device, ClaimsPrincipal subject) { var ssoConfig = await GetSsoConfigurationDataAsync(subject); - return await _userDecryptionOptionsBuilder + return await UserDecryptionOptionsBuilder .ForUser(user) .WithDevice(device) .WithSso(ssoConfig) diff --git a/src/Identity/IdentityServer/IUserDecryptionOptionsBuilder.cs b/src/Identity/IdentityServer/IUserDecryptionOptionsBuilder.cs index 6078d524dad8..12109619d3e9 100644 --- a/src/Identity/IdentityServer/IUserDecryptionOptionsBuilder.cs +++ b/src/Identity/IdentityServer/IUserDecryptionOptionsBuilder.cs @@ -5,8 +5,9 @@ namespace Bit.Identity.IdentityServer; public interface IUserDecryptionOptionsBuilder { - Task BuildAsync(); IUserDecryptionOptionsBuilder ForUser(User user); IUserDecryptionOptionsBuilder WithDevice(Device device); IUserDecryptionOptionsBuilder WithSso(SsoConfig ssoConfig); + IUserDecryptionOptionsBuilder WithWebAuthnLoginCredential(WebAuthnCredential credential); + Task BuildAsync(); } \ No newline at end of file diff --git a/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs b/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs index 4934f48f48ed..a7a468264730 100644 --- a/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs +++ b/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs @@ -7,6 +7,7 @@ using Bit.Identity.Utilities; using Bit.Core.Auth.Utilities; using Bit.Core.Repositories; +using Bit.Core.Auth.Entities; namespace Bit.Identity.IdentityServer; @@ -60,6 +61,15 @@ public IUserDecryptionOptionsBuilder WithDevice(Device device) return this; } + public IUserDecryptionOptionsBuilder WithWebAuthnLoginCredential(WebAuthnCredential credential) + { + if (credential.GetPrfStatus() == WebAuthnPrfStatus.Enabled) + { + _options.WebAuthnPrfOptions = new WebAuthnPrfDecryptionOption(credential.EncryptedPrivateKey, credential.EncryptedUserKey); + } + return this; + } + public async Task BuildAsync() { BuildKeyConnectorOptions(); diff --git a/src/Identity/IdentityServer/WebAuthnGrantValidator.cs b/src/Identity/IdentityServer/WebAuthnGrantValidator.cs index a4e1e74f68cf..310b421dee63 100644 --- a/src/Identity/IdentityServer/WebAuthnGrantValidator.cs +++ b/src/Identity/IdentityServer/WebAuthnGrantValidator.cs @@ -77,13 +77,15 @@ public async Task ValidateAsync(ExtensionGrantValidationContext context) return; } - var user = await _userService.CompleteWebAuthLoginAssertionAsync(token.Options, deviceResponse); + var (user, credential) = await _userService.CompleteWebAuthLoginAssertionAsync(token.Options, deviceResponse); var validatorContext = new CustomValidatorRequestContext { User = user, KnownDevice = await KnownDeviceAsync(user, context.Request) }; + UserDecryptionOptionsBuilder.WithWebAuthnLoginCredential(credential); + await ValidateAsync(context, context.Request, validatorContext); } diff --git a/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs b/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs index 8b42be2d6aa5..cc4c9a7f3848 100644 --- a/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs +++ b/test/Identity.Test/IdentityServer/UserDecryptionOptionsBuilderTests.cs @@ -51,6 +51,32 @@ public async Task ForUser_WhenUserDoesNotHaveMasterPassword_ShouldNotReturnMaste Assert.False(result.HasMasterPassword); } + [Theory, BitAutoData] + public async Task WithWebAuthnLoginCredential_WhenCredentialDoesNotContainPrfKeys_ShouldNotReturnPrfOption(WebAuthnCredential credential) + { + credential.EncryptedPrivateKey = null; + credential.EncryptedPublicKey = null; + credential.EncryptedUserKey = null; + + var result = await _builder.WithWebAuthnLoginCredential(credential).BuildAsync(); + + Assert.Null(result.WebAuthnPrfOptions); + } + + [Theory, BitAutoData] + public async Task WithWebAuthnLoginCredential_WhenCredentialContainsPrfKeys_ShouldReturnPrfOption(WebAuthnCredential credential) + { + credential.EncryptedPrivateKey = "encryptedPrivateKey"; + credential.EncryptedPublicKey = "encryptedPublicKey"; + credential.EncryptedUserKey = "encryptedUserKey"; + + var result = await _builder.WithWebAuthnLoginCredential(credential).BuildAsync(); + + Assert.NotNull(result.WebAuthnPrfOptions); + Assert.Equal(credential.EncryptedPrivateKey, result.WebAuthnPrfOptions!.EncryptedPrivateKey); + Assert.Equal(credential.EncryptedUserKey, result.WebAuthnPrfOptions!.EncryptedUserKey); + } + [Theory, BitAutoData] public async Task Build_WhenKeyConnectorIsEnabled_ShouldReturnKeyConnectorOptions(SsoConfig ssoConfig, SsoConfigurationData configurationData) { From cdabd17f982425d37d1e153da918468c59a3ca27 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Mon, 30 Oct 2023 10:48:17 +0100 Subject: [PATCH 44/50] [PM-2032] chore: clean-up todos --- .../Controllers/AccountsController.cs | 26 ------------------- 1 file changed, 26 deletions(-) diff --git a/src/Identity/Controllers/AccountsController.cs b/src/Identity/Controllers/AccountsController.cs index 5d2f77399643..bf2f3fec511d 100644 --- a/src/Identity/Controllers/AccountsController.cs +++ b/src/Identity/Controllers/AccountsController.cs @@ -13,7 +13,6 @@ using Bit.Core.Tokens; using Bit.Core.Utilities; using Bit.SharedWeb.Utilities; -using Fido2NetLib; using Microsoft.AspNetCore.Mvc; namespace Bit.Identity.Controllers; @@ -86,16 +85,8 @@ public async Task PostPrelogin([FromBody] PreloginRequest [ApiExplorerSettings(IgnoreApi = true)] // Disable Swagger due to CredentialCreateOptions not converting properly [RequireFeature(FeatureFlagKeys.PasswordlessLogin)] // TODO: Add tests - // TODO: Create proper models for this call public async Task PostWebAuthnLoginAssertionOptions(WebAuthnLoginAssertionOptionsRequestModel 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(); var tokenable = new WebAuthnLoginAssertionOptionsTokenable(WebAuthnLoginAssertionOptionsScope.Authentication, options); @@ -107,21 +98,4 @@ public async Task PostWebAuthnLoginA Token = token }; } - - // TODO: Remove, this will move to the grant validator - //[HttpPost("webauthn-assertion")] - //[RequireFeature(FeatureFlagKeys.PasswordlessLogin)] - //// 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 5c4b14f5013c4adf232a7640ca031fa054c19145 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Mon, 30 Oct 2023 10:51:09 +0100 Subject: [PATCH 45/50] [PM-2023] chore: remove token which is no longer used --- .../Tokenables/WebAuthnLoginTokenable.cs | 43 ------------------- .../Services/Implementations/UserService.cs | 7 +-- .../Utilities/ServiceCollectionExtensions.cs | 6 --- test/Core.Test/Services/UserServiceTests.cs | 3 +- 4 files changed, 2 insertions(+), 57 deletions(-) delete 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 deleted file mode 100644 index b27b1fb35538..000000000000 --- a/src/Core/Auth/Models/Business/Tokenables/WebAuthnLoginTokenable.cs +++ /dev/null @@ -1,43 +0,0 @@ -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); - } - - 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 e0759e8f808d..fe1a8f5c602c 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -3,7 +3,6 @@ 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; @@ -12,7 +11,6 @@ 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; @@ -60,7 +58,6 @@ public class UserService : UserManager, IUserService, IDisposable private readonly IProviderUserRepository _providerUserRepository; private readonly IStripeSyncService _stripeSyncService; private readonly IWebAuthnCredentialRepository _webAuthnCredentialRepository; - private readonly IDataProtectorTokenFactory _webAuthnLoginTokenizer; public UserService( IUserRepository userRepository, @@ -92,8 +89,7 @@ public UserService( IOrganizationService organizationService, IProviderUserRepository providerUserRepository, IStripeSyncService stripeSyncService, - IWebAuthnCredentialRepository webAuthnRepository, - IDataProtectorTokenFactory webAuthnLoginTokenizer) + IWebAuthnCredentialRepository webAuthnRepository) : base( store, optionsAccessor, @@ -131,7 +127,6 @@ public UserService( _providerUserRepository = providerUserRepository; _stripeSyncService = stripeSyncService; _webAuthnCredentialRepository = webAuthnRepository; - _webAuthnLoginTokenizer = webAuthnLoginTokenizer; } public Guid? GetProperUserId(ClaimsPrincipal principal) diff --git a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs index 0fd3a67c9824..497a7d4cbdf6 100644 --- a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs +++ b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs @@ -162,12 +162,6 @@ 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>>())); services.AddSingleton>(serviceProvider => new DataProtectorTokenFactory( WebAuthnCredentialCreateOptionsTokenable.ClearTextPrefix, diff --git a/test/Core.Test/Services/UserServiceTests.cs b/test/Core.Test/Services/UserServiceTests.cs index bca1b1deca53..a2f2aa4c9314 100644 --- a/test/Core.Test/Services/UserServiceTests.cs +++ b/test/Core.Test/Services/UserServiceTests.cs @@ -274,8 +274,7 @@ public async Task VerifySecretAsync_Works( sutProvider.GetDependency(), sutProvider.GetDependency(), sutProvider.GetDependency(), - sutProvider.GetDependency(), - sutProvider.GetDependency>() + sutProvider.GetDependency() ); var actualIsVerified = await sut.VerifySecretAsync(user, secret); From c5aedc91305cc380e39499e3f6e2f4f105dfdf0c Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Mon, 30 Oct 2023 11:00:45 +0100 Subject: [PATCH 46/50] [PM-2032] chore: remove todo --- src/Core/Services/Implementations/UserService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index fe1a8f5c602c..cb42974eddd6 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -592,7 +592,7 @@ public Task StartWebAuthnLoginAssertionAsync() throw new BadRequestException("Invalid credential."); } - // TODO: Callback to ensure credential ID is unique. Do we care? I don't think so. + // Always return true, since we've already filtered the credentials after user id IsUserHandleOwnerOfCredentialIdAsync callback = (args, cancellationToken) => Task.FromResult(true); var credentialPublicKey = CoreHelpers.Base64UrlDecode(credential.PublicKey); var assertionVerificationResult = await _fido2.MakeAssertionAsync( From 9cb7b57969da44db80a187e9b57eb1efa7c3f4bc Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Mon, 30 Oct 2023 11:45:12 +0100 Subject: [PATCH 47/50] [PM-2032] feat: improve assertion error handling --- src/Core/Auth/Utilities/GuidUtilities.cs | 17 ++++ .../Services/Implementations/UserService.cs | 15 ++- test/Core.Test/Services/UserServiceTests.cs | 94 ++++++++++++++++++- 3 files changed, 122 insertions(+), 4 deletions(-) create mode 100644 src/Core/Auth/Utilities/GuidUtilities.cs diff --git a/src/Core/Auth/Utilities/GuidUtilities.cs b/src/Core/Auth/Utilities/GuidUtilities.cs new file mode 100644 index 000000000000..503bf77005f8 --- /dev/null +++ b/src/Core/Auth/Utilities/GuidUtilities.cs @@ -0,0 +1,17 @@ +namespace Bit.Core.Auth.Utilities; + +public static class GuidUtilities +{ + public static bool TryParseBytes(ReadOnlySpan bytes, out Guid guid) + { + try + { + guid = new Guid(bytes); + return true; + } catch { + guid = Guid.Empty; + return false; + } + } +} + diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index cb42974eddd6..e3fe7a3222a1 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -4,6 +4,7 @@ using Bit.Core.Auth.Enums; using Bit.Core.Auth.Models; using Bit.Core.Auth.Repositories; +using Bit.Core.Auth.Utilities; using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Enums; @@ -581,12 +582,20 @@ public Task StartWebAuthnLoginAssertionAsync() public async Task<(User, WebAuthnCredential)> CompleteWebAuthLoginAssertionAsync(AssertionOptions options, AuthenticatorAssertionRawResponse assertionResponse) { - var userId = new Guid(assertionResponse.Response.UserHandle); + if (!GuidUtilities.TryParseBytes(assertionResponse.Response.UserHandle, out var userId)) + { + throw new BadRequestException("Invalid credential."); + } + var user = await _userRepository.GetByIdAsync(userId); + if (user == null) + { + throw new BadRequestException("Invalid credential."); + } var userCredentials = await _webAuthnCredentialRepository.GetManyByUserIdAsync(user.Id); - var assertionId = CoreHelpers.Base64UrlEncode(assertionResponse.Id); - var credential = userCredentials.FirstOrDefault(c => c.CredentialId == assertionId); + var assertedCredentialId = CoreHelpers.Base64UrlEncode(assertionResponse.Id); + var credential = userCredentials.FirstOrDefault(c => c.CredentialId == assertedCredentialId); if (credential == null) { throw new BadRequestException("Invalid credential."); diff --git a/test/Core.Test/Services/UserServiceTests.cs b/test/Core.Test/Services/UserServiceTests.cs index a2f2aa4c9314..e59006533385 100644 --- a/test/Core.Test/Services/UserServiceTests.cs +++ b/test/Core.Test/Services/UserServiceTests.cs @@ -1,4 +1,5 @@ -using System.Text.Json; +using System.Text; +using System.Text.Json; using AutoFixture; using Bit.Core.Auth.Entities; using Bit.Core.Auth.Enums; @@ -7,6 +8,7 @@ using Bit.Core.Auth.Repositories; using Bit.Core.Context; using Bit.Core.Entities; +using Bit.Core.Exceptions; using Bit.Core.Models.Business; using Bit.Core.Models.Data.Organizations; using Bit.Core.Repositories; @@ -14,18 +16,22 @@ using Bit.Core.Settings; using Bit.Core.Tokens; using Bit.Core.Tools.Services; +using Bit.Core.Utilities; using Bit.Core.Vault.Repositories; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; using Bit.Test.Common.Helpers; using Fido2NetLib; +using Fido2NetLib.Objects; using Microsoft.AspNetCore.DataProtection; using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using NSubstitute; using NSubstitute.ReceivedExtensions; +using NSubstitute.ReturnsExtensions; using Xunit; +using static Fido2NetLib.AuthenticatorAssertionRawResponse; namespace Bit.Core.Test.Services; @@ -199,6 +205,92 @@ public async void CompleteWebAuthLoginRegistrationAsync_ExceedsExistingCredentia sutProvider.GetDependency().DidNotReceive(); } + [Theory, BitAutoData] + public async void CompleteWebAuthLoginAssertionAsync_InvalidUserHandle_ThrowsBadRequestException(SutProvider sutProvider, AssertionOptions options, AuthenticatorAssertionRawResponse response) + { + // Arrange + response.Response.UserHandle = Encoding.UTF8.GetBytes("invalid-user-handle"); + + // Act + var result = async () => await sutProvider.Sut.CompleteWebAuthLoginAssertionAsync(options, response); + + // Assert + await Assert.ThrowsAsync(result); + } + + [Theory, BitAutoData] + public async void CompleteWebAuthLoginAssertionAsync_UserNotFound_ThrowsBadRequestException(SutProvider sutProvider, User user, AssertionOptions options, AuthenticatorAssertionRawResponse response) + { + // Arrange + response.Response.UserHandle = user.Id.ToByteArray(); + sutProvider.GetDependency().GetByIdAsync(user.Id).ReturnsNull(); + + // Act + var result = async () => await sutProvider.Sut.CompleteWebAuthLoginAssertionAsync(options, response); + + // Assert + await Assert.ThrowsAsync(result); + } + + [Theory, BitAutoData] + public async void CompleteWebAuthLoginAssertionAsync_NoMatchingCredentialExists_ThrowsBadRequestException(SutProvider sutProvider, User user, AssertionOptions options, AuthenticatorAssertionRawResponse response) + { + // Arrange + response.Response.UserHandle = user.Id.ToByteArray(); + sutProvider.GetDependency().GetByIdAsync(user.Id).Returns(user); + sutProvider.GetDependency().GetManyByUserIdAsync(user.Id).Returns(new WebAuthnCredential[] {}); + + // Act + var result = async () => await sutProvider.Sut.CompleteWebAuthLoginAssertionAsync(options, response); + + // Assert + await Assert.ThrowsAsync(result); + } + + [Theory, BitAutoData] + public async void CompleteWebAuthLoginAssertionAsync_AssertionFails_ThrowsBadRequestException(SutProvider sutProvider, User user, AssertionOptions options, AuthenticatorAssertionRawResponse response, WebAuthnCredential credential, AssertionVerificationResult assertionResult) + { + // Arrange + var credentialId = Guid.NewGuid().ToByteArray(); + credential.CredentialId = CoreHelpers.Base64UrlEncode(credentialId); + response.Id = credentialId; + response.Response.UserHandle = user.Id.ToByteArray(); + assertionResult.Status = "Not ok"; + sutProvider.GetDependency().GetByIdAsync(user.Id).Returns(user); + sutProvider.GetDependency().GetManyByUserIdAsync(user.Id).Returns(new WebAuthnCredential[] { credential }); + sutProvider.GetDependency().MakeAssertionAsync(response, options, Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(assertionResult); + + // Act + var result = async () => await sutProvider.Sut.CompleteWebAuthLoginAssertionAsync(options, response); + + // Assert + await Assert.ThrowsAsync(result); + } + + [Theory, BitAutoData] + public async void CompleteWebAuthLoginAssertionAsync_AssertionSucceeds_ReturnsUserAndCredential(SutProvider sutProvider, User user, AssertionOptions options, AuthenticatorAssertionRawResponse response, WebAuthnCredential credential, AssertionVerificationResult assertionResult) + { + // Arrange + var credentialId = Guid.NewGuid().ToByteArray(); + credential.CredentialId = CoreHelpers.Base64UrlEncode(credentialId); + response.Id = credentialId; + response.Response.UserHandle = user.Id.ToByteArray(); + assertionResult.Status = "ok"; + sutProvider.GetDependency().GetByIdAsync(user.Id).Returns(user); + sutProvider.GetDependency().GetManyByUserIdAsync(user.Id).Returns(new WebAuthnCredential[] { credential }); + sutProvider.GetDependency().MakeAssertionAsync(response, options, Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(assertionResult); + + // Act + var result = await sutProvider.Sut.CompleteWebAuthLoginAssertionAsync(options, response); + + // Assert + var (userResult, credentialResult) = result; + Assert.Equal(user, userResult); + Assert.Equal(credential, credentialResult); + } + [Flags] public enum ShouldCheck { From 6f7efaa950f9b4d183825422f415cc7d920ca022 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Mon, 30 Oct 2023 11:47:34 +0100 Subject: [PATCH 48/50] [PM-2032] fix: linting issues --- .../Tokenables/WebAuthnLoginAssertionOptionsTokenable.cs | 2 +- src/Core/Auth/Utilities/GuidUtilities.cs | 4 +++- .../IdentityServer/IUserDecryptionOptionsBuilder.cs | 2 +- .../IdentityServer/UserDecryptionOptionsBuilder.cs | 8 ++++---- test/Core.Test/Services/UserServiceTests.cs | 5 +---- 5 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/Core/Auth/Models/Business/Tokenables/WebAuthnLoginAssertionOptionsTokenable.cs b/src/Core/Auth/Models/Business/Tokenables/WebAuthnLoginAssertionOptionsTokenable.cs index e3aebd01c06c..29320f09eedb 100644 --- a/src/Core/Auth/Models/Business/Tokenables/WebAuthnLoginAssertionOptionsTokenable.cs +++ b/src/Core/Auth/Models/Business/Tokenables/WebAuthnLoginAssertionOptionsTokenable.cs @@ -15,7 +15,7 @@ public class WebAuthnLoginAssertionOptionsTokenable : ExpiringTokenable public string Identifier { get; set; } = TokenIdentifier; public AssertionOptions Options { get; set; } - public WebAuthnLoginAssertionOptionsScope Scope { get; set; } + public WebAuthnLoginAssertionOptionsScope Scope { get; set; } [JsonConstructor] public WebAuthnLoginAssertionOptionsTokenable() diff --git a/src/Core/Auth/Utilities/GuidUtilities.cs b/src/Core/Auth/Utilities/GuidUtilities.cs index 503bf77005f8..043e326b1220 100644 --- a/src/Core/Auth/Utilities/GuidUtilities.cs +++ b/src/Core/Auth/Utilities/GuidUtilities.cs @@ -8,7 +8,9 @@ public static bool TryParseBytes(ReadOnlySpan bytes, out Guid guid) { guid = new Guid(bytes); return true; - } catch { + } + catch + { guid = Guid.Empty; return false; } diff --git a/src/Identity/IdentityServer/IUserDecryptionOptionsBuilder.cs b/src/Identity/IdentityServer/IUserDecryptionOptionsBuilder.cs index 12109619d3e9..dad9d8e27de2 100644 --- a/src/Identity/IdentityServer/IUserDecryptionOptionsBuilder.cs +++ b/src/Identity/IdentityServer/IUserDecryptionOptionsBuilder.cs @@ -10,4 +10,4 @@ public interface IUserDecryptionOptionsBuilder IUserDecryptionOptionsBuilder WithSso(SsoConfig ssoConfig); IUserDecryptionOptionsBuilder WithWebAuthnLoginCredential(WebAuthnCredential credential); Task BuildAsync(); -} \ No newline at end of file +} diff --git a/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs b/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs index a7a468264730..bd871d9edd5e 100644 --- a/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs +++ b/src/Identity/IdentityServer/UserDecryptionOptionsBuilder.cs @@ -1,13 +1,13 @@ using Bit.Core; +using Bit.Core.Auth.Entities; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Models.Api.Response; +using Bit.Core.Auth.Utilities; using Bit.Core.Context; using Bit.Core.Entities; +using Bit.Core.Repositories; using Bit.Core.Services; using Bit.Identity.Utilities; -using Bit.Core.Auth.Utilities; -using Bit.Core.Repositories; -using Bit.Core.Auth.Entities; namespace Bit.Identity.IdentityServer; @@ -152,4 +152,4 @@ private async Task BuildTrustedDeviceOptions() encryptedPrivateKey, encryptedUserKey); } -} \ No newline at end of file +} diff --git a/test/Core.Test/Services/UserServiceTests.cs b/test/Core.Test/Services/UserServiceTests.cs index e59006533385..891213e87be9 100644 --- a/test/Core.Test/Services/UserServiceTests.cs +++ b/test/Core.Test/Services/UserServiceTests.cs @@ -4,7 +4,6 @@ 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; @@ -14,7 +13,6 @@ using Bit.Core.Repositories; using Bit.Core.Services; using Bit.Core.Settings; -using Bit.Core.Tokens; using Bit.Core.Tools.Services; using Bit.Core.Utilities; using Bit.Core.Vault.Repositories; @@ -31,7 +29,6 @@ using NSubstitute.ReceivedExtensions; using NSubstitute.ReturnsExtensions; using Xunit; -using static Fido2NetLib.AuthenticatorAssertionRawResponse; namespace Bit.Core.Test.Services; @@ -238,7 +235,7 @@ public async void CompleteWebAuthLoginAssertionAsync_NoMatchingCredentialExists_ // Arrange response.Response.UserHandle = user.Id.ToByteArray(); sutProvider.GetDependency().GetByIdAsync(user.Id).Returns(user); - sutProvider.GetDependency().GetManyByUserIdAsync(user.Id).Returns(new WebAuthnCredential[] {}); + sutProvider.GetDependency().GetManyByUserIdAsync(user.Id).Returns(new WebAuthnCredential[] { }); // Act var result = async () => await sutProvider.Sut.CompleteWebAuthLoginAssertionAsync(options, response); From ca109356d8605d82ec5873da4d48e80c2a1e3639 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Mon, 30 Oct 2023 13:22:43 +0100 Subject: [PATCH 49/50] [PM-2032] fix: revert changes to `launchSettings.json` --- bitwarden_license/src/Scim/Properties/launchSettings.json | 2 +- bitwarden_license/src/Sso/Properties/launchSettings.json | 6 +++--- .../Scim.IntegrationTest/Properties/launchSettings.json | 2 +- src/Admin/Properties/launchSettings.json | 2 +- src/Api/Properties/launchSettings.json | 2 +- src/Billing/Properties/launchSettings.json | 2 +- src/Events/Properties/launchSettings.json | 2 +- src/EventsProcessor/Properties/launchSettings.json | 2 +- src/Icons/Properties/launchSettings.json | 2 +- src/Identity/Properties/launchSettings.json | 2 +- src/Notifications/Properties/launchSettings.json | 2 +- test/Api.IntegrationTest/Properties/launchSettings.json | 2 +- .../Identity.IntegrationTest/Properties/launchSettings.json | 2 +- util/Server/Properties/launchSettings.json | 2 +- 14 files changed, 16 insertions(+), 16 deletions(-) diff --git a/bitwarden_license/src/Scim/Properties/launchSettings.json b/bitwarden_license/src/Scim/Properties/launchSettings.json index a6add7c52311..aa77a519810a 100644 --- a/bitwarden_license/src/Scim/Properties/launchSettings.json +++ b/bitwarden_license/src/Scim/Properties/launchSettings.json @@ -24,4 +24,4 @@ } } } -} \ No newline at end of file +} diff --git a/bitwarden_license/src/Sso/Properties/launchSettings.json b/bitwarden_license/src/Sso/Properties/launchSettings.json index 561dcd84c71f..ce32b2110566 100644 --- a/bitwarden_license/src/Sso/Properties/launchSettings.json +++ b/bitwarden_license/src/Sso/Properties/launchSettings.json @@ -3,7 +3,7 @@ "windowsAuthentication": false, "anonymousAuthentication": true, "iisExpress": { - "applicationUrl": "http://0.0.0.0:51822", + "applicationUrl": "http://localhost:51822", "sslPort": 0 } }, @@ -16,7 +16,7 @@ }, "Sso": { "commandName": "Project", - "applicationUrl": "http://0.0.0.0:51822", + "applicationUrl": "http://localhost:51822", "environmentVariables": { "ASPNETCORE_ENVIRONMENT": "Development" } @@ -30,4 +30,4 @@ } } } -} \ No newline at end of file +} diff --git a/bitwarden_license/test/Scim.IntegrationTest/Properties/launchSettings.json b/bitwarden_license/test/Scim.IntegrationTest/Properties/launchSettings.json index f3895db525a1..485fc8885a52 100644 --- a/bitwarden_license/test/Scim.IntegrationTest/Properties/launchSettings.json +++ b/bitwarden_license/test/Scim.IntegrationTest/Properties/launchSettings.json @@ -13,4 +13,4 @@ } } } -} \ No newline at end of file +} diff --git a/src/Admin/Properties/launchSettings.json b/src/Admin/Properties/launchSettings.json index 8e3857c915c1..16e75d4388e0 100644 --- a/src/Admin/Properties/launchSettings.json +++ b/src/Admin/Properties/launchSettings.json @@ -30,4 +30,4 @@ } } } -} \ No newline at end of file +} diff --git a/src/Api/Properties/launchSettings.json b/src/Api/Properties/launchSettings.json index b67505b0fc8c..f40c28ea99d4 100644 --- a/src/Api/Properties/launchSettings.json +++ b/src/Api/Properties/launchSettings.json @@ -30,4 +30,4 @@ } } } -} \ No newline at end of file +} diff --git a/src/Billing/Properties/launchSettings.json b/src/Billing/Properties/launchSettings.json index b4497ceca279..5b3fd376476d 100644 --- a/src/Billing/Properties/launchSettings.json +++ b/src/Billing/Properties/launchSettings.json @@ -24,4 +24,4 @@ } } } -} \ No newline at end of file +} diff --git a/src/Events/Properties/launchSettings.json b/src/Events/Properties/launchSettings.json index 0e2ceda6c90e..a743b51c5696 100644 --- a/src/Events/Properties/launchSettings.json +++ b/src/Events/Properties/launchSettings.json @@ -30,4 +30,4 @@ } } } -} \ No newline at end of file +} diff --git a/src/EventsProcessor/Properties/launchSettings.json b/src/EventsProcessor/Properties/launchSettings.json index 835fe5c55b26..f772b225dfd8 100644 --- a/src/EventsProcessor/Properties/launchSettings.json +++ b/src/EventsProcessor/Properties/launchSettings.json @@ -22,4 +22,4 @@ } } } -} \ No newline at end of file +} diff --git a/src/Icons/Properties/launchSettings.json b/src/Icons/Properties/launchSettings.json index c6cc79cced1e..9588da8c5f4f 100644 --- a/src/Icons/Properties/launchSettings.json +++ b/src/Icons/Properties/launchSettings.json @@ -24,4 +24,4 @@ } } } -} \ No newline at end of file +} diff --git a/src/Identity/Properties/launchSettings.json b/src/Identity/Properties/launchSettings.json index de4db21fbbd9..8dcc422bcdb9 100644 --- a/src/Identity/Properties/launchSettings.json +++ b/src/Identity/Properties/launchSettings.json @@ -30,4 +30,4 @@ } } } -} \ No newline at end of file +} diff --git a/src/Notifications/Properties/launchSettings.json b/src/Notifications/Properties/launchSettings.json index 74adbc567d86..adff79ae353c 100644 --- a/src/Notifications/Properties/launchSettings.json +++ b/src/Notifications/Properties/launchSettings.json @@ -30,4 +30,4 @@ } } } -} \ No newline at end of file +} diff --git a/test/Api.IntegrationTest/Properties/launchSettings.json b/test/Api.IntegrationTest/Properties/launchSettings.json index 57c702253115..04fd816afe74 100644 --- a/test/Api.IntegrationTest/Properties/launchSettings.json +++ b/test/Api.IntegrationTest/Properties/launchSettings.json @@ -13,4 +13,4 @@ } } } -} \ No newline at end of file +} diff --git a/test/Identity.IntegrationTest/Properties/launchSettings.json b/test/Identity.IntegrationTest/Properties/launchSettings.json index 46f4892b335e..43e4ad659f4b 100644 --- a/test/Identity.IntegrationTest/Properties/launchSettings.json +++ b/test/Identity.IntegrationTest/Properties/launchSettings.json @@ -8,4 +8,4 @@ } } } -} \ No newline at end of file +} diff --git a/util/Server/Properties/launchSettings.json b/util/Server/Properties/launchSettings.json index 0943477d82fd..22aba45f0a71 100644 --- a/util/Server/Properties/launchSettings.json +++ b/util/Server/Properties/launchSettings.json @@ -8,4 +8,4 @@ } } } -} \ No newline at end of file +} From 8087dda47a57530ece29b79febfe8ba2c88860e3 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Mon, 30 Oct 2023 13:40:58 +0100 Subject: [PATCH 50/50] [PM-2023] chore: clean up assertion endpoint --- .../WebAuthnLoginAssertionOptionsRequestModel.cs | 10 ---------- src/Identity/Controllers/AccountsController.cs | 4 +--- 2 files changed, 1 insertion(+), 13 deletions(-) delete mode 100644 src/Core/Auth/Models/Api/Request/Accounts/WebAuthnLoginAssertionOptionsRequestModel.cs diff --git a/src/Core/Auth/Models/Api/Request/Accounts/WebAuthnLoginAssertionOptionsRequestModel.cs b/src/Core/Auth/Models/Api/Request/Accounts/WebAuthnLoginAssertionOptionsRequestModel.cs deleted file mode 100644 index 4d4ce9c0f1dc..000000000000 --- a/src/Core/Auth/Models/Api/Request/Accounts/WebAuthnLoginAssertionOptionsRequestModel.cs +++ /dev/null @@ -1,10 +0,0 @@ -namespace Bit.Core.Auth.Models.Api.Request.Accounts; - -public class WebAuthnLoginAssertionOptionsRequestModel -{ - // Requried to add support for non-discoverable logins - // [EmailAddress] - // [StringLength(256)] - // public string Email { get; set; } -} - diff --git a/src/Identity/Controllers/AccountsController.cs b/src/Identity/Controllers/AccountsController.cs index bf2f3fec511d..a04e7478a60c 100644 --- a/src/Identity/Controllers/AccountsController.cs +++ b/src/Identity/Controllers/AccountsController.cs @@ -82,10 +82,8 @@ public async Task PostPrelogin([FromBody] PreloginRequest } [HttpPost("webauthn/assertion-options")] - [ApiExplorerSettings(IgnoreApi = true)] // Disable Swagger due to CredentialCreateOptions not converting properly [RequireFeature(FeatureFlagKeys.PasswordlessLogin)] - // TODO: Add tests - public async Task PostWebAuthnLoginAssertionOptions(WebAuthnLoginAssertionOptionsRequestModel model) + public async Task PostWebAuthnLoginAssertionOptions() { var options = await _userService.StartWebAuthnLoginAssertionAsync();