From aef7ea8c20a5e091fa5337eb8d698905f4ddf7e1 Mon Sep 17 00:00:00 2001 From: enmande <3836813+enmande@users.noreply.github.com> Date: Tue, 21 Apr 2026 11:39:16 -0400 Subject: [PATCH 01/26] feat(mp-service): Add MasterPasswordService foundation. --- .../SetInitialOrUpdateExistingPasswordData.cs | 43 +++ .../Data/SetInitialPasswordData.cs | 67 +++++ .../Data/UpdateExistingPasswordAndKdfData.cs | 55 ++++ .../Data/UpdateExistingPasswordData.cs | 57 ++++ .../Interfaces/IMasterPasswordService.cs | 162 +++++++++++ .../MasterPasswordService.cs | 254 ++++++++++++++++++ 6 files changed, 638 insertions(+) create mode 100644 src/Core/Auth/UserFeatures/UserMasterPassword/Data/SetInitialOrUpdateExistingPasswordData.cs create mode 100644 src/Core/Auth/UserFeatures/UserMasterPassword/Data/SetInitialPasswordData.cs create mode 100644 src/Core/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordAndKdfData.cs create mode 100644 src/Core/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordData.cs create mode 100644 src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs create mode 100644 src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/Data/SetInitialOrUpdateExistingPasswordData.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/Data/SetInitialOrUpdateExistingPasswordData.cs new file mode 100644 index 000000000000..80c75d68141c --- /dev/null +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/Data/SetInitialOrUpdateExistingPasswordData.cs @@ -0,0 +1,43 @@ +using Bit.Core.KeyManagement.Models.Data; + +namespace Bit.Core.Auth.UserFeatures.UserMasterPassword.Data; + +public class SetInitialOrUpdateExistingPasswordData +{ + public required MasterPasswordAuthenticationData MasterPasswordAuthentication { get; set; } + public required MasterPasswordUnlockData MasterPasswordUnlock { get; set; } + + /// + /// When true, runs the new password hash through the registered + /// pipeline before hashing. + /// Set to false only in flows where password policy validation has already been enforced + /// (e.g. admin-initiated recovery). Defaults to true. + /// + public bool ValidatePassword { get; set; } = true; + /// + /// When true, rotates , which invalidates + /// all active sessions and authentication tokens for the user. Set to false only when + /// intentionally preserving existing sessions. Defaults to true. + /// + public bool RefreshStamp { get; set; } = true; + + public string? MasterPasswordHint { get; set; } = null; + + public SetInitialPasswordData ToSetInitialData() => new() + { + MasterPasswordAuthentication = MasterPasswordAuthentication, + MasterPasswordUnlock = MasterPasswordUnlock, + ValidatePassword = ValidatePassword, + RefreshStamp = RefreshStamp, + MasterPasswordHint = MasterPasswordHint + }; + + public UpdateExistingPasswordData ToUpdateExistingData() => new() + { + MasterPasswordAuthentication = MasterPasswordAuthentication, + MasterPasswordUnlock = MasterPasswordUnlock, + ValidatePassword = ValidatePassword, + RefreshStamp = RefreshStamp, + MasterPasswordHint = MasterPasswordHint + }; +} diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/Data/SetInitialPasswordData.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/Data/SetInitialPasswordData.cs new file mode 100644 index 000000000000..d4be56836688 --- /dev/null +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/Data/SetInitialPasswordData.cs @@ -0,0 +1,67 @@ +using Bit.Core.Entities; +using Bit.Core.Exceptions; +using Bit.Core.KeyManagement.Models.Data; + +namespace Bit.Core.Auth.UserFeatures.UserMasterPassword.Data; + +public class SetInitialPasswordData +{ + public required MasterPasswordAuthenticationData MasterPasswordAuthentication { get; set; } + public required MasterPasswordUnlockData MasterPasswordUnlock { get; set; } + + /// + /// When true, runs the new password hash through the registered + /// pipeline before hashing. + /// Set to false only in flows where password policy validation has already been enforced + /// (e.g. admin-initiated recovery). Defaults to true. + /// + public bool ValidatePassword { get; set; } = true; + /// + /// When true, rotates , which invalidates + /// all active sessions and authentication tokens for the user. Set to false only when + /// intentionally preserving existing sessions. Defaults to true. + /// + public bool RefreshStamp { get; set; } = true; + + public string? MasterPasswordHint { get; set; } = null; + + public void ValidateDataForUser(User user) + { + // Validate that the user does not have a master password set. + if (user.HasMasterPassword()) + { + throw new BadRequestException("User already has a master password set."); + } + + // Validate that there is no key set since there is no master password. The key + // and MasterPassword property are siblings in that they should either both be + // present or both be null, even for all TDE/KeyConnector users. + if (user.Key != null) + { + throw new BadRequestException("User already has a key set."); + } + + // Validate that there is no salt set. + if (user.MasterPasswordSalt != null) + { + throw new BadRequestException("User already has a master password set."); + } + + // Once a user is in the KeyConnector state they cannot become a master password + // user ever again so we can check here to make sure that they shouldn't ever be + // setting a password + if (user.UsesKeyConnector) + { + throw new BadRequestException("Cannot set an initial password of a user with Key Connector."); + } + + // Compatibility-window invariant: during Stage 1 of email-salt separation (PM-27044), + // the client MUST send salt == email.lower.trim on initial SET. The server cannot yet + // handle divergent salts; GetMasterPasswordSalt() falls back to email when MasterPasswordSalt + // is null, and a mismatch here would make the user un-decryptable on next login. Centralized + // here so both TDE and SSO JIT initial-SET flows enforce the same rule. This check is + // removed in Stage 3 when PM-28143 feature flag clears and independent salts are safe. + MasterPasswordAuthentication.ValidateSaltUnchangedForUser(user); + MasterPasswordUnlock.ValidateSaltUnchangedForUser(user); + } +} diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordAndKdfData.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordAndKdfData.cs new file mode 100644 index 000000000000..3d29bad15656 --- /dev/null +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordAndKdfData.cs @@ -0,0 +1,55 @@ +using Bit.Core.Entities; +using Bit.Core.Exceptions; +using Bit.Core.KeyManagement.Models.Data; + +namespace Bit.Core.Auth.UserFeatures.UserMasterPassword.Data; + +public class UpdateExistingPasswordAndKdfData +{ + public required MasterPasswordAuthenticationData MasterPasswordAuthentication { get; set; } + public required MasterPasswordUnlockData MasterPasswordUnlock { get; set; } + + /// + /// When true, runs the new password hash through the registered + /// pipeline before hashing. + /// Set to false only in flows where password policy validation has already been enforced + /// (e.g. admin-initiated recovery). Defaults to true. + /// + public bool ValidatePassword { get; set; } = true; + /// + /// When true, rotates , which invalidates + /// all active sessions and authentication tokens for the user. Set to false only when + /// intentionally preserving existing sessions. Defaults to true. + /// + public bool RefreshStamp { get; set; } = true; + + public string? MasterPasswordHint { get; set; } = null; + + public void ValidateDataForUser(User user) + { + // Validate that the user has a master password already, if not then they shouldn't be updating they should + // be setting initial. + if (!user.HasMasterPassword()) + { + throw new BadRequestException("User does not have an existing master password to update."); + } + + // KDF parameters govern how the master password is stretched into the encryption key. + // Key Connector replaces the master password entirely — the encryption key is managed + // by an external service — so KDF rotation has no meaningful target. The existing + // ChangeKdfCommand blocks this implicitly (CheckPasswordAsync fails against a null + // master password), but this guard makes the categorical inapplicability explicit. + // Note: org owners/admins cannot be KC users (enforced at conversion time in + // UserService.CheckCanUseKeyConnector), so no role-based edge case exists. + if (user.UsesKeyConnector) + { + throw new BadRequestException("Cannot update password of a user with Key Connector."); + } + + // Do not validate if kdf is the same here on the user because we are changing it. + + // Validate Salt is unchanged for user + MasterPasswordAuthentication.ValidateSaltUnchangedForUser(user); + MasterPasswordUnlock.ValidateSaltUnchangedForUser(user); + } +} diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordData.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordData.cs new file mode 100644 index 000000000000..0be0322cceb1 --- /dev/null +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordData.cs @@ -0,0 +1,57 @@ +using Bit.Core.Entities; +using Bit.Core.Exceptions; +using Bit.Core.KeyManagement.Models.Data; + +namespace Bit.Core.Auth.UserFeatures.UserMasterPassword.Data; + +public class UpdateExistingPasswordData +{ + public required MasterPasswordAuthenticationData MasterPasswordAuthentication { get; set; } + public required MasterPasswordUnlockData MasterPasswordUnlock { get; set; } + + /// + /// When true, runs the new password hash through the registered + /// pipeline before hashing. + /// Set to false only in flows where password policy validation has already been enforced + /// (e.g. admin-initiated recovery). Defaults to true. + /// + public bool ValidatePassword { get; set; } = true; + /// + /// When true, rotates , which invalidates + /// all active sessions and authentication tokens for the user. Set to false only when + /// intentionally preserving existing sessions. Defaults to true. + /// + public bool RefreshStamp { get; set; } = true; + + public string? MasterPasswordHint { get; set; } = null; + + public void ValidateDataForUser(User user) + { + // Validate that the user has a master password already, if not then they shouldn't be updating they should + // be setting initial. + if (!user.HasMasterPassword()) + { + throw new BadRequestException("User does not have an existing master password to update."); + } + + // Key Connector users' encryption keys are managed by an external service, replacing the + // master password entirely (MasterPassword is set to null on conversion). Master password + // operations are categorically inapplicable to these users. This guard is defense-in-depth: + // the HasMasterPassword() check above would also catch KC users, but this makes the + // rejection reason explicit. Note: org owners/admins are structurally prohibited from + // using Key Connector (enforced at conversion time in UserService.CheckCanUseKeyConnector), + // so there is no owner/admin edge case to handle here. + if (user.UsesKeyConnector) + { + throw new BadRequestException("Cannot update password of a user with Key Connector."); + } + + // Validate KDF is unchanged for user + MasterPasswordAuthentication.Kdf.ValidateUnchangedForUser(user); + MasterPasswordUnlock.Kdf.ValidateUnchangedForUser(user); + + // Validate Salt is unchanged for user + MasterPasswordAuthentication.ValidateSaltUnchangedForUser(user); + MasterPasswordUnlock.ValidateSaltUnchangedForUser(user); + } +} diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs new file mode 100644 index 000000000000..fc14f76117d0 --- /dev/null +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs @@ -0,0 +1,162 @@ +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Data; +using Bit.Core.Entities; +using Bit.Core.Repositories; +using Microsoft.AspNetCore.Identity; +using OneOf; + +namespace Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; + +/// +/// This service bundles up all the ways we set an initial master password or update +/// an existing one into one place so we can perform the same validation and timestamp setting. +/// +/// Meant to be used compositionally within other processes. Can be leveraged in controllers / commands / services. +/// Operations in here should be CRUD-like, not flow based logic with business logic. +/// +/// There should never be business logic in this service. It is to bottleneck all flows that change and set +/// initial password so we can perform validation of the conditions while setting an initial password and when updating +/// an existing password. +/// +/// DAVE could you write this better +/// The public api is following a specific naming structure where its "MUTATE|SAVE-OPERATION-ASYNC" +/// +/// DAVE +/// Look into returning the User or IdentityError +/// IdentityError[] | User +/// also remove the idea of Mutate and have callers be responsible for saving the updated user entity. +/// +/// DAVE +/// Consider filtering/erroring on non hydrated users in this service. Probably don't need to think too +/// hard about this one. +/// +/// Thank you Dave! +/// +public interface IMasterPasswordService +{ + /// + /// Inspects the user's current state and dispatches to either + /// or + /// accordingly. + /// Mutates the object in memory only — no database write is performed. + /// + /// + /// The user object to mutate. Whether the user already has a master password determines + /// which code path executes. + /// + /// + /// Combined cryptographic and authentication data that covers both the set-initial and + /// update-existing paths. Converted internally via + /// or + /// . + /// + /// + /// On success, the modified . On failure, an array of + /// describing validation failures. + /// + Task> PrepareSetInitialOrUpdateExistingMasterPasswordAsync(User user, SetInitialOrUpdateExistingPasswordData setOrUpdatePasswordData); + + /// + /// Applies a new initial master password to the object in memory only — + /// no database write is performed. Use when the caller controls persistence (e.g. key management + /// flows that must compose this mutation with other transactional operations). + /// + /// + /// The user object to mutate. Must not already have a master password; must have no existing + /// Key or MasterPasswordSalt; must not be a Key Connector user. + /// Validated via . + /// + /// + /// Cryptographic and authentication data required to set the initial password, including + /// MasterPasswordAuthentication (hashed credential used for login), + /// MasterPasswordUnlock (KDF parameters and wrapped user key), + /// and control flags ValidatePassword and RefreshStamp. + /// + /// + /// On success, the modified . On failure, an array of + /// describing validation failures. + /// + Task> PrepareSetInitialMasterPasswordAsync(User user, SetInitialPasswordData setInitialPasswordData); + + /// + /// Note: This is to be used in the future when a TDE user wants to self serve set a password. + /// + /// Applies a new initial master password to the object and persists + /// the updated user to the database. Use when no external transaction coordination is needed. + /// + /// + /// The user object to mutate and persist. Subject to the same preconditions as + /// . + /// + /// + /// Cryptographic and authentication data required to set the initial password. See + /// for field details. + /// + /// + /// On success, the modified . On failure, an array of + /// describing validation failures. + /// + Task> SaveSetInitialMasterPasswordAsync(User user, SetInitialPasswordData setInitialPasswordData); + + /// + /// Returns a deferred database write (as an delegate) for setting + /// the initial master password. The delegate is intended to be passed to + /// , which executes all supplied delegates + /// within a single SQL transaction. Composing this delegate with others (e.g. cryptographic key + /// writes) ensures every write succeeds or the entire batch rolls back atomically — a guarantee + /// cannot provide on its own. + /// + /// + /// The user whose initial master password state will be written when the returned delegate is invoked. + /// + /// + /// Cryptographic and authentication data required to set the initial password. See + /// for field details. + /// + /// + /// An delegate suitable for inclusion in a batch passed to + /// . + /// + UpdateUserData BuildUpdateUserDelegateSetInitialMasterPassword(User user, SetInitialPasswordData setInitialPasswordData); + + /// + /// Applies a new master password over the user's existing one, mutating the + /// object in memory only — no database write is performed. + /// Use when the caller controls persistence. + /// + /// + /// The user object to mutate. Must already have a master password; + /// must not be a Key Connector user. KDF parameters and salt must be unchanged relative to the values in + /// . Validated via + /// . + /// + /// + /// Cryptographic and authentication data for the updated password, including + /// MasterPasswordAuthentication, MasterPasswordUnlock, + /// and control flags ValidatePassword and RefreshStamp. + /// + /// + /// On success, the modified . On failure, an array of + /// describing validation failures. + /// + Task> PrepareUpdateExistingMasterPasswordAsync(User user, UpdateExistingPasswordData updateExistingData); + + Task> SaveUpdateExistingMasterPasswordAndKdfAsync(User user, UpdateExistingPasswordAndKdfData updateExistingExistingData); + + /// + /// Applies a new master password over the user's existing one and persists the updated user + /// to the database. Use when no external transaction coordination is needed. + /// + /// + /// The user object to mutate and persist. Subject to the same preconditions as + /// . + /// + /// + /// Cryptographic and authentication data for the updated password. See + /// for field details. + /// + /// + /// On success, the modified . On failure, an array of + /// describing validation failures. + /// + Task> SaveUpdateExistingMasterPasswordAsync(User user, UpdateExistingPasswordData updateExistingData); +} diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs new file mode 100644 index 000000000000..e022849acd37 --- /dev/null +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs @@ -0,0 +1,254 @@ +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Data; +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; +using Bit.Core.Entities; +using Bit.Core.Repositories; +using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.Logging; +using OneOf; + +namespace Bit.Core.Auth.UserFeatures.UserMasterPassword; + +public class MasterPasswordService( + IUserRepository userRepository, + TimeProvider timeProvider, + IPasswordHasher passwordHasher, + IEnumerable> passwordValidators, + UserManager userManager, + ILogger logger) + : IMasterPasswordService +{ + private readonly IUserRepository _userRepository = userRepository; + private readonly TimeProvider _timeProvider = timeProvider; + private readonly IPasswordHasher _passwordHasher = passwordHasher; + private readonly IEnumerable> _passwordValidators = passwordValidators; + private readonly UserManager _userManager = userManager; + private readonly ILogger _logger = logger; + + public async Task> PrepareSetInitialOrUpdateExistingMasterPasswordAsync( + User user, + SetInitialOrUpdateExistingPasswordData setOrUpdatePasswordData) + { + EnsureUserIsHydrated(user); + + if (user.HasMasterPassword()) + { + return await PrepareUpdateExistingMasterPasswordAsync( + user, + setOrUpdatePasswordData.ToUpdateExistingData()); + } + + return await PrepareSetInitialMasterPasswordAsync( + user, + setOrUpdatePasswordData.ToSetInitialData()); + } + + public async Task> PrepareSetInitialMasterPasswordAsync( + User user, + SetInitialPasswordData setInitialData) + { + EnsureUserIsHydrated(user); + setInitialData.ValidateDataForUser(user); + + var result = await UpdateExistingPasswordHashAsync( + user, + setInitialData.MasterPasswordAuthentication.MasterPasswordAuthenticationHash, + setInitialData.ValidatePassword, + setInitialData.RefreshStamp); + if (!result.Succeeded) + { + return result.Errors.ToArray(); + } + + // Set kdf data on the user + user.Key = setInitialData.MasterPasswordUnlock.MasterKeyWrappedUserKey; + user.Kdf = setInitialData.MasterPasswordUnlock.Kdf.KdfType; + user.KdfIterations = setInitialData.MasterPasswordUnlock.Kdf.Iterations; + user.KdfMemory = setInitialData.MasterPasswordUnlock.Kdf.Memory; + user.KdfParallelism = setInitialData.MasterPasswordUnlock.Kdf.Parallelism; + + // Set salt on the user + user.MasterPasswordSalt = setInitialData.MasterPasswordUnlock.Salt; + + // Always override the master password hint, even if it's null. + user.MasterPasswordHint = setInitialData.MasterPasswordHint; + + // Update time markers on the user + var now = _timeProvider.GetUtcNow().UtcDateTime; + user.LastPasswordChangeDate = now; + user.RevisionDate = user.AccountRevisionDate = now; + + return user; + } + + public async Task> SaveSetInitialMasterPasswordAsync( + User user, + SetInitialPasswordData setInitialData) + { + EnsureUserIsHydrated(user); + var result = await PrepareSetInitialMasterPasswordAsync(user, setInitialData); + if (result.IsT1) + { + return result.AsT1; + } + + await _userRepository.ReplaceAsync(user); + + return user; + } + + public UpdateUserData BuildUpdateUserDelegateSetInitialMasterPassword( + User user, + SetInitialPasswordData setInitialData) + { + EnsureUserIsHydrated(user); + setInitialData.ValidateDataForUser(user); + + // Hash the provided user master password authentication hash on the server side + var serverSideHashedMasterPasswordAuthenticationHash = _passwordHasher.HashPassword(user, + setInitialData.MasterPasswordAuthentication.MasterPasswordAuthenticationHash); + + var setMasterPasswordTask = _userRepository.SetMasterPassword(user.Id, + setInitialData.MasterPasswordUnlock, serverSideHashedMasterPasswordAuthenticationHash, + setInitialData.MasterPasswordHint); + + return setMasterPasswordTask; + } + + public async Task> PrepareUpdateExistingMasterPasswordAsync( + User user, + UpdateExistingPasswordData updateExistingData) + { + EnsureUserIsHydrated(user); + updateExistingData.ValidateDataForUser(user); + + var result = await UpdateExistingPasswordHashAsync( + user, + updateExistingData.MasterPasswordAuthentication.MasterPasswordAuthenticationHash, + updateExistingData.ValidatePassword, + updateExistingData.RefreshStamp); + + if (!result.Succeeded) + { + return result.Errors.ToArray(); + } + + var now = _timeProvider.GetUtcNow().UtcDateTime; + + user.Key = updateExistingData.MasterPasswordUnlock.MasterKeyWrappedUserKey; + + // Always override the master password hint, even if it's null. + user.MasterPasswordHint = updateExistingData.MasterPasswordHint; + + user.LastPasswordChangeDate = now; + user.RevisionDate = user.AccountRevisionDate = now; + + return user; + } + + public async Task> SaveUpdateExistingMasterPasswordAndKdfAsync( + User user, + UpdateExistingPasswordAndKdfData updateExistingExistingData) + { + EnsureUserIsHydrated(user); + updateExistingExistingData.ValidateDataForUser(user); + + var result = await UpdateExistingPasswordHashAsync( + user, + updateExistingExistingData.MasterPasswordAuthentication.MasterPasswordAuthenticationHash, + updateExistingExistingData.ValidatePassword, + updateExistingExistingData.RefreshStamp); + + if (!result.Succeeded) + { + return result.Errors.ToArray(); + } + + var now = _timeProvider.GetUtcNow().UtcDateTime; + + user.Key = updateExistingExistingData.MasterPasswordUnlock.MasterKeyWrappedUserKey; + + user.Kdf = updateExistingExistingData.MasterPasswordUnlock.Kdf.KdfType; + user.KdfIterations = updateExistingExistingData.MasterPasswordUnlock.Kdf.Iterations; + user.KdfMemory = updateExistingExistingData.MasterPasswordUnlock.Kdf.Memory; + user.KdfParallelism = updateExistingExistingData.MasterPasswordUnlock.Kdf.Parallelism; + + // Always override the master password hint, even if it's null. + user.MasterPasswordHint = updateExistingExistingData.MasterPasswordHint; + + user.LastPasswordChangeDate = now; + user.LastKdfChangeDate = now; + user.RevisionDate = user.AccountRevisionDate = now; + + await _userRepository.ReplaceAsync(user); + + return user; + } + + public async Task> SaveUpdateExistingMasterPasswordAsync( + User user, + UpdateExistingPasswordData updateExistingData) + { + EnsureUserIsHydrated(user); + var result = await PrepareUpdateExistingMasterPasswordAsync(user, updateExistingData); + if (result.IsT1) + { + return result.AsT1; + } + + await _userRepository.ReplaceAsync(user); + + return user; + } + + private async Task UpdateExistingPasswordHashAsync(User user, string newPassword, + bool validatePassword = true, bool refreshStamp = true) + { + if (validatePassword) + { + var validate = await ValidatePasswordInternalAsync(user, newPassword); + if (!validate.Succeeded) + { + return validate; + } + } + + user.MasterPassword = _passwordHasher.HashPassword(user, newPassword); + if (refreshStamp) + { + user.SecurityStamp = Guid.NewGuid().ToString(); + } + + return IdentityResult.Success; + } + + // A properly initialized or database-hydrated User should have at a minimum a non-default user ID. + private static void EnsureUserIsHydrated(User user) + { + if (user.Id == default) + { + throw new ArgumentException("User must be hydrated with an assigned identity.", nameof(user)); + } + } + + private async Task ValidatePasswordInternalAsync(User user, string password) + { + var errors = new List(); + foreach (var v in _passwordValidators) + { + var result = await v.ValidateAsync(_userManager, user, password); + if (!result.Succeeded) + { + errors.AddRange(result.Errors); + } + } + + if (errors.Count > 0) + { + _logger.LogWarning("User {userId} password validation failed: {errors}.", user.Id, + string.Join(";", errors.Select(e => e.Code))); + return IdentityResult.Failed(errors.ToArray()); + } + + return IdentityResult.Success; + } +} From d7c0d7e617612d2cc333d0657cd3a9ba9e52add5 Mon Sep 17 00:00:00 2001 From: enmande <3836813+enmande@users.noreply.github.com> Date: Tue, 21 Apr 2026 12:05:20 -0400 Subject: [PATCH 02/26] docs(mp-service): Resolve incoming comments, document contract. --- .../Interfaces/IMasterPasswordService.cs | 100 +++++++++++++----- 1 file changed, 72 insertions(+), 28 deletions(-) diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs index fc14f76117d0..35ab4941189b 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs @@ -7,29 +7,55 @@ namespace Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; /// -/// This service bundles up all the ways we set an initial master password or update -/// an existing one into one place so we can perform the same validation and timestamp setting. +/// Centralized mutation point for all master password set, change, and rotate operations. +/// Provides consistent validation, password hashing, and timestamp management across every +/// flow that establishes or updates a user's master password. /// -/// Meant to be used compositionally within other processes. Can be leveraged in controllers / commands / services. -/// Operations in here should be CRUD-like, not flow based logic with business logic. +/// Compositional, not orchestrating. This service handles CRUD-like mutations +/// only. Business logic (e.g., authorization checks, org validation, push notifications, event +/// logging) remains a caller responsibility. /// -/// There should never be business logic in this service. It is to bottleneck all flows that change and set -/// initial password so we can perform validation of the conditions while setting an initial password and when updating -/// an existing password. +/// Three persistence tiers: +/// +/// +/// Prepare* Modifies the object in memory only. The caller +/// controls when and how persistence occurs. Use when composing additional mutations before +/// saving (e.g. admin recovery flows that also clear 2FA or set ForcePasswordReset). +/// Returns OneOf<User, IdentityError[]>. +/// +/// +/// Save* Prepares the mutation and persists to the database via +/// IUserRepository.ReplaceAsync. Use for standalone operations where no +/// further mutation is needed. Returns OneOf<User, IdentityError[]>. +/// +/// +/// Build* Returns a deferred delegate for +/// batch transactions. Use when the +/// password set is part of a larger transactional write that must succeed or fail atomically +/// (e.g. TDE key + password in a single SQL transaction) +/// +/// /// -/// DAVE could you write this better -/// The public api is following a specific naming structure where its "MUTATE|SAVE-OPERATION-ASYNC" +/// Set vs Update contract: +/// +/// +/// SET (initial): Client sends all data (hash, salt, KDF). Server sets all +/// fields. Stage 1 caveat: server enforces salt == email.ToLowerInvariant().Trim() +/// (PM-28143 removes this in Stage 3). +/// +/// +/// UPDATE (hash only): Client sends all data. Server validates KDF and salt +/// are unchanged, updates only the hash and wrapped user key. +/// +/// +/// UPDATE (KDF): Client sends all data. Server validates salt is unchanged, +/// updates hash, KDF, and wrapped user key. +/// +/// /// -/// DAVE -/// Look into returning the User or IdentityError -/// IdentityError[] | User -/// also remove the idea of Mutate and have callers be responsible for saving the updated user entity. -/// -/// DAVE -/// Consider filtering/erroring on non hydrated users in this service. Probably don't need to think too -/// hard about this one. -/// -/// Thank you Dave! +/// Source of truth: On SET, the client is the source of truth. On UPDATE, +/// the server is the source of truth for fields that must not change — it validates the client's +/// values match what's stored before applying the update. /// public interface IMasterPasswordService { @@ -37,7 +63,7 @@ public interface IMasterPasswordService /// Inspects the user's current state and dispatches to either /// or /// accordingly. - /// Mutates the object in memory only — no database write is performed. + /// Prepares the object in memory only. /// /// /// The user object to mutate. Whether the user already has a master password determines @@ -56,9 +82,8 @@ public interface IMasterPasswordService Task> PrepareSetInitialOrUpdateExistingMasterPasswordAsync(User user, SetInitialOrUpdateExistingPasswordData setOrUpdatePasswordData); /// - /// Applies a new initial master password to the object in memory only — - /// no database write is performed. Use when the caller controls persistence (e.g. key management - /// flows that must compose this mutation with other transactional operations). + /// Applies a new initial master password to the object in memory only. + /// Use for flows that must compose this mutation with other operations inside a larger transaction. /// /// /// The user object to mutate. Must not already have a master password; must have no existing @@ -78,10 +103,10 @@ public interface IMasterPasswordService Task> PrepareSetInitialMasterPasswordAsync(User user, SetInitialPasswordData setInitialPasswordData); /// - /// Note: This is to be used in the future when a TDE user wants to self serve set a password. + /// Note: This is to be used in the future when a TDE user wants to set a password with self-service. /// /// Applies a new initial master password to the object and persists - /// the updated user to the database. Use when no external transaction coordination is needed. + /// the updated user. Use when no external transaction coordination is needed. /// /// /// The user object to mutate and persist. Subject to the same preconditions as @@ -102,7 +127,7 @@ public interface IMasterPasswordService /// the initial master password. The delegate is intended to be passed to /// , which executes all supplied delegates /// within a single SQL transaction. Composing this delegate with others (e.g. cryptographic key - /// writes) ensures every write succeeds or the entire batch rolls back atomically — a guarantee + /// writes) ensures every write succeeds or the entire batch rolls back atomically, a guarantee /// cannot provide on its own. /// /// @@ -120,8 +145,8 @@ public interface IMasterPasswordService /// /// Applies a new master password over the user's existing one, mutating the - /// object in memory only — no database write is performed. - /// Use when the caller controls persistence. + /// object in memory only. + /// Use for flows that must compose this mutation with other operations inside a larger transaction. /// /// /// The user object to mutate. Must already have a master password; @@ -140,6 +165,25 @@ public interface IMasterPasswordService /// Task> PrepareUpdateExistingMasterPasswordAsync(User user, UpdateExistingPasswordData updateExistingData); + /// + /// Applies a new master password and updated KDF parameters over the user's existing ones + /// and persists the updated user to the database. Salt must remain unchanged; KDF is + /// intentionally allowed to change. Use for KDF rotation flows. + /// + /// + /// The user object to mutate and persist. Must already have a master password; + /// must not be a Key Connector user. Salt must be unchanged. Validated via + /// . + /// + /// + /// Cryptographic and authentication data for the updated password and KDF parameters, + /// including MasterPasswordAuthentication, MasterPasswordUnlock, + /// and control flags ValidatePassword and RefreshStamp. + /// + /// + /// On success, the modified . On failure, an array of + /// describing validation failures. + /// Task> SaveUpdateExistingMasterPasswordAndKdfAsync(User user, UpdateExistingPasswordAndKdfData updateExistingExistingData); /// From 13b0ac2aef52f2c89d32794c0754c91da68224ca Mon Sep 17 00:00:00 2001 From: enmande <3836813+enmande@users.noreply.github.com> Date: Tue, 21 Apr 2026 13:07:51 -0400 Subject: [PATCH 03/26] feat(mp-service): Add KDF-setting helper and DI. --- .../MasterPasswordService.cs | 25 +++++++++++-------- .../UserServiceCollectionExtensions.cs | 2 ++ 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs index e022849acd37..cbd2ad5196c2 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs @@ -1,6 +1,7 @@ using Bit.Core.Auth.UserFeatures.UserMasterPassword.Data; using Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; using Bit.Core.Entities; +using Bit.Core.KeyManagement.Models.Data; using Bit.Core.Repositories; using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.Logging; @@ -59,12 +60,8 @@ public async Task> PrepareSetInitialMasterPasswordA return result.Errors.ToArray(); } - // Set kdf data on the user user.Key = setInitialData.MasterPasswordUnlock.MasterKeyWrappedUserKey; - user.Kdf = setInitialData.MasterPasswordUnlock.Kdf.KdfType; - user.KdfIterations = setInitialData.MasterPasswordUnlock.Kdf.Iterations; - user.KdfMemory = setInitialData.MasterPasswordUnlock.Kdf.Memory; - user.KdfParallelism = setInitialData.MasterPasswordUnlock.Kdf.Parallelism; + SetKdfStateOnUser(user, setInitialData.MasterPasswordUnlock.Kdf); // Set salt on the user user.MasterPasswordSalt = setInitialData.MasterPasswordUnlock.Salt; @@ -166,11 +163,7 @@ public async Task> SaveUpdateExistingMasterPassword var now = _timeProvider.GetUtcNow().UtcDateTime; user.Key = updateExistingExistingData.MasterPasswordUnlock.MasterKeyWrappedUserKey; - - user.Kdf = updateExistingExistingData.MasterPasswordUnlock.Kdf.KdfType; - user.KdfIterations = updateExistingExistingData.MasterPasswordUnlock.Kdf.Iterations; - user.KdfMemory = updateExistingExistingData.MasterPasswordUnlock.Kdf.Memory; - user.KdfParallelism = updateExistingExistingData.MasterPasswordUnlock.Kdf.Parallelism; + SetKdfStateOnUser(user, updateExistingExistingData.MasterPasswordUnlock.Kdf); // Always override the master password hint, even if it's null. user.MasterPasswordHint = updateExistingExistingData.MasterPasswordHint; @@ -221,6 +214,18 @@ private async Task UpdateExistingPasswordHashAsync(User user, st return IdentityResult.Success; } + /// + /// Applies KDF parameters from the supplied to the . + /// Used by both initial-set and KDF-rotation paths. + /// + private static void SetKdfStateOnUser(User user, KdfSettings kdf) + { + user.Kdf = kdf.KdfType; + user.KdfIterations = kdf.Iterations; + user.KdfMemory = kdf.Memory; + user.KdfParallelism = kdf.Parallelism; + } + // A properly initialized or database-hydrated User should have at a minimum a non-default user ID. private static void EnsureUserIsHydrated(User user) { diff --git a/src/Core/Auth/UserFeatures/UserServiceCollectionExtensions.cs b/src/Core/Auth/UserFeatures/UserServiceCollectionExtensions.cs index e9e0dbffc1a2..035e31cbe511 100644 --- a/src/Core/Auth/UserFeatures/UserServiceCollectionExtensions.cs +++ b/src/Core/Auth/UserFeatures/UserServiceCollectionExtensions.cs @@ -16,6 +16,7 @@ using Bit.Core.Services; using Bit.Core.Settings; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection.Extensions; namespace Bit.Core.Auth.UserFeatures; @@ -24,6 +25,7 @@ public static class UserServiceCollectionExtensions public static void AddUserServices(this IServiceCollection services, IGlobalSettings globalSettings) { services.AddScoped(); + services.TryAddScoped(); services.AddEmergencyAccessCommands(); services.AddUserPasswordCommands(); services.AddUserRegistrationCommands(); From 45813b6e1bd286553a8ac5bb8c29f9f5e4c03772 Mon Sep 17 00:00:00 2001 From: enmande <3836813+enmande@users.noreply.github.com> Date: Tue, 21 Apr 2026 14:23:00 -0400 Subject: [PATCH 04/26] test(mp-service): Add tests. --- .../MasterPasswordServiceTests.cs | 1261 +++++++++++++++++ 1 file changed, 1261 insertions(+) create mode 100644 test/Core.Test/Auth/UserFeatures/UserMasterPassword/MasterPasswordServiceTests.cs diff --git a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/MasterPasswordServiceTests.cs b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/MasterPasswordServiceTests.cs new file mode 100644 index 000000000000..04a6fde46ab6 --- /dev/null +++ b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/MasterPasswordServiceTests.cs @@ -0,0 +1,1261 @@ +using Bit.Core.Auth.UserFeatures.UserMasterPassword; +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Data; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Exceptions; +using Bit.Core.KeyManagement.Models.Data; +using Bit.Core.Repositories; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using Microsoft.AspNetCore.Identity; +using NSubstitute; +using Xunit; + +namespace Bit.Core.Test.Auth.UserFeatures.UserMasterPassword; + +[SutProviderCustomize] +public class MasterPasswordServiceTests +{ + private static SutProvider CreateSutProvider() + => new SutProvider().WithFakeTimeProvider().Create(); + + // Returns a KdfSettings that exactly matches the user's stored KDF values plus a salt derived + // from the user's current GetMasterPasswordSalt() output. + private static (KdfSettings kdf, string salt) GetMatchingKdfAndSalt(User user) + { + var kdf = new KdfSettings + { + KdfType = user.Kdf, + Iterations = user.KdfIterations, + Memory = user.KdfMemory, + Parallelism = user.KdfParallelism + }; + var salt = user.GetMasterPasswordSalt(); + return (kdf, salt); + } + + private static SetInitialPasswordData BuildSetInitialData(User user, string? hint = null, + bool validatePassword = false) + { + // Stage 1: salt == email while MasterPasswordSalt is null (PM-28143 separates them in Stage 3). + var salt = user.GetMasterPasswordSalt(); + var kdf = new KdfSettings + { + KdfType = user.Kdf, + Iterations = user.KdfIterations, + Memory = user.KdfMemory, + Parallelism = user.KdfParallelism + }; + return new SetInitialPasswordData + { + MasterPasswordAuthentication = new MasterPasswordAuthenticationData + { + Salt = salt, + MasterPasswordAuthenticationHash = "test-hash", + Kdf = kdf + }, + MasterPasswordUnlock = new MasterPasswordUnlockData + { + Salt = salt, + MasterKeyWrappedUserKey = "wrapped-key", + Kdf = kdf + }, + MasterPasswordHint = hint, + ValidatePassword = validatePassword, + RefreshStamp = false + }; + } + + private static UpdateExistingPasswordData BuildUpdateExistingData(User user, string? hint = null, + bool validatePassword = false) + { + var (kdf, salt) = GetMatchingKdfAndSalt(user); + return new UpdateExistingPasswordData + { + MasterPasswordAuthentication = new MasterPasswordAuthenticationData + { + Salt = salt, + MasterPasswordAuthenticationHash = "test-hash", + Kdf = kdf + }, + MasterPasswordUnlock = new MasterPasswordUnlockData + { + Salt = salt, + MasterKeyWrappedUserKey = "wrapped-key", + Kdf = kdf + }, + MasterPasswordHint = hint, + ValidatePassword = validatePassword, + RefreshStamp = false + }; + } + + private static UpdateExistingPasswordAndKdfData BuildUpdateExistingAndKdfData(User user, + KdfSettings? newKdf = null, string? hint = null, bool validatePassword = false) + { + var salt = user.GetMasterPasswordSalt(); + var kdf = newKdf ?? new KdfSettings + { + KdfType = KdfType.Argon2id, + Iterations = 3, + Memory = 64, + Parallelism = 4 + }; + return new UpdateExistingPasswordAndKdfData + { + MasterPasswordAuthentication = new MasterPasswordAuthenticationData + { + Salt = salt, + MasterPasswordAuthenticationHash = "test-hash", + Kdf = kdf + }, + MasterPasswordUnlock = new MasterPasswordUnlockData + { + Salt = salt, + MasterKeyWrappedUserKey = "wrapped-key", + Kdf = kdf + }, + MasterPasswordHint = hint, + ValidatePassword = validatePassword, + RefreshStamp = false + }; + } + + // --- PrepareSetInitialMasterPassword --- + + [Theory, BitAutoData] + public async Task PrepareSetInitialMasterPassword_Success(User user) + { + var sutProvider = CreateSutProvider(); + user.MasterPassword = null; + user.Key = null; + user.MasterPasswordSalt = null; + user.UsesKeyConnector = false; + + var data = BuildSetInitialData(user); + var expectedHash = "server-side-hash"; + sutProvider.GetDependency>() + .HashPassword(user, data.MasterPasswordAuthentication.MasterPasswordAuthenticationHash) + .Returns(expectedHash); + + var result = await sutProvider.Sut.PrepareSetInitialMasterPasswordAsync(user, data); + + var expectedTime = sutProvider.GetDependency().GetUtcNow().UtcDateTime; + + Assert.True(result.IsT0); + Assert.Same(user, result.AsT0); + Assert.Equal(expectedHash, user.MasterPassword); + Assert.Equal(data.MasterPasswordUnlock.MasterKeyWrappedUserKey, user.Key); + Assert.Equal(data.MasterPasswordUnlock.Salt, user.MasterPasswordSalt); + Assert.Equal(data.MasterPasswordUnlock.Kdf.KdfType, user.Kdf); + Assert.Equal(data.MasterPasswordUnlock.Kdf.Iterations, user.KdfIterations); + Assert.Equal(expectedTime, user.LastPasswordChangeDate); + Assert.Equal(expectedTime, user.RevisionDate); + Assert.Equal(user.RevisionDate, user.AccountRevisionDate); + } + + [Theory, BitAutoData] + public async Task PrepareSetInitialMasterPassword_SetsMasterPasswordHint(User user, string hint) + { + var sutProvider = CreateSutProvider(); + user.MasterPassword = null; + user.Key = null; + user.MasterPasswordSalt = null; + user.UsesKeyConnector = false; + + var data = BuildSetInitialData(user, hint: hint); + sutProvider.GetDependency>() + .HashPassword(Arg.Any(), Arg.Any()) + .Returns("hash"); + + var result = await sutProvider.Sut.PrepareSetInitialMasterPasswordAsync(user, data); + + Assert.True(result.IsT0); + Assert.Equal(hint, result.AsT0.MasterPasswordHint); + } + + [Theory, BitAutoData] + public async Task PrepareSetInitialMasterPassword_ThrowsWhenUserNotHydrated(User user) + { + var sutProvider = CreateSutProvider(); + user.Id = default; + user.MasterPassword = null; + user.Key = null; + user.MasterPasswordSalt = null; + user.UsesKeyConnector = false; + + var data = BuildSetInitialData(user); + + await Assert.ThrowsAsync( + () => sutProvider.Sut.PrepareSetInitialMasterPasswordAsync(user, data)); + } + + [Theory, BitAutoData] + public async Task PrepareSetInitialMasterPassword_RefreshStampTrue_RotatesSecurityStamp(User user) + { + var sutProvider = CreateSutProvider(); + user.MasterPassword = null; + user.Key = null; + user.MasterPasswordSalt = null; + user.UsesKeyConnector = false; + + // Build data with RefreshStamp = true (the default — do not override). + var salt = user.GetMasterPasswordSalt(); + var kdf = new KdfSettings + { + KdfType = user.Kdf, + Iterations = user.KdfIterations, + Memory = user.KdfMemory, + Parallelism = user.KdfParallelism + }; + var data = new SetInitialPasswordData + { + MasterPasswordAuthentication = new MasterPasswordAuthenticationData + { + Salt = salt, + MasterPasswordAuthenticationHash = "test-hash", + Kdf = kdf + }, + MasterPasswordUnlock = new MasterPasswordUnlockData + { + Salt = salt, + MasterKeyWrappedUserKey = "wrapped-key", + Kdf = kdf + }, + ValidatePassword = false, + RefreshStamp = true + }; + sutProvider.GetDependency>() + .HashPassword(Arg.Any(), Arg.Any()) + .Returns("hash"); + + var originalStamp = user.SecurityStamp; + + await sutProvider.Sut.PrepareSetInitialMasterPasswordAsync(user, data); + + Assert.NotEqual(originalStamp, user.SecurityStamp); + } + + [Theory, BitAutoData] + public async Task PrepareSetInitialMasterPassword_RefreshStampFalse_PreservesSecurityStamp(User user) + { + var sutProvider = CreateSutProvider(); + user.MasterPassword = null; + user.Key = null; + user.MasterPasswordSalt = null; + user.UsesKeyConnector = false; + + var data = BuildSetInitialData(user); // RefreshStamp = false + sutProvider.GetDependency>() + .HashPassword(Arg.Any(), Arg.Any()) + .Returns("hash"); + + var originalStamp = user.SecurityStamp; + + await sutProvider.Sut.PrepareSetInitialMasterPasswordAsync(user, data); + + Assert.Equal(originalStamp, user.SecurityStamp); + } + + // --- PrepareSetInitialMasterPassword — OneOf return shape --- + + [Theory, BitAutoData] + public async Task PrepareSetInitialMasterPassword_Success_ReturnsUserAsT0(User user) + { + var sutProvider = CreateSutProvider(); + user.MasterPassword = null; + user.Key = null; + user.MasterPasswordSalt = null; + user.UsesKeyConnector = false; + + var data = BuildSetInitialData(user); + sutProvider.GetDependency>() + .HashPassword(Arg.Any(), Arg.Any()) + .Returns("hash"); + + var result = await sutProvider.Sut.PrepareSetInitialMasterPasswordAsync(user, data); + + Assert.True(result.IsT0); + Assert.Same(user, result.AsT0); + } + + [Theory, BitAutoData] + public async Task PrepareSetInitialMasterPassword_ValidationFailure_ReturnsErrorsAsT1(User user) + { + var error = new IdentityError { Code = "test", Description = "test error" }; + var validator = Substitute.For>(); + validator.ValidateAsync(Arg.Any>(), Arg.Any(), Arg.Any()) + .Returns(IdentityResult.Failed(error)); + + var sutProvider = new SutProvider() + .WithFakeTimeProvider() + .SetDependency>>(new[] { validator }) + .Create(); + + user.MasterPassword = null; + user.Key = null; + user.MasterPasswordSalt = null; + user.UsesKeyConnector = false; + + var data = BuildSetInitialData(user, validatePassword: true); + + var result = await sutProvider.Sut.PrepareSetInitialMasterPasswordAsync(user, data); + + Assert.True(result.IsT1); + Assert.NotEmpty(result.AsT1); + } + + // --- SaveSetInitialMasterPassword --- + + [Theory, BitAutoData] + public async Task SaveSetInitialMasterPassword_PreparesAndPersists(User user) + { + var sutProvider = CreateSutProvider(); + user.MasterPassword = null; + user.Key = null; + user.MasterPasswordSalt = null; + user.UsesKeyConnector = false; + + var data = BuildSetInitialData(user); + sutProvider.GetDependency>() + .HashPassword(Arg.Any(), Arg.Any()) + .Returns("hash"); + + var result = await sutProvider.Sut.SaveSetInitialMasterPasswordAsync(user, data); + + Assert.True(result.IsT0); + Assert.NotNull(user.MasterPassword); + await sutProvider.GetDependency().Received().ReplaceAsync(user); + } + + [Theory, BitAutoData] + public async Task SaveSetInitialMasterPassword_WhenValidationFails_ReturnsErrorsAndDoesNotPersist(User user) + { + var error = new IdentityError { Code = "pwd-invalid", Description = "Password is too weak." }; + var validator = Substitute.For>(); + validator.ValidateAsync(Arg.Any>(), Arg.Any(), Arg.Any()) + .Returns(IdentityResult.Failed(error)); + + var sutProvider = new SutProvider() + .WithFakeTimeProvider() + .SetDependency>>(new[] { validator }) + .Create(); + + user.MasterPassword = null; + user.Key = null; + user.MasterPasswordSalt = null; + user.UsesKeyConnector = false; + + var data = BuildSetInitialData(user, validatePassword: true); + + var result = await sutProvider.Sut.SaveSetInitialMasterPasswordAsync(user, data); + + Assert.True(result.IsT1); + Assert.NotEmpty(result.AsT1); + await sutProvider.GetDependency().DidNotReceive().ReplaceAsync(Arg.Any()); + } + + [Theory, BitAutoData] + public async Task SaveSetInitialMasterPassword_ThrowsWhenUserNotHydrated(User user) + { + var sutProvider = CreateSutProvider(); + user.Id = default; + + var data = BuildSetInitialData(user); + + await Assert.ThrowsAsync( + () => sutProvider.Sut.SaveSetInitialMasterPasswordAsync(user, data)); + } + + // --- PrepareUpdateExistingMasterPassword --- + + [Theory, BitAutoData] + public async Task PrepareUpdateExistingMasterPassword_Success(User user) + { + var sutProvider = CreateSutProvider(); + user.MasterPassword = "existing-hash"; + user.UsesKeyConnector = false; + + var data = BuildUpdateExistingData(user); + var expectedHash = "new-server-hash"; + sutProvider.GetDependency>() + .HashPassword(user, data.MasterPasswordAuthentication.MasterPasswordAuthenticationHash) + .Returns(expectedHash); + + var result = await sutProvider.Sut.PrepareUpdateExistingMasterPasswordAsync(user, data); + + var expectedTime = sutProvider.GetDependency().GetUtcNow().UtcDateTime; + + Assert.True(result.IsT0); + Assert.Same(user, result.AsT0); + Assert.Equal(expectedHash, user.MasterPassword); + Assert.Equal(data.MasterPasswordUnlock.MasterKeyWrappedUserKey, user.Key); + Assert.Equal(expectedTime, user.LastPasswordChangeDate); + Assert.Equal(expectedTime, user.RevisionDate); + Assert.Equal(user.RevisionDate, user.AccountRevisionDate); + } + + [Theory, BitAutoData] + public async Task PrepareUpdateExistingMasterPassword_SetsMasterPasswordHint(User user, string hint) + { + var sutProvider = CreateSutProvider(); + user.MasterPassword = "existing-hash"; + user.UsesKeyConnector = false; + + var data = BuildUpdateExistingData(user, hint: hint); + sutProvider.GetDependency>() + .HashPassword(Arg.Any(), Arg.Any()) + .Returns("hash"); + + var result = await sutProvider.Sut.PrepareUpdateExistingMasterPasswordAsync(user, data); + + Assert.True(result.IsT0); + Assert.Equal(hint, result.AsT0.MasterPasswordHint); + } + + [Theory, BitAutoData] + public async Task PrepareUpdateExistingMasterPassword_ThrowsWhenUserNotHydrated(User user) + { + var sutProvider = CreateSutProvider(); + user.Id = default; + user.MasterPassword = "existing-hash"; + + var data = BuildUpdateExistingData(user); + + await Assert.ThrowsAsync( + () => sutProvider.Sut.PrepareUpdateExistingMasterPasswordAsync(user, data)); + } + + // --- PrepareUpdateExistingMasterPassword — OneOf return shape --- + + [Theory, BitAutoData] + public async Task PrepareUpdateExistingMasterPassword_ValidationFailure_ReturnsErrorsAsT1(User user) + { + var error = new IdentityError { Code = "test", Description = "test error" }; + var validator = Substitute.For>(); + validator.ValidateAsync(Arg.Any>(), Arg.Any(), Arg.Any()) + .Returns(IdentityResult.Failed(error)); + + var sutProvider = new SutProvider() + .WithFakeTimeProvider() + .SetDependency>>(new[] { validator }) + .Create(); + + user.MasterPassword = "existing-hash"; + user.UsesKeyConnector = false; + + var data = BuildUpdateExistingData(user, validatePassword: true); + + var result = await sutProvider.Sut.PrepareUpdateExistingMasterPasswordAsync(user, data); + + Assert.True(result.IsT1); + Assert.NotEmpty(result.AsT1); + } + + // --- SaveUpdateExistingMasterPassword --- + + [Theory, BitAutoData] + public async Task SaveUpdateExistingMasterPassword_PreparesAndPersists(User user) + { + var sutProvider = CreateSutProvider(); + user.MasterPassword = "existing-hash"; + user.UsesKeyConnector = false; + + var data = BuildUpdateExistingData(user); + sutProvider.GetDependency>() + .HashPassword(Arg.Any(), Arg.Any()) + .Returns("new-hash"); + + var result = await sutProvider.Sut.SaveUpdateExistingMasterPasswordAsync(user, data); + + Assert.True(result.IsT0); + Assert.Equal("new-hash", user.MasterPassword); + await sutProvider.GetDependency().Received().ReplaceAsync(user); + } + + [Theory, BitAutoData] + public async Task SaveUpdateExistingMasterPassword_WhenValidationFails_ReturnsErrorsAndDoesNotPersist(User user) + { + var error = new IdentityError { Code = "pwd-invalid", Description = "Password is too weak." }; + var validator = Substitute.For>(); + validator.ValidateAsync(Arg.Any>(), Arg.Any(), Arg.Any()) + .Returns(IdentityResult.Failed(error)); + + var sutProvider = new SutProvider() + .WithFakeTimeProvider() + .SetDependency>>(new[] { validator }) + .Create(); + + user.MasterPassword = "existing-hash"; + user.UsesKeyConnector = false; + + var data = BuildUpdateExistingData(user, validatePassword: true); + + var result = await sutProvider.Sut.SaveUpdateExistingMasterPasswordAsync(user, data); + + Assert.True(result.IsT1); + Assert.NotEmpty(result.AsT1); + await sutProvider.GetDependency().DidNotReceive().ReplaceAsync(Arg.Any()); + } + + // --- PrepareSetInitialOrUpdateExistingMasterPassword — Dispatch routing --- + + [Theory, BitAutoData] + public async Task PrepareSetInitialOrUpdateExisting_RoutesToSetInitial_WhenNoMasterPassword(User user) + { + var sutProvider = CreateSutProvider(); + user.MasterPassword = null; + user.Key = null; + user.MasterPasswordSalt = null; + user.UsesKeyConnector = false; + + // Stage 1: salt == email while MasterPasswordSalt is null (PM-28143 separates them in Stage 3). + var salt = user.GetMasterPasswordSalt(); + var kdf = new KdfSettings + { + KdfType = user.Kdf, + Iterations = user.KdfIterations, + Memory = user.KdfMemory, + Parallelism = user.KdfParallelism + }; + var data = new SetInitialOrUpdateExistingPasswordData + { + MasterPasswordAuthentication = new MasterPasswordAuthenticationData + { + Salt = salt, + MasterPasswordAuthenticationHash = "test-hash", + Kdf = kdf + }, + MasterPasswordUnlock = new MasterPasswordUnlockData + { + Salt = salt, + MasterKeyWrappedUserKey = "wrapped-key", + Kdf = kdf + }, + ValidatePassword = false, + RefreshStamp = false + }; + sutProvider.GetDependency>() + .HashPassword(Arg.Any(), Arg.Any()) + .Returns("hash"); + + var result = await sutProvider.Sut.PrepareSetInitialOrUpdateExistingMasterPasswordAsync(user, data); + + Assert.True(result.IsT0); + // Set-initial path hashes the password and sets the wrapped key — proving it ran, not update. + Assert.NotNull(user.MasterPassword); + Assert.Equal("wrapped-key", user.Key); + } + + [Theory, BitAutoData] + public async Task PrepareSetInitialOrUpdateExisting_RoutesToUpdateExisting_WhenHasMasterPassword(User user) + { + var sutProvider = CreateSutProvider(); + user.MasterPassword = "existing-hash"; + user.UsesKeyConnector = false; + + var (kdf, salt) = GetMatchingKdfAndSalt(user); + var data = new SetInitialOrUpdateExistingPasswordData + { + MasterPasswordAuthentication = new MasterPasswordAuthenticationData + { + Salt = salt, + MasterPasswordAuthenticationHash = "test-hash", + Kdf = kdf + }, + MasterPasswordUnlock = new MasterPasswordUnlockData + { + Salt = salt, + MasterKeyWrappedUserKey = "wrapped-key", + Kdf = kdf + }, + ValidatePassword = false, + RefreshStamp = false + }; + sutProvider.GetDependency>() + .HashPassword(Arg.Any(), Arg.Any()) + .Returns("new-hash"); + + var result = await sutProvider.Sut.PrepareSetInitialOrUpdateExistingMasterPasswordAsync(user, data); + + Assert.True(result.IsT0); + // Update-existing path hashes the new password and sets the wrapped key. + Assert.Equal("new-hash", user.MasterPassword); + Assert.Equal("wrapped-key", user.Key); + } + + [Theory, BitAutoData] + public async Task PrepareSetInitialOrUpdateExisting_PropagatesValidationErrors_WhenUpdatePathFails(User user) + { + var sutProvider = CreateSutProvider(); + user.MasterPassword = "existing-hash"; + user.UsesKeyConnector = false; + + // Use a salt that does not match the user's current salt to trigger BadRequestException + var (kdf, _) = GetMatchingKdfAndSalt(user); + var wrongSalt = "wrong-salt-value"; + var data = new SetInitialOrUpdateExistingPasswordData + { + MasterPasswordAuthentication = new MasterPasswordAuthenticationData + { + Salt = wrongSalt, + MasterPasswordAuthenticationHash = "test-hash", + Kdf = kdf + }, + MasterPasswordUnlock = new MasterPasswordUnlockData + { + Salt = wrongSalt, + MasterKeyWrappedUserKey = "wrapped-key", + Kdf = kdf + }, + ValidatePassword = false, + RefreshStamp = false + }; + + await Assert.ThrowsAsync( + () => sutProvider.Sut.PrepareSetInitialOrUpdateExistingMasterPasswordAsync(user, data)); + } + + [Theory, BitAutoData] + public async Task PrepareSetInitialOrUpdateExisting_ThrowsWhenUserNotHydrated(User user) + { + var sutProvider = CreateSutProvider(); + user.Id = default; + + var (kdf, salt) = GetMatchingKdfAndSalt(user); + var data = new SetInitialOrUpdateExistingPasswordData + { + MasterPasswordAuthentication = new MasterPasswordAuthenticationData + { + Salt = salt, + MasterPasswordAuthenticationHash = "test-hash", + Kdf = kdf + }, + MasterPasswordUnlock = new MasterPasswordUnlockData + { + Salt = salt, + MasterKeyWrappedUserKey = "wrapped-key", + Kdf = kdf + }, + ValidatePassword = false, + RefreshStamp = false + }; + + await Assert.ThrowsAsync( + () => sutProvider.Sut.PrepareSetInitialOrUpdateExistingMasterPasswordAsync(user, data)); + } + + // --- SaveUpdateExistingMasterPasswordAndKdf --- + + [Theory, BitAutoData] + public async Task SaveUpdateExistingMasterPasswordAndKdf_Success(User user) + { + var sutProvider = CreateSutProvider(); + user.MasterPassword = "existing-hash"; + user.UsesKeyConnector = false; + + var data = BuildUpdateExistingAndKdfData(user); + sutProvider.GetDependency>() + .HashPassword(Arg.Any(), Arg.Any()) + .Returns("new-hash"); + + var result = await sutProvider.Sut.SaveUpdateExistingMasterPasswordAndKdfAsync(user, data); + + var expectedTime = sutProvider.GetDependency().GetUtcNow().UtcDateTime; + + Assert.True(result.IsT0); + Assert.Equal(data.MasterPasswordUnlock.Kdf.KdfType, user.Kdf); + Assert.Equal(data.MasterPasswordUnlock.Kdf.Iterations, user.KdfIterations); + Assert.Equal(expectedTime, user.LastKdfChangeDate); + await sutProvider.GetDependency().Received().ReplaceAsync(user); + } + + [Theory, BitAutoData] + public async Task SaveUpdateExistingMasterPasswordAndKdf_RotatesPbkdf2ToArgon2id(User user) + { + var sutProvider = CreateSutProvider(); + user.MasterPassword = "existing-hash"; + user.UsesKeyConnector = false; + user.Kdf = KdfType.PBKDF2_SHA256; + user.KdfIterations = 600000; + user.KdfMemory = null; + user.KdfParallelism = null; + + var newKdf = new KdfSettings + { + KdfType = KdfType.Argon2id, + Iterations = 3, + Memory = 64, + Parallelism = 4 + }; + var data = BuildUpdateExistingAndKdfData(user, newKdf: newKdf); + sutProvider.GetDependency>() + .HashPassword(Arg.Any(), Arg.Any()) + .Returns("new-hash"); + + var result = await sutProvider.Sut.SaveUpdateExistingMasterPasswordAndKdfAsync(user, data); + + Assert.True(result.IsT0); + Assert.Equal(KdfType.Argon2id, user.Kdf); + Assert.Equal(3, user.KdfIterations); + Assert.Equal(64, user.KdfMemory); + Assert.Equal(4, user.KdfParallelism); + } + + [Theory, BitAutoData] + public async Task SaveUpdateExistingMasterPasswordAndKdf_RotatesArgon2idToPbkdf2(User user) + { + var sutProvider = CreateSutProvider(); + user.MasterPassword = "existing-hash"; + user.UsesKeyConnector = false; + user.Kdf = KdfType.Argon2id; + user.KdfIterations = 3; + user.KdfMemory = 64; + user.KdfParallelism = 4; + + var newKdf = new KdfSettings + { + KdfType = KdfType.PBKDF2_SHA256, + Iterations = 600000, + Memory = null, + Parallelism = null + }; + var data = BuildUpdateExistingAndKdfData(user, newKdf: newKdf); + sutProvider.GetDependency>() + .HashPassword(Arg.Any(), Arg.Any()) + .Returns("new-hash"); + + var result = await sutProvider.Sut.SaveUpdateExistingMasterPasswordAndKdfAsync(user, data); + + Assert.True(result.IsT0); + Assert.Equal(KdfType.PBKDF2_SHA256, user.Kdf); + Assert.Equal(600000, user.KdfIterations); + Assert.Null(user.KdfMemory); + Assert.Null(user.KdfParallelism); + } + + [Theory, BitAutoData] + public async Task SaveUpdateExistingMasterPasswordAndKdf_WhenValidationFails_ReturnsErrorsAndDoesNotPersist(User user) + { + var error = new IdentityError { Code = "pwd-invalid", Description = "Password is too weak." }; + var validator = Substitute.For>(); + validator.ValidateAsync(Arg.Any>(), Arg.Any(), Arg.Any()) + .Returns(IdentityResult.Failed(error)); + + var sutProvider = new SutProvider() + .WithFakeTimeProvider() + .SetDependency>>(new[] { validator }) + .Create(); + + user.MasterPassword = "existing-hash"; + user.UsesKeyConnector = false; + + var data = BuildUpdateExistingAndKdfData(user, validatePassword: true); + + var result = await sutProvider.Sut.SaveUpdateExistingMasterPasswordAndKdfAsync(user, data); + + Assert.True(result.IsT1); + Assert.NotEmpty(result.AsT1); + await sutProvider.GetDependency().DidNotReceive().ReplaceAsync(Arg.Any()); + } + + [Theory, BitAutoData] + public async Task SaveUpdateExistingMasterPasswordAndKdf_ThrowsWhenSaltChanged(User user) + { + var sutProvider = CreateSutProvider(); + user.MasterPassword = "existing-hash"; + user.UsesKeyConnector = false; + + var salt = user.GetMasterPasswordSalt(); + var kdf = new KdfSettings + { + KdfType = KdfType.Argon2id, + Iterations = 3, + Memory = 64, + Parallelism = 4 + }; + var data = new UpdateExistingPasswordAndKdfData + { + MasterPasswordAuthentication = new MasterPasswordAuthenticationData + { + Salt = "wrong-salt", + MasterPasswordAuthenticationHash = "test-hash", + Kdf = kdf + }, + MasterPasswordUnlock = new MasterPasswordUnlockData + { + Salt = salt, + MasterKeyWrappedUserKey = "wrapped-key", + Kdf = kdf + }, + ValidatePassword = false, + RefreshStamp = false + }; + + await Assert.ThrowsAsync( + () => sutProvider.Sut.SaveUpdateExistingMasterPasswordAndKdfAsync(user, data)); + } + + [Theory, BitAutoData] + public async Task SaveUpdateExistingMasterPasswordAndKdf_ThrowsWhenNoExistingPassword(User user) + { + var sutProvider = CreateSutProvider(); + user.MasterPassword = null; + + var data = BuildUpdateExistingAndKdfData(user); + + await Assert.ThrowsAsync( + () => sutProvider.Sut.SaveUpdateExistingMasterPasswordAndKdfAsync(user, data)); + } + + // --- BuildUpdateUserDelegateSetInitialMasterPassword --- + + public class BuildUpdateUserDelegateSetInitialMasterPasswordTests + { + [Theory, BitAutoData] + public void BuildUpdateUserDelegate_ThrowsWhenUserNotHydrated(User user) + { + var sutProvider = new SutProvider().WithFakeTimeProvider().Create(); + user.Id = default; + user.MasterPassword = null; + user.Key = null; + user.MasterPasswordSalt = null; + user.UsesKeyConnector = false; + + var data = BuildSetInitialDataForUser(user); + + Assert.Throws( + () => sutProvider.Sut.BuildUpdateUserDelegateSetInitialMasterPassword(user, data)); + } + + [Theory, BitAutoData] + public void BuildUpdateUserDelegate_HappyPath_ReturnsNonNullDelegateAndDoesNotPersist(User user) + { + var sutProvider = new SutProvider().WithFakeTimeProvider().Create(); + user.MasterPassword = null; + user.Key = null; + user.MasterPasswordSalt = null; + user.UsesKeyConnector = false; + + var data = BuildSetInitialDataForUser(user); + sutProvider.GetDependency>() + .HashPassword(Arg.Any(), Arg.Any()) + .Returns("server-hash"); + + var result = sutProvider.Sut.BuildUpdateUserDelegateSetInitialMasterPassword(user, data); + + // The Build* tier returns a delegate — it must not persist directly. + Assert.NotNull(result); + sutProvider.GetDependency().DidNotReceive().ReplaceAsync(Arg.Any()); + // Hashing is eager at build time, not deferred. + sutProvider.GetDependency>() + .Received() + .HashPassword(user, Arg.Any()); + } + + [Theory, BitAutoData] + public void BuildUpdateUserDelegate_ThrowsWhenUserAlreadyHasMasterPassword(User user) + { + var sutProvider = new SutProvider().WithFakeTimeProvider().Create(); + // User already has a master password — ValidateDataForUser must be called eagerly. + user.MasterPassword = "existing-hash"; + + var data = BuildSetInitialDataForUser(user); + + Assert.Throws( + () => sutProvider.Sut.BuildUpdateUserDelegateSetInitialMasterPassword(user, data)); + } + + private static SetInitialPasswordData BuildSetInitialDataForUser(User user) + { + var salt = user.GetMasterPasswordSalt(); + var kdf = new KdfSettings + { + KdfType = user.Kdf, + Iterations = user.KdfIterations, + Memory = user.KdfMemory, + Parallelism = user.KdfParallelism + }; + return new SetInitialPasswordData + { + MasterPasswordAuthentication = new MasterPasswordAuthenticationData + { + Salt = salt, + MasterPasswordAuthenticationHash = "test-hash", + Kdf = kdf + }, + MasterPasswordUnlock = new MasterPasswordUnlockData + { + Salt = salt, + MasterKeyWrappedUserKey = "wrapped-key", + Kdf = kdf + }, + ValidatePassword = false, + RefreshStamp = false + }; + } + } + + // --- Data model validation: SetInitialPasswordData --- + + public class SetInitialPasswordDataTests + { + private static User BuildValidSetInitialUser() + { + var user = new User + { + Id = Guid.NewGuid(), + Email = "test@example.com", + MasterPassword = null, + Key = null, + MasterPasswordSalt = null, + UsesKeyConnector = false, + Kdf = KdfType.PBKDF2_SHA256, + KdfIterations = 600000 + }; + return user; + } + + private static SetInitialPasswordData BuildData(User user, string? saltOverride = null) + { + // Stage 1: salt == email while MasterPasswordSalt is null (PM-28143 separates them in Stage 3). + var salt = saltOverride ?? user.GetMasterPasswordSalt(); + var kdf = new KdfSettings + { + KdfType = user.Kdf, + Iterations = user.KdfIterations, + Memory = user.KdfMemory, + Parallelism = user.KdfParallelism + }; + return new SetInitialPasswordData + { + MasterPasswordAuthentication = new MasterPasswordAuthenticationData + { + Salt = salt, + MasterPasswordAuthenticationHash = "hash", + Kdf = kdf + }, + MasterPasswordUnlock = new MasterPasswordUnlockData + { + Salt = salt, + MasterKeyWrappedUserKey = "wrapped-key", + Kdf = kdf + } + }; + } + + [Fact] + public void ValidateDataForUser_Accepts_WhenUserHasNoMasterPassword() + { + var user = BuildValidSetInitialUser(); + var data = BuildData(user); + + // Should not throw + data.ValidateDataForUser(user); + } + + [Fact] + public void ValidateDataForUser_Throws_WhenUserHasMasterPassword() + { + var user = BuildValidSetInitialUser(); + user.MasterPassword = "existing-hash"; + var data = BuildData(user); + + Assert.Throws(() => data.ValidateDataForUser(user)); + } + + [Fact] + public void ValidateDataForUser_Throws_WhenUserHasKey() + { + var user = BuildValidSetInitialUser(); + user.Key = "existing-key"; + var data = BuildData(user); + + Assert.Throws(() => data.ValidateDataForUser(user)); + } + + [Fact] + public void ValidateDataForUser_Throws_WhenUserHasSalt() + { + var user = BuildValidSetInitialUser(); + user.MasterPasswordSalt = "existing-salt"; + var data = BuildData(user, saltOverride: "existing-salt"); + + Assert.Throws(() => data.ValidateDataForUser(user)); + } + + [Fact] + public void ValidateDataForUser_Throws_WhenUserIsKeyConnector() + { + var user = BuildValidSetInitialUser(); + user.UsesKeyConnector = true; + var data = BuildData(user); + + Assert.Throws(() => data.ValidateDataForUser(user)); + } + + [Fact] + public void ValidateDataForUser_Throws_WhenSaltMismatch() + { + var user = BuildValidSetInitialUser(); + var data = BuildData(user, saltOverride: "wrong-salt"); + + Assert.Throws(() => data.ValidateDataForUser(user)); + } + + [Fact] + public void ValidateDataForUser_Throws_WhenAuthenticationSaltMismatch_UnlockSaltCorrect() + { + var user = BuildValidSetInitialUser(); + var correctSalt = user.GetMasterPasswordSalt(); + var kdf = new KdfSettings + { + KdfType = user.Kdf, + Iterations = user.KdfIterations, + Memory = user.KdfMemory, + Parallelism = user.KdfParallelism + }; + // Authentication salt is wrong; Unlock salt is correct. + var data = new SetInitialPasswordData + { + MasterPasswordAuthentication = new MasterPasswordAuthenticationData + { + Salt = "wrong-auth-salt", + MasterPasswordAuthenticationHash = "hash", + Kdf = kdf + }, + MasterPasswordUnlock = new MasterPasswordUnlockData + { + Salt = correctSalt, + MasterKeyWrappedUserKey = "wrapped-key", + Kdf = kdf + } + }; + + Assert.Throws(() => data.ValidateDataForUser(user)); + } + + [Fact] + public void ValidateDataForUser_Throws_WhenUnlockSaltMismatch_AuthenticationSaltCorrect() + { + var user = BuildValidSetInitialUser(); + var correctSalt = user.GetMasterPasswordSalt(); + var kdf = new KdfSettings + { + KdfType = user.Kdf, + Iterations = user.KdfIterations, + Memory = user.KdfMemory, + Parallelism = user.KdfParallelism + }; + // Unlock salt is wrong; Authentication salt is correct. + var data = new SetInitialPasswordData + { + MasterPasswordAuthentication = new MasterPasswordAuthenticationData + { + Salt = correctSalt, + MasterPasswordAuthenticationHash = "hash", + Kdf = kdf + }, + MasterPasswordUnlock = new MasterPasswordUnlockData + { + Salt = "wrong-unlock-salt", + MasterKeyWrappedUserKey = "wrapped-key", + Kdf = kdf + } + }; + + Assert.Throws(() => data.ValidateDataForUser(user)); + } + } + + // --- Data model validation: UpdateExistingPasswordData --- + + public class UpdateExistingPasswordDataTests + { + private static User BuildValidUpdateUser() + { + var user = new User + { + Id = Guid.NewGuid(), + Email = "test@example.com", + MasterPassword = "existing-hash", + Key = "existing-key", + MasterPasswordSalt = "stored-salt", + UsesKeyConnector = false, + Kdf = KdfType.PBKDF2_SHA256, + KdfIterations = 600000 + }; + return user; + } + + private static UpdateExistingPasswordData BuildData(User user, string? saltOverride = null, + KdfSettings? kdfOverride = null) + { + var salt = saltOverride ?? user.GetMasterPasswordSalt(); + var kdf = kdfOverride ?? new KdfSettings + { + KdfType = user.Kdf, + Iterations = user.KdfIterations, + Memory = user.KdfMemory, + Parallelism = user.KdfParallelism + }; + return new UpdateExistingPasswordData + { + MasterPasswordAuthentication = new MasterPasswordAuthenticationData + { + Salt = salt, + MasterPasswordAuthenticationHash = "hash", + Kdf = kdf + }, + MasterPasswordUnlock = new MasterPasswordUnlockData + { + Salt = salt, + MasterKeyWrappedUserKey = "wrapped-key", + Kdf = kdf + } + }; + } + + [Fact] + public void ValidateDataForUser_Accepts_WhenUserHasMasterPassword_KdfAndSaltMatch() + { + var user = BuildValidUpdateUser(); + var data = BuildData(user); + + // Should not throw + data.ValidateDataForUser(user); + } + + [Fact] + public void ValidateDataForUser_Throws_WhenUserHasNoMasterPassword() + { + var user = BuildValidUpdateUser(); + user.MasterPassword = null; + var data = BuildData(user); + + Assert.Throws(() => data.ValidateDataForUser(user)); + } + + [Fact] + public void ValidateDataForUser_Throws_WhenUserIsKeyConnector() + { + var user = BuildValidUpdateUser(); + user.UsesKeyConnector = true; + var data = BuildData(user); + + Assert.Throws(() => data.ValidateDataForUser(user)); + } + + [Fact] + public void ValidateDataForUser_Throws_WhenKdfChanged() + { + var user = BuildValidUpdateUser(); + var mismatchedKdf = new KdfSettings + { + KdfType = KdfType.Argon2id, + Iterations = 3, + Memory = 64, + Parallelism = 4 + }; + var data = BuildData(user, kdfOverride: mismatchedKdf); + + Assert.Throws(() => data.ValidateDataForUser(user)); + } + + [Fact] + public void ValidateDataForUser_Throws_WhenSaltChanged() + { + var user = BuildValidUpdateUser(); + var data = BuildData(user, saltOverride: "wrong-salt"); + + Assert.Throws(() => data.ValidateDataForUser(user)); + } + } + + // --- Data model validation: UpdateExistingPasswordAndKdfData --- + + public class UpdateExistingPasswordAndKdfDataTests + { + private static User BuildValidUser() + { + return new User + { + Id = Guid.NewGuid(), + Email = "test@example.com", + MasterPassword = "existing-hash", + Key = "existing-key", + MasterPasswordSalt = "stored-salt", + UsesKeyConnector = false, + Kdf = KdfType.PBKDF2_SHA256, + KdfIterations = 600000 + }; + } + + private static UpdateExistingPasswordAndKdfData BuildData(User user, string? saltOverride = null) + { + var salt = saltOverride ?? user.GetMasterPasswordSalt(); + var newKdf = new KdfSettings + { + KdfType = KdfType.Argon2id, + Iterations = 3, + Memory = 64, + Parallelism = 4 + }; + return new UpdateExistingPasswordAndKdfData + { + MasterPasswordAuthentication = new MasterPasswordAuthenticationData + { + Salt = salt, + MasterPasswordAuthenticationHash = "hash", + Kdf = newKdf + }, + MasterPasswordUnlock = new MasterPasswordUnlockData + { + Salt = salt, + MasterKeyWrappedUserKey = "wrapped-key", + Kdf = newKdf + } + }; + } + + [Fact] + public void ValidateDataForUser_Accepts_WhenUserHasMasterPassword_SaltMatch_KdfChanged() + { + var user = BuildValidUser(); + var data = BuildData(user); + + // Should not throw — KDF change is permitted here + data.ValidateDataForUser(user); + } + + [Fact] + public void ValidateDataForUser_Throws_WhenUserHasNoMasterPassword() + { + var user = BuildValidUser(); + user.MasterPassword = null; + var data = BuildData(user); + + Assert.Throws(() => data.ValidateDataForUser(user)); + } + + [Fact] + public void ValidateDataForUser_Throws_WhenUserIsKeyConnector() + { + var user = BuildValidUser(); + user.UsesKeyConnector = true; + var data = BuildData(user); + + Assert.Throws(() => data.ValidateDataForUser(user)); + } + + [Fact] + public void ValidateDataForUser_Throws_WhenSaltChanged() + { + var user = BuildValidUser(); + var data = BuildData(user, saltOverride: "wrong-salt"); + + Assert.Throws(() => data.ValidateDataForUser(user)); + } + } + +} From 50c62a8003ad68cf75c9051e817abb39118450dd Mon Sep 17 00:00:00 2001 From: enmande <3836813+enmande@users.noreply.github.com> Date: Wed, 22 Apr 2026 12:15:14 -0400 Subject: [PATCH 05/26] feat(mp-service): Add enforecement in Build delegate for stamp/validate pw flags, tag data update ticket. --- .../MasterPasswordService.cs | 25 ++++- .../Repositories/UserRepository.cs | 3 + .../Repositories/UserRepository.cs | 3 + .../MasterPasswordServiceTests.cs | 96 +++++++++++++++++++ 4 files changed, 126 insertions(+), 1 deletion(-) diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs index cbd2ad5196c2..48325c49d942 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs @@ -1,6 +1,7 @@ using Bit.Core.Auth.UserFeatures.UserMasterPassword.Data; using Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; using Bit.Core.Entities; +using Bit.Core.Exceptions; using Bit.Core.KeyManagement.Models.Data; using Bit.Core.Repositories; using Microsoft.AspNetCore.Identity; @@ -108,7 +109,29 @@ public UpdateUserData BuildUpdateUserDelegateSetInitialMasterPassword( setInitialData.MasterPasswordUnlock, serverSideHashedMasterPasswordAuthenticationHash, setInitialData.MasterPasswordHint); - return setMasterPasswordTask; + return async (connection, transaction) => + { + if (setInitialData.ValidatePassword) + { + var validate = await ValidatePasswordInternalAsync(user, + setInitialData.MasterPasswordAuthentication.MasterPasswordAuthenticationHash); + if (!validate.Succeeded) + { + throw new BadRequestException( + string.Join("; ", validate.Errors.Select(e => e.Description))); + } + } + + if (setInitialData.RefreshStamp) + { + // TODO (PM-35501): IUserRepository.SetMasterPassword does not persist + // SecurityStamp (sproc + EF query both omit it). Rotation here is + // in-memory only until the primitive is extended. + user.SecurityStamp = Guid.NewGuid().ToString(); + } + + await setMasterPasswordTask(connection, transaction); + }; } public async Task> PrepareUpdateExistingMasterPasswordAsync( diff --git a/src/Infrastructure.Dapper/Repositories/UserRepository.cs b/src/Infrastructure.Dapper/Repositories/UserRepository.cs index ddb56e6f31bd..364d7108a716 100644 --- a/src/Infrastructure.Dapper/Repositories/UserRepository.cs +++ b/src/Infrastructure.Dapper/Repositories/UserRepository.cs @@ -488,6 +488,9 @@ public UpdateUserData SetMasterPassword(Guid userId, MasterPasswordUnlockData ma RevisionDate = timestamp, AccountRevisionDate = timestamp, MasterPasswordSalt = masterPasswordUnlockData.Salt + // TODO (PM-35501): Add SecurityStamp so the rotation done in + // MasterPasswordService.BuildUpdateUserDelegateSetInitialMasterPassword + // is persisted. }, transaction: transaction, commandType: CommandType.StoredProcedure); diff --git a/src/Infrastructure.EntityFramework/Repositories/UserRepository.cs b/src/Infrastructure.EntityFramework/Repositories/UserRepository.cs index f155cbe2beee..542a29aeb869 100644 --- a/src/Infrastructure.EntityFramework/Repositories/UserRepository.cs +++ b/src/Infrastructure.EntityFramework/Repositories/UserRepository.cs @@ -563,6 +563,9 @@ public UpdateUserData SetMasterPassword(Guid userId, MasterPasswordUnlockData ma userEntity.RevisionDate = timestamp; userEntity.AccountRevisionDate = timestamp; userEntity.MasterPasswordSalt = masterPasswordUnlockData.Salt; + // TODO (PM-35501): Persist SecurityStamp so the rotation done in + // MasterPasswordService.BuildUpdateUserDelegateSetInitialMasterPassword + // is persisted. await dbContext.SaveChangesAsync(); }; } diff --git a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/MasterPasswordServiceTests.cs b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/MasterPasswordServiceTests.cs index 04a6fde46ab6..f635792b3942 100644 --- a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/MasterPasswordServiceTests.cs +++ b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/MasterPasswordServiceTests.cs @@ -866,6 +866,102 @@ public void BuildUpdateUserDelegate_ThrowsWhenUserAlreadyHasMasterPassword(User () => sutProvider.Sut.BuildUpdateUserDelegateSetInitialMasterPassword(user, data)); } + // Contract: SetInitialPasswordData.ValidatePassword XML-docs say the password validator + // pipeline runs when true. All API layers/operations must honor these flags. + [Theory, BitAutoData] + public async Task BuildUpdateUserDelegate_WhenValidatePasswordTrue_InvokesValidator(User user) + { + var validator = Substitute.For>(); + validator.ValidateAsync(Arg.Any>(), Arg.Any(), Arg.Any()) + .Returns(IdentityResult.Success); + + var sutProvider = new SutProvider() + .WithFakeTimeProvider() + .SetDependency>>(new[] { validator }) + .Create(); + + user.MasterPassword = null; + user.Key = null; + user.MasterPasswordSalt = null; + user.UsesKeyConnector = false; + + UpdateUserData noopWrite = (_, _) => Task.CompletedTask; + sutProvider.GetDependency() + .SetMasterPassword(Arg.Any(), Arg.Any(), + Arg.Any(), Arg.Any()) + .Returns(noopWrite); + + var data = BuildSetInitialDataForUser(user); + data.ValidatePassword = true; + + var write = sutProvider.Sut.BuildUpdateUserDelegateSetInitialMasterPassword(user, data); + await write(null, null); + + await validator.Received() + .ValidateAsync(Arg.Any>(), user, Arg.Any()); + } + + // Contract: validator failure must surface through the delegate — Build* callers composing + // a batch transaction need the failure to roll the transaction back, not silently persist. + [Theory, BitAutoData] + public async Task BuildUpdateUserDelegate_WhenValidationFails_DelegateInvocationThrows(User user) + { + var error = new IdentityError { Code = "pwd-invalid", Description = "Password is too weak." }; + var validator = Substitute.For>(); + validator.ValidateAsync(Arg.Any>(), Arg.Any(), Arg.Any()) + .Returns(IdentityResult.Failed(error)); + + var sutProvider = new SutProvider() + .WithFakeTimeProvider() + .SetDependency>>(new[] { validator }) + .Create(); + + user.MasterPassword = null; + user.Key = null; + user.MasterPasswordSalt = null; + user.UsesKeyConnector = false; + + UpdateUserData noopWrite = (_, _) => Task.CompletedTask; + sutProvider.GetDependency() + .SetMasterPassword(Arg.Any(), Arg.Any(), + Arg.Any(), Arg.Any()) + .Returns(noopWrite); + + var data = BuildSetInitialDataForUser(user); + data.ValidatePassword = true; + + var write = sutProvider.Sut.BuildUpdateUserDelegateSetInitialMasterPassword(user, data); + + await Assert.ThrowsAsync(() => write(null, null)); + } + + // Contract: SetInitialPasswordData.RefreshStamp XML-docs say SecurityStamp rotates when true. + // Prepare*/Save* honor this; Build* must too (rotation composed into the returned delegate). + [Theory, BitAutoData] + public async Task BuildUpdateUserDelegate_WhenRefreshStampTrue_RotatesSecurityStamp(User user) + { + var sutProvider = new SutProvider().WithFakeTimeProvider().Create(); + user.MasterPassword = null; + user.Key = null; + user.MasterPasswordSalt = null; + user.UsesKeyConnector = false; + user.SecurityStamp = "original-stamp"; + + UpdateUserData noopWrite = (_, _) => Task.CompletedTask; + sutProvider.GetDependency() + .SetMasterPassword(Arg.Any(), Arg.Any(), + Arg.Any(), Arg.Any()) + .Returns(noopWrite); + + var data = BuildSetInitialDataForUser(user); + data.RefreshStamp = true; + + var write = sutProvider.Sut.BuildUpdateUserDelegateSetInitialMasterPassword(user, data); + await write(null, null); + + Assert.NotEqual("original-stamp", user.SecurityStamp); + } + private static SetInitialPasswordData BuildSetInitialDataForUser(User user) { var salt = user.GetMasterPasswordSalt(); From 86aa6819086c852bf9a3476e9494efccd61b0280 Mon Sep 17 00:00:00 2001 From: enmande <3836813+enmande@users.noreply.github.com> Date: Wed, 22 Apr 2026 14:00:36 -0400 Subject: [PATCH 06/26] refactor(mp-service): Align validate/hash/compose/execute pattern. --- .../MasterPasswordService.cs | 16 +++--- .../MasterPasswordServiceTests.cs | 50 +++++++++++++++++-- 2 files changed, 52 insertions(+), 14 deletions(-) diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs index 48325c49d942..1603ddb21929 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs @@ -101,14 +101,6 @@ public UpdateUserData BuildUpdateUserDelegateSetInitialMasterPassword( EnsureUserIsHydrated(user); setInitialData.ValidateDataForUser(user); - // Hash the provided user master password authentication hash on the server side - var serverSideHashedMasterPasswordAuthenticationHash = _passwordHasher.HashPassword(user, - setInitialData.MasterPasswordAuthentication.MasterPasswordAuthenticationHash); - - var setMasterPasswordTask = _userRepository.SetMasterPassword(user.Id, - setInitialData.MasterPasswordUnlock, serverSideHashedMasterPasswordAuthenticationHash, - setInitialData.MasterPasswordHint); - return async (connection, transaction) => { if (setInitialData.ValidatePassword) @@ -130,7 +122,13 @@ public UpdateUserData BuildUpdateUserDelegateSetInitialMasterPassword( user.SecurityStamp = Guid.NewGuid().ToString(); } - await setMasterPasswordTask(connection, transaction); + // Hash the provided user master password authentication hash on the server side + var serverSideHashedMasterPasswordAuthenticationHash = _passwordHasher.HashPassword(user, + setInitialData.MasterPasswordAuthentication.MasterPasswordAuthenticationHash); + + await _userRepository.SetMasterPassword(user.Id, + setInitialData.MasterPasswordUnlock, serverSideHashedMasterPasswordAuthenticationHash, + setInitialData.MasterPasswordHint)(connection, transaction); }; } diff --git a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/MasterPasswordServiceTests.cs b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/MasterPasswordServiceTests.cs index f635792b3942..c73ad5350907 100644 --- a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/MasterPasswordServiceTests.cs +++ b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/MasterPasswordServiceTests.cs @@ -616,6 +616,49 @@ await Assert.ThrowsAsync( () => sutProvider.Sut.PrepareSetInitialOrUpdateExistingMasterPasswordAsync(user, data)); } + [Theory, BitAutoData] + public async Task PrepareSetInitialOrUpdateExisting_PropagatesValidationErrors_WhenSetInitialPathFails(User user) + { + var error = new IdentityError { Code = "pwd-invalid", Description = "Password is too weak." }; + var validator = Substitute.For>(); + validator.ValidateAsync(Arg.Any>(), Arg.Any(), Arg.Any()) + .Returns(IdentityResult.Failed(error)); + + var sutProvider = new SutProvider() + .WithFakeTimeProvider() + .SetDependency>>(new[] { validator }) + .Create(); + + user.MasterPassword = null; + user.Key = null; + user.MasterPasswordSalt = null; + user.UsesKeyConnector = false; + + var (kdf, salt) = GetMatchingKdfAndSalt(user); + var data = new SetInitialOrUpdateExistingPasswordData + { + MasterPasswordAuthentication = new MasterPasswordAuthenticationData + { + Salt = salt, + MasterPasswordAuthenticationHash = "test-hash", + Kdf = kdf + }, + MasterPasswordUnlock = new MasterPasswordUnlockData + { + Salt = salt, + MasterKeyWrappedUserKey = "wrapped-key", + Kdf = kdf + }, + ValidatePassword = true, + RefreshStamp = false + }; + + var result = await sutProvider.Sut.PrepareSetInitialOrUpdateExistingMasterPasswordAsync(user, data); + + Assert.True(result.IsT1); + Assert.NotEmpty(result.AsT1); + } + [Theory, BitAutoData] public async Task PrepareSetInitialOrUpdateExisting_ThrowsWhenUserNotHydrated(User user) { @@ -838,18 +881,15 @@ public void BuildUpdateUserDelegate_HappyPath_ReturnsNonNullDelegateAndDoesNotPe user.UsesKeyConnector = false; var data = BuildSetInitialDataForUser(user); - sutProvider.GetDependency>() - .HashPassword(Arg.Any(), Arg.Any()) - .Returns("server-hash"); var result = sutProvider.Sut.BuildUpdateUserDelegateSetInitialMasterPassword(user, data); // The Build* tier returns a delegate — it must not persist directly. Assert.NotNull(result); sutProvider.GetDependency().DidNotReceive().ReplaceAsync(Arg.Any()); - // Hashing is eager at build time, not deferred. + // Hashing is deferred into the delegate (validate → hash → persist), not eager at build time. sutProvider.GetDependency>() - .Received() + .DidNotReceive() .HashPassword(user, Arg.Any()); } From b2f9dd65effc3e1d25035776ddb7af49213cd96f Mon Sep 17 00:00:00 2001 From: enmande <3836813+enmande@users.noreply.github.com> Date: Wed, 22 Apr 2026 14:27:49 -0400 Subject: [PATCH 07/26] test(mp-service): Tighten test assertions. --- .../MasterPasswordServiceTests.cs | 45 ++++++------------- 1 file changed, 13 insertions(+), 32 deletions(-) diff --git a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/MasterPasswordServiceTests.cs b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/MasterPasswordServiceTests.cs index c73ad5350907..07a26b327b20 100644 --- a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/MasterPasswordServiceTests.cs +++ b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/MasterPasswordServiceTests.cs @@ -257,28 +257,6 @@ public async Task PrepareSetInitialMasterPassword_RefreshStampFalse_PreservesSec Assert.Equal(originalStamp, user.SecurityStamp); } - // --- PrepareSetInitialMasterPassword — OneOf return shape --- - - [Theory, BitAutoData] - public async Task PrepareSetInitialMasterPassword_Success_ReturnsUserAsT0(User user) - { - var sutProvider = CreateSutProvider(); - user.MasterPassword = null; - user.Key = null; - user.MasterPasswordSalt = null; - user.UsesKeyConnector = false; - - var data = BuildSetInitialData(user); - sutProvider.GetDependency>() - .HashPassword(Arg.Any(), Arg.Any()) - .Returns("hash"); - - var result = await sutProvider.Sut.PrepareSetInitialMasterPasswordAsync(user, data); - - Assert.True(result.IsT0); - Assert.Same(user, result.AsT0); - } - [Theory, BitAutoData] public async Task PrepareSetInitialMasterPassword_ValidationFailure_ReturnsErrorsAsT1(User user) { @@ -585,7 +563,7 @@ public async Task PrepareSetInitialOrUpdateExisting_RoutesToUpdateExisting_WhenH } [Theory, BitAutoData] - public async Task PrepareSetInitialOrUpdateExisting_PropagatesValidationErrors_WhenUpdatePathFails(User user) + public async Task PrepareSetInitialOrUpdateExisting_ThrowsBadRequest_WhenUpdatePathSaltMismatch(User user) { var sutProvider = CreateSutProvider(); user.MasterPassword = "existing-hash"; @@ -887,10 +865,6 @@ public void BuildUpdateUserDelegate_HappyPath_ReturnsNonNullDelegateAndDoesNotPe // The Build* tier returns a delegate — it must not persist directly. Assert.NotNull(result); sutProvider.GetDependency().DidNotReceive().ReplaceAsync(Arg.Any()); - // Hashing is deferred into the delegate (validate → hash → persist), not eager at build time. - sutProvider.GetDependency>() - .DidNotReceive() - .HashPassword(user, Arg.Any()); } [Theory, BitAutoData] @@ -906,10 +880,10 @@ public void BuildUpdateUserDelegate_ThrowsWhenUserAlreadyHasMasterPassword(User () => sutProvider.Sut.BuildUpdateUserDelegateSetInitialMasterPassword(user, data)); } - // Contract: SetInitialPasswordData.ValidatePassword XML-docs say the password validator - // pipeline runs when true. All API layers/operations must honor these flags. + // Contract: when ValidatePassword is true and validation succeeds the delegate must complete + // and pass the server-side hashed password to the repository. [Theory, BitAutoData] - public async Task BuildUpdateUserDelegate_WhenValidatePasswordTrue_InvokesValidator(User user) + public async Task BuildUpdateUserDelegate_WhenValidatePasswordTrue_CompletesAndPersistsHash(User user) { var validator = Substitute.For>(); validator.ValidateAsync(Arg.Any>(), Arg.Any(), Arg.Any()) @@ -925,6 +899,11 @@ public async Task BuildUpdateUserDelegate_WhenValidatePasswordTrue_InvokesValida user.MasterPasswordSalt = null; user.UsesKeyConnector = false; + var expectedHash = "expected-hash"; + sutProvider.GetDependency>() + .HashPassword(Arg.Any(), Arg.Any()) + .Returns(expectedHash); + UpdateUserData noopWrite = (_, _) => Task.CompletedTask; sutProvider.GetDependency() .SetMasterPassword(Arg.Any(), Arg.Any(), @@ -937,8 +916,10 @@ public async Task BuildUpdateUserDelegate_WhenValidatePasswordTrue_InvokesValida var write = sutProvider.Sut.BuildUpdateUserDelegateSetInitialMasterPassword(user, data); await write(null, null); - await validator.Received() - .ValidateAsync(Arg.Any>(), user, Arg.Any()); + // The Build* tier's output is the values handed to the repository — user.MasterPassword + // is not mutated; the hash is passed through to SetMasterPassword directly. + sutProvider.GetDependency().Received() + .SetMasterPassword(user.Id, data.MasterPasswordUnlock, expectedHash, Arg.Any()); } // Contract: validator failure must surface through the delegate — Build* callers composing From b7db39bb308744cb5d5c5933006767d940ea360b Mon Sep 17 00:00:00 2001 From: enmande <3836813+enmande@users.noreply.github.com> Date: Wed, 22 Apr 2026 16:14:29 -0400 Subject: [PATCH 08/26] refactor(mp-service) chants: unlock and authenticate. --- .../SetInitialOrUpdateExistingPasswordData.cs | 6 +- .../Data/SetInitialPasswordData.cs | 4 +- .../Data/UpdateExistingPasswordAndKdfData.cs | 4 +- .../Data/UpdateExistingPasswordData.cs | 6 +- .../Interfaces/IMasterPasswordService.cs | 6 +- .../MasterPasswordServiceTests.cs | 140 +++++++++--------- 6 files changed, 83 insertions(+), 83 deletions(-) diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/Data/SetInitialOrUpdateExistingPasswordData.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/Data/SetInitialOrUpdateExistingPasswordData.cs index 80c75d68141c..b094363e98f2 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/Data/SetInitialOrUpdateExistingPasswordData.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/Data/SetInitialOrUpdateExistingPasswordData.cs @@ -4,8 +4,8 @@ namespace Bit.Core.Auth.UserFeatures.UserMasterPassword.Data; public class SetInitialOrUpdateExistingPasswordData { - public required MasterPasswordAuthenticationData MasterPasswordAuthentication { get; set; } public required MasterPasswordUnlockData MasterPasswordUnlock { get; set; } + public required MasterPasswordAuthenticationData MasterPasswordAuthentication { get; set; } /// /// When true, runs the new password hash through the registered @@ -25,8 +25,8 @@ public class SetInitialOrUpdateExistingPasswordData public SetInitialPasswordData ToSetInitialData() => new() { - MasterPasswordAuthentication = MasterPasswordAuthentication, MasterPasswordUnlock = MasterPasswordUnlock, + MasterPasswordAuthentication = MasterPasswordAuthentication, ValidatePassword = ValidatePassword, RefreshStamp = RefreshStamp, MasterPasswordHint = MasterPasswordHint @@ -34,8 +34,8 @@ public class SetInitialOrUpdateExistingPasswordData public UpdateExistingPasswordData ToUpdateExistingData() => new() { - MasterPasswordAuthentication = MasterPasswordAuthentication, MasterPasswordUnlock = MasterPasswordUnlock, + MasterPasswordAuthentication = MasterPasswordAuthentication, ValidatePassword = ValidatePassword, RefreshStamp = RefreshStamp, MasterPasswordHint = MasterPasswordHint diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/Data/SetInitialPasswordData.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/Data/SetInitialPasswordData.cs index d4be56836688..5fe68217c5da 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/Data/SetInitialPasswordData.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/Data/SetInitialPasswordData.cs @@ -6,8 +6,8 @@ namespace Bit.Core.Auth.UserFeatures.UserMasterPassword.Data; public class SetInitialPasswordData { - public required MasterPasswordAuthenticationData MasterPasswordAuthentication { get; set; } public required MasterPasswordUnlockData MasterPasswordUnlock { get; set; } + public required MasterPasswordAuthenticationData MasterPasswordAuthentication { get; set; } /// /// When true, runs the new password hash through the registered @@ -61,7 +61,7 @@ public void ValidateDataForUser(User user) // is null, and a mismatch here would make the user un-decryptable on next login. Centralized // here so both TDE and SSO JIT initial-SET flows enforce the same rule. This check is // removed in Stage 3 when PM-28143 feature flag clears and independent salts are safe. - MasterPasswordAuthentication.ValidateSaltUnchangedForUser(user); MasterPasswordUnlock.ValidateSaltUnchangedForUser(user); + MasterPasswordAuthentication.ValidateSaltUnchangedForUser(user); } } diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordAndKdfData.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordAndKdfData.cs index 3d29bad15656..2c9c73077ebb 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordAndKdfData.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordAndKdfData.cs @@ -6,8 +6,8 @@ namespace Bit.Core.Auth.UserFeatures.UserMasterPassword.Data; public class UpdateExistingPasswordAndKdfData { - public required MasterPasswordAuthenticationData MasterPasswordAuthentication { get; set; } public required MasterPasswordUnlockData MasterPasswordUnlock { get; set; } + public required MasterPasswordAuthenticationData MasterPasswordAuthentication { get; set; } /// /// When true, runs the new password hash through the registered @@ -49,7 +49,7 @@ public void ValidateDataForUser(User user) // Do not validate if kdf is the same here on the user because we are changing it. // Validate Salt is unchanged for user - MasterPasswordAuthentication.ValidateSaltUnchangedForUser(user); MasterPasswordUnlock.ValidateSaltUnchangedForUser(user); + MasterPasswordAuthentication.ValidateSaltUnchangedForUser(user); } } diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordData.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordData.cs index 0be0322cceb1..2c7ef218897e 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordData.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordData.cs @@ -6,8 +6,8 @@ namespace Bit.Core.Auth.UserFeatures.UserMasterPassword.Data; public class UpdateExistingPasswordData { - public required MasterPasswordAuthenticationData MasterPasswordAuthentication { get; set; } public required MasterPasswordUnlockData MasterPasswordUnlock { get; set; } + public required MasterPasswordAuthenticationData MasterPasswordAuthentication { get; set; } /// /// When true, runs the new password hash through the registered @@ -47,11 +47,11 @@ public void ValidateDataForUser(User user) } // Validate KDF is unchanged for user - MasterPasswordAuthentication.Kdf.ValidateUnchangedForUser(user); MasterPasswordUnlock.Kdf.ValidateUnchangedForUser(user); + MasterPasswordAuthentication.Kdf.ValidateUnchangedForUser(user); // Validate Salt is unchanged for user - MasterPasswordAuthentication.ValidateSaltUnchangedForUser(user); MasterPasswordUnlock.ValidateSaltUnchangedForUser(user); + MasterPasswordAuthentication.ValidateSaltUnchangedForUser(user); } } diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs index 35ab4941189b..257f2845b8a9 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs @@ -92,8 +92,8 @@ public interface IMasterPasswordService /// /// /// Cryptographic and authentication data required to set the initial password, including - /// MasterPasswordAuthentication (hashed credential used for login), /// MasterPasswordUnlock (KDF parameters and wrapped user key), + /// MasterPasswordAuthentication (hashed credential used for login), /// and control flags ValidatePassword and RefreshStamp. /// /// @@ -156,7 +156,7 @@ public interface IMasterPasswordService /// /// /// Cryptographic and authentication data for the updated password, including - /// MasterPasswordAuthentication, MasterPasswordUnlock, + /// MasterPasswordUnlock, MasterPasswordAuthentication, /// and control flags ValidatePassword and RefreshStamp. /// /// @@ -177,7 +177,7 @@ public interface IMasterPasswordService /// /// /// Cryptographic and authentication data for the updated password and KDF parameters, - /// including MasterPasswordAuthentication, MasterPasswordUnlock, + /// including MasterPasswordUnlock, MasterPasswordAuthentication, /// and control flags ValidatePassword and RefreshStamp. /// /// diff --git a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/MasterPasswordServiceTests.cs b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/MasterPasswordServiceTests.cs index 07a26b327b20..659c91dd1e93 100644 --- a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/MasterPasswordServiceTests.cs +++ b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/MasterPasswordServiceTests.cs @@ -48,16 +48,16 @@ private static SetInitialPasswordData BuildSetInitialData(User user, string? hin }; return new SetInitialPasswordData { - MasterPasswordAuthentication = new MasterPasswordAuthenticationData + MasterPasswordUnlock = new MasterPasswordUnlockData { Salt = salt, - MasterPasswordAuthenticationHash = "test-hash", + MasterKeyWrappedUserKey = "wrapped-key", Kdf = kdf }, - MasterPasswordUnlock = new MasterPasswordUnlockData + MasterPasswordAuthentication = new MasterPasswordAuthenticationData { Salt = salt, - MasterKeyWrappedUserKey = "wrapped-key", + MasterPasswordAuthenticationHash = "test-hash", Kdf = kdf }, MasterPasswordHint = hint, @@ -72,16 +72,16 @@ private static UpdateExistingPasswordData BuildUpdateExistingData(User user, str var (kdf, salt) = GetMatchingKdfAndSalt(user); return new UpdateExistingPasswordData { - MasterPasswordAuthentication = new MasterPasswordAuthenticationData + MasterPasswordUnlock = new MasterPasswordUnlockData { Salt = salt, - MasterPasswordAuthenticationHash = "test-hash", + MasterKeyWrappedUserKey = "wrapped-key", Kdf = kdf }, - MasterPasswordUnlock = new MasterPasswordUnlockData + MasterPasswordAuthentication = new MasterPasswordAuthenticationData { Salt = salt, - MasterKeyWrappedUserKey = "wrapped-key", + MasterPasswordAuthenticationHash = "test-hash", Kdf = kdf }, MasterPasswordHint = hint, @@ -103,16 +103,16 @@ private static UpdateExistingPasswordAndKdfData BuildUpdateExistingAndKdfData(Us }; return new UpdateExistingPasswordAndKdfData { - MasterPasswordAuthentication = new MasterPasswordAuthenticationData + MasterPasswordUnlock = new MasterPasswordUnlockData { Salt = salt, - MasterPasswordAuthenticationHash = "test-hash", + MasterKeyWrappedUserKey = "wrapped-key", Kdf = kdf }, - MasterPasswordUnlock = new MasterPasswordUnlockData + MasterPasswordAuthentication = new MasterPasswordAuthenticationData { Salt = salt, - MasterKeyWrappedUserKey = "wrapped-key", + MasterPasswordAuthenticationHash = "test-hash", Kdf = kdf }, MasterPasswordHint = hint, @@ -210,16 +210,16 @@ public async Task PrepareSetInitialMasterPassword_RefreshStampTrue_RotatesSecuri }; var data = new SetInitialPasswordData { - MasterPasswordAuthentication = new MasterPasswordAuthenticationData + MasterPasswordUnlock = new MasterPasswordUnlockData { Salt = salt, - MasterPasswordAuthenticationHash = "test-hash", + MasterKeyWrappedUserKey = "wrapped-key", Kdf = kdf }, - MasterPasswordUnlock = new MasterPasswordUnlockData + MasterPasswordAuthentication = new MasterPasswordAuthenticationData { Salt = salt, - MasterKeyWrappedUserKey = "wrapped-key", + MasterPasswordAuthenticationHash = "test-hash", Kdf = kdf }, ValidatePassword = false, @@ -498,16 +498,16 @@ public async Task PrepareSetInitialOrUpdateExisting_RoutesToSetInitial_WhenNoMas }; var data = new SetInitialOrUpdateExistingPasswordData { - MasterPasswordAuthentication = new MasterPasswordAuthenticationData + MasterPasswordUnlock = new MasterPasswordUnlockData { Salt = salt, - MasterPasswordAuthenticationHash = "test-hash", + MasterKeyWrappedUserKey = "wrapped-key", Kdf = kdf }, - MasterPasswordUnlock = new MasterPasswordUnlockData + MasterPasswordAuthentication = new MasterPasswordAuthenticationData { Salt = salt, - MasterKeyWrappedUserKey = "wrapped-key", + MasterPasswordAuthenticationHash = "test-hash", Kdf = kdf }, ValidatePassword = false, @@ -535,16 +535,16 @@ public async Task PrepareSetInitialOrUpdateExisting_RoutesToUpdateExisting_WhenH var (kdf, salt) = GetMatchingKdfAndSalt(user); var data = new SetInitialOrUpdateExistingPasswordData { - MasterPasswordAuthentication = new MasterPasswordAuthenticationData + MasterPasswordUnlock = new MasterPasswordUnlockData { Salt = salt, - MasterPasswordAuthenticationHash = "test-hash", + MasterKeyWrappedUserKey = "wrapped-key", Kdf = kdf }, - MasterPasswordUnlock = new MasterPasswordUnlockData + MasterPasswordAuthentication = new MasterPasswordAuthenticationData { Salt = salt, - MasterKeyWrappedUserKey = "wrapped-key", + MasterPasswordAuthenticationHash = "test-hash", Kdf = kdf }, ValidatePassword = false, @@ -574,16 +574,16 @@ public async Task PrepareSetInitialOrUpdateExisting_ThrowsBadRequest_WhenUpdateP var wrongSalt = "wrong-salt-value"; var data = new SetInitialOrUpdateExistingPasswordData { - MasterPasswordAuthentication = new MasterPasswordAuthenticationData + MasterPasswordUnlock = new MasterPasswordUnlockData { Salt = wrongSalt, - MasterPasswordAuthenticationHash = "test-hash", + MasterKeyWrappedUserKey = "wrapped-key", Kdf = kdf }, - MasterPasswordUnlock = new MasterPasswordUnlockData + MasterPasswordAuthentication = new MasterPasswordAuthenticationData { Salt = wrongSalt, - MasterKeyWrappedUserKey = "wrapped-key", + MasterPasswordAuthenticationHash = "test-hash", Kdf = kdf }, ValidatePassword = false, @@ -615,16 +615,16 @@ public async Task PrepareSetInitialOrUpdateExisting_PropagatesValidationErrors_W var (kdf, salt) = GetMatchingKdfAndSalt(user); var data = new SetInitialOrUpdateExistingPasswordData { - MasterPasswordAuthentication = new MasterPasswordAuthenticationData + MasterPasswordUnlock = new MasterPasswordUnlockData { Salt = salt, - MasterPasswordAuthenticationHash = "test-hash", + MasterKeyWrappedUserKey = "wrapped-key", Kdf = kdf }, - MasterPasswordUnlock = new MasterPasswordUnlockData + MasterPasswordAuthentication = new MasterPasswordAuthenticationData { Salt = salt, - MasterKeyWrappedUserKey = "wrapped-key", + MasterPasswordAuthenticationHash = "test-hash", Kdf = kdf }, ValidatePassword = true, @@ -646,16 +646,16 @@ public async Task PrepareSetInitialOrUpdateExisting_ThrowsWhenUserNotHydrated(Us var (kdf, salt) = GetMatchingKdfAndSalt(user); var data = new SetInitialOrUpdateExistingPasswordData { - MasterPasswordAuthentication = new MasterPasswordAuthenticationData + MasterPasswordUnlock = new MasterPasswordUnlockData { Salt = salt, - MasterPasswordAuthenticationHash = "test-hash", + MasterKeyWrappedUserKey = "wrapped-key", Kdf = kdf }, - MasterPasswordUnlock = new MasterPasswordUnlockData + MasterPasswordAuthentication = new MasterPasswordAuthenticationData { Salt = salt, - MasterKeyWrappedUserKey = "wrapped-key", + MasterPasswordAuthenticationHash = "test-hash", Kdf = kdf }, ValidatePassword = false, @@ -797,18 +797,18 @@ public async Task SaveUpdateExistingMasterPasswordAndKdf_ThrowsWhenSaltChanged(U }; var data = new UpdateExistingPasswordAndKdfData { - MasterPasswordAuthentication = new MasterPasswordAuthenticationData - { - Salt = "wrong-salt", - MasterPasswordAuthenticationHash = "test-hash", - Kdf = kdf - }, MasterPasswordUnlock = new MasterPasswordUnlockData { Salt = salt, MasterKeyWrappedUserKey = "wrapped-key", Kdf = kdf }, + MasterPasswordAuthentication = new MasterPasswordAuthenticationData + { + Salt = "wrong-salt", + MasterPasswordAuthenticationHash = "test-hash", + Kdf = kdf + }, ValidatePassword = false, RefreshStamp = false }; @@ -995,16 +995,16 @@ private static SetInitialPasswordData BuildSetInitialDataForUser(User user) }; return new SetInitialPasswordData { - MasterPasswordAuthentication = new MasterPasswordAuthenticationData + MasterPasswordUnlock = new MasterPasswordUnlockData { Salt = salt, - MasterPasswordAuthenticationHash = "test-hash", + MasterKeyWrappedUserKey = "wrapped-key", Kdf = kdf }, - MasterPasswordUnlock = new MasterPasswordUnlockData + MasterPasswordAuthentication = new MasterPasswordAuthenticationData { Salt = salt, - MasterKeyWrappedUserKey = "wrapped-key", + MasterPasswordAuthenticationHash = "test-hash", Kdf = kdf }, ValidatePassword = false, @@ -1046,16 +1046,16 @@ private static SetInitialPasswordData BuildData(User user, string? saltOverride }; return new SetInitialPasswordData { - MasterPasswordAuthentication = new MasterPasswordAuthenticationData + MasterPasswordUnlock = new MasterPasswordUnlockData { Salt = salt, - MasterPasswordAuthenticationHash = "hash", + MasterKeyWrappedUserKey = "wrapped-key", Kdf = kdf }, - MasterPasswordUnlock = new MasterPasswordUnlockData + MasterPasswordAuthentication = new MasterPasswordAuthenticationData { Salt = salt, - MasterKeyWrappedUserKey = "wrapped-key", + MasterPasswordAuthenticationHash = "hash", Kdf = kdf } }; @@ -1135,17 +1135,17 @@ public void ValidateDataForUser_Throws_WhenAuthenticationSaltMismatch_UnlockSalt // Authentication salt is wrong; Unlock salt is correct. var data = new SetInitialPasswordData { - MasterPasswordAuthentication = new MasterPasswordAuthenticationData - { - Salt = "wrong-auth-salt", - MasterPasswordAuthenticationHash = "hash", - Kdf = kdf - }, MasterPasswordUnlock = new MasterPasswordUnlockData { Salt = correctSalt, MasterKeyWrappedUserKey = "wrapped-key", Kdf = kdf + }, + MasterPasswordAuthentication = new MasterPasswordAuthenticationData + { + Salt = "wrong-auth-salt", + MasterPasswordAuthenticationHash = "hash", + Kdf = kdf } }; @@ -1167,17 +1167,17 @@ public void ValidateDataForUser_Throws_WhenUnlockSaltMismatch_AuthenticationSalt // Unlock salt is wrong; Authentication salt is correct. var data = new SetInitialPasswordData { - MasterPasswordAuthentication = new MasterPasswordAuthenticationData - { - Salt = correctSalt, - MasterPasswordAuthenticationHash = "hash", - Kdf = kdf - }, MasterPasswordUnlock = new MasterPasswordUnlockData { Salt = "wrong-unlock-salt", MasterKeyWrappedUserKey = "wrapped-key", Kdf = kdf + }, + MasterPasswordAuthentication = new MasterPasswordAuthenticationData + { + Salt = correctSalt, + MasterPasswordAuthenticationHash = "hash", + Kdf = kdf } }; @@ -1218,16 +1218,16 @@ private static UpdateExistingPasswordData BuildData(User user, string? saltOverr }; return new UpdateExistingPasswordData { - MasterPasswordAuthentication = new MasterPasswordAuthenticationData + MasterPasswordUnlock = new MasterPasswordUnlockData { Salt = salt, - MasterPasswordAuthenticationHash = "hash", + MasterKeyWrappedUserKey = "wrapped-key", Kdf = kdf }, - MasterPasswordUnlock = new MasterPasswordUnlockData + MasterPasswordAuthentication = new MasterPasswordAuthenticationData { Salt = salt, - MasterKeyWrappedUserKey = "wrapped-key", + MasterPasswordAuthenticationHash = "hash", Kdf = kdf } }; @@ -1320,16 +1320,16 @@ private static UpdateExistingPasswordAndKdfData BuildData(User user, string? sal }; return new UpdateExistingPasswordAndKdfData { - MasterPasswordAuthentication = new MasterPasswordAuthenticationData + MasterPasswordUnlock = new MasterPasswordUnlockData { Salt = salt, - MasterPasswordAuthenticationHash = "hash", + MasterKeyWrappedUserKey = "wrapped-key", Kdf = newKdf }, - MasterPasswordUnlock = new MasterPasswordUnlockData + MasterPasswordAuthentication = new MasterPasswordAuthenticationData { Salt = salt, - MasterKeyWrappedUserKey = "wrapped-key", + MasterPasswordAuthenticationHash = "hash", Kdf = newKdf } }; From ba43405a175438a51309b50f8e52c01d923ee83c Mon Sep 17 00:00:00 2001 From: enmande <3836813+enmande@users.noreply.github.com> Date: Wed, 22 Apr 2026 16:45:23 -0400 Subject: [PATCH 09/26] docs(mp-service): Re-fit some XML doc comment tags for general support. --- .../Interfaces/IMasterPasswordService.cs | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs index 257f2845b8a9..60b326112781 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs @@ -11,11 +11,11 @@ namespace Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; /// Provides consistent validation, password hashing, and timestamp management across every /// flow that establishes or updates a user's master password. /// -/// Compositional, not orchestrating. This service handles CRUD-like mutations +/// Compositional, not orchestrating. This service handles CRUD-like mutations /// only. Business logic (e.g., authorization checks, org validation, push notifications, event /// logging) remains a caller responsibility. /// -/// Three persistence tiers: +/// Three persistence tiers: /// /// /// Prepare* Modifies the object in memory only. The caller @@ -36,24 +36,27 @@ namespace Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; /// /// /// -/// Set vs Update contract: -/// +/// Set vs Update contract: +/// /// -/// SET (initial): Client sends all data (hash, salt, KDF). Server sets all +/// SET (initial) +/// Client sends all data (hash, salt, KDF). Server sets all /// fields. Stage 1 caveat: server enforces salt == email.ToLowerInvariant().Trim() -/// (PM-28143 removes this in Stage 3). +/// (PM-28143 removes this in Stage 3). /// /// -/// UPDATE (hash only): Client sends all data. Server validates KDF and salt -/// are unchanged, updates only the hash and wrapped user key. +/// UPDATE (hash only) +/// Client sends all data. Server validates KDF and salt +/// are unchanged, updates only the hash and wrapped user key. /// /// -/// UPDATE (KDF): Client sends all data. Server validates salt is unchanged, -/// updates hash, KDF, and wrapped user key. +/// UPDATE (KDF) +/// Client sends all data. Server validates salt is unchanged, +/// updates hash, KDF, and wrapped user key. /// /// /// -/// Source of truth: On SET, the client is the source of truth. On UPDATE, +/// Source of truth: On SET, the client is the source of truth. On UPDATE, /// the server is the source of truth for fields that must not change — it validates the client's /// values match what's stored before applying the update. /// From 22731cb10f8cfc65dff32bc853eccd72038c6fa3 Mon Sep 17 00:00:00 2001 From: enmande <3836813+enmande@users.noreply.github.com> Date: Thu, 23 Apr 2026 09:58:45 -0400 Subject: [PATCH 10/26] docs(mp-service): Address review comment feedback. --- .../Interfaces/IMasterPasswordService.cs | 6 ++-- .../MasterPasswordService.cs | 30 ++++++++++++++----- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs index 60b326112781..f7fd22fb7df8 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs @@ -7,7 +7,7 @@ namespace Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; /// -/// Centralized mutation point for all master password set, change, and rotate operations. +/// Centralized mutation point for all master password set and update operations. /// Provides consistent validation, password hashing, and timestamp management across every /// flow that establishes or updates a user's master password. /// @@ -170,8 +170,8 @@ public interface IMasterPasswordService /// /// Applies a new master password and updated KDF parameters over the user's existing ones - /// and persists the updated user to the database. Salt must remain unchanged; KDF is - /// intentionally allowed to change. Use for KDF rotation flows. + /// and persists the updated user to the database. Salt must remain unchanged; KDF + /// validation is intentionally skipped. Use for KDF rotation flows. /// /// /// The user object to mutate and persist. Must already have a master password; diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs index 1603ddb21929..1ec50c225ade 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs @@ -51,6 +51,10 @@ public async Task> PrepareSetInitialMasterPasswordA EnsureUserIsHydrated(user); setInitialData.ValidateDataForUser(user); + // Note: Keep "unlock then authenticate" pattern in mind. + // This is a purposeful inversion of that principle: + // Authentication is derivative of unlock, and is the mechanism for validation; + // eager validation keeps the logic easier to reason about, so it is performed first. var result = await UpdateExistingPasswordHashAsync( user, setInitialData.MasterPasswordAuthentication.MasterPasswordAuthenticationHash, @@ -67,7 +71,7 @@ public async Task> PrepareSetInitialMasterPasswordA // Set salt on the user user.MasterPasswordSalt = setInitialData.MasterPasswordUnlock.Salt; - // Always override the master password hint, even if it's null. + // Always override the master password hint, even if it's null user.MasterPasswordHint = setInitialData.MasterPasswordHint; // Update time markers on the user @@ -103,6 +107,10 @@ public UpdateUserData BuildUpdateUserDelegateSetInitialMasterPassword( return async (connection, transaction) => { + // Note: Keep "unlock then authenticate" pattern in mind. + // This is a purposeful inversion of that principle: + // Authentication is derivative of unlock, and is the mechanism for validation; + // eager validation keeps the logic easier to reason about, so it is performed first. if (setInitialData.ValidatePassword) { var validate = await ValidatePasswordInternalAsync(user, @@ -139,6 +147,10 @@ public async Task> PrepareUpdateExistingMasterPassw EnsureUserIsHydrated(user); updateExistingData.ValidateDataForUser(user); + // Note: Keep "unlock then authenticate" pattern in mind. + // This is a purposeful inversion of that principle: + // Authentication is derivative of unlock, and is the mechanism for validation; + // eager validation keeps the logic easier to reason about, so it is performed first. var result = await UpdateExistingPasswordHashAsync( user, updateExistingData.MasterPasswordAuthentication.MasterPasswordAuthenticationHash, @@ -150,13 +162,13 @@ public async Task> PrepareUpdateExistingMasterPassw return result.Errors.ToArray(); } - var now = _timeProvider.GetUtcNow().UtcDateTime; - user.Key = updateExistingData.MasterPasswordUnlock.MasterKeyWrappedUserKey; - // Always override the master password hint, even if it's null. + // Always override the master password hint, even if it's null user.MasterPasswordHint = updateExistingData.MasterPasswordHint; + // Update time markers on the user + var now = _timeProvider.GetUtcNow().UtcDateTime; user.LastPasswordChangeDate = now; user.RevisionDate = user.AccountRevisionDate = now; @@ -170,6 +182,10 @@ public async Task> SaveUpdateExistingMasterPassword EnsureUserIsHydrated(user); updateExistingExistingData.ValidateDataForUser(user); + // Note: Keep "unlock then authenticate" pattern in mind. + // This is a purposeful inversion of that principle: + // Authentication is derivative of unlock, and is the mechanism for validation; + // eager validation keeps the logic easier to reason about, so it is performed first. var result = await UpdateExistingPasswordHashAsync( user, updateExistingExistingData.MasterPasswordAuthentication.MasterPasswordAuthenticationHash, @@ -181,14 +197,14 @@ public async Task> SaveUpdateExistingMasterPassword return result.Errors.ToArray(); } - var now = _timeProvider.GetUtcNow().UtcDateTime; - user.Key = updateExistingExistingData.MasterPasswordUnlock.MasterKeyWrappedUserKey; SetKdfStateOnUser(user, updateExistingExistingData.MasterPasswordUnlock.Kdf); - // Always override the master password hint, even if it's null. + // Always override the master password hint, even if it's null user.MasterPasswordHint = updateExistingExistingData.MasterPasswordHint; + // Update time markers on the user + var now = _timeProvider.GetUtcNow().UtcDateTime; user.LastPasswordChangeDate = now; user.LastKdfChangeDate = now; user.RevisionDate = user.AccountRevisionDate = now; From 58ffe45c6a7028faeccbc3ae3caf471a485c9396 Mon Sep 17 00:00:00 2001 From: enmande <3836813+enmande@users.noreply.github.com> Date: Thu, 23 Apr 2026 10:20:39 -0400 Subject: [PATCH 11/26] refactor(mp-service): Apply result.Tx handling to all OneOf returns. --- .../UserMasterPassword/MasterPasswordService.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs index 1ec50c225ade..ea7fd11aec93 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs @@ -93,9 +93,9 @@ public async Task> SaveSetInitialMasterPasswordAsyn return result.AsT1; } - await _userRepository.ReplaceAsync(user); + await _userRepository.ReplaceAsync(result.AsT0); - return user; + return result.AsT0; } public UpdateUserData BuildUpdateUserDelegateSetInitialMasterPassword( @@ -225,9 +225,9 @@ public async Task> SaveUpdateExistingMasterPassword return result.AsT1; } - await _userRepository.ReplaceAsync(user); + await _userRepository.ReplaceAsync(result.AsT0); - return user; + return result.AsT0; } private async Task UpdateExistingPasswordHashAsync(User user, string newPassword, From 7b8b66986b94c80ae42e2dfcdf17b17592d08ddf Mon Sep 17 00:00:00 2001 From: enmande <3836813+enmande@users.noreply.github.com> Date: Thu, 23 Apr 2026 13:40:15 -0400 Subject: [PATCH 12/26] docs(mp-service): Refine unlock vs authentication data comments. --- .../MasterPasswordService.cs | 40 +++++++++++-------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs index ea7fd11aec93..2934994d75ab 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs @@ -51,10 +51,12 @@ public async Task> PrepareSetInitialMasterPasswordA EnsureUserIsHydrated(user); setInitialData.ValidateDataForUser(user); - // Note: Keep "unlock then authenticate" pattern in mind. - // This is a purposeful inversion of that principle: - // Authentication is derivative of unlock, and is the mechanism for validation; - // eager validation keeps the logic easier to reason about, so it is performed first. + // Note: Keep "unlock and authenticate" pattern in mind. + // We name "unlock" first as a naming convention, + // e.g., MatherPasswordUnlockAndAuthenticationDataModel. + // The two are complimentary, but mechanically Authenticate comes first. + // Eager validation keeps the logic easier to reason about. + // Authentication is the mechanism for validation, unlock is the capability. var result = await UpdateExistingPasswordHashAsync( user, setInitialData.MasterPasswordAuthentication.MasterPasswordAuthenticationHash, @@ -107,10 +109,12 @@ public UpdateUserData BuildUpdateUserDelegateSetInitialMasterPassword( return async (connection, transaction) => { - // Note: Keep "unlock then authenticate" pattern in mind. - // This is a purposeful inversion of that principle: - // Authentication is derivative of unlock, and is the mechanism for validation; - // eager validation keeps the logic easier to reason about, so it is performed first. + // Note: Keep "unlock and authenticate" pattern in mind. + // We name "unlock" first as a naming convention, + // e.g., MatherPasswordUnlockAndAuthenticationDataModel. + // The two are complimentary, but mechanically Authenticate comes first. + // Eager validation keeps the logic easier to reason about. + // Authentication is the mechanism for validation, unlock is the capability. if (setInitialData.ValidatePassword) { var validate = await ValidatePasswordInternalAsync(user, @@ -147,10 +151,12 @@ public async Task> PrepareUpdateExistingMasterPassw EnsureUserIsHydrated(user); updateExistingData.ValidateDataForUser(user); - // Note: Keep "unlock then authenticate" pattern in mind. - // This is a purposeful inversion of that principle: - // Authentication is derivative of unlock, and is the mechanism for validation; - // eager validation keeps the logic easier to reason about, so it is performed first. + // Note: Keep "unlock and authenticate" pattern in mind. + // We name "unlock" first as a naming convention, + // e.g., MatherPasswordUnlockAndAuthenticationDataModel. + // The two are complimentary, but mechanically Authenticate comes first. + // Eager validation keeps the logic easier to reason about. + // Authentication is the mechanism for validation, unlock is the capability. var result = await UpdateExistingPasswordHashAsync( user, updateExistingData.MasterPasswordAuthentication.MasterPasswordAuthenticationHash, @@ -182,10 +188,12 @@ public async Task> SaveUpdateExistingMasterPassword EnsureUserIsHydrated(user); updateExistingExistingData.ValidateDataForUser(user); - // Note: Keep "unlock then authenticate" pattern in mind. - // This is a purposeful inversion of that principle: - // Authentication is derivative of unlock, and is the mechanism for validation; - // eager validation keeps the logic easier to reason about, so it is performed first. + // Note: Keep "unlock and authenticate" pattern in mind. + // We name "unlock" first as a naming convention, + // e.g., MatherPasswordUnlockAndAuthenticationDataModel. + // The two are complimentary, but mechanically Authenticate comes first. + // Eager validation keeps the logic easier to reason about. + // Authentication is the mechanism for validation, unlock is the capability. var result = await UpdateExistingPasswordHashAsync( user, updateExistingExistingData.MasterPasswordAuthentication.MasterPasswordAuthenticationHash, From bf82f2ed9e773372429e3faabddc2786b7e49d86 Mon Sep 17 00:00:00 2001 From: enmande <3836813+enmande@users.noreply.github.com> Date: Thu, 23 Apr 2026 13:43:51 -0400 Subject: [PATCH 13/26] refactor(mp-service): Rename for saveExistingData (too much existing). --- .../Interfaces/IMasterPasswordService.cs | 4 ++-- .../UserMasterPassword/MasterPasswordService.cs | 16 ++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs index f7fd22fb7df8..6aed0244363f 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs @@ -178,7 +178,7 @@ public interface IMasterPasswordService /// must not be a Key Connector user. Salt must be unchanged. Validated via /// . /// - /// + /// /// Cryptographic and authentication data for the updated password and KDF parameters, /// including MasterPasswordUnlock, MasterPasswordAuthentication, /// and control flags ValidatePassword and RefreshStamp. @@ -187,7 +187,7 @@ public interface IMasterPasswordService /// On success, the modified . On failure, an array of /// describing validation failures. /// - Task> SaveUpdateExistingMasterPasswordAndKdfAsync(User user, UpdateExistingPasswordAndKdfData updateExistingExistingData); + Task> SaveUpdateExistingMasterPasswordAndKdfAsync(User user, UpdateExistingPasswordAndKdfData updateExistingData); /// /// Applies a new master password over the user's existing one and persists the updated user diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs index 2934994d75ab..b7875e2b1355 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs @@ -183,10 +183,10 @@ public async Task> PrepareUpdateExistingMasterPassw public async Task> SaveUpdateExistingMasterPasswordAndKdfAsync( User user, - UpdateExistingPasswordAndKdfData updateExistingExistingData) + UpdateExistingPasswordAndKdfData updateExistingData) { EnsureUserIsHydrated(user); - updateExistingExistingData.ValidateDataForUser(user); + updateExistingData.ValidateDataForUser(user); // Note: Keep "unlock and authenticate" pattern in mind. // We name "unlock" first as a naming convention, @@ -196,20 +196,20 @@ public async Task> SaveUpdateExistingMasterPassword // Authentication is the mechanism for validation, unlock is the capability. var result = await UpdateExistingPasswordHashAsync( user, - updateExistingExistingData.MasterPasswordAuthentication.MasterPasswordAuthenticationHash, - updateExistingExistingData.ValidatePassword, - updateExistingExistingData.RefreshStamp); + updateExistingData.MasterPasswordAuthentication.MasterPasswordAuthenticationHash, + updateExistingData.ValidatePassword, + updateExistingData.RefreshStamp); if (!result.Succeeded) { return result.Errors.ToArray(); } - user.Key = updateExistingExistingData.MasterPasswordUnlock.MasterKeyWrappedUserKey; - SetKdfStateOnUser(user, updateExistingExistingData.MasterPasswordUnlock.Kdf); + user.Key = updateExistingData.MasterPasswordUnlock.MasterKeyWrappedUserKey; + SetKdfStateOnUser(user, updateExistingData.MasterPasswordUnlock.Kdf); // Always override the master password hint, even if it's null - user.MasterPasswordHint = updateExistingExistingData.MasterPasswordHint; + user.MasterPasswordHint = updateExistingData.MasterPasswordHint; // Update time markers on the user var now = _timeProvider.GetUtcNow().UtcDateTime; From d9b98b336286dfe67bf4fc3108fe6a0f6ca3ca52 Mon Sep 17 00:00:00 2001 From: enmande <3836813+enmande@users.noreply.github.com> Date: Thu, 23 Apr 2026 14:53:27 -0400 Subject: [PATCH 14/26] docs(mp-service): Restore PM-34905 userrepository TODOs. --- src/Infrastructure.Dapper/Repositories/UserRepository.cs | 1 + .../Repositories/UserRepository.cs | 1 + 2 files changed, 2 insertions(+) diff --git a/src/Infrastructure.Dapper/Repositories/UserRepository.cs b/src/Infrastructure.Dapper/Repositories/UserRepository.cs index 364d7108a716..9ecfbe1da329 100644 --- a/src/Infrastructure.Dapper/Repositories/UserRepository.cs +++ b/src/Infrastructure.Dapper/Repositories/UserRepository.cs @@ -491,6 +491,7 @@ public UpdateUserData SetMasterPassword(Guid userId, MasterPasswordUnlockData ma // TODO (PM-35501): Add SecurityStamp so the rotation done in // MasterPasswordService.BuildUpdateUserDelegateSetInitialMasterPassword // is persisted. + // TODO Need to add User.LastPasswordChangeDate here in PM-34905 }, transaction: transaction, commandType: CommandType.StoredProcedure); diff --git a/src/Infrastructure.EntityFramework/Repositories/UserRepository.cs b/src/Infrastructure.EntityFramework/Repositories/UserRepository.cs index 542a29aeb869..a184b8a29b99 100644 --- a/src/Infrastructure.EntityFramework/Repositories/UserRepository.cs +++ b/src/Infrastructure.EntityFramework/Repositories/UserRepository.cs @@ -566,6 +566,7 @@ public UpdateUserData SetMasterPassword(Guid userId, MasterPasswordUnlockData ma // TODO (PM-35501): Persist SecurityStamp so the rotation done in // MasterPasswordService.BuildUpdateUserDelegateSetInitialMasterPassword // is persisted. + // userEntity.LastPasswordChangeDate = timestamp; This needs adding in PM-34905 await dbContext.SaveChangesAsync(); }; } From 3d22e2991bae44ecb17a9f92ed76674e9550cae8 Mon Sep 17 00:00:00 2001 From: enmande <3836813+enmande@users.noreply.github.com> Date: Fri, 24 Apr 2026 08:50:18 -0400 Subject: [PATCH 15/26] refactor(mp-service): Apply test naming clarification. --- .../MasterPasswordServiceTests.cs | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/MasterPasswordServiceTests.cs b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/MasterPasswordServiceTests.cs index 659c91dd1e93..22e2b3625a41 100644 --- a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/MasterPasswordServiceTests.cs +++ b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/MasterPasswordServiceTests.cs @@ -66,7 +66,7 @@ private static SetInitialPasswordData BuildSetInitialData(User user, string? hin }; } - private static UpdateExistingPasswordData BuildUpdateExistingData(User user, string? hint = null, + private static UpdateExistingPasswordData BuildUpdateExistingPasswordData(User user, string? hint = null, bool validatePassword = false) { var (kdf, salt) = GetMatchingKdfAndSalt(user); @@ -90,7 +90,7 @@ private static UpdateExistingPasswordData BuildUpdateExistingData(User user, str }; } - private static UpdateExistingPasswordAndKdfData BuildUpdateExistingAndKdfData(User user, + private static UpdateExistingPasswordAndKdfData BuildUpdateExistingPasswordAndKdfData(User user, KdfSettings? newKdf = null, string? hint = null, bool validatePassword = false) { var salt = user.GetMasterPasswordSalt(); @@ -258,7 +258,7 @@ public async Task PrepareSetInitialMasterPassword_RefreshStampFalse_PreservesSec } [Theory, BitAutoData] - public async Task PrepareSetInitialMasterPassword_ValidationFailure_ReturnsErrorsAsT1(User user) + public async Task PrepareSetInitialMasterPassword_ValidationFailure_ReturnsErrorsAsArrayOfIdentityError(User user) { var error = new IdentityError { Code = "test", Description = "test error" }; var validator = Substitute.For>(); @@ -354,7 +354,7 @@ public async Task PrepareUpdateExistingMasterPassword_Success(User user) user.MasterPassword = "existing-hash"; user.UsesKeyConnector = false; - var data = BuildUpdateExistingData(user); + var data = BuildUpdateExistingPasswordData(user); var expectedHash = "new-server-hash"; sutProvider.GetDependency>() .HashPassword(user, data.MasterPasswordAuthentication.MasterPasswordAuthenticationHash) @@ -380,7 +380,7 @@ public async Task PrepareUpdateExistingMasterPassword_SetsMasterPasswordHint(Use user.MasterPassword = "existing-hash"; user.UsesKeyConnector = false; - var data = BuildUpdateExistingData(user, hint: hint); + var data = BuildUpdateExistingPasswordData(user, hint: hint); sutProvider.GetDependency>() .HashPassword(Arg.Any(), Arg.Any()) .Returns("hash"); @@ -398,7 +398,7 @@ public async Task PrepareUpdateExistingMasterPassword_ThrowsWhenUserNotHydrated( user.Id = default; user.MasterPassword = "existing-hash"; - var data = BuildUpdateExistingData(user); + var data = BuildUpdateExistingPasswordData(user); await Assert.ThrowsAsync( () => sutProvider.Sut.PrepareUpdateExistingMasterPasswordAsync(user, data)); @@ -422,7 +422,7 @@ public async Task PrepareUpdateExistingMasterPassword_ValidationFailure_ReturnsE user.MasterPassword = "existing-hash"; user.UsesKeyConnector = false; - var data = BuildUpdateExistingData(user, validatePassword: true); + var data = BuildUpdateExistingPasswordData(user, validatePassword: true); var result = await sutProvider.Sut.PrepareUpdateExistingMasterPasswordAsync(user, data); @@ -439,7 +439,7 @@ public async Task SaveUpdateExistingMasterPassword_PreparesAndPersists(User user user.MasterPassword = "existing-hash"; user.UsesKeyConnector = false; - var data = BuildUpdateExistingData(user); + var data = BuildUpdateExistingPasswordData(user); sutProvider.GetDependency>() .HashPassword(Arg.Any(), Arg.Any()) .Returns("new-hash"); @@ -467,7 +467,7 @@ public async Task SaveUpdateExistingMasterPassword_WhenValidationFails_ReturnsEr user.MasterPassword = "existing-hash"; user.UsesKeyConnector = false; - var data = BuildUpdateExistingData(user, validatePassword: true); + var data = BuildUpdateExistingPasswordData(user, validatePassword: true); var result = await sutProvider.Sut.SaveUpdateExistingMasterPasswordAsync(user, data); @@ -675,7 +675,7 @@ public async Task SaveUpdateExistingMasterPasswordAndKdf_Success(User user) user.MasterPassword = "existing-hash"; user.UsesKeyConnector = false; - var data = BuildUpdateExistingAndKdfData(user); + var data = BuildUpdateExistingPasswordAndKdfData(user); sutProvider.GetDependency>() .HashPassword(Arg.Any(), Arg.Any()) .Returns("new-hash"); @@ -709,7 +709,7 @@ public async Task SaveUpdateExistingMasterPasswordAndKdf_RotatesPbkdf2ToArgon2id Memory = 64, Parallelism = 4 }; - var data = BuildUpdateExistingAndKdfData(user, newKdf: newKdf); + var data = BuildUpdateExistingPasswordAndKdfData(user, newKdf: newKdf); sutProvider.GetDependency>() .HashPassword(Arg.Any(), Arg.Any()) .Returns("new-hash"); @@ -741,7 +741,7 @@ public async Task SaveUpdateExistingMasterPasswordAndKdf_RotatesArgon2idToPbkdf2 Memory = null, Parallelism = null }; - var data = BuildUpdateExistingAndKdfData(user, newKdf: newKdf); + var data = BuildUpdateExistingPasswordAndKdfData(user, newKdf: newKdf); sutProvider.GetDependency>() .HashPassword(Arg.Any(), Arg.Any()) .Returns("new-hash"); @@ -771,7 +771,7 @@ public async Task SaveUpdateExistingMasterPasswordAndKdf_WhenValidationFails_Ret user.MasterPassword = "existing-hash"; user.UsesKeyConnector = false; - var data = BuildUpdateExistingAndKdfData(user, validatePassword: true); + var data = BuildUpdateExistingPasswordAndKdfData(user, validatePassword: true); var result = await sutProvider.Sut.SaveUpdateExistingMasterPasswordAndKdfAsync(user, data); @@ -823,7 +823,7 @@ public async Task SaveUpdateExistingMasterPasswordAndKdf_ThrowsWhenNoExistingPas var sutProvider = CreateSutProvider(); user.MasterPassword = null; - var data = BuildUpdateExistingAndKdfData(user); + var data = BuildUpdateExistingPasswordAndKdfData(user); await Assert.ThrowsAsync( () => sutProvider.Sut.SaveUpdateExistingMasterPasswordAndKdfAsync(user, data)); From 343ed33261684a21dc81b68b353e0a2510944364 Mon Sep 17 00:00:00 2001 From: enmande <3836813+enmande@users.noreply.github.com> Date: Fri, 24 Apr 2026 08:59:58 -0400 Subject: [PATCH 16/26] refactor(mp-service): Make service internal to Core. --- .../UserMasterPassword/Interfaces/IMasterPasswordService.cs | 2 +- .../UserFeatures/UserMasterPassword/MasterPasswordService.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs index 6aed0244363f..dbc9867d6783 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs @@ -60,7 +60,7 @@ namespace Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; /// the server is the source of truth for fields that must not change — it validates the client's /// values match what's stored before applying the update. /// -public interface IMasterPasswordService +internal interface IMasterPasswordService { /// /// Inspects the user's current state and dispatches to either diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs index b7875e2b1355..94bc738a7f7e 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs @@ -10,7 +10,7 @@ namespace Bit.Core.Auth.UserFeatures.UserMasterPassword; -public class MasterPasswordService( +internal class MasterPasswordService( IUserRepository userRepository, TimeProvider timeProvider, IPasswordHasher passwordHasher, From b79907270b623b62711387ce0aaaee5eeb7321d9 Mon Sep 17 00:00:00 2001 From: enmande <3836813+enmande@users.noreply.github.com> Date: Fri, 24 Apr 2026 14:18:37 -0400 Subject: [PATCH 17/26] docs(mp-service): Update method comment formats: what, use when, constraints. --- .../Interfaces/IMasterPasswordService.cs | 131 +++++++++++++++--- 1 file changed, 115 insertions(+), 16 deletions(-) diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs index dbc9867d6783..fe3d9fcd3841 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs @@ -67,6 +67,19 @@ internal interface IMasterPasswordService /// or /// accordingly. /// Prepares the object in memory only. + /// + /// + /// Use when: composing a set-initial or update-existing mutation with other operations + /// before saving, and the caller does not need to select the path explicitly — dispatch + /// is determined by the user's current master password state. + /// + /// + /// + /// Constraints: Delegated to the dispatched method; see + /// or + /// . + /// + /// /// /// /// The user object to mutate. Whether the user already has a master password determines @@ -85,8 +98,21 @@ internal interface IMasterPasswordService Task> PrepareSetInitialOrUpdateExistingMasterPasswordAsync(User user, SetInitialOrUpdateExistingPasswordData setOrUpdatePasswordData); /// - /// Applies a new initial master password to the object in memory only. - /// Use for flows that must compose this mutation with other operations inside a larger transaction. + /// Applies a new initial master password to the object in memory only. + /// + /// + /// Use when: composing this mutation with other operations inside a larger transaction. + /// + /// + /// + /// Constraints: + /// + /// User must not already have a master password. + /// User must have no existing Key or MasterPasswordSalt. + /// User must not be a Key Connector user. + /// + /// + /// /// /// /// The user object to mutate. Must not already have a master password; must have no existing @@ -106,14 +132,28 @@ internal interface IMasterPasswordService Task> PrepareSetInitialMasterPasswordAsync(User user, SetInitialPasswordData setInitialPasswordData); /// - /// Note: This is to be used in the future when a TDE user wants to set a password with self-service. - /// /// Applies a new initial master password to the object and persists - /// the updated user. Use when no external transaction coordination is needed. + /// the updated user. + /// + /// + /// Use when: a TDE user wants to set a password via self-service and no external transaction + /// coordination is needed. Note: intended for future use. + /// + /// + /// + /// Constraints (see also ): + /// + /// User must not already have a master password. + /// User must have no existing Key or MasterPasswordSalt. + /// User must not be a Key Connector user. + /// + /// + /// /// /// - /// The user object to mutate and persist. Subject to the same preconditions as - /// . + /// The user object to mutate and persist. Must not already have a master password; must have no + /// existing Key or MasterPasswordSalt; must not be a Key Connector user. + /// Validated via . /// /// /// Cryptographic and authentication data required to set the initial password. See @@ -129,12 +169,29 @@ internal interface IMasterPasswordService /// Returns a deferred database write (as an delegate) for setting /// the initial master password. The delegate is intended to be passed to /// , which executes all supplied delegates - /// within a single SQL transaction. Composing this delegate with others (e.g. cryptographic key - /// writes) ensures every write succeeds or the entire batch rolls back atomically, a guarantee + /// within a single SQL transaction. + /// + /// + /// Use when: the password set is part of a larger transactional write that must succeed or fail + /// atomically (e.g., TDE key + password in a single SQL transaction). Composing this delegate + /// with others ensures every write succeeds or the entire batch rolls back, a guarantee /// cannot provide on its own. + /// + /// + /// + /// Constraints (applied when the returned delegate is invoked; see also ): + /// + /// User must not already have a master password. + /// User must have no existing Key or MasterPasswordSalt. + /// User must not be a Key Connector user. + /// + /// + /// /// /// - /// The user whose initial master password state will be written when the returned delegate is invoked. + /// The user whose initial master password state will be written when the returned delegate is + /// invoked. Must not already have a master password; must have no existing Key or + /// MasterPasswordSalt; must not be a Key Connector user. /// /// /// Cryptographic and authentication data required to set the initial password. See @@ -149,7 +206,20 @@ internal interface IMasterPasswordService /// /// Applies a new master password over the user's existing one, mutating the /// object in memory only. - /// Use for flows that must compose this mutation with other operations inside a larger transaction. + /// + /// + /// Use when: composing this mutation with other operations inside a larger transaction. + /// + /// + /// + /// Constraints: + /// + /// User must already have a master password. + /// User must not be a Key Connector user. + /// KDF parameters and salt must be unchanged relative to the values in . + /// + /// + /// /// /// /// The user object to mutate. Must already have a master password; @@ -170,8 +240,21 @@ internal interface IMasterPasswordService /// /// Applies a new master password and updated KDF parameters over the user's existing ones - /// and persists the updated user to the database. Salt must remain unchanged; KDF - /// validation is intentionally skipped. Use for KDF rotation flows. + /// and persists the updated user to the database. KDF validation is intentionally skipped. + /// + /// + /// Use when: rotating KDF parameters. + /// + /// + /// + /// Constraints: + /// + /// User must already have a master password. + /// User must not be a Key Connector user. + /// Salt must be unchanged. + /// + /// + /// /// /// /// The user object to mutate and persist. Must already have a master password; @@ -191,11 +274,27 @@ internal interface IMasterPasswordService /// /// Applies a new master password over the user's existing one and persists the updated user - /// to the database. Use when no external transaction coordination is needed. + /// to the database. + /// + /// + /// Use when: no external transaction coordination is needed. + /// + /// + /// + /// Constraints (see also ): + /// + /// User must already have a master password. + /// User must not be a Key Connector user. + /// KDF parameters and salt must be unchanged relative to the values in . + /// + /// + /// /// /// - /// The user object to mutate and persist. Subject to the same preconditions as - /// . + /// The user object to mutate and persist. Must already have a master password; + /// must not be a Key Connector user. KDF parameters and salt must be unchanged relative to the + /// values in . Validated via + /// . /// /// /// Cryptographic and authentication data for the updated password. See From e1314dfe4da226c652715af8720ba445755b8602 Mon Sep 17 00:00:00 2001 From: enmande <3836813+enmande@users.noreply.github.com> Date: Mon, 27 Apr 2026 08:03:20 -0400 Subject: [PATCH 18/26] docs(mp-service): Update interface docs for consistency. --- .../Interfaces/IMasterPasswordService.cs | 53 +++++++++++-------- 1 file changed, 32 insertions(+), 21 deletions(-) diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs index fe3d9fcd3841..313dec9f20ef 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs @@ -7,56 +7,67 @@ namespace Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; /// -/// Centralized mutation point for all master password set and update operations. +/// Shared service for all master password set and update operations. /// Provides consistent validation, password hashing, and timestamp management across every /// flow that establishes or updates a user's master password. /// -/// Compositional, not orchestrating. This service handles CRUD-like mutations +/// Compositional, not orchestrating. This service handles CRUD-like mutations and/or persistence /// only. Business logic (e.g., authorization checks, org validation, push notifications, event /// logging) remains a caller responsibility. /// -/// Three persistence tiers: +/// +/// What our verbs do: +/// This surface has a 3-verb API. +/// Choose between Prepare, Save, or Build; each has a consistent mutation/persistence +/// opinion. +/// /// /// -/// Prepare* Modifies the object in memory only. The caller -/// controls when and how persistence occurs. Use when composing additional mutations before -/// saving (e.g. admin recovery flows that also clear 2FA or set ForcePasswordReset). -/// Returns OneOf<User, IdentityError[]>. +/// Prepare Modifies the object in memory only. The caller +/// controls when and how persistence occurs. Use when composing mutations before +/// saving (e.g. admin recovery flows that also clear 2FA or set ). +/// Returns of , array of . /// /// -/// Save* Prepares the mutation and persists to the database via -/// IUserRepository.ReplaceAsync. Use for standalone operations where no -/// further mutation is needed. Returns OneOf<User, IdentityError[]>. +/// Save Prepares the mutation and persists to the database via +/// . Use for standalone operations where no +/// further mutation is needed. +/// Returns of , array of . /// /// -/// Build* Returns a deferred delegate for +/// Build Returns a deferred delegate for /// batch transactions. Use when the /// password set is part of a larger transactional write that must succeed or fail atomically /// (e.g. TDE key + password in a single SQL transaction) /// /// /// -/// Set vs Update contract: -/// +/// +/// When to choose an operation: +/// Choose between Set (initial/new) and Update (existing); each has purpose-made validation +/// and consistency constraints. +/// +/// /// -/// SET (initial) +/// Set /// Client sends all data (hash, salt, KDF). Server sets all /// fields. Stage 1 caveat: server enforces salt == email.ToLowerInvariant().Trim() -/// (PM-28143 removes this in Stage 3). +/// (PM-28143 removes this in Stage 3). Use when creating a new user. /// /// -/// UPDATE (hash only) +/// Update (hash only) /// Client sends all data. Server validates KDF and salt -/// are unchanged, updates only the hash and wrapped user key. +/// are unchanged, updates only the hash and wrapped user key. Use when +/// updating master password. /// /// -/// UPDATE (KDF) -/// Client sends all data. Server validates salt is unchanged, -/// updates hash, KDF, and wrapped user key. +/// Update (KDF) +/// Client sends all data. Server validates salt is unchanged. +/// Use when updating KDF settings and master key-wrapped user key. /// /// /// -/// Source of truth: On SET, the client is the source of truth. On UPDATE, +/// Source of truth: On Set, the client is the source of truth. On Update, /// the server is the source of truth for fields that must not change — it validates the client's /// values match what's stored before applying the update. /// From 8ec3bfc92b85cc6549ff20ca9da0230561399424 Mon Sep 17 00:00:00 2001 From: enmande <3836813+enmande@users.noreply.github.com> Date: Mon, 27 Apr 2026 08:51:09 -0400 Subject: [PATCH 19/26] refactor(mp-service): Rename internal helpers to Apply, add documentation. --- .../MasterPasswordService.cs | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs index 94bc738a7f7e..049b2c8c2ec7 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs @@ -57,7 +57,7 @@ public async Task> PrepareSetInitialMasterPasswordA // The two are complimentary, but mechanically Authenticate comes first. // Eager validation keeps the logic easier to reason about. // Authentication is the mechanism for validation, unlock is the capability. - var result = await UpdateExistingPasswordHashAsync( + var result = await ApplyPasswordHashAsync( user, setInitialData.MasterPasswordAuthentication.MasterPasswordAuthenticationHash, setInitialData.ValidatePassword, @@ -68,7 +68,7 @@ public async Task> PrepareSetInitialMasterPasswordA } user.Key = setInitialData.MasterPasswordUnlock.MasterKeyWrappedUserKey; - SetKdfStateOnUser(user, setInitialData.MasterPasswordUnlock.Kdf); + ApplyKdfStateOnUser(user, setInitialData.MasterPasswordUnlock.Kdf); // Set salt on the user user.MasterPasswordSalt = setInitialData.MasterPasswordUnlock.Salt; @@ -157,7 +157,7 @@ public async Task> PrepareUpdateExistingMasterPassw // The two are complimentary, but mechanically Authenticate comes first. // Eager validation keeps the logic easier to reason about. // Authentication is the mechanism for validation, unlock is the capability. - var result = await UpdateExistingPasswordHashAsync( + var result = await ApplyPasswordHashAsync( user, updateExistingData.MasterPasswordAuthentication.MasterPasswordAuthenticationHash, updateExistingData.ValidatePassword, @@ -194,7 +194,7 @@ public async Task> SaveUpdateExistingMasterPassword // The two are complimentary, but mechanically Authenticate comes first. // Eager validation keeps the logic easier to reason about. // Authentication is the mechanism for validation, unlock is the capability. - var result = await UpdateExistingPasswordHashAsync( + var result = await ApplyPasswordHashAsync( user, updateExistingData.MasterPasswordAuthentication.MasterPasswordAuthenticationHash, updateExistingData.ValidatePassword, @@ -206,7 +206,7 @@ public async Task> SaveUpdateExistingMasterPassword } user.Key = updateExistingData.MasterPasswordUnlock.MasterKeyWrappedUserKey; - SetKdfStateOnUser(user, updateExistingData.MasterPasswordUnlock.Kdf); + ApplyKdfStateOnUser(user, updateExistingData.MasterPasswordUnlock.Kdf); // Always override the master password hint, even if it's null user.MasterPasswordHint = updateExistingData.MasterPasswordHint; @@ -238,7 +238,13 @@ public async Task> SaveUpdateExistingMasterPassword return result.AsT0; } - private async Task UpdateExistingPasswordHashAsync(User user, string newPassword, + /// + ///Applies via hashing the to the . + ///Optionally validates the password, flagged with + ///and rotates the security stamp, flagged with . + ///Used by both initial-set and update master password paths. + /// + private async Task ApplyPasswordHashAsync(User user, string newPassword, bool validatePassword = true, bool refreshStamp = true) { if (validatePassword) @@ -263,7 +269,7 @@ private async Task UpdateExistingPasswordHashAsync(User user, st /// Applies KDF parameters from the supplied to the . /// Used by both initial-set and KDF-rotation paths. /// - private static void SetKdfStateOnUser(User user, KdfSettings kdf) + private static void ApplyKdfStateOnUser(User user, KdfSettings kdf) { user.Kdf = kdf.KdfType; user.KdfIterations = kdf.Iterations; From b18c6ac7babae3bdf0a7d6736b6dd1d902ab353a Mon Sep 17 00:00:00 2001 From: enmande <3836813+enmande@users.noreply.github.com> Date: Mon, 27 Apr 2026 09:10:29 -0400 Subject: [PATCH 20/26] docs(mp-service): Add summary and use-when annotations to data models. --- .../Data/SetInitialOrUpdateExistingPasswordData.cs | 11 +++++++++++ .../Data/SetInitialPasswordData.cs | 12 ++++++++++++ .../Data/UpdateExistingPasswordAndKdfData.cs | 10 ++++++++++ .../Data/UpdateExistingPasswordData.cs | 12 ++++++++++++ 4 files changed, 45 insertions(+) diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/Data/SetInitialOrUpdateExistingPasswordData.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/Data/SetInitialOrUpdateExistingPasswordData.cs index b094363e98f2..c3fd90a7c802 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/Data/SetInitialOrUpdateExistingPasswordData.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/Data/SetInitialOrUpdateExistingPasswordData.cs @@ -2,6 +2,17 @@ namespace Bit.Core.Auth.UserFeatures.UserMasterPassword.Data; +/// +/// Combined input data covering both the set-initial and update-existing paths. Converts to +/// or via +/// or . +/// +/// +/// Use when: constructing a call to +/// , +/// where the caller does not need to select the set-initial or update-existing path explicitly. +/// +/// public class SetInitialOrUpdateExistingPasswordData { public required MasterPasswordUnlockData MasterPasswordUnlock { get; set; } diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/Data/SetInitialPasswordData.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/Data/SetInitialPasswordData.cs index 5fe68217c5da..814dde606fed 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/Data/SetInitialPasswordData.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/Data/SetInitialPasswordData.cs @@ -4,6 +4,18 @@ namespace Bit.Core.Auth.UserFeatures.UserMasterPassword.Data; +/// +/// Input data for setting an initial master password on a user who has none. Carries the +/// cryptographic material, authentication credential, and control flags consumed by +/// . +/// +/// +/// Use when: constructing a call to +/// , +/// , or +/// . +/// +/// public class SetInitialPasswordData { public required MasterPasswordUnlockData MasterPasswordUnlock { get; set; } diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordAndKdfData.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordAndKdfData.cs index 2c9c73077ebb..ed9812a890cc 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordAndKdfData.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordAndKdfData.cs @@ -4,6 +4,16 @@ namespace Bit.Core.Auth.UserFeatures.UserMasterPassword.Data; +/// +/// Input data for updating a user's existing master password hash together with new KDF parameters. +/// Salt is validated as unchanged; KDF validation is intentionally skipped since KDF is being replaced. +/// +/// +/// Use when: constructing a call to +/// +/// (KDF rotation flows). For hash-only updates, use instead. +/// +/// public class UpdateExistingPasswordAndKdfData { public required MasterPasswordUnlockData MasterPasswordUnlock { get; set; } diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordData.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordData.cs index 2c7ef218897e..0f3d9c3934c5 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordData.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordData.cs @@ -4,6 +4,18 @@ namespace Bit.Core.Auth.UserFeatures.UserMasterPassword.Data; +/// +/// Input data for updating a user's existing master password hash without changing KDF parameters. +/// Carries the cryptographic material, authentication credential, and control flags consumed by +/// . KDF and salt are validated as unchanged. +/// +/// +/// Use when: constructing a call to +/// or +/// . +/// For KDF rotation, use instead. +/// +/// public class UpdateExistingPasswordData { public required MasterPasswordUnlockData MasterPasswordUnlock { get; set; } From 1915ca3936d8eefec5eb428be87bb999e5c27017 Mon Sep 17 00:00:00 2001 From: enmande <3836813+enmande@users.noreply.github.com> Date: Mon, 27 Apr 2026 09:15:51 -0400 Subject: [PATCH 21/26] docs(mp-service): Add annotation preferring non-Build API verbs where possible. --- .../UserMasterPassword/Interfaces/IMasterPasswordService.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs index 313dec9f20ef..785793559443 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs @@ -198,6 +198,11 @@ internal interface IMasterPasswordService /// /// /// + /// + /// NOTE: This exists to support an existing pattern. Long-term preference is to be able to sunset this method + /// and lean on Prepare and Save verbs only. Please prefer those verbs if possible. + /// + /// /// /// /// The user whose initial master password state will be written when the returned delegate is From 9f42c6c232c5cb8b4b912ebad85292b12c89303e Mon Sep 17 00:00:00 2001 From: enmande <3836813+enmande@users.noreply.github.com> Date: Mon, 27 Apr 2026 09:25:36 -0400 Subject: [PATCH 22/26] test(mp-service): Refactor data model tests into discrete files. --- .../Data/SetInitialPasswordDataTests.cs | 178 +++++++++ .../UpdateExistingPasswordAndKdfDataTests.cs | 92 +++++ .../Data/UpdateExistingPasswordDataTests.cs | 110 ++++++ .../MasterPasswordServiceTests.cs | 362 ------------------ 4 files changed, 380 insertions(+), 362 deletions(-) create mode 100644 test/Core.Test/Auth/UserFeatures/UserMasterPassword/Data/SetInitialPasswordDataTests.cs create mode 100644 test/Core.Test/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordAndKdfDataTests.cs create mode 100644 test/Core.Test/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordDataTests.cs diff --git a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/Data/SetInitialPasswordDataTests.cs b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/Data/SetInitialPasswordDataTests.cs new file mode 100644 index 000000000000..dd5952514d14 --- /dev/null +++ b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/Data/SetInitialPasswordDataTests.cs @@ -0,0 +1,178 @@ +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Data; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Exceptions; +using Bit.Core.KeyManagement.Models.Data; +using Xunit; + +namespace Bit.Core.Test.Auth.UserFeatures.UserMasterPassword.Data; + +public class SetInitialPasswordDataTests +{ + private static User BuildValidSetInitialUser() + { + var user = new User + { + Id = Guid.NewGuid(), + Email = "test@example.com", + MasterPassword = null, + Key = null, + MasterPasswordSalt = null, + UsesKeyConnector = false, + Kdf = KdfType.PBKDF2_SHA256, + KdfIterations = 600000 + }; + return user; + } + + private static SetInitialPasswordData BuildData(User user, string? saltOverride = null) + { + // Stage 1: salt == email while MasterPasswordSalt is null (PM-28143 separates them in Stage 3). + var salt = saltOverride ?? user.GetMasterPasswordSalt(); + var kdf = new KdfSettings + { + KdfType = user.Kdf, + Iterations = user.KdfIterations, + Memory = user.KdfMemory, + Parallelism = user.KdfParallelism + }; + return new SetInitialPasswordData + { + MasterPasswordUnlock = new MasterPasswordUnlockData + { + Salt = salt, + MasterKeyWrappedUserKey = "wrapped-key", + Kdf = kdf + }, + MasterPasswordAuthentication = new MasterPasswordAuthenticationData + { + Salt = salt, + MasterPasswordAuthenticationHash = "hash", + Kdf = kdf + } + }; + } + + [Fact] + public void ValidateDataForUser_Accepts_WhenUserHasNoMasterPassword() + { + var user = BuildValidSetInitialUser(); + var data = BuildData(user); + + // Should not throw + data.ValidateDataForUser(user); + } + + [Fact] + public void ValidateDataForUser_Throws_WhenUserHasMasterPassword() + { + var user = BuildValidSetInitialUser(); + user.MasterPassword = "existing-hash"; + var data = BuildData(user); + + Assert.Throws(() => data.ValidateDataForUser(user)); + } + + [Fact] + public void ValidateDataForUser_Throws_WhenUserHasKey() + { + var user = BuildValidSetInitialUser(); + user.Key = "existing-key"; + var data = BuildData(user); + + Assert.Throws(() => data.ValidateDataForUser(user)); + } + + [Fact] + public void ValidateDataForUser_Throws_WhenUserHasSalt() + { + var user = BuildValidSetInitialUser(); + user.MasterPasswordSalt = "existing-salt"; + var data = BuildData(user, saltOverride: "existing-salt"); + + Assert.Throws(() => data.ValidateDataForUser(user)); + } + + [Fact] + public void ValidateDataForUser_Throws_WhenUserIsKeyConnector() + { + var user = BuildValidSetInitialUser(); + user.UsesKeyConnector = true; + var data = BuildData(user); + + Assert.Throws(() => data.ValidateDataForUser(user)); + } + + [Fact] + public void ValidateDataForUser_Throws_WhenSaltMismatch() + { + var user = BuildValidSetInitialUser(); + var data = BuildData(user, saltOverride: "wrong-salt"); + + Assert.Throws(() => data.ValidateDataForUser(user)); + } + + [Fact] + public void ValidateDataForUser_Throws_WhenAuthenticationSaltMismatch_UnlockSaltCorrect() + { + var user = BuildValidSetInitialUser(); + var correctSalt = user.GetMasterPasswordSalt(); + var kdf = new KdfSettings + { + KdfType = user.Kdf, + Iterations = user.KdfIterations, + Memory = user.KdfMemory, + Parallelism = user.KdfParallelism + }; + // Authentication salt is wrong; Unlock salt is correct. + var data = new SetInitialPasswordData + { + MasterPasswordUnlock = new MasterPasswordUnlockData + { + Salt = correctSalt, + MasterKeyWrappedUserKey = "wrapped-key", + Kdf = kdf + }, + MasterPasswordAuthentication = new MasterPasswordAuthenticationData + { + Salt = "wrong-auth-salt", + MasterPasswordAuthenticationHash = "hash", + Kdf = kdf + } + }; + + Assert.Throws(() => data.ValidateDataForUser(user)); + } + + [Fact] + public void ValidateDataForUser_Throws_WhenUnlockSaltMismatch_AuthenticationSaltCorrect() + { + var user = BuildValidSetInitialUser(); + var correctSalt = user.GetMasterPasswordSalt(); + var kdf = new KdfSettings + { + KdfType = user.Kdf, + Iterations = user.KdfIterations, + Memory = user.KdfMemory, + Parallelism = user.KdfParallelism + }; + // Unlock salt is wrong; Authentication salt is correct. + var data = new SetInitialPasswordData + { + MasterPasswordUnlock = new MasterPasswordUnlockData + { + Salt = "wrong-unlock-salt", + MasterKeyWrappedUserKey = "wrapped-key", + Kdf = kdf + }, + MasterPasswordAuthentication = new MasterPasswordAuthenticationData + { + Salt = correctSalt, + MasterPasswordAuthenticationHash = "hash", + Kdf = kdf + } + }; + + Assert.Throws(() => data.ValidateDataForUser(user)); + } +} diff --git a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordAndKdfDataTests.cs b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordAndKdfDataTests.cs new file mode 100644 index 000000000000..c737076d3aac --- /dev/null +++ b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordAndKdfDataTests.cs @@ -0,0 +1,92 @@ +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Data; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Exceptions; +using Bit.Core.KeyManagement.Models.Data; +using Xunit; + +namespace Bit.Core.Test.Auth.UserFeatures.UserMasterPassword.Data; + +public class UpdateExistingPasswordAndKdfDataTests +{ + private static User BuildValidUser() + { + return new User + { + Id = Guid.NewGuid(), + Email = "test@example.com", + MasterPassword = "existing-hash", + Key = "existing-key", + MasterPasswordSalt = "stored-salt", + UsesKeyConnector = false, + Kdf = KdfType.PBKDF2_SHA256, + KdfIterations = 600000 + }; + } + + private static UpdateExistingPasswordAndKdfData BuildData(User user, string? saltOverride = null) + { + var salt = saltOverride ?? user.GetMasterPasswordSalt(); + var newKdf = new KdfSettings + { + KdfType = KdfType.Argon2id, + Iterations = 3, + Memory = 64, + Parallelism = 4 + }; + return new UpdateExistingPasswordAndKdfData + { + MasterPasswordUnlock = new MasterPasswordUnlockData + { + Salt = salt, + MasterKeyWrappedUserKey = "wrapped-key", + Kdf = newKdf + }, + MasterPasswordAuthentication = new MasterPasswordAuthenticationData + { + Salt = salt, + MasterPasswordAuthenticationHash = "hash", + Kdf = newKdf + } + }; + } + + [Fact] + public void ValidateDataForUser_Accepts_WhenUserHasMasterPassword_SaltMatch_KdfChanged() + { + var user = BuildValidUser(); + var data = BuildData(user); + + // Should not throw — KDF change is permitted here + data.ValidateDataForUser(user); + } + + [Fact] + public void ValidateDataForUser_Throws_WhenUserHasNoMasterPassword() + { + var user = BuildValidUser(); + user.MasterPassword = null; + var data = BuildData(user); + + Assert.Throws(() => data.ValidateDataForUser(user)); + } + + [Fact] + public void ValidateDataForUser_Throws_WhenUserIsKeyConnector() + { + var user = BuildValidUser(); + user.UsesKeyConnector = true; + var data = BuildData(user); + + Assert.Throws(() => data.ValidateDataForUser(user)); + } + + [Fact] + public void ValidateDataForUser_Throws_WhenSaltChanged() + { + var user = BuildValidUser(); + var data = BuildData(user, saltOverride: "wrong-salt"); + + Assert.Throws(() => data.ValidateDataForUser(user)); + } +} diff --git a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordDataTests.cs b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordDataTests.cs new file mode 100644 index 000000000000..f478ce683b15 --- /dev/null +++ b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordDataTests.cs @@ -0,0 +1,110 @@ +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Data; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Exceptions; +using Bit.Core.KeyManagement.Models.Data; +using Xunit; + +namespace Bit.Core.Test.Auth.UserFeatures.UserMasterPassword.Data; + +public class UpdateExistingPasswordDataTests +{ + private static User BuildValidUpdateUser() + { + var user = new User + { + Id = Guid.NewGuid(), + Email = "test@example.com", + MasterPassword = "existing-hash", + Key = "existing-key", + MasterPasswordSalt = "stored-salt", + UsesKeyConnector = false, + Kdf = KdfType.PBKDF2_SHA256, + KdfIterations = 600000 + }; + return user; + } + + private static UpdateExistingPasswordData BuildData(User user, string? saltOverride = null, + KdfSettings? kdfOverride = null) + { + var salt = saltOverride ?? user.GetMasterPasswordSalt(); + var kdf = kdfOverride ?? new KdfSettings + { + KdfType = user.Kdf, + Iterations = user.KdfIterations, + Memory = user.KdfMemory, + Parallelism = user.KdfParallelism + }; + return new UpdateExistingPasswordData + { + MasterPasswordUnlock = new MasterPasswordUnlockData + { + Salt = salt, + MasterKeyWrappedUserKey = "wrapped-key", + Kdf = kdf + }, + MasterPasswordAuthentication = new MasterPasswordAuthenticationData + { + Salt = salt, + MasterPasswordAuthenticationHash = "hash", + Kdf = kdf + } + }; + } + + [Fact] + public void ValidateDataForUser_Accepts_WhenUserHasMasterPassword_KdfAndSaltMatch() + { + var user = BuildValidUpdateUser(); + var data = BuildData(user); + + // Should not throw + data.ValidateDataForUser(user); + } + + [Fact] + public void ValidateDataForUser_Throws_WhenUserHasNoMasterPassword() + { + var user = BuildValidUpdateUser(); + user.MasterPassword = null; + var data = BuildData(user); + + Assert.Throws(() => data.ValidateDataForUser(user)); + } + + [Fact] + public void ValidateDataForUser_Throws_WhenUserIsKeyConnector() + { + var user = BuildValidUpdateUser(); + user.UsesKeyConnector = true; + var data = BuildData(user); + + Assert.Throws(() => data.ValidateDataForUser(user)); + } + + [Fact] + public void ValidateDataForUser_Throws_WhenKdfChanged() + { + var user = BuildValidUpdateUser(); + var mismatchedKdf = new KdfSettings + { + KdfType = KdfType.Argon2id, + Iterations = 3, + Memory = 64, + Parallelism = 4 + }; + var data = BuildData(user, kdfOverride: mismatchedKdf); + + Assert.Throws(() => data.ValidateDataForUser(user)); + } + + [Fact] + public void ValidateDataForUser_Throws_WhenSaltChanged() + { + var user = BuildValidUpdateUser(); + var data = BuildData(user, saltOverride: "wrong-salt"); + + Assert.Throws(() => data.ValidateDataForUser(user)); + } +} diff --git a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/MasterPasswordServiceTests.cs b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/MasterPasswordServiceTests.cs index 22e2b3625a41..470cbeefcef6 100644 --- a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/MasterPasswordServiceTests.cs +++ b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/MasterPasswordServiceTests.cs @@ -1013,366 +1013,4 @@ private static SetInitialPasswordData BuildSetInitialDataForUser(User user) } } - // --- Data model validation: SetInitialPasswordData --- - - public class SetInitialPasswordDataTests - { - private static User BuildValidSetInitialUser() - { - var user = new User - { - Id = Guid.NewGuid(), - Email = "test@example.com", - MasterPassword = null, - Key = null, - MasterPasswordSalt = null, - UsesKeyConnector = false, - Kdf = KdfType.PBKDF2_SHA256, - KdfIterations = 600000 - }; - return user; - } - - private static SetInitialPasswordData BuildData(User user, string? saltOverride = null) - { - // Stage 1: salt == email while MasterPasswordSalt is null (PM-28143 separates them in Stage 3). - var salt = saltOverride ?? user.GetMasterPasswordSalt(); - var kdf = new KdfSettings - { - KdfType = user.Kdf, - Iterations = user.KdfIterations, - Memory = user.KdfMemory, - Parallelism = user.KdfParallelism - }; - return new SetInitialPasswordData - { - MasterPasswordUnlock = new MasterPasswordUnlockData - { - Salt = salt, - MasterKeyWrappedUserKey = "wrapped-key", - Kdf = kdf - }, - MasterPasswordAuthentication = new MasterPasswordAuthenticationData - { - Salt = salt, - MasterPasswordAuthenticationHash = "hash", - Kdf = kdf - } - }; - } - - [Fact] - public void ValidateDataForUser_Accepts_WhenUserHasNoMasterPassword() - { - var user = BuildValidSetInitialUser(); - var data = BuildData(user); - - // Should not throw - data.ValidateDataForUser(user); - } - - [Fact] - public void ValidateDataForUser_Throws_WhenUserHasMasterPassword() - { - var user = BuildValidSetInitialUser(); - user.MasterPassword = "existing-hash"; - var data = BuildData(user); - - Assert.Throws(() => data.ValidateDataForUser(user)); - } - - [Fact] - public void ValidateDataForUser_Throws_WhenUserHasKey() - { - var user = BuildValidSetInitialUser(); - user.Key = "existing-key"; - var data = BuildData(user); - - Assert.Throws(() => data.ValidateDataForUser(user)); - } - - [Fact] - public void ValidateDataForUser_Throws_WhenUserHasSalt() - { - var user = BuildValidSetInitialUser(); - user.MasterPasswordSalt = "existing-salt"; - var data = BuildData(user, saltOverride: "existing-salt"); - - Assert.Throws(() => data.ValidateDataForUser(user)); - } - - [Fact] - public void ValidateDataForUser_Throws_WhenUserIsKeyConnector() - { - var user = BuildValidSetInitialUser(); - user.UsesKeyConnector = true; - var data = BuildData(user); - - Assert.Throws(() => data.ValidateDataForUser(user)); - } - - [Fact] - public void ValidateDataForUser_Throws_WhenSaltMismatch() - { - var user = BuildValidSetInitialUser(); - var data = BuildData(user, saltOverride: "wrong-salt"); - - Assert.Throws(() => data.ValidateDataForUser(user)); - } - - [Fact] - public void ValidateDataForUser_Throws_WhenAuthenticationSaltMismatch_UnlockSaltCorrect() - { - var user = BuildValidSetInitialUser(); - var correctSalt = user.GetMasterPasswordSalt(); - var kdf = new KdfSettings - { - KdfType = user.Kdf, - Iterations = user.KdfIterations, - Memory = user.KdfMemory, - Parallelism = user.KdfParallelism - }; - // Authentication salt is wrong; Unlock salt is correct. - var data = new SetInitialPasswordData - { - MasterPasswordUnlock = new MasterPasswordUnlockData - { - Salt = correctSalt, - MasterKeyWrappedUserKey = "wrapped-key", - Kdf = kdf - }, - MasterPasswordAuthentication = new MasterPasswordAuthenticationData - { - Salt = "wrong-auth-salt", - MasterPasswordAuthenticationHash = "hash", - Kdf = kdf - } - }; - - Assert.Throws(() => data.ValidateDataForUser(user)); - } - - [Fact] - public void ValidateDataForUser_Throws_WhenUnlockSaltMismatch_AuthenticationSaltCorrect() - { - var user = BuildValidSetInitialUser(); - var correctSalt = user.GetMasterPasswordSalt(); - var kdf = new KdfSettings - { - KdfType = user.Kdf, - Iterations = user.KdfIterations, - Memory = user.KdfMemory, - Parallelism = user.KdfParallelism - }; - // Unlock salt is wrong; Authentication salt is correct. - var data = new SetInitialPasswordData - { - MasterPasswordUnlock = new MasterPasswordUnlockData - { - Salt = "wrong-unlock-salt", - MasterKeyWrappedUserKey = "wrapped-key", - Kdf = kdf - }, - MasterPasswordAuthentication = new MasterPasswordAuthenticationData - { - Salt = correctSalt, - MasterPasswordAuthenticationHash = "hash", - Kdf = kdf - } - }; - - Assert.Throws(() => data.ValidateDataForUser(user)); - } - } - - // --- Data model validation: UpdateExistingPasswordData --- - - public class UpdateExistingPasswordDataTests - { - private static User BuildValidUpdateUser() - { - var user = new User - { - Id = Guid.NewGuid(), - Email = "test@example.com", - MasterPassword = "existing-hash", - Key = "existing-key", - MasterPasswordSalt = "stored-salt", - UsesKeyConnector = false, - Kdf = KdfType.PBKDF2_SHA256, - KdfIterations = 600000 - }; - return user; - } - - private static UpdateExistingPasswordData BuildData(User user, string? saltOverride = null, - KdfSettings? kdfOverride = null) - { - var salt = saltOverride ?? user.GetMasterPasswordSalt(); - var kdf = kdfOverride ?? new KdfSettings - { - KdfType = user.Kdf, - Iterations = user.KdfIterations, - Memory = user.KdfMemory, - Parallelism = user.KdfParallelism - }; - return new UpdateExistingPasswordData - { - MasterPasswordUnlock = new MasterPasswordUnlockData - { - Salt = salt, - MasterKeyWrappedUserKey = "wrapped-key", - Kdf = kdf - }, - MasterPasswordAuthentication = new MasterPasswordAuthenticationData - { - Salt = salt, - MasterPasswordAuthenticationHash = "hash", - Kdf = kdf - } - }; - } - - [Fact] - public void ValidateDataForUser_Accepts_WhenUserHasMasterPassword_KdfAndSaltMatch() - { - var user = BuildValidUpdateUser(); - var data = BuildData(user); - - // Should not throw - data.ValidateDataForUser(user); - } - - [Fact] - public void ValidateDataForUser_Throws_WhenUserHasNoMasterPassword() - { - var user = BuildValidUpdateUser(); - user.MasterPassword = null; - var data = BuildData(user); - - Assert.Throws(() => data.ValidateDataForUser(user)); - } - - [Fact] - public void ValidateDataForUser_Throws_WhenUserIsKeyConnector() - { - var user = BuildValidUpdateUser(); - user.UsesKeyConnector = true; - var data = BuildData(user); - - Assert.Throws(() => data.ValidateDataForUser(user)); - } - - [Fact] - public void ValidateDataForUser_Throws_WhenKdfChanged() - { - var user = BuildValidUpdateUser(); - var mismatchedKdf = new KdfSettings - { - KdfType = KdfType.Argon2id, - Iterations = 3, - Memory = 64, - Parallelism = 4 - }; - var data = BuildData(user, kdfOverride: mismatchedKdf); - - Assert.Throws(() => data.ValidateDataForUser(user)); - } - - [Fact] - public void ValidateDataForUser_Throws_WhenSaltChanged() - { - var user = BuildValidUpdateUser(); - var data = BuildData(user, saltOverride: "wrong-salt"); - - Assert.Throws(() => data.ValidateDataForUser(user)); - } - } - - // --- Data model validation: UpdateExistingPasswordAndKdfData --- - - public class UpdateExistingPasswordAndKdfDataTests - { - private static User BuildValidUser() - { - return new User - { - Id = Guid.NewGuid(), - Email = "test@example.com", - MasterPassword = "existing-hash", - Key = "existing-key", - MasterPasswordSalt = "stored-salt", - UsesKeyConnector = false, - Kdf = KdfType.PBKDF2_SHA256, - KdfIterations = 600000 - }; - } - - private static UpdateExistingPasswordAndKdfData BuildData(User user, string? saltOverride = null) - { - var salt = saltOverride ?? user.GetMasterPasswordSalt(); - var newKdf = new KdfSettings - { - KdfType = KdfType.Argon2id, - Iterations = 3, - Memory = 64, - Parallelism = 4 - }; - return new UpdateExistingPasswordAndKdfData - { - MasterPasswordUnlock = new MasterPasswordUnlockData - { - Salt = salt, - MasterKeyWrappedUserKey = "wrapped-key", - Kdf = newKdf - }, - MasterPasswordAuthentication = new MasterPasswordAuthenticationData - { - Salt = salt, - MasterPasswordAuthenticationHash = "hash", - Kdf = newKdf - } - }; - } - - [Fact] - public void ValidateDataForUser_Accepts_WhenUserHasMasterPassword_SaltMatch_KdfChanged() - { - var user = BuildValidUser(); - var data = BuildData(user); - - // Should not throw — KDF change is permitted here - data.ValidateDataForUser(user); - } - - [Fact] - public void ValidateDataForUser_Throws_WhenUserHasNoMasterPassword() - { - var user = BuildValidUser(); - user.MasterPassword = null; - var data = BuildData(user); - - Assert.Throws(() => data.ValidateDataForUser(user)); - } - - [Fact] - public void ValidateDataForUser_Throws_WhenUserIsKeyConnector() - { - var user = BuildValidUser(); - user.UsesKeyConnector = true; - var data = BuildData(user); - - Assert.Throws(() => data.ValidateDataForUser(user)); - } - - [Fact] - public void ValidateDataForUser_Throws_WhenSaltChanged() - { - var user = BuildValidUser(); - var data = BuildData(user, saltOverride: "wrong-salt"); - - Assert.Throws(() => data.ValidateDataForUser(user)); - } - } - } From aa5bd3e8122f4de4e5fdb43e77aaecbbc5845c53 Mon Sep 17 00:00:00 2001 From: enmande <3836813+enmande@users.noreply.github.com> Date: Mon, 27 Apr 2026 10:40:11 -0400 Subject: [PATCH 23/26] test(mp-service): Address additional coverage cases. --- .../Data/SetInitialPasswordDataTests.cs | 45 +-- .../UpdateExistingPasswordAndKdfDataTests.cs | 29 ++ .../Data/UpdateExistingPasswordDataTests.cs | 84 ++++ .../MasterPasswordServiceTests.cs | 382 +++++++++++++----- 4 files changed, 395 insertions(+), 145 deletions(-) diff --git a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/Data/SetInitialPasswordDataTests.cs b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/Data/SetInitialPasswordDataTests.cs index dd5952514d14..6cd662fd643e 100644 --- a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/Data/SetInitialPasswordDataTests.cs +++ b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/Data/SetInitialPasswordDataTests.cs @@ -112,41 +112,13 @@ public void ValidateDataForUser_Throws_WhenSaltMismatch() Assert.Throws(() => data.ValidateDataForUser(user)); } - [Fact] - public void ValidateDataForUser_Throws_WhenAuthenticationSaltMismatch_UnlockSaltCorrect() - { - var user = BuildValidSetInitialUser(); - var correctSalt = user.GetMasterPasswordSalt(); - var kdf = new KdfSettings - { - KdfType = user.Kdf, - Iterations = user.KdfIterations, - Memory = user.KdfMemory, - Parallelism = user.KdfParallelism - }; - // Authentication salt is wrong; Unlock salt is correct. - var data = new SetInitialPasswordData - { - MasterPasswordUnlock = new MasterPasswordUnlockData - { - Salt = correctSalt, - MasterKeyWrappedUserKey = "wrapped-key", - Kdf = kdf - }, - MasterPasswordAuthentication = new MasterPasswordAuthenticationData - { - Salt = "wrong-auth-salt", - MasterPasswordAuthenticationHash = "hash", - Kdf = kdf - } - }; - - Assert.Throws(() => data.ValidateDataForUser(user)); - } - - [Fact] - public void ValidateDataForUser_Throws_WhenUnlockSaltMismatch_AuthenticationSaltCorrect() + [Theory] + [InlineData(true)] // unlock salt wrong, authentication salt correct + [InlineData(false)] // unlock salt correct, authentication salt wrong + public void ValidateDataForUser_Throws_WhenSaltMismatch_ValidatesBothFieldsIndependently(bool invalidateUnlockSaltInsteadOfAuthenticationSalt) { + // One salt will always be invalid in these tests -- the flag signals which; + // either/both should create an exceptional case. var user = BuildValidSetInitialUser(); var correctSalt = user.GetMasterPasswordSalt(); var kdf = new KdfSettings @@ -156,18 +128,17 @@ public void ValidateDataForUser_Throws_WhenUnlockSaltMismatch_AuthenticationSalt Memory = user.KdfMemory, Parallelism = user.KdfParallelism }; - // Unlock salt is wrong; Authentication salt is correct. var data = new SetInitialPasswordData { MasterPasswordUnlock = new MasterPasswordUnlockData { - Salt = "wrong-unlock-salt", + Salt = invalidateUnlockSaltInsteadOfAuthenticationSalt ? "wrong-salt" : correctSalt, MasterKeyWrappedUserKey = "wrapped-key", Kdf = kdf }, MasterPasswordAuthentication = new MasterPasswordAuthenticationData { - Salt = correctSalt, + Salt = invalidateUnlockSaltInsteadOfAuthenticationSalt ? correctSalt : "wrong-salt", MasterPasswordAuthenticationHash = "hash", Kdf = kdf } diff --git a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordAndKdfDataTests.cs b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordAndKdfDataTests.cs index c737076d3aac..40c8e3946a24 100644 --- a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordAndKdfDataTests.cs +++ b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordAndKdfDataTests.cs @@ -89,4 +89,33 @@ public void ValidateDataForUser_Throws_WhenSaltChanged() Assert.Throws(() => data.ValidateDataForUser(user)); } + + [Theory] + [InlineData(true)] // unlock salt wrong, authentication salt correct + [InlineData(false)] // unlock salt correct, authentication salt wrong + public void ValidateDataForUser_Throws_WhenSaltMismatch_ValidatesBothFieldsIndependently(bool invalidateUnlockSaltInsteadOfAuthenticationSalt) + { + // One salt will always be invalid in these tests -- the flag signals which; + // either/both should create an exceptional case. + var user = BuildValidUser(); + var correctSalt = user.GetMasterPasswordSalt(); + var newKdf = new KdfSettings { KdfType = KdfType.Argon2id, Iterations = 3, Memory = 64, Parallelism = 4 }; + var data = new UpdateExistingPasswordAndKdfData + { + MasterPasswordUnlock = new MasterPasswordUnlockData + { + Salt = invalidateUnlockSaltInsteadOfAuthenticationSalt ? "wrong-salt" : correctSalt, + MasterKeyWrappedUserKey = "wrapped-key", + Kdf = newKdf + }, + MasterPasswordAuthentication = new MasterPasswordAuthenticationData + { + Salt = invalidateUnlockSaltInsteadOfAuthenticationSalt ? correctSalt : "wrong-salt", + MasterPasswordAuthenticationHash = "hash", + Kdf = newKdf + } + }; + + Assert.Throws(() => data.ValidateDataForUser(user)); + } } diff --git a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordDataTests.cs b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordDataTests.cs index f478ce683b15..56cbac31c852 100644 --- a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordDataTests.cs +++ b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordDataTests.cs @@ -63,6 +63,21 @@ public void ValidateDataForUser_Accepts_WhenUserHasMasterPassword_KdfAndSaltMatc data.ValidateDataForUser(user); } + [Fact] + public void ValidateDataForUser_Accepts_WhenUserHasLegacyBelowMinimumPbkdf2Iterations() + { + // Legacy users created before the 600k minimum was enforced (Dec 2023) retain their + // original iteration count. The password-only change path must accept their existing + // KDF unchanged — ValidateUnchangedForUser mirrors the stored values; KdfSettingsValidator + // is never called on this path. + var user = BuildValidUpdateUser(); + user.KdfIterations = 100_000; + var data = BuildData(user); // mirrors user's existing 100k KDF back + + // Should not throw + data.ValidateDataForUser(user); + } + [Fact] public void ValidateDataForUser_Throws_WhenUserHasNoMasterPassword() { @@ -107,4 +122,73 @@ public void ValidateDataForUser_Throws_WhenSaltChanged() Assert.Throws(() => data.ValidateDataForUser(user)); } + + [Theory] + [InlineData(true)] // unlock salt wrong, authentication salt correct + [InlineData(false)] // unlock salt correct, authentication salt wrong + public void ValidateDataForUser_Throws_WhenSaltMismatch_ValidatesBothFieldsIndependently(bool invalidateUnlockSaltInsteadOfAuthenticationSalt) + { + // One salt will always be invalid in these tests -- the flag signals which; + // either/both should create an exceptional case. + var user = BuildValidUpdateUser(); + var correctSalt = user.GetMasterPasswordSalt(); + var kdf = new KdfSettings + { + KdfType = user.Kdf, + Iterations = user.KdfIterations, + Memory = user.KdfMemory, + Parallelism = user.KdfParallelism + }; + var data = new UpdateExistingPasswordData + { + MasterPasswordUnlock = new MasterPasswordUnlockData + { + Salt = invalidateUnlockSaltInsteadOfAuthenticationSalt ? "wrong-salt" : correctSalt, + MasterKeyWrappedUserKey = "wrapped-key", + Kdf = kdf + }, + MasterPasswordAuthentication = new MasterPasswordAuthenticationData + { + Salt = invalidateUnlockSaltInsteadOfAuthenticationSalt ? correctSalt : "wrong-salt", + MasterPasswordAuthenticationHash = "hash", + Kdf = kdf + } + }; + + Assert.Throws(() => data.ValidateDataForUser(user)); + } + + [Theory] + [InlineData(true)] // unlock KDF wrong, authentication KDF correct + [InlineData(false)] // unlock KDF correct, authentication KDF wrong + public void ValidateDataForUser_Throws_WhenKdfMismatch_ValidatesBothFieldsIndependently(bool unlockKdfIsWrong) + { + var user = BuildValidUpdateUser(); + var correctSalt = user.GetMasterPasswordSalt(); + var correctKdf = new KdfSettings + { + KdfType = user.Kdf, + Iterations = user.KdfIterations, + Memory = user.KdfMemory, + Parallelism = user.KdfParallelism + }; + var wrongKdf = new KdfSettings { KdfType = KdfType.Argon2id, Iterations = 3, Memory = 64, Parallelism = 4 }; + var data = new UpdateExistingPasswordData + { + MasterPasswordUnlock = new MasterPasswordUnlockData + { + Salt = correctSalt, + MasterKeyWrappedUserKey = "wrapped-key", + Kdf = unlockKdfIsWrong ? wrongKdf : correctKdf + }, + MasterPasswordAuthentication = new MasterPasswordAuthenticationData + { + Salt = correctSalt, + MasterPasswordAuthenticationHash = "hash", + Kdf = unlockKdfIsWrong ? correctKdf : wrongKdf + } + }; + + Assert.Throws(() => data.ValidateDataForUser(user)); + } } diff --git a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/MasterPasswordServiceTests.cs b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/MasterPasswordServiceTests.cs index 470cbeefcef6..121751efc274 100644 --- a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/MasterPasswordServiceTests.cs +++ b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/MasterPasswordServiceTests.cs @@ -8,6 +8,8 @@ using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; using NSubstitute; using Xunit; @@ -16,8 +18,22 @@ namespace Bit.Core.Test.Auth.UserFeatures.UserMasterPassword; [SutProviderCustomize] public class MasterPasswordServiceTests { + // MasterPasswordService is internal; NSubstitute cannot proxy ILogger + // from the DynamicProxyGenAssembly2 runtime assembly without InternalsVisibleTo. Provide + // NullLogger explicitly so Castle never needs to generate a proxy for the type parameter. private static SutProvider CreateSutProvider() - => new SutProvider().WithFakeTimeProvider().Create(); + => new SutProvider() + .WithFakeTimeProvider() + .SetDependency>(NullLogger.Instance) + .Create(); + + private static SutProvider CreateSutProviderWithValidator( + IPasswordValidator validator) + => new SutProvider() + .WithFakeTimeProvider() + .SetDependency>(NullLogger.Instance) + .SetDependency>>(new[] { validator }) + .Create(); // Returns a KdfSettings that exactly matches the user's stored KDF values plus a salt derived // from the user's current GetMasterPasswordSalt() output. @@ -35,7 +51,7 @@ private static (KdfSettings kdf, string salt) GetMatchingKdfAndSalt(User user) } private static SetInitialPasswordData BuildSetInitialData(User user, string? hint = null, - bool validatePassword = false) + bool validatePassword = false, bool refreshStamp = false) { // Stage 1: salt == email while MasterPasswordSalt is null (PM-28143 separates them in Stage 3). var salt = user.GetMasterPasswordSalt(); @@ -62,12 +78,12 @@ private static SetInitialPasswordData BuildSetInitialData(User user, string? hin }, MasterPasswordHint = hint, ValidatePassword = validatePassword, - RefreshStamp = false + RefreshStamp = refreshStamp }; } private static UpdateExistingPasswordData BuildUpdateExistingPasswordData(User user, string? hint = null, - bool validatePassword = false) + bool validatePassword = false, bool refreshStamp = false) { var (kdf, salt) = GetMatchingKdfAndSalt(user); return new UpdateExistingPasswordData @@ -86,12 +102,12 @@ private static UpdateExistingPasswordData BuildUpdateExistingPasswordData(User u }, MasterPasswordHint = hint, ValidatePassword = validatePassword, - RefreshStamp = false + RefreshStamp = refreshStamp }; } private static UpdateExistingPasswordAndKdfData BuildUpdateExistingPasswordAndKdfData(User user, - KdfSettings? newKdf = null, string? hint = null, bool validatePassword = false) + KdfSettings? newKdf = null, string? hint = null, bool validatePassword = false, bool refreshStamp = false) { var salt = user.GetMasterPasswordSalt(); var kdf = newKdf ?? new KdfSettings @@ -117,7 +133,7 @@ private static UpdateExistingPasswordAndKdfData BuildUpdateExistingPasswordAndKd }, MasterPasswordHint = hint, ValidatePassword = validatePassword, - RefreshStamp = false + RefreshStamp = refreshStamp }; } @@ -149,6 +165,8 @@ public async Task PrepareSetInitialMasterPassword_Success(User user) Assert.Equal(data.MasterPasswordUnlock.Salt, user.MasterPasswordSalt); Assert.Equal(data.MasterPasswordUnlock.Kdf.KdfType, user.Kdf); Assert.Equal(data.MasterPasswordUnlock.Kdf.Iterations, user.KdfIterations); + Assert.Equal(data.MasterPasswordUnlock.Kdf.Memory, user.KdfMemory); + Assert.Equal(data.MasterPasswordUnlock.Kdf.Parallelism, user.KdfParallelism); Assert.Equal(expectedTime, user.LastPasswordChangeDate); Assert.Equal(expectedTime, user.RevisionDate); Assert.Equal(user.RevisionDate, user.AccountRevisionDate); @@ -175,86 +193,68 @@ public async Task PrepareSetInitialMasterPassword_SetsMasterPasswordHint(User us } [Theory, BitAutoData] - public async Task PrepareSetInitialMasterPassword_ThrowsWhenUserNotHydrated(User user) + public async Task PrepareSetInitialMasterPassword_NullHint_OverwritesExistingHint(User user) { var sutProvider = CreateSutProvider(); - user.Id = default; user.MasterPassword = null; user.Key = null; user.MasterPasswordSalt = null; user.UsesKeyConnector = false; + user.MasterPasswordHint = "existing-hint"; - var data = BuildSetInitialData(user); + var data = BuildSetInitialData(user, hint: null); + sutProvider.GetDependency>() + .HashPassword(Arg.Any(), Arg.Any()) + .Returns("hash"); - await Assert.ThrowsAsync( - () => sutProvider.Sut.PrepareSetInitialMasterPasswordAsync(user, data)); + var result = await sutProvider.Sut.PrepareSetInitialMasterPasswordAsync(user, data); + + Assert.True(result.IsT0); + Assert.Null(result.AsT0.MasterPasswordHint); } [Theory, BitAutoData] - public async Task PrepareSetInitialMasterPassword_RefreshStampTrue_RotatesSecurityStamp(User user) + public async Task PrepareSetInitialMasterPassword_ThrowsWhenUserNotHydrated(User user) { var sutProvider = CreateSutProvider(); + user.Id = default; user.MasterPassword = null; user.Key = null; user.MasterPasswordSalt = null; user.UsesKeyConnector = false; - // Build data with RefreshStamp = true (the default — do not override). - var salt = user.GetMasterPasswordSalt(); - var kdf = new KdfSettings - { - KdfType = user.Kdf, - Iterations = user.KdfIterations, - Memory = user.KdfMemory, - Parallelism = user.KdfParallelism - }; - var data = new SetInitialPasswordData - { - MasterPasswordUnlock = new MasterPasswordUnlockData - { - Salt = salt, - MasterKeyWrappedUserKey = "wrapped-key", - Kdf = kdf - }, - MasterPasswordAuthentication = new MasterPasswordAuthenticationData - { - Salt = salt, - MasterPasswordAuthenticationHash = "test-hash", - Kdf = kdf - }, - ValidatePassword = false, - RefreshStamp = true - }; - sutProvider.GetDependency>() - .HashPassword(Arg.Any(), Arg.Any()) - .Returns("hash"); - - var originalStamp = user.SecurityStamp; - - await sutProvider.Sut.PrepareSetInitialMasterPasswordAsync(user, data); + var data = BuildSetInitialData(user); - Assert.NotEqual(originalStamp, user.SecurityStamp); + await Assert.ThrowsAsync( + () => sutProvider.Sut.PrepareSetInitialMasterPasswordAsync(user, data)); } - [Theory, BitAutoData] - public async Task PrepareSetInitialMasterPassword_RefreshStampFalse_PreservesSecurityStamp(User user) + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task PrepareSetInitialMasterPassword_SecurityStampRotation_HonorsRefreshStampFlag(bool refreshStamp) { var sutProvider = CreateSutProvider(); - user.MasterPassword = null; - user.Key = null; - user.MasterPasswordSalt = null; - user.UsesKeyConnector = false; - - var data = BuildSetInitialData(user); // RefreshStamp = false + var user = new User + { + Id = Guid.NewGuid(), + Email = "test@example.com", + MasterPassword = null, + Key = null, + MasterPasswordSalt = null, + UsesKeyConnector = false, + Kdf = KdfType.PBKDF2_SHA256, + KdfIterations = 600000, + SecurityStamp = "original-stamp" + }; + var data = BuildSetInitialData(user, refreshStamp: refreshStamp); sutProvider.GetDependency>() .HashPassword(Arg.Any(), Arg.Any()) .Returns("hash"); - var originalStamp = user.SecurityStamp; - await sutProvider.Sut.PrepareSetInitialMasterPasswordAsync(user, data); - Assert.Equal(originalStamp, user.SecurityStamp); + Assert.Equal(refreshStamp, user.SecurityStamp != "original-stamp"); } [Theory, BitAutoData] @@ -265,10 +265,7 @@ public async Task PrepareSetInitialMasterPassword_ValidationFailure_ReturnsError validator.ValidateAsync(Arg.Any>(), Arg.Any(), Arg.Any()) .Returns(IdentityResult.Failed(error)); - var sutProvider = new SutProvider() - .WithFakeTimeProvider() - .SetDependency>>(new[] { validator }) - .Create(); + var sutProvider = CreateSutProviderWithValidator(validator); user.MasterPassword = null; user.Key = null; @@ -314,10 +311,7 @@ public async Task SaveSetInitialMasterPassword_WhenValidationFails_ReturnsErrors validator.ValidateAsync(Arg.Any>(), Arg.Any(), Arg.Any()) .Returns(IdentityResult.Failed(error)); - var sutProvider = new SutProvider() - .WithFakeTimeProvider() - .SetDependency>>(new[] { validator }) - .Create(); + var sutProvider = CreateSutProviderWithValidator(validator); user.MasterPassword = null; user.Key = null; @@ -414,10 +408,7 @@ public async Task PrepareUpdateExistingMasterPassword_ValidationFailure_ReturnsE validator.ValidateAsync(Arg.Any>(), Arg.Any(), Arg.Any()) .Returns(IdentityResult.Failed(error)); - var sutProvider = new SutProvider() - .WithFakeTimeProvider() - .SetDependency>>(new[] { validator }) - .Create(); + var sutProvider = CreateSutProviderWithValidator(validator); user.MasterPassword = "existing-hash"; user.UsesKeyConnector = false; @@ -430,6 +421,63 @@ public async Task PrepareUpdateExistingMasterPassword_ValidationFailure_ReturnsE Assert.NotEmpty(result.AsT1); } + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task PrepareUpdateExistingMasterPassword_SecurityStampRotation_HonorsRefreshStampFlag(bool refreshStamp) + { + var sutProvider = CreateSutProvider(); + var user = new User + { + Id = Guid.NewGuid(), + Email = "test@example.com", + MasterPassword = "existing-hash", + MasterPasswordSalt = "stored-salt", + UsesKeyConnector = false, + Kdf = KdfType.PBKDF2_SHA256, + KdfIterations = 600000, + SecurityStamp = "original-stamp" + }; + var data = BuildUpdateExistingPasswordData(user, refreshStamp: refreshStamp); + sutProvider.GetDependency>() + .HashPassword(Arg.Any(), Arg.Any()) + .Returns("hash"); + + await sutProvider.Sut.PrepareUpdateExistingMasterPasswordAsync(user, data); + + Assert.Equal(refreshStamp, user.SecurityStamp != "original-stamp"); + } + + [Theory] + [InlineData(600_000)] // current minimum + [InlineData(100_000)] // legacy — below the current 600k minimum; must not be blocked + public async Task PrepareUpdateExistingMasterPassword_SucceedsRegardlessOfPbkdf2IterationCount(int kdfIterations) + { + var sutProvider = CreateSutProvider(); + var user = new User + { + Id = Guid.NewGuid(), + Email = "test@example.com", + MasterPassword = "existing-hash", + MasterPasswordSalt = "stored-salt", + UsesKeyConnector = false, + Kdf = KdfType.PBKDF2_SHA256, + KdfIterations = kdfIterations + }; + var data = BuildUpdateExistingPasswordData(user); + sutProvider.GetDependency>() + .HashPassword(Arg.Any(), Arg.Any()) + .Returns("new-hash"); + + var result = await sutProvider.Sut.PrepareUpdateExistingMasterPasswordAsync(user, data); + + // Password change must succeed for any existing iteration count — legacy users + // below the current 600k minimum must not be locked out of changing their password. + Assert.True(result.IsT0); + // KDF must remain unchanged — this path updates the hash only, never the KDF. + Assert.Equal(kdfIterations, user.KdfIterations); + } + // --- SaveUpdateExistingMasterPassword --- [Theory, BitAutoData] @@ -459,10 +507,7 @@ public async Task SaveUpdateExistingMasterPassword_WhenValidationFails_ReturnsEr validator.ValidateAsync(Arg.Any>(), Arg.Any(), Arg.Any()) .Returns(IdentityResult.Failed(error)); - var sutProvider = new SutProvider() - .WithFakeTimeProvider() - .SetDependency>>(new[] { validator }) - .Create(); + var sutProvider = CreateSutProviderWithValidator(validator); user.MasterPassword = "existing-hash"; user.UsesKeyConnector = false; @@ -476,6 +521,19 @@ public async Task SaveUpdateExistingMasterPassword_WhenValidationFails_ReturnsEr await sutProvider.GetDependency().DidNotReceive().ReplaceAsync(Arg.Any()); } + [Theory, BitAutoData] + public async Task SaveUpdateExistingMasterPassword_ThrowsWhenUserNotHydrated(User user) + { + var sutProvider = CreateSutProvider(); + user.Id = default; + user.MasterPassword = "existing-hash"; + + var data = BuildUpdateExistingPasswordData(user); + + await Assert.ThrowsAsync( + () => sutProvider.Sut.SaveUpdateExistingMasterPasswordAsync(user, data)); + } + // --- PrepareSetInitialOrUpdateExistingMasterPassword — Dispatch routing --- [Theory, BitAutoData] @@ -602,10 +660,7 @@ public async Task PrepareSetInitialOrUpdateExisting_PropagatesValidationErrors_W validator.ValidateAsync(Arg.Any>(), Arg.Any(), Arg.Any()) .Returns(IdentityResult.Failed(error)); - var sutProvider = new SutProvider() - .WithFakeTimeProvider() - .SetDependency>>(new[] { validator }) - .Create(); + var sutProvider = CreateSutProviderWithValidator(validator); user.MasterPassword = null; user.Key = null; @@ -637,6 +692,44 @@ public async Task PrepareSetInitialOrUpdateExisting_PropagatesValidationErrors_W Assert.NotEmpty(result.AsT1); } + [Theory, BitAutoData] + public async Task PrepareSetInitialOrUpdateExisting_PropagatesValidationErrors_WhenUpdateExistingPathFails(User user) + { + var error = new IdentityError { Code = "pwd-invalid", Description = "Password is too weak." }; + var validator = Substitute.For>(); + validator.ValidateAsync(Arg.Any>(), Arg.Any(), Arg.Any()) + .Returns(IdentityResult.Failed(error)); + + var sutProvider = CreateSutProviderWithValidator(validator); + + user.MasterPassword = "existing-hash"; + user.UsesKeyConnector = false; + + var (kdf, salt) = GetMatchingKdfAndSalt(user); + var data = new SetInitialOrUpdateExistingPasswordData + { + MasterPasswordUnlock = new MasterPasswordUnlockData + { + Salt = salt, + MasterKeyWrappedUserKey = "wrapped-key", + Kdf = kdf + }, + MasterPasswordAuthentication = new MasterPasswordAuthenticationData + { + Salt = salt, + MasterPasswordAuthenticationHash = "test-hash", + Kdf = kdf + }, + ValidatePassword = true, + RefreshStamp = false + }; + + var result = await sutProvider.Sut.PrepareSetInitialOrUpdateExistingMasterPasswordAsync(user, data); + + Assert.True(result.IsT1); + Assert.NotEmpty(result.AsT1); + } + [Theory, BitAutoData] public async Task PrepareSetInitialOrUpdateExisting_ThrowsWhenUserNotHydrated(User user) { @@ -675,7 +768,7 @@ public async Task SaveUpdateExistingMasterPasswordAndKdf_Success(User user) user.MasterPassword = "existing-hash"; user.UsesKeyConnector = false; - var data = BuildUpdateExistingPasswordAndKdfData(user); + var data = BuildUpdateExistingPasswordAndKdfData(user, hint: "test-hint"); sutProvider.GetDependency>() .HashPassword(Arg.Any(), Arg.Any()) .Returns("new-hash"); @@ -685,9 +778,14 @@ public async Task SaveUpdateExistingMasterPasswordAndKdf_Success(User user) var expectedTime = sutProvider.GetDependency().GetUtcNow().UtcDateTime; Assert.True(result.IsT0); + Assert.Equal(data.MasterPasswordUnlock.MasterKeyWrappedUserKey, user.Key); Assert.Equal(data.MasterPasswordUnlock.Kdf.KdfType, user.Kdf); Assert.Equal(data.MasterPasswordUnlock.Kdf.Iterations, user.KdfIterations); + Assert.Equal("test-hint", user.MasterPasswordHint); + Assert.Equal(expectedTime, user.LastPasswordChangeDate); Assert.Equal(expectedTime, user.LastKdfChangeDate); + Assert.Equal(expectedTime, user.RevisionDate); + Assert.Equal(user.RevisionDate, user.AccountRevisionDate); await sutProvider.GetDependency().Received().ReplaceAsync(user); } @@ -763,10 +861,7 @@ public async Task SaveUpdateExistingMasterPasswordAndKdf_WhenValidationFails_Ret validator.ValidateAsync(Arg.Any>(), Arg.Any(), Arg.Any()) .Returns(IdentityResult.Failed(error)); - var sutProvider = new SutProvider() - .WithFakeTimeProvider() - .SetDependency>>(new[] { validator }) - .Create(); + var sutProvider = CreateSutProviderWithValidator(validator); user.MasterPassword = "existing-hash"; user.UsesKeyConnector = false; @@ -829,6 +924,59 @@ await Assert.ThrowsAsync( () => sutProvider.Sut.SaveUpdateExistingMasterPasswordAndKdfAsync(user, data)); } + [Theory, BitAutoData] + public async Task SaveUpdateExistingMasterPasswordAndKdf_ThrowsWhenUserNotHydrated(User user) + { + var sutProvider = CreateSutProvider(); + user.Id = default; + user.MasterPassword = "existing-hash"; + + var data = BuildUpdateExistingPasswordAndKdfData(user); + + await Assert.ThrowsAsync( + () => sutProvider.Sut.SaveUpdateExistingMasterPasswordAndKdfAsync(user, data)); + } + + [Theory, BitAutoData] + public async Task SaveUpdateExistingMasterPasswordAndKdf_ThrowsForKeyConnectorUser(User user) + { + var sutProvider = CreateSutProvider(); + user.MasterPassword = "existing-hash"; + user.UsesKeyConnector = true; + + var data = BuildUpdateExistingPasswordAndKdfData(user); + + await Assert.ThrowsAsync( + () => sutProvider.Sut.SaveUpdateExistingMasterPasswordAndKdfAsync(user, data)); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task SaveUpdateExistingMasterPasswordAndKdf_SecurityStampRotation_HonorsRefreshStampFlag(bool refreshStamp) + { + var sutProvider = CreateSutProvider(); + var user = new User + { + Id = Guid.NewGuid(), + Email = "test@example.com", + MasterPassword = "existing-hash", + MasterPasswordSalt = "stored-salt", + UsesKeyConnector = false, + Kdf = KdfType.PBKDF2_SHA256, + KdfIterations = 600000, + SecurityStamp = "original-stamp" + }; + var data = BuildUpdateExistingPasswordAndKdfData(user, refreshStamp: refreshStamp); + sutProvider.GetDependency>() + .HashPassword(Arg.Any(), Arg.Any()) + .Returns("hash"); + + await sutProvider.Sut.SaveUpdateExistingMasterPasswordAndKdfAsync(user, data); + + Assert.Equal(refreshStamp, user.SecurityStamp != "original-stamp"); + } + // --- BuildUpdateUserDelegateSetInitialMasterPassword --- public class BuildUpdateUserDelegateSetInitialMasterPasswordTests @@ -836,7 +984,7 @@ public class BuildUpdateUserDelegateSetInitialMasterPasswordTests [Theory, BitAutoData] public void BuildUpdateUserDelegate_ThrowsWhenUserNotHydrated(User user) { - var sutProvider = new SutProvider().WithFakeTimeProvider().Create(); + var sutProvider = CreateSutProvider(); user.Id = default; user.MasterPassword = null; user.Key = null; @@ -850,27 +998,42 @@ public void BuildUpdateUserDelegate_ThrowsWhenUserNotHydrated(User user) } [Theory, BitAutoData] - public void BuildUpdateUserDelegate_HappyPath_ReturnsNonNullDelegateAndDoesNotPersist(User user) + public async Task BuildUpdateUserDelegate_HappyPath_ReturnsNonNullDelegateAndDoesNotPersist(User user) { - var sutProvider = new SutProvider().WithFakeTimeProvider().Create(); + var sutProvider = CreateSutProvider(); user.MasterPassword = null; user.Key = null; user.MasterPasswordSalt = null; user.UsesKeyConnector = false; + sutProvider.GetDependency>() + .HashPassword(Arg.Any(), Arg.Any()) + .Returns("hashed"); + + UpdateUserData noopWrite = (_, _) => Task.CompletedTask; + sutProvider.GetDependency() + .SetMasterPassword(Arg.Any(), Arg.Any(), + Arg.Any(), Arg.Any()) + .Returns(noopWrite); + var data = BuildSetInitialDataForUser(user); - var result = sutProvider.Sut.BuildUpdateUserDelegateSetInitialMasterPassword(user, data); + var write = sutProvider.Sut.BuildUpdateUserDelegateSetInitialMasterPassword(user, data); // The Build* tier returns a delegate — it must not persist directly. - Assert.NotNull(result); - sutProvider.GetDependency().DidNotReceive().ReplaceAsync(Arg.Any()); + Assert.NotNull(write); + await sutProvider.GetDependency().DidNotReceive().ReplaceAsync(Arg.Any()); + + // Invoking the delegate must write via SetMasterPassword, not ReplaceAsync. + await write(null, null); + sutProvider.GetDependency().Received() + .SetMasterPassword(user.Id, data.MasterPasswordUnlock, "hashed", data.MasterPasswordHint); } [Theory, BitAutoData] public void BuildUpdateUserDelegate_ThrowsWhenUserAlreadyHasMasterPassword(User user) { - var sutProvider = new SutProvider().WithFakeTimeProvider().Create(); + var sutProvider = CreateSutProvider(); // User already has a master password — ValidateDataForUser must be called eagerly. user.MasterPassword = "existing-hash"; @@ -889,10 +1052,7 @@ public async Task BuildUpdateUserDelegate_WhenValidatePasswordTrue_CompletesAndP validator.ValidateAsync(Arg.Any>(), Arg.Any(), Arg.Any()) .Returns(IdentityResult.Success); - var sutProvider = new SutProvider() - .WithFakeTimeProvider() - .SetDependency>>(new[] { validator }) - .Create(); + var sutProvider = CreateSutProviderWithValidator(validator); user.MasterPassword = null; user.Key = null; @@ -932,10 +1092,7 @@ public async Task BuildUpdateUserDelegate_WhenValidationFails_DelegateInvocation validator.ValidateAsync(Arg.Any>(), Arg.Any(), Arg.Any()) .Returns(IdentityResult.Failed(error)); - var sutProvider = new SutProvider() - .WithFakeTimeProvider() - .SetDependency>>(new[] { validator }) - .Create(); + var sutProvider = CreateSutProviderWithValidator(validator); user.MasterPassword = null; user.Key = null; @@ -958,15 +1115,24 @@ public async Task BuildUpdateUserDelegate_WhenValidationFails_DelegateInvocation // Contract: SetInitialPasswordData.RefreshStamp XML-docs say SecurityStamp rotates when true. // Prepare*/Save* honor this; Build* must too (rotation composed into the returned delegate). - [Theory, BitAutoData] - public async Task BuildUpdateUserDelegate_WhenRefreshStampTrue_RotatesSecurityStamp(User user) + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task BuildUpdateUserDelegate_WhenRefreshStamp_SecurityStampRotation_HonorsFlag(bool refreshStamp) { - var sutProvider = new SutProvider().WithFakeTimeProvider().Create(); - user.MasterPassword = null; - user.Key = null; - user.MasterPasswordSalt = null; - user.UsesKeyConnector = false; - user.SecurityStamp = "original-stamp"; + var sutProvider = CreateSutProvider(); + var user = new User + { + Id = Guid.NewGuid(), + Email = "test@example.com", + MasterPassword = null, + Key = null, + MasterPasswordSalt = null, + UsesKeyConnector = false, + Kdf = KdfType.PBKDF2_SHA256, + KdfIterations = 600000, + SecurityStamp = "original-stamp" + }; UpdateUserData noopWrite = (_, _) => Task.CompletedTask; sutProvider.GetDependency() @@ -975,12 +1141,12 @@ public async Task BuildUpdateUserDelegate_WhenRefreshStampTrue_RotatesSecuritySt .Returns(noopWrite); var data = BuildSetInitialDataForUser(user); - data.RefreshStamp = true; + data.RefreshStamp = refreshStamp; var write = sutProvider.Sut.BuildUpdateUserDelegateSetInitialMasterPassword(user, data); await write(null, null); - Assert.NotEqual("original-stamp", user.SecurityStamp); + Assert.Equal(refreshStamp, user.SecurityStamp != "original-stamp"); } private static SetInitialPasswordData BuildSetInitialDataForUser(User user) From ab25cafb6e9063813e3922b8afe100da050bd535 Mon Sep 17 00:00:00 2001 From: enmande <3836813+enmande@users.noreply.github.com> Date: Mon, 27 Apr 2026 10:50:58 -0400 Subject: [PATCH 24/26] docs(mp-service): Spelling. --- .../Interfaces/IMasterPasswordService.cs | 2 +- .../UserMasterPassword/MasterPasswordService.cs | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs index 785793559443..34dbd67bf849 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs @@ -190,7 +190,7 @@ internal interface IMasterPasswordService /// /// /// - /// Constraints (applied when the returned delegate is invoked; see also ): + /// Constraints (see also ): /// /// User must not already have a master password. /// User must have no existing Key or MasterPasswordSalt. diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs index 049b2c8c2ec7..058c57323784 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs @@ -53,7 +53,7 @@ public async Task> PrepareSetInitialMasterPasswordA // Note: Keep "unlock and authenticate" pattern in mind. // We name "unlock" first as a naming convention, - // e.g., MatherPasswordUnlockAndAuthenticationDataModel. + // e.g., MasterPasswordUnlockAndAuthenticationDataModel. // The two are complimentary, but mechanically Authenticate comes first. // Eager validation keeps the logic easier to reason about. // Authentication is the mechanism for validation, unlock is the capability. @@ -111,7 +111,7 @@ public UpdateUserData BuildUpdateUserDelegateSetInitialMasterPassword( { // Note: Keep "unlock and authenticate" pattern in mind. // We name "unlock" first as a naming convention, - // e.g., MatherPasswordUnlockAndAuthenticationDataModel. + // e.g., MasterPasswordUnlockAndAuthenticationDataModel. // The two are complimentary, but mechanically Authenticate comes first. // Eager validation keeps the logic easier to reason about. // Authentication is the mechanism for validation, unlock is the capability. @@ -153,7 +153,7 @@ public async Task> PrepareUpdateExistingMasterPassw // Note: Keep "unlock and authenticate" pattern in mind. // We name "unlock" first as a naming convention, - // e.g., MatherPasswordUnlockAndAuthenticationDataModel. + // e.g., MasterPasswordUnlockAndAuthenticationDataModel. // The two are complimentary, but mechanically Authenticate comes first. // Eager validation keeps the logic easier to reason about. // Authentication is the mechanism for validation, unlock is the capability. @@ -190,7 +190,7 @@ public async Task> SaveUpdateExistingMasterPassword // Note: Keep "unlock and authenticate" pattern in mind. // We name "unlock" first as a naming convention, - // e.g., MatherPasswordUnlockAndAuthenticationDataModel. + // e.g., MasterPasswordUnlockAndAuthenticationDataModel. // The two are complimentary, but mechanically Authenticate comes first. // Eager validation keeps the logic easier to reason about. // Authentication is the mechanism for validation, unlock is the capability. From 6cfee8b62359714ce29dc3a89c5454220149db61 Mon Sep 17 00:00:00 2001 From: enmande <3836813+enmande@users.noreply.github.com> Date: Mon, 27 Apr 2026 16:18:49 -0400 Subject: [PATCH 25/26] refactor(mp-service): Extract user security stamp rotation to its own helper. --- .../MasterPasswordService.cs | 62 +++++++++++++------ 1 file changed, 44 insertions(+), 18 deletions(-) diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs index 058c57323784..780350ed367b 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs @@ -57,16 +57,20 @@ public async Task> PrepareSetInitialMasterPasswordA // The two are complimentary, but mechanically Authenticate comes first. // Eager validation keeps the logic easier to reason about. // Authentication is the mechanism for validation, unlock is the capability. - var result = await ApplyPasswordHashAsync( + var result = await ApplyMasterPasswordAuthenticationHashAsync( user, setInitialData.MasterPasswordAuthentication.MasterPasswordAuthenticationHash, - setInitialData.ValidatePassword, - setInitialData.RefreshStamp); + setInitialData.ValidatePassword); if (!result.Succeeded) { return result.Errors.ToArray(); } + if (setInitialData.RefreshStamp) + { + ApplyUserSecurityStampRotation(user); + } + user.Key = setInitialData.MasterPasswordUnlock.MasterKeyWrappedUserKey; ApplyKdfStateOnUser(user, setInitialData.MasterPasswordUnlock.Kdf); @@ -131,7 +135,7 @@ public UpdateUserData BuildUpdateUserDelegateSetInitialMasterPassword( // TODO (PM-35501): IUserRepository.SetMasterPassword does not persist // SecurityStamp (sproc + EF query both omit it). Rotation here is // in-memory only until the primitive is extended. - user.SecurityStamp = Guid.NewGuid().ToString(); + ApplyUserSecurityStampRotation(user); } // Hash the provided user master password authentication hash on the server side @@ -157,17 +161,21 @@ public async Task> PrepareUpdateExistingMasterPassw // The two are complimentary, but mechanically Authenticate comes first. // Eager validation keeps the logic easier to reason about. // Authentication is the mechanism for validation, unlock is the capability. - var result = await ApplyPasswordHashAsync( + var result = await ApplyMasterPasswordAuthenticationHashAsync( user, updateExistingData.MasterPasswordAuthentication.MasterPasswordAuthenticationHash, - updateExistingData.ValidatePassword, - updateExistingData.RefreshStamp); + updateExistingData.ValidatePassword); if (!result.Succeeded) { return result.Errors.ToArray(); } + if (updateExistingData.RefreshStamp) + { + ApplyUserSecurityStampRotation(user); + } + user.Key = updateExistingData.MasterPasswordUnlock.MasterKeyWrappedUserKey; // Always override the master password hint, even if it's null @@ -194,17 +202,21 @@ public async Task> SaveUpdateExistingMasterPassword // The two are complimentary, but mechanically Authenticate comes first. // Eager validation keeps the logic easier to reason about. // Authentication is the mechanism for validation, unlock is the capability. - var result = await ApplyPasswordHashAsync( + var result = await ApplyMasterPasswordAuthenticationHashAsync( user, updateExistingData.MasterPasswordAuthentication.MasterPasswordAuthenticationHash, - updateExistingData.ValidatePassword, - updateExistingData.RefreshStamp); + updateExistingData.ValidatePassword); if (!result.Succeeded) { return result.Errors.ToArray(); } + if (updateExistingData.RefreshStamp) + { + ApplyUserSecurityStampRotation(user); + } + user.Key = updateExistingData.MasterPasswordUnlock.MasterKeyWrappedUserKey; ApplyKdfStateOnUser(user, updateExistingData.MasterPasswordUnlock.Kdf); @@ -240,12 +252,11 @@ public async Task> SaveUpdateExistingMasterPassword /// ///Applies via hashing the to the . - ///Optionally validates the password, flagged with - ///and rotates the security stamp, flagged with . + ///Optionally validates the password, flagged with . ///Used by both initial-set and update master password paths. /// - private async Task ApplyPasswordHashAsync(User user, string newPassword, - bool validatePassword = true, bool refreshStamp = true) + private async Task ApplyMasterPasswordAuthenticationHashAsync(User user, string newPassword, + bool validatePassword = true) { if (validatePassword) { @@ -257,14 +268,29 @@ private async Task ApplyPasswordHashAsync(User user, string newP } user.MasterPassword = _passwordHasher.HashPassword(user, newPassword); - if (refreshStamp) - { - user.SecurityStamp = Guid.NewGuid().ToString(); - } return IdentityResult.Success; } + /// + /// Rotates by replacing it with a new random value. + /// + /// The security stamp is an opaque random identifier included as a claim in refresh tokens issued + /// by IdentityServer. On every token refresh, the claim value is compared against the user's current + /// stamp; a mismatch marks the token inactive and rejects the refresh. Rotating the stamp therefore + /// immediately invalidates all active sessions without requiring a password change. + /// + /// + /// Call this after any operation that changes the user's authentication credential or cryptographic + /// state — setting or updating a master password hash, rotating KDF parameters — where session + /// continuity is not intentionally preserved. + /// + /// + private static void ApplyUserSecurityStampRotation(User user) + { + user.SecurityStamp = Guid.NewGuid().ToString(); + } + /// /// Applies KDF parameters from the supplied to the . /// Used by both initial-set and KDF-rotation paths. From a672bdcae9f780796b6403b9cbad12137fc73135 Mon Sep 17 00:00:00 2001 From: enmande <3836813+enmande@users.noreply.github.com> Date: Mon, 27 Apr 2026 16:39:27 -0400 Subject: [PATCH 26/26] docs(mp-service): Clarify authentication hash documentation. --- .../MasterPasswordService.cs | 56 +++++++++---------- 1 file changed, 25 insertions(+), 31 deletions(-) diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs index 780350ed367b..12d3cb228263 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs @@ -51,12 +51,6 @@ public async Task> PrepareSetInitialMasterPasswordA EnsureUserIsHydrated(user); setInitialData.ValidateDataForUser(user); - // Note: Keep "unlock and authenticate" pattern in mind. - // We name "unlock" first as a naming convention, - // e.g., MasterPasswordUnlockAndAuthenticationDataModel. - // The two are complimentary, but mechanically Authenticate comes first. - // Eager validation keeps the logic easier to reason about. - // Authentication is the mechanism for validation, unlock is the capability. var result = await ApplyMasterPasswordAuthenticationHashAsync( user, setInitialData.MasterPasswordAuthentication.MasterPasswordAuthenticationHash, @@ -113,12 +107,6 @@ public UpdateUserData BuildUpdateUserDelegateSetInitialMasterPassword( return async (connection, transaction) => { - // Note: Keep "unlock and authenticate" pattern in mind. - // We name "unlock" first as a naming convention, - // e.g., MasterPasswordUnlockAndAuthenticationDataModel. - // The two are complimentary, but mechanically Authenticate comes first. - // Eager validation keeps the logic easier to reason about. - // Authentication is the mechanism for validation, unlock is the capability. if (setInitialData.ValidatePassword) { var validate = await ValidatePasswordInternalAsync(user, @@ -155,12 +143,6 @@ public async Task> PrepareUpdateExistingMasterPassw EnsureUserIsHydrated(user); updateExistingData.ValidateDataForUser(user); - // Note: Keep "unlock and authenticate" pattern in mind. - // We name "unlock" first as a naming convention, - // e.g., MasterPasswordUnlockAndAuthenticationDataModel. - // The two are complimentary, but mechanically Authenticate comes first. - // Eager validation keeps the logic easier to reason about. - // Authentication is the mechanism for validation, unlock is the capability. var result = await ApplyMasterPasswordAuthenticationHashAsync( user, updateExistingData.MasterPasswordAuthentication.MasterPasswordAuthenticationHash, @@ -196,12 +178,6 @@ public async Task> SaveUpdateExistingMasterPassword EnsureUserIsHydrated(user); updateExistingData.ValidateDataForUser(user); - // Note: Keep "unlock and authenticate" pattern in mind. - // We name "unlock" first as a naming convention, - // e.g., MasterPasswordUnlockAndAuthenticationDataModel. - // The two are complimentary, but mechanically Authenticate comes first. - // Eager validation keeps the logic easier to reason about. - // Authentication is the mechanism for validation, unlock is the capability. var result = await ApplyMasterPasswordAuthenticationHashAsync( user, updateExistingData.MasterPasswordAuthentication.MasterPasswordAuthenticationHash, @@ -250,24 +226,42 @@ public async Task> SaveUpdateExistingMasterPassword return result.AsT0; } - /// - ///Applies via hashing the to the . - ///Optionally validates the password, flagged with . - ///Used by both initial-set and update master password paths. + /// + /// Server-side hashes the client-supplied master password authentication hash + /// () and writes the result to + /// . + /// + /// The client derives the authentication hash from the salted plaintext master password via KDF before + /// transmission, so the plaintext never reaches the server. The server applies a second hash via + /// , meaning + /// stores a hash-of-hash. At login, this + /// stored value is compared against a freshly client-derived hash to verify identity. + /// + /// + /// The authentication hash is distinct from the master password unlock material: both derive from + /// the same KDF pass over the salted user-provided plaintext, but unlock material (the master-key-wrapped user key, + /// salt, and KDF parameters) enables vault decryption client-side and is never validated server-side. + /// Authentication proves identity; unlock enables capability. + /// + /// + /// When is true, the hash is first run through the + /// registered password validator pipeline to enforce password policy before hashing. + /// + /// /// - private async Task ApplyMasterPasswordAuthenticationHashAsync(User user, string newPassword, + private async Task ApplyMasterPasswordAuthenticationHashAsync(User user, string newAuthenticationHash, bool validatePassword = true) { if (validatePassword) { - var validate = await ValidatePasswordInternalAsync(user, newPassword); + var validate = await ValidatePasswordInternalAsync(user, newAuthenticationHash); if (!validate.Succeeded) { return validate; } } - user.MasterPassword = _passwordHasher.HashPassword(user, newPassword); + user.MasterPassword = _passwordHasher.HashPassword(user, newAuthenticationHash); return IdentityResult.Success; }