diff --git a/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs b/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs index 7e90b61567cc..d3c73c9d35e7 100644 --- a/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs +++ b/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs @@ -41,6 +41,7 @@ using Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Requests; using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Identity; using Microsoft.AspNetCore.Mvc; using AccountRecoveryV2 = Bit.Core.AdminConsole.OrganizationFeatures.AccountRecovery.v2; using V1_RevokeOrganizationUserCommand = Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.RevokeUser.v1.IRevokeOrganizationUserCommand; @@ -533,14 +534,25 @@ public async Task PutRecoverAccount(Guid orgId, Guid id, [FromBody] Org return Handle(await _adminRecoverAccountCommandV2.RecoverAccountAsync(commandRequest)); } - var result = await _adminRecoverAccountCommand.RecoverAccountAsync( - orgId, targetOrganizationUser, model.NewMasterPasswordHash!, model.Key!); - if (result.Succeeded) + IdentityResult identityResult; + if (model.RequestHasNewDataTypes()) + { + identityResult = await _adminRecoverAccountCommand.RecoverAccountAsync( + orgId, targetOrganizationUser, model.UnlockData!.ToData(), model.AuthenticationData!.ToData()); + } + // To be removed in PM-33141 + else + { + identityResult = await _adminRecoverAccountCommand.RecoverAccountAsync( + orgId, targetOrganizationUser, model.NewMasterPasswordHash!, model.Key!); + } + + if (identityResult.Succeeded) { return TypedResults.Ok(); } - foreach (var error in result.Errors) + foreach (var error in identityResult.Errors) { ModelState.AddModelError(string.Empty, error.Description); } diff --git a/src/Api/Auth/Controllers/AccountsController.cs b/src/Api/Auth/Controllers/AccountsController.cs index 3cee44befacb..576014fe108f 100644 --- a/src/Api/Auth/Controllers/AccountsController.cs +++ b/src/Api/Auth/Controllers/AccountsController.cs @@ -13,6 +13,7 @@ using Bit.Core.Auth.Models.Api.Request.Accounts; using Bit.Core.Auth.Services; using Bit.Core.Auth.UserFeatures.TdeOffboardingPassword.Interfaces; +using Bit.Core.Auth.UserFeatures.TempPassword.Interfaces; using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; using Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; using Bit.Core.Enums; @@ -25,64 +26,47 @@ using Bit.Core.Services; using Bit.Core.Utilities; using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Identity; using Microsoft.AspNetCore.Mvc; namespace Bit.Api.Auth.Controllers; [Route("accounts")] [Authorize(Policies.Application)] -public class AccountsController : Controller +public class AccountsController( + IOrganizationService organizationService, + IOrganizationUserRepository organizationUserRepository, + IProviderUserRepository providerUserRepository, + IUserService userService, + ISelfServicePasswordChangeCommand selfServicePasswordChangeCommand, + IPolicyService policyService, + IFinishSsoJitProvisionMasterPasswordCommand finishSsoJitProvisionMasterPasswordCommand, + ISetInitialMasterPasswordCommandV1 setInitialMasterPasswordCommandV1, + ITdeSetPasswordCommand tdeSetPasswordCommand, + ITdeOffboardingPasswordCommand tdeOffboardingPasswordCommand, + IReplaceAdminSetTemporaryPasswordCommand replaceAdminSetTemporaryPasswordCommand, + ITwoFactorIsEnabledQuery twoFactorIsEnabledQuery, + IUserAccountKeysQuery userAccountKeysQuery, + ITwoFactorEmailService twoFactorEmailService, + IChangeKdfCommand changeKdfCommand, + IUserRepository userRepository) : Controller { - private readonly IOrganizationService _organizationService; - private readonly IOrganizationUserRepository _organizationUserRepository; - private readonly IProviderUserRepository _providerUserRepository; - private readonly IUserService _userService; - private readonly IPolicyService _policyService; - private readonly ISetInitialMasterPasswordCommandV1 _setInitialMasterPasswordCommandV1; - private readonly IFinishSsoJitProvisionMasterPasswordCommand _finishSsoJitProvisionMasterPasswordCommand; - private readonly ITdeSetPasswordCommand _tdeSetPasswordCommand; - private readonly ITdeOffboardingPasswordCommand _tdeOffboardingPasswordCommand; - private readonly ITwoFactorIsEnabledQuery _twoFactorIsEnabledQuery; - private readonly IFeatureService _featureService; - private readonly IUserAccountKeysQuery _userAccountKeysQuery; - private readonly ITwoFactorEmailService _twoFactorEmailService; - private readonly IChangeKdfCommand _changeKdfCommand; - private readonly IUserRepository _userRepository; - - public AccountsController( - IOrganizationService organizationService, - IOrganizationUserRepository organizationUserRepository, - IProviderUserRepository providerUserRepository, - IUserService userService, - IPolicyService policyService, - IFinishSsoJitProvisionMasterPasswordCommand finishSsoJitProvisionMasterPasswordCommand, - ISetInitialMasterPasswordCommandV1 setInitialMasterPasswordCommandV1, - ITdeSetPasswordCommand tdeSetPasswordCommand, - ITdeOffboardingPasswordCommand tdeOffboardingPasswordCommand, - ITwoFactorIsEnabledQuery twoFactorIsEnabledQuery, - IFeatureService featureService, - IUserAccountKeysQuery userAccountKeysQuery, - ITwoFactorEmailService twoFactorEmailService, - IChangeKdfCommand changeKdfCommand, - IUserRepository userRepository - ) - { - _organizationService = organizationService; - _organizationUserRepository = organizationUserRepository; - _providerUserRepository = providerUserRepository; - _userService = userService; - _policyService = policyService; - _finishSsoJitProvisionMasterPasswordCommand = finishSsoJitProvisionMasterPasswordCommand; - _setInitialMasterPasswordCommandV1 = setInitialMasterPasswordCommandV1; - _tdeSetPasswordCommand = tdeSetPasswordCommand; - _tdeOffboardingPasswordCommand = tdeOffboardingPasswordCommand; - _twoFactorIsEnabledQuery = twoFactorIsEnabledQuery; - _featureService = featureService; - _userAccountKeysQuery = userAccountKeysQuery; - _twoFactorEmailService = twoFactorEmailService; - _changeKdfCommand = changeKdfCommand; - _userRepository = userRepository; - } + private readonly IOrganizationService _organizationService = organizationService; + private readonly IOrganizationUserRepository _organizationUserRepository = organizationUserRepository; + private readonly IProviderUserRepository _providerUserRepository = providerUserRepository; + private readonly IUserService _userService = userService; + private readonly ISelfServicePasswordChangeCommand _selfServicePasswordChangeCommand = selfServicePasswordChangeCommand; + private readonly IPolicyService _policyService = policyService; + private readonly ISetInitialMasterPasswordCommandV1 _setInitialMasterPasswordCommandV1 = setInitialMasterPasswordCommandV1; + private readonly IFinishSsoJitProvisionMasterPasswordCommand _finishSsoJitProvisionMasterPasswordCommand = finishSsoJitProvisionMasterPasswordCommand; + private readonly ITdeSetPasswordCommand _tdeSetPasswordCommand = tdeSetPasswordCommand; + private readonly ITdeOffboardingPasswordCommand _tdeOffboardingPasswordCommand = tdeOffboardingPasswordCommand; + private readonly IReplaceAdminSetTemporaryPasswordCommand _replaceAdminSetTemporaryPasswordCommand = replaceAdminSetTemporaryPasswordCommand; + private readonly ITwoFactorIsEnabledQuery _twoFactorIsEnabledQuery = twoFactorIsEnabledQuery; + private readonly IUserAccountKeysQuery _userAccountKeysQuery = userAccountKeysQuery; + private readonly ITwoFactorEmailService _twoFactorEmailService = twoFactorEmailService; + private readonly IChangeKdfCommand _changeKdfCommand = changeKdfCommand; + private readonly IUserRepository _userRepository = userRepository; [HttpPost("password-hint")] @@ -188,6 +172,14 @@ public async Task PostVerifyEmailToken([FromBody] VerifyEmailRequestModel model) throw new BadRequestException(ModelState); } + /// + /// For a user updating an existing password. + /// + /// If calling this when a user does not have a master password, it will fail. + /// + /// + /// + /// [HttpPost("password")] public async Task PostPassword([FromBody] PasswordRequestModel model) { @@ -197,8 +189,23 @@ public async Task PostPassword([FromBody] PasswordRequestModel model) throw new UnauthorizedAccessException(); } - var result = await _userService.ChangePasswordAsync(user, model.MasterPasswordHash, - model.NewMasterPasswordHash, model.MasterPasswordHint, model.Key); + IdentityResult result; + if (model.RequestHasNewDataTypes()) + { + result = await _selfServicePasswordChangeCommand.ChangePasswordAsync( + user, + model.MasterPasswordHash, + model.UnlockData!.ToData(), + model.AuthenticationData!.ToData(), + model.MasterPasswordHint); + } + // To be removed in PM-33141 + else + { + result = await _userService.ChangePasswordAsync(user, model.MasterPasswordHash, + model.NewMasterPasswordHash, model.MasterPasswordHint, model.Key); + } + if (result.Succeeded) { return; @@ -222,7 +229,7 @@ public async Task PostSetPasswordAsync([FromBody] SetInitialPasswordRequestModel throw new UnauthorizedAccessException(); } - if (model.IsV2Request()) + if (model.RequestHasNewDataTypes()) { if (model.IsTdeSetPasswordRequest()) { @@ -235,7 +242,7 @@ public async Task PostSetPasswordAsync([FromBody] SetInitialPasswordRequestModel } else { - // TODO removed with https://bitwarden.atlassian.net/browse/PM-27327 + // To be removed in PM-33141 try { user = model.ToUser(user); @@ -248,8 +255,8 @@ public async Task PostSetPasswordAsync([FromBody] SetInitialPasswordRequestModel var result = await _setInitialMasterPasswordCommandV1.SetInitialMasterPasswordAsync( user, - model.MasterPasswordHash, - model.Key, + model.MasterPasswordHash!, + model.Key!, model.OrgIdentifier); if (result.Succeeded) @@ -288,7 +295,7 @@ public async Task PostVerifyPassword([FromBod } [HttpPost("kdf")] - public async Task PostKdf([FromBody] PasswordRequestModel model) + public async Task PostKdf([FromBody] ChangeKdfRequestModel model) { var user = await _userService.GetUserByPrincipalAsync(User); if (user == null) @@ -301,7 +308,10 @@ public async Task PostKdf([FromBody] PasswordRequestModel model) throw new BadRequestException("AuthenticationData and UnlockData must be provided."); } - var result = await _changeKdfCommand.ChangeKdfAsync(user, model.MasterPasswordHash, model.AuthenticationData.ToData(), model.UnlockData.ToData()); + var result = await _changeKdfCommand.ChangeKdfAsync(user, + model.MasterPasswordHash, + model.AuthenticationData.ToData(), + model.UnlockData.ToData()); if (result.Succeeded) { return; @@ -646,7 +656,25 @@ public async Task PutUpdateTempPasswordAsync([FromBody] UpdateTempPasswordReques throw new UnauthorizedAccessException(); } - var result = await _userService.UpdateTempPasswordAsync(user, model.NewMasterPasswordHash, model.Key, model.MasterPasswordHint); + IdentityResult result; + if (model.RequestHasNewDataTypes()) + { + result = await _replaceAdminSetTemporaryPasswordCommand.Replace( + user, + model.UnlockData!.ToData(), + model.AuthenticationData!.ToData(), + model.MasterPasswordHint); + } + // To be removed in PM-33141 + else + { + result = await _userService.UpdateTempPasswordAsync( + user, + model.NewMasterPasswordHash, + model.Key, + model.MasterPasswordHint); + } + if (result.Succeeded) { return; @@ -669,7 +697,17 @@ public async Task PutUpdateTdePasswordAsync([FromBody] UpdateTdeOffboardingPassw throw new UnauthorizedAccessException(); } - var result = await _tdeOffboardingPasswordCommand.UpdateTdeOffboardingPasswordAsync(user, model.NewMasterPasswordHash, model.Key, model.MasterPasswordHint); + IdentityResult result; + if (model.RequestHasNewDataTypes()) + { + result = await _tdeOffboardingPasswordCommand.UpdateTdeOffboardingPasswordAsync(user, model.UnlockData!.ToData(), model.AuthenticationData!.ToData(), model.MasterPasswordHint); + } + // To be removed in PM-33141 + else + { + result = await _tdeOffboardingPasswordCommand.UpdateTdeOffboardingPasswordAsync(user, model.NewMasterPasswordHash!, model.Key!, model.MasterPasswordHint); + } + if (result.Succeeded) { return; diff --git a/src/Api/Auth/Controllers/EmergencyAccessController.cs b/src/Api/Auth/Controllers/EmergencyAccessController.cs index bd87e82c8a97..15bdcd81e7e0 100644 --- a/src/Api/Auth/Controllers/EmergencyAccessController.cs +++ b/src/Api/Auth/Controllers/EmergencyAccessController.cs @@ -13,6 +13,7 @@ using Bit.Core.Services; using Bit.Core.Settings; using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Identity; using Microsoft.AspNetCore.Mvc; namespace Bit.Api.Auth.Controllers; @@ -170,10 +171,34 @@ public async Task Takeover(Guid id) } [HttpPost("{id}/password")] - public async Task Password(Guid id, [FromBody] EmergencyAccessPasswordRequestModel model) + public async Task Password(Guid id, [FromBody] EmergencyAccessPasswordRequestModel model) { var user = await _userService.GetUserByPrincipalAsync(User); - await _emergencyAccessService.PasswordAsync(id, user, model.NewMasterPasswordHash, model.Key); + + IdentityResult identityResult; + if (model.RequestHasNewDataTypes()) + { + identityResult = await _emergencyAccessService.FinishRecoveryTakeoverAsync(id, user, model.UnlockData!.ToData(), model.AuthenticationData!.ToData()); + + if (identityResult.Succeeded) + { + return TypedResults.Ok(); + } + + foreach (var error in identityResult.Errors) + { + ModelState.AddModelError(string.Empty, error.Description); + } + + return TypedResults.BadRequest(ModelState); + } + // To be removed in PM-33141 + else + { + await _emergencyAccessService.FinishRecoveryTakeoverAsync(id, user, model.NewMasterPasswordHash!, model.Key!); + + return TypedResults.Ok(); + } } [HttpPost("{id}/view")] diff --git a/src/Api/Auth/Models/Request/Accounts/ChangeKdfRequestModel.cs b/src/Api/Auth/Models/Request/Accounts/ChangeKdfRequestModel.cs new file mode 100644 index 000000000000..0da6a7db57a3 --- /dev/null +++ b/src/Api/Auth/Models/Request/Accounts/ChangeKdfRequestModel.cs @@ -0,0 +1,45 @@ +using System.ComponentModel.DataAnnotations; +using Bit.Core.KeyManagement.Models.Api.Request; + +namespace Bit.Api.Auth.Models.Request.Accounts; + +/// +/// We recognize this probably doesn't need the obsolete fields but we are leaving it in to be cleaned up anywho +/// later. +/// +public class ChangeKdfRequestModel : IValidatableObject +{ + [Required] + public required string MasterPasswordHash { get; set; } + [Obsolete("To be removed in PM-33141")] + [StringLength(300)] + public string? NewMasterPasswordHash { get; set; } + [Obsolete("To be removed in PM-33141")] + public string? Key { get; set; } + + // Should be made required in PM-33141 + public MasterPasswordAuthenticationDataRequestModel? AuthenticationData { get; set; } + // Should be made required in PM-33141 + public MasterPasswordUnlockDataRequestModel? UnlockData { get; set; } + + // To be removed in PM-33141 + public IEnumerable Validate(ValidationContext validationContext) + { + var hasNewPayloads = AuthenticationData is not null && UnlockData is not null; + var hasLegacyPayloads = NewMasterPasswordHash is not null && Key is not null; + + if (hasNewPayloads && hasLegacyPayloads) + { + yield return new ValidationResult( + "Cannot provide both new payloads (UnlockData/AuthenticationData) and legacy payloads (NewMasterPasswordHash/Key).", + [nameof(AuthenticationData), nameof(UnlockData), nameof(NewMasterPasswordHash), nameof(Key)]); + } + + if (!hasNewPayloads && !hasLegacyPayloads) + { + yield return new ValidationResult( + "Must provide either new payloads (UnlockData/AuthenticationData) or legacy payloads (NewMasterPasswordHash/Key).", + [nameof(AuthenticationData), nameof(UnlockData), nameof(NewMasterPasswordHash), nameof(Key)]); + } + } +} diff --git a/src/Api/Auth/Models/Request/Accounts/PasswordRequestModel.cs b/src/Api/Auth/Models/Request/Accounts/PasswordRequestModel.cs index ab8c727852dd..d2d993a9f9e7 100644 --- a/src/Api/Auth/Models/Request/Accounts/PasswordRequestModel.cs +++ b/src/Api/Auth/Models/Request/Accounts/PasswordRequestModel.cs @@ -3,17 +3,47 @@ namespace Bit.Api.Auth.Models.Request.Accounts; -public class PasswordRequestModel : SecretVerificationRequestModel +public class PasswordRequestModel : IValidatableObject { [Required] + public required string MasterPasswordHash { get; set; } + [Obsolete("To be removed in PM-33141")] [StringLength(300)] - public required string NewMasterPasswordHash { get; set; } + public string? NewMasterPasswordHash { get; set; } + [Obsolete("To be removed in PM-33141")] + public string? Key { get; set; } [StringLength(50)] public string? MasterPasswordHint { get; set; } - [Required] - public required string Key { get; set; } - // Note: These will eventually become required, but not all consumers are moved over yet. + // Should be made required in PM-33141 public MasterPasswordAuthenticationDataRequestModel? AuthenticationData { get; set; } + // Should be made required in PM-33141 public MasterPasswordUnlockDataRequestModel? UnlockData { get; set; } + + // To be removed in PM-33141 + public bool RequestHasNewDataTypes() + { + return UnlockData is not null && AuthenticationData is not null; + } + + // To be removed in PM-33141 + public IEnumerable Validate(ValidationContext validationContext) + { + var hasNewPayloads = AuthenticationData is not null && UnlockData is not null; + var hasLegacyPayloads = NewMasterPasswordHash is not null && Key is not null; + + if (hasNewPayloads && hasLegacyPayloads) + { + yield return new ValidationResult( + "Cannot provide both new payloads (UnlockData/AuthenticationData) and legacy payloads (NewMasterPasswordHash/Key).", + [nameof(AuthenticationData), nameof(UnlockData), nameof(NewMasterPasswordHash), nameof(Key)]); + } + + if (!hasNewPayloads && !hasLegacyPayloads) + { + yield return new ValidationResult( + "Must provide either new payloads (UnlockData/AuthenticationData) or legacy payloads (NewMasterPasswordHash/Key).", + [nameof(AuthenticationData), nameof(UnlockData), nameof(NewMasterPasswordHash), nameof(Key)]); + } + } } diff --git a/src/Api/Auth/Models/Request/Accounts/SetInitialPasswordRequestModel.cs b/src/Api/Auth/Models/Request/Accounts/SetInitialPasswordRequestModel.cs index 37a7901fee50..80bf70583ac7 100644 --- a/src/Api/Auth/Models/Request/Accounts/SetInitialPasswordRequestModel.cs +++ b/src/Api/Auth/Models/Request/Accounts/SetInitialPasswordRequestModel.cs @@ -58,7 +58,7 @@ public User ToUser(User existingUser) public IEnumerable Validate(ValidationContext validationContext) { - if (IsV2Request()) + if (RequestHasNewDataTypes()) { // V2 registration @@ -134,7 +134,8 @@ public IEnumerable Validate(ValidationContext validationContex } } - public bool IsV2Request() + // To be removed in PM-33141 + public bool RequestHasNewDataTypes() { // AccountKeys can be null for TDE users, so we don't check that here return MasterPasswordAuthentication != null && MasterPasswordUnlock != null; diff --git a/src/Api/Auth/Models/Request/Accounts/UpdateTdeOffboardingPasswordRequestModel.cs b/src/Api/Auth/Models/Request/Accounts/UpdateTdeOffboardingPasswordRequestModel.cs index e99c9907562e..e08996b1de65 100644 --- a/src/Api/Auth/Models/Request/Accounts/UpdateTdeOffboardingPasswordRequestModel.cs +++ b/src/Api/Auth/Models/Request/Accounts/UpdateTdeOffboardingPasswordRequestModel.cs @@ -1,17 +1,45 @@ -// FIXME: Update this file to be null safe and then delete the line below -#nullable disable - -using System.ComponentModel.DataAnnotations; +using System.ComponentModel.DataAnnotations; +using Bit.Core.KeyManagement.Models.Api.Request; namespace Bit.Api.Auth.Models.Request.Accounts; -public class UpdateTdeOffboardingPasswordRequestModel +public class UpdateTdeOffboardingPasswordRequestModel : IValidatableObject { - [Required] + [Obsolete("To be removed in PM-33141")] [StringLength(300)] - public string NewMasterPasswordHash { get; set; } - [Required] - public string Key { get; set; } + public string? NewMasterPasswordHash { get; set; } + [Obsolete("To be removed in PM-33141")] + public string? Key { get; set; } [StringLength(50)] - public string MasterPasswordHint { get; set; } + public string? MasterPasswordHint { get; set; } + + public MasterPasswordAuthenticationDataRequestModel? AuthenticationData { get; set; } + public MasterPasswordUnlockDataRequestModel? UnlockData { get; set; } + + // To be removed in PM-33141 + public bool RequestHasNewDataTypes() + { + return UnlockData is not null && AuthenticationData is not null; + } + + // To be removed in PM-33141 + public IEnumerable Validate(ValidationContext validationContext) + { + var hasNewPayloads = AuthenticationData is not null && UnlockData is not null; + var hasLegacyPayloads = NewMasterPasswordHash is not null && Key is not null; + + if (hasNewPayloads && hasLegacyPayloads) + { + yield return new ValidationResult( + "Cannot provide both new payloads (UnlockData/AuthenticationData) and legacy payloads (NewMasterPasswordHash/Key).", + [nameof(AuthenticationData), nameof(UnlockData), nameof(NewMasterPasswordHash), nameof(Key)]); + } + + if (!hasNewPayloads && !hasLegacyPayloads) + { + yield return new ValidationResult( + "Must provide either new payloads (UnlockData/AuthenticationData) or legacy payloads (NewMasterPasswordHash/Key).", + [nameof(AuthenticationData), nameof(UnlockData), nameof(NewMasterPasswordHash), nameof(Key)]); + } + } } diff --git a/src/Api/Auth/Models/Request/Accounts/UpdateTempPasswordRequestModel.cs b/src/Api/Auth/Models/Request/Accounts/UpdateTempPasswordRequestModel.cs index e071726edf8f..396ebcce630f 100644 --- a/src/Api/Auth/Models/Request/Accounts/UpdateTempPasswordRequestModel.cs +++ b/src/Api/Auth/Models/Request/Accounts/UpdateTempPasswordRequestModel.cs @@ -1,13 +1,45 @@ -// FIXME: Update this file to be null safe and then delete the line below -#nullable disable - -using System.ComponentModel.DataAnnotations; -using Bit.Api.Models.Request.Organizations; +using System.ComponentModel.DataAnnotations; +using Bit.Core.KeyManagement.Models.Api.Request; namespace Bit.Api.Auth.Models.Request.Accounts; -public class UpdateTempPasswordRequestModel : OrganizationUserResetPasswordRequestModel +public class UpdateTempPasswordRequestModel { + [Obsolete("To be removed in PM-33141")] + [StringLength(300)] + public string? NewMasterPasswordHash { get; set; } + [Obsolete("To be removed in PM-33141")] + public string? Key { get; set; } [StringLength(50)] - public string MasterPasswordHint { get; set; } + public string? MasterPasswordHint { get; set; } + + public MasterPasswordUnlockDataRequestModel? UnlockData { get; set; } + public MasterPasswordAuthenticationDataRequestModel? AuthenticationData { get; set; } + + // To be removed in PM-33141 + public bool RequestHasNewDataTypes() + { + return UnlockData is not null && AuthenticationData is not null; + } + + // To be removed in PM-33141 + public IEnumerable Validate(ValidationContext validationContext) + { + var hasNewPayloads = UnlockData is not null && AuthenticationData is not null; + var hasLegacyPayloads = NewMasterPasswordHash is not null && Key is not null; + + if (hasNewPayloads && hasLegacyPayloads) + { + yield return new ValidationResult( + "Cannot provide both new payloads (UnlockData/AuthenticationData) and legacy payloads (NewMasterPasswordHash/Key).", + [nameof(UnlockData), nameof(AuthenticationData), nameof(NewMasterPasswordHash), nameof(Key)]); + } + + if (!hasNewPayloads && !hasLegacyPayloads) + { + yield return new ValidationResult( + "Must provide either new payloads (UnlockData/AuthenticationData) or legacy payloads (NewMasterPasswordHash/Key).", + [nameof(UnlockData), nameof(AuthenticationData), nameof(NewMasterPasswordHash), nameof(Key)]); + } + } } diff --git a/src/Api/Auth/Models/Request/EmergencyAccessRequestModels.cs b/src/Api/Auth/Models/Request/EmergencyAccessRequestModels.cs index 71e90f102acf..dece8cf75138 100644 --- a/src/Api/Auth/Models/Request/EmergencyAccessRequestModels.cs +++ b/src/Api/Auth/Models/Request/EmergencyAccessRequestModels.cs @@ -1,9 +1,7 @@ -// FIXME: Update this file to be null safe and then delete the line below -#nullable disable - -using System.ComponentModel.DataAnnotations; +using System.ComponentModel.DataAnnotations; using Bit.Core.Auth.Entities; using Bit.Core.Auth.Enums; +using Bit.Core.KeyManagement.Models.Api.Request; using Bit.Core.Utilities; namespace Bit.Api.Auth.Models.Request; @@ -13,7 +11,7 @@ public class EmergencyAccessInviteRequestModel [Required] [StrictEmailAddress] [StringLength(256)] - public string Email { get; set; } + public required string Email { get; set; } [Required] public EmergencyAccessType? Type { get; set; } [Required] @@ -28,7 +26,7 @@ public class EmergencyAccessUpdateRequestModel [Required] [Range(1, short.MaxValue)] public int WaitTimeDays { get; set; } - public string KeyEncrypted { get; set; } + public required string KeyEncrypted { get; set; } public EmergencyAccess ToEmergencyAccess(EmergencyAccess existingEmergencyAccess) { @@ -43,13 +41,43 @@ public EmergencyAccess ToEmergencyAccess(EmergencyAccess existingEmergencyAccess } } -public class EmergencyAccessPasswordRequestModel +public class EmergencyAccessPasswordRequestModel : IValidatableObject { - [Required] + [Obsolete("To be removed in PM-33141")] [StringLength(300)] - public string NewMasterPasswordHash { get; set; } - [Required] - public string Key { get; set; } + public string? NewMasterPasswordHash { get; set; } + [Obsolete("To be removed in PM-33141")] + public string? Key { get; set; } + + public MasterPasswordUnlockDataRequestModel? UnlockData { get; set; } + public MasterPasswordAuthenticationDataRequestModel? AuthenticationData { get; set; } + + // To be removed in PM-33141 + public bool RequestHasNewDataTypes() + { + return UnlockData is not null && AuthenticationData is not null; + } + + // To be removed in PM-33141 + public IEnumerable Validate(ValidationContext validationContext) + { + var hasNewPayloads = UnlockData is not null && AuthenticationData is not null; + var hasLegacyPayloads = NewMasterPasswordHash is not null && Key is not null; + + if (hasNewPayloads && hasLegacyPayloads) + { + yield return new ValidationResult( + "Cannot provide both new payloads (UnlockData/AuthenticationData) and legacy payloads (NewMasterPasswordHash/Key).", + [nameof(UnlockData), nameof(AuthenticationData), nameof(NewMasterPasswordHash), nameof(Key)]); + } + + if (!hasNewPayloads && !hasLegacyPayloads) + { + yield return new ValidationResult( + "Must provide either new payloads (UnlockData/AuthenticationData) or legacy payloads (NewMasterPasswordHash/Key).", + [nameof(UnlockData), nameof(AuthenticationData), nameof(NewMasterPasswordHash), nameof(Key)]); + } + } } public class EmergencyAccessWithIdRequestModel : EmergencyAccessUpdateRequestModel diff --git a/src/Api/Models/Request/Organizations/OrganizationUserResetPasswordRequestModel.cs b/src/Api/Models/Request/Organizations/OrganizationUserResetPasswordRequestModel.cs index a257e62ea24a..f56fb4fd40ea 100644 --- a/src/Api/Models/Request/Organizations/OrganizationUserResetPasswordRequestModel.cs +++ b/src/Api/Models/Request/Organizations/OrganizationUserResetPasswordRequestModel.cs @@ -1,25 +1,62 @@ using System.ComponentModel.DataAnnotations; using Bit.Core.AdminConsole.OrganizationFeatures.AccountRecovery.v2; using Bit.Core.Entities; +using Bit.Core.KeyManagement.Models.Api.Request; namespace Bit.Api.Models.Request.Organizations; -public class OrganizationUserResetPasswordRequestModel +public class OrganizationUserResetPasswordRequestModel : IValidatableObject { public bool ResetMasterPassword { get; set; } public bool ResetTwoFactor { get; set; } + [Obsolete("To be removed in PM-33141")] [StringLength(300)] public string? NewMasterPasswordHash { get; set; } + [Obsolete("To be removed in PM-33141")] public string? Key { get; set; } + public MasterPasswordUnlockDataRequestModel? UnlockData { get; set; } + public MasterPasswordAuthenticationDataRequestModel? AuthenticationData { get; set; } + public RecoverAccountRequest ToCommandRequest(Guid orgId, OrganizationUser organizationUser) => new() { OrgId = orgId, OrganizationUser = organizationUser, ResetMasterPassword = ResetMasterPassword, ResetTwoFactor = ResetTwoFactor, + // To be removed in PM-33141 NewMasterPasswordHash = NewMasterPasswordHash, - Key = Key + // To be removed in PM-33141 + Key = Key, + UnlockData = UnlockData, + AuthenticationData = AuthenticationData, }; + + // To be removed in PM-33141 + public bool RequestHasNewDataTypes() + { + return UnlockData is not null && AuthenticationData is not null; + } + + // To be removed in PM-33141 + public IEnumerable Validate(ValidationContext validationContext) + { + var hasNewPayloads = UnlockData is not null && AuthenticationData is not null; + var hasLegacyPayloads = NewMasterPasswordHash is not null && Key is not null; + + if (hasNewPayloads && hasLegacyPayloads) + { + yield return new ValidationResult( + "Cannot provide both new payloads (UnlockData/AuthenticationData) and legacy payloads (NewMasterPasswordHash/Key).", + [nameof(UnlockData), nameof(AuthenticationData), nameof(NewMasterPasswordHash), nameof(Key)]); + } + + if (!hasNewPayloads && !hasLegacyPayloads) + { + yield return new ValidationResult( + "Must provide either new payloads (UnlockData/AuthenticationData) or legacy payloads (NewMasterPasswordHash/Key).", + [nameof(UnlockData), nameof(AuthenticationData), nameof(NewMasterPasswordHash), nameof(Key)]); + } + } } diff --git a/src/Core/AdminConsole/OrganizationFeatures/AccountRecovery/AdminRecoverAccountCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/AccountRecovery/AdminRecoverAccountCommand.cs index fe0c7bc5878a..4b1c230fef4e 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/AccountRecovery/AdminRecoverAccountCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/AccountRecovery/AdminRecoverAccountCommand.cs @@ -1,8 +1,11 @@ using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.OrganizationFeatures.Policies; +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Data; +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; +using Bit.Core.KeyManagement.Models.Data; using Bit.Core.Platform.Push; using Bit.Core.Repositories; using Bit.Core.Services; @@ -10,15 +13,18 @@ namespace Bit.Core.AdminConsole.OrganizationFeatures.AccountRecovery; -public class AdminRecoverAccountCommand(IOrganizationRepository organizationRepository, +public class AdminRecoverAccountCommand( + IOrganizationRepository organizationRepository, IPolicyQuery policyQuery, IUserRepository userRepository, IMailService mailService, IEventService eventService, IPushNotificationService pushNotificationService, IUserService userService, - TimeProvider timeProvider) : IAdminRecoverAccountCommand + TimeProvider timeProvider, + IMasterPasswordService masterPasswordService) : IAdminRecoverAccountCommand { + [Obsolete("Will be replaced with the below function once we transition the endpoint to fully using the unlock and authentication data. To be removed in PM-33141")] public async Task RecoverAccountAsync(Guid orgId, OrganizationUser organizationUser, string newMasterPassword, string key) { @@ -75,4 +81,87 @@ public async Task RecoverAccountAsync(Guid orgId, return IdentityResult.Success; } + + public async Task RecoverAccountAsync( + Guid orgId, + OrganizationUser organizationUser, + MasterPasswordUnlockData unlockData, + MasterPasswordAuthenticationData authenticationData) + { + // Org must be able to use reset password + var org = await organizationRepository.GetByIdAsync(orgId); + if (org == null || !org.UseResetPassword) + { + throw new BadRequestException("Organization does not allow password reset."); + } + + // Enterprise policy must be enabled + var resetPasswordPolicy = await policyQuery.RunAsync(orgId, PolicyType.ResetPassword); + if (!resetPasswordPolicy.Enabled) + { + throw new BadRequestException("Organization does not have the password reset policy enabled."); + } + + // Org User must be confirmed and have a ResetPasswordKey + if (organizationUser == null || + organizationUser.Status != OrganizationUserStatusType.Confirmed || + organizationUser.OrganizationId != orgId || + !organizationUser.IsEnrolledInAccountRecovery() || + !organizationUser.UserId.HasValue) + { + throw new BadRequestException("Organization User not valid"); + } + + var user = await userService.GetUserByIdAsync(organizationUser.UserId.Value); + if (user == null) + { + throw new NotFoundException(); + } + + if (user.UsesKeyConnector) + { + throw new BadRequestException("Cannot reset password of a user with Key Connector."); + } + + IdentityResult mutationResult; + + // We can recover an account for users who both have a master password and + // those who do not. TDE users can be recovered and will not have a password + if (user.HasMasterPassword()) + { + mutationResult = await masterPasswordService.MutateUpdateExistingMasterPasswordAsync( + user, + new UpdateExistingPasswordData + { + MasterPasswordUnlock = unlockData, + MasterPasswordAuthentication = authenticationData, + }); + } + else + { + mutationResult = await masterPasswordService.MutateSetInitialMasterPasswordAsync( + user, + new SetInitialPasswordData + { + MasterPasswordUnlock = unlockData, + MasterPasswordAuthentication = authenticationData, + }); + } + + if (!mutationResult.Succeeded) + { + return mutationResult; + } + + // Extra modifications for this particular scenario to the user object + user.ForcePasswordReset = true; + + await userRepository.ReplaceAsync(user); + + await mailService.SendAdminResetPasswordEmailAsync(user.Email, user.Name, org.DisplayName(), true, false); + await eventService.LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_AdminResetPassword); + await pushNotificationService.PushLogOutAsync(user.Id); + + return IdentityResult.Success; + } } diff --git a/src/Core/AdminConsole/OrganizationFeatures/AccountRecovery/IAdminRecoverAccountCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/AccountRecovery/IAdminRecoverAccountCommand.cs index 75babc643edf..27b87bb227bf 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/AccountRecovery/IAdminRecoverAccountCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/AccountRecovery/IAdminRecoverAccountCommand.cs @@ -1,5 +1,6 @@ using Bit.Core.Entities; using Bit.Core.Exceptions; +using Bit.Core.KeyManagement.Models.Data; using Microsoft.AspNetCore.Identity; namespace Bit.Core.AdminConsole.OrganizationFeatures.AccountRecovery; @@ -19,6 +20,20 @@ public interface IAdminRecoverAccountCommand /// An IdentityResult indicating success or failure. /// When organization settings, policy, or user state is invalid. /// When the user does not exist. + [Obsolete("Will be replaced with the below function once we transition the endpoint to fully using the unlock and authentication data. To be removed in PM-33141")] Task RecoverAccountAsync(Guid orgId, OrganizationUser organizationUser, string newMasterPassword, string key); + + /// + /// Recovers an organization user's account by resetting their master password. + /// + /// The organization the user belongs to. + /// The organization user being recovered. + /// The user's new master-password unlock data. + /// The user's new master-password authentication data. + /// An IdentityResult indicating success or failure. + /// When organization settings, policy, or user state is invalid. + /// When the user does not exist. + Task RecoverAccountAsync(Guid orgId, OrganizationUser organizationUser, + MasterPasswordUnlockData unlockData, MasterPasswordAuthenticationData authenticationData); } diff --git a/src/Core/AdminConsole/OrganizationFeatures/AccountRecovery/v2/AdminRecoverAccountCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/AccountRecovery/v2/AdminRecoverAccountCommand.cs index 90ba8e7b8734..e387843caba8 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/AccountRecovery/v2/AdminRecoverAccountCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/AccountRecovery/v2/AdminRecoverAccountCommand.cs @@ -5,6 +5,8 @@ using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; using Bit.Core.AdminConsole.Utilities.v2.Results; using Bit.Core.Auth.UserFeatures.TwoFactorAuth; +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Data; +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Models.Data.Organizations.OrganizationUsers; @@ -23,6 +25,7 @@ public class AdminRecoverAccountCommand( IEventService eventService, IPushNotificationService pushNotificationService, IUserService userService, + IMasterPasswordService masterPasswordService, IResetUserTwoFactorCommand resetUserTwoFactorCommand, IPolicyRequirementQuery policyRequirementQuery, IRevokeNonCompliantOrganizationUserCommand revokeNonCompliantOrganizationUserCommand, @@ -52,20 +55,24 @@ public async Task RecoverAccountAsync(RecoverAccountRequest reque // Password reset if (request.ResetMasterPassword) { - var result = await userService.UpdatePasswordHash(user, request.NewMasterPasswordHash!); - if (!result.Succeeded) + // Unwind this with PM-33141 to only use the new payload + if (request.RequestHasNewDataTypes()) { - var errorMessage = string.Join(", ", result.Errors.Select(e => e.Description)); - return new PasswordUpdateFailedError(errorMessage); + var result = await HandlePayloadsWithUnlockAndAuthenticationDataAsync(user, request); + if (result.IsError) + { + return result; + } + } + // To be removed in PM-33141 + else + { + var result = await HandlePayloadWithDeprecatedRawDataAsync(user, request); + if (result is { IsSuccess: false }) + { + return result; + } } - - var now = timeProvider.GetUtcNow().UtcDateTime; - user.RevisionDate = user.AccountRevisionDate = now; - user.LastPasswordChangeDate = now; - user.ForcePasswordReset = true; - user.Key = request.Key; - - await userRepository.ReplaceAsync(user); } // 2FA reset @@ -131,4 +138,54 @@ await revokeNonCompliantOrganizationUserCommand.RevokeNonCompliantOrganizationUs await Task.WhenAll(revokeOrgUserTasks); } + + /// + /// TODO: Change this function to ResetMasterPassword during PM-33141 + /// + private async Task HandlePayloadsWithUnlockAndAuthenticationDataAsync(User user, RecoverAccountRequest request) + { + // We can recover an account for users who both have a master password and + // those who do not. TDE users can be account recovered which will not have + // an initial master password set. + var identityResultFromMutation = await masterPasswordService.MutateSetInitialOrUpdateExistingMasterPasswordAsync( + user, + new SetInitialOrUpdateExistingPasswordData + { + MasterPasswordUnlock = request.UnlockData!.ToData(), + MasterPasswordAuthentication = request.AuthenticationData!.ToData(), + }); + + if (!identityResultFromMutation.Succeeded) + { + var errorMessage = string.Join(", ", identityResultFromMutation.Errors.Select(e => e.Description)); + return new PasswordUpdateFailedError(errorMessage); + } + + // When we are recovering an account we want to force a password reset on the user. + user.ForcePasswordReset = true; + + await userRepository.ReplaceAsync(user); + + return new None(); + } + + [Obsolete("To be removed in PM-33141")] + private async Task HandlePayloadWithDeprecatedRawDataAsync(User user, RecoverAccountRequest request) + { + var result = await userService.UpdatePasswordHash(user, request.NewMasterPasswordHash!); + if (!result.Succeeded) + { + var errorMessage = string.Join(", ", result.Errors.Select(e => e.Description)); + return new PasswordUpdateFailedError(errorMessage); + } + + var now = timeProvider.GetUtcNow().UtcDateTime; + user.RevisionDate = user.AccountRevisionDate = now; + user.LastPasswordChangeDate = now; + user.ForcePasswordReset = true; + user.Key = request.Key; + + await userRepository.ReplaceAsync(user); + return null; + } } diff --git a/src/Core/AdminConsole/OrganizationFeatures/AccountRecovery/v2/RecoverAccountRequest.cs b/src/Core/AdminConsole/OrganizationFeatures/AccountRecovery/v2/RecoverAccountRequest.cs index db817e47acb4..2035e66ae7cf 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/AccountRecovery/v2/RecoverAccountRequest.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/AccountRecovery/v2/RecoverAccountRequest.cs @@ -1,4 +1,5 @@ using Bit.Core.Entities; +using Bit.Core.KeyManagement.Models.Api.Request; namespace Bit.Core.AdminConsole.OrganizationFeatures.AccountRecovery.v2; @@ -8,6 +9,18 @@ public record RecoverAccountRequest public required OrganizationUser OrganizationUser { get; init; } public required bool ResetMasterPassword { get; init; } public required bool ResetTwoFactor { get; init; } + + public MasterPasswordUnlockDataRequestModel? UnlockData; + public MasterPasswordAuthenticationDataRequestModel? AuthenticationData; + + [Obsolete("To be removed in PM-33141")] public string? NewMasterPasswordHash { get; init; } + [Obsolete("To be removed in PM-33141")] public string? Key { get; init; } + + // To be removed in PM-33141 + public bool RequestHasNewDataTypes() + { + return UnlockData is not null && AuthenticationData is not null; + } } diff --git a/src/Core/Auth/Models/Data/SetInitialMasterPasswordDataModel.cs b/src/Core/Auth/Models/Data/SetInitialMasterPasswordDataModel.cs index 82bcb3da5ebd..fcfc7449c8de 100644 --- a/src/Core/Auth/Models/Data/SetInitialMasterPasswordDataModel.cs +++ b/src/Core/Auth/Models/Data/SetInitialMasterPasswordDataModel.cs @@ -1,4 +1,5 @@ -using Bit.Core.KeyManagement.Models.Data; +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Data; +using Bit.Core.KeyManagement.Models.Data; namespace Bit.Core.Auth.Models.Data; @@ -20,4 +21,11 @@ public class SetInitialMasterPasswordDataModel /// public required UserAccountKeysData? AccountKeys { get; set; } public string? MasterPasswordHint { get; set; } + + public SetInitialPasswordData ToSetInitialPasswordData() => new SetInitialPasswordData + { + MasterPasswordAuthentication = MasterPasswordAuthentication, + MasterPasswordUnlock = MasterPasswordUnlock, + MasterPasswordHint = MasterPasswordHint + }; } diff --git a/src/Core/Auth/UserFeatures/EmergencyAccess/EmergencyAccessService.cs b/src/Core/Auth/UserFeatures/EmergencyAccess/EmergencyAccessService.cs index f596dc471b62..b08766d1fc13 100644 --- a/src/Core/Auth/UserFeatures/EmergencyAccess/EmergencyAccessService.cs +++ b/src/Core/Auth/UserFeatures/EmergencyAccess/EmergencyAccessService.cs @@ -9,9 +9,12 @@ using Bit.Core.Auth.Enums; using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Auth.Models.Data; +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Data; +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; +using Bit.Core.KeyManagement.Models.Data; using Bit.Core.Repositories; using Bit.Core.Services; using Bit.Core.Settings; @@ -19,6 +22,7 @@ using Bit.Core.Vault.Models.Data; using Bit.Core.Vault.Repositories; using Bit.Core.Vault.Services; +using Microsoft.AspNetCore.Identity; namespace Bit.Core.Auth.UserFeatures.EmergencyAccess; @@ -32,6 +36,7 @@ public class EmergencyAccessService : IEmergencyAccessService private readonly ICipherService _cipherService; private readonly IMailService _mailService; private readonly IUserService _userService; + private readonly IMasterPasswordService _masterPasswordService; private readonly GlobalSettings _globalSettings; private readonly IDataProtectorTokenFactory _dataProtectorTokenizer; private readonly IRemoveOrganizationUserCommand _removeOrganizationUserCommand; @@ -46,6 +51,7 @@ public EmergencyAccessService( ICipherService cipherService, IMailService mailService, IUserService userService, + IMasterPasswordService masterPasswordService, GlobalSettings globalSettings, IDataProtectorTokenFactory dataProtectorTokenizer, IRemoveOrganizationUserCommand removeOrganizationUserCommand, @@ -59,6 +65,7 @@ public EmergencyAccessService( _cipherService = cipherService; _mailService = mailService; _userService = userService; + _masterPasswordService = masterPasswordService; _globalSettings = globalSettings; _dataProtectorTokenizer = dataProtectorTokenizer; _removeOrganizationUserCommand = removeOrganizationUserCommand; @@ -357,8 +364,8 @@ public async Task> GetPoliciesAsync(Guid emergencyAccessId, return (emergencyAccess, grantor); } - // TODO PM-21687: rename this to something like FinishRecoveryTakeoverAsync - public async Task PasswordAsync(Guid emergencyAccessId, User granteeUser, string newMasterPasswordHash, string key) + [Obsolete("To be removed in PM-33141")] + public async Task FinishRecoveryTakeoverAsync(Guid emergencyAccessId, User granteeUser, string newMasterPasswordHash, string key) { var emergencyAccess = await _emergencyAccessRepository.GetByIdAsync(emergencyAccessId); @@ -390,6 +397,60 @@ public async Task PasswordAsync(Guid emergencyAccessId, User granteeUser, string } } + public async Task FinishRecoveryTakeoverAsync( + Guid emergencyAccessId, + User granteeUser, + MasterPasswordUnlockData unlockData, + MasterPasswordAuthenticationData authenticationData) + { + var emergencyAccess = await _emergencyAccessRepository.GetByIdAsync(emergencyAccessId); + + if (!IsValidRequest(emergencyAccess, granteeUser, EmergencyAccessType.Takeover)) + { + throw new BadRequestException("Emergency Access not valid."); + } + + var grantor = await _userRepository.GetByIdAsync(emergencyAccess.GrantorId); + + if (grantor == null) + { + throw new BadRequestException("Grantor not found when trying to finish recovery takeover."); + } + + var identityResult = await _masterPasswordService.MutateSetInitialOrUpdateExistingMasterPasswordAsync( + user: grantor, + new SetInitialOrUpdateExistingPasswordData + { + MasterPasswordUnlock = unlockData, + MasterPasswordAuthentication = authenticationData, + }); + + if (!identityResult.Succeeded) + { + return identityResult; + } + + // Side effects that we still need to run when performing emergency access. + + // Disable TwoFactor providers since they will otherwise block logins + grantor.SetTwoFactorProviders([]); + // Disable New Device Verification since it will otherwise block logins + grantor.VerifyDevices = false; + + await _userRepository.ReplaceAsync(grantor); + + // Remove grantor from all organizations unless Owner + var orgUser = await _organizationUserRepository.GetManyByUserAsync(grantor.Id); + foreach (var o in orgUser) + { + if (o.Type != OrganizationUserType.Owner) + { + await _removeOrganizationUserCommand.RemoveUserAsync(o.OrganizationId, grantor.Id); + } + } + return IdentityResult.Success; + } + public async Task SendNotificationsAsync() { var toNotify = await _emergencyAccessRepository.GetManyToNotifyAsync(); diff --git a/src/Core/Auth/UserFeatures/EmergencyAccess/IEmergencyAccessService.cs b/src/Core/Auth/UserFeatures/EmergencyAccess/IEmergencyAccessService.cs index bfd725ac955f..4032642d9937 100644 --- a/src/Core/Auth/UserFeatures/EmergencyAccess/IEmergencyAccessService.cs +++ b/src/Core/Auth/UserFeatures/EmergencyAccess/IEmergencyAccessService.cs @@ -3,8 +3,10 @@ using Bit.Core.Auth.Models.Data; using Bit.Core.Entities; using Bit.Core.Enums; +using Bit.Core.KeyManagement.Models.Data; using Bit.Core.Services; using Bit.Core.Vault.Models.Data; +using Microsoft.AspNetCore.Identity; namespace Bit.Core.Auth.UserFeatures.EmergencyAccess; @@ -117,7 +119,17 @@ public interface IEmergencyAccessService /// new password hash set by grantee user /// new encrypted user key /// void - Task PasswordAsync(Guid emergencyAccessId, User granteeUser, string newMasterPasswordHash, string key); + [Obsolete("Deprecated because we are switching to use unlock and authentication data types. To be removed in PM-33141")] + Task FinishRecoveryTakeoverAsync(Guid emergencyAccessId, User granteeUser, string newMasterPasswordHash, string key); + /// + /// Updates the grantor's password hash and updates the key for the EmergencyAccess entity using the + /// + /// Emergency Access Id being acted on + /// user making the request + /// + /// + /// + Task FinishRecoveryTakeoverAsync(Guid emergencyAccessId, User granteeUser, MasterPasswordUnlockData unlockData, MasterPasswordAuthenticationData authenticationData); /// /// sends a reminder email that there is a pending request for recovery. /// diff --git a/src/Core/Auth/UserFeatures/TdeOffboardingPassword/Interfaces/ITdeOffboardingPasswordCommand.cs b/src/Core/Auth/UserFeatures/TdeOffboardingPassword/Interfaces/ITdeOffboardingPasswordCommand.cs index 1ff64ffabb79..babe62a4d76c 100644 --- a/src/Core/Auth/UserFeatures/TdeOffboardingPassword/Interfaces/ITdeOffboardingPasswordCommand.cs +++ b/src/Core/Auth/UserFeatures/TdeOffboardingPassword/Interfaces/ITdeOffboardingPasswordCommand.cs @@ -1,4 +1,5 @@ using Bit.Core.Entities; +using Bit.Core.KeyManagement.Models.Data; using Microsoft.AspNetCore.Identity; namespace Bit.Core.Auth.UserFeatures.TdeOffboardingPassword.Interfaces; @@ -9,6 +10,10 @@ namespace Bit.Core.Auth.UserFeatures.TdeOffboardingPassword.Interfaces; /// public interface ITdeOffboardingPasswordCommand { + [Obsolete("To be removed in PM-33141")] public Task UpdateTdeOffboardingPasswordAsync(User user, string masterPassword, string key, - string orgSsoIdentifier); + string? masterPasswordHint); + + public Task UpdateTdeOffboardingPasswordAsync(User user, MasterPasswordUnlockData unlockData, + MasterPasswordAuthenticationData authenticationData, string? masterPasswordHint); } diff --git a/src/Core/Auth/UserFeatures/TdeOffboardingPassword/TdeOffboardingPasswordCommand.cs b/src/Core/Auth/UserFeatures/TdeOffboardingPassword/TdeOffboardingPasswordCommand.cs index 719ff9ce9dcc..f85784da41c8 100644 --- a/src/Core/Auth/UserFeatures/TdeOffboardingPassword/TdeOffboardingPasswordCommand.cs +++ b/src/Core/Auth/UserFeatures/TdeOffboardingPassword/TdeOffboardingPasswordCommand.cs @@ -1,8 +1,11 @@ using Bit.Core.Auth.Repositories; using Bit.Core.Auth.UserFeatures.TdeOffboardingPassword.Interfaces; +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Data; +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; +using Bit.Core.KeyManagement.Models.Data; using Bit.Core.Platform.Push; using Bit.Core.Repositories; using Bit.Core.Services; @@ -14,6 +17,7 @@ public class TdeOffboardingPasswordCommand : ITdeOffboardingPasswordCommand { private readonly IUserService _userService; private readonly IUserRepository _userRepository; + private readonly IMasterPasswordService _masterPasswordService; private readonly IEventService _eventService; private readonly IOrganizationUserRepository _organizationUserRepository; private readonly ISsoUserRepository _ssoUserRepository; @@ -24,6 +28,7 @@ public class TdeOffboardingPasswordCommand : ITdeOffboardingPasswordCommand public TdeOffboardingPasswordCommand( IUserService userService, IUserRepository userRepository, + IMasterPasswordService masterPasswordService, IEventService eventService, IOrganizationUserRepository organizationUserRepository, ISsoUserRepository ssoUserRepository, @@ -32,6 +37,7 @@ public TdeOffboardingPasswordCommand( { _userService = userService; _userRepository = userRepository; + _masterPasswordService = masterPasswordService; _eventService = eventService; _organizationUserRepository = organizationUserRepository; _ssoUserRepository = ssoUserRepository; @@ -39,7 +45,8 @@ public TdeOffboardingPasswordCommand( _pushService = pushService; } - public async Task UpdateTdeOffboardingPasswordAsync(User user, string newMasterPassword, string key, string hint) + [Obsolete("To be removed in PM-33141")] + public async Task UpdateTdeOffboardingPasswordAsync(User user, string newMasterPassword, string key, string? hint) { if (string.IsNullOrWhiteSpace(newMasterPassword)) { @@ -97,4 +104,57 @@ public async Task UpdateTdeOffboardingPasswordAsync(User user, s return IdentityResult.Success; } + public async Task UpdateTdeOffboardingPasswordAsync( + User user, + MasterPasswordUnlockData unlockData, + MasterPasswordAuthenticationData authenticationData, + string? masterPasswordHint) + { + var orgUserDetails = await _organizationUserRepository.GetManyDetailsByUserAsync(user.Id); + orgUserDetails = orgUserDetails.Where(x => x.UseSso).ToList(); + if (orgUserDetails.Count == 0) + { + throw new BadRequestException("User is not part of any organization that has SSO enabled."); + } + + var orgSsoUsers = await Task.WhenAll(orgUserDetails.Select(async x => await _ssoUserRepository.GetByUserIdOrganizationIdAsync(x.OrganizationId, user.Id))); + if (orgSsoUsers.Length != 1) + { + throw new BadRequestException("User is part of no or multiple SSO configurations."); + } + + var orgUser = orgUserDetails.First(); + var orgSsoConfig = await _ssoConfigRepository.GetByOrganizationIdAsync(orgUser.OrganizationId); + if (orgSsoConfig == null) + { + throw new BadRequestException("Organization SSO configuration not found."); + } + + if (orgSsoConfig.GetData().MemberDecryptionType != Enums.MemberDecryptionType.MasterPassword) + { + throw new BadRequestException("Organization SSO Member Decryption Type is not Master Password."); + } + + // We only want to be setting an initial master password here, if they already have one, + // we are in an error state. + var identityResult = await _masterPasswordService.MutateSetInitialMasterPasswordAsync(user, new SetInitialPasswordData + { + MasterPasswordUnlock = unlockData, + MasterPasswordAuthentication = authenticationData, + MasterPasswordHint = masterPasswordHint + }); + if (!identityResult.Succeeded) + { + return identityResult; + } + + // Side effect of running TDE offboarding, we want to force reset + user.ForcePasswordReset = false; + + await _userRepository.ReplaceAsync(user); + await _eventService.LogUserEventAsync(user.Id, EventType.User_TdeOffboardingPasswordSet); + await _pushService.PushLogOutAsync(user.Id); + + return IdentityResult.Success; + } } diff --git a/src/Core/Auth/UserFeatures/TempPassword/Interfaces/IReplaceAdminSetTemporaryPasswordCommand.cs b/src/Core/Auth/UserFeatures/TempPassword/Interfaces/IReplaceAdminSetTemporaryPasswordCommand.cs new file mode 100644 index 000000000000..1a92c10204d5 --- /dev/null +++ b/src/Core/Auth/UserFeatures/TempPassword/Interfaces/IReplaceAdminSetTemporaryPasswordCommand.cs @@ -0,0 +1,14 @@ +using Bit.Core.Entities; +using Bit.Core.KeyManagement.Models.Data; +using Microsoft.AspNetCore.Identity; + +namespace Bit.Core.Auth.UserFeatures.TempPassword.Interfaces; + +public interface IReplaceAdminSetTemporaryPasswordCommand +{ + Task Replace( + User user, + MasterPasswordUnlockData unlockData, + MasterPasswordAuthenticationData authenticationData, + string? masterPasswordHint); +} diff --git a/src/Core/Auth/UserFeatures/TempPassword/ReplaceAdminSetTemporaryPasswordCommand.cs b/src/Core/Auth/UserFeatures/TempPassword/ReplaceAdminSetTemporaryPasswordCommand.cs new file mode 100644 index 000000000000..0c15206438d9 --- /dev/null +++ b/src/Core/Auth/UserFeatures/TempPassword/ReplaceAdminSetTemporaryPasswordCommand.cs @@ -0,0 +1,56 @@ +using Bit.Core.Auth.UserFeatures.TempPassword.Interfaces; +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Data; +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Exceptions; +using Bit.Core.KeyManagement.Models.Data; +using Bit.Core.Platform.Push; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Microsoft.AspNetCore.Identity; + +namespace Bit.Core.Auth.UserFeatures.TempPassword; + +/// +/// Write tests for this. +/// +public class ReplaceAdminSetTemporaryPasswordCommand( + IMasterPasswordService masterPasswordService, + IUserRepository userRepository, + IMailService mailService, + IEventService eventService, + IPushNotificationService pushService) : IReplaceAdminSetTemporaryPasswordCommand +{ + public async Task Replace( + User user, + MasterPasswordUnlockData unlockData, + MasterPasswordAuthenticationData authenticationData, + string? masterPasswordHint) + { + if (!user.ForcePasswordReset) + { + throw new BadRequestException("User does not have a temporary password to update."); + } + + var result = await masterPasswordService.MutateUpdateExistingMasterPasswordAsync(user, new UpdateExistingPasswordData + { + MasterPasswordUnlock = unlockData, + MasterPasswordAuthentication = authenticationData, + MasterPasswordHint = masterPasswordHint, + }); + if (!result.Succeeded) + { + return result; + } + + user.ForcePasswordReset = false; + + await userRepository.ReplaceAsync(user); + await mailService.SendUpdatedTempPasswordEmailAsync(user.Email, user.Name ?? string.Empty); + await eventService.LogUserEventAsync(user.Id, EventType.User_UpdatedTempPassword); + await pushService.PushLogOutAsync(user.Id); + + return IdentityResult.Success; + } +} 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..68442e29329a --- /dev/null +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/Data/SetInitialPasswordData.cs @@ -0,0 +1,58 @@ +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."); + } + } +} 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..ffae89d09bb2 --- /dev/null +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordAndKdfData.cs @@ -0,0 +1,49 @@ +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."); + } + + // DAVE double check me here that a key connector user cannot rotate kdf. + 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..5ed64afec880 --- /dev/null +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/Data/UpdateExistingPasswordData.cs @@ -0,0 +1,52 @@ +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."); + } + + // DAVE investigate if this is correct for owners and admins, can owners and admins update existing passwords + // within the context of a key connector organization. + 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/FinishSsoJitProvisionMasterPasswordCommand.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/FinishSsoJitProvisionMasterPasswordCommand.cs index 87c2157f984a..d48e6a1e1be8 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/FinishSsoJitProvisionMasterPasswordCommand.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/FinishSsoJitProvisionMasterPasswordCommand.cs @@ -6,7 +6,6 @@ using Bit.Core.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.Repositories; using Bit.Core.Services; -using Microsoft.AspNetCore.Identity; namespace Bit.Core.Auth.UserFeatures.UserMasterPassword; @@ -14,23 +13,27 @@ public class FinishSsoJitProvisionMasterPasswordCommand : IFinishSsoJitProvision { private readonly IUserService _userService; private readonly IUserRepository _userRepository; + private readonly IMasterPasswordService _masterPasswordService; private readonly IAcceptOrgUserCommand _acceptOrgUserCommand; private readonly IOrganizationUserRepository _organizationUserRepository; private readonly IOrganizationRepository _organizationRepository; - private readonly IPasswordHasher _passwordHasher; private readonly IEventService _eventService; - public FinishSsoJitProvisionMasterPasswordCommand(IUserService userService, IUserRepository userRepository, - IAcceptOrgUserCommand acceptOrgUserCommand, IOrganizationUserRepository organizationUserRepository, - IOrganizationRepository organizationRepository, IPasswordHasher passwordHasher, + public FinishSsoJitProvisionMasterPasswordCommand( + IUserService userService, + IUserRepository userRepository, + IMasterPasswordService masterPasswordService, + IAcceptOrgUserCommand acceptOrgUserCommand, + IOrganizationUserRepository organizationUserRepository, + IOrganizationRepository organizationRepository, IEventService eventService) { _userService = userService; _userRepository = userRepository; + _masterPasswordService = masterPasswordService; _acceptOrgUserCommand = acceptOrgUserCommand; _organizationUserRepository = organizationUserRepository; _organizationRepository = organizationRepository; - _passwordHasher = passwordHasher; _eventService = eventService; } @@ -63,15 +66,13 @@ public async Task FinishProvisionAsync(User user, throw new BadRequestException("User not found within organization."); } - // Hash the provided user master password authentication hash on the server side - var serverSideHashedMasterPasswordAuthenticationHash = _passwordHasher.HashPassword(user, - masterPasswordDataModel.MasterPasswordAuthentication.MasterPasswordAuthenticationHash); + var updateUserData = + _masterPasswordService.BuildUpdateUserDelegateSetInitialMasterPassword( + user, + masterPasswordDataModel.ToSetInitialPasswordData()); - var setMasterPasswordTask = _userRepository.SetMasterPassword(user.Id, - masterPasswordDataModel.MasterPasswordUnlock, serverSideHashedMasterPasswordAuthenticationHash, - masterPasswordDataModel.MasterPasswordHint); await _userRepository.SetV2AccountCryptographicStateAsync(user.Id, masterPasswordDataModel.AccountKeys, - [setMasterPasswordTask]); + [updateUserData]); await _eventService.LogUserEventAsync(user.Id, EventType.User_ChangedPassword); 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..6bf3a40a9bb6 --- /dev/null +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/IMasterPasswordService.cs @@ -0,0 +1,170 @@ +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Data; +using Bit.Core.Entities; +using Bit.Core.Repositories; +using Microsoft.AspNetCore.Identity; + +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 + /// . + /// + /// + /// if the mutation succeeded; a failure result + /// containing validation errors if ValidatePassword is set and the password + /// fails the registered pipeline. + /// + Task MutateSetInitialOrUpdateExistingMasterPasswordAsync(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. + /// + /// + /// if the mutation succeeded; a failure result + /// containing validation errors if ValidatePassword is set and the password + /// fails the registered pipeline. + /// + Task MutateSetInitialMasterPasswordAsync(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. + /// + /// + /// if the mutation and save succeeded; a failure result + /// containing validation errors if ValidatePassword is set and the password + /// fails the registered pipeline. + /// + 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. + /// + /// Note: despite the Async suffix, this method is synchronous — it constructs and returns + /// the delegate without performing any I/O. + /// + /// + /// + /// 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. Will not update a master password salt. 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. + /// + /// + /// if the mutation succeeded; a failure result + /// containing validation errors if ValidatePassword is set and the password + /// fails the registered pipeline. + /// + Task MutateUpdateExistingMasterPasswordAsync(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. + /// + /// + /// if the mutation and save succeeded; a failure result + /// containing validation errors if ValidatePassword is set and the password + /// fails the registered pipeline. + /// + Task SaveUpdateExistingMasterPasswordAsync(User user, UpdateExistingPasswordData updateExistingData); +} diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/ISelfServicePasswordChangeCommand.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/ISelfServicePasswordChangeCommand.cs new file mode 100644 index 000000000000..1275cde77647 --- /dev/null +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/ISelfServicePasswordChangeCommand.cs @@ -0,0 +1,15 @@ +using Bit.Core.Entities; +using Bit.Core.KeyManagement.Models.Data; +using Microsoft.AspNetCore.Identity; + +namespace Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; + +public interface ISelfServicePasswordChangeCommand +{ + Task ChangePasswordAsync( + User user, + string masterPasswordHash, + MasterPasswordUnlockData unlockData, + MasterPasswordAuthenticationData authenticationData, + string? masterPasswordHint); +} diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs new file mode 100644 index 000000000000..dcd963ec41e5 --- /dev/null +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/MasterPasswordService.cs @@ -0,0 +1,245 @@ +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; + +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 MutateSetInitialOrUpdateExistingMasterPasswordAsync( + User user, + SetInitialOrUpdateExistingPasswordData setOrUpdatePasswordData) + { + IdentityResult mutationResult; + if (user.HasMasterPassword()) + { + mutationResult = await MutateUpdateExistingMasterPasswordAsync( + user, + setOrUpdatePasswordData.ToUpdateExistingData()); + } + else + { + mutationResult = await MutateSetInitialMasterPasswordAsync( + user, + setOrUpdatePasswordData.ToSetInitialData()); + } + + return mutationResult; + } + + public async Task MutateSetInitialMasterPasswordAsync( + User user, + SetInitialPasswordData setInitialData) + { + setInitialData.ValidateDataForUser(user); + + var result = await UpdateExistingPasswordHashAsync( + user, + setInitialData.MasterPasswordAuthentication.MasterPasswordAuthenticationHash, + setInitialData.ValidatePassword, + setInitialData.RefreshStamp); + if (!result.Succeeded) + { + return result; + } + + // 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 IdentityResult.Success; + } + + public async Task SaveSetInitialMasterPasswordAsync( + User user, + SetInitialPasswordData setInitialData) + { + // No need to validate because we will validate in the sibling call here + var result = await MutateSetInitialMasterPasswordAsync(user, setInitialData); + if (!result.Succeeded) + { + return result; + } + + await _userRepository.ReplaceAsync(user); + + return IdentityResult.Success; + } + + public UpdateUserData BuildUpdateUserDelegateSetInitialMasterPassword( + User user, + SetInitialPasswordData setInitialData) + { + 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 MutateUpdateExistingMasterPasswordAsync( + User user, + UpdateExistingPasswordData updateExistingData) + { + // Start by validating the update payload + updateExistingData.ValidateDataForUser(user); + + var result = await UpdateExistingPasswordHashAsync( + user, + updateExistingData.MasterPasswordAuthentication.MasterPasswordAuthenticationHash, + updateExistingData.ValidatePassword, + updateExistingData.RefreshStamp); + + if (!result.Succeeded) + { + return result; + } + + 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 IdentityResult.Success; + } + + public async Task SaveUpdateExistingMasterPasswordAndKdfAsync( + User user, + UpdateExistingPasswordAndKdfData updateExistingExistingData) + { + // Start by validating the update payload + updateExistingExistingData.ValidateDataForUser(user); + + var result = await UpdateExistingPasswordHashAsync( + user, + updateExistingExistingData.MasterPasswordAuthentication.MasterPasswordAuthenticationHash, + updateExistingExistingData.ValidatePassword, + updateExistingExistingData.RefreshStamp); + + if (!result.Succeeded) + { + return result; + } + + 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 IdentityResult.Success; + } + + public async Task SaveUpdateExistingMasterPasswordAsync( + User user, + UpdateExistingPasswordData updateExistingData) + { + // No need to validate because we will validate in the sibling call here. + var result = await MutateUpdateExistingMasterPasswordAsync(user, updateExistingData); + if (!result.Succeeded) + { + return result; + } + + await _userRepository.ReplaceAsync(user); + + return IdentityResult.Success; + } + + 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; + } + + 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; + } +} diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/SelfServicePasswordChangeCommand.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/SelfServicePasswordChangeCommand.cs new file mode 100644 index 000000000000..8faf5cf71d10 --- /dev/null +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/SelfServicePasswordChangeCommand.cs @@ -0,0 +1,34 @@ +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.Services; +using Microsoft.AspNetCore.Identity; + +namespace Bit.Core.Auth.UserFeatures.UserMasterPassword; + +public class SelfServicePasswordChangeCommand( + IUserService userService, + IMasterPasswordService masterPasswordService, + IdentityErrorDescriber identityErrorDescriber) : ISelfServicePasswordChangeCommand +{ + public async Task ChangePasswordAsync( + User user, + string masterPasswordHash, + MasterPasswordUnlockData unlockData, + MasterPasswordAuthenticationData authenticationData, + string? masterPasswordHint) + { + if (!await userService.CheckPasswordAsync(user, masterPasswordHash)) + { + return IdentityResult.Failed(identityErrorDescriber.PasswordMismatch()); + } + + return await masterPasswordService.SaveUpdateExistingMasterPasswordAsync(user, new UpdateExistingPasswordData + { + MasterPasswordUnlock = unlockData, + MasterPasswordAuthentication = authenticationData, + MasterPasswordHint = masterPasswordHint + }); + } +} diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommandV1.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommandV1.cs index df5f0d02f7b3..cea36702a4f8 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommandV1.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommandV1.cs @@ -42,6 +42,7 @@ public SetInitialMasterPasswordCommandV1( _organizationRepository = organizationRepository; } + [Obsolete("To be removed in PM-33141")] public async Task SetInitialMasterPasswordAsync(User user, string masterPassword, string key, string orgSsoIdentifier) { diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/TdeSetPasswordCommand.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/TdeSetPasswordCommand.cs index afd28e95d93e..add87e2c4e3e 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/TdeSetPasswordCommand.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/TdeSetPasswordCommand.cs @@ -1,49 +1,45 @@ using Bit.Core.Auth.Models.Data; +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Data; using Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.Repositories; using Bit.Core.Services; -using Microsoft.AspNetCore.Identity; namespace Bit.Core.Auth.UserFeatures.UserMasterPassword; public class TdeSetPasswordCommand : ITdeSetPasswordCommand { private readonly IUserRepository _userRepository; + private readonly IMasterPasswordService _masterPasswordService; private readonly IOrganizationUserRepository _organizationUserRepository; private readonly IOrganizationRepository _organizationRepository; - private readonly IPasswordHasher _passwordHasher; private readonly IEventService _eventService; - public TdeSetPasswordCommand(IUserRepository userRepository, - IOrganizationUserRepository organizationUserRepository, IOrganizationRepository organizationRepository, - IPasswordHasher passwordHasher, IEventService eventService) + public TdeSetPasswordCommand( + IUserRepository userRepository, + IMasterPasswordService masterPasswordService, + IOrganizationUserRepository organizationUserRepository, + IOrganizationRepository organizationRepository, + IEventService eventService) { _userRepository = userRepository; + _masterPasswordService = masterPasswordService; _organizationUserRepository = organizationUserRepository; _organizationRepository = organizationRepository; - _passwordHasher = passwordHasher; _eventService = eventService; } public async Task SetMasterPasswordAsync(User user, SetInitialMasterPasswordDataModel masterPasswordDataModel) { - if (user.Key != null) - { - throw new BadRequestException("User already has a master password set."); - } - + // TDE scenario specific check if (user.PublicKey == null || user.PrivateKey == null) { throw new BadRequestException("TDE user account keys must be set before setting initial master password."); } - // Prevent a de-synced salt value from creating an un-decryptable unlock method - masterPasswordDataModel.MasterPasswordAuthentication.ValidateSaltUnchangedForUser(user); - masterPasswordDataModel.MasterPasswordUnlock.ValidateSaltUnchangedForUser(user); - + // Does this need to be here? Why is this here? var org = await _organizationRepository.GetByIdentifierAsync(masterPasswordDataModel.OrgSsoIdentifier); if (org == null) { @@ -56,13 +52,14 @@ public async Task SetMasterPasswordAsync(User user, SetInitialMasterPasswordData throw new BadRequestException("User not found within organization."); } - // Hash the provided user master password authentication hash on the server side - var serverSideHashedMasterPasswordAuthenticationHash = _passwordHasher.HashPassword(user, - masterPasswordDataModel.MasterPasswordAuthentication.MasterPasswordAuthenticationHash); + var setMasterPasswordTask = _masterPasswordService.BuildUpdateUserDelegateSetInitialMasterPassword(user, + new SetInitialPasswordData + { + MasterPasswordUnlock = masterPasswordDataModel.MasterPasswordUnlock, + MasterPasswordAuthentication = masterPasswordDataModel.MasterPasswordAuthentication, + MasterPasswordHint = masterPasswordDataModel.MasterPasswordHint, + }); - var setMasterPasswordTask = _userRepository.SetMasterPassword(user.Id, - masterPasswordDataModel.MasterPasswordUnlock, serverSideHashedMasterPasswordAuthenticationHash, - masterPasswordDataModel.MasterPasswordHint); await _userRepository.UpdateUserDataAsync([setMasterPasswordTask]); await _eventService.LogUserEventAsync(user.Id, EventType.User_ChangedPassword); diff --git a/src/Core/Auth/UserFeatures/UserServiceCollectionExtensions.cs b/src/Core/Auth/UserFeatures/UserServiceCollectionExtensions.cs index 3c671f9726bd..c8d8fb115fa7 100644 --- a/src/Core/Auth/UserFeatures/UserServiceCollectionExtensions.cs +++ b/src/Core/Auth/UserFeatures/UserServiceCollectionExtensions.cs @@ -5,6 +5,8 @@ using Bit.Core.Auth.UserFeatures.Registration; using Bit.Core.Auth.UserFeatures.Registration.Implementations; using Bit.Core.Auth.UserFeatures.TdeOffboardingPassword.Interfaces; +using Bit.Core.Auth.UserFeatures.TempPassword; +using Bit.Core.Auth.UserFeatures.TempPassword.Interfaces; using Bit.Core.Auth.UserFeatures.TwoFactorAuth; using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Implementations; using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; @@ -28,6 +30,7 @@ public static void AddUserServices(this IServiceCollection services, IGlobalSett services.AddDeviceTrustCommands(); services.AddEmergencyAccessCommands(); services.AddUserPasswordCommands(); + services.AddUpdateTempPasswordCommands(); services.AddUserRegistrationCommands(); services.AddWebAuthnLoginCommands(); services.AddTdeOffboardingPasswordCommands(); @@ -55,6 +58,8 @@ private static void AddUserPasswordCommands(this IServiceCollection services) services.AddScoped(); services.AddScoped(); services.AddScoped(); + services.AddScoped(); + services.AddScoped(); } private static void AddTdeOffboardingPasswordCommands(this IServiceCollection services) @@ -62,6 +67,11 @@ private static void AddTdeOffboardingPasswordCommands(this IServiceCollection se services.AddScoped(); } + private static void AddUpdateTempPasswordCommands(this IServiceCollection services) + { + services.AddScoped(); + } + private static void AddUserRegistrationCommands(this IServiceCollection services) { services.AddScoped(); diff --git a/src/Core/KeyManagement/Kdf/Implementations/ChangeKdfCommand.cs b/src/Core/KeyManagement/Kdf/Implementations/ChangeKdfCommand.cs index 83e47c4931d0..073f95250225 100644 --- a/src/Core/KeyManagement/Kdf/Implementations/ChangeKdfCommand.cs +++ b/src/Core/KeyManagement/Kdf/Implementations/ChangeKdfCommand.cs @@ -1,4 +1,6 @@ -using Bit.Core.Entities; +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Data; +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; +using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.KeyManagement.Models.Data; @@ -17,17 +19,19 @@ public class ChangeKdfCommand : IChangeKdfCommand private readonly IUserService _userService; private readonly IPushNotificationService _pushService; private readonly IUserRepository _userRepository; + private readonly IMasterPasswordService _masterPasswordService; private readonly IdentityErrorDescriber _identityErrorDescriber; private readonly ILogger _logger; private readonly IFeatureService _featureService; public ChangeKdfCommand(IUserService userService, IPushNotificationService pushService, - IUserRepository userRepository, IdentityErrorDescriber describer, ILogger logger, - IFeatureService featureService) + IUserRepository userRepository, IMasterPasswordService masterPasswordService, IdentityErrorDescriber describer, + ILogger logger, IFeatureService featureService) { _userService = userService; _pushService = pushService; _userRepository = userRepository; + _masterPasswordService = masterPasswordService; _identityErrorDescriber = describer; _logger = logger; _featureService = featureService; @@ -42,12 +46,6 @@ public async Task ChangeKdfAsync(User user, string masterPasswor return IdentityResult.Failed(_identityErrorDescriber.PasswordMismatch()); } - // Validate to prevent user account from becoming un-decryptable from invalid parameters - // - // Prevent a de-synced salt value from creating an un-decryptable unlock method - authenticationData.ValidateSaltUnchangedForUser(user); - unlockData.ValidateSaltUnchangedForUser(user); - // Currently KDF settings are not saved separately for authentication and unlock and must therefore be equal if (!authenticationData.Kdf.Equals(unlockData.Kdf)) { @@ -62,42 +60,20 @@ public async Task ChangeKdfAsync(User user, string masterPasswor var logoutOnKdfChange = !_featureService.IsEnabled(FeatureFlagKeys.NoLogoutOnKdfChange); - // Update the user with the new KDF settings - // This updates the authentication data and unlock data for the user separately. Currently these still - // use shared values for KDF settings and salt. - // The authentication hash, and the unlock data each are dependent on: - // - The master password (entered by the user every time) - // - The KDF settings (iterations, memory, parallelism) - // - The salt - // These combinations - (password, authentication hash, KDF settings, salt) and (password, unlock data, KDF settings, salt) - // must remain consistent to unlock correctly. + var updateExisingPasswordResult = await _masterPasswordService.SaveUpdateExistingMasterPasswordAndKdfAsync(user, + new UpdateExistingPasswordAndKdfData + { + MasterPasswordUnlock = unlockData, + MasterPasswordAuthentication = authenticationData, + RefreshStamp = logoutOnKdfChange + }); - // Authentication - // Note: This mutates the user but does not yet save it to DB. That is done atomically, later. - // This entire operation MUST be atomic to prevent a user from being locked out of their account. - // Salt is ensured to be the same as unlock data, and the value stored in the account and not updated. - // KDF is ensured to be the same as unlock data above and updated below. - var result = await _userService.UpdatePasswordHash(user, authenticationData.MasterPasswordAuthenticationHash, - refreshStamp: logoutOnKdfChange); - if (!result.Succeeded) + if (!updateExisingPasswordResult.Succeeded) { _logger.LogWarning("Change KDF failed for user {userId}.", user.Id); - return result; + return updateExisingPasswordResult; } - // Salt is ensured to be the same as authentication data, and the value stored in the account, and is not updated. - // Kdf - These will be seperated in the future, but for now are ensured to be the same as authentication data above. - user.Key = unlockData.MasterKeyWrappedUserKey; - user.Kdf = unlockData.Kdf.KdfType; - user.KdfIterations = unlockData.Kdf.Iterations; - user.KdfMemory = unlockData.Kdf.Memory; - user.KdfParallelism = unlockData.Kdf.Parallelism; - - var now = DateTime.UtcNow; - user.RevisionDate = user.AccountRevisionDate = now; - user.LastKdfChangeDate = now; - - await _userRepository.ReplaceAsync(user); if (logoutOnKdfChange) { await _pushService.PushLogOutAsync(user.Id); diff --git a/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountKeysCommand.cs b/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountKeysCommand.cs index 7acb6b01e9d6..52c666e6f120 100644 --- a/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountKeysCommand.cs +++ b/src/Core/KeyManagement/UserKey/Implementations/RotateUserAccountKeysCommand.cs @@ -72,6 +72,7 @@ public RotateUserAccountKeysCommand(IUserService userService, IUserRepository us _userSignatureKeyPairRepository = userSignatureKeyPairRepository; } + /// TODO: Need to handle this one too. /// public async Task PasswordChangeAndRotateUserAccountKeysAsync(User user, PasswordChangeAndRotateUserAccountKeysData model) { diff --git a/src/Core/Services/IUserService.cs b/src/Core/Services/IUserService.cs index c021fa2668e2..76396a179281 100644 --- a/src/Core/Services/IUserService.cs +++ b/src/Core/Services/IUserService.cs @@ -28,12 +28,14 @@ public interface IUserService Task InitiateEmailChangeAsync(User user, string newEmail); Task ChangeEmailAsync(User user, string masterPassword, string newEmail, string newMasterPassword, string token, string key); + [Obsolete("To be removed in PM-33141")] Task ChangePasswordAsync(User user, string masterPassword, string newMasterPassword, string passwordHint, string key); // TODO removed with https://bitwarden.atlassian.net/browse/PM-27328 [Obsolete("Use ISetKeyConnectorKeyCommand instead. This method will be removed in a future version.")] Task SetKeyConnectorKeyAsync(User user, string key, string orgIdentifier); Task ConvertToKeyConnectorAsync(User user, string keyConnectorKeyWrappedUserKey); Task AdminResetPasswordAsync(OrganizationUserType type, Guid orgId, Guid id, string newMasterPassword, string key); + [Obsolete("To be removed in PM-33141")] Task UpdateTempPasswordAsync(User user, string newMasterPassword, string key, string hint); Task RefreshSecurityStampAsync(User user, string masterPasswordHash); Task UpdateTwoFactorProviderAsync(User user, TwoFactorProviderType type, bool setEnabled = true, bool logEvent = true); @@ -50,6 +52,7 @@ Task ChangeEmailAsync(User user, string masterPassword, string n Task UpdatePremiumExpirationAsync(Guid userId, DateTime? expirationDate); Task GenerateLicenseAsync(User user, SubscriptionInfo subscriptionInfo = null, int? version = null); + // TODO: Evaluate moving to the new master password service PM-<> Task CheckPasswordAsync(User user, string password); /// /// Checks if the user has access to premium features, either through a personal subscription or through an organization. @@ -72,6 +75,7 @@ Task GenerateLicenseAsync(User user, SubscriptionInfo subscriptionI Task HasPremiumFromOrganization(User user); Task GenerateSignInTokenAsync(User user, string purpose); + [Obsolete("Migrating to the MasterPasswordService for updating a password hash.", true)] Task UpdatePasswordHash(User user, string newPassword, bool validatePassword = true, bool refreshStamp = true); Task RotateApiKeyAsync(User user); diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index c1a4b078379e..0b71278075a6 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -481,6 +481,7 @@ public async Task ValidateClaimedUserDomainAsync(User user, stri }); } + [Obsolete("To be removed in PM-33141")] public async Task ChangePasswordAsync(User user, string masterPassword, string newMasterPassword, string passwordHint, string key) { @@ -657,6 +658,7 @@ public async Task AdminResetPasswordAsync(OrganizationUserType c return IdentityResult.Success; } + [Obsolete("To be removed in PM-33141")] public async Task UpdateTempPasswordAsync(User user, string newMasterPassword, string key, string hint) { if (!user.ForcePasswordReset) @@ -1027,6 +1029,7 @@ public static bool IsLegacyUser(User user) return user.Key == null && user.MasterPassword != null && user.PrivateKey != null; } + [Obsolete("Migrating to the MasterPasswordService")] private async Task ValidatePasswordInternal(User user, string password) { var errors = new List(); diff --git a/src/Infrastructure.Dapper/Repositories/UserRepository.cs b/src/Infrastructure.Dapper/Repositories/UserRepository.cs index eaeaf5f1805a..1ca83f8c0fa2 100644 --- a/src/Infrastructure.Dapper/Repositories/UserRepository.cs +++ b/src/Infrastructure.Dapper/Repositories/UserRepository.cs @@ -487,6 +487,7 @@ public UpdateUserData SetMasterPassword(Guid userId, MasterPasswordUnlockData ma KdfParallelism = masterPasswordUnlockData.Kdf.Parallelism, RevisionDate = timestamp, AccountRevisionDate = timestamp, + // Need to add User.LastPasswordChangeDate here in PM-34905 MasterPasswordSalt = masterPasswordUnlockData.Salt }, transaction: transaction, diff --git a/src/Infrastructure.EntityFramework/Repositories/UserRepository.cs b/src/Infrastructure.EntityFramework/Repositories/UserRepository.cs index b003d997bac1..028165f79e42 100644 --- a/src/Infrastructure.EntityFramework/Repositories/UserRepository.cs +++ b/src/Infrastructure.EntityFramework/Repositories/UserRepository.cs @@ -562,6 +562,7 @@ public UpdateUserData SetMasterPassword(Guid userId, MasterPasswordUnlockData ma userEntity.KdfParallelism = masterPasswordUnlockData.Kdf.Parallelism; userEntity.RevisionDate = timestamp; userEntity.AccountRevisionDate = timestamp; + // userEntity.LastPasswordChangeDate = timestamp; This needs adding in PM-34905 userEntity.MasterPasswordSalt = masterPasswordUnlockData.Salt; await dbContext.SaveChangesAsync(); }; diff --git a/test/Api.IntegrationTest/Controllers/AccountsControllerTest.cs b/test/Api.IntegrationTest/Controllers/AccountsControllerTest.cs index 24d738ae18a6..c14876275a2f 100644 --- a/test/Api.IntegrationTest/Controllers/AccountsControllerTest.cs +++ b/test/Api.IntegrationTest/Controllers/AccountsControllerTest.cs @@ -242,6 +242,19 @@ public async Task PostKdf_AuthenticationDataOrUnlockDataNull_BadRequest(bool aut var response = await PostKdfAsync(authenticationData, unlockData); + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + var content = await response.Content.ReadAsStringAsync(); + Assert.Contains("Must provide either new payloads (UnlockData/AuthenticationData) or legacy payloads (NewMasterPasswordHash/Key).", content); + } + + [Obsolete("To be removed in PM-33141")] + [Fact] + public async Task Legacy_PostKdf_AuthenticationDataOrUnlockDataNull_BadRequest() + { + await _loginHelper.LoginAsync(_ownerEmail); + + var response = await PostKdfLegacyAsync(); + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); var content = await response.Content.ReadAsStringAsync(); Assert.Contains("AuthenticationData and UnlockData must be provided.", content); @@ -266,11 +279,9 @@ public async Task PostKdf_InvalidMasterPasswordHash_BadRequest() Salt = _ownerEmail }; - var requestModel = new PasswordRequestModel + var requestModel = new ChangeKdfRequestModel { MasterPasswordHash = "wrong-master-password-hash", - NewMasterPasswordHash = _newMasterPasswordHash, - Key = _masterKeyWrappedUserKey, AuthenticationData = authenticationData, UnlockData = unlockData }; @@ -284,6 +295,28 @@ public async Task PostKdf_InvalidMasterPasswordHash_BadRequest() Assert.Contains("Incorrect password", content); } + [Obsolete("To be removed in PM-33141")] + [Fact] + public async Task Legacy_PostKdf_InvalidMasterPasswordHash_BadRequest() + { + await _loginHelper.LoginAsync(_ownerEmail); + + var requestModel = new PasswordRequestModel + { + MasterPasswordHash = "wrong-master-password-hash", + NewMasterPasswordHash = _newMasterPasswordHash, + Key = _masterKeyWrappedUserKey + }; + + using var message = new HttpRequestMessage(HttpMethod.Post, "/accounts/kdf"); + message.Content = JsonContent.Create(requestModel); + var response = await _client.SendAsync(message); + + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + var content = await response.Content.ReadAsStringAsync(); + Assert.Contains("AuthenticationData and UnlockData must be provided.", content); + } + [Fact] public async Task PostKdf_ChangedSaltInAuthenticationData_BadRequest() { @@ -310,6 +343,19 @@ public async Task PostKdf_ChangedSaltInAuthenticationData_BadRequest() Assert.Contains("Invalid master password salt.", content); } + [Obsolete("To be removed in PM-33141")] + [Fact] + public async Task Legacy_PostKdf_ChangedSaltInAuthenticationData_BadRequest() + { + await _loginHelper.LoginAsync(_ownerEmail); + + var response = await PostKdfLegacyAsync(); + + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + var content = await response.Content.ReadAsStringAsync(); + Assert.Contains("AuthenticationData and UnlockData must be provided.", content); + } + [Fact] public async Task PostKdf_ChangedSaltInUnlockData_BadRequest() { @@ -336,6 +382,19 @@ public async Task PostKdf_ChangedSaltInUnlockData_BadRequest() Assert.Contains("Invalid master password salt.", content); } + [Obsolete("To be removed in PM-33141")] + [Fact] + public async Task Legacy_PostKdf_ChangedSaltInUnlockData_BadRequest() + { + await _loginHelper.LoginAsync(_ownerEmail); + + var response = await PostKdfLegacyAsync(); + + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + var content = await response.Content.ReadAsStringAsync(); + Assert.Contains("AuthenticationData and UnlockData must be provided.", content); + } + [Fact] public async Task PostKdf_KdfNotMatching_BadRequest() { @@ -362,6 +421,19 @@ public async Task PostKdf_KdfNotMatching_BadRequest() Assert.Contains("KDF settings must be equal for authentication and unlock.", content); } + [Obsolete("To be removed in PM-33141")] + [Fact] + public async Task Legacy_PostKdf_KdfNotMatching_BadRequest() + { + await _loginHelper.LoginAsync(_ownerEmail); + + var response = await PostKdfLegacyAsync(); + + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + var content = await response.Content.ReadAsStringAsync(); + Assert.Contains("AuthenticationData and UnlockData must be provided.", content); + } + [Theory] [InlineData(KdfType.PBKDF2_SHA256, 1, null, null)] [InlineData(KdfType.Argon2id, 4, null, 5)] @@ -406,11 +478,9 @@ public async Task PostKdf_InvalidNewMasterPassword_BadRequest() Salt = _ownerEmail }; - var requestModel = new PasswordRequestModel + var requestModel = new ChangeKdfRequestModel { MasterPasswordHash = _masterPasswordHash, - NewMasterPasswordHash = newMasterPasswordHash, - Key = _masterKeyWrappedUserKey, AuthenticationData = authenticationData, UnlockData = unlockData }; @@ -424,6 +494,30 @@ public async Task PostKdf_InvalidNewMasterPassword_BadRequest() Assert.Contains("Passwords must be at least", content); } + [Obsolete("To be removed in PM-33141")] + [Fact] + public async Task Legacy_PostKdf_InvalidNewMasterPassword_BadRequest() + { + var newMasterPasswordHash = "too-short"; + + await _loginHelper.LoginAsync(_ownerEmail); + + var requestModel = new PasswordRequestModel + { + MasterPasswordHash = _masterPasswordHash, + NewMasterPasswordHash = newMasterPasswordHash, + Key = _masterKeyWrappedUserKey + }; + + using var message = new HttpRequestMessage(HttpMethod.Post, "/accounts/kdf"); + message.Content = JsonContent.Create(requestModel); + var response = await _client.SendAsync(message); + + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + var content = await response.Content.ReadAsStringAsync(); + Assert.Contains("AuthenticationData and UnlockData must be provided.", content); + } + private async Task PostKdfWithKdfRequestAsync(KdfRequestModel kdfRequest) { var authenticationData = new MasterPasswordAuthenticationDataRequestModel @@ -447,11 +541,9 @@ private async Task PostKdfAsync( MasterPasswordAuthenticationDataRequestModel? authenticationDataRequest, MasterPasswordUnlockDataRequestModel? unlockDataRequest) { - var requestModel = new PasswordRequestModel + var requestModel = new ChangeKdfRequestModel { MasterPasswordHash = _masterPasswordHash, - NewMasterPasswordHash = _newMasterPasswordHash, - Key = _masterKeyWrappedUserKey, AuthenticationData = authenticationDataRequest, UnlockData = unlockDataRequest }; @@ -461,6 +553,21 @@ private async Task PostKdfAsync( return await _client.SendAsync(message); } + [Obsolete("To be removed in PM-33141")] + private async Task PostKdfLegacyAsync() + { + var requestModel = new PasswordRequestModel + { + MasterPasswordHash = _masterPasswordHash, + NewMasterPasswordHash = _newMasterPasswordHash, + Key = _masterKeyWrappedUserKey + }; + + using var message = new HttpRequestMessage(HttpMethod.Post, "/accounts/kdf"); + message.Content = JsonContent.Create(requestModel); + return await _client.SendAsync(message); + } + [Theory] [BitAutoData] public async Task PostSetPasswordAsync_V1_MasterPasswordDecryption_Success(string organizationSsoIdentifier) diff --git a/test/Api.Test/AdminConsole/Controllers/OrganizationUsersControllerTests.cs b/test/Api.Test/AdminConsole/Controllers/OrganizationUsersControllerTests.cs index 539428a78269..e4fb35afe2d6 100644 --- a/test/Api.Test/AdminConsole/Controllers/OrganizationUsersControllerTests.cs +++ b/test/Api.Test/AdminConsole/Controllers/OrganizationUsersControllerTests.cs @@ -20,6 +20,7 @@ using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; +using Bit.Core.KeyManagement.Models.Data; using Bit.Core.Models.Api; using Bit.Core.Models.Business; using Bit.Core.Models.Data; @@ -570,14 +571,14 @@ public async Task PutRecoverAccount_FlagOff_WhenRecoverAccountSucceeds_ReturnsOk .IsEnabled(FeatureFlagKeys.AdminResetTwoFactor) .Returns(false); sutProvider.GetDependency() - .RecoverAccountAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .RecoverAccountAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) .Returns(Microsoft.AspNetCore.Identity.IdentityResult.Success); var result = await sutProvider.Sut.PutRecoverAccount(orgId, orgUserId, model); Assert.IsType(result); await sutProvider.GetDependency().Received(1) - .RecoverAccountAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()); + .RecoverAccountAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()); } [Theory] @@ -598,7 +599,7 @@ public async Task PutRecoverAccount_FlagOff_WhenRecoverAccountFails_ReturnsBadRe .IsEnabled(FeatureFlagKeys.AdminResetTwoFactor) .Returns(false); sutProvider.GetDependency() - .RecoverAccountAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .RecoverAccountAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) .Returns(Microsoft.AspNetCore.Identity.IdentityResult.Failed(new Microsoft.AspNetCore.Identity.IdentityError { Description = "Error message" })); var result = await sutProvider.Sut.PutRecoverAccount(orgId, orgUserId, model); diff --git a/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs b/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs index 259fca3332bd..fbb08cfcc993 100644 --- a/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs +++ b/test/Api.Test/Auth/Controllers/AccountsControllerTests.cs @@ -7,6 +7,7 @@ using Bit.Core.Auth.Models.Data; using Bit.Core.Auth.Services; using Bit.Core.Auth.UserFeatures.TdeOffboardingPassword.Interfaces; +using Bit.Core.Auth.UserFeatures.TempPassword.Interfaces; using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; using Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; using Bit.Core.Entities; @@ -37,9 +38,10 @@ public class AccountsControllerTests : IDisposable private readonly IFinishSsoJitProvisionMasterPasswordCommand _finishSsoJitProvisionMasterPasswordCommand; private readonly ISetInitialMasterPasswordCommandV1 _setInitialMasterPasswordCommandV1; private readonly ITwoFactorIsEnabledQuery _twoFactorIsEnabledQuery; + private readonly ISelfServicePasswordChangeCommand _selfServicePasswordChangeCommand; private readonly ITdeSetPasswordCommand _tdeSetPasswordCommand; private readonly ITdeOffboardingPasswordCommand _tdeOffboardingPasswordCommand; - private readonly IFeatureService _featureService; + private readonly IReplaceAdminSetTemporaryPasswordCommand _replaceAdminSetTemporaryPasswordCommand; private readonly IUserAccountKeysQuery _userAccountKeysQuery; private readonly ITwoFactorEmailService _twoFactorEmailService; private readonly IChangeKdfCommand _changeKdfCommand; @@ -56,8 +58,9 @@ public AccountsControllerTests() _setInitialMasterPasswordCommandV1 = Substitute.For(); _twoFactorIsEnabledQuery = Substitute.For(); _tdeSetPasswordCommand = Substitute.For(); + _selfServicePasswordChangeCommand = Substitute.For(); _tdeOffboardingPasswordCommand = Substitute.For(); - _featureService = Substitute.For(); + _replaceAdminSetTemporaryPasswordCommand = Substitute.For(); _userAccountKeysQuery = Substitute.For(); _twoFactorEmailService = Substitute.For(); _changeKdfCommand = Substitute.For(); @@ -68,13 +71,14 @@ public AccountsControllerTests() _organizationUserRepository, _providerUserRepository, _userService, + _selfServicePasswordChangeCommand, _policyService, _finishSsoJitProvisionMasterPasswordCommand, _setInitialMasterPasswordCommandV1, _tdeSetPasswordCommand, _tdeOffboardingPasswordCommand, + _replaceAdminSetTemporaryPasswordCommand, _twoFactorIsEnabledQuery, - _featureService, _userAccountKeysQuery, _twoFactorEmailService, _changeKdfCommand, @@ -639,7 +643,7 @@ public async Task ResendNewDeviceVerificationEmail_WhenTokenValid_SendsEmail(Use [Theory] [BitAutoData] - public async Task PostKdf_UserNotFound_ShouldFail(PasswordRequestModel model) + public async Task PostKdf_UserNotFound_ShouldFail(ChangeKdfRequestModel model) { _userService.GetUserByPrincipalAsync(Arg.Any()).Returns(Task.FromResult(null)); @@ -650,7 +654,7 @@ public async Task PostKdf_UserNotFound_ShouldFail(PasswordRequestModel model) [Theory] [BitAutoData] public async Task PostKdf_WithNullAuthenticationData_ShouldFail( - User user, PasswordRequestModel model) + User user, ChangeKdfRequestModel model) { _userService.GetUserByPrincipalAsync(Arg.Any()).Returns(Task.FromResult(user)); model.AuthenticationData = null; @@ -664,7 +668,7 @@ public async Task PostKdf_WithNullAuthenticationData_ShouldFail( [Theory] [BitAutoData] public async Task PostKdf_WithNullUnlockData_ShouldFail( - User user, PasswordRequestModel model) + User user, ChangeKdfRequestModel model) { _userService.GetUserByPrincipalAsync(Arg.Any()).Returns(Task.FromResult(user)); model.UnlockData = null; @@ -678,7 +682,7 @@ public async Task PostKdf_WithNullUnlockData_ShouldFail( [Theory] [BitAutoData] public async Task PostKdf_ChangeKdfFailed_ShouldFail( - User user, PasswordRequestModel model) + User user, ChangeKdfRequestModel model) { _userService.GetUserByPrincipalAsync(Arg.Any()).Returns(Task.FromResult(user)); _changeKdfCommand.ChangeKdfAsync(Arg.Any(), Arg.Any(), @@ -696,7 +700,7 @@ public async Task PostKdf_ChangeKdfFailed_ShouldFail( [Theory] [BitAutoData] public async Task PostKdf_ChangeKdfSuccess_NoError( - User user, PasswordRequestModel model) + User user, ChangeKdfRequestModel model) { _userService.GetUserByPrincipalAsync(Arg.Any()).Returns(Task.FromResult(user)); _changeKdfCommand.ChangeKdfAsync(Arg.Any(), Arg.Any(), diff --git a/test/Api.Test/Auth/Models/Request/Accounts/SetInitialPasswordRequestModelTests.cs b/test/Api.Test/Auth/Models/Request/Accounts/SetInitialPasswordRequestModelTests.cs index 97e69dacbca4..39d24f11b0a5 100644 --- a/test/Api.Test/Auth/Models/Request/Accounts/SetInitialPasswordRequestModelTests.cs +++ b/test/Api.Test/Auth/Models/Request/Accounts/SetInitialPasswordRequestModelTests.cs @@ -343,7 +343,7 @@ public void IsV2Request_WithV2Properties_ReturnsTrue(string orgIdentifier) }; // Act - var result = model.IsV2Request(); + var result = model.RequestHasNewDataTypes(); // Assert Assert.True(result); @@ -370,7 +370,7 @@ public void IsV2Request_WithoutMasterPasswordAuthentication_ReturnsFalse(string }; // Act - var result = model.IsV2Request(); + var result = model.RequestHasNewDataTypes(); // Assert Assert.False(result); @@ -397,7 +397,7 @@ public void IsV2Request_WithoutMasterPasswordUnlock_ReturnsFalse(string orgIdent }; // Act - var result = model.IsV2Request(); + var result = model.RequestHasNewDataTypes(); // Assert Assert.False(result); @@ -418,7 +418,7 @@ public void IsV2Request_WithV1Properties_ReturnsFalse(string orgIdentifier) }; // Act - var result = model.IsV2Request(); + var result = model.RequestHasNewDataTypes(); // Assert Assert.False(result); diff --git a/test/Api.Test/Auth/Models/Request/EmergencyAccessRequestModelsTests.cs b/test/Api.Test/Auth/Models/Request/EmergencyAccessRequestModelsTests.cs index e57bec545567..d9f1dc49b39b 100644 --- a/test/Api.Test/Auth/Models/Request/EmergencyAccessRequestModelsTests.cs +++ b/test/Api.Test/Auth/Models/Request/EmergencyAccessRequestModelsTests.cs @@ -61,6 +61,7 @@ public void Validate_WaitTimeDays_BelowMinimum_Invalid(int waitTimeDays) { Type = EmergencyAccessType.View, WaitTimeDays = waitTimeDays, + KeyEncrypted = "", }; var result = Validate(model); Assert.Contains(result, r => r.MemberNames.Contains("WaitTimeDays")); @@ -77,6 +78,7 @@ public void Validate_WaitTimeDays_ValidRange_Valid(int waitTimeDays) { Type = EmergencyAccessType.View, WaitTimeDays = waitTimeDays, + KeyEncrypted = "", }; var result = Validate(model); Assert.DoesNotContain(result, r => r.MemberNames.Contains("WaitTimeDays")); @@ -128,6 +130,7 @@ public void ToEmergencyAccess_AlwaysUpdatesTypeAndWaitTimeDays() { Type = EmergencyAccessType.Takeover, WaitTimeDays = 14, + KeyEncrypted = "", }; var existing = new EmergencyAccess { diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/AccountRecovery/AdminRecoverAccountCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/AccountRecovery/AdminRecoverAccountCommandTests.cs index 14a1e80d7ce8..271dd5360b4b 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/AccountRecovery/AdminRecoverAccountCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/AccountRecovery/AdminRecoverAccountCommandTests.cs @@ -4,9 +4,12 @@ using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; using Bit.Core.AdminConsole.OrganizationFeatures.AccountRecovery; using Bit.Core.AdminConsole.OrganizationFeatures.Policies; +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Data; +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; +using Bit.Core.KeyManagement.Models.Data; using Bit.Core.Platform.Push; using Bit.Core.Repositories; using Bit.Core.Services; @@ -14,7 +17,6 @@ using Bit.Core.Test.AutoFixture.OrganizationUserFixtures; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; -using Microsoft.AspNetCore.Identity; using NSubstitute; using Xunit; @@ -25,9 +27,9 @@ public class AdminRecoverAccountCommandTests { [Theory] [BitAutoData] - public async Task RecoverAccountAsync_Success( - string newMasterPassword, - string key, + public async Task RecoverAccountAsync_UserHasMasterPassword_CallsUpdate( + MasterPasswordUnlockData unlockData, + MasterPasswordAuthenticationData authenticationData, Organization organization, OrganizationUser organizationUser, User user, @@ -38,23 +40,63 @@ public async Task RecoverAccountAsync_Success( SetupValidOrganization(sutProvider, organization); SetupValidPolicy(sutProvider, organization, policy); SetupValidOrganizationUser(organizationUser, organization.Id); - SetupValidUser(sutProvider, user, organizationUser); - SetupSuccessfulPasswordUpdate(sutProvider, user, newMasterPassword); + SetupValidUser(sutProvider, user, organizationUser, hasMasterPassword: true); // Act - var result = await sutProvider.Sut.RecoverAccountAsync(organization.Id, organizationUser, newMasterPassword, key); + var result = await sutProvider.Sut.RecoverAccountAsync(organization.Id, organizationUser, unlockData, authenticationData); // Assert Assert.True(result.Succeeded); - await AssertSuccessAsync(sutProvider, user, key, organization, organizationUser); + await sutProvider.GetDependency().Received(1) + .MutateUpdateExistingMasterPasswordAsync( + Arg.Any(), + Arg.Is(d => + d.MasterPasswordUnlock == unlockData && + d.MasterPasswordAuthentication == authenticationData)); + await sutProvider.GetDependency().DidNotReceive() + .MutateSetInitialMasterPasswordAsync(Arg.Any(), Arg.Any()); + await AssertCommonSuccessSideEffectsAsync(sutProvider, user, organization, organizationUser); + } + + [Theory] + [BitAutoData] + public async Task RecoverAccountAsync_UserHasNoMasterPassword_CallsSetInitial( + MasterPasswordUnlockData unlockData, + MasterPasswordAuthenticationData authenticationData, + Organization organization, + OrganizationUser organizationUser, + User user, + [Policy(PolicyType.ResetPassword, true)] PolicyStatus policy, + SutProvider sutProvider) + { + // Arrange + SetupValidOrganization(sutProvider, organization); + SetupValidPolicy(sutProvider, organization, policy); + SetupValidOrganizationUser(organizationUser, organization.Id); + SetupValidUser(sutProvider, user, organizationUser, hasMasterPassword: false); + + // Act + var result = await sutProvider.Sut.RecoverAccountAsync(organization.Id, organizationUser, unlockData, authenticationData); + + // Assert + Assert.True(result.Succeeded); + await sutProvider.GetDependency().Received(1) + .MutateSetInitialMasterPasswordAsync( + Arg.Any(), + Arg.Is(d => + d.MasterPasswordUnlock == unlockData && + d.MasterPasswordAuthentication == authenticationData)); + await sutProvider.GetDependency().DidNotReceive() + .MutateUpdateExistingMasterPasswordAsync(Arg.Any(), Arg.Any()); + await AssertCommonSuccessSideEffectsAsync(sutProvider, user, organization, organizationUser); } [Theory] [BitAutoData] public async Task RecoverAccountAsync_OrganizationDoesNotExist_ThrowsBadRequest( [OrganizationUser] OrganizationUser organizationUser, - string newMasterPassword, - string key, + MasterPasswordUnlockData unlockData, + MasterPasswordAuthenticationData authenticationData, SutProvider sutProvider) { // Arrange @@ -65,15 +107,15 @@ public async Task RecoverAccountAsync_OrganizationDoesNotExist_ThrowsBadRequest( // Act & Assert var exception = await Assert.ThrowsAsync(() => - sutProvider.Sut.RecoverAccountAsync(orgId, organizationUser, newMasterPassword, key)); + sutProvider.Sut.RecoverAccountAsync(orgId, organizationUser, unlockData, authenticationData)); Assert.Equal("Organization does not allow password reset.", exception.Message); } [Theory] [BitAutoData] public async Task RecoverAccountAsync_OrganizationDoesNotAllowResetPassword_ThrowsBadRequest( - string newMasterPassword, - string key, + MasterPasswordUnlockData unlockData, + MasterPasswordAuthenticationData authenticationData, Organization organization, [OrganizationUser] OrganizationUser organizationUser, SutProvider sutProvider) @@ -86,15 +128,15 @@ public async Task RecoverAccountAsync_OrganizationDoesNotAllowResetPassword_Thro // Act & Assert var exception = await Assert.ThrowsAsync(() => - sutProvider.Sut.RecoverAccountAsync(organization.Id, organizationUser, newMasterPassword, key)); + sutProvider.Sut.RecoverAccountAsync(organization.Id, organizationUser, unlockData, authenticationData)); Assert.Equal("Organization does not allow password reset.", exception.Message); } [Theory] [BitAutoData] public async Task RecoverAccountAsync_InvalidPolicy_ThrowsBadRequest( - string newMasterPassword, - string key, + MasterPasswordUnlockData unlockData, + MasterPasswordAuthenticationData authenticationData, Organization organization, [Policy(PolicyType.ResetPassword, false)] PolicyStatus policy, SutProvider sutProvider) @@ -106,7 +148,7 @@ public async Task RecoverAccountAsync_InvalidPolicy_ThrowsBadRequest( // Act & Assert var exception = await Assert.ThrowsAsync(() => sutProvider.Sut.RecoverAccountAsync(organization.Id, new OrganizationUser { Id = Guid.NewGuid() }, - newMasterPassword, key)); + unlockData, authenticationData)); Assert.Equal("Organization does not have the password reset policy enabled.", exception.Message); } @@ -174,8 +216,8 @@ public static IEnumerable InvalidOrganizationUsers() public async Task RecoverAccountAsync_OrganizationUserIsInvalid_ThrowsBadRequest( OrganizationUser organizationUser, Organization organization, - string newMasterPassword, - string key, + MasterPasswordUnlockData unlockData, + MasterPasswordAuthenticationData authenticationData, [Policy(PolicyType.ResetPassword, true)] PolicyStatus policy, SutProvider sutProvider) { @@ -185,15 +227,15 @@ public async Task RecoverAccountAsync_OrganizationUserIsInvalid_ThrowsBadRequest // Act & Assert var exception = await Assert.ThrowsAsync(() => - sutProvider.Sut.RecoverAccountAsync(organization.Id, organizationUser, newMasterPassword, key)); + sutProvider.Sut.RecoverAccountAsync(organization.Id, organizationUser, unlockData, authenticationData)); Assert.Equal("Organization User not valid", exception.Message); } [Theory] [BitAutoData] public async Task RecoverAccountAsync_UserDoesNotExist_ThrowsNotFoundException( - string newMasterPassword, - string key, + MasterPasswordUnlockData unlockData, + MasterPasswordAuthenticationData authenticationData, Organization organization, OrganizationUser organizationUser, [Policy(PolicyType.ResetPassword, true)] PolicyStatus policy, @@ -209,14 +251,14 @@ public async Task RecoverAccountAsync_UserDoesNotExist_ThrowsNotFoundException( // Act & Assert await Assert.ThrowsAsync(() => - sutProvider.Sut.RecoverAccountAsync(organization.Id, organizationUser, newMasterPassword, key)); + sutProvider.Sut.RecoverAccountAsync(organization.Id, organizationUser, unlockData, authenticationData)); } [Theory] [BitAutoData] public async Task RecoverAccountAsync_UserUsesKeyConnector_ThrowsBadRequest( - string newMasterPassword, - string key, + MasterPasswordUnlockData unlockData, + MasterPasswordAuthenticationData authenticationData, Organization organization, OrganizationUser organizationUser, User user, @@ -234,7 +276,7 @@ public async Task RecoverAccountAsync_UserUsesKeyConnector_ThrowsBadRequest( // Act & Assert var exception = await Assert.ThrowsAsync(() => - sutProvider.Sut.RecoverAccountAsync(organization.Id, organizationUser, newMasterPassword, key)); + sutProvider.Sut.RecoverAccountAsync(organization.Id, organizationUser, unlockData, authenticationData)); Assert.Equal("Cannot reset password of a user with Key Connector.", exception.Message); } @@ -261,32 +303,29 @@ private static void SetupValidOrganizationUser(OrganizationUser organizationUser organizationUser.Type = OrganizationUserType.User; } - private static void SetupValidUser(SutProvider sutProvider, User user, OrganizationUser organizationUser) + private static void SetupValidUser(SutProvider sutProvider, User user, + OrganizationUser organizationUser, bool hasMasterPassword) { user.Id = organizationUser.UserId!.Value; user.UsesKeyConnector = false; + user.MasterPassword = hasMasterPassword ? "existing-hash" : null; + user.Key = hasMasterPassword ? user.Key : null; sutProvider.GetDependency() .GetUserByIdAsync(user.Id) .Returns(user); + sutProvider.GetDependency() + .MutateUpdateExistingMasterPasswordAsync(Arg.Any(), Arg.Any()) + .Returns(Microsoft.AspNetCore.Identity.IdentityResult.Success); + sutProvider.GetDependency() + .MutateSetInitialMasterPasswordAsync(Arg.Any(), Arg.Any()) + .Returns(Microsoft.AspNetCore.Identity.IdentityResult.Success); } - private static void SetupSuccessfulPasswordUpdate(SutProvider sutProvider, User user, string newMasterPassword) - { - sutProvider.GetDependency() - .UpdatePasswordHash(user, newMasterPassword) - .Returns(IdentityResult.Success); - } - - private static async Task AssertSuccessAsync(SutProvider sutProvider, User user, string key, - Organization organization, OrganizationUser organizationUser) + private static async Task AssertCommonSuccessSideEffectsAsync(SutProvider sutProvider, + User user, Organization organization, OrganizationUser organizationUser) { await sutProvider.GetDependency().Received(1).ReplaceAsync( - Arg.Is(u => - u.Id == user.Id && - u.Key == key && - u.ForcePasswordReset == true && - u.RevisionDate == u.AccountRevisionDate && - u.LastPasswordChangeDate == u.RevisionDate)); + Arg.Is(u => u.Id == user.Id && u.ForcePasswordReset)); await sutProvider.GetDependency().Received(1).SendAdminResetPasswordEmailAsync( Arg.Is(user.Email), diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/AccountRecovery/v2/AdminRecoverAccountCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/AccountRecovery/v2/AdminRecoverAccountCommandTests.cs index c0052a949b59..2de7c12b34ca 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/AccountRecovery/v2/AdminRecoverAccountCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/AccountRecovery/v2/AdminRecoverAccountCommandTests.cs @@ -4,8 +4,11 @@ using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; using Bit.Core.AdminConsole.Utilities.v2.Validation; using Bit.Core.Auth.UserFeatures.TwoFactorAuth; +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Data; +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; using Bit.Core.Entities; using Bit.Core.Enums; +using Bit.Core.KeyManagement.Models.Api.Request; using Bit.Core.Platform.Push; using Bit.Core.Repositories; using Bit.Core.Services; @@ -20,9 +23,10 @@ namespace Bit.Core.Test.AdminConsole.OrganizationFeatures.AccountRecovery.v2; [SutProviderCustomize] public class AdminRecoverAccountCommandTests { + [Obsolete("To be removed in PM-33141")] [Theory] [BitAutoData] - public async Task RecoverAccountAsync_ResetMasterPasswordOnly_Success( + public async Task Legacy_RecoverAccountAsync_ResetMasterPasswordOnly_Success( string newMasterPassword, string key, Organization organization, @@ -78,7 +82,62 @@ await sutProvider.GetDependency().Received(1) [Theory] [BitAutoData] - public async Task RecoverAccountAsync_ResetTwoFactorOnly_Success( + public async Task RecoverAccountAsync_ResetMasterPasswordOnly_Success( + MasterPasswordUnlockDataRequestModel unlockData, + MasterPasswordAuthenticationDataRequestModel authenticationData, + Organization organization, + OrganizationUser organizationUser, + User user, + SutProvider sutProvider) + { + // Arrange + SetupOrganization(sutProvider, organization); + SetupUser(sutProvider, user, organizationUser); + SetupSuccessfulMasterPasswordServiceUpdate(sutProvider, user); + SetupPolicy(sutProvider, user); + + var request = CreateNewRequest(organization.Id, organizationUser, + resetMasterPassword: true, resetTwoFactor: false, + unlockData: unlockData, authenticationData: authenticationData); + SetupValidValidator(sutProvider); + + // Act + var result = await sutProvider.Sut.RecoverAccountAsync(request); + + // Assert + Assert.True(result.IsSuccess); + + await sutProvider.GetDependency().Received(1) + .MutateSetInitialOrUpdateExistingMasterPasswordAsync( + user, Arg.Any()); + + await sutProvider.GetDependency().Received(1).ReplaceAsync(user); + + Assert.True(user.ForcePasswordReset); + + await sutProvider.GetDependency().Received(1).SendAdminResetPasswordEmailAsync( + Arg.Is(user.Email), + Arg.Is(user.Name), + Arg.Is(organization.DisplayName()), + Arg.Is(true), + Arg.Is(false)); + + await sutProvider.GetDependency().Received(1).LogOrganizationUserEventAsync( + Arg.Is(organizationUser), + Arg.Is(EventType.OrganizationUser_AdminResetPassword)); + + await sutProvider.GetDependency().DidNotReceive().LogOrganizationUserEventAsync( + Arg.Any(), + Arg.Is(EventType.OrganizationUser_AdminResetTwoFactor)); + + await sutProvider.GetDependency().Received(1) + .PushLogOutAsync(user.Id); + } + + [Obsolete("To be removed in PM-33141")] + [Theory] + [BitAutoData] + public async Task Legacy_RecoverAccountAsync_ResetTwoFactorOnly_Success( Organization organization, OrganizationUser organizationUser, User user, @@ -126,7 +185,57 @@ await sutProvider.GetDependency().Received(1) [Theory] [BitAutoData] - public async Task RecoverAccountAsync_ResetBoth_Success( + public async Task RecoverAccountAsync_ResetTwoFactorOnly_Success( + Organization organization, + OrganizationUser organizationUser, + User user, + SutProvider sutProvider) + { + // Arrange + SetupOrganization(sutProvider, organization); + SetupUser(sutProvider, user, organizationUser); + SetupPolicy(sutProvider, user); + + var request = CreateNewRequest(organization.Id, organizationUser, + resetMasterPassword: false, resetTwoFactor: true); + SetupValidValidator(sutProvider); + + // Act + var result = await sutProvider.Sut.RecoverAccountAsync(request); + + // Assert + Assert.True(result.IsSuccess); + + await sutProvider.GetDependency().Received(1) + .ResetAsync(user); + + await sutProvider.GetDependency().DidNotReceive() + .MutateSetInitialOrUpdateExistingMasterPasswordAsync( + Arg.Any(), Arg.Any()); + + await sutProvider.GetDependency().Received(1).SendAdminResetPasswordEmailAsync( + Arg.Is(user.Email), + Arg.Is(user.Name), + Arg.Is(organization.DisplayName()), + Arg.Is(false), + Arg.Is(true)); + + await sutProvider.GetDependency().DidNotReceive().LogOrganizationUserEventAsync( + Arg.Any(), + Arg.Is(EventType.OrganizationUser_AdminResetPassword)); + + await sutProvider.GetDependency().Received(1).LogOrganizationUserEventAsync( + Arg.Is(organizationUser), + Arg.Is(EventType.OrganizationUser_AdminResetTwoFactor)); + + await sutProvider.GetDependency().Received(1) + .PushLogOutAsync(user.Id); + } + + [Obsolete("To be removed in PM-33141")] + [Theory] + [BitAutoData] + public async Task Legacy_RecoverAccountAsync_ResetBoth_Success( string newMasterPassword, string key, Organization organization, @@ -185,7 +294,65 @@ await sutProvider.GetDependency().Received(1) [Theory] [BitAutoData] - public async Task RecoverAccountAsync_UpdatePasswordHashFails_ReturnsError( + public async Task RecoverAccountAsync_ResetBoth_Success( + MasterPasswordUnlockDataRequestModel unlockData, + MasterPasswordAuthenticationDataRequestModel authenticationData, + Organization organization, + OrganizationUser organizationUser, + User user, + SutProvider sutProvider) + { + // Arrange + SetupOrganization(sutProvider, organization); + SetupUser(sutProvider, user, organizationUser); + SetupSuccessfulMasterPasswordServiceUpdate(sutProvider, user); + SetupPolicy(sutProvider, user); + + var request = CreateNewRequest(organization.Id, organizationUser, + resetMasterPassword: true, resetTwoFactor: true, + unlockData: unlockData, authenticationData: authenticationData); + SetupValidValidator(sutProvider); + + // Act + var result = await sutProvider.Sut.RecoverAccountAsync(request); + + // Assert + Assert.True(result.IsSuccess); + + await sutProvider.GetDependency().Received(1) + .MutateSetInitialOrUpdateExistingMasterPasswordAsync( + user, Arg.Any()); + + await sutProvider.GetDependency().Received(1).ReplaceAsync(user); + + Assert.True(user.ForcePasswordReset); + + await sutProvider.GetDependency().Received(1) + .ResetAsync(user); + + await sutProvider.GetDependency().Received(1).SendAdminResetPasswordEmailAsync( + Arg.Is(user.Email), + Arg.Is(user.Name), + Arg.Is(organization.DisplayName()), + Arg.Is(true), + Arg.Is(true)); + + await sutProvider.GetDependency().Received(1).LogOrganizationUserEventAsync( + Arg.Is(organizationUser), + Arg.Is(EventType.OrganizationUser_AdminResetPassword)); + + await sutProvider.GetDependency().Received(1).LogOrganizationUserEventAsync( + Arg.Is(organizationUser), + Arg.Is(EventType.OrganizationUser_AdminResetTwoFactor)); + + await sutProvider.GetDependency().Received(1) + .PushLogOutAsync(user.Id); + } + + [Obsolete("To be removed in PM-33141")] + [Theory] + [BitAutoData] + public async Task Legacy_RecoverAccountAsync_UpdatePasswordHashFails_ReturnsError( string newMasterPassword, string key, Organization organization, @@ -231,6 +398,55 @@ await sutProvider.GetDependency().DidNotReceive() .PushLogOutAsync(Arg.Any()); } + [Theory] + [BitAutoData] + public async Task RecoverAccountAsync_MasterPasswordServiceFails_ReturnsError( + MasterPasswordUnlockDataRequestModel unlockData, + MasterPasswordAuthenticationDataRequestModel authenticationData, + Organization organization, + OrganizationUser organizationUser, + User user, + SutProvider sutProvider) + { + // Arrange + SetupOrganization(sutProvider, organization); + SetupUser(sutProvider, user, organizationUser); + SetupPolicy(sutProvider, user); + + var failedResult = IdentityResult.Failed(new IdentityError { Description = "Password update failed" }); + sutProvider.GetDependency() + .MutateSetInitialOrUpdateExistingMasterPasswordAsync( + user, Arg.Any()) + .Returns(failedResult); + + var request = CreateNewRequest(organization.Id, organizationUser, + resetMasterPassword: true, resetTwoFactor: false, + unlockData: unlockData, authenticationData: authenticationData); + SetupValidValidator(sutProvider); + + // Act + var result = await sutProvider.Sut.RecoverAccountAsync(request); + + // Assert + Assert.True(result.IsError); + Assert.IsType(result.AsError); + Assert.Contains("Password update failed", result.AsError.Message); + + await sutProvider.GetDependency().DidNotReceive() + .ReplaceAsync(Arg.Any()); + + await sutProvider.GetDependency().DidNotReceive() + .SendAdminResetPasswordEmailAsync( + Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any(), Arg.Any()); + + await sutProvider.GetDependency().DidNotReceive() + .LogOrganizationUserEventAsync(Arg.Any(), Arg.Any()); + + await sutProvider.GetDependency().DidNotReceive() + .PushLogOutAsync(Arg.Any()); + } + [Theory] [BitAutoData] public async Task RecoverAccountAsync_ValidationFails_ReturnsError( @@ -277,6 +493,25 @@ private static RecoverAccountRequest CreateRequest( }; } + private static RecoverAccountRequest CreateNewRequest( + Guid orgId, + OrganizationUser organizationUser, + bool resetMasterPassword, + bool resetTwoFactor, + MasterPasswordUnlockDataRequestModel? unlockData = null, + MasterPasswordAuthenticationDataRequestModel? authenticationData = null) + { + return new RecoverAccountRequest + { + OrgId = orgId, + OrganizationUser = organizationUser, + ResetMasterPassword = resetMasterPassword, + ResetTwoFactor = resetTwoFactor, + UnlockData = unlockData, + AuthenticationData = authenticationData, + }; + } + private static void SetupValidValidator(SutProvider sutProvider) { sutProvider.GetDependency() @@ -299,6 +534,7 @@ private static void SetupUser(SutProvider sutProvide .Returns(user); } + [Obsolete("To be removed in PM-33141")] private static void SetupSuccessfulPasswordUpdate(SutProvider sutProvider, User user, string newMasterPassword) { sutProvider.GetDependency() @@ -306,6 +542,14 @@ private static void SetupSuccessfulPasswordUpdate(SutProvider sutProvider, User user) + { + sutProvider.GetDependency() + .MutateSetInitialOrUpdateExistingMasterPasswordAsync( + user, Arg.Any()) + .Returns(IdentityResult.Success); + } + private static void SetupPolicy(SutProvider sutProvider, User user) { // 2FA policy does not apply diff --git a/test/Core.Test/Auth/UserFeatures/EmergencyAccess/EmergencyAccessServiceTests.cs b/test/Core.Test/Auth/UserFeatures/EmergencyAccess/EmergencyAccessServiceTests.cs index 288930740838..555ae0c9e845 100644 --- a/test/Core.Test/Auth/UserFeatures/EmergencyAccess/EmergencyAccessServiceTests.cs +++ b/test/Core.Test/Auth/UserFeatures/EmergencyAccess/EmergencyAccessServiceTests.cs @@ -8,14 +8,18 @@ using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Auth.Models.Data; using Bit.Core.Auth.UserFeatures.EmergencyAccess; +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Data; +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; +using Bit.Core.KeyManagement.Models.Data; using Bit.Core.Repositories; using Bit.Core.Services; using Bit.Core.Tokens; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; +using Microsoft.AspNetCore.Identity; using NSubstitute; using Xunit; @@ -1413,8 +1417,9 @@ public async Task TakeoverAsync_Success_ReturnsEmergencyAccessAndGrantorUser( Assert.Equal(result.Item2, grantor); } + [Obsolete("To be removed in PM-33141")] [Theory, BitAutoData] - public async Task PasswordAsync_RequestNotValid_EmergencyAccessIsNull_ThrowsBadRequest( + public async Task FinishRecoveryTakeoverAsync_Legacy_RequestNotValid_EmergencyAccessIsNull_ThrowsBadRequest( SutProvider sutProvider) { sutProvider.GetDependency() @@ -1422,13 +1427,14 @@ public async Task PasswordAsync_RequestNotValid_EmergencyAccessIsNull_ThrowsBadR .Returns((Core.Auth.Entities.EmergencyAccess)null); var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.PasswordAsync(default, default, default, default)); + () => sutProvider.Sut.FinishRecoveryTakeoverAsync(default, default, (string)null, (string)null)); Assert.Contains("Emergency Access not valid.", exception.Message); } + [Obsolete("To be removed in PM-33141")] [Theory, BitAutoData] - public async Task PasswordAsync_RequestNotValid_GranteeNotEqualToRequestingUser_ThrowsBadRequest( + public async Task FinishRecoveryTakeoverAsync_Legacy_RequestNotValid_GranteeNotEqualToRequestingUser_ThrowsBadRequest( SutProvider sutProvider, Core.Auth.Entities.EmergencyAccess emergencyAccess, User granteeUser) @@ -1440,17 +1446,18 @@ public async Task PasswordAsync_RequestNotValid_GranteeNotEqualToRequestingUser_ .Returns(emergencyAccess); var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.PasswordAsync(emergencyAccess.Id, granteeUser, default, default)); + () => sutProvider.Sut.FinishRecoveryTakeoverAsync(emergencyAccess.Id, granteeUser, (string)null, (string)null)); Assert.Contains("Emergency Access not valid.", exception.Message); } + [Obsolete("To be removed in PM-33141")] [Theory] [BitAutoData(EmergencyAccessStatusType.Invited)] [BitAutoData(EmergencyAccessStatusType.Accepted)] [BitAutoData(EmergencyAccessStatusType.Confirmed)] [BitAutoData(EmergencyAccessStatusType.RecoveryInitiated)] - public async Task PasswordAsync_RequestNotValid_StatusType_ThrowsBadRequest( + public async Task FinishRecoveryTakeoverAsync_Legacy_RequestNotValid_StatusType_ThrowsBadRequest( EmergencyAccessStatusType statusType, SutProvider sutProvider, Core.Auth.Entities.EmergencyAccess emergencyAccess, @@ -1464,13 +1471,14 @@ public async Task PasswordAsync_RequestNotValid_StatusType_ThrowsBadRequest( .Returns(emergencyAccess); var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.PasswordAsync(emergencyAccess.Id, granteeUser, default, default)); + () => sutProvider.Sut.FinishRecoveryTakeoverAsync(emergencyAccess.Id, granteeUser, (string)null, (string)null)); Assert.Contains("Emergency Access not valid.", exception.Message); } + [Obsolete("To be removed in PM-33141")] [Theory, BitAutoData] - public async Task PasswordAsync_RequestNotValid_TypeIsView_ThrowsBadRequest( + public async Task FinishRecoveryTakeoverAsync_Legacy_RequestNotValid_TypeIsView_ThrowsBadRequest( SutProvider sutProvider, Core.Auth.Entities.EmergencyAccess emergencyAccess, User granteeUser) @@ -1483,13 +1491,14 @@ public async Task PasswordAsync_RequestNotValid_TypeIsView_ThrowsBadRequest( .Returns(emergencyAccess); var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.PasswordAsync(emergencyAccess.Id, granteeUser, default, default)); + () => sutProvider.Sut.FinishRecoveryTakeoverAsync(emergencyAccess.Id, granteeUser, (string)null, (string)null)); Assert.Contains("Emergency Access not valid.", exception.Message); } + [Obsolete("To be removed in PM-33141")] [Theory, BitAutoData] - public async Task PasswordAsync_NonOrgUser_Success( + public async Task FinishRecoveryTakeoverAsync_Legacy_NonOrgUser_Success( SutProvider sutProvider, Core.Auth.Entities.EmergencyAccess emergencyAccess, User granteeUser, @@ -1509,7 +1518,7 @@ public async Task PasswordAsync_NonOrgUser_Success( .GetByIdAsync(emergencyAccess.GrantorId) .Returns(grantorUser); - await sutProvider.Sut.PasswordAsync(emergencyAccess.Id, granteeUser, passwordHash, key); + await sutProvider.Sut.FinishRecoveryTakeoverAsync(emergencyAccess.Id, granteeUser, passwordHash, key); await sutProvider.GetDependency() .Received(1) @@ -1519,11 +1528,12 @@ await sutProvider.GetDependency() .ReplaceAsync(Arg.Is(u => u.VerifyDevices == false && u.Key == key)); } + [Obsolete("To be removed in PM-33141")] [Theory] [BitAutoData(OrganizationUserType.User)] [BitAutoData(OrganizationUserType.Admin)] [BitAutoData(OrganizationUserType.Custom)] - public async Task PasswordAsync_OrgUser_NotOrganizationOwner_RemovedFromOrganization_Success( + public async Task FinishRecoveryTakeoverAsync_Legacy_OrgUser_NotOrganizationOwner_RemovedFromOrganization_Success( OrganizationUserType userType, SutProvider sutProvider, Core.Auth.Entities.EmergencyAccess emergencyAccess, @@ -1551,7 +1561,7 @@ public async Task PasswordAsync_OrgUser_NotOrganizationOwner_RemovedFromOrganiza .GetManyByUserAsync(grantorUser.Id) .Returns([organizationUser]); - await sutProvider.Sut.PasswordAsync(emergencyAccess.Id, granteeUser, passwordHash, key); + await sutProvider.Sut.FinishRecoveryTakeoverAsync(emergencyAccess.Id, granteeUser, passwordHash, key); await sutProvider.GetDependency() .Received(1) @@ -1564,8 +1574,9 @@ await sutProvider.GetDependency() .RemoveUserAsync(organizationUser.OrganizationId, organizationUser.UserId.Value); } + [Obsolete("To be removed in PM-33141")] [Theory, BitAutoData] - public async Task PasswordAsync_OrgUser_IsOrganizationOwner_NotRemovedFromOrganization_Success( + public async Task FinishRecoveryTakeoverAsync_Legacy_OrgUser_IsOrganizationOwner_NotRemovedFromOrganization_Success( SutProvider sutProvider, Core.Auth.Entities.EmergencyAccess emergencyAccess, User granteeUser, @@ -1592,7 +1603,7 @@ public async Task PasswordAsync_OrgUser_IsOrganizationOwner_NotRemovedFromOrgani .GetManyByUserAsync(grantorUser.Id) .Returns([organizationUser]); - await sutProvider.Sut.PasswordAsync(emergencyAccess.Id, granteeUser, passwordHash, key); + await sutProvider.Sut.FinishRecoveryTakeoverAsync(emergencyAccess.Id, granteeUser, passwordHash, key); await sutProvider.GetDependency() .Received(1) @@ -1605,8 +1616,9 @@ await sutProvider.GetDependency() .RemoveUserAsync(organizationUser.OrganizationId, organizationUser.UserId.Value); } + [Obsolete("To be removed in PM-33141")] [Theory, BitAutoData] - public async Task PasswordAsync_Disables_NewDeviceVerification_And_TwoFactorProviders_On_The_Grantor( + public async Task FinishRecoveryTakeoverAsync_Legacy_Disables_NewDeviceVerification_And_TwoFactorProviders_On_The_Grantor( SutProvider sutProvider, User requestingUser, User grantor) { grantor.UsesKeyConnector = true; @@ -1633,7 +1645,281 @@ public async Task PasswordAsync_Disables_NewDeviceVerification_And_TwoFactorProv .GetByIdAsync(grantor.Id) .Returns(grantor); - await sutProvider.Sut.PasswordAsync(Guid.NewGuid(), requestingUser, "blablahash", "blablakey"); + await sutProvider.Sut.FinishRecoveryTakeoverAsync(Guid.NewGuid(), requestingUser, "blablahash", "blablakey"); + + Assert.Empty(grantor.GetTwoFactorProviders()); + Assert.False(grantor.VerifyDevices); + await sutProvider.GetDependency().Received().ReplaceAsync(grantor); + } + + [Theory, BitAutoData] + public async Task FinishRecoveryTakeoverAsync_RequestNotValid_EmergencyAccessIsNull_ThrowsBadRequest( + SutProvider sutProvider, + MasterPasswordUnlockData unlockData, + MasterPasswordAuthenticationData authenticationData) + { + sutProvider.GetDependency() + .GetByIdAsync(Arg.Any()) + .Returns((Core.Auth.Entities.EmergencyAccess)null); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.FinishRecoveryTakeoverAsync(default, default, unlockData, authenticationData)); + + Assert.Contains("Emergency Access not valid.", exception.Message); + } + + [Theory, BitAutoData] + public async Task FinishRecoveryTakeoverAsync_RequestNotValid_GranteeNotEqualToRequestingUser_ThrowsBadRequest( + SutProvider sutProvider, + Core.Auth.Entities.EmergencyAccess emergencyAccess, + User granteeUser, + MasterPasswordUnlockData unlockData, + MasterPasswordAuthenticationData authenticationData) + { + emergencyAccess.Status = EmergencyAccessStatusType.RecoveryApproved; + emergencyAccess.Type = EmergencyAccessType.Takeover; + sutProvider.GetDependency() + .GetByIdAsync(Arg.Any()) + .Returns(emergencyAccess); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.FinishRecoveryTakeoverAsync(emergencyAccess.Id, granteeUser, unlockData, authenticationData)); + + Assert.Contains("Emergency Access not valid.", exception.Message); + } + + [Theory] + [BitAutoData(EmergencyAccessStatusType.Invited)] + [BitAutoData(EmergencyAccessStatusType.Accepted)] + [BitAutoData(EmergencyAccessStatusType.Confirmed)] + [BitAutoData(EmergencyAccessStatusType.RecoveryInitiated)] + public async Task FinishRecoveryTakeoverAsync_RequestNotValid_StatusType_ThrowsBadRequest( + EmergencyAccessStatusType statusType, + SutProvider sutProvider, + Core.Auth.Entities.EmergencyAccess emergencyAccess, + User granteeUser, + MasterPasswordUnlockData unlockData, + MasterPasswordAuthenticationData authenticationData) + { + emergencyAccess.GranteeId = granteeUser.Id; + emergencyAccess.Status = statusType; + emergencyAccess.Type = EmergencyAccessType.Takeover; + sutProvider.GetDependency() + .GetByIdAsync(Arg.Any()) + .Returns(emergencyAccess); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.FinishRecoveryTakeoverAsync(emergencyAccess.Id, granteeUser, unlockData, authenticationData)); + + Assert.Contains("Emergency Access not valid.", exception.Message); + } + + [Theory, BitAutoData] + public async Task FinishRecoveryTakeoverAsync_RequestNotValid_TypeIsView_ThrowsBadRequest( + SutProvider sutProvider, + Core.Auth.Entities.EmergencyAccess emergencyAccess, + User granteeUser, + MasterPasswordUnlockData unlockData, + MasterPasswordAuthenticationData authenticationData) + { + emergencyAccess.GranteeId = granteeUser.Id; + emergencyAccess.Status = EmergencyAccessStatusType.RecoveryApproved; + emergencyAccess.Type = EmergencyAccessType.View; + sutProvider.GetDependency() + .GetByIdAsync(Arg.Any()) + .Returns(emergencyAccess); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.FinishRecoveryTakeoverAsync(emergencyAccess.Id, granteeUser, unlockData, authenticationData)); + + Assert.Contains("Emergency Access not valid.", exception.Message); + } + + [Theory, BitAutoData] + public async Task FinishRecoveryTakeoverAsync_GrantorNotFound_ThrowsBadRequest( + SutProvider sutProvider, + Core.Auth.Entities.EmergencyAccess emergencyAccess, + User granteeUser, + MasterPasswordUnlockData unlockData, + MasterPasswordAuthenticationData authenticationData) + { + emergencyAccess.GranteeId = granteeUser.Id; + emergencyAccess.Status = EmergencyAccessStatusType.RecoveryApproved; + emergencyAccess.Type = EmergencyAccessType.Takeover; + sutProvider.GetDependency() + .GetByIdAsync(Arg.Any()) + .Returns(emergencyAccess); + sutProvider.GetDependency() + .GetByIdAsync(emergencyAccess.GrantorId) + .Returns((User)null); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.FinishRecoveryTakeoverAsync(emergencyAccess.Id, granteeUser, unlockData, authenticationData)); + + Assert.Contains("Grantor not found when trying to finish recovery takeover.", exception.Message); + } + + [Theory, BitAutoData] + public async Task FinishRecoveryTakeoverAsync_NonOrgUser_Success( + SutProvider sutProvider, + Core.Auth.Entities.EmergencyAccess emergencyAccess, + User granteeUser, + User grantorUser, + MasterPasswordUnlockData unlockData, + MasterPasswordAuthenticationData authenticationData) + { + emergencyAccess.GranteeId = granteeUser.Id; + emergencyAccess.GrantorId = grantorUser.Id; + emergencyAccess.Status = EmergencyAccessStatusType.RecoveryApproved; + emergencyAccess.Type = EmergencyAccessType.Takeover; + sutProvider.GetDependency() + .GetByIdAsync(Arg.Any()) + .Returns(emergencyAccess); + sutProvider.GetDependency() + .GetByIdAsync(emergencyAccess.GrantorId) + .Returns(grantorUser); + sutProvider.GetDependency() + .MutateSetInitialOrUpdateExistingMasterPasswordAsync(Arg.Any(), Arg.Any()) + .Returns(IdentityResult.Success); + + await sutProvider.Sut.FinishRecoveryTakeoverAsync(emergencyAccess.Id, granteeUser, unlockData, authenticationData); + + await sutProvider.GetDependency() + .Received(1) + .MutateSetInitialOrUpdateExistingMasterPasswordAsync( + grantorUser, + Arg.Is(d => + d.MasterPasswordUnlock == unlockData && + d.MasterPasswordAuthentication == authenticationData)); + await sutProvider.GetDependency() + .Received(1) + .ReplaceAsync(Arg.Is(u => u.VerifyDevices == false)); + } + + [Theory] + [BitAutoData(OrganizationUserType.User)] + [BitAutoData(OrganizationUserType.Admin)] + [BitAutoData(OrganizationUserType.Custom)] + public async Task FinishRecoveryTakeoverAsync_OrgUser_NotOrganizationOwner_RemovedFromOrganization_Success( + OrganizationUserType userType, + SutProvider sutProvider, + Core.Auth.Entities.EmergencyAccess emergencyAccess, + User granteeUser, + User grantorUser, + OrganizationUser organizationUser, + MasterPasswordUnlockData unlockData, + MasterPasswordAuthenticationData authenticationData) + { + emergencyAccess.GranteeId = granteeUser.Id; + emergencyAccess.GrantorId = grantorUser.Id; + emergencyAccess.Status = EmergencyAccessStatusType.RecoveryApproved; + emergencyAccess.Type = EmergencyAccessType.Takeover; + sutProvider.GetDependency() + .GetByIdAsync(Arg.Any()) + .Returns(emergencyAccess); + sutProvider.GetDependency() + .GetByIdAsync(emergencyAccess.GrantorId) + .Returns(grantorUser); + organizationUser.UserId = grantorUser.Id; + organizationUser.Type = userType; + sutProvider.GetDependency() + .GetManyByUserAsync(grantorUser.Id) + .Returns([organizationUser]); + sutProvider.GetDependency() + .MutateSetInitialOrUpdateExistingMasterPasswordAsync(Arg.Any(), Arg.Any()) + .Returns(IdentityResult.Success); + + await sutProvider.Sut.FinishRecoveryTakeoverAsync(emergencyAccess.Id, granteeUser, unlockData, authenticationData); + + await sutProvider.GetDependency() + .Received(1) + .MutateSetInitialOrUpdateExistingMasterPasswordAsync(grantorUser, Arg.Any()); + await sutProvider.GetDependency() + .Received(1) + .ReplaceAsync(Arg.Is(u => u.VerifyDevices == false)); + await sutProvider.GetDependency() + .Received(1) + .RemoveUserAsync(organizationUser.OrganizationId, organizationUser.UserId.Value); + } + + [Theory, BitAutoData] + public async Task FinishRecoveryTakeoverAsync_OrgUser_IsOrganizationOwner_NotRemovedFromOrganization_Success( + SutProvider sutProvider, + Core.Auth.Entities.EmergencyAccess emergencyAccess, + User granteeUser, + User grantorUser, + OrganizationUser organizationUser, + MasterPasswordUnlockData unlockData, + MasterPasswordAuthenticationData authenticationData) + { + emergencyAccess.GranteeId = granteeUser.Id; + emergencyAccess.GrantorId = grantorUser.Id; + emergencyAccess.Status = EmergencyAccessStatusType.RecoveryApproved; + emergencyAccess.Type = EmergencyAccessType.Takeover; + sutProvider.GetDependency() + .GetByIdAsync(Arg.Any()) + .Returns(emergencyAccess); + sutProvider.GetDependency() + .GetByIdAsync(emergencyAccess.GrantorId) + .Returns(grantorUser); + organizationUser.UserId = grantorUser.Id; + organizationUser.Type = OrganizationUserType.Owner; + sutProvider.GetDependency() + .GetManyByUserAsync(grantorUser.Id) + .Returns([organizationUser]); + sutProvider.GetDependency() + .MutateSetInitialOrUpdateExistingMasterPasswordAsync(Arg.Any(), Arg.Any()) + .Returns(IdentityResult.Success); + + await sutProvider.Sut.FinishRecoveryTakeoverAsync(emergencyAccess.Id, granteeUser, unlockData, authenticationData); + + await sutProvider.GetDependency() + .Received(1) + .MutateSetInitialOrUpdateExistingMasterPasswordAsync(grantorUser, Arg.Any()); + await sutProvider.GetDependency() + .Received(1) + .ReplaceAsync(Arg.Is(u => u.VerifyDevices == false)); + await sutProvider.GetDependency() + .Received(0) + .RemoveUserAsync(organizationUser.OrganizationId, organizationUser.UserId.Value); + } + + [Theory, BitAutoData] + public async Task FinishRecoveryTakeoverAsync_Disables_NewDeviceVerification_And_TwoFactorProviders_On_The_Grantor( + SutProvider sutProvider, + User requestingUser, + User grantor, + MasterPasswordUnlockData unlockData, + MasterPasswordAuthenticationData authenticationData) + { + grantor.UsesKeyConnector = true; + grantor.SetTwoFactorProviders(new Dictionary + { + [TwoFactorProviderType.Email] = new TwoFactorProvider + { + MetaData = new Dictionary { ["Email"] = "asdfasf" }, + Enabled = true + } + }); + var emergencyAccess = new Core.Auth.Entities.EmergencyAccess + { + GrantorId = grantor.Id, + GranteeId = requestingUser.Id, + Status = EmergencyAccessStatusType.RecoveryApproved, + Type = EmergencyAccessType.Takeover, + }; + + sutProvider.GetDependency() + .GetByIdAsync(Arg.Any()) + .Returns(emergencyAccess); + sutProvider.GetDependency() + .GetByIdAsync(grantor.Id) + .Returns(grantor); + sutProvider.GetDependency() + .MutateSetInitialOrUpdateExistingMasterPasswordAsync(Arg.Any(), Arg.Any()) + .Returns(IdentityResult.Success); + + await sutProvider.Sut.FinishRecoveryTakeoverAsync(Guid.NewGuid(), requestingUser, unlockData, authenticationData); Assert.Empty(grantor.GetTwoFactorProviders()); Assert.False(grantor.VerifyDevices); diff --git a/test/Core.Test/Auth/UserFeatures/TdeOffboarding/TdeOffboardingPasswordCommandTests.cs b/test/Core.Test/Auth/UserFeatures/TdeOffboarding/TdeOffboardingPasswordCommandTests.cs index 49558783f864..40fbf632984a 100644 --- a/test/Core.Test/Auth/UserFeatures/TdeOffboarding/TdeOffboardingPasswordCommandTests.cs +++ b/test/Core.Test/Auth/UserFeatures/TdeOffboarding/TdeOffboardingPasswordCommandTests.cs @@ -15,11 +15,15 @@ namespace Bit.Core.Test.Auth.UserFeatures.UserMasterPassword; +/// +/// Add tests for new function to ensure parity +/// [SutProviderCustomize] public class TdeOffboardingPasswordTests { [Theory] [BitAutoData] + [Obsolete("To be removed in PM-33141")] public async Task TdeOffboardingPasswordCommand_Success(SutProvider sutProvider, User user, string masterPassword, string key, string hint, OrganizationUserOrganizationDetails orgUserDetails, SsoUser ssoUser) { @@ -56,6 +60,7 @@ public async Task TdeOffboardingPasswordCommand_Success(SutProvider sutProvider, User user, string masterPassword, string key, string hint, OrganizationUserOrganizationDetails orgUserDetails, SsoUser ssoUser) { diff --git a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/FinishSsoJitProvisionMasterPasswordCommandTests.cs b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/FinishSsoJitProvisionMasterPasswordCommandTests.cs index c1315fe46707..302c49d95832 100644 --- a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/FinishSsoJitProvisionMasterPasswordCommandTests.cs +++ b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/FinishSsoJitProvisionMasterPasswordCommandTests.cs @@ -1,6 +1,8 @@ using Bit.Core.AdminConsole.Entities; using Bit.Core.Auth.Models.Data; using Bit.Core.Auth.UserFeatures.UserMasterPassword; +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Data; +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; @@ -10,13 +12,15 @@ using Bit.Core.Services; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; -using Microsoft.AspNetCore.Identity; using NSubstitute; using NSubstitute.ReturnsExtensions; using Xunit; namespace Bit.Core.Test.Auth.UserFeatures.UserMasterPassword; +/// + +/// [SutProviderCustomize] public class FinishSsoJitProvisionMasterPasswordCommandTests { @@ -24,7 +28,7 @@ public class FinishSsoJitProvisionMasterPasswordCommandTests [BitAutoData] public async Task FinishProvisionAsync_Success(SutProvider sutProvider, User user, UserAccountKeysData accountKeys, KdfSettings kdfSettings, - Organization org, OrganizationUser orgUser, string serverSideHash, string masterPasswordHint) + Organization org, OrganizationUser orgUser, string masterPasswordHint) { // Arrange user.Key = null; @@ -38,14 +42,9 @@ public async Task FinishProvisionAsync_Success(SutProvider>() - .HashPassword(user, model.MasterPasswordAuthentication.MasterPasswordAuthenticationHash) - .Returns(serverSideHash); - - // Mock SetMasterPassword to return a specific UpdateUserData delegate UpdateUserData mockUpdateUserData = (connection, transaction) => Task.CompletedTask; - sutProvider.GetDependency() - .SetMasterPassword(user.Id, model.MasterPasswordUnlock, serverSideHash, model.MasterPasswordHint) + sutProvider.GetDependency() + .BuildUpdateUserDelegateSetInitialMasterPassword(user, Arg.Any()) .Returns(mockUpdateUserData); // Act @@ -144,7 +143,7 @@ public async Task FinishProvisionAsync_InvalidSalt_ThrowsBadRequestException( public async Task FinishProvisionAsync_NullSalt_UsesEmailFallback( SutProvider sutProvider, User user, UserAccountKeysData accountKeys, KdfSettings kdfSettings, - Organization org, OrganizationUser orgUser, string serverSideHash, string masterPasswordHint) + Organization org, OrganizationUser orgUser, string masterPasswordHint) { // Arrange user.Key = null; @@ -164,13 +163,9 @@ public async Task FinishProvisionAsync_NullSalt_UsesEmailFallback( .GetByOrganizationAsync(org.Id, user.Id) .Returns(orgUser); - sutProvider.GetDependency>() - .HashPassword(user, model.MasterPasswordAuthentication.MasterPasswordAuthenticationHash) - .Returns(serverSideHash); - UpdateUserData mockUpdateUserData = (connection, transaction) => Task.CompletedTask; - sutProvider.GetDependency() - .SetMasterPassword(user.Id, model.MasterPasswordUnlock, serverSideHash, model.MasterPasswordHint) + sutProvider.GetDependency() + .BuildUpdateUserDelegateSetInitialMasterPassword(user, Arg.Any()) .Returns(mockUpdateUserData); // Act — should not throw since email fallback provides a valid salt 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..504d77bfbcb1 --- /dev/null +++ b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/MasterPasswordServiceTests.cs @@ -0,0 +1,413 @@ +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; + +public class MasterPasswordServiceTests +{ + private static SutProvider CreateSutProvider() + => new SutProvider().WithFakeTimeProvider().Create(); + + [Theory, BitAutoData] + public async Task SetInitialMasterPassword_Success(User user, string masterPasswordHash, string key, KdfSettings kdf, string salt) + { + // Arrange + var sutProvider = CreateSutProvider(); + user.MasterPassword = null; + user.Key = null; + user.MasterPasswordSalt = null; + user.UsesKeyConnector = false; + var expectedHash = "server-hashed-" + masterPasswordHash; + sutProvider.GetDependency>() + .HashPassword(user, masterPasswordHash) + .Returns(expectedHash); + + var setInitialData = new SetInitialPasswordData + { + MasterPasswordAuthentication = new MasterPasswordAuthenticationData + { + Kdf = kdf, + MasterPasswordAuthenticationHash = masterPasswordHash, + Salt = salt + }, + MasterPasswordUnlock = new MasterPasswordUnlockData + { + Kdf = kdf, + MasterKeyWrappedUserKey = key, + Salt = salt + }, + ValidatePassword = false + }; + + // Act + await sutProvider.Sut.MutateSetInitialMasterPasswordAsync(user, setInitialData); + + // Assert + Assert.Equal(expectedHash, user.MasterPassword); + Assert.Equal(key, user.Key); + Assert.Equal(kdf.KdfType, user.Kdf); + Assert.Equal(kdf.Iterations, user.KdfIterations); + Assert.Equal(kdf.Memory, user.KdfMemory); + Assert.Equal(kdf.Parallelism, user.KdfParallelism); + Assert.Equal(salt, user.MasterPasswordSalt); + Assert.NotNull(user.LastPasswordChangeDate); + } + + [Theory, BitAutoData] + public async Task SetInitialMasterPassword_SetsMasterPasswordHint(User user, string masterPasswordHash, string key, KdfSettings kdf, string salt, string hint) + { + // Arrange + var sutProvider = CreateSutProvider(); + user.MasterPassword = null; + user.Key = null; + user.MasterPasswordSalt = null; + user.UsesKeyConnector = false; + + var setInitialData = new SetInitialPasswordData + { + MasterPasswordAuthentication = new MasterPasswordAuthenticationData + { + Kdf = kdf, + MasterPasswordAuthenticationHash = masterPasswordHash, + Salt = salt + }, + MasterPasswordUnlock = new MasterPasswordUnlockData + { + Kdf = kdf, + MasterKeyWrappedUserKey = key, + Salt = salt + }, + MasterPasswordHint = hint, + ValidatePassword = false + }; + + // Act + await sutProvider.Sut.MutateSetInitialMasterPasswordAsync(user, setInitialData); + + // Assert + Assert.Equal(hint, user.MasterPasswordHint); + } + + [Theory, BitAutoData] + public async Task SetInitialMasterPassword_ThrowsWhenMasterPasswordAlreadySet(User user, string masterPasswordHash, string key, KdfSettings kdf, string salt) + { + // Arrange + var sutProvider = CreateSutProvider(); + user.MasterPassword = "existing-hash"; + user.Key = null; + user.MasterPasswordSalt = null; + + var setInitialData = new SetInitialPasswordData + { + MasterPasswordAuthentication = new MasterPasswordAuthenticationData + { + Kdf = kdf, + MasterPasswordAuthenticationHash = masterPasswordHash, + Salt = salt + }, + MasterPasswordUnlock = new MasterPasswordUnlockData + { + Kdf = kdf, + MasterKeyWrappedUserKey = key, + Salt = salt + } + }; + + // Act & Assert + var exception = await Assert.ThrowsAsync(() => + sutProvider.Sut.MutateSetInitialMasterPasswordAsync(user, setInitialData)); + Assert.Equal("User already has a master password set.", exception.Message); + } + + [Theory, BitAutoData] + public async Task SetInitialMasterPassword_ThrowsWhenKeyAlreadySet(User user, string masterPasswordHash, string key, KdfSettings kdf, string salt) + { + // Arrange + var sutProvider = CreateSutProvider(); + user.MasterPassword = null; + user.Key = "existing-key"; + user.MasterPasswordSalt = null; + + var setInitialData = new SetInitialPasswordData + { + MasterPasswordAuthentication = new MasterPasswordAuthenticationData + { + Kdf = kdf, + MasterPasswordAuthenticationHash = masterPasswordHash, + Salt = salt + }, + MasterPasswordUnlock = new MasterPasswordUnlockData + { + Kdf = kdf, + MasterKeyWrappedUserKey = key, + Salt = salt + } + }; + + // Act & Assert + var exception = await Assert.ThrowsAsync(() => + sutProvider.Sut.MutateSetInitialMasterPasswordAsync(user, setInitialData)); + Assert.Equal("User already has a key set.", exception.Message); + } + + [Theory, BitAutoData] + public async Task SetInitialMasterPasswordAsync_CallsMutationThenSavesUser( + User user, string masterPasswordHash, string key, KdfSettings kdf, string salt) + { + // Arrange + var sutProvider = CreateSutProvider(); + user.MasterPassword = null; + user.Key = null; + user.MasterPasswordSalt = null; + user.UsesKeyConnector = false; + + var setInitialData = new SetInitialPasswordData + { + MasterPasswordAuthentication = new MasterPasswordAuthenticationData + { + Kdf = kdf, + MasterPasswordAuthenticationHash = masterPasswordHash, + Salt = salt + }, + MasterPasswordUnlock = new MasterPasswordUnlockData + { + Kdf = kdf, + MasterKeyWrappedUserKey = key, + Salt = salt + }, + ValidatePassword = false + }; + + // Act + await sutProvider.Sut.SaveSetInitialMasterPasswordAsync(user, setInitialData); + + // Assert: mutation was applied + Assert.NotNull(user.MasterPassword); + Assert.Equal(key, user.Key); + + // Assert: user was persisted + await sutProvider.GetDependency() + .Received(1) + .ReplaceAsync(user); + } + + // ------------------------------------------------------------------------- + // UpdateMasterPassword + // ------------------------------------------------------------------------- + + [Theory, BitAutoData] + public async Task UpdateMasterPassword_Success(User user, string masterPasswordHash, string key, string salt) + { + // Arrange + var sutProvider = CreateSutProvider(); + user.MasterPassword = "existing-hash"; + user.MasterPasswordSalt = salt; + user.UsesKeyConnector = false; + var kdf = new KdfSettings + { + KdfType = user.Kdf, + Iterations = user.KdfIterations, + Memory = user.KdfMemory, + Parallelism = user.KdfParallelism + }; + var expectedHash = "server-hashed-" + masterPasswordHash; + sutProvider.GetDependency>() + .HashPassword(user, masterPasswordHash) + .Returns(expectedHash); + + var updateData = new UpdateExistingPasswordData + { + MasterPasswordAuthentication = new MasterPasswordAuthenticationData + { + Kdf = kdf, + MasterPasswordAuthenticationHash = masterPasswordHash, + Salt = salt + }, + MasterPasswordUnlock = new MasterPasswordUnlockData + { + Kdf = kdf, + MasterKeyWrappedUserKey = key, + Salt = salt + }, + ValidatePassword = false + }; + + // Act + await sutProvider.Sut.MutateUpdateExistingMasterPasswordAsync(user, updateData); + + // Assert + Assert.Equal(expectedHash, user.MasterPassword); + Assert.Equal(key, user.Key); + Assert.NotNull(user.LastPasswordChangeDate); + // KDF fields must be unchanged + Assert.Equal(kdf.KdfType, user.Kdf); + Assert.Equal(kdf.Iterations, user.KdfIterations); + Assert.Equal(kdf.Memory, user.KdfMemory); + Assert.Equal(kdf.Parallelism, user.KdfParallelism); + } + + [Theory, BitAutoData] + public async Task UpdateMasterPassword_SetsMasterPasswordHint(User user, string masterPasswordHash, string key, string salt, string hint) + { + // Arrange + var sutProvider = CreateSutProvider(); + user.MasterPassword = "existing-hash"; + user.MasterPasswordSalt = salt; + user.UsesKeyConnector = false; + var kdf = new KdfSettings + { + KdfType = user.Kdf, + Iterations = user.KdfIterations, + Memory = user.KdfMemory, + Parallelism = user.KdfParallelism + }; + + var updateData = new UpdateExistingPasswordData + { + MasterPasswordAuthentication = new MasterPasswordAuthenticationData + { + Kdf = kdf, + MasterPasswordAuthenticationHash = masterPasswordHash, + Salt = salt + }, + MasterPasswordUnlock = new MasterPasswordUnlockData + { + Kdf = kdf, + MasterKeyWrappedUserKey = key, + Salt = salt + }, + MasterPasswordHint = hint, + ValidatePassword = false + }; + + // Act + await sutProvider.Sut.MutateUpdateExistingMasterPasswordAsync(user, updateData); + + // Assert + Assert.Equal(hint, user.MasterPasswordHint); + } + + [Theory, BitAutoData] + public async Task UpdateMasterPassword_ThrowsWhenNoExistingPassword(User user, string masterPasswordHash, string key, KdfSettings kdf, string salt) + { + // Arrange + var sutProvider = CreateSutProvider(); + user.MasterPassword = null; + + var updateData = new UpdateExistingPasswordData + { + MasterPasswordAuthentication = new MasterPasswordAuthenticationData + { + Kdf = kdf, + MasterPasswordAuthenticationHash = masterPasswordHash, + Salt = salt + }, + MasterPasswordUnlock = new MasterPasswordUnlockData + { + Kdf = kdf, + MasterKeyWrappedUserKey = key, + Salt = salt + } + }; + + // Act & Assert + var exception = await Assert.ThrowsAsync(() => + sutProvider.Sut.MutateUpdateExistingMasterPasswordAsync(user, updateData)); + Assert.Equal("User does not have an existing master password to update.", exception.Message); + } + + [Theory, BitAutoData] + public async Task UpdateMasterPassword_ThrowsWhenKdfMismatch(User user, string masterPasswordHash, string key, string salt) + { + // Arrange + var sutProvider = CreateSutProvider(); + user.MasterPassword = "existing-hash"; + user.UsesKeyConnector = false; + user.Kdf = KdfType.PBKDF2_SHA256; + user.KdfIterations = 600000; + // Pass KDF settings that differ from user's stored KDF + var mismatchedKdf = new KdfSettings + { + KdfType = KdfType.Argon2id, + Iterations = 3, + Memory = 64, + Parallelism = 4 + }; + + var updateData = new UpdateExistingPasswordData + { + MasterPasswordAuthentication = new MasterPasswordAuthenticationData + { + Kdf = mismatchedKdf, + MasterPasswordAuthenticationHash = masterPasswordHash, + Salt = salt + }, + MasterPasswordUnlock = new MasterPasswordUnlockData + { + Kdf = mismatchedKdf, + MasterKeyWrappedUserKey = key, + Salt = salt + } + }; + + // Act & Assert + await Assert.ThrowsAsync(() => + sutProvider.Sut.MutateUpdateExistingMasterPasswordAsync(user, updateData)); + } + + [Theory, BitAutoData] + public async Task UpdateMasterPasswordAsync_CallsMutationThenSavesUser(User user, string masterPasswordHash, string key, string salt) + { + // Arrange + var sutProvider = CreateSutProvider(); + user.MasterPassword = "existing-hash"; + user.MasterPasswordSalt = salt; + user.UsesKeyConnector = false; + var kdf = new KdfSettings + { + KdfType = user.Kdf, + Iterations = user.KdfIterations, + Memory = user.KdfMemory, + Parallelism = user.KdfParallelism + }; + + var updateData = new UpdateExistingPasswordData + { + MasterPasswordAuthentication = new MasterPasswordAuthenticationData + { + Kdf = kdf, + MasterPasswordAuthenticationHash = masterPasswordHash, + Salt = salt + }, + MasterPasswordUnlock = new MasterPasswordUnlockData + { + Kdf = kdf, + MasterKeyWrappedUserKey = key, + Salt = salt + }, + ValidatePassword = false + }; + + // Act + await sutProvider.Sut.SaveUpdateExistingMasterPasswordAsync(user, updateData); + + // Assert: mutation was applied + Assert.NotNull(user.MasterPassword); + Assert.Equal(key, user.Key); + + // Assert: user was persisted + await sutProvider.GetDependency() + .Received(1) + .ReplaceAsync(user); + } +} diff --git a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommandV1Tests.cs b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommandV1Tests.cs index d87b2730260a..62ee0175808e 100644 --- a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommandV1Tests.cs +++ b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommandV1Tests.cs @@ -16,6 +16,7 @@ namespace Bit.Core.Test.Auth.UserFeatures.UserMasterPassword; [SutProviderCustomize] +[Obsolete("To be removed in PM-33141")] public class SetInitialMasterPasswordCommandV1Tests { [Theory] diff --git a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/TdeSetPasswordCommandTests.cs b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/TdeSetPasswordCommandTests.cs index 6aea42970063..64261a18a2ae 100644 --- a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/TdeSetPasswordCommandTests.cs +++ b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/TdeSetPasswordCommandTests.cs @@ -1,6 +1,8 @@ using Bit.Core.AdminConsole.Entities; using Bit.Core.Auth.Models.Data; using Bit.Core.Auth.UserFeatures.UserMasterPassword; +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Data; +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; @@ -9,7 +11,6 @@ using Bit.Core.Services; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; -using Microsoft.AspNetCore.Identity; using NSubstitute; using NSubstitute.ReturnsExtensions; using Xunit; @@ -23,7 +24,7 @@ public class TdeSetPasswordCommandTests [BitAutoData] public async Task OnboardMasterPassword_Success(SutProvider sutProvider, User user, KdfSettings kdfSettings, - Organization org, OrganizationUser orgUser, string serverSideHash, string masterPasswordHint) + Organization org, OrganizationUser orgUser, string masterPasswordHint) { // Arrange user.Key = null; @@ -39,14 +40,9 @@ public async Task OnboardMasterPassword_Success(SutProvider>() - .HashPassword(user, model.MasterPasswordAuthentication.MasterPasswordAuthenticationHash) - .Returns(serverSideHash); - - // Mock SetMasterPassword to return a specific UpdateUserData delegate UpdateUserData mockUpdateUserData = (connection, transaction) => Task.CompletedTask; - sutProvider.GetDependency() - .SetMasterPassword(user.Id, model.MasterPasswordUnlock, serverSideHash, model.MasterPasswordHint) + sutProvider.GetDependency() + .BuildUpdateUserDelegateSetInitialMasterPassword(user, Arg.Any()) .Returns(mockUpdateUserData); // Act @@ -70,7 +66,7 @@ await sutProvider.GetDependency().Received(1) public async Task OnboardMasterPassword_NullSalt_UsesEmailFallback( SutProvider sutProvider, User user, KdfSettings kdfSettings, - Organization org, OrganizationUser orgUser, string serverSideHash, string masterPasswordHint) + Organization org, OrganizationUser orgUser, string masterPasswordHint) { // Arrange user.Key = null; @@ -92,13 +88,9 @@ public async Task OnboardMasterPassword_NullSalt_UsesEmailFallback( .GetByOrganizationAsync(org.Id, user.Id) .Returns(orgUser); - sutProvider.GetDependency>() - .HashPassword(user, model.MasterPasswordAuthentication.MasterPasswordAuthenticationHash) - .Returns(serverSideHash); - UpdateUserData mockUpdateUserData = (connection, transaction) => Task.CompletedTask; - sutProvider.GetDependency() - .SetMasterPassword(user.Id, model.MasterPasswordUnlock, serverSideHash, model.MasterPasswordHint) + sutProvider.GetDependency() + .BuildUpdateUserDelegateSetInitialMasterPassword(user, Arg.Any()) .Returns(mockUpdateUserData); // Act — should not throw since email fallback provides a valid salt @@ -109,23 +101,6 @@ await sutProvider.GetDependency().Received(1) .LogUserEventAsync(user.Id, EventType.User_ChangedPassword); } - [Theory] - [BitAutoData] - public async Task OnboardMasterPassword_UserAlreadyHasPassword_ThrowsBadRequestException( - SutProvider sutProvider, - User user, KdfSettings kdfSettings, string orgSsoIdentifier, string masterPasswordHint) - { - // Arrange - user.Key = "existing-key"; - var model = CreateValidModel(user, kdfSettings, orgSsoIdentifier, masterPasswordHint); - - // Act & Assert - var exception = - await Assert.ThrowsAsync(async () => - await sutProvider.Sut.SetMasterPasswordAsync(user, model)); - Assert.Equal("User already has a master password set.", exception.Message); - } - [Theory] [BitAutoData([null, "private-key"])] [BitAutoData("public-key", null)] @@ -148,47 +123,6 @@ await Assert.ThrowsAsync(async () => Assert.Equal("TDE user account keys must be set before setting initial master password.", exception.Message); } - [Theory] - [BitAutoData("wrong-salt", null)] - [BitAutoData([null, "wrong-salt"])] - [BitAutoData("wrong-salt", "different-wrong-salt")] - public async Task OnboardMasterPassword_InvalidSalt_ThrowsBadRequestException( - string? authSaltOverride, string? unlockSaltOverride, - SutProvider sutProvider, - User user, KdfSettings kdfSettings, string orgSsoIdentifier, string masterPasswordHint) - { - // Arrange - user.Key = null; - user.PublicKey = "public-key"; - user.PrivateKey = "private-key"; - var correctSalt = user.GetMasterPasswordSalt(); - var model = new SetInitialMasterPasswordDataModel - { - MasterPasswordAuthentication = - new MasterPasswordAuthenticationData - { - Salt = authSaltOverride ?? correctSalt, - MasterPasswordAuthenticationHash = "hash", - Kdf = kdfSettings - }, - MasterPasswordUnlock = new MasterPasswordUnlockData - { - Salt = unlockSaltOverride ?? correctSalt, - MasterKeyWrappedUserKey = "wrapped-key", - Kdf = kdfSettings - }, - AccountKeys = null, - OrgSsoIdentifier = orgSsoIdentifier, - MasterPasswordHint = masterPasswordHint - }; - - // Act & Assert - var exception = - await Assert.ThrowsAsync(async () => - await sutProvider.Sut.SetMasterPasswordAsync(user, model)); - Assert.Equal("Invalid master password salt.", exception.Message); - } - [Theory] [BitAutoData] public async Task OnboardMasterPassword_InvalidOrgSsoIdentifier_ThrowsBadRequestException( diff --git a/test/Core.Test/KeyManagement/Kdf/ChangeKdfCommandTests.cs b/test/Core.Test/KeyManagement/Kdf/ChangeKdfCommandTests.cs index 991935b92896..55ede98cda5d 100644 --- a/test/Core.Test/KeyManagement/Kdf/ChangeKdfCommandTests.cs +++ b/test/Core.Test/KeyManagement/Kdf/ChangeKdfCommandTests.cs @@ -1,12 +1,13 @@ #nullable enable +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Data; +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.KeyManagement.Kdf.Implementations; using Bit.Core.KeyManagement.Models.Data; using Bit.Core.Platform.Push; -using Bit.Core.Repositories; using Bit.Core.Services; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; @@ -25,7 +26,8 @@ public async Task ChangeKdfAsync_ChangesKdfAsync(SutProvider s { sutProvider.GetDependency().CheckPasswordAsync(Arg.Any(), Arg.Any()) .Returns(Task.FromResult(true)); - sutProvider.GetDependency().UpdatePasswordHash(Arg.Any(), Arg.Any()) + sutProvider.GetDependency() + .SaveUpdateExistingMasterPasswordAndKdfAsync(Arg.Any(), Arg.Any()) .Returns(Task.FromResult(IdentityResult.Success)); var kdf = new KdfSettings { KdfType = Enums.KdfType.Argon2id, Iterations = 4, Memory = 512, Parallelism = 4 }; @@ -44,13 +46,11 @@ public async Task ChangeKdfAsync_ChangesKdfAsync(SutProvider s await sutProvider.Sut.ChangeKdfAsync(user, "masterPassword", authenticationData, unlockData); - await sutProvider.GetDependency().Received(1).ReplaceAsync(Arg.Is(u => - u.Id == user.Id - && u.Kdf == Enums.KdfType.Argon2id - && u.KdfIterations == 4 - && u.KdfMemory == 512 - && u.KdfParallelism == 4 - )); + await sutProvider.GetDependency().Received(1) + .SaveUpdateExistingMasterPasswordAndKdfAsync(user, Arg.Is(d => + d.MasterPasswordAuthentication == authenticationData + && d.MasterPasswordUnlock == unlockData + )); } [Theory] @@ -129,23 +129,19 @@ public async Task }; sutProvider.GetDependency().CheckPasswordAsync(Arg.Any(), Arg.Any()) .Returns(Task.FromResult(true)); - sutProvider.GetDependency() - .UpdatePasswordHash(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + sutProvider.GetDependency() + .SaveUpdateExistingMasterPasswordAndKdfAsync(Arg.Any(), Arg.Any()) .Returns(Task.FromResult(IdentityResult.Success)); sutProvider.GetDependency().IsEnabled(Arg.Any()).Returns(false); await sutProvider.Sut.ChangeKdfAsync(user, "masterPassword", authenticationData, unlockData); - await sutProvider.GetDependency().Received(1).ReplaceAsync(Arg.Is(u => - u.Id == user.Id - && u.Kdf == constantKdf.KdfType - && u.KdfIterations == constantKdf.Iterations - && u.KdfMemory == constantKdf.Memory - && u.KdfParallelism == constantKdf.Parallelism - && u.Key == "new-wrapped-key" - )); - await sutProvider.GetDependency().Received(1).UpdatePasswordHash(user, - authenticationData.MasterPasswordAuthenticationHash, validatePassword: true, refreshStamp: true); + await sutProvider.GetDependency().Received(1) + .SaveUpdateExistingMasterPasswordAndKdfAsync(user, Arg.Is(d => + d.MasterPasswordAuthentication == authenticationData + && d.MasterPasswordUnlock == unlockData + && d.RefreshStamp == true + )); await sutProvider.GetDependency().Received(1).PushLogOutAsync(user.Id); sutProvider.GetDependency().Received(1).IsEnabled(FeatureFlagKeys.NoLogoutOnKdfChange); } @@ -177,23 +173,19 @@ public async Task }; sutProvider.GetDependency().CheckPasswordAsync(Arg.Any(), Arg.Any()) .Returns(Task.FromResult(true)); - sutProvider.GetDependency() - .UpdatePasswordHash(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + sutProvider.GetDependency() + .SaveUpdateExistingMasterPasswordAndKdfAsync(Arg.Any(), Arg.Any()) .Returns(Task.FromResult(IdentityResult.Success)); sutProvider.GetDependency().IsEnabled(Arg.Any()).Returns(true); await sutProvider.Sut.ChangeKdfAsync(user, "masterPassword", authenticationData, unlockData); - await sutProvider.GetDependency().Received(1).ReplaceAsync(Arg.Is(u => - u.Id == user.Id - && u.Kdf == constantKdf.KdfType - && u.KdfIterations == constantKdf.Iterations - && u.KdfMemory == constantKdf.Memory - && u.KdfParallelism == constantKdf.Parallelism - && u.Key == "new-wrapped-key" - )); - await sutProvider.GetDependency().Received(1).UpdatePasswordHash(user, - authenticationData.MasterPasswordAuthenticationHash, validatePassword: true, refreshStamp: false); + await sutProvider.GetDependency().Received(1) + .SaveUpdateExistingMasterPasswordAndKdfAsync(user, Arg.Is(d => + d.MasterPasswordAuthentication == authenticationData + && d.MasterPasswordUnlock == unlockData + && d.RefreshStamp == false + )); await sutProvider.GetDependency().Received(1) .PushLogOutAsync(user.Id, false, PushNotificationLogOutReason.KdfChange); await sutProvider.GetDependency().Received(1).PushSyncSettingsAsync(user.Id); @@ -286,7 +278,8 @@ public async Task ChangeKdfAsync_UpdatePasswordHashFails_ReturnsFailure(SutProvi sutProvider.GetDependency().CheckPasswordAsync(Arg.Any(), Arg.Any()) .Returns(Task.FromResult(true)); var failedResult = IdentityResult.Failed(new IdentityError { Code = "TestFail", Description = "Test fail" }); - sutProvider.GetDependency().UpdatePasswordHash(Arg.Any(), Arg.Any()) + sutProvider.GetDependency() + .SaveUpdateExistingMasterPasswordAndKdfAsync(Arg.Any(), Arg.Any()) .Returns(Task.FromResult(failedResult)); var kdf = new KdfSettings { KdfType = Enums.KdfType.Argon2id, Iterations = 4, Memory = 512, Parallelism = 4 };