From 8cb80305341098673771e863b8b5266ad19bdce0 Mon Sep 17 00:00:00 2001 From: Patrick-Pimentel-Bitwarden Date: Thu, 15 Jan 2026 15:55:27 -0500 Subject: [PATCH] feat(register): [PM-27084] Account Register Uses New Data Types (#6715) * feat(register): [PM-27084] Account Register Uses New Data Types - Implementation * test(register): [PM-27084] Account Register Uses New Data Types - Added tests --- .../Request/Accounts/PasswordRequestModel.cs | 6 +- .../SetInitialPasswordRequestModel.cs | 1 - .../Accounts/RegisterFinishRequestModel.cs | 201 ++++++- src/Core/Entities/User.cs | 6 +- .../Models/Api/Request}/KdfRequestModel.cs | 11 +- ...rPasswordAuthenticationDataRequestModel.cs | 6 +- .../MasterPasswordUnlockDataRequestModel.cs | 6 +- .../Data/MasterPasswordAuthenticationData.cs | 5 + ...sterPasswordUnlockAndAuthenticationData.cs | 3 +- .../Models/Data/MasterPasswordUnlockData.cs | 5 + src/Core/Utilities/KdfSettingsValidator.cs | 1 + .../Controllers/AccountsController.cs | 61 ++- .../Controllers/AccountsControllerTest.cs | 4 +- .../Controllers/AccountsControllerTests.cs | 1 - .../SetInitialPasswordRequestModelTests.cs | 1 - .../RegisterFinishRequestModelFixtures.cs | 4 +- .../RegisterFinishRequestModelTests.cs | 183 +++++++ .../Controllers/AccountsControllerTests.cs | 502 +++++++++++++++++- .../Factories/IdentityApplicationFactory.cs | 101 +++- 19 files changed, 1045 insertions(+), 63 deletions(-) rename src/{Api/KeyManagement/Models/Requests => Core/KeyManagement/Models/Api/Request}/KdfRequestModel.cs (59%) rename src/{Api/KeyManagement/Models/Requests => Core/KeyManagement/Models/Api/Request}/MasterPasswordAuthenticationDataRequestModel.cs (71%) rename src/{Api/KeyManagement/Models/Requests => Core/KeyManagement/Models/Api/Request}/MasterPasswordUnlockDataRequestModel.cs (71%) diff --git a/src/Api/Auth/Models/Request/Accounts/PasswordRequestModel.cs b/src/Api/Auth/Models/Request/Accounts/PasswordRequestModel.cs index 8fa51e9f34..ab8c727852 100644 --- a/src/Api/Auth/Models/Request/Accounts/PasswordRequestModel.cs +++ b/src/Api/Auth/Models/Request/Accounts/PasswordRequestModel.cs @@ -1,7 +1,5 @@ -#nullable enable - -using System.ComponentModel.DataAnnotations; -using Bit.Api.KeyManagement.Models.Requests; +using System.ComponentModel.DataAnnotations; +using Bit.Core.KeyManagement.Models.Api.Request; namespace Bit.Api.Auth.Models.Request.Accounts; diff --git a/src/Api/Auth/Models/Request/Accounts/SetInitialPasswordRequestModel.cs b/src/Api/Auth/Models/Request/Accounts/SetInitialPasswordRequestModel.cs index 55ffdca94b..37a7901fee 100644 --- a/src/Api/Auth/Models/Request/Accounts/SetInitialPasswordRequestModel.cs +++ b/src/Api/Auth/Models/Request/Accounts/SetInitialPasswordRequestModel.cs @@ -1,5 +1,4 @@ using System.ComponentModel.DataAnnotations; -using Bit.Api.KeyManagement.Models.Requests; using Bit.Core.Auth.Models.Api.Request.Accounts; using Bit.Core.Auth.Models.Data; using Bit.Core.Entities; diff --git a/src/Core/Auth/Models/Api/Request/Accounts/RegisterFinishRequestModel.cs b/src/Core/Auth/Models/Api/Request/Accounts/RegisterFinishRequestModel.cs index 0ac7dbbcb4..cb66540a6b 100644 --- a/src/Core/Auth/Models/Api/Request/Accounts/RegisterFinishRequestModel.cs +++ b/src/Core/Auth/Models/Api/Request/Accounts/RegisterFinishRequestModel.cs @@ -1,6 +1,6 @@ -#nullable enable -using Bit.Core.Entities; +using Bit.Core.Entities; using Bit.Core.Enums; +using Bit.Core.KeyManagement.Models.Api.Request; using Bit.Core.Utilities; namespace Bit.Core.Auth.Models.Api.Request.Accounts; @@ -21,19 +21,32 @@ public class RegisterFinishRequestModel : IValidatableObject public required string Email { get; set; } public string? EmailVerificationToken { get; set; } + public MasterPasswordAuthenticationDataRequestModel? MasterPasswordAuthentication { get; set; } + public MasterPasswordUnlockDataRequestModel? MasterPasswordUnlock { get; set; } + + // PM-28143 - Remove property below (made optional during migration to MasterPasswordUnlockData) [StringLength(1000)] - public required string MasterPasswordHash { get; set; } + // Made optional but there will still be a thrown error if it does not exist either here or + // in the MasterPasswordAuthenticationData. + public string? MasterPasswordHash { get; set; } [StringLength(50)] public string? MasterPasswordHint { get; set; } - public required string UserSymmetricKey { get; set; } + // PM-28143 - Remove property below (made optional during migration to MasterPasswordUnlockData) + // Made optional but there will still be a thrown error if it does not exist either here or + // in the MasterPasswordAuthenticationData. + public string? UserSymmetricKey { get; set; } public required KeysRequestModel UserAsymmetricKeys { get; set; } - public required KdfType Kdf { get; set; } - public required int KdfIterations { get; set; } + // PM-28143 - Remove line below (made optional during migration to MasterPasswordUnlockData) + public KdfType? Kdf { get; set; } + // PM-28143 - Remove line below (made optional during migration to MasterPasswordUnlockData) + public int? KdfIterations { get; set; } + // PM-28143 - Remove line below public int? KdfMemory { get; set; } + // PM-28143 - Remove line below public int? KdfParallelism { get; set; } public Guid? OrganizationUserId { get; set; } @@ -54,11 +67,14 @@ public class RegisterFinishRequestModel : IValidatableObject { Email = Email, MasterPasswordHint = MasterPasswordHint, - Kdf = Kdf, - KdfIterations = KdfIterations, - KdfMemory = KdfMemory, - KdfParallelism = KdfParallelism, - Key = UserSymmetricKey, + Kdf = (KdfType)(MasterPasswordUnlock?.Kdf.KdfType ?? Kdf)!, + KdfIterations = (int)(MasterPasswordUnlock?.Kdf.Iterations ?? KdfIterations)!, + // KdfMemory and KdfParallelism are optional (only used for Argon2id) + KdfMemory = MasterPasswordUnlock?.Kdf.Memory ?? KdfMemory, + KdfParallelism = MasterPasswordUnlock?.Kdf.Parallelism ?? KdfParallelism, + // PM-28827 To be added when MasterPasswordSalt is added to the user column + // MasterPasswordSalt = MasterPasswordUnlock?.Salt ?? Email.ToLower().Trim(), + Key = MasterPasswordUnlock?.MasterKeyWrappedUserKey ?? UserSymmetricKey }; UserAsymmetricKeys.ToUser(user); @@ -72,7 +88,9 @@ public class RegisterFinishRequestModel : IValidatableObject { return RegisterFinishTokenType.EmailVerification; } - if (!string.IsNullOrEmpty(OrgInviteToken) && OrganizationUserId.HasValue) + if (!string.IsNullOrEmpty(OrgInviteToken) + && OrganizationUserId.HasValue + && OrganizationUserId.Value != Guid.Empty) { return RegisterFinishTokenType.OrganizationInvite; } @@ -80,11 +98,15 @@ public class RegisterFinishRequestModel : IValidatableObject { return RegisterFinishTokenType.OrgSponsoredFreeFamilyPlan; } - if (!string.IsNullOrWhiteSpace(AcceptEmergencyAccessInviteToken) && AcceptEmergencyAccessId.HasValue) + if (!string.IsNullOrWhiteSpace(AcceptEmergencyAccessInviteToken) + && AcceptEmergencyAccessId.HasValue + && AcceptEmergencyAccessId.Value != Guid.Empty) { return RegisterFinishTokenType.EmergencyAccessInvite; } - if (!string.IsNullOrWhiteSpace(ProviderInviteToken) && ProviderUserId.HasValue) + if (!string.IsNullOrWhiteSpace(ProviderInviteToken) + && ProviderUserId.HasValue + && ProviderUserId.Value != Guid.Empty) { return RegisterFinishTokenType.ProviderInvite; } @@ -92,9 +114,156 @@ public class RegisterFinishRequestModel : IValidatableObject throw new InvalidOperationException("Invalid token type."); } - public IEnumerable Validate(ValidationContext validationContext) { - return KdfSettingsValidator.Validate(Kdf, KdfIterations, KdfMemory, KdfParallelism); + // 1. Authentication data containing hash and hash at root level check + if (MasterPasswordAuthentication != null && MasterPasswordHash != null) + { + if (MasterPasswordAuthentication.MasterPasswordAuthenticationHash != MasterPasswordHash) + { + yield return new ValidationResult( + $"{nameof(MasterPasswordAuthentication.MasterPasswordAuthenticationHash)} and root level {nameof(MasterPasswordHash)} provided and are not equal. Only provide one.", + [nameof(MasterPasswordAuthentication.MasterPasswordAuthenticationHash), nameof(MasterPasswordHash)]); + } + } // 1.5 if there is no master password hash that is unacceptable even though they are both optional in the model + else if (MasterPasswordAuthentication == null && MasterPasswordHash == null) + { + yield return new ValidationResult( + $"{nameof(MasterPasswordAuthentication.MasterPasswordAuthenticationHash)} and {nameof(MasterPasswordHash)} not found on request, one needs to be defined.", + [nameof(MasterPasswordAuthentication.MasterPasswordAuthenticationHash), nameof(MasterPasswordHash)]); + } + + // 2. Validate kdf settings. + if (MasterPasswordUnlock != null) + { + foreach (var validationResult in KdfSettingsValidator.Validate(MasterPasswordUnlock.ToData().Kdf)) + { + yield return validationResult; + } + } + + if (MasterPasswordAuthentication != null) + { + foreach (var validationResult in KdfSettingsValidator.Validate(MasterPasswordAuthentication.ToData().Kdf)) + { + yield return validationResult; + } + } + + // 3. Validate root kdf values if kdf values are not in the unlock and authentication. + if (MasterPasswordUnlock == null && MasterPasswordAuthentication == null) + { + var hasMissingRequiredKdfInputs = false; + if (Kdf == null) + { + yield return new ValidationResult($"{nameof(Kdf)} not found on RequestModel", [nameof(Kdf)]); + hasMissingRequiredKdfInputs = true; + } + if (KdfIterations == null) + { + yield return new ValidationResult($"{nameof(KdfIterations)} not found on RequestModel", [nameof(KdfIterations)]); + hasMissingRequiredKdfInputs = true; + } + + if (!hasMissingRequiredKdfInputs) + { + foreach (var validationResult in KdfSettingsValidator.Validate( + Kdf!.Value, + KdfIterations!.Value, + KdfMemory, + KdfParallelism)) + { + yield return validationResult; + } + } + } + else if (MasterPasswordUnlock == null && MasterPasswordAuthentication != null) + { + // Authentication provided but Unlock missing + yield return new ValidationResult($"{nameof(MasterPasswordUnlock)} not found on RequestModel", [nameof(MasterPasswordUnlock)]); + } + else if (MasterPasswordUnlock != null && MasterPasswordAuthentication == null) + { + // Unlock provided but Authentication missing + yield return new ValidationResult($"{nameof(MasterPasswordAuthentication)} not found on RequestModel", [nameof(MasterPasswordAuthentication)]); + } + + // 3. Lastly, validate access token type and presence. Must be done last because of yield break. + RegisterFinishTokenType tokenType; + var tokenTypeResolved = true; + try + { + tokenType = GetTokenType(); + } + catch (InvalidOperationException) + { + tokenTypeResolved = false; + tokenType = default; + } + + if (!tokenTypeResolved) + { + yield return new ValidationResult("No valid registration token provided"); + yield break; + } + + switch (tokenType) + { + case RegisterFinishTokenType.EmailVerification: + if (string.IsNullOrEmpty(EmailVerificationToken)) + { + yield return new ValidationResult( + $"{nameof(EmailVerificationToken)} absent when processing register/finish.", + [nameof(EmailVerificationToken)]); + } + break; + case RegisterFinishTokenType.OrganizationInvite: + if (string.IsNullOrEmpty(OrgInviteToken)) + { + yield return new ValidationResult( + $"{nameof(OrgInviteToken)} absent when processing register/finish.", + [nameof(OrgInviteToken)]); + } + break; + case RegisterFinishTokenType.OrgSponsoredFreeFamilyPlan: + if (string.IsNullOrEmpty(OrgSponsoredFreeFamilyPlanToken)) + { + yield return new ValidationResult( + $"{nameof(OrgSponsoredFreeFamilyPlanToken)} absent when processing register/finish.", + [nameof(OrgSponsoredFreeFamilyPlanToken)]); + } + break; + case RegisterFinishTokenType.EmergencyAccessInvite: + if (string.IsNullOrEmpty(AcceptEmergencyAccessInviteToken)) + { + yield return new ValidationResult( + $"{nameof(AcceptEmergencyAccessInviteToken)} absent when processing register/finish.", + [nameof(AcceptEmergencyAccessInviteToken)]); + } + if (!AcceptEmergencyAccessId.HasValue || AcceptEmergencyAccessId.Value == Guid.Empty) + { + yield return new ValidationResult( + $"{nameof(AcceptEmergencyAccessId)} absent when processing register/finish.", + [nameof(AcceptEmergencyAccessId)]); + } + break; + case RegisterFinishTokenType.ProviderInvite: + if (string.IsNullOrEmpty(ProviderInviteToken)) + { + yield return new ValidationResult( + $"{nameof(ProviderInviteToken)} absent when processing register/finish.", + [nameof(ProviderInviteToken)]); + } + if (!ProviderUserId.HasValue || ProviderUserId.Value == Guid.Empty) + { + yield return new ValidationResult( + $"{nameof(ProviderUserId)} absent when processing register/finish.", + [nameof(ProviderUserId)]); + } + break; + default: + yield return new ValidationResult("Invalid registration finish request"); + break; + } } } diff --git a/src/Core/Entities/User.cs b/src/Core/Entities/User.cs index 669e32bcbe..422dc37c6e 100644 --- a/src/Core/Entities/User.cs +++ b/src/Core/Entities/User.cs @@ -7,8 +7,6 @@ using Bit.Core.KeyManagement.Models.Data; using Bit.Core.Utilities; using Microsoft.AspNetCore.Identity; -#nullable enable - namespace Bit.Core.Entities; public class User : ITableObject, IStorableSubscriber, IRevisable, ITwoFactorProvidersUser @@ -51,7 +49,7 @@ public class User : ITableObject, IStorableSubscriber, IRevisable, ITwoFac public string? Key { get; set; } /// /// The raw public key, without a signature from the user's signature key. - /// + /// public string? PublicKey { get; set; } /// /// User key wrapped private key. @@ -107,6 +105,8 @@ public class User : ITableObject, IStorableSubscriber, IRevisable, ITwoFac public DateTime? LastKeyRotationDate { get; set; } public DateTime? LastEmailChangeDate { get; set; } public bool VerifyDevices { get; set; } = true; + // PM-28827 Uncomment below line. + // public string? MasterPasswordSalt { get; set; } public string GetMasterPasswordSalt() { diff --git a/src/Api/KeyManagement/Models/Requests/KdfRequestModel.cs b/src/Core/KeyManagement/Models/Api/Request/KdfRequestModel.cs similarity index 59% rename from src/Api/KeyManagement/Models/Requests/KdfRequestModel.cs rename to src/Core/KeyManagement/Models/Api/Request/KdfRequestModel.cs index 904304a633..edcd7f760f 100644 --- a/src/Api/KeyManagement/Models/Requests/KdfRequestModel.cs +++ b/src/Core/KeyManagement/Models/Api/Request/KdfRequestModel.cs @@ -1,10 +1,11 @@ using System.ComponentModel.DataAnnotations; using Bit.Core.Enums; using Bit.Core.KeyManagement.Models.Data; +using Bit.Core.Utilities; -namespace Bit.Api.KeyManagement.Models.Requests; +namespace Bit.Core.KeyManagement.Models.Api.Request; -public class KdfRequestModel +public class KdfRequestModel : IValidatableObject { [Required] public required KdfType KdfType { get; init; } @@ -23,4 +24,10 @@ public class KdfRequestModel Parallelism = Parallelism }; } + + public IEnumerable Validate(ValidationContext validationContext) + { + // Generic per-request KDF validation for any request model embedding KdfRequestModel + return KdfSettingsValidator.Validate(ToData()); + } } diff --git a/src/Api/KeyManagement/Models/Requests/MasterPasswordAuthenticationDataRequestModel.cs b/src/Core/KeyManagement/Models/Api/Request/MasterPasswordAuthenticationDataRequestModel.cs similarity index 71% rename from src/Api/KeyManagement/Models/Requests/MasterPasswordAuthenticationDataRequestModel.cs rename to src/Core/KeyManagement/Models/Api/Request/MasterPasswordAuthenticationDataRequestModel.cs index 4f70a1135f..04c22cc3a6 100644 --- a/src/Api/KeyManagement/Models/Requests/MasterPasswordAuthenticationDataRequestModel.cs +++ b/src/Core/KeyManagement/Models/Api/Request/MasterPasswordAuthenticationDataRequestModel.cs @@ -1,8 +1,12 @@ using System.ComponentModel.DataAnnotations; using Bit.Core.KeyManagement.Models.Data; -namespace Bit.Api.KeyManagement.Models.Requests; +namespace Bit.Core.KeyManagement.Models.Api.Request; +/// +/// Use this datatype when interfacing with requests to create a separation of concern. +/// See to use for commands, queries, services. +/// public class MasterPasswordAuthenticationDataRequestModel { public required KdfRequestModel Kdf { get; init; } diff --git a/src/Api/KeyManagement/Models/Requests/MasterPasswordUnlockDataRequestModel.cs b/src/Core/KeyManagement/Models/Api/Request/MasterPasswordUnlockDataRequestModel.cs similarity index 71% rename from src/Api/KeyManagement/Models/Requests/MasterPasswordUnlockDataRequestModel.cs rename to src/Core/KeyManagement/Models/Api/Request/MasterPasswordUnlockDataRequestModel.cs index e1d7863cae..8d7df86374 100644 --- a/src/Api/KeyManagement/Models/Requests/MasterPasswordUnlockDataRequestModel.cs +++ b/src/Core/KeyManagement/Models/Api/Request/MasterPasswordUnlockDataRequestModel.cs @@ -2,8 +2,12 @@ using Bit.Core.KeyManagement.Models.Data; using Bit.Core.Utilities; -namespace Bit.Api.KeyManagement.Models.Requests; +namespace Bit.Core.KeyManagement.Models.Api.Request; +/// +/// Use this datatype when interfacing with requests to create a separation of concern. +/// See to use for commands, queries, services. +/// public class MasterPasswordUnlockDataRequestModel { public required KdfRequestModel Kdf { get; init; } diff --git a/src/Core/KeyManagement/Models/Data/MasterPasswordAuthenticationData.cs b/src/Core/KeyManagement/Models/Data/MasterPasswordAuthenticationData.cs index 1bc7006cef..6e53dfa744 100644 --- a/src/Core/KeyManagement/Models/Data/MasterPasswordAuthenticationData.cs +++ b/src/Core/KeyManagement/Models/Data/MasterPasswordAuthenticationData.cs @@ -1,8 +1,13 @@ using Bit.Core.Entities; using Bit.Core.Exceptions; +using Bit.Core.KeyManagement.Models.Api.Request; namespace Bit.Core.KeyManagement.Models.Data; +/// +/// Use this datatype when interfacing with commands, queries, services to create a separation of concern. +/// See to use for requests. +/// public class MasterPasswordAuthenticationData { public required KdfSettings Kdf { get; init; } diff --git a/src/Core/KeyManagement/Models/Data/MasterPasswordUnlockAndAuthenticationData.cs b/src/Core/KeyManagement/Models/Data/MasterPasswordUnlockAndAuthenticationData.cs index ad3a0b692b..b79ce8bce1 100644 --- a/src/Core/KeyManagement/Models/Data/MasterPasswordUnlockAndAuthenticationData.cs +++ b/src/Core/KeyManagement/Models/Data/MasterPasswordUnlockAndAuthenticationData.cs @@ -1,5 +1,4 @@ -#nullable enable -using Bit.Core.Entities; +using Bit.Core.Entities; using Bit.Core.Enums; namespace Bit.Core.KeyManagement.Models.Data; diff --git a/src/Core/KeyManagement/Models/Data/MasterPasswordUnlockData.cs b/src/Core/KeyManagement/Models/Data/MasterPasswordUnlockData.cs index cb18ed2a78..f8139cba99 100644 --- a/src/Core/KeyManagement/Models/Data/MasterPasswordUnlockData.cs +++ b/src/Core/KeyManagement/Models/Data/MasterPasswordUnlockData.cs @@ -1,8 +1,13 @@ using Bit.Core.Entities; using Bit.Core.Exceptions; +using Bit.Core.KeyManagement.Models.Api.Request; namespace Bit.Core.KeyManagement.Models.Data; +/// +/// Use this datatype when interfacing with commands, queries, services to create a separation of concern. +/// See to use for requests. +/// public class MasterPasswordUnlockData { public required KdfSettings Kdf { get; init; } diff --git a/src/Core/Utilities/KdfSettingsValidator.cs b/src/Core/Utilities/KdfSettingsValidator.cs index f89e8ddb66..e5690ad469 100644 --- a/src/Core/Utilities/KdfSettingsValidator.cs +++ b/src/Core/Utilities/KdfSettingsValidator.cs @@ -6,6 +6,7 @@ namespace Bit.Core.Utilities; public static class KdfSettingsValidator { + // PM-28143 - Remove below when fixing ticket public static IEnumerable Validate(KdfType kdfType, int kdfIterations, int? kdfMemory, int? kdfParallelism) { switch (kdfType) diff --git a/src/Identity/Controllers/AccountsController.cs b/src/Identity/Controllers/AccountsController.cs index b7d4342c1b..e9807fb1fc 100644 --- a/src/Identity/Controllers/AccountsController.cs +++ b/src/Identity/Controllers/AccountsController.cs @@ -1,8 +1,4 @@ -// FIXME: Update this file to be null safe and then delete the line below -#nullable disable - -using System.Diagnostics; -using System.Text; +using System.Text; using Bit.Core; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Models.Api.Request.Accounts; @@ -42,7 +38,7 @@ public class AccountsController : Controller private readonly IFeatureService _featureService; private readonly IDataProtectorTokenFactory _registrationEmailVerificationTokenDataFactory; - private readonly byte[] _defaultKdfHmacKey = null; + private readonly byte[]? _defaultKdfHmacKey = null; private static readonly List _defaultKdfResults = [ // The first result (index 0) should always return the "normal" default. @@ -145,40 +141,55 @@ public class AccountsController : Controller [HttpPost("register/finish")] public async Task PostRegisterFinish([FromBody] RegisterFinishRequestModel model) { - var user = model.ToUser(); + User user = model.ToUser(); // Users will either have an emailed token or an email verification token - not both. - IdentityResult identityResult = null; + IdentityResult? identityResult = null; + + // PM-28143 - Just use the MasterPasswordAuthenticationData.MasterPasswordAuthenticationHash + string masterPasswordAuthenticationHash = model.MasterPasswordAuthentication?.MasterPasswordAuthenticationHash + ?? model.MasterPasswordHash!; switch (model.GetTokenType()) { case RegisterFinishTokenType.EmailVerification: - identityResult = - await _registerUserCommand.RegisterUserViaEmailVerificationToken(user, model.MasterPasswordHash, - model.EmailVerificationToken); - + identityResult = await _registerUserCommand.RegisterUserViaEmailVerificationToken( + user, + masterPasswordAuthenticationHash, + model.EmailVerificationToken!); return ProcessRegistrationResult(identityResult, user); + case RegisterFinishTokenType.OrganizationInvite: - identityResult = await _registerUserCommand.RegisterUserViaOrganizationInviteToken(user, model.MasterPasswordHash, - model.OrgInviteToken, model.OrganizationUserId); - + identityResult = await _registerUserCommand.RegisterUserViaOrganizationInviteToken( + user, + masterPasswordAuthenticationHash, + model.OrgInviteToken!, + model.OrganizationUserId); return ProcessRegistrationResult(identityResult, user); + case RegisterFinishTokenType.OrgSponsoredFreeFamilyPlan: - identityResult = await _registerUserCommand.RegisterUserViaOrganizationSponsoredFreeFamilyPlanInviteToken(user, model.MasterPasswordHash, model.OrgSponsoredFreeFamilyPlanToken); - + identityResult = await _registerUserCommand.RegisterUserViaOrganizationSponsoredFreeFamilyPlanInviteToken( + user, + masterPasswordAuthenticationHash, + model.OrgSponsoredFreeFamilyPlanToken!); return ProcessRegistrationResult(identityResult, user); + case RegisterFinishTokenType.EmergencyAccessInvite: - Debug.Assert(model.AcceptEmergencyAccessId.HasValue); - identityResult = await _registerUserCommand.RegisterUserViaAcceptEmergencyAccessInviteToken(user, model.MasterPasswordHash, - model.AcceptEmergencyAccessInviteToken, model.AcceptEmergencyAccessId.Value); - + identityResult = await _registerUserCommand.RegisterUserViaAcceptEmergencyAccessInviteToken( + user, + masterPasswordAuthenticationHash, + model.AcceptEmergencyAccessInviteToken!, + (Guid)model.AcceptEmergencyAccessId!); return ProcessRegistrationResult(identityResult, user); + case RegisterFinishTokenType.ProviderInvite: - Debug.Assert(model.ProviderUserId.HasValue); - identityResult = await _registerUserCommand.RegisterUserViaProviderInviteToken(user, model.MasterPasswordHash, - model.ProviderInviteToken, model.ProviderUserId.Value); - + identityResult = await _registerUserCommand.RegisterUserViaProviderInviteToken( + user, + masterPasswordAuthenticationHash, + model.ProviderInviteToken!, + (Guid)model.ProviderUserId!); return ProcessRegistrationResult(identityResult, user); + default: throw new BadRequestException("Invalid registration finish request"); } diff --git a/test/Api.IntegrationTest/Controllers/AccountsControllerTest.cs b/test/Api.IntegrationTest/Controllers/AccountsControllerTest.cs index d055418f3a..9860775e31 100644 --- a/test/Api.IntegrationTest/Controllers/AccountsControllerTest.cs +++ b/test/Api.IntegrationTest/Controllers/AccountsControllerTest.cs @@ -3,7 +3,6 @@ using System.Text.Json; using Bit.Api.Auth.Models.Request.Accounts; using Bit.Api.IntegrationTest.Factories; using Bit.Api.IntegrationTest.Helpers; -using Bit.Api.KeyManagement.Models.Requests; using Bit.Api.Models.Response; using Bit.Core; using Bit.Core.Auth.Entities; @@ -12,6 +11,7 @@ using Bit.Core.Auth.Models.Data; using Bit.Core.Auth.Repositories; using Bit.Core.Entities; using Bit.Core.Enums; +using Bit.Core.KeyManagement.Models.Api.Request; using Bit.Core.KeyManagement.Repositories; using Bit.Core.Models.Data; using Bit.Core.Platform.Push; @@ -378,7 +378,7 @@ public class AccountsControllerTest : IClassFixture, IAsy Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); var content = await response.Content.ReadAsStringAsync(); - Assert.Contains("KDF settings are invalid", content); + Assert.Contains("The model state is invalid", content); } [Fact] diff --git a/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs b/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs index 6cddd341d5..665d1e52c1 100644 --- a/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs +++ b/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs @@ -1,7 +1,6 @@ using System.Security.Claims; using Bit.Api.Auth.Controllers; using Bit.Api.Auth.Models.Request.Accounts; -using Bit.Api.KeyManagement.Models.Requests; using Bit.Core.AdminConsole.Repositories; using Bit.Core.AdminConsole.Services; using Bit.Core.Auth.Models.Api.Request.Accounts; diff --git a/test/Api.Test/Auth/Models/Request/Accounts/SetInitialPasswordRequestModelTests.cs b/test/Api.Test/Auth/Models/Request/Accounts/SetInitialPasswordRequestModelTests.cs index ce8ba1811e..97e69dacbc 100644 --- a/test/Api.Test/Auth/Models/Request/Accounts/SetInitialPasswordRequestModelTests.cs +++ b/test/Api.Test/Auth/Models/Request/Accounts/SetInitialPasswordRequestModelTests.cs @@ -1,6 +1,5 @@ using System.ComponentModel.DataAnnotations; using Bit.Api.Auth.Models.Request.Accounts; -using Bit.Api.KeyManagement.Models.Requests; using Bit.Core.Auth.Models.Api.Request.Accounts; using Bit.Core.Entities; using Bit.Core.Enums; diff --git a/test/Core.Test/Auth/AutoFixture/RegisterFinishRequestModelFixtures.cs b/test/Core.Test/Auth/AutoFixture/RegisterFinishRequestModelFixtures.cs index a751a16f31..22fca7ab59 100644 --- a/test/Core.Test/Auth/AutoFixture/RegisterFinishRequestModelFixtures.cs +++ b/test/Core.Test/Auth/AutoFixture/RegisterFinishRequestModelFixtures.cs @@ -29,7 +29,9 @@ internal class RegisterFinishRequestModelCustomization : ICustomization .With(o => o.OrgInviteToken, OrgInviteToken) .With(o => o.OrgSponsoredFreeFamilyPlanToken, OrgSponsoredFreeFamilyPlanToken) .With(o => o.AcceptEmergencyAccessInviteToken, AcceptEmergencyAccessInviteToken) - .With(o => o.ProviderInviteToken, ProviderInviteToken)); + .With(o => o.ProviderInviteToken, ProviderInviteToken) + .Without(o => o.MasterPasswordAuthentication) + .Without(o => o.MasterPasswordUnlock)); } } diff --git a/test/Core.Test/Auth/Models/Api/Request/Accounts/RegisterFinishRequestModelTests.cs b/test/Core.Test/Auth/Models/Api/Request/Accounts/RegisterFinishRequestModelTests.cs index 588ca878fc..3c099ce962 100644 --- a/test/Core.Test/Auth/Models/Api/Request/Accounts/RegisterFinishRequestModelTests.cs +++ b/test/Core.Test/Auth/Models/Api/Request/Accounts/RegisterFinishRequestModelTests.cs @@ -1,5 +1,6 @@ using Bit.Core.Auth.Models.Api.Request.Accounts; using Bit.Core.Enums; +using Bit.Core.KeyManagement.Models.Api.Request; using Bit.Test.Common.AutoFixture.Attributes; using Xunit; @@ -7,6 +8,17 @@ namespace Bit.Core.Test.Auth.Models.Api.Request.Accounts; public class RegisterFinishRequestModelTests { + private static List Validate(RegisterFinishRequestModel model) + { + var results = new List(); + System.ComponentModel.DataAnnotations.Validator.TryValidateObject( + model, + new System.ComponentModel.DataAnnotations.ValidationContext(model), + results, + true); + return results; + } + [Theory] [BitAutoData] public void GetTokenType_Returns_EmailVerification(string email, string masterPasswordHash, @@ -170,4 +182,175 @@ public class RegisterFinishRequestModelTests Assert.Equal(userAsymmetricKeys.PublicKey, result.PublicKey); Assert.Equal(userAsymmetricKeys.EncryptedPrivateKey, result.PrivateKey); } + + [Fact] + public void Validate_WhenBothAuthAndRootHashProvidedButNotEqual_ReturnsMismatchError() + { + var model = new RegisterFinishRequestModel + { + Email = "user@example.com", + MasterPasswordHash = "root-hash", + UserAsymmetricKeys = new KeysRequestModel { PublicKey = "pk", EncryptedPrivateKey = "sk" }, + // Provide both unlock and authentication with valid KDF so only the mismatch rule fires + MasterPasswordUnlock = new MasterPasswordUnlockDataRequestModel + { + Kdf = new KdfRequestModel { KdfType = KdfType.PBKDF2_SHA256, Iterations = AuthConstants.PBKDF2_ITERATIONS.Default }, + MasterKeyWrappedUserKey = "wrapped", + Salt = "salt" + }, + MasterPasswordAuthentication = new MasterPasswordAuthenticationDataRequestModel + { + Kdf = new KdfRequestModel { KdfType = KdfType.PBKDF2_SHA256, Iterations = AuthConstants.PBKDF2_ITERATIONS.Default }, + MasterPasswordAuthenticationHash = "auth-hash", // different than root + Salt = "salt" + }, + // Provide any valid token so we don't fail token validation + EmailVerificationToken = "token" + }; + + var results = Validate(model); + + Assert.Contains(results, r => + r.ErrorMessage == $"{nameof(MasterPasswordAuthenticationDataRequestModel.MasterPasswordAuthenticationHash)} and root level {nameof(RegisterFinishRequestModel.MasterPasswordHash)} provided and are not equal. Only provide one."); + } + + [Fact] + public void Validate_WhenAuthProvidedButUnlockMissing_ReturnsUnlockMissingError() + { + var model = new RegisterFinishRequestModel + { + Email = "user@example.com", + UserAsymmetricKeys = new KeysRequestModel { PublicKey = "pk", EncryptedPrivateKey = "sk" }, + MasterPasswordAuthentication = new MasterPasswordAuthenticationDataRequestModel + { + Kdf = new KdfRequestModel { KdfType = KdfType.PBKDF2_SHA256, Iterations = AuthConstants.PBKDF2_ITERATIONS.Default }, + MasterPasswordAuthenticationHash = "auth-hash", + Salt = "salt" + }, + EmailVerificationToken = "token" + }; + + var results = Validate(model); + + Assert.Contains(results, r => r.ErrorMessage == "MasterPasswordUnlock not found on RequestModel"); + } + + [Fact] + public void Validate_WhenUnlockProvidedButAuthMissing_ReturnsAuthMissingError() + { + var model = new RegisterFinishRequestModel + { + Email = "user@example.com", + UserAsymmetricKeys = new KeysRequestModel { PublicKey = "pk", EncryptedPrivateKey = "sk" }, + MasterPasswordUnlock = new MasterPasswordUnlockDataRequestModel + { + Kdf = new KdfRequestModel { KdfType = KdfType.PBKDF2_SHA256, Iterations = AuthConstants.PBKDF2_ITERATIONS.Default }, + MasterKeyWrappedUserKey = "wrapped", + Salt = "salt" + }, + EmailVerificationToken = "token" + }; + + var results = Validate(model); + + Assert.Contains(results, r => r.ErrorMessage == "MasterPasswordAuthentication not found on RequestModel"); + } + + [Fact] + public void Validate_WhenNeitherAuthNorUnlock_AndRootKdfMissing_ReturnsBothRootKdfErrors() + { + var model = new RegisterFinishRequestModel + { + Email = "user@example.com", + UserAsymmetricKeys = new KeysRequestModel { PublicKey = "pk", EncryptedPrivateKey = "sk" }, + // No MasterPasswordUnlock, no MasterPasswordAuthentication + // No root Kdf and KdfIterations to trigger both errors + EmailVerificationToken = "token" + }; + + var results = Validate(model); + + Assert.Contains(results, r => r.ErrorMessage == $"{nameof(RegisterFinishRequestModel.Kdf)} not found on RequestModel"); + Assert.Contains(results, r => r.ErrorMessage == $"{nameof(RegisterFinishRequestModel.KdfIterations)} not found on RequestModel"); + } + + [Fact] + public void Validate_WhenAuthAndRootHashBothMissing_ReturnsMissingHashErrorOnly() + { + var model = new RegisterFinishRequestModel + { + Email = "user@example.com", + UserAsymmetricKeys = new KeysRequestModel { PublicKey = "pk", EncryptedPrivateKey = "sk" }, + // Both MasterPasswordAuthentication and MasterPasswordHash are missing + MasterPasswordAuthentication = null, + MasterPasswordHash = null, + // Provide valid root KDF to avoid root KDF errors + Kdf = KdfType.PBKDF2_SHA256, + KdfIterations = AuthConstants.PBKDF2_ITERATIONS.Default, + EmailVerificationToken = "token" // avoid token error + }; + + var results = Validate(model); + + // Only the new missing hash error should be present + Assert.Single(results); + Assert.Equal($"{nameof(MasterPasswordAuthenticationDataRequestModel.MasterPasswordAuthenticationHash)} and {nameof(RegisterFinishRequestModel.MasterPasswordHash)} not found on request, one needs to be defined.", results[0].ErrorMessage); + Assert.Contains(nameof(MasterPasswordAuthenticationDataRequestModel.MasterPasswordAuthenticationHash), results[0].MemberNames); + Assert.Contains(nameof(RegisterFinishRequestModel.MasterPasswordHash), results[0].MemberNames); + } + + [Fact] + public void Validate_WhenAllFieldsValidWithSubModels_IsValid() + { + var model = new RegisterFinishRequestModel + { + Email = "user@example.com", + UserAsymmetricKeys = new KeysRequestModel { PublicKey = "pk", EncryptedPrivateKey = "sk" }, + MasterPasswordUnlock = new MasterPasswordUnlockDataRequestModel + { + Kdf = new KdfRequestModel { KdfType = KdfType.PBKDF2_SHA256, Iterations = AuthConstants.PBKDF2_ITERATIONS.Default }, + MasterKeyWrappedUserKey = "wrapped", + Salt = "salt" + }, + MasterPasswordAuthentication = new MasterPasswordAuthenticationDataRequestModel + { + Kdf = new KdfRequestModel { KdfType = KdfType.PBKDF2_SHA256, Iterations = AuthConstants.PBKDF2_ITERATIONS.Default }, + MasterPasswordAuthenticationHash = "auth-hash", + Salt = "salt" + }, + EmailVerificationToken = "token" + }; + + var results = Validate(model); + + Assert.Empty(results); + } + + [Fact] + public void Validate_WhenNoValidRegistrationTokenProvided_ReturnsTokenErrorOnly() + { + var model = new RegisterFinishRequestModel + { + Email = "user@example.com", + UserAsymmetricKeys = new KeysRequestModel { PublicKey = "pk", EncryptedPrivateKey = "sk" }, + MasterPasswordUnlock = new MasterPasswordUnlockDataRequestModel + { + Kdf = new KdfRequestModel { KdfType = KdfType.PBKDF2_SHA256, Iterations = AuthConstants.PBKDF2_ITERATIONS.Default }, + MasterKeyWrappedUserKey = "wrapped", + Salt = "salt" + }, + MasterPasswordAuthentication = new MasterPasswordAuthenticationDataRequestModel + { + Kdf = new KdfRequestModel { KdfType = KdfType.PBKDF2_SHA256, Iterations = AuthConstants.PBKDF2_ITERATIONS.Default }, + MasterPasswordAuthenticationHash = "auth-hash", + Salt = "salt" + } + // No token fields set + }; + + var results = Validate(model); + + Assert.Single(results); + Assert.Equal("No valid registration token provided", results[0].ErrorMessage); + } } diff --git a/test/Identity.Test/Controllers/AccountsControllerTests.cs b/test/Identity.Test/Controllers/AccountsControllerTests.cs index 42e033bdd7..86e461d155 100644 --- a/test/Identity.Test/Controllers/AccountsControllerTests.cs +++ b/test/Identity.Test/Controllers/AccountsControllerTests.cs @@ -1,4 +1,5 @@ -using System.Reflection; +using System.ComponentModel.DataAnnotations; +using System.Reflection; using System.Text; using Bit.Core; using Bit.Core.Auth.Models.Api.Request.Accounts; @@ -9,6 +10,7 @@ using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; +using Bit.Core.KeyManagement.Models.Api.Request; using Bit.Core.Models.Data; using Bit.Core.Repositories; using Bit.Core.Services; @@ -590,6 +592,504 @@ public class AccountsControllerTests : IDisposable await Assert.ThrowsAsync(() => _sut.PostRegisterVerificationEmailClicked(requestModel)); } + // PM-28143 - When removing the old properties, update this test to just test the new properties working + // as expected. + [Theory, BitAutoData] + public async Task PostRegisterFinish_EmailVerification_BothDataForms_ProduceEquivalentOutcomes( + string email, + string emailVerificationToken, + string masterPasswordHash, + string masterKeyWrappedUserKey, + string publicKey, + string encryptedPrivateKey) + { + // Arrange: new-form model (MasterPasswordAuthenticationData + MasterPasswordUnlockData) + + var kdfData = new KdfRequestModel + { + KdfType = KdfType.Argon2id, + Iterations = AuthConstants.ARGON2_ITERATIONS.Default, + Memory = AuthConstants.ARGON2_MEMORY.Default, + Parallelism = AuthConstants.ARGON2_PARALLELISM.Default + }; + + var newModel = new RegisterFinishRequestModel + { + Email = email, + EmailVerificationToken = emailVerificationToken, + MasterPasswordAuthentication = new MasterPasswordAuthenticationDataRequestModel + { + Kdf = kdfData, + MasterPasswordAuthenticationHash = masterPasswordHash, + Salt = email // salt choice is not validated here during registration + }, + MasterPasswordUnlock = new MasterPasswordUnlockDataRequestModel + { + Kdf = kdfData, + MasterKeyWrappedUserKey = masterKeyWrappedUserKey, + Salt = email + }, + UserAsymmetricKeys = new KeysRequestModel + { + PublicKey = publicKey, + EncryptedPrivateKey = encryptedPrivateKey + } + }; + + // Arrange: legacy-form model (MasterPasswordHash + legacy KDF + UserSymmetricKey) + var legacyModel = new RegisterFinishRequestModel + { + Email = email, + EmailVerificationToken = emailVerificationToken, + MasterPasswordHash = masterPasswordHash, + Kdf = KdfType.Argon2id, + KdfIterations = AuthConstants.ARGON2_ITERATIONS.Default, + KdfMemory = AuthConstants.ARGON2_MEMORY.Default, + KdfParallelism = AuthConstants.ARGON2_PARALLELISM.Default, + UserSymmetricKey = masterKeyWrappedUserKey, + UserAsymmetricKeys = new KeysRequestModel + { + PublicKey = publicKey, + EncryptedPrivateKey = encryptedPrivateKey + } + }; + + var newUser = newModel.ToUser(); + var legacyUser = legacyModel.ToUser(); + + _registerUserCommand + .RegisterUserViaEmailVerificationToken(Arg.Any(), masterPasswordHash, emailVerificationToken) + .Returns(Task.FromResult(IdentityResult.Success)); + + // Act: call with new form + var newResult = await _sut.PostRegisterFinish(newModel); + // Act: call with legacy form + var legacyResult = await _sut.PostRegisterFinish(legacyModel); + + // Assert: outcomes are identical in effect (success response) + Assert.NotNull(newResult); + Assert.NotNull(legacyResult); + + // Assert: effective users are equivalent + Assert.Equal(legacyUser.Email, newUser.Email); + Assert.Equal(legacyUser.MasterPasswordHint, newUser.MasterPasswordHint); + Assert.Equal(legacyUser.Kdf, newUser.Kdf); + Assert.Equal(legacyUser.KdfIterations, newUser.KdfIterations); + Assert.Equal(legacyUser.KdfMemory, newUser.KdfMemory); + Assert.Equal(legacyUser.KdfParallelism, newUser.KdfParallelism); + Assert.Equal(legacyUser.Key, newUser.Key); + Assert.Equal(legacyUser.PublicKey, newUser.PublicKey); + Assert.Equal(legacyUser.PrivateKey, newUser.PrivateKey); + + // Assert: hash forwarded identically from both inputs + await _registerUserCommand.Received(2).RegisterUserViaEmailVerificationToken( + Arg.Is(u => + u.Email == newUser.Email && + u.Kdf == newUser.Kdf && + u.KdfIterations == newUser.KdfIterations && + u.KdfMemory == newUser.KdfMemory && + u.KdfParallelism == newUser.KdfParallelism && + u.Key == newUser.Key), + masterPasswordHash, + emailVerificationToken); + + await _registerUserCommand.Received(2).RegisterUserViaEmailVerificationToken( + Arg.Is(u => + u.Email == legacyUser.Email && + u.Kdf == legacyUser.Kdf && + u.KdfIterations == legacyUser.KdfIterations && + u.KdfMemory == legacyUser.KdfMemory && + u.KdfParallelism == legacyUser.KdfParallelism && + u.Key == legacyUser.Key), + masterPasswordHash, + emailVerificationToken); + } + + // PM-28143 - When removing the old properties, update this test to just test the new properties working + // as expected. + [Theory, BitAutoData] + public async Task PostRegisterFinish_OrgInvite_BothDataForms_ProduceEquivalentOutcomes( + string email, + string orgInviteToken, + Guid organizationUserId, + string masterPasswordHash, + string masterKeyWrappedUserKey, + string publicKey, + string encryptedPrivateKey) + { + var kdfData = new KdfRequestModel + { + KdfType = KdfType.Argon2id, + Iterations = AuthConstants.ARGON2_ITERATIONS.Default, + Memory = AuthConstants.ARGON2_MEMORY.Default, + Parallelism = AuthConstants.ARGON2_PARALLELISM.Default + }; + + // Arrange: new-form model (MasterPasswordAuthenticationData + MasterPasswordUnlockData) + var newModel = new RegisterFinishRequestModel + { + Email = email, + OrgInviteToken = orgInviteToken, + OrganizationUserId = organizationUserId, + MasterPasswordAuthentication = new MasterPasswordAuthenticationDataRequestModel + { + Kdf = kdfData, + MasterPasswordAuthenticationHash = masterPasswordHash, + Salt = email + }, + MasterPasswordUnlock = new MasterPasswordUnlockDataRequestModel + { + Kdf = kdfData, + MasterKeyWrappedUserKey = masterKeyWrappedUserKey, + Salt = email + }, + UserAsymmetricKeys = new KeysRequestModel + { + PublicKey = publicKey, + EncryptedPrivateKey = encryptedPrivateKey + } + }; + + // Arrange: legacy-form model (MasterPasswordHash + legacy KDF + UserSymmetricKey) + var legacyModel = new RegisterFinishRequestModel + { + Email = email, + OrgInviteToken = orgInviteToken, + OrganizationUserId = organizationUserId, + MasterPasswordHash = masterPasswordHash, + Kdf = kdfData.KdfType, + KdfIterations = kdfData.Iterations, + KdfMemory = kdfData.Memory, + KdfParallelism = kdfData.Parallelism, + UserSymmetricKey = masterKeyWrappedUserKey, + UserAsymmetricKeys = new KeysRequestModel + { + PublicKey = publicKey, + EncryptedPrivateKey = encryptedPrivateKey + } + }; + + var newUser = newModel.ToUser(); + var legacyUser = legacyModel.ToUser(); + + _registerUserCommand + .RegisterUserViaOrganizationInviteToken(Arg.Any(), masterPasswordHash, orgInviteToken, organizationUserId) + .Returns(Task.FromResult(IdentityResult.Success)); + + // Act + var newResult = await _sut.PostRegisterFinish(newModel); + var legacyResult = await _sut.PostRegisterFinish(legacyModel); + + // Assert success + Assert.NotNull(newResult); + Assert.NotNull(legacyResult); + + // Assert: effective users are equivalent + Assert.Equal(legacyUser.Email, newUser.Email); + Assert.Equal(legacyUser.MasterPasswordHint, newUser.MasterPasswordHint); + Assert.Equal(legacyUser.Kdf, newUser.Kdf); + Assert.Equal(legacyUser.KdfIterations, newUser.KdfIterations); + Assert.Equal(legacyUser.KdfMemory, newUser.KdfMemory); + Assert.Equal(legacyUser.KdfParallelism, newUser.KdfParallelism); + Assert.Equal(legacyUser.Key, newUser.Key); + Assert.Equal(legacyUser.PublicKey, newUser.PublicKey); + Assert.Equal(legacyUser.PrivateKey, newUser.PrivateKey); + + // Assert: hash forwarded identically from both inputs + await _registerUserCommand.Received(2).RegisterUserViaOrganizationInviteToken( + Arg.Is(u => + u.Email == newUser.Email && + u.Kdf == newUser.Kdf && + u.KdfIterations == newUser.KdfIterations && + u.KdfMemory == newUser.KdfMemory && + u.KdfParallelism == newUser.KdfParallelism && + u.Key == newUser.Key), + masterPasswordHash, + orgInviteToken, + organizationUserId); + + await _registerUserCommand.Received(2).RegisterUserViaOrganizationInviteToken( + Arg.Is(u => + u.Email == legacyUser.Email && + u.Kdf == legacyUser.Kdf && + u.KdfIterations == legacyUser.KdfIterations && + u.KdfMemory == legacyUser.KdfMemory && + u.KdfParallelism == legacyUser.KdfParallelism && + u.Key == legacyUser.Key), + masterPasswordHash, + orgInviteToken, + organizationUserId); + } + + [Theory, BitAutoData] + public async Task PostRegisterFinish_NewForm_UsesUnlockDataForKdfAndKey_WhenRootFieldsNull( + string email, + string emailVerificationToken, + string masterPasswordHash, + string masterKeyWrappedUserKey, + int iterations, + string publicKey, + string encryptedPrivateKey) + { + // Arrange: Provide only unlock-data KDF + key; leave root KDF fields null + var unlockKdf = new KdfRequestModel + { + KdfType = KdfType.PBKDF2_SHA256, + Iterations = iterations + }; + + var model = new RegisterFinishRequestModel + { + Email = email, + EmailVerificationToken = emailVerificationToken, + MasterPasswordAuthentication = new MasterPasswordAuthenticationDataRequestModel + { + // present but not used by ToUser for KDF/Key + Kdf = unlockKdf, + MasterPasswordAuthenticationHash = masterPasswordHash, + Salt = email + }, + MasterPasswordUnlock = new MasterPasswordUnlockDataRequestModel + { + Kdf = unlockKdf, + MasterKeyWrappedUserKey = masterKeyWrappedUserKey, + Salt = email + }, + // root KDF fields intentionally null + Kdf = null, + KdfIterations = null, + UserAsymmetricKeys = new KeysRequestModel + { + PublicKey = publicKey, + EncryptedPrivateKey = encryptedPrivateKey + } + }; + + _registerUserCommand + .RegisterUserViaEmailVerificationToken(Arg.Any(), masterPasswordHash, emailVerificationToken) + .Returns(Task.FromResult(IdentityResult.Success)); + + // Act + var _ = await _sut.PostRegisterFinish(model); + + // Assert: The user passed to command uses unlock-data values + await _registerUserCommand.Received(1).RegisterUserViaEmailVerificationToken( + Arg.Is(u => + u.Email == email && + u.Kdf == unlockKdf.KdfType && + u.KdfIterations == unlockKdf.Iterations && + u.Key == masterKeyWrappedUserKey), + masterPasswordHash, + emailVerificationToken); + } + + [Theory, BitAutoData] + public async Task PostRegisterFinish_LegacyForm_UsesRootFields_WhenUnlockDataNull( + string email, + string emailVerificationToken, + string masterPasswordHash, + string legacyKey, + string publicKey, + string encryptedPrivateKey) + { + // Arrange: Provide only legacy root KDF + key; no unlock-data provided + var model = new RegisterFinishRequestModel + { + Email = email, + EmailVerificationToken = emailVerificationToken, + MasterPasswordHash = masterPasswordHash, + Kdf = KdfType.PBKDF2_SHA256, + KdfIterations = AuthConstants.PBKDF2_ITERATIONS.Default, + UserSymmetricKey = legacyKey, + MasterPasswordUnlock = null, + UserAsymmetricKeys = new KeysRequestModel + { + PublicKey = publicKey, + EncryptedPrivateKey = encryptedPrivateKey + } + }; + + _registerUserCommand + .RegisterUserViaEmailVerificationToken(Arg.Any(), masterPasswordHash, emailVerificationToken) + .Returns(Task.FromResult(IdentityResult.Success)); + + // Act + var _ = await _sut.PostRegisterFinish(model); + + // Assert: The user passed to command uses root values + await _registerUserCommand.Received(1).RegisterUserViaEmailVerificationToken( + Arg.Is(u => + u.Email == email && + u.Kdf == KdfType.PBKDF2_SHA256 && + u.KdfIterations == AuthConstants.PBKDF2_ITERATIONS.Default && + u.Key == legacyKey), + masterPasswordHash, + emailVerificationToken); + } + + [Theory, BitAutoData] + public void RegisterFinishRequestModel_Validate_Throws_WhenUnlockAndAuthDataMismatch( + string email, + string authHash, + string masterKeyWrappedUserKey, + string publicKey, + string encryptedPrivateKey) + { + // Arrange: authentication and unlock have different KDF and/or salt + var authKdf = new KdfRequestModel + { + KdfType = KdfType.PBKDF2_SHA256, + Iterations = AuthConstants.PBKDF2_ITERATIONS.Default + }; + var unlockKdf = new KdfRequestModel + { + KdfType = KdfType.Argon2id, + Iterations = AuthConstants.ARGON2_ITERATIONS.Default, + Memory = AuthConstants.ARGON2_MEMORY.Default, + Parallelism = AuthConstants.ARGON2_PARALLELISM.Default + }; + + var model = new RegisterFinishRequestModel + { + Email = email, + MasterPasswordAuthentication = new MasterPasswordAuthenticationDataRequestModel + { + Kdf = authKdf, + MasterPasswordAuthenticationHash = authHash, + Salt = email + }, + MasterPasswordUnlock = new MasterPasswordUnlockDataRequestModel + { + Kdf = unlockKdf, + MasterKeyWrappedUserKey = masterKeyWrappedUserKey, + Salt = email + }, + UserAsymmetricKeys = new KeysRequestModel + { + PublicKey = publicKey, + EncryptedPrivateKey = encryptedPrivateKey + } + }; + + // Provide a minimal valid token type to satisfy model-level token validation + model.EmailVerificationToken = "test-token"; + + var ctx = new ValidationContext(model); + + // Act + var results = model.Validate(ctx).ToList(); + + // Assert mismatched auth/unlock is allowed + Assert.Empty(results); + } + + [Theory, BitAutoData] + public void RegisterFinishRequestModel_Validate_Throws_WhenSaltMismatch( + string email, + string authHash, + string masterKeyWrappedUserKey, + string publicKey, + string encryptedPrivateKey) + { + var unlockKdf = new KdfRequestModel + { + KdfType = KdfType.Argon2id, + Iterations = AuthConstants.ARGON2_ITERATIONS.Default, + Memory = AuthConstants.ARGON2_MEMORY.Default, + Parallelism = AuthConstants.ARGON2_PARALLELISM.Default + }; + + var model = new RegisterFinishRequestModel + { + Email = email, + MasterPasswordAuthentication = new MasterPasswordAuthenticationDataRequestModel + { + Kdf = unlockKdf, + MasterPasswordAuthenticationHash = authHash, + Salt = email + }, + MasterPasswordUnlock = new MasterPasswordUnlockDataRequestModel + { + Kdf = unlockKdf, + MasterKeyWrappedUserKey = masterKeyWrappedUserKey, + // Intentionally different salt to force mismatch + Salt = email + ".mismatch" + }, + UserAsymmetricKeys = new KeysRequestModel + { + PublicKey = publicKey, + EncryptedPrivateKey = encryptedPrivateKey + } + }; + + // Provide a minimal valid token type to satisfy model-level token validation + model.EmailVerificationToken = "test-token"; + + var ctx = new ValidationContext(model); + + // Act + var results = model.Validate(ctx).ToList(); + + // Assert mismatched salts between auth/unlock are allowed + Assert.Empty(results); + } + + [Theory, BitAutoData] + public void RegisterFinishRequestModel_Validate_Throws_WhenAuthHashAndRootHashMismatch( + string email, + string authHash, + string differentRootHash, + string masterKeyWrappedUserKey, + string publicKey, + string encryptedPrivateKey) + { + // Arrange: same KDF/salt, but authentication hash differs from legacy root hash + var kdf = new KdfRequestModel + { + KdfType = KdfType.PBKDF2_SHA256, + Iterations = AuthConstants.PBKDF2_ITERATIONS.Default + }; + + var model = new RegisterFinishRequestModel + { + Email = email, + MasterPasswordAuthentication = new MasterPasswordAuthenticationDataRequestModel + { + Kdf = kdf, + MasterPasswordAuthenticationHash = authHash, + Salt = email + }, + MasterPasswordUnlock = new MasterPasswordUnlockDataRequestModel + { + Kdf = kdf, + MasterKeyWrappedUserKey = masterKeyWrappedUserKey, + Salt = email + }, + // Intentionally set the legacy field to a different value to trigger the throw + MasterPasswordHash = differentRootHash, + UserAsymmetricKeys = new KeysRequestModel + { + PublicKey = publicKey, + EncryptedPrivateKey = encryptedPrivateKey + } + }; + + // Provide a minimal valid token type to satisfy model-level token validation + model.EmailVerificationToken = "test-token"; + + var ctx = new ValidationContext(model); + + // Act + var results = model.Validate(ctx).ToList(); + + // Assert: validation result exists with expected message and member names + var mismatchResult = Assert.Single(results.Where(r => + r.ErrorMessage == + "MasterPasswordAuthenticationHash and root level MasterPasswordHash provided and are not equal. Only provide one.")); + Assert.Contains("MasterPasswordAuthenticationHash", mismatchResult.MemberNames); + Assert.Contains("MasterPasswordHash", mismatchResult.MemberNames); + } + private void SetDefaultKdfHmacKey(byte[]? newKey) { var fieldInfo = typeof(AccountsController).GetField("_defaultKdfHmacKey", BindingFlags.NonPublic | BindingFlags.Instance); diff --git a/test/IntegrationTestCommon/Factories/IdentityApplicationFactory.cs b/test/IntegrationTestCommon/Factories/IdentityApplicationFactory.cs index ba12d1e1f4..e190dda427 100644 --- a/test/IntegrationTestCommon/Factories/IdentityApplicationFactory.cs +++ b/test/IntegrationTestCommon/Factories/IdentityApplicationFactory.cs @@ -3,10 +3,13 @@ using System.Collections.Concurrent; using System.Net.Http.Json; +using System.Text; using System.Text.Json; +using Bit.Core; using Bit.Core.Auth.Models.Api.Request.Accounts; using Bit.Core.Entities; using Bit.Core.Enums; +using Bit.Core.KeyManagement.Models.Api.Request; using Bit.Core.Services; using Bit.Identity; using Bit.Test.Common.Helpers; @@ -23,6 +26,7 @@ public class IdentityApplicationFactory : WebApplicationFactoryBase public const string DefaultDeviceIdentifier = "92b9d953-b9b6-4eaf-9d3e-11d57144dfeb"; public const string DefaultUserEmail = "DefaultEmail@bitwarden.com"; public const string DefaultUserPasswordHash = "default_password_hash"; + private const string DefaultEncryptedString = "2.3Uk+WNBIoU5xzmVFNcoWzz==|1MsPIYuRfdOHfu/0uY6H2Q==|/98sp4wb6pHP1VTZ9JcNCYgQjEUMFPlqJgCwRk1YXKg="; /// /// A dictionary to store registration tokens for email verification. We cannot substitute the IMailService more than once, so @@ -195,6 +199,68 @@ public class IdentityApplicationFactory : WebApplicationFactoryBase RegisterFinishRequestModel requestModel, bool marketingEmails = true) { + // Ensure required fields for registration finish are present. + // Prefer legacy-path defaults (root fields) to minimize changes to tests. + // PM-28143 - When MasterPasswordAuthenticationData is required, delete all handling of MasterPasswordHash. + requestModel.MasterPasswordHash ??= DefaultUserPasswordHash; + // PM-28143 - When KDF is sourced exclusively from MasterPasswordUnlockData, delete the root Kdf defaults below. + requestModel.Kdf ??= KdfType.PBKDF2_SHA256; + requestModel.KdfIterations ??= AuthConstants.PBKDF2_ITERATIONS.Default; + // Ensure a symmetric key is provided when no unlock data is present + // PM-28143 - When MasterPasswordUnlockData is required, delete the UserSymmetricKey fallback block below. + if (requestModel.MasterPasswordUnlock == null && string.IsNullOrWhiteSpace(requestModel.UserSymmetricKey)) + { + requestModel.UserSymmetricKey = "user_symmetric_key"; + } + + // Align unlock/auth data KDF with root KDF so login uses the provided master password hash. + // PM-28143 - After removing root Kdf fields, build KDF exclusively from MasterPasswordUnlockData.Kdf and delete this alignment section. + var effectiveKdfType = requestModel.Kdf ?? KdfType.PBKDF2_SHA256; + var effectiveIterations = requestModel.KdfIterations ?? AuthConstants.PBKDF2_ITERATIONS.Default; + int? effectiveMemory = null; + int? effectiveParallelism = null; + if (effectiveKdfType == KdfType.Argon2id) + { + effectiveIterations = AuthConstants.ARGON2_ITERATIONS.InsideRange(effectiveIterations) + ? effectiveIterations + : AuthConstants.ARGON2_ITERATIONS.Default; + effectiveMemory = AuthConstants.ARGON2_MEMORY.Default; + effectiveParallelism = AuthConstants.ARGON2_PARALLELISM.Default; + } + + var alignedKdf = new KdfRequestModel + { + KdfType = effectiveKdfType, + Iterations = effectiveIterations, + Memory = effectiveMemory, + Parallelism = effectiveParallelism + }; + + if (requestModel.MasterPasswordUnlock != null) + { + var unlock = requestModel.MasterPasswordUnlock; + // Always force a valid encrypted string for tests to avoid model validation failures. + requestModel.MasterPasswordUnlock = new MasterPasswordUnlockDataRequestModel + { + Kdf = alignedKdf, + MasterKeyWrappedUserKey = unlock.MasterKeyWrappedUserKey, + Salt = string.IsNullOrWhiteSpace(unlock.Salt) ? requestModel.Email : unlock.Salt + }; + } + + if (requestModel.MasterPasswordAuthentication != null) + { + // Ensure registration uses the same hash the tests will provide at login. + // PM-28143 - When MasterPasswordAuthenticationData is the only source of the auth hash, + // stop overriding it from MasterPasswordHash and delete this whole reassignment block. + requestModel.MasterPasswordAuthentication = new MasterPasswordAuthenticationDataRequestModel + { + Kdf = alignedKdf, + MasterPasswordAuthenticationHash = requestModel.MasterPasswordHash, + Salt = requestModel.Email + }; + } + var sendVerificationEmailReqModel = new RegisterSendVerificationEmailRequestModel { Email = requestModel.Email, @@ -211,8 +277,11 @@ public class IdentityApplicationFactory : WebApplicationFactoryBase requestModel.EmailVerificationToken = RegistrationTokens[requestModel.Email]; var postRegisterFinishHttpContext = await PostRegisterFinishAsync(requestModel); - - Assert.Equal(StatusCodes.Status200OK, postRegisterFinishHttpContext.Response.StatusCode); + if (postRegisterFinishHttpContext.Response.StatusCode != StatusCodes.Status200OK) + { + var body = await ReadResponseBodyAsync(postRegisterFinishHttpContext); + Assert.Fail($"register/finish failed (status {postRegisterFinishHttpContext.Response.StatusCode}). Body: {body}"); + } var database = GetDatabaseContext(); var user = await database.Users @@ -222,4 +291,32 @@ public class IdentityApplicationFactory : WebApplicationFactoryBase return user; } + + private static async Task ReadResponseBodyAsync(HttpContext ctx) + { + try + { + if (ctx?.Response?.Body == null) + { + return ""; + } + var stream = ctx.Response.Body; + if (stream.CanSeek) + { + stream.Seek(0, SeekOrigin.Begin); + } + using var reader = new StreamReader(stream, Encoding.UTF8, detectEncodingFromByteOrderMarks: false, leaveOpen: true); + var text = await reader.ReadToEndAsync(); + if (stream.CanSeek) + { + stream.Seek(0, SeekOrigin.Begin); + } + return string.IsNullOrWhiteSpace(text) ? "" : text; + } + catch (Exception ex) + { + return $""; + } + } + }