From 7a8ee710da49d67019c7d0bc1ce367f1e980b1b2 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Wed, 19 Mar 2025 11:34:33 +0100 Subject: [PATCH] [PM-19279] Add prelogin response (#5511) * Add prelogin response * Fix test * Fix more tests * Fix tests * Fix SQL warnings * Fix difference between migration and sql SP * Attempt to fix tests * Attempt to fix tests * Attempt to fix * Fix namespace * Attempt to fix error * Fix different SP / migration * Attempt to fix migration * Fix * Fix --- .../Auth/Controllers/AccountsController.cs | 6 +- .../Controllers/AccountsController.cs | 20 ++- .../Accounts/PreloginResponseModel.cs | 7 +- .../OpaqueKeyExchangeCredentialRepository.cs | 24 +++ ...ityFrameworkServiceCollectionExtensions.cs | 1 + .../Utilities/ServiceCollectionExtensions.cs | 2 +- .../OpaqueKeyExchangeCredential_Create.sql | 10 +- ...OpaqueKeyExchangeCredential_DeleteById.sql | 2 +- .../dbo/Stored Procedures/User_DeleteById.sql | 2 +- .../Controllers/AccountsControllerTests.cs | 6 +- test/Core.Test/Services/UserServiceTests.cs | 7 +- .../Controllers/AccountsControllerTests.cs | 19 +++ ...2_00_CreateOpaqueKeyExchangeCredential.sql | 157 +++++++++++++++++- 13 files changed, 239 insertions(+), 24 deletions(-) create mode 100644 src/Infrastructure.EntityFramework/Auth/Repositories/OpaqueKeyExchangeCredentialRepository.cs diff --git a/src/Api/Auth/Controllers/AccountsController.cs b/src/Api/Auth/Controllers/AccountsController.cs index 0a3b31a74a..f1998d8266 100644 --- a/src/Api/Auth/Controllers/AccountsController.cs +++ b/src/Api/Auth/Controllers/AccountsController.cs @@ -15,7 +15,6 @@ using Bit.Core.AdminConsole.Services; using Bit.Core.Auth.Entities; using Bit.Core.Auth.Models.Api.Request.Accounts; using Bit.Core.Auth.Models.Data; -using Bit.Core.Auth.Services; using Bit.Core.Auth.UserFeatures.TdeOffboardingPassword.Interfaces; using Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; using Bit.Core.Entities; @@ -58,7 +57,6 @@ public class AccountsController : Controller _organizationUserValidator; private readonly IRotationValidator, IEnumerable> _webauthnKeyValidator; - private readonly IOpaqueKeyExchangeService _opaqueKeyExchangeService; public AccountsController( @@ -78,8 +76,7 @@ public class AccountsController : Controller emergencyAccessValidator, IRotationValidator, IReadOnlyList> organizationUserValidator, - IRotationValidator, IEnumerable> webAuthnKeyValidator, - IOpaqueKeyExchangeService opaqueKeyExchangeService + IRotationValidator, IEnumerable> webAuthnKeyValidator ) { _organizationService = organizationService; @@ -97,7 +94,6 @@ public class AccountsController : Controller _emergencyAccessValidator = emergencyAccessValidator; _organizationUserValidator = organizationUserValidator; _webauthnKeyValidator = webAuthnKeyValidator; - _opaqueKeyExchangeService = opaqueKeyExchangeService; } diff --git a/src/Identity/Controllers/AccountsController.cs b/src/Identity/Controllers/AccountsController.cs index c840a7ddc5..0ca5d50c5d 100644 --- a/src/Identity/Controllers/AccountsController.cs +++ b/src/Identity/Controllers/AccountsController.cs @@ -1,10 +1,13 @@ using System.Diagnostics; using System.Text; +using System.Text.Json; using Bit.Core; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Models.Api.Request.Accounts; +using Bit.Core.Auth.Models.Api.Request.Opaque; using Bit.Core.Auth.Models.Api.Response.Accounts; using Bit.Core.Auth.Models.Business.Tokenables; +using Bit.Core.Auth.Repositories; using Bit.Core.Auth.Services; using Bit.Core.Auth.UserFeatures.Registration; using Bit.Core.Auth.UserFeatures.WebAuthnLogin; @@ -45,6 +48,7 @@ public class AccountsController : Controller private readonly IReferenceEventService _referenceEventService; private readonly IFeatureService _featureService; private readonly IDataProtectorTokenFactory _registrationEmailVerificationTokenDataFactory; + private readonly IOpaqueKeyExchangeCredentialRepository _opaqueKeyExchangeCredentialRepository; private readonly byte[] _defaultKdfHmacKey = null; private static readonly List _defaultKdfResults = @@ -93,6 +97,7 @@ public class AccountsController : Controller IReferenceEventService referenceEventService, IFeatureService featureService, IDataProtectorTokenFactory registrationEmailVerificationTokenDataFactory, + IOpaqueKeyExchangeCredentialRepository opaqueKeyExchangeCredentialRepository, GlobalSettings globalSettings ) { @@ -107,6 +112,7 @@ public class AccountsController : Controller _referenceEventService = referenceEventService; _featureService = featureService; _registrationEmailVerificationTokenDataFactory = registrationEmailVerificationTokenDataFactory; + _opaqueKeyExchangeCredentialRepository = opaqueKeyExchangeCredentialRepository; if (CoreHelpers.SettingHasValue(globalSettings.KdfDefaultHashKey)) { @@ -255,11 +261,21 @@ public class AccountsController : Controller public async Task PostPrelogin([FromBody] PreloginRequestModel model) { var kdfInformation = await _userRepository.GetKdfInformationByEmailAsync(model.Email); - if (kdfInformation == null) + var user = await _userRepository.GetByEmailAsync(model.Email); + if (kdfInformation == null || user == null) { kdfInformation = GetDefaultKdf(model.Email); } - return new PreloginResponseModel(kdfInformation); + + var credential = await _opaqueKeyExchangeCredentialRepository.GetByUserIdAsync(user.Id); + if (credential != null) + { + return new PreloginResponseModel(kdfInformation, JsonSerializer.Deserialize(credential.CipherConfiguration)!); + } + else + { + return new PreloginResponseModel(kdfInformation, null); + } } [HttpGet("webauthn/assertion-options")] diff --git a/src/Identity/Models/Response/Accounts/PreloginResponseModel.cs b/src/Identity/Models/Response/Accounts/PreloginResponseModel.cs index 129aa3e7a9..46c988edd6 100644 --- a/src/Identity/Models/Response/Accounts/PreloginResponseModel.cs +++ b/src/Identity/Models/Response/Accounts/PreloginResponseModel.cs @@ -1,20 +1,23 @@ -using Bit.Core.Enums; +using Bit.Core.Auth.Models.Api.Request.Opaque; +using Bit.Core.Enums; using Bit.Core.Models.Data; namespace Bit.Identity.Models.Response.Accounts; public class PreloginResponseModel { - public PreloginResponseModel(UserKdfInformation kdfInformation) + public PreloginResponseModel(UserKdfInformation kdfInformation, CipherConfiguration opaqueConfiguration) { Kdf = kdfInformation.Kdf; KdfIterations = kdfInformation.KdfIterations; KdfMemory = kdfInformation.KdfMemory; KdfParallelism = kdfInformation.KdfParallelism; + OpaqueConfiguration = opaqueConfiguration; } public KdfType Kdf { get; set; } public int KdfIterations { get; set; } public int? KdfMemory { get; set; } public int? KdfParallelism { get; set; } + public CipherConfiguration OpaqueConfiguration { get; set; } } diff --git a/src/Infrastructure.EntityFramework/Auth/Repositories/OpaqueKeyExchangeCredentialRepository.cs b/src/Infrastructure.EntityFramework/Auth/Repositories/OpaqueKeyExchangeCredentialRepository.cs new file mode 100644 index 0000000000..51ba304e3e --- /dev/null +++ b/src/Infrastructure.EntityFramework/Auth/Repositories/OpaqueKeyExchangeCredentialRepository.cs @@ -0,0 +1,24 @@ +using AutoMapper; +using Bit.Core.Auth.Entities; +using Bit.Core.Auth.Models.Data; +using Bit.Core.Auth.Repositories; +using Bit.Core.KeyManagement.UserKey; +using Microsoft.Extensions.DependencyInjection; + +namespace Bit.Infrastructure.EntityFramework.Repositories; + +public class OpaqueKeyExchangeCredentialRepository : Repository, IOpaqueKeyExchangeCredentialRepository +{ + public OpaqueKeyExchangeCredentialRepository(IServiceScopeFactory serviceScopeFactory, IMapper mapper) : base(serviceScopeFactory, mapper, (DatabaseContext context) => null) + { + } + + public Task GetByUserIdAsync(Guid userId) + { + return null; + } + public UpdateEncryptedDataForKeyRotation UpdateKeysForRotationAsync(Guid userId, IEnumerable credentials) + { + return null; + } +} diff --git a/src/Infrastructure.EntityFramework/EntityFrameworkServiceCollectionExtensions.cs b/src/Infrastructure.EntityFramework/EntityFrameworkServiceCollectionExtensions.cs index 3f805bbe2c..cb1325a21f 100644 --- a/src/Infrastructure.EntityFramework/EntityFrameworkServiceCollectionExtensions.cs +++ b/src/Infrastructure.EntityFramework/EntityFrameworkServiceCollectionExtensions.cs @@ -87,6 +87,7 @@ public static class EntityFrameworkServiceCollectionExtensions services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); + services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); diff --git a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs index a0bee13f2e..3255d1044b 100644 --- a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs +++ b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs @@ -110,6 +110,7 @@ public static class ServiceCollectionExtensions public static void AddBaseServices(this IServiceCollection services, IGlobalSettings globalSettings) { services.AddScoped(); + services.AddSingleton(); services.AddUserServices(globalSettings); services.AddTrialInitiationServices(); services.AddOrganizationServices(globalSettings); @@ -118,7 +119,6 @@ public static class ServiceCollectionExtensions services.AddScoped(); services.AddScoped(); services.AddScoped(); - services.AddScoped(); services.AddSingleton(); services.AddScoped(); services.AddScoped(); diff --git a/src/Sql/Auth/dbo/Stored Procedures/OpaqueKeyExchangeCredential_Create.sql b/src/Sql/Auth/dbo/Stored Procedures/OpaqueKeyExchangeCredential_Create.sql index bcc324ae86..bc0a470a29 100644 --- a/src/Sql/Auth/dbo/Stored Procedures/OpaqueKeyExchangeCredential_Create.sql +++ b/src/Sql/Auth/dbo/Stored Procedures/OpaqueKeyExchangeCredential_Create.sql @@ -1,11 +1,11 @@ CREATE PROCEDURE [dbo].[OpaqueKeyExchangeCredential_Create] @Id UNIQUEIDENTIFIER OUTPUT, @UserId UNIQUEIDENTIFIER, - @CipherConfiguration VARCHAR(MAX) NOT NULL, - @CredentialBlob VARCHAR(MAX) NOT NULL, - @EncryptedPublicKey VARCHAR(MAX) NOT NULL, - @EncryptedPrivateKey VARCHAR(MAX) NOT NULL, - @EncryptedUserKey VARCHAR(MAX) NOT NULL, + @CipherConfiguration VARCHAR(MAX), + @CredentialBlob VARCHAR(MAX), + @EncryptedPublicKey VARCHAR(MAX), + @EncryptedPrivateKey VARCHAR(MAX), + @EncryptedUserKey VARCHAR(MAX), @CreationDate DATETIME2(7) AS BEGIN diff --git a/src/Sql/Auth/dbo/Stored Procedures/OpaqueKeyExchangeCredential_DeleteById.sql b/src/Sql/Auth/dbo/Stored Procedures/OpaqueKeyExchangeCredential_DeleteById.sql index f2634da1cf..8bd4d01be3 100644 --- a/src/Sql/Auth/dbo/Stored Procedures/OpaqueKeyExchangeCredential_DeleteById.sql +++ b/src/Sql/Auth/dbo/Stored Procedures/OpaqueKeyExchangeCredential_DeleteById.sql @@ -1,5 +1,5 @@ CREATE PROCEDURE [dbo].[OpaqueKeyExchangeCredential_DeleteById] - @Id UNIQUEIDENTIFIER, + @Id UNIQUEIDENTIFIER AS BEGIN DELETE diff --git a/src/Sql/dbo/Stored Procedures/User_DeleteById.sql b/src/Sql/dbo/Stored Procedures/User_DeleteById.sql index 55797e57a2..5f8f66c4b5 100644 --- a/src/Sql/dbo/Stored Procedures/User_DeleteById.sql +++ b/src/Sql/dbo/Stored Procedures/User_DeleteById.sql @@ -29,7 +29,7 @@ BEGIN FROM [dbo].[OpaqueKeyExchangeCredential] WHERE - [UserId] = @UserId + [UserId] = @Id -- Delete WebAuthnCredentials DELETE diff --git a/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs b/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs index 15c7573aca..e5b47d7c9c 100644 --- a/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs +++ b/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs @@ -291,12 +291,12 @@ public class AccountsControllerTests : IDisposable { var user = GenerateExampleUser(); ConfigureUserServiceToReturnValidPrincipalFor(user); - _userService.ChangePasswordAsync(user, default, default, default, default) + _userService.ChangePasswordAsync(user, default, default, default, default, null) .Returns(Task.FromResult(IdentityResult.Success)); await _sut.PostPassword(new PasswordRequestModel()); - await _userService.Received(1).ChangePasswordAsync(user, default, default, default, default); + await _userService.Received(1).ChangePasswordAsync(user, default, default, default, default, null); } [Fact] @@ -314,7 +314,7 @@ public class AccountsControllerTests : IDisposable { var user = GenerateExampleUser(); ConfigureUserServiceToReturnValidPrincipalFor(user); - _userService.ChangePasswordAsync(user, default, default, default, default) + _userService.ChangePasswordAsync(user, default, default, default, default, null) .Returns(Task.FromResult(IdentityResult.Failed())); await Assert.ThrowsAsync( diff --git a/test/Core.Test/Services/UserServiceTests.cs b/test/Core.Test/Services/UserServiceTests.cs index 3158c1595c..7944345423 100644 --- a/test/Core.Test/Services/UserServiceTests.cs +++ b/test/Core.Test/Services/UserServiceTests.cs @@ -9,6 +9,7 @@ using Bit.Core.AdminConsole.Services; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Models; using Bit.Core.Auth.Models.Business.Tokenables; +using Bit.Core.Auth.Services; using Bit.Core.Billing.Services; using Bit.Core.Context; using Bit.Core.Entities; @@ -324,7 +325,8 @@ public class UserServiceTests sutProvider.GetDependency(), sutProvider.GetDependency(), sutProvider.GetDependency(), - sutProvider.GetDependency() + sutProvider.GetDependency(), + sutProvider.GetDependency() ); var actualIsVerified = await sut.VerifySecretAsync(user, secret); @@ -911,7 +913,8 @@ public class UserServiceTests sutProvider.GetDependency(), sutProvider.GetDependency(), sutProvider.GetDependency(), - sutProvider.GetDependency() + sutProvider.GetDependency(), + sutProvider.GetDependency() ); } } diff --git a/test/Identity.Test/Controllers/AccountsControllerTests.cs b/test/Identity.Test/Controllers/AccountsControllerTests.cs index 03db0a5904..4928d08316 100644 --- a/test/Identity.Test/Controllers/AccountsControllerTests.cs +++ b/test/Identity.Test/Controllers/AccountsControllerTests.cs @@ -3,6 +3,7 @@ using System.Text; using Bit.Core; using Bit.Core.Auth.Models.Api.Request.Accounts; using Bit.Core.Auth.Models.Business.Tokenables; +using Bit.Core.Auth.Repositories; using Bit.Core.Auth.Services; using Bit.Core.Auth.UserFeatures.Registration; using Bit.Core.Auth.UserFeatures.WebAuthnLogin; @@ -45,6 +46,7 @@ public class AccountsControllerTests : IDisposable private readonly IReferenceEventService _referenceEventService; private readonly IFeatureService _featureService; private readonly IDataProtectorTokenFactory _registrationEmailVerificationTokenDataFactory; + private readonly IOpaqueKeyExchangeCredentialRepository _opaqueKeyExchangeCredentialRepository; private readonly GlobalSettings _globalSettings; @@ -61,6 +63,7 @@ public class AccountsControllerTests : IDisposable _referenceEventService = Substitute.For(); _featureService = Substitute.For(); _registrationEmailVerificationTokenDataFactory = Substitute.For>(); + _opaqueKeyExchangeCredentialRepository = Substitute.For(); _globalSettings = Substitute.For(); _sut = new AccountsController( @@ -75,6 +78,7 @@ public class AccountsControllerTests : IDisposable _referenceEventService, _featureService, _registrationEmailVerificationTokenDataFactory, + _opaqueKeyExchangeCredentialRepository, _globalSettings ); } @@ -93,6 +97,11 @@ public class AccountsControllerTests : IDisposable KdfIterations = AuthConstants.PBKDF2_ITERATIONS.Default }; _userRepository.GetKdfInformationByEmailAsync(Arg.Any()).Returns(userKdfInfo); + var mockUser = new User + { + Email = "user@example.com" + }; + _userRepository.GetByEmailAsync(Arg.Any()).Returns(mockUser); var response = await _sut.PostPrelogin(new PreloginRequestModel { Email = "user@example.com" }); @@ -105,6 +114,11 @@ public class AccountsControllerTests : IDisposable { SetDefaultKdfHmacKey(null); _userRepository.GetKdfInformationByEmailAsync(Arg.Any()).Returns(Task.FromResult(null)); + var mockUser = new User + { + Email = "user@example.com" + }; + _userRepository.GetByEmailAsync(Arg.Any()).Returns(mockUser); var response = await _sut.PostPrelogin(new PreloginRequestModel { Email = "user@example.com" }); @@ -121,6 +135,11 @@ public class AccountsControllerTests : IDisposable SetDefaultKdfHmacKey(defaultKey); _userRepository.GetKdfInformationByEmailAsync(Arg.Any()).Returns(Task.FromResult(null)); + var mockUser = new User + { + Email = "user@example.com" + }; + _userRepository.GetByEmailAsync(Arg.Any()).Returns(mockUser); var fieldInfo = typeof(AccountsController).GetField("_defaultKdfResults", BindingFlags.NonPublic | BindingFlags.Static); if (fieldInfo == null) diff --git a/util/Migrator/DbScripts/2025-03-12_00_CreateOpaqueKeyExchangeCredential.sql b/util/Migrator/DbScripts/2025-03-12_00_CreateOpaqueKeyExchangeCredential.sql index a64d7e6a48..1a92b87b51 100644 --- a/util/Migrator/DbScripts/2025-03-12_00_CreateOpaqueKeyExchangeCredential.sql +++ b/util/Migrator/DbScripts/2025-03-12_00_CreateOpaqueKeyExchangeCredential.sql @@ -25,7 +25,7 @@ CREATE OR ALTER PROCEDURE [dbo].[OpaqueKeyExchangeCredential_Create] @CipherConfiguration VARCHAR(MAX), @CredentialBlob VARCHAR(MAX), @EncryptedPublicKey VARCHAR(MAX), - @EncryptedPrivateKey TINYINT, + @EncryptedPrivateKey VARCHAR(MAX), @EncryptedUserKey VARCHAR(MAX), @CreationDate DATETIME2(7) AS @@ -127,4 +127,157 @@ BEGIN [Id] = @Id END -GO \ No newline at end of file +GO + +ALTER PROCEDURE [dbo].[User_DeleteById] + @Id UNIQUEIDENTIFIER +WITH + RECOMPILE +AS +BEGIN + SET NOCOUNT ON + DECLARE @BatchSize INT = 100 + + -- Delete ciphers + WHILE @BatchSize > 0 + BEGIN + BEGIN TRANSACTION User_DeleteById_Ciphers + + DELETE TOP(@BatchSize) + FROM + [dbo].[Cipher] + WHERE + [UserId] = @Id + + SET @BatchSize = @@ROWCOUNT + + COMMIT TRANSACTION User_DeleteById_Ciphers + END + + BEGIN TRANSACTION User_DeleteById + -- Delete OpaqueKeyExchangeCredentials + DELETE + FROM + [dbo].[OpaqueKeyExchangeCredential] + WHERE + [UserId] = @Id + + -- Delete WebAuthnCredentials + DELETE + FROM + [dbo].[WebAuthnCredential] + WHERE + [UserId] = @Id + + -- Delete folders + DELETE + FROM + [dbo].[Folder] + WHERE + [UserId] = @Id + + -- Delete AuthRequest, must be before Device + DELETE + FROM + [dbo].[AuthRequest] + WHERE + [UserId] = @Id + + -- Delete devices + DELETE + FROM + [dbo].[Device] + WHERE + [UserId] = @Id + + -- Delete collection users + DELETE + CU + FROM + [dbo].[CollectionUser] CU + INNER JOIN + [dbo].[OrganizationUser] OU ON OU.[Id] = CU.[OrganizationUserId] + WHERE + OU.[UserId] = @Id + + -- Delete group users + DELETE + GU + FROM + [dbo].[GroupUser] GU + INNER JOIN + [dbo].[OrganizationUser] OU ON OU.[Id] = GU.[OrganizationUserId] + WHERE + OU.[UserId] = @Id + + -- Delete AccessPolicy + DELETE + AP + FROM + [dbo].[AccessPolicy] AP + INNER JOIN + [dbo].[OrganizationUser] OU ON OU.[Id] = AP.[OrganizationUserId] + WHERE + [UserId] = @Id + + -- Delete organization users + DELETE + FROM + [dbo].[OrganizationUser] + WHERE + [UserId] = @Id + + -- Delete provider users + DELETE + FROM + [dbo].[ProviderUser] + WHERE + [UserId] = @Id + + -- Delete SSO Users + DELETE + FROM + [dbo].[SsoUser] + WHERE + [UserId] = @Id + + -- Delete Emergency Accesses + DELETE + FROM + [dbo].[EmergencyAccess] + WHERE + [GrantorId] = @Id + OR + [GranteeId] = @Id + + -- Delete Sends + DELETE + FROM + [dbo].[Send] + WHERE + [UserId] = @Id + + -- Delete Notification Status + DELETE + FROM + [dbo].[NotificationStatus] + WHERE + [UserId] = @Id + + -- Delete Notification + DELETE + FROM + [dbo].[Notification] + WHERE + [UserId] = @Id + + -- Finally, delete the user + DELETE + FROM + [dbo].[User] + WHERE + [Id] = @Id + + COMMIT TRANSACTION User_DeleteById +END +GO