From c23d91377302bd6b6297a4266b2a41a464a9cfcd Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Tue, 29 Aug 2023 17:23:35 -0400 Subject: [PATCH 01/80] PM-3275 - Add new GetMasterPasswordPolicy endpoint which will allow authenticated clients to get an enabled MP org policy if it exists for the purposes of enforcing those policy requirements when setting a password. --- src/Api/Controllers/PoliciesController.cs | 32 ++++++++++++++++++++--- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/src/Api/Controllers/PoliciesController.cs b/src/Api/Controllers/PoliciesController.cs index e352e109083a..93f1c0c6d2be 100644 --- a/src/Api/Controllers/PoliciesController.cs +++ b/src/Api/Controllers/PoliciesController.cs @@ -104,18 +104,20 @@ public async Task> GetByToken(string orgI return new ListResponseModel(responses); } + // TODO: remove GetByInvitedUser once all clients are updated to use the GetMasterPasswordPolicy endpoint below + // TODO: create PM-??? tech debt item to track this work. In theory, target release for removal should be 2024.01.0 + // TODO: figure out how to mark this as deprecated. [Obsolete("Deprecated API", false)]? [AllowAnonymous] [HttpGet("invited-user")] - public async Task> GetByInvitedUser(string orgId, [FromQuery] string userId) + public async Task> GetByInvitedUser(Guid orgId, [FromQuery] string userId) { var user = await _userService.GetUserByIdAsync(new Guid(userId)); if (user == null) { throw new UnauthorizedAccessException(); } - var orgIdGuid = new Guid(orgId); var orgUsersByUserId = await _organizationUserRepository.GetManyByUserAsync(user.Id); - var orgUser = orgUsersByUserId.SingleOrDefault(u => u.OrganizationId == orgIdGuid); + var orgUser = orgUsersByUserId.SingleOrDefault(u => u.OrganizationId == orgId); if (orgUser == null) { throw new NotFoundException(); @@ -125,11 +127,33 @@ public async Task> GetByInvitedUser(strin throw new UnauthorizedAccessException(); } - var policies = await _policyRepository.GetManyByOrganizationIdAsync(orgIdGuid); + var policies = await _policyRepository.GetManyByOrganizationIdAsync(orgId); var responses = policies.Where(p => p.Enabled).Select(p => new PolicyResponseModel(p)); return new ListResponseModel(responses); } + [HttpGet("master-password")] + public async Task GetMasterPasswordPolicy(Guid orgId) + { + var userId = _userService.GetProperUserId(User).Value; + + var orgUsersByUserId = await _organizationUserRepository.GetManyByUserAsync(userId); + var orgUser = orgUsersByUserId.SingleOrDefault(u => u.OrganizationId == orgId); + if (orgUser == null) + { + throw new NotFoundException(); + } + + var policy = await _policyRepository.GetByOrganizationIdTypeAsync(orgId, PolicyType.MasterPassword); + + if (!policy.Enabled) + { + throw new NotFoundException(); + } + + return new PolicyResponseModel(policy); + } + [HttpPut("{type}")] public async Task Put(string orgId, int type, [FromBody] PolicyRequestModel model) { From 5142d552ee5b561bc3a680aa8d97792c308b60af Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Wed, 30 Aug 2023 18:48:52 -0400 Subject: [PATCH 02/80] PM-3275 - AccountsController.cs - PostSetPasswordAsync - (1) Convert UserService.setPasswordAsync into new SetInitialMasterPasswordCommand (2) Refactor SetInitialMasterPasswordCommand to only accept post SSO users who are in the invited state (3) Add TODOs for more cleanup work and more commands --- src/Api/Controllers/AccountsController.cs | 9 +- .../SetInitialMasterPasswordCommand.cs | 106 ++++++++++++++++++ .../ISetInitialMasterPasswordCommand.cs | 10 ++ .../UserServiceCollectionExtensions.cs | 24 ++++ src/Core/Services/IUserService.cs | 4 +- .../Implementations/OrganizationService.cs | 5 +- .../Services/Implementations/UserService.cs | 37 +----- .../Utilities/ServiceCollectionExtensions.cs | 3 +- 8 files changed, 156 insertions(+), 42 deletions(-) create mode 100644 src/Core/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommand.cs create mode 100644 src/Core/Auth/UserFeatures/UserMasterPassword/interfaces/ISetInitialMasterPasswordCommand.cs create mode 100644 src/Core/Auth/UserFeatures/UserServiceCollectionExtensions.cs diff --git a/src/Api/Controllers/AccountsController.cs b/src/Api/Controllers/AccountsController.cs index d199c223debe..d668ebefb5e7 100644 --- a/src/Api/Controllers/AccountsController.cs +++ b/src/Api/Controllers/AccountsController.cs @@ -7,6 +7,7 @@ using Bit.Core.Auth.Models.Api.Request.Accounts; using Bit.Core.Auth.Models.Api.Response.Accounts; using Bit.Core.Auth.Services; +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; using Bit.Core.Auth.Utilities; using Bit.Core.Enums; using Bit.Core.Enums.Provider; @@ -45,6 +46,7 @@ public class AccountsController : Controller private readonly ISendService _sendService; private readonly ICaptchaValidationService _captchaValidationService; private readonly IPolicyService _policyService; + private readonly ISetInitialMasterPasswordCommand _setInitialMasterPasswordCommand; public AccountsController( GlobalSettings globalSettings, @@ -59,7 +61,9 @@ public AccountsController( ISendRepository sendRepository, ISendService sendService, ICaptchaValidationService captchaValidationService, - IPolicyService policyService) + IPolicyService policyService, + ISetInitialMasterPasswordCommand setInitialMasterPasswordCommand + ) { _cipherRepository = cipherRepository; _folderRepository = folderRepository; @@ -74,6 +78,7 @@ public AccountsController( _sendService = sendService; _captchaValidationService = captchaValidationService; _policyService = policyService; + _setInitialMasterPasswordCommand = setInitialMasterPasswordCommand; } #region DEPRECATED (Moved to Identity Service) @@ -251,7 +256,7 @@ public async Task PostSetPasswordAsync([FromBody] SetPasswordRequestModel model) throw new UnauthorizedAccessException(); } - var result = await _userService.SetPasswordAsync(model.ToUser(user), model.MasterPasswordHash, model.Key, + var result = await _setInitialMasterPasswordCommand.SetInitialMasterPasswordAsync(model.ToUser(user), model.MasterPasswordHash, model.Key, model.OrgIdentifier); if (result.Succeeded) { diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommand.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommand.cs new file mode 100644 index 000000000000..4b5b9eccbba9 --- /dev/null +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommand.cs @@ -0,0 +1,106 @@ +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; +using Microsoft.Extensions.Logging; + +namespace Bit.Core.Auth.UserFeatures.UserMasterPassword; + +/// +/// Manages the setting of the initial master password for a user in an organization. +/// This class is primarily invoked in two scenarios: +/// 1) In organizations configured with Single Sign-On (SSO) and master password decryption: +/// just in time (JIT) provisioned users logging in via SSO are required to set a master password. +/// 2) In organizations configured with SSO and trusted devices decryption: +/// Users who are upgraded to have admin account recovery permissions must set a master password +/// to ensure their ability to reset other users' accounts. +/// +public class SetInitialMasterPasswordCommand : ISetInitialMasterPasswordCommand +{ + private readonly ILogger _logger; + private readonly IdentityErrorDescriber _identityErrorDescriber; + private readonly IUserService _userService; + private readonly IUserRepository _userRepository; + private readonly IEventService _eventService; + private readonly IOrganizationService _organizationService; + private readonly IOrganizationUserRepository _organizationUserRepository; + private readonly IOrganizationRepository _organizationRepository; + + + public SetInitialMasterPasswordCommand( ILogger logger, + IdentityErrorDescriber identityErrorDescriber, + IUserService userService, + IUserRepository userRepository, + IEventService eventService, + IOrganizationService organizationService, + IOrganizationUserRepository organizationUserRepository, + IOrganizationRepository organizationRepository) + { + _logger = logger; + _identityErrorDescriber = identityErrorDescriber; + _userService = userService; + _userRepository = userRepository; + _eventService = eventService; + _organizationService = organizationService; + _organizationUserRepository = organizationUserRepository; + _organizationRepository = organizationRepository; + } + + public async Task SetInitialMasterPasswordAsync(User user, string masterPassword, string key, + string orgIdentifier = null) + { + if (user == null) + { + throw new ArgumentNullException(nameof(user)); + } + + if (!string.IsNullOrWhiteSpace(user.MasterPassword)) + { + _logger.LogWarning("Change password failed for user {userId} - already has password.", user.Id); + return IdentityResult.Failed(_identityErrorDescriber.UserAlreadyHasPassword()); + } + + var result = await _userService.UpdatePasswordHash(user, masterPassword, validatePassword: true, refreshStamp: false); + if (!result.Succeeded) + { + return result; + } + + user.RevisionDate = user.AccountRevisionDate = DateTime.UtcNow; + user.Key = key; + + await _userRepository.ReplaceAsync(user); + await _eventService.LogUserEventAsync(user.Id, EventType.User_ChangedPassword); + + var org = await _organizationRepository.GetByIdentifierAsync(orgIdentifier); + + if (org == null) + { + throw new BadRequestException("Organization invalid."); + } + + var orgUser = await _organizationUserRepository.GetByOrganizationAsync(org.Id, user.Id); + + if (orgUser == null) + { + throw new BadRequestException("User not found within organization."); + } + + // TDE users who go from a user without admin acct recovery permission to having it will be + // required to set a MP for the first time and we don't want to re-execute the accept logic + // as they are already confirmed. + // TLDR: only accept post SSO user if they are invited + if (!string.IsNullOrWhiteSpace(orgIdentifier) && orgUser.Status == OrganizationUserStatusType.Invited) + { + // TODO: should pass org user to AcceptUserAsync + // TODO: convert AcceptUserAsync to own command AcceptOrgUserCommand + await _organizationService.AcceptUserAsync(orgUser, user, _userService); + } + + return IdentityResult.Success; + } + +} diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/interfaces/ISetInitialMasterPasswordCommand.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/interfaces/ISetInitialMasterPasswordCommand.cs new file mode 100644 index 000000000000..5d6074aaa4e0 --- /dev/null +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/interfaces/ISetInitialMasterPasswordCommand.cs @@ -0,0 +1,10 @@ +using Bit.Core.Entities; +using Microsoft.AspNetCore.Identity; + +namespace Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; + +public interface ISetInitialMasterPasswordCommand +{ + public Task SetInitialMasterPasswordAsync(User user, string masterPassword, string key, + string orgIdentifier = null); +} diff --git a/src/Core/Auth/UserFeatures/UserServiceCollectionExtensions.cs b/src/Core/Auth/UserFeatures/UserServiceCollectionExtensions.cs new file mode 100644 index 000000000000..eff162c7397b --- /dev/null +++ b/src/Core/Auth/UserFeatures/UserServiceCollectionExtensions.cs @@ -0,0 +1,24 @@ + + +using Bit.Core.Auth.UserFeatures.UserMasterPassword; +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; +using Bit.Core.Services; +using Bit.Core.Settings; +using Microsoft.Extensions.DependencyInjection; + +namespace Bit.Core.Auth.UserFeatures; + +public static class UserServiceCollectionExtensions +{ + public static void AddUserServices(this IServiceCollection services, IGlobalSettings globalSettings) + { + services.AddScoped(); + services.AddUserPasswordCommands(); + } + + private static void AddUserPasswordCommands(this IServiceCollection services) + { + services.AddScoped(); + } + +} diff --git a/src/Core/Services/IUserService.cs b/src/Core/Services/IUserService.cs index d0c078d40645..c789e2d4fcad 100644 --- a/src/Core/Services/IUserService.cs +++ b/src/Core/Services/IUserService.cs @@ -33,7 +33,6 @@ public interface IUserService Task ChangeEmailAsync(User user, string masterPassword, string newEmail, string newMasterPassword, string token, string key); Task ChangePasswordAsync(User user, string masterPassword, string newMasterPassword, string passwordHint, string key); - Task SetPasswordAsync(User user, string newMasterPassword, string key, string orgIdentifier = null); Task SetKeyConnectorKeyAsync(User user, string key, string orgIdentifier); Task ConvertToKeyConnectorAsync(User user); Task AdminResetPasswordAsync(OrganizationUserType type, Guid orgId, Guid id, string newMasterPassword, string key); @@ -74,6 +73,9 @@ Task GenerateLicenseAsync(User user, SubscriptionInfo subscriptionI Task TwoFactorIsEnabledAsync(ITwoFactorProvidersUser user); Task TwoFactorProviderIsEnabledAsync(TwoFactorProviderType provider, ITwoFactorProvidersUser user); Task GenerateSignInTokenAsync(User user, string purpose); + + Task UpdatePasswordHash(User user, string newPassword, + bool validatePassword = true, bool refreshStamp = true); Task RotateApiKeyAsync(User user); string GetUserName(ClaimsPrincipal principal); Task SendOTPAsync(User user); diff --git a/src/Core/Services/Implementations/OrganizationService.cs b/src/Core/Services/Implementations/OrganizationService.cs index b7424a53e101..38bf991ef779 100644 --- a/src/Core/Services/Implementations/OrganizationService.cs +++ b/src/Core/Services/Implementations/OrganizationService.cs @@ -1103,9 +1103,8 @@ public async Task AcceptUserAsync(string orgIdentifier, User u { throw new BadRequestException("Organization invalid."); } - - var usersOrgs = await _organizationUserRepository.GetManyByUserAsync(user.Id); - var orgUser = usersOrgs.FirstOrDefault(u => u.OrganizationId == org.Id); + + var orgUser = await _organizationUserRepository.GetByOrganizationAsync(org.Id, user.Id); if (orgUser == null) { throw new BadRequestException("User not found within organization."); diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index 643e1bf18ea8..024a4a8878db 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -600,6 +600,7 @@ await _stripeSyncService.UpdateCustomerEmailAddress(user.GatewayCustomerId, return IdentityResult.Success; } + // TODO: delete this unused method public override Task ChangePasswordAsync(User user, string masterPassword, string newMasterPassword) { throw new NotImplementedException(); @@ -638,40 +639,6 @@ public async Task ChangePasswordAsync(User user, string masterPa return IdentityResult.Failed(_identityErrorDescriber.PasswordMismatch()); } - public async Task SetPasswordAsync(User user, string masterPassword, string key, - string orgIdentifier = null) - { - if (user == null) - { - throw new ArgumentNullException(nameof(user)); - } - - if (!string.IsNullOrWhiteSpace(user.MasterPassword)) - { - Logger.LogWarning("Change password failed for user {userId} - already has password.", user.Id); - return IdentityResult.Failed(_identityErrorDescriber.UserAlreadyHasPassword()); - } - - var result = await UpdatePasswordHash(user, masterPassword, true, false); - if (!result.Succeeded) - { - return result; - } - - user.RevisionDate = user.AccountRevisionDate = DateTime.UtcNow; - user.Key = key; - - await _userRepository.ReplaceAsync(user); - await _eventService.LogUserEventAsync(user.Id, EventType.User_ChangedPassword); - - if (!string.IsNullOrWhiteSpace(orgIdentifier)) - { - await _organizationService.AcceptUserAsync(orgIdentifier, user, this); - } - - return IdentityResult.Success; - } - public async Task SetKeyConnectorKeyAsync(User user, string key, string orgIdentifier) { var identityResult = CheckCanUseKeyConnector(user); @@ -1352,7 +1319,7 @@ public async Task GenerateSignInTokenAsync(User user, string purpose) return token; } - private async Task UpdatePasswordHash(User user, string newPassword, + public async Task UpdatePasswordHash(User user, string newPassword, bool validatePassword = true, bool refreshStamp = true) { if (validatePassword) diff --git a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs index ea97af041958..e42e86f2db58 100644 --- a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs +++ b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs @@ -10,6 +10,7 @@ using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Auth.Services; using Bit.Core.Auth.Services.Implementations; +using Bit.Core.Auth.UserFeatures; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.HostedServices; @@ -124,7 +125,7 @@ public static SupportedDatabaseProviders AddDatabaseRepositories(this IServiceCo public static void AddBaseServices(this IServiceCollection services, IGlobalSettings globalSettings) { services.AddScoped(); - services.AddScoped(); + services.AddUserServices(globalSettings); services.AddOrganizationServices(globalSettings); services.AddScoped(); services.AddScoped(); From c28035bb376899b2d81a2d55b2f080bd5976bee7 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Wed, 30 Aug 2023 18:51:40 -0400 Subject: [PATCH 03/80] PM-3275 - Update AccountsControllerTests.cs to add new SetInitialMasterPasswordCommand --- test/Api.Test/Controllers/AccountsControllerTests.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/Api.Test/Controllers/AccountsControllerTests.cs b/test/Api.Test/Controllers/AccountsControllerTests.cs index 0dff6db93d7a..a0acc2e8a0d4 100644 --- a/test/Api.Test/Controllers/AccountsControllerTests.cs +++ b/test/Api.Test/Controllers/AccountsControllerTests.cs @@ -3,6 +3,7 @@ using Bit.Api.Controllers; using Bit.Core.Auth.Models.Api.Request.Accounts; using Bit.Core.Auth.Services; +using Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; @@ -36,6 +37,7 @@ public class AccountsControllerTests : IDisposable private readonly IProviderUserRepository _providerUserRepository; private readonly ICaptchaValidationService _captchaValidationService; private readonly IPolicyService _policyService; + private readonly ISetInitialMasterPasswordCommand _setInitialMasterPasswordCommand; public AccountsControllerTests() { @@ -52,6 +54,8 @@ public AccountsControllerTests() _sendService = Substitute.For(); _captchaValidationService = Substitute.For(); _policyService = Substitute.For(); + _setInitialMasterPasswordCommand = Substitute.For(); + _sut = new AccountsController( _globalSettings, _cipherRepository, @@ -65,7 +69,8 @@ public AccountsControllerTests() _sendRepository, _sendService, _captchaValidationService, - _policyService + _policyService, + _setInitialMasterPasswordCommand ); } From 96cb464205af37e020acd352d19a77ab56a820dc Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Wed, 30 Aug 2023 18:56:05 -0400 Subject: [PATCH 04/80] PM-3275 - UserService.cs - Remove non implemented ChangePasswordAsync method --- src/Core/Services/Implementations/UserService.cs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index 024a4a8878db..47c0bc696abf 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -600,12 +600,6 @@ await _stripeSyncService.UpdateCustomerEmailAddress(user.GatewayCustomerId, return IdentityResult.Success; } - // TODO: delete this unused method - public override Task ChangePasswordAsync(User user, string masterPassword, string newMasterPassword) - { - throw new NotImplementedException(); - } - public async Task ChangePasswordAsync(User user, string masterPassword, string newMasterPassword, string passwordHint, string key) { From c6ce52217e53f07eaeaf137b392ba7582d4db6f8 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Thu, 31 Aug 2023 11:18:20 -0400 Subject: [PATCH 05/80] PM-3275 - The new SetInitialMasterPasswordCommand leveraged the OrganizationService.cs AcceptUserAsync method so while I was in here I converted the AcceptUserAsync methods into a new AcceptOrgUserCommand.cs and turned the private method which accepted an existing org user public for use in the SetInitialMasterPasswordCommand --- .../OrganizationUsersController.cs | 11 +- .../SetInitialMasterPasswordCommand.cs | 13 +- ...OrganizationServiceCollectionExtensions.cs | 1 + .../OrganizationUsers/AcceptOrgUserCommand.cs | 186 ++++++++++++++++++ .../Interfaces/IAcceptOrgUserCommand.cs | 12 ++ src/Core/Services/IOrganizationService.cs | 3 - .../Implementations/OrganizationService.cs | 147 -------------- .../Services/Implementations/UserService.cs | 9 +- .../OrganizationUsersControllerTests.cs | 13 +- test/Core.Test/Services/UserServiceTests.cs | 3 +- 10 files changed, 226 insertions(+), 172 deletions(-) create mode 100644 src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs create mode 100644 src/Core/OrganizationFeatures/OrganizationUsers/Interfaces/IAcceptOrgUserCommand.cs diff --git a/src/Api/Controllers/OrganizationUsersController.cs b/src/Api/Controllers/OrganizationUsersController.cs index 3dbaea3ff90e..fc7df1d12d5f 100644 --- a/src/Api/Controllers/OrganizationUsersController.cs +++ b/src/Api/Controllers/OrganizationUsersController.cs @@ -30,6 +30,7 @@ public class OrganizationUsersController : Controller private readonly ICurrentContext _currentContext; private readonly ICountNewSmSeatsRequiredQuery _countNewSmSeatsRequiredQuery; private readonly IUpdateSecretsManagerSubscriptionCommand _updateSecretsManagerSubscriptionCommand; + private readonly IAcceptOrgUserCommand _acceptOrgUserCommand; public OrganizationUsersController( IOrganizationRepository organizationRepository, @@ -41,7 +42,8 @@ public OrganizationUsersController( IPolicyRepository policyRepository, ICurrentContext currentContext, ICountNewSmSeatsRequiredQuery countNewSmSeatsRequiredQuery, - IUpdateSecretsManagerSubscriptionCommand updateSecretsManagerSubscriptionCommand) + IUpdateSecretsManagerSubscriptionCommand updateSecretsManagerSubscriptionCommand, + IAcceptOrgUserCommand acceptOrgUserCommand) { _organizationRepository = organizationRepository; _organizationUserRepository = organizationUserRepository; @@ -53,6 +55,7 @@ public OrganizationUsersController( _currentContext = currentContext; _countNewSmSeatsRequiredQuery = countNewSmSeatsRequiredQuery; _updateSecretsManagerSubscriptionCommand = updateSecretsManagerSubscriptionCommand; + _acceptOrgUserCommand = acceptOrgUserCommand; } [HttpGet("{id}")] @@ -194,7 +197,7 @@ public async Task AcceptInit(Guid orgId, Guid organizationUserId, [FromBody] Org } await _organizationService.InitPendingOrganization(user.Id, orgId, model.Keys.PublicKey, model.Keys.EncryptedPrivateKey, model.CollectionName); - await _organizationService.AcceptUserAsync(organizationUserId, user, model.Token, _userService); + await _acceptOrgUserCommand.AcceptOrgUserAsync(organizationUserId, user, model.Token, _userService); await _organizationService.ConfirmUserAsync(orgId, organizationUserId, model.Key, user.Id, _userService); } @@ -216,7 +219,7 @@ public async Task Accept(Guid orgId, Guid organizationUserId, [FromBody] Organiz throw new BadRequestException(string.Empty, "Master Password reset is required, but not provided."); } - await _organizationService.AcceptUserAsync(organizationUserId, user, model.Token, _userService); + await _acceptOrgUserCommand.AcceptOrgUserAsync(organizationUserId, user, model.Token, _userService); if (useMasterPasswordPolicy) { @@ -327,7 +330,7 @@ await _organizationService.UpdateUserResetPasswordEnrollmentAsync( var orgUser = await _organizationUserRepository.GetByOrganizationAsync(orgId, user.Id); if (orgUser.Status == OrganizationUserStatusType.Invited) { - await _organizationService.AcceptUserAsync(orgId, user, _userService); + await _acceptOrgUserCommand.AcceptOrgUserAsync(orgId, user, _userService); } } diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommand.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommand.cs index 4b5b9eccbba9..b43c1c19f867 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommand.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommand.cs @@ -2,6 +2,7 @@ using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; +using Bit.Core.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.Repositories; using Bit.Core.Services; using Microsoft.AspNetCore.Identity; @@ -10,7 +11,7 @@ namespace Bit.Core.Auth.UserFeatures.UserMasterPassword; /// -/// Manages the setting of the initial master password for a user in an organization. +/// Manages the setting of the initial master password for a in an organization. /// This class is primarily invoked in two scenarios: /// 1) In organizations configured with Single Sign-On (SSO) and master password decryption: /// just in time (JIT) provisioned users logging in via SSO are required to set a master password. @@ -25,7 +26,7 @@ public class SetInitialMasterPasswordCommand : ISetInitialMasterPasswordCommand private readonly IUserService _userService; private readonly IUserRepository _userRepository; private readonly IEventService _eventService; - private readonly IOrganizationService _organizationService; + private readonly IAcceptOrgUserCommand _acceptOrgUserCommand; private readonly IOrganizationUserRepository _organizationUserRepository; private readonly IOrganizationRepository _organizationRepository; @@ -35,7 +36,7 @@ public SetInitialMasterPasswordCommand( ILogger IUserService userService, IUserRepository userRepository, IEventService eventService, - IOrganizationService organizationService, + IAcceptOrgUserCommand acceptOrgUserCommand, IOrganizationUserRepository organizationUserRepository, IOrganizationRepository organizationRepository) { @@ -44,7 +45,7 @@ public SetInitialMasterPasswordCommand( ILogger _userService = userService; _userRepository = userRepository; _eventService = eventService; - _organizationService = organizationService; + _acceptOrgUserCommand = acceptOrgUserCommand; _organizationUserRepository = organizationUserRepository; _organizationRepository = organizationRepository; } @@ -95,9 +96,7 @@ public async Task SetInitialMasterPasswordAsync(User user, strin // TLDR: only accept post SSO user if they are invited if (!string.IsNullOrWhiteSpace(orgIdentifier) && orgUser.Status == OrganizationUserStatusType.Invited) { - // TODO: should pass org user to AcceptUserAsync - // TODO: convert AcceptUserAsync to own command AcceptOrgUserCommand - await _organizationService.AcceptUserAsync(orgUser, user, _userService); + await _acceptOrgUserCommand.AcceptOrgUserAsync(orgUser, user, _userService); } return IdentityResult.Success; diff --git a/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs b/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs index a534ed5bea2d..f8be72c6cfd4 100644 --- a/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs +++ b/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs @@ -124,6 +124,7 @@ private static void AddOrganizationAuthCommands(this IServiceCollection services private static void AddOrganizationUserCommandsQueries(this IServiceCollection services) { services.AddScoped(); + services.AddScoped(); } // TODO: move to OrganizationSubscriptionServiceCollectionExtensions when OrganizationUser methods are moved out of diff --git a/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs b/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs new file mode 100644 index 000000000000..de2b39eceaba --- /dev/null +++ b/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs @@ -0,0 +1,186 @@ +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Exceptions; +using Bit.Core.OrganizationFeatures.OrganizationUsers.Interfaces; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Bit.Core.Settings; +using Bit.Core.Utilities; +using Microsoft.AspNetCore.DataProtection; + +namespace Bit.Core.OrganizationFeatures.OrganizationUsers; + +public class AcceptOrgUserCommand : IAcceptOrgUserCommand +{ + private readonly IDataProtector _dataProtector; + private readonly IGlobalSettings _globalSettings; + private readonly IOrganizationUserRepository _organizationUserRepository; + private readonly IOrganizationRepository _organizationRepository; + private readonly IPolicyService _policyService; + private readonly IMailService _mailService; + + public AcceptOrgUserCommand( + IDataProtectionProvider dataProtectionProvider, + IGlobalSettings globalSettings, + IOrganizationUserRepository organizationUserRepository, + IOrganizationRepository organizationRepository, + IPolicyService policyService, + IMailService mailService) + { + + _dataProtector = dataProtectionProvider.CreateProtector("OrganizationServiceDataProtector"); + _globalSettings = globalSettings; + _organizationUserRepository = organizationUserRepository; + _organizationRepository = organizationRepository; + _policyService = policyService; + _mailService = mailService; + } + + public async Task AcceptOrgUserAsync(Guid organizationUserId, User user, string token, + IUserService userService) + { + var orgUser = await _organizationUserRepository.GetByIdAsync(organizationUserId); + if (orgUser == null) + { + throw new BadRequestException("User invalid."); + } + + if (!CoreHelpers.UserInviteTokenIsValid(_dataProtector, token, user.Email, orgUser.Id, _globalSettings)) + { + throw new BadRequestException("Invalid token."); + } + + var existingOrgUserCount = await _organizationUserRepository.GetCountByOrganizationAsync( + orgUser.OrganizationId, user.Email, true); + if (existingOrgUserCount > 0) + { + if (orgUser.Status == OrganizationUserStatusType.Accepted) + { + throw new BadRequestException("Invitation already accepted. You will receive an email when your organization membership is confirmed."); + } + throw new BadRequestException("You are already part of this organization."); + } + + if (string.IsNullOrWhiteSpace(orgUser.Email) || + !orgUser.Email.Equals(user.Email, StringComparison.InvariantCultureIgnoreCase)) + { + throw new BadRequestException("User email does not match invite."); + } + + return await AcceptOrgUserAsync(orgUser, user, userService); + } + + public async Task AcceptOrgUserAsync(string orgIdentifier, User user, IUserService userService) + { + var org = await _organizationRepository.GetByIdentifierAsync(orgIdentifier); + if (org == null) + { + throw new BadRequestException("Organization invalid."); + } + + var orgUser = await _organizationUserRepository.GetByOrganizationAsync(org.Id, user.Id); + if (orgUser == null) + { + throw new BadRequestException("User not found within organization."); + } + + return await AcceptOrgUserAsync(orgUser, user, userService); + } + + public async Task AcceptOrgUserAsync(Guid organizationId, User user, IUserService userService) + { + var org = await _organizationRepository.GetByIdAsync(organizationId); + if (org == null) + { + throw new BadRequestException("Organization invalid."); + } + + var usersOrgs = await _organizationUserRepository.GetManyByUserAsync(user.Id); + var orgUser = usersOrgs.FirstOrDefault(u => u.OrganizationId == org.Id); + if (orgUser == null) + { + throw new BadRequestException("User not found within organization."); + } + + return await AcceptOrgUserAsync(orgUser, user, userService); + } + + public async Task AcceptOrgUserAsync(OrganizationUser orgUser, User user, + IUserService userService) + { + if (orgUser.Status == OrganizationUserStatusType.Revoked) + { + throw new BadRequestException("Your organization access has been revoked."); + } + + if (orgUser.Status != OrganizationUserStatusType.Invited) + { + throw new BadRequestException("Already accepted."); + } + + if (orgUser.Type == OrganizationUserType.Owner || orgUser.Type == OrganizationUserType.Admin) + { + var org = await _organizationRepository.GetByIdAsync(orgUser.OrganizationId); + if (org.PlanType == PlanType.Free) + { + var adminCount = await _organizationUserRepository.GetCountByFreeOrganizationAdminUserAsync( + user.Id); + if (adminCount > 0) + { + throw new BadRequestException("You can only be an admin of one free organization."); + } + } + } + + // Enforce Single Organization Policy of organization user is trying to join + var allOrgUsers = await _organizationUserRepository.GetManyByUserAsync(user.Id); + var hasOtherOrgs = allOrgUsers.Any(ou => ou.OrganizationId != orgUser.OrganizationId); + var invitedSingleOrgPolicies = await _policyService.GetPoliciesApplicableToUserAsync(user.Id, + PolicyType.SingleOrg, OrganizationUserStatusType.Invited); + + if (hasOtherOrgs && invitedSingleOrgPolicies.Any(p => p.OrganizationId == orgUser.OrganizationId)) + { + throw new BadRequestException("You may not join this organization until you leave or remove " + + "all other organizations."); + } + + // Enforce Single Organization Policy of other organizations user is a member of + var anySingleOrgPolicies = await _policyService.AnyPoliciesApplicableToUserAsync(user.Id, + PolicyType.SingleOrg); + if (anySingleOrgPolicies) + { + throw new BadRequestException("You cannot join this organization because you are a member of " + + "another organization which forbids it"); + } + + // Enforce Two Factor Authentication Policy of organization user is trying to join + if (!await userService.TwoFactorIsEnabledAsync(user)) + { + var invitedTwoFactorPolicies = await _policyService.GetPoliciesApplicableToUserAsync(user.Id, + PolicyType.TwoFactorAuthentication, OrganizationUserStatusType.Invited); + if (invitedTwoFactorPolicies.Any(p => p.OrganizationId == orgUser.OrganizationId)) + { + throw new BadRequestException("You cannot join this organization until you enable " + + "two-step login on your user account."); + } + } + + orgUser.Status = OrganizationUserStatusType.Accepted; + orgUser.UserId = user.Id; + orgUser.Email = null; + + await _organizationUserRepository.ReplaceAsync(orgUser); + + var admins = await _organizationUserRepository.GetManyByMinimumRoleAsync(orgUser.OrganizationId, OrganizationUserType.Admin); + var adminEmails = admins.Select(a => a.Email).Distinct().ToList(); + + if (adminEmails.Count > 0) + { + var organization = await _organizationRepository.GetByIdAsync(orgUser.OrganizationId); + await _mailService.SendOrganizationAcceptedEmailAsync(organization, user.Email, adminEmails); + } + + return orgUser; + } + +} diff --git a/src/Core/OrganizationFeatures/OrganizationUsers/Interfaces/IAcceptOrgUserCommand.cs b/src/Core/OrganizationFeatures/OrganizationUsers/Interfaces/IAcceptOrgUserCommand.cs new file mode 100644 index 000000000000..5f2c2a7a4cc3 --- /dev/null +++ b/src/Core/OrganizationFeatures/OrganizationUsers/Interfaces/IAcceptOrgUserCommand.cs @@ -0,0 +1,12 @@ +using Bit.Core.Entities; +using Bit.Core.Services; + +namespace Bit.Core.OrganizationFeatures.OrganizationUsers.Interfaces; + +public interface IAcceptOrgUserCommand +{ + Task AcceptOrgUserAsync(Guid organizationUserId, User user, string token, IUserService userService); + Task AcceptOrgUserAsync(string orgIdentifier, User user, IUserService userService); + Task AcceptOrgUserAsync(Guid organizationId, User user, IUserService userService); + Task AcceptOrgUserAsync(OrganizationUser orgUser, User user, IUserService userService); +} diff --git a/src/Core/Services/IOrganizationService.cs b/src/Core/Services/IOrganizationService.cs index 832aa9de4c1f..f60afe4ae339 100644 --- a/src/Core/Services/IOrganizationService.cs +++ b/src/Core/Services/IOrganizationService.cs @@ -39,9 +39,6 @@ Task InviteUserAsync(Guid organizationId, EventSystemUser syst OrganizationUserType type, bool accessAll, string externalId, IEnumerable collections, IEnumerable groups); Task>> ResendInvitesAsync(Guid organizationId, Guid? invitingUserId, IEnumerable organizationUsersId); Task ResendInviteAsync(Guid organizationId, Guid? invitingUserId, Guid organizationUserId, bool initOrganization = false); - Task AcceptUserAsync(Guid organizationUserId, User user, string token, IUserService userService); - Task AcceptUserAsync(string orgIdentifier, User user, IUserService userService); - Task AcceptUserAsync(Guid organizationId, User user, IUserService userService); Task ConfirmUserAsync(Guid organizationId, Guid organizationUserId, string key, Guid confirmingUserId, IUserService userService); Task>> ConfirmUsersAsync(Guid organizationId, Dictionary keys, diff --git a/src/Core/Services/Implementations/OrganizationService.cs b/src/Core/Services/Implementations/OrganizationService.cs index 38bf991ef779..a114333036af 100644 --- a/src/Core/Services/Implementations/OrganizationService.cs +++ b/src/Core/Services/Implementations/OrganizationService.cs @@ -1062,153 +1062,6 @@ private async Task SendInviteAsync(OrganizationUser orgUser, Organization organi await _mailService.SendOrganizationInviteEmailAsync(organization.Name, orgUser, new ExpiringToken(token, now.AddDays(5)), organization.PlanType == PlanType.Free, initOrganization); } - public async Task AcceptUserAsync(Guid organizationUserId, User user, string token, - IUserService userService) - { - var orgUser = await _organizationUserRepository.GetByIdAsync(organizationUserId); - if (orgUser == null) - { - throw new BadRequestException("User invalid."); - } - - if (!CoreHelpers.UserInviteTokenIsValid(_dataProtector, token, user.Email, orgUser.Id, _globalSettings)) - { - throw new BadRequestException("Invalid token."); - } - - var existingOrgUserCount = await _organizationUserRepository.GetCountByOrganizationAsync( - orgUser.OrganizationId, user.Email, true); - if (existingOrgUserCount > 0) - { - if (orgUser.Status == OrganizationUserStatusType.Accepted) - { - throw new BadRequestException("Invitation already accepted. You will receive an email when your organization membership is confirmed."); - } - throw new BadRequestException("You are already part of this organization."); - } - - if (string.IsNullOrWhiteSpace(orgUser.Email) || - !orgUser.Email.Equals(user.Email, StringComparison.InvariantCultureIgnoreCase)) - { - throw new BadRequestException("User email does not match invite."); - } - - return await AcceptUserAsync(orgUser, user, userService); - } - - public async Task AcceptUserAsync(string orgIdentifier, User user, IUserService userService) - { - var org = await _organizationRepository.GetByIdentifierAsync(orgIdentifier); - if (org == null) - { - throw new BadRequestException("Organization invalid."); - } - - var orgUser = await _organizationUserRepository.GetByOrganizationAsync(org.Id, user.Id); - if (orgUser == null) - { - throw new BadRequestException("User not found within organization."); - } - - return await AcceptUserAsync(orgUser, user, userService); - } - - public async Task AcceptUserAsync(Guid organizationId, User user, IUserService userService) - { - var org = await _organizationRepository.GetByIdAsync(organizationId); - if (org == null) - { - throw new BadRequestException("Organization invalid."); - } - - var usersOrgs = await _organizationUserRepository.GetManyByUserAsync(user.Id); - var orgUser = usersOrgs.FirstOrDefault(u => u.OrganizationId == org.Id); - if (orgUser == null) - { - throw new BadRequestException("User not found within organization."); - } - - return await AcceptUserAsync(orgUser, user, userService); - } - - private async Task AcceptUserAsync(OrganizationUser orgUser, User user, - IUserService userService) - { - if (orgUser.Status == OrganizationUserStatusType.Revoked) - { - throw new BadRequestException("Your organization access has been revoked."); - } - - if (orgUser.Status != OrganizationUserStatusType.Invited) - { - throw new BadRequestException("Already accepted."); - } - - if (orgUser.Type == OrganizationUserType.Owner || orgUser.Type == OrganizationUserType.Admin) - { - var org = await GetOrgById(orgUser.OrganizationId); - if (org.PlanType == PlanType.Free) - { - var adminCount = await _organizationUserRepository.GetCountByFreeOrganizationAdminUserAsync( - user.Id); - if (adminCount > 0) - { - throw new BadRequestException("You can only be an admin of one free organization."); - } - } - } - - // Enforce Single Organization Policy of organization user is trying to join - var allOrgUsers = await _organizationUserRepository.GetManyByUserAsync(user.Id); - var hasOtherOrgs = allOrgUsers.Any(ou => ou.OrganizationId != orgUser.OrganizationId); - var invitedSingleOrgPolicies = await _policyService.GetPoliciesApplicableToUserAsync(user.Id, - PolicyType.SingleOrg, OrganizationUserStatusType.Invited); - - if (hasOtherOrgs && invitedSingleOrgPolicies.Any(p => p.OrganizationId == orgUser.OrganizationId)) - { - throw new BadRequestException("You may not join this organization until you leave or remove " + - "all other organizations."); - } - - // Enforce Single Organization Policy of other organizations user is a member of - var anySingleOrgPolicies = await _policyService.AnyPoliciesApplicableToUserAsync(user.Id, - PolicyType.SingleOrg); - if (anySingleOrgPolicies) - { - throw new BadRequestException("You cannot join this organization because you are a member of " + - "another organization which forbids it"); - } - - // Enforce Two Factor Authentication Policy of organization user is trying to join - if (!await userService.TwoFactorIsEnabledAsync(user)) - { - var invitedTwoFactorPolicies = await _policyService.GetPoliciesApplicableToUserAsync(user.Id, - PolicyType.TwoFactorAuthentication, OrganizationUserStatusType.Invited); - if (invitedTwoFactorPolicies.Any(p => p.OrganizationId == orgUser.OrganizationId)) - { - throw new BadRequestException("You cannot join this organization until you enable " + - "two-step login on your user account."); - } - } - - orgUser.Status = OrganizationUserStatusType.Accepted; - orgUser.UserId = user.Id; - orgUser.Email = null; - - await _organizationUserRepository.ReplaceAsync(orgUser); - - var admins = await _organizationUserRepository.GetManyByMinimumRoleAsync(orgUser.OrganizationId, OrganizationUserType.Admin); - var adminEmails = admins.Select(a => a.Email).Distinct().ToList(); - - if (adminEmails.Count > 0) - { - var organization = await _organizationRepository.GetByIdAsync(orgUser.OrganizationId); - await _mailService.SendOrganizationAcceptedEmailAsync(organization, user.Email, adminEmails); - } - - return orgUser; - } - public async Task ConfirmUserAsync(Guid organizationId, Guid organizationUserId, string key, Guid confirmingUserId, IUserService userService) { diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index 47c0bc696abf..b15058e4ec47 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -7,6 +7,7 @@ using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.Models.Business; +using Bit.Core.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.Repositories; using Bit.Core.Settings; using Bit.Core.Tools.Entities; @@ -52,7 +53,7 @@ public class UserService : UserManager, IUserService, IDisposable private readonly IFido2 _fido2; private readonly ICurrentContext _currentContext; private readonly IGlobalSettings _globalSettings; - private readonly IOrganizationService _organizationService; + private readonly IAcceptOrgUserCommand _acceptOrgUserCommand; private readonly IProviderUserRepository _providerUserRepository; private readonly IStripeSyncService _stripeSyncService; @@ -83,7 +84,7 @@ public UserService( IFido2 fido2, ICurrentContext currentContext, IGlobalSettings globalSettings, - IOrganizationService organizationService, + IAcceptOrgUserCommand acceptOrgUserCommand, IProviderUserRepository providerUserRepository, IStripeSyncService stripeSyncService) : base( @@ -119,7 +120,7 @@ public UserService( _fido2 = fido2; _currentContext = currentContext; _globalSettings = globalSettings; - _organizationService = organizationService; + _acceptOrgUserCommand = acceptOrgUserCommand; _providerUserRepository = providerUserRepository; _stripeSyncService = stripeSyncService; } @@ -648,7 +649,7 @@ public async Task SetKeyConnectorKeyAsync(User user, string key, await _userRepository.ReplaceAsync(user); await _eventService.LogUserEventAsync(user.Id, EventType.User_MigratedKeyToKeyConnector); - await _organizationService.AcceptUserAsync(orgIdentifier, user, this); + await _acceptOrgUserCommand.AcceptOrgUserAsync(orgIdentifier, user, this); return IdentityResult.Success; } diff --git a/test/Api.Test/Controllers/OrganizationUsersControllerTests.cs b/test/Api.Test/Controllers/OrganizationUsersControllerTests.cs index 6053d17ddba7..07b10db6956e 100644 --- a/test/Api.Test/Controllers/OrganizationUsersControllerTests.cs +++ b/test/Api.Test/Controllers/OrganizationUsersControllerTests.cs @@ -2,6 +2,7 @@ using Bit.Api.Models.Request.Organizations; using Bit.Core.Entities; using Bit.Core.Models.Data.Organizations.Policies; +using Bit.Core.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.Repositories; using Bit.Core.Services; using Bit.Core.Utilities; @@ -27,7 +28,7 @@ public async Task PutResetPasswordEnrollment_InivitedUser_AcceptsInvite(Guid org await sutProvider.Sut.PutResetPasswordEnrollment(orgId, userId, model); - await sutProvider.GetDependency().Received(1).AcceptUserAsync(orgId, user, sutProvider.GetDependency()); + await sutProvider.GetDependency().Received(1).AcceptOrgUserAsync(orgId, user, sutProvider.GetDependency()); } [Theory] @@ -41,7 +42,7 @@ public async Task PutResetPasswordEnrollment_ConfirmedUser_AcceptsInvite(Guid or await sutProvider.Sut.PutResetPasswordEnrollment(orgId, userId, model); - await sutProvider.GetDependency().Received(0).AcceptUserAsync(orgId, user, sutProvider.GetDependency()); + await sutProvider.GetDependency().Received(0).AcceptOrgUserAsync(orgId, user, sutProvider.GetDependency()); } [Theory] @@ -63,8 +64,8 @@ public async Task Accept_NoMasterPasswordReset(Guid orgId, Guid orgUserId, await sutProvider.Sut.Accept(orgId, orgUserId, model); - await sutProvider.GetDependency().Received(1) - .AcceptUserAsync(orgUserId, user, model.Token, sutProvider.GetDependency()); + await sutProvider.GetDependency().Received(1) + .AcceptOrgUserAsync(orgUserId, user, model.Token, sutProvider.GetDependency()); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() .UpdateUserResetPasswordEnrollmentAsync(default, default, default, default); } @@ -85,8 +86,8 @@ public async Task Accept_RequireMasterPasswordReset(Guid orgId, Guid orgUserId, await sutProvider.Sut.Accept(orgId, orgUserId, model); - await sutProvider.GetDependency().Received(1) - .AcceptUserAsync(orgUserId, user, model.Token, sutProvider.GetDependency()); + await sutProvider.GetDependency().Received(1) + .AcceptOrgUserAsync(orgUserId, user, model.Token, sutProvider.GetDependency()); await sutProvider.GetDependency().Received(1) .UpdateUserResetPasswordEnrollmentAsync(orgId, user.Id, model.ResetPasswordKey, user.Id); } diff --git a/test/Core.Test/Services/UserServiceTests.cs b/test/Core.Test/Services/UserServiceTests.cs index 92de67a4e62e..107fd55ebafb 100644 --- a/test/Core.Test/Services/UserServiceTests.cs +++ b/test/Core.Test/Services/UserServiceTests.cs @@ -5,6 +5,7 @@ using Bit.Core.Entities; using Bit.Core.Models.Business; using Bit.Core.Models.Data.Organizations; +using Bit.Core.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.Repositories; using Bit.Core.Services; using Bit.Core.Settings; @@ -251,7 +252,7 @@ public async Task VerifySecretAsync_Works( sutProvider.GetDependency(), sutProvider.GetDependency(), sutProvider.GetDependency(), - sutProvider.GetDependency(), + sutProvider.GetDependency(), sutProvider.GetDependency(), sutProvider.GetDependency()); From 6ccd5b52787676aead7a53731f57e1747831a04c Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Thu, 31 Aug 2023 12:05:07 -0400 Subject: [PATCH 06/80] PM-3275 - Dotnet format --- .../UserMasterPassword/SetInitialMasterPasswordCommand.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommand.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommand.cs index b43c1c19f867..a53522733e40 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommand.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommand.cs @@ -31,7 +31,7 @@ public class SetInitialMasterPasswordCommand : ISetInitialMasterPasswordCommand private readonly IOrganizationRepository _organizationRepository; - public SetInitialMasterPasswordCommand( ILogger logger, + public SetInitialMasterPasswordCommand(ILogger logger, IdentityErrorDescriber identityErrorDescriber, IUserService userService, IUserRepository userRepository, From 34ea7408c9300a653abb6da4a371c45cccc02677 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Thu, 31 Aug 2023 16:03:16 -0400 Subject: [PATCH 07/80] PM-3275 - Test SetInitialMasterPasswordCommand --- .../SetInitialMasterPasswordCommandTests.cs | 179 ++++++++++++++++++ 1 file changed, 179 insertions(+) create mode 100644 test/Core.Test/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommandTests.cs diff --git a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommandTests.cs b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommandTests.cs new file mode 100644 index 000000000000..4a91c35f0952 --- /dev/null +++ b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommandTests.cs @@ -0,0 +1,179 @@ +using Bit.Core.Auth.UserFeatures.UserMasterPassword; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Exceptions; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using Microsoft.AspNetCore.Identity; +using NSubstitute; +using Bit.Core.OrganizationFeatures.OrganizationUsers.Interfaces; +using NSubstitute.ReturnsExtensions; +using Xunit; + +namespace Bit.Core.Test.Auth.UserFeatures.UserMasterPassword; + +[SutProviderCustomize] +public class SetInitialMasterPasswordCommandTests +{ + [Theory] + [BitAutoData] + public async Task SetInitialMasterPassword_Success(SutProvider sutProvider, + User user, string masterPassword, string key, string orgIdentifier, + Organization org, OrganizationUser orgUser) + { + // Arrange + user.MasterPassword = null; + var identityResult = IdentityResult.Success; + + sutProvider.GetDependency() + .UpdatePasswordHash(Arg.Any(), Arg.Any(), true, false) + .Returns(identityResult); + + sutProvider.GetDependency() + .GetByIdentifierAsync(orgIdentifier) + .Returns(org); + + sutProvider.GetDependency() + .GetByOrganizationAsync(org.Id, user.Id) + .Returns(orgUser); + + // Act + var result = await sutProvider.Sut.SetInitialMasterPasswordAsync(user, masterPassword, key, orgIdentifier); + + // Assert + Assert.Equal(IdentityResult.Success, result); + } + + [Theory] + [BitAutoData] + public async Task SetInitialMasterPassword_UserIsNull_ThrowsArgumentNullException(SutProvider sutProvider, string masterPassword, string key, string orgIdentifier) + { + // Act & Assert + await Assert.ThrowsAsync(async () => await sutProvider.Sut.SetInitialMasterPasswordAsync(null, masterPassword, key, orgIdentifier)); + } + + [Theory] + [BitAutoData] + public async Task SetInitialMasterPassword_AlreadyHasPassword(SutProvider sutProvider, User user, string masterPassword, string key, string orgIdentifier) + { + // Arrange + user.MasterPassword = "ExistingPassword"; + + // Act + var result = await sutProvider.Sut.SetInitialMasterPasswordAsync(user, masterPassword, key, orgIdentifier); + + // Assert + Assert.False(result.Succeeded); + } + + + [Theory] + [BitAutoData] + public async Task SetInitialMasterPassword_InvalidOrganization_Throws(SutProvider sutProvider, User user, string masterPassword, string key, string orgIdentifier) + { + // Arrange + user.MasterPassword = null; + var identityResult = IdentityResult.Success; + + sutProvider.GetDependency() + .UpdatePasswordHash(Arg.Any(), Arg.Any(), true, false) + .Returns(identityResult); + + sutProvider.GetDependency() + .GetByIdentifierAsync(orgIdentifier) + .ReturnsNull(); + + // Act & Assert + var exception = await Assert.ThrowsAsync(async () => await sutProvider.Sut.SetInitialMasterPasswordAsync(user, masterPassword, key, orgIdentifier)); + Assert.Equal("Organization invalid.", exception.Message); + } + + [Theory] + [BitAutoData] + public async Task SetInitialMasterPassword_UserNotFoundInOrganization_Throws(SutProvider sutProvider, User user, string masterPassword, string key, Organization org) + { + // Arrange + user.MasterPassword = null; + var identityResult = IdentityResult.Success; + + sutProvider.GetDependency() + .UpdatePasswordHash(Arg.Any(), Arg.Any(), true, false) + .Returns(identityResult); + + sutProvider.GetDependency() + .GetByIdentifierAsync(Arg.Any()) + .Returns(org); + + sutProvider.GetDependency() + .GetByOrganizationAsync(org.Id, user.Id) + .ReturnsNull(); + + // Act & Assert + var exception = await Assert.ThrowsAsync(async () => await sutProvider.Sut.SetInitialMasterPasswordAsync(user, masterPassword, key, org.Identifier)); + Assert.Equal("User not found within organization.", exception.Message); + } + + [Theory] + [BitAutoData] + public async Task SetInitialMasterPassword_ConfirmedOrgUser_DoesNotCallAcceptOrgUser(SutProvider sutProvider, + User user, string masterPassword, string key, string orgIdentifier, Organization org, OrganizationUser orgUser) + { + // Arrange + user.MasterPassword = null; + var identityResult = IdentityResult.Success; + + sutProvider.GetDependency() + .UpdatePasswordHash(Arg.Any(), Arg.Any(), true, false) + .Returns(identityResult); + + sutProvider.GetDependency() + .GetByIdentifierAsync(orgIdentifier) + .Returns(org); + + orgUser.Status = OrganizationUserStatusType.Confirmed; + sutProvider.GetDependency() + .GetByOrganizationAsync(org.Id, user.Id) + .Returns(orgUser); + + + // Act + var result = await sutProvider.Sut.SetInitialMasterPasswordAsync(user, masterPassword, key, orgIdentifier); + + // Assert + Assert.Equal(IdentityResult.Success, result); + await sutProvider.GetDependency().DidNotReceive().AcceptOrgUserAsync(Arg.Any(), Arg.Any(), Arg.Any()); + } + + [Theory] + [BitAutoData] + public async Task SetInitialMasterPassword_InvitedOrgUser_CallsAcceptOrgUser(SutProvider sutProvider, + User user, string masterPassword, string key, string orgIdentifier, Organization org, OrganizationUser orgUser) + { + // Arrange + user.MasterPassword = null; + var identityResult = IdentityResult.Success; + + sutProvider.GetDependency() + .UpdatePasswordHash(Arg.Any(), Arg.Any(), true, false) + .Returns(identityResult); + + sutProvider.GetDependency() + .GetByIdentifierAsync(orgIdentifier) + .Returns(org); + + orgUser.Status = OrganizationUserStatusType.Invited; + sutProvider.GetDependency() + .GetByOrganizationAsync(org.Id, user.Id) + .Returns(orgUser); + + // Act + var result = await sutProvider.Sut.SetInitialMasterPasswordAsync(user, masterPassword, key, orgIdentifier); + + // Assert + Assert.Equal(IdentityResult.Success, result); + await sutProvider.GetDependency().Received(1).AcceptOrgUserAsync(orgUser, user, sutProvider.GetDependency()); + } + +} From 505c2c226b18ee71f822ad24f5760e0396c196d2 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Mon, 4 Sep 2023 13:17:57 -0400 Subject: [PATCH 08/80] Dotnet format --- .../ISetInitialMasterPasswordCommand.cs | 0 .../OrganizationUsers/AcceptOrgUserCommand.cs | 1 - 2 files changed, 1 deletion(-) rename src/Core/Auth/UserFeatures/UserMasterPassword/{interfaces => Interfaces}/ISetInitialMasterPasswordCommand.cs (100%) diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/interfaces/ISetInitialMasterPasswordCommand.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/ISetInitialMasterPasswordCommand.cs similarity index 100% rename from src/Core/Auth/UserFeatures/UserMasterPassword/interfaces/ISetInitialMasterPasswordCommand.cs rename to src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/ISetInitialMasterPasswordCommand.cs diff --git a/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs b/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs index de2b39eceaba..53a8ca3b46ef 100644 --- a/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs +++ b/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs @@ -182,5 +182,4 @@ public async Task AcceptOrgUserAsync(OrganizationUser orgUser, return orgUser; } - } From 4da9fb28f7550b20ec274c59eb86a762905b88c2 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Mon, 4 Sep 2023 13:51:27 -0400 Subject: [PATCH 09/80] PM-3275 - In process AcceptOrgUserCommandTests.cs --- .../AcceptOrgUserCommandTests.cs | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs diff --git a/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs b/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs new file mode 100644 index 000000000000..6071a8d0cb3c --- /dev/null +++ b/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs @@ -0,0 +1,50 @@ +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.OrganizationFeatures.OrganizationUsers; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Bit.Core.Settings; +using Bit.Core.Utilities; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using Microsoft.AspNetCore.DataProtection; +using NSubstitute; +using Xunit; + +namespace Bit.Core.Test.OrganizationFeatures.OrganizationUsers; + +[SutProviderCustomize] +public class AcceptOrgUserCommandTests +{ + private readonly IUserService _userService; + + public AcceptOrgUserCommandTests() + { + _userService = Substitute.For(); + } + + [Theory] + [BitAutoData] + public async Task AcceptOrgUser_ByOrgUserId_Success(SutProvider sutProvider, + User user, Guid organizationUserId, string token, OrganizationUser orgUser) + { + // Arrange + orgUser.Status = OrganizationUserStatusType.Invited; + sutProvider.GetDependency() + .GetByIdAsync(organizationUserId) + .Returns(orgUser); + + // TODO: can't mock static methods + // See CoreHelpersTests to figure out how to properly mock valid token + CoreHelpers.UserInviteTokenIsValid(Arg.Any(), token, user.Email, orgUser.Id, + Arg.Any()) + .Returns(true); + + // Act + var result = await sutProvider.Sut.AcceptOrgUserAsync(organizationUserId, user, token, _userService); + + + // Assert + // Assert.Equal(IdentityResult.Success, result); + } +} From 083e41e089754610fdbcec07147b98462d8f5639 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Wed, 13 Sep 2023 15:06:55 -0400 Subject: [PATCH 10/80] PM-3275 - Migrate changes from AC-244 / #3199 over into new AcceptOrgUserCommand --- .../OrganizationUsers/AcceptOrgUserCommand.cs | 16 ++++++++++++++-- .../Interfaces/IAcceptOrgUserCommand.cs | 6 ++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs b/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs index 53a8ca3b46ef..69478f89a865 100644 --- a/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs +++ b/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs @@ -18,6 +18,7 @@ public class AcceptOrgUserCommand : IAcceptOrgUserCommand private readonly IOrganizationRepository _organizationRepository; private readonly IPolicyService _policyService; private readonly IMailService _mailService; + private readonly IUserRepository _userRepository; public AcceptOrgUserCommand( IDataProtectionProvider dataProtectionProvider, @@ -25,7 +26,8 @@ public AcceptOrgUserCommand( IOrganizationUserRepository organizationUserRepository, IOrganizationRepository organizationRepository, IPolicyService policyService, - IMailService mailService) + IMailService mailService, + IUserRepository userRepository) { _dataProtector = dataProtectionProvider.CreateProtector("OrganizationServiceDataProtector"); @@ -34,6 +36,7 @@ public AcceptOrgUserCommand( _organizationRepository = organizationRepository; _policyService = policyService; _mailService = mailService; + _userRepository = userRepository; } public async Task AcceptOrgUserAsync(Guid organizationUserId, User user, string token, @@ -67,7 +70,16 @@ public async Task AcceptOrgUserAsync(Guid organizationUserId, throw new BadRequestException("User email does not match invite."); } - return await AcceptOrgUserAsync(orgUser, user, userService); + var organizationUser = await AcceptOrgUserAsync(orgUser, user, userService); + + // Verify user email if they accept org invite via email link + if (user.EmailVerified == false) + { + user.EmailVerified = true; + await _userRepository.ReplaceAsync(user); + } + + return organizationUser; } public async Task AcceptOrgUserAsync(string orgIdentifier, User user, IUserService userService) diff --git a/src/Core/OrganizationFeatures/OrganizationUsers/Interfaces/IAcceptOrgUserCommand.cs b/src/Core/OrganizationFeatures/OrganizationUsers/Interfaces/IAcceptOrgUserCommand.cs index 5f2c2a7a4cc3..6650f892c510 100644 --- a/src/Core/OrganizationFeatures/OrganizationUsers/Interfaces/IAcceptOrgUserCommand.cs +++ b/src/Core/OrganizationFeatures/OrganizationUsers/Interfaces/IAcceptOrgUserCommand.cs @@ -5,6 +5,12 @@ namespace Bit.Core.OrganizationFeatures.OrganizationUsers.Interfaces; public interface IAcceptOrgUserCommand { + /// + /// Moves an OrganizationUser into the Accepted status and marks their email as verified. + /// This method is used where the user has clicked the invitation link sent by email. + /// + /// The token embedded in the email invitation link + /// The accepted OrganizationUser. Task AcceptOrgUserAsync(Guid organizationUserId, User user, string token, IUserService userService); Task AcceptOrgUserAsync(string orgIdentifier, User user, IUserService userService); Task AcceptOrgUserAsync(Guid organizationId, User user, IUserService userService); From 724fdb0442eff08407e296c505a344608498c287 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Wed, 13 Sep 2023 15:53:57 -0400 Subject: [PATCH 11/80] PM-3275 - AcceptOrgUserCommand.cs - create data protector specifically for this command --- .../OrganizationUsers/AcceptOrgUserCommand.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs b/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs index 69478f89a865..61566d0f36fe 100644 --- a/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs +++ b/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs @@ -30,7 +30,7 @@ public AcceptOrgUserCommand( IUserRepository userRepository) { - _dataProtector = dataProtectionProvider.CreateProtector("OrganizationServiceDataProtector"); + _dataProtector = dataProtectionProvider.CreateProtector("AcceptOrgUserCommandDataProtector"); _globalSettings = globalSettings; _organizationUserRepository = organizationUserRepository; _organizationRepository = organizationRepository; From dea79ab3c78c60b2c0f57e2aefb71568f5c20322 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Wed, 13 Sep 2023 15:55:16 -0400 Subject: [PATCH 12/80] PM-3275 - Add TODO for renaming / removing overloading of methods to improve readability / clarity --- .../OrganizationUsers/AcceptOrgUserCommand.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs b/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs index 61566d0f36fe..c3156efeeec4 100644 --- a/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs +++ b/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs @@ -39,6 +39,10 @@ public AcceptOrgUserCommand( _userRepository = userRepository; } + // TODO: consider removing overloading and renaming based on purpose for increased clarity / readability + // AcceptOrgUserWithEmailedTokenAsync ? + // AcceptOrgUserByOrgIdentifierAsync ? + // AcceptOrgUserByOrgIdAsync ? public async Task AcceptOrgUserAsync(Guid organizationUserId, User user, string token, IUserService userService) { From a2f16233c341ffe6924752bbfb7cf1f83803e7f2 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Wed, 13 Sep 2023 15:57:36 -0400 Subject: [PATCH 13/80] PM-3275 - AcceptOrgUserCommand.cs - refactor AcceptOrgUserAsync by OrgId to retrieve orgUser with _organizationUserRepository.GetByOrganizationAsync which gets a single user instead of a collection --- .../OrganizationUsers/AcceptOrgUserCommand.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs b/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs index c3156efeeec4..8af6a05b3146 100644 --- a/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs +++ b/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs @@ -111,8 +111,7 @@ public async Task AcceptOrgUserAsync(Guid organizationId, User throw new BadRequestException("Organization invalid."); } - var usersOrgs = await _organizationUserRepository.GetManyByUserAsync(user.Id); - var orgUser = usersOrgs.FirstOrDefault(u => u.OrganizationId == org.Id); + var orgUser = await _organizationUserRepository.GetByOrganizationAsync(org.Id, user.Id); if (orgUser == null) { throw new BadRequestException("User not found within organization."); From bc9a46b9e694dcfe66fce272dd4f1f153a6bafac Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Wed, 13 Sep 2023 15:58:37 -0400 Subject: [PATCH 14/80] PM-3275 - AcceptOrgUserCommand.cs - update name in TODO for evaluation later --- .../OrganizationUsers/AcceptOrgUserCommand.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs b/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs index 8af6a05b3146..0fdc822b5642 100644 --- a/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs +++ b/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs @@ -40,7 +40,7 @@ public AcceptOrgUserCommand( } // TODO: consider removing overloading and renaming based on purpose for increased clarity / readability - // AcceptOrgUserWithEmailedTokenAsync ? + // AcceptOrgUserWithEmailedTokenAsync or AcceptOrgUserByOrgUserIdAndTokenAsync ? // AcceptOrgUserByOrgIdentifierAsync ? // AcceptOrgUserByOrgIdAsync ? public async Task AcceptOrgUserAsync(Guid organizationUserId, User user, string token, From 483ea389b638cab4554ecfaf14ff5f5bacaec26a Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Thu, 14 Sep 2023 00:43:20 -0400 Subject: [PATCH 15/80] PM-3275 / PM-1196 - (1) Slightly refactor SsoEmail2faSessionTokenable to provide public static GetTokenLifeTime() method for testing (2) Add missed tests to SsoEmail2faSessionTokenable in preparation for building tests for new OrgUserInviteTokenable.cs --- .../Tokenables/SsoEmail2faSessionTokenable.cs | 10 +- .../SsoEmail2faSessionTokenableTests.cs | 173 ++++++++++++++++++ 2 files changed, 179 insertions(+), 4 deletions(-) create mode 100644 test/Core.Test/Auth/Models/Business/Tokenables/SsoEmail2faSessionTokenableTests.cs diff --git a/src/Core/Auth/Models/Business/Tokenables/SsoEmail2faSessionTokenable.cs b/src/Core/Auth/Models/Business/Tokenables/SsoEmail2faSessionTokenable.cs index e8c8a33f569d..6dbb7bae3e4b 100644 --- a/src/Core/Auth/Models/Business/Tokenables/SsoEmail2faSessionTokenable.cs +++ b/src/Core/Auth/Models/Business/Tokenables/SsoEmail2faSessionTokenable.cs @@ -6,11 +6,13 @@ namespace Bit.Core.Auth.Models.Business.Tokenables; // This token just provides a verifiable authN mechanism for the API service // TwoFactorController.cs SendEmailLogin anonymous endpoint so it cannot be -// used maliciously. +// used maliciously. public class SsoEmail2faSessionTokenable : ExpiringTokenable { // Just over 2 min expiration (client expires session after 2 min) - private static readonly TimeSpan _tokenLifetime = TimeSpan.FromMinutes(2.05); + private const double _tokenLifetimeInMinutes = 2.05; + public static TimeSpan GetTokenLifetime() => TimeSpan.FromMinutes(_tokenLifetimeInMinutes); + public const string ClearTextPrefix = "BwSsoEmail2FaSessionToken_"; public const string DataProtectorPurpose = "SsoEmail2faSessionTokenDataProtector"; @@ -24,7 +26,7 @@ public class SsoEmail2faSessionTokenable : ExpiringTokenable [JsonConstructor] public SsoEmail2faSessionTokenable() { - ExpirationDate = DateTime.UtcNow.Add(_tokenLifetime); + ExpirationDate = DateTime.UtcNow.Add(GetTokenLifetime()); } public SsoEmail2faSessionTokenable(User user) : this() @@ -44,7 +46,7 @@ public bool TokenIsValid(User user) Email.Equals(user.Email, StringComparison.InvariantCultureIgnoreCase); } - // Validates deserialized + // Validates deserialized protected override bool TokenIsValid() => Identifier == TokenIdentifier && Id != default && !string.IsNullOrWhiteSpace(Email); } diff --git a/test/Core.Test/Auth/Models/Business/Tokenables/SsoEmail2faSessionTokenableTests.cs b/test/Core.Test/Auth/Models/Business/Tokenables/SsoEmail2faSessionTokenableTests.cs new file mode 100644 index 000000000000..0274b666c590 --- /dev/null +++ b/test/Core.Test/Auth/Models/Business/Tokenables/SsoEmail2faSessionTokenableTests.cs @@ -0,0 +1,173 @@ +using AutoFixture.Xunit2; +using Bit.Core.Auth.Models.Business.Tokenables; +using Bit.Core.Entities; +using Bit.Core.Tokens; +using Xunit; +namespace Bit.Core.Test.Auth.Models.Business.Tokenables; + +// Note: test names follow MethodName_StateUnderTest_ExpectedBehavior pattern. +public class SsoEmail2faSessionTokenableTests +{ + // Allow a small tolerance for possible execution delays or clock precision. + private readonly TimeSpan _timeTolerance = TimeSpan.FromMilliseconds(10); + + /// + /// Tests the default constructor behavior when passed a null user. + /// + [Fact] + public void Constructor_NullUser_PropertiesSetToDefault() + { + var token = new SsoEmail2faSessionTokenable(null); + + Assert.Equal(default, token.Id); + Assert.Equal(default, token.Email); + } + + /// + /// Tests that when a valid user is provided to the constructor, the resulting token properties match the user. + /// + [Theory, AutoData] + public void Constructor_ValidUser_PropertiesSetFromUser(User user) + { + var token = new SsoEmail2faSessionTokenable(user); + + Assert.Equal(user.Id, token.Id); + Assert.Equal(user.Email, token.Email); + } + + /// + /// Tests the default expiration behavior immediately after initialization. + /// + [Fact] + public void Constructor_AfterInitialization_ExpirationSetToExpectedDuration() + { + var token = new SsoEmail2faSessionTokenable(); + var expectedExpiration = DateTime.UtcNow + SsoEmail2faSessionTokenable.GetTokenLifetime(); + + Assert.True(expectedExpiration - token.ExpirationDate < _timeTolerance); + } + + /// + /// Tests that a custom expiration date is preserved after token initialization. + /// + [Fact] + public void Constructor_CustomExpirationDate_ExpirationMatchesProvidedValue() + { + var customExpiration = DateTime.UtcNow.AddHours(3); + var token = new SsoEmail2faSessionTokenable + { + ExpirationDate = customExpiration + }; + + Assert.True(Math.Abs((customExpiration - token.ExpirationDate).TotalMilliseconds) < _timeTolerance.TotalMilliseconds); + } + + /// + /// Tests the validity of a token initialized with a null user. + /// + [Fact] + public void Valid_NullUser_ReturnsFalse() + { + var token = new SsoEmail2faSessionTokenable(null); + + Assert.False(token.Valid); + } + + /// + /// Tests the validity of a token with a non-matching identifier. + /// + [Theory, AutoData] + public void Valid_WrongIdentifier_ReturnsFalse(User user) + { + var token = new SsoEmail2faSessionTokenable(user) + { + Identifier = "not correct" + }; + + Assert.False(token.Valid); + } + + /// + /// Tests the token validity when user ID is null. + /// + [Theory, AutoData] + public void TokenIsValid_NullUserId_ReturnsFalse(User user) + { + user.Id = default; // Guid.Empty + var token = new SsoEmail2faSessionTokenable(user); + + Assert.False(token.TokenIsValid(user)); + } + + /// + /// Tests the token validity when user's email is null. + /// + [Theory, AutoData] + public void TokenIsValid_NullEmail_ReturnsFalse(User user) + { + user.Email = null; + var token = new SsoEmail2faSessionTokenable(user); + + Assert.False(token.TokenIsValid(user)); + } + + /// + /// Tests the token validity when user ID and email match the token properties. + /// + [Theory, AutoData] + public void TokenIsValid_MatchingUserIdAndEmail_ReturnsTrue(User user) + { + var token = new SsoEmail2faSessionTokenable(user); + + Assert.True(token.TokenIsValid(user)); + } + + /// + /// Ensures that the token is invalid when the provided user's ID doesn't match the token's ID. + /// + [Theory, AutoData] + public void TokenIsValid_WrongUserId_ReturnsFalse(User user) + { + // Given a token initialized with a user's details + var token = new SsoEmail2faSessionTokenable(user); + + // modify the user's ID + user.Id = Guid.NewGuid(); + + // Then the token should be considered invalid + Assert.False(token.TokenIsValid(user)); + } + + /// + /// Ensures that the token is invalid when the provided user's email doesn't match the token's email. + /// + [Theory, AutoData] + public void TokenIsValid_WrongEmail_ReturnsFalse(User user) + { + // Given a token initialized with a user's details + var token = new SsoEmail2faSessionTokenable(user); + + // modify the user's email + user.Email = "nonMatchingEmail@example.com"; + + // Then the token should be considered invalid + Assert.False(token.TokenIsValid(user)); + } + + /// + /// Tests the deserialization of a token to ensure that the expiration date is preserved. + /// + [Theory, AutoData] + public void FromToken_SerializedToken_PreservesExpirationDate(User user) + { + var expectedDateTime = DateTime.UtcNow.AddHours(-5); + var token = new SsoEmail2faSessionTokenable(user) + { + ExpirationDate = expectedDateTime + }; + + var result = Tokenable.FromToken(token.ToToken()); + + Assert.Equal(expectedDateTime, result.ExpirationDate, precision: _timeTolerance); + } +} From bd0785ef0b700d07f13c3bc3033f87999abcfde8 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Thu, 14 Sep 2023 10:51:07 -0400 Subject: [PATCH 16/80] PM-3275 / PM-1196 - Removing SsoEmail2faSessionTokenable.cs changes + tests as I've handled that separately in a new PR (#3270) for newly created task PM-3925 --- .../Tokenables/SsoEmail2faSessionTokenable.cs | 11 +- .../SsoEmail2faSessionTokenableTests.cs | 173 ------------------ 2 files changed, 2 insertions(+), 182 deletions(-) delete mode 100644 test/Core.Test/Auth/Models/Business/Tokenables/SsoEmail2faSessionTokenableTests.cs diff --git a/src/Core/Auth/Models/Business/Tokenables/SsoEmail2faSessionTokenable.cs b/src/Core/Auth/Models/Business/Tokenables/SsoEmail2faSessionTokenable.cs index 6dbb7bae3e4b..7714abb283d7 100644 --- a/src/Core/Auth/Models/Business/Tokenables/SsoEmail2faSessionTokenable.cs +++ b/src/Core/Auth/Models/Business/Tokenables/SsoEmail2faSessionTokenable.cs @@ -10,23 +10,18 @@ namespace Bit.Core.Auth.Models.Business.Tokenables; public class SsoEmail2faSessionTokenable : ExpiringTokenable { // Just over 2 min expiration (client expires session after 2 min) - private const double _tokenLifetimeInMinutes = 2.05; - public static TimeSpan GetTokenLifetime() => TimeSpan.FromMinutes(_tokenLifetimeInMinutes); - + private static readonly TimeSpan _tokenLifetime = TimeSpan.FromMinutes(2.05); public const string ClearTextPrefix = "BwSsoEmail2FaSessionToken_"; public const string DataProtectorPurpose = "SsoEmail2faSessionTokenDataProtector"; public const string TokenIdentifier = "SsoEmail2faSessionToken"; - public string Identifier { get; set; } = TokenIdentifier; public Guid Id { get; set; } public string Email { get; set; } - - [JsonConstructor] public SsoEmail2faSessionTokenable() { - ExpirationDate = DateTime.UtcNow.Add(GetTokenLifetime()); + ExpirationDate = DateTime.UtcNow.Add(_tokenLifetime); } public SsoEmail2faSessionTokenable(User user) : this() @@ -34,14 +29,12 @@ public SsoEmail2faSessionTokenable(User user) : this() Id = user?.Id ?? default; Email = user?.Email; } - public bool TokenIsValid(User user) { if (Id == default || Email == default || user == null) { return false; } - return Id == user.Id && Email.Equals(user.Email, StringComparison.InvariantCultureIgnoreCase); } diff --git a/test/Core.Test/Auth/Models/Business/Tokenables/SsoEmail2faSessionTokenableTests.cs b/test/Core.Test/Auth/Models/Business/Tokenables/SsoEmail2faSessionTokenableTests.cs deleted file mode 100644 index 0274b666c590..000000000000 --- a/test/Core.Test/Auth/Models/Business/Tokenables/SsoEmail2faSessionTokenableTests.cs +++ /dev/null @@ -1,173 +0,0 @@ -using AutoFixture.Xunit2; -using Bit.Core.Auth.Models.Business.Tokenables; -using Bit.Core.Entities; -using Bit.Core.Tokens; -using Xunit; -namespace Bit.Core.Test.Auth.Models.Business.Tokenables; - -// Note: test names follow MethodName_StateUnderTest_ExpectedBehavior pattern. -public class SsoEmail2faSessionTokenableTests -{ - // Allow a small tolerance for possible execution delays or clock precision. - private readonly TimeSpan _timeTolerance = TimeSpan.FromMilliseconds(10); - - /// - /// Tests the default constructor behavior when passed a null user. - /// - [Fact] - public void Constructor_NullUser_PropertiesSetToDefault() - { - var token = new SsoEmail2faSessionTokenable(null); - - Assert.Equal(default, token.Id); - Assert.Equal(default, token.Email); - } - - /// - /// Tests that when a valid user is provided to the constructor, the resulting token properties match the user. - /// - [Theory, AutoData] - public void Constructor_ValidUser_PropertiesSetFromUser(User user) - { - var token = new SsoEmail2faSessionTokenable(user); - - Assert.Equal(user.Id, token.Id); - Assert.Equal(user.Email, token.Email); - } - - /// - /// Tests the default expiration behavior immediately after initialization. - /// - [Fact] - public void Constructor_AfterInitialization_ExpirationSetToExpectedDuration() - { - var token = new SsoEmail2faSessionTokenable(); - var expectedExpiration = DateTime.UtcNow + SsoEmail2faSessionTokenable.GetTokenLifetime(); - - Assert.True(expectedExpiration - token.ExpirationDate < _timeTolerance); - } - - /// - /// Tests that a custom expiration date is preserved after token initialization. - /// - [Fact] - public void Constructor_CustomExpirationDate_ExpirationMatchesProvidedValue() - { - var customExpiration = DateTime.UtcNow.AddHours(3); - var token = new SsoEmail2faSessionTokenable - { - ExpirationDate = customExpiration - }; - - Assert.True(Math.Abs((customExpiration - token.ExpirationDate).TotalMilliseconds) < _timeTolerance.TotalMilliseconds); - } - - /// - /// Tests the validity of a token initialized with a null user. - /// - [Fact] - public void Valid_NullUser_ReturnsFalse() - { - var token = new SsoEmail2faSessionTokenable(null); - - Assert.False(token.Valid); - } - - /// - /// Tests the validity of a token with a non-matching identifier. - /// - [Theory, AutoData] - public void Valid_WrongIdentifier_ReturnsFalse(User user) - { - var token = new SsoEmail2faSessionTokenable(user) - { - Identifier = "not correct" - }; - - Assert.False(token.Valid); - } - - /// - /// Tests the token validity when user ID is null. - /// - [Theory, AutoData] - public void TokenIsValid_NullUserId_ReturnsFalse(User user) - { - user.Id = default; // Guid.Empty - var token = new SsoEmail2faSessionTokenable(user); - - Assert.False(token.TokenIsValid(user)); - } - - /// - /// Tests the token validity when user's email is null. - /// - [Theory, AutoData] - public void TokenIsValid_NullEmail_ReturnsFalse(User user) - { - user.Email = null; - var token = new SsoEmail2faSessionTokenable(user); - - Assert.False(token.TokenIsValid(user)); - } - - /// - /// Tests the token validity when user ID and email match the token properties. - /// - [Theory, AutoData] - public void TokenIsValid_MatchingUserIdAndEmail_ReturnsTrue(User user) - { - var token = new SsoEmail2faSessionTokenable(user); - - Assert.True(token.TokenIsValid(user)); - } - - /// - /// Ensures that the token is invalid when the provided user's ID doesn't match the token's ID. - /// - [Theory, AutoData] - public void TokenIsValid_WrongUserId_ReturnsFalse(User user) - { - // Given a token initialized with a user's details - var token = new SsoEmail2faSessionTokenable(user); - - // modify the user's ID - user.Id = Guid.NewGuid(); - - // Then the token should be considered invalid - Assert.False(token.TokenIsValid(user)); - } - - /// - /// Ensures that the token is invalid when the provided user's email doesn't match the token's email. - /// - [Theory, AutoData] - public void TokenIsValid_WrongEmail_ReturnsFalse(User user) - { - // Given a token initialized with a user's details - var token = new SsoEmail2faSessionTokenable(user); - - // modify the user's email - user.Email = "nonMatchingEmail@example.com"; - - // Then the token should be considered invalid - Assert.False(token.TokenIsValid(user)); - } - - /// - /// Tests the deserialization of a token to ensure that the expiration date is preserved. - /// - [Theory, AutoData] - public void FromToken_SerializedToken_PreservesExpirationDate(User user) - { - var expectedDateTime = DateTime.UtcNow.AddHours(-5); - var token = new SsoEmail2faSessionTokenable(user) - { - ExpirationDate = expectedDateTime - }; - - var result = Tokenable.FromToken(token.ToToken()); - - Assert.Equal(expectedDateTime, result.ExpirationDate, precision: _timeTolerance); - } -} From b30e2da1d3e79642ba393bdf7278a4680dc2f1b8 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Fri, 15 Sep 2023 18:04:30 -0400 Subject: [PATCH 17/80] PM-3275 - ExpiringTokenable.cs - add clarifying comments to help distinguish between the Valid property and the TokenIsValid method. --- src/Core/Tokens/ExpiringTokenable.cs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/Core/Tokens/ExpiringTokenable.cs b/src/Core/Tokens/ExpiringTokenable.cs index 089405e53636..5e90a2406606 100644 --- a/src/Core/Tokens/ExpiringTokenable.cs +++ b/src/Core/Tokens/ExpiringTokenable.cs @@ -7,7 +7,16 @@ public abstract class ExpiringTokenable : Tokenable { [JsonConverter(typeof(EpochDateTimeJsonConverter))] public DateTime ExpirationDate { get; set; } + + /// + /// Checks if the token is still within its valid duration and if its data is valid. + /// For data validation, this property relies on the method. + /// public override bool Valid => ExpirationDate > DateTime.UtcNow && TokenIsValid(); + /// + /// Validates that the token data properties are correct. + /// For expiration checks, refer to the property. + /// protected abstract bool TokenIsValid(); } From 1d279ee080437c85928646835282cb918a74385a Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Fri, 15 Sep 2023 18:19:37 -0400 Subject: [PATCH 18/80] PM-3275 - Create OrgUserInviteTokenable.cs and add tests in OrgUserInviteTokenableTests.cs --- .../Tokenables/OrgUserInviteTokenable.cs | 51 ++++ .../Tokenables/OrgUserInviteTokenableTests.cs | 266 ++++++++++++++++++ 2 files changed, 317 insertions(+) create mode 100644 src/Core/Auth/Models/Business/Tokenables/OrgUserInviteTokenable.cs create mode 100644 test/Core.Test/Auth/Models/Business/Tokenables/OrgUserInviteTokenableTests.cs diff --git a/src/Core/Auth/Models/Business/Tokenables/OrgUserInviteTokenable.cs b/src/Core/Auth/Models/Business/Tokenables/OrgUserInviteTokenable.cs new file mode 100644 index 000000000000..0d6209400603 --- /dev/null +++ b/src/Core/Auth/Models/Business/Tokenables/OrgUserInviteTokenable.cs @@ -0,0 +1,51 @@ +using Bit.Core.Tokens; +using System.Text.Json.Serialization; +using Bit.Core.Entities; + +namespace Bit.Core.Auth.Models.Business.Tokenables; + +public class OrgUserInviteTokenable : ExpiringTokenable +{ + // TODO: Ideally this would be internal and only visible to the test project. + public static TimeSpan GetTokenLifetime() => TimeSpan.FromDays(5); + + public const string ClearTextPrefix = "BwOrgUserInviteToken_"; + + // Backwards compatibility Note: + // Must use existing DataProtectorPurpose to be able to decrypt tokens + // in emailed invites that have not yet been accepted. + public const string DataProtectorPurpose = "OrganizationServiceDataProtector"; + + public const string TokenIdentifier = "OrgUserInviteToken"; + + public string Identifier { get; set; } = TokenIdentifier; + public Guid OrgUserId { get; set; } + public string OrgUserEmail { get; set; } + + [JsonConstructor] + public OrgUserInviteTokenable() + { + ExpirationDate = DateTime.UtcNow.Add(GetTokenLifetime()); + } + + public OrgUserInviteTokenable(OrganizationUser orgUser) : this() + { + OrgUserId = orgUser?.Id ?? default; + OrgUserEmail = orgUser?.Email; + } + + public bool TokenIsValid(OrganizationUser orgUser) + { + if (OrgUserId == default || OrgUserEmail == default || orgUser == null) + { + return false; + } + + return OrgUserId == orgUser.Id && + OrgUserEmail.Equals(orgUser.Email, StringComparison.InvariantCultureIgnoreCase); + } + + // Validates deserialized + protected override bool TokenIsValid() => + Identifier == TokenIdentifier && OrgUserId != default && !string.IsNullOrWhiteSpace(OrgUserEmail); +} diff --git a/test/Core.Test/Auth/Models/Business/Tokenables/OrgUserInviteTokenableTests.cs b/test/Core.Test/Auth/Models/Business/Tokenables/OrgUserInviteTokenableTests.cs new file mode 100644 index 000000000000..0f1df1cd313e --- /dev/null +++ b/test/Core.Test/Auth/Models/Business/Tokenables/OrgUserInviteTokenableTests.cs @@ -0,0 +1,266 @@ +using AutoFixture.Xunit2; +using Bit.Core.Auth.Models.Business.Tokenables; +using Bit.Core.Entities; +using Bit.Core.Tokens; +using System; +using Xunit; + + +namespace Bit.Core.Test.Auth.Models.Business.Tokenables +{ + // Note: test names follow MethodName_StateUnderTest_ExpectedBehavior pattern. + public class OrgUserInviteTokenableTests + { + // Allow a small tolerance for possible execution delays or clock precision. + private readonly TimeSpan _timeTolerance = TimeSpan.FromMilliseconds(10); + + /// + /// Tests that the default constructor sets the expiration date to the expected duration. + /// + [Fact] + public void Constructor_DefaultInitialization_ExpirationSetToExpectedDuration() + { + var token = new OrgUserInviteTokenable(); + var expectedExpiration = DateTime.UtcNow + OrgUserInviteTokenable.GetTokenLifetime(); + + Assert.True(TimesAreCloseEnough(expectedExpiration, token.ExpirationDate, _timeTolerance)); + } + + /// + /// Tests that the constructor sets the properties correctly from a valid OrganizationUser object. + /// + [Theory, AutoData] + public void Constructor_ValidOrgUser_PropertiesSetFromOrgUser(OrganizationUser orgUser) + { + var token = new OrgUserInviteTokenable(orgUser); + + Assert.Equal(orgUser.Id, token.OrgUserId); + Assert.Equal(orgUser.Email, token.OrgUserEmail); + } + + /// + /// Tests that the constructor sets the properties to default values when given a null OrganizationUser object. + /// + [Fact] + public void Constructor_NullOrgUser_PropertiesSetToDefault() + { + var token = new OrgUserInviteTokenable(null); + + Assert.Equal(default, token.OrgUserId); + Assert.Equal(default, token.OrgUserEmail); + } + + /// + /// Tests that a custom expiration date is preserved after token initialization. + /// + [Fact] + public void Constructor_CustomExpirationDate_ExpirationMatchesProvidedValue() + { + var customExpiration = DateTime.UtcNow.AddHours(3); + var token = new OrgUserInviteTokenable + { + ExpirationDate = customExpiration + }; + + Assert.True(TimesAreCloseEnough(customExpiration, token.ExpirationDate, _timeTolerance)); + } + + /// + /// Tests the validity of a token initialized with a null org user. + /// + [Fact] + public void Valid_NullOrgUser_ReturnsFalse() + { + var token = new OrgUserInviteTokenable(null); + + Assert.False(token.Valid); + } + + /// + /// Tests the validity of a token with a non-matching identifier. + /// + [Fact] + public void Valid_WrongIdentifier_ReturnsFalse() + { + var token = new OrgUserInviteTokenable + { + Identifier = "IncorrectIdentifier" + }; + + Assert.False(token.Valid); + } + + /// + /// Tests the validity of the token when the OrgUserId is set to default. + /// + [Fact] + public void Valid_DefaultOrgUserId_ReturnsFalse() + { + var token = new OrgUserInviteTokenable + { + OrgUserId = default // Guid.Empty + }; + + Assert.False(token.Valid); + } + + /// + /// Tests the validity of the token when the OrgUserEmail is null or empty. + /// + [Theory] + [InlineData(null)] + [InlineData("")] + public void Valid_NullOrEmptyOrgUserEmail_ReturnsFalse(string email) + { + var token = new OrgUserInviteTokenable + { + OrgUserEmail = email + }; + + Assert.False(token.Valid); + } + + + /// + /// Tests the validity of the token when the token is expired. + /// + [Fact] + public void Valid_ExpiredToken_ReturnsFalse() + { + var expiredDate = DateTime.UtcNow.AddHours(-3); + var token = new OrgUserInviteTokenable + { + ExpirationDate = expiredDate + }; + + Assert.False(token.Valid); + } + + + /// + /// Tests the TokenIsValid method when given a null OrganizationUser object. + /// + [Fact] + public void TokenIsValid_NullOrgUser_ReturnsFalse() + { + var token = new OrgUserInviteTokenable(null); + + Assert.False(token.TokenIsValid(null)); + } + + /// + /// Tests the TokenIsValid method when the OrgUserId does not match. + /// + [Theory, AutoData] + public void TokenIsValid_WrongUserId_ReturnsFalse(OrganizationUser orgUser) + { + var token = new OrgUserInviteTokenable(orgUser) + { + OrgUserId = Guid.NewGuid() // Force a different ID + }; + + Assert.False(token.TokenIsValid(orgUser)); + } + + /// + /// Tests the TokenIsValid method when the OrgUserEmail does not match. + /// + [Theory, AutoData] + public void TokenIsValid_WrongEmail_ReturnsFalse(OrganizationUser orgUser) + { + var token = new OrgUserInviteTokenable(orgUser) + { + OrgUserEmail = "wrongemail@example.com" // Force a different email + }; + + Assert.False(token.TokenIsValid(orgUser)); + } + + /// + /// Tests the TokenIsValid method when both OrgUserId and OrgUserEmail match. + /// + [Theory, AutoData] + public void TokenIsValid_MatchingUserIdAndEmail_ReturnsTrue(OrganizationUser orgUser) + { + var token = new OrgUserInviteTokenable(orgUser); + + Assert.True(token.TokenIsValid(orgUser)); + } + + /// + /// Tests the TokenIsValid method to ensure email comparison is case-insensitive. + /// + [Theory, AutoData] + public void TokenIsValid_EmailCaseInsensitiveComparison_ReturnsTrue(OrganizationUser orgUser) + { + var token = new OrgUserInviteTokenable(orgUser); + + // Modify the orgUser's email case + orgUser.Email = orgUser.Email.ToUpperInvariant(); + + Assert.True(token.TokenIsValid(orgUser)); + } + + + /// + /// Tests the TokenIsValid method when the token is expired. + /// Should return true as TokenIsValid only validates token data -- not token expiration. + /// + [Theory, AutoData] + public void TokenIsValid_ExpiredToken_ReturnsTrue(OrganizationUser orgUser) + { + var expiredDate = DateTime.UtcNow.AddHours(-3); + var token = new OrgUserInviteTokenable(orgUser) + { + ExpirationDate = expiredDate + }; + + Assert.True(token.TokenIsValid(orgUser)); + } + + /// + /// Tests the deserialization of a token to ensure that the ExpirationDate is preserved. + /// + [Theory, AutoData] + public void FromToken_SerializedToken_PreservesExpirationDate(OrganizationUser orgUser) + { + // Arbitrary time for testing + var expectedDateTime = DateTime.UtcNow.AddHours(-3); + var token = new OrgUserInviteTokenable(orgUser) + { + ExpirationDate = expectedDateTime + }; + + var result = Tokenable.FromToken(token.ToToken()); + + Assert.True(TimesAreCloseEnough(expectedDateTime, result.ExpirationDate, _timeTolerance)); + } + + /// + /// Tests the deserialization of a token to ensure that the OrgUserId property is preserved. + /// + [Theory, AutoData] + public void FromToken_SerializedToken_PreservesOrgUserId(OrganizationUser orgUser) + { + var token = new OrgUserInviteTokenable(orgUser); + var result = Tokenable.FromToken(token.ToToken()); + Assert.Equal(orgUser.Id, result.OrgUserId); + } + + /// + /// Tests the deserialization of a token to ensure that the OrgUserEmail property is preserved. + /// + [Theory, AutoData] + public void FromToken_SerializedToken_PreservesOrgUserEmail(OrganizationUser orgUser) + { + var token = new OrgUserInviteTokenable(orgUser); + var result = Tokenable.FromToken(token.ToToken()); + Assert.Equal(orgUser.Email, result.OrgUserEmail); + } + + private bool TimesAreCloseEnough(DateTime time1, DateTime time2, TimeSpan tolerance) + { + return (time1 - time2).Duration() < tolerance; + } + } +} From daffc93e7b81f41c32860c21741bc580ee7cfe42 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Mon, 18 Sep 2023 14:23:33 -0400 Subject: [PATCH 19/80] PM-3275 - OrganizationService.cs - Refactor Org User Invite methods to use new OrgUserInviteTokenable instead of manual creation of a token --- .../Implementations/OrganizationService.cs | 42 ++++++++++++------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/src/Core/Services/Implementations/OrganizationService.cs b/src/Core/Services/Implementations/OrganizationService.cs index 3dd6069bf31d..e424a3a9a6f1 100644 --- a/src/Core/Services/Implementations/OrganizationService.cs +++ b/src/Core/Services/Implementations/OrganizationService.cs @@ -2,6 +2,7 @@ using System.Text.Json; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Models.Business; +using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Auth.Repositories; using Bit.Core.Context; using Bit.Core.Entities; @@ -15,11 +16,11 @@ using Bit.Core.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.Repositories; using Bit.Core.Settings; +using Bit.Core.Tokens; using Bit.Core.Tools.Enums; using Bit.Core.Tools.Models.Business; using Bit.Core.Tools.Services; using Bit.Core.Utilities; -using Microsoft.AspNetCore.DataProtection; using Microsoft.Extensions.Logging; using Stripe; @@ -32,7 +33,6 @@ public class OrganizationService : IOrganizationService private readonly ICollectionRepository _collectionRepository; private readonly IUserRepository _userRepository; private readonly IGroupRepository _groupRepository; - private readonly IDataProtector _dataProtector; private readonly IMailService _mailService; private readonly IPushNotificationService _pushNotificationService; private readonly IPushRegistrationService _pushRegistrationService; @@ -54,6 +54,7 @@ public class OrganizationService : IOrganizationService private readonly IProviderUserRepository _providerUserRepository; private readonly ICountNewSmSeatsRequiredQuery _countNewSmSeatsRequiredQuery; private readonly IUpdateSecretsManagerSubscriptionCommand _updateSecretsManagerSubscriptionCommand; + private readonly IDataProtectorTokenFactory _orgUserInviteTokenDataFactory; public OrganizationService( IOrganizationRepository organizationRepository, @@ -61,7 +62,6 @@ public OrganizationService( ICollectionRepository collectionRepository, IUserRepository userRepository, IGroupRepository groupRepository, - IDataProtectionProvider dataProtectionProvider, IMailService mailService, IPushNotificationService pushNotificationService, IPushRegistrationService pushRegistrationService, @@ -82,14 +82,14 @@ public OrganizationService( IProviderOrganizationRepository providerOrganizationRepository, IProviderUserRepository providerUserRepository, ICountNewSmSeatsRequiredQuery countNewSmSeatsRequiredQuery, - IUpdateSecretsManagerSubscriptionCommand updateSecretsManagerSubscriptionCommand) + IUpdateSecretsManagerSubscriptionCommand updateSecretsManagerSubscriptionCommand, + IDataProtectorTokenFactory orgUserInviteTokenDataFactory) { _organizationRepository = organizationRepository; _organizationUserRepository = organizationUserRepository; _collectionRepository = collectionRepository; _userRepository = userRepository; _groupRepository = groupRepository; - _dataProtector = dataProtectionProvider.CreateProtector("OrganizationServiceDataProtector"); _mailService = mailService; _pushNotificationService = pushNotificationService; _pushRegistrationService = pushRegistrationService; @@ -111,6 +111,7 @@ public OrganizationService( _providerUserRepository = providerUserRepository; _countNewSmSeatsRequiredQuery = countNewSmSeatsRequiredQuery; _updateSecretsManagerSubscriptionCommand = updateSecretsManagerSubscriptionCommand; + _orgUserInviteTokenDataFactory = orgUserInviteTokenDataFactory; } public async Task ReplacePaymentMethodAsync(Guid organizationId, string paymentToken, @@ -1046,20 +1047,33 @@ public async Task ResendInviteAsync(Guid organizationId, Guid? invitingUserId, G private async Task SendInvitesAsync(IEnumerable orgUsers, Organization organization) { - string MakeToken(OrganizationUser orgUser) => - _dataProtector.Protect($"OrganizationUserInvite {orgUser.Id} {orgUser.Email} {CoreHelpers.ToEpocMilliseconds(DateTime.UtcNow)}"); + (OrganizationUser, ExpiringToken) MakeOrgUserExpiringTokenPair(OrganizationUser orgUser) + { + var orgUserInviteTokenable = new OrgUserInviteTokenable(orgUser); + var protectedToken = _orgUserInviteTokenDataFactory.Protect(orgUserInviteTokenable); + return (orgUser, new ExpiringToken(protectedToken, orgUserInviteTokenable.ExpirationDate)); + } + + var orgUsersWithExpTokens = orgUsers.Select(MakeOrgUserExpiringTokenPair); - await _mailService.BulkSendOrganizationInviteEmailAsync(organization.Name, - orgUsers.Select(o => (o, new ExpiringToken(MakeToken(o), DateTime.UtcNow.AddDays(5)))), organization.PlanType == PlanType.Free); + await _mailService.BulkSendOrganizationInviteEmailAsync( + organization.Name, + orgUsersWithExpTokens, + organization.PlanType == PlanType.Free + ); } private async Task SendInviteAsync(OrganizationUser orgUser, Organization organization, bool initOrganization) { - var now = DateTime.UtcNow; - var nowMillis = CoreHelpers.ToEpocMilliseconds(now); - var token = _dataProtector.Protect( - $"OrganizationUserInvite {orgUser.Id} {orgUser.Email} {nowMillis}"); - await _mailService.SendOrganizationInviteEmailAsync(organization.Name, orgUser, new ExpiringToken(token, now.AddDays(5)), organization.PlanType == PlanType.Free, initOrganization); + var orgUserInviteTokenable = new OrgUserInviteTokenable(orgUser); + var protectedToken = _orgUserInviteTokenDataFactory.Protect(orgUserInviteTokenable); + await _mailService.SendOrganizationInviteEmailAsync( + organization.Name, + orgUser, + new ExpiringToken(protectedToken, orgUserInviteTokenable.ExpirationDate), + organization.PlanType == PlanType.Free, + initOrganization + ); } public async Task ConfirmUserAsync(Guid organizationId, Guid organizationUserId, string key, From 1bde67202ca771ca1a21268343e2fb6f1d11bdfb Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Mon, 18 Sep 2023 14:23:53 -0400 Subject: [PATCH 20/80] PM-3275 - OrgUserInviteTokenable.cs - clarify backwards compat note --- .../Auth/Models/Business/Tokenables/OrgUserInviteTokenable.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Core/Auth/Models/Business/Tokenables/OrgUserInviteTokenable.cs b/src/Core/Auth/Models/Business/Tokenables/OrgUserInviteTokenable.cs index 0d6209400603..623a21595989 100644 --- a/src/Core/Auth/Models/Business/Tokenables/OrgUserInviteTokenable.cs +++ b/src/Core/Auth/Models/Business/Tokenables/OrgUserInviteTokenable.cs @@ -12,7 +12,9 @@ public class OrgUserInviteTokenable : ExpiringTokenable public const string ClearTextPrefix = "BwOrgUserInviteToken_"; // Backwards compatibility Note: - // Must use existing DataProtectorPurpose to be able to decrypt tokens + // Previously, tokens were manually created in the OrganizationService using a data protector + // initialized with purpose: "OrganizationServiceDataProtector" + // So, we must continue to use the existing purpose to be able to decrypt tokens // in emailed invites that have not yet been accepted. public const string DataProtectorPurpose = "OrganizationServiceDataProtector"; From 6fd9a0ccd251ab057fee13411c2796355b45cb54 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Mon, 18 Sep 2023 14:29:25 -0400 Subject: [PATCH 21/80] PM-3275 - AcceptOrgUserCommand.cs - Add TODOs + minor name refactor --- .../OrganizationUsers/AcceptOrgUserCommand.cs | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs b/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs index 0fdc822b5642..1355987ba1dc 100644 --- a/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs +++ b/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs @@ -30,7 +30,8 @@ public AcceptOrgUserCommand( IUserRepository userRepository) { - _dataProtector = dataProtectionProvider.CreateProtector("AcceptOrgUserCommandDataProtector"); + // TODO: Not going to be able to remove this for 1 releases for backwards compatibility reasons + _dataProtector = dataProtectionProvider.CreateProtector("OrganizationServiceDataProtector"); _globalSettings = globalSettings; _organizationUserRepository = organizationUserRepository; _organizationRepository = organizationRepository; @@ -40,9 +41,10 @@ public AcceptOrgUserCommand( } // TODO: consider removing overloading and renaming based on purpose for increased clarity / readability - // AcceptOrgUserWithEmailedTokenAsync or AcceptOrgUserByOrgUserIdAndTokenAsync ? - // AcceptOrgUserByOrgIdentifierAsync ? - // AcceptOrgUserByOrgIdAsync ? + // Yes + // AcceptOrgUserByTokenAsync + // AcceptOrgUserByOrgSsoIdentifierAsync + // AcceptOrgUserByOrgIdAsync public async Task AcceptOrgUserAsync(Guid organizationUserId, User user, string token, IUserService userService) { @@ -52,6 +54,11 @@ public async Task AcceptOrgUserAsync(Guid organizationUserId, throw new BadRequestException("User invalid."); } + // TODO: For backwards compatibility, going to have to try and convert token into OrgUserInviteTokenable first + // and then fallback to CoreHelpers.UserInviteTokenIsValid otherwise we will break all existing invites + // probably will need try catch b/c old tokens aren't valid JSON + // As tokens only are valid for 5 days, only need this code for 1 release. + if (!CoreHelpers.UserInviteTokenIsValid(_dataProtector, token, user.Email, orgUser.Id, _globalSettings)) { throw new BadRequestException("Invalid token."); @@ -86,9 +93,9 @@ public async Task AcceptOrgUserAsync(Guid organizationUserId, return organizationUser; } - public async Task AcceptOrgUserAsync(string orgIdentifier, User user, IUserService userService) + public async Task AcceptOrgUserAsync(string orgSsoIdentifier, User user, IUserService userService) { - var org = await _organizationRepository.GetByIdentifierAsync(orgIdentifier); + var org = await _organizationRepository.GetByIdentifierAsync(orgSsoIdentifier); if (org == null) { throw new BadRequestException("Organization invalid."); From 9654b8856713983ba2c1aff7b77e0810072d51fc Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Mon, 18 Sep 2023 14:35:09 -0400 Subject: [PATCH 22/80] PM-3275 - AcceptOrgUserCommand.cs - replace method overloading with more easily readable names. --- src/Api/Controllers/OrganizationUsersController.cs | 6 +++--- .../OrganizationUsers/AcceptOrgUserCommand.cs | 11 +++-------- .../Interfaces/IAcceptOrgUserCommand.cs | 6 +++--- src/Core/Services/Implementations/UserService.cs | 2 +- .../Controllers/OrganizationUsersControllerTests.cs | 8 ++++---- 5 files changed, 14 insertions(+), 19 deletions(-) diff --git a/src/Api/Controllers/OrganizationUsersController.cs b/src/Api/Controllers/OrganizationUsersController.cs index ce001c851542..1398c0e79d61 100644 --- a/src/Api/Controllers/OrganizationUsersController.cs +++ b/src/Api/Controllers/OrganizationUsersController.cs @@ -200,7 +200,7 @@ public async Task AcceptInit(Guid orgId, Guid organizationUserId, [FromBody] Org } await _organizationService.InitPendingOrganization(user.Id, orgId, model.Keys.PublicKey, model.Keys.EncryptedPrivateKey, model.CollectionName); - await _acceptOrgUserCommand.AcceptOrgUserAsync(organizationUserId, user, model.Token, _userService); + await _acceptOrgUserCommand.AcceptOrgUserByTokenAsync(organizationUserId, user, model.Token, _userService); await _organizationService.ConfirmUserAsync(orgId, organizationUserId, model.Key, user.Id, _userService); } @@ -222,7 +222,7 @@ public async Task Accept(Guid orgId, Guid organizationUserId, [FromBody] Organiz throw new BadRequestException(string.Empty, "Master Password reset is required, but not provided."); } - await _acceptOrgUserCommand.AcceptOrgUserAsync(organizationUserId, user, model.Token, _userService); + await _acceptOrgUserCommand.AcceptOrgUserByTokenAsync(organizationUserId, user, model.Token, _userService); if (useMasterPasswordPolicy) { @@ -333,7 +333,7 @@ await _organizationService.UpdateUserResetPasswordEnrollmentAsync( var orgUser = await _organizationUserRepository.GetByOrganizationAsync(orgId, user.Id); if (orgUser.Status == OrganizationUserStatusType.Invited) { - await _acceptOrgUserCommand.AcceptOrgUserAsync(orgId, user, _userService); + await _acceptOrgUserCommand.AcceptOrgUserByOrgIdAsync(orgId, user, _userService); } } diff --git a/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs b/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs index 1355987ba1dc..513ea1719146 100644 --- a/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs +++ b/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs @@ -40,12 +40,7 @@ public AcceptOrgUserCommand( _userRepository = userRepository; } - // TODO: consider removing overloading and renaming based on purpose for increased clarity / readability - // Yes - // AcceptOrgUserByTokenAsync - // AcceptOrgUserByOrgSsoIdentifierAsync - // AcceptOrgUserByOrgIdAsync - public async Task AcceptOrgUserAsync(Guid organizationUserId, User user, string token, + public async Task AcceptOrgUserByTokenAsync(Guid organizationUserId, User user, string token, IUserService userService) { var orgUser = await _organizationUserRepository.GetByIdAsync(organizationUserId); @@ -93,7 +88,7 @@ public async Task AcceptOrgUserAsync(Guid organizationUserId, return organizationUser; } - public async Task AcceptOrgUserAsync(string orgSsoIdentifier, User user, IUserService userService) + public async Task AcceptOrgUserByOrgSsoIdAsync(string orgSsoIdentifier, User user, IUserService userService) { var org = await _organizationRepository.GetByIdentifierAsync(orgSsoIdentifier); if (org == null) @@ -110,7 +105,7 @@ public async Task AcceptOrgUserAsync(string orgSsoIdentifier, return await AcceptOrgUserAsync(orgUser, user, userService); } - public async Task AcceptOrgUserAsync(Guid organizationId, User user, IUserService userService) + public async Task AcceptOrgUserByOrgIdAsync(Guid organizationId, User user, IUserService userService) { var org = await _organizationRepository.GetByIdAsync(organizationId); if (org == null) diff --git a/src/Core/OrganizationFeatures/OrganizationUsers/Interfaces/IAcceptOrgUserCommand.cs b/src/Core/OrganizationFeatures/OrganizationUsers/Interfaces/IAcceptOrgUserCommand.cs index 6650f892c510..551901aa1085 100644 --- a/src/Core/OrganizationFeatures/OrganizationUsers/Interfaces/IAcceptOrgUserCommand.cs +++ b/src/Core/OrganizationFeatures/OrganizationUsers/Interfaces/IAcceptOrgUserCommand.cs @@ -11,8 +11,8 @@ public interface IAcceptOrgUserCommand /// /// The token embedded in the email invitation link /// The accepted OrganizationUser. - Task AcceptOrgUserAsync(Guid organizationUserId, User user, string token, IUserService userService); - Task AcceptOrgUserAsync(string orgIdentifier, User user, IUserService userService); - Task AcceptOrgUserAsync(Guid organizationId, User user, IUserService userService); + Task AcceptOrgUserByTokenAsync(Guid organizationUserId, User user, string token, IUserService userService); + Task AcceptOrgUserByOrgSsoIdAsync(string orgIdentifier, User user, IUserService userService); + Task AcceptOrgUserByOrgIdAsync(Guid organizationId, User user, IUserService userService); Task AcceptOrgUserAsync(OrganizationUser orgUser, User user, IUserService userService); } diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index b15058e4ec47..fe33041c2242 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -649,7 +649,7 @@ public async Task SetKeyConnectorKeyAsync(User user, string key, await _userRepository.ReplaceAsync(user); await _eventService.LogUserEventAsync(user.Id, EventType.User_MigratedKeyToKeyConnector); - await _acceptOrgUserCommand.AcceptOrgUserAsync(orgIdentifier, user, this); + await _acceptOrgUserCommand.AcceptOrgUserByOrgSsoIdAsync(orgIdentifier, user, this); return IdentityResult.Success; } diff --git a/test/Api.Test/Controllers/OrganizationUsersControllerTests.cs b/test/Api.Test/Controllers/OrganizationUsersControllerTests.cs index 07b10db6956e..f394d91eac12 100644 --- a/test/Api.Test/Controllers/OrganizationUsersControllerTests.cs +++ b/test/Api.Test/Controllers/OrganizationUsersControllerTests.cs @@ -28,7 +28,7 @@ public async Task PutResetPasswordEnrollment_InivitedUser_AcceptsInvite(Guid org await sutProvider.Sut.PutResetPasswordEnrollment(orgId, userId, model); - await sutProvider.GetDependency().Received(1).AcceptOrgUserAsync(orgId, user, sutProvider.GetDependency()); + await sutProvider.GetDependency().Received(1).AcceptOrgUserByOrgIdAsync(orgId, user, sutProvider.GetDependency()); } [Theory] @@ -42,7 +42,7 @@ public async Task PutResetPasswordEnrollment_ConfirmedUser_AcceptsInvite(Guid or await sutProvider.Sut.PutResetPasswordEnrollment(orgId, userId, model); - await sutProvider.GetDependency().Received(0).AcceptOrgUserAsync(orgId, user, sutProvider.GetDependency()); + await sutProvider.GetDependency().Received(0).AcceptOrgUserByOrgIdAsync(orgId, user, sutProvider.GetDependency()); } [Theory] @@ -65,7 +65,7 @@ public async Task Accept_NoMasterPasswordReset(Guid orgId, Guid orgUserId, await sutProvider.Sut.Accept(orgId, orgUserId, model); await sutProvider.GetDependency().Received(1) - .AcceptOrgUserAsync(orgUserId, user, model.Token, sutProvider.GetDependency()); + .AcceptOrgUserByTokenAsync(orgUserId, user, model.Token, sutProvider.GetDependency()); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() .UpdateUserResetPasswordEnrollmentAsync(default, default, default, default); } @@ -87,7 +87,7 @@ public async Task Accept_RequireMasterPasswordReset(Guid orgId, Guid orgUserId, await sutProvider.Sut.Accept(orgId, orgUserId, model); await sutProvider.GetDependency().Received(1) - .AcceptOrgUserAsync(orgUserId, user, model.Token, sutProvider.GetDependency()); + .AcceptOrgUserByTokenAsync(orgUserId, user, model.Token, sutProvider.GetDependency()); await sutProvider.GetDependency().Received(1) .UpdateUserResetPasswordEnrollmentAsync(orgId, user.Id, model.ResetPasswordKey, user.Id); } From dda6c90eb0b1cd4cb82708a1b09f56b9fb7df037 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Mon, 18 Sep 2023 15:12:15 -0400 Subject: [PATCH 23/80] PM-3275 - AcceptOrgUserCommand.cs - Update ValidateOrgUserInviteToken to add new token validation while maintaining backwards compatibility for 1 release. --- .../OrganizationUsers/AcceptOrgUserCommand.cs | 34 ++++++++++++++----- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs b/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs index 513ea1719146..f8577bd7b675 100644 --- a/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs +++ b/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs @@ -1,10 +1,12 @@ -using Bit.Core.Entities; +using Bit.Core.Auth.Models.Business.Tokenables; +using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.Repositories; using Bit.Core.Services; using Bit.Core.Settings; +using Bit.Core.Tokens; using Bit.Core.Utilities; using Microsoft.AspNetCore.DataProtection; @@ -19,6 +21,7 @@ public class AcceptOrgUserCommand : IAcceptOrgUserCommand private readonly IPolicyService _policyService; private readonly IMailService _mailService; private readonly IUserRepository _userRepository; + private readonly IDataProtectorTokenFactory _orgUserInviteTokenDataFactory; public AcceptOrgUserCommand( IDataProtectionProvider dataProtectionProvider, @@ -27,10 +30,11 @@ public AcceptOrgUserCommand( IOrganizationRepository organizationRepository, IPolicyService policyService, IMailService mailService, - IUserRepository userRepository) + IUserRepository userRepository, + IDataProtectorTokenFactory orgUserInviteTokenDataFactory) { - // TODO: Not going to be able to remove this for 1 releases for backwards compatibility reasons + // TODO: remove data protector when old token validation removed _dataProtector = dataProtectionProvider.CreateProtector("OrganizationServiceDataProtector"); _globalSettings = globalSettings; _organizationUserRepository = organizationUserRepository; @@ -38,6 +42,14 @@ public AcceptOrgUserCommand( _policyService = policyService; _mailService = mailService; _userRepository = userRepository; + _orgUserInviteTokenDataFactory = orgUserInviteTokenDataFactory; + } + + private bool ValidateOrgUserInviteToken(string orgUserInviteToken, OrganizationUser orgUser) + { + return _orgUserInviteTokenDataFactory.TryUnprotect(orgUserInviteToken, out var decryptedToken) + && decryptedToken.Valid + && decryptedToken.TokenIsValid(orgUser); } public async Task AcceptOrgUserByTokenAsync(Guid organizationUserId, User user, string token, @@ -49,12 +61,17 @@ public async Task AcceptOrgUserByTokenAsync(Guid organizationU throw new BadRequestException("User invalid."); } - // TODO: For backwards compatibility, going to have to try and convert token into OrgUserInviteTokenable first - // and then fallback to CoreHelpers.UserInviteTokenIsValid otherwise we will break all existing invites - // probably will need try catch b/c old tokens aren't valid JSON - // As tokens only are valid for 5 days, only need this code for 1 release. + // Tokens will have been created in two ways in the OrganizationService invite methods: + // 1. New way - via OrgUserInviteTokenable + // 2. Old way - via manual process using data protector initialized with purpose: "OrganizationServiceDataProtector" - if (!CoreHelpers.UserInviteTokenIsValid(_dataProtector, token, user.Email, orgUser.Id, _globalSettings)) + // For backwards compatibility, must check validity of both types of tokens and accept if either is valid + // TODO: remove old token validation logic after 1 release as token are only valid for 5 days + + var isNewTokenInvalid = !ValidateOrgUserInviteToken(token, orgUser); + var isOldTokenInvalid = !CoreHelpers.UserInviteTokenIsValid(_dataProtector, token, user.Email, orgUser.Id, _globalSettings); + + if (isNewTokenInvalid && isOldTokenInvalid) { throw new BadRequestException("Invalid token."); } @@ -199,4 +216,5 @@ public async Task AcceptOrgUserAsync(OrganizationUser orgUser, return orgUser; } + } From b27c1879d3135a96b14be2f148272e4533860aaf Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Mon, 18 Sep 2023 15:28:40 -0400 Subject: [PATCH 24/80] dotnet format --- .../Tokenables/OrgUserInviteTokenable.cs | 4 +- .../Tokenables/OrgUserInviteTokenableTests.cs | 444 +++++++++--------- .../SetInitialMasterPasswordCommandTests.cs | 6 +- 3 files changed, 226 insertions(+), 228 deletions(-) diff --git a/src/Core/Auth/Models/Business/Tokenables/OrgUserInviteTokenable.cs b/src/Core/Auth/Models/Business/Tokenables/OrgUserInviteTokenable.cs index 623a21595989..469e9c31facc 100644 --- a/src/Core/Auth/Models/Business/Tokenables/OrgUserInviteTokenable.cs +++ b/src/Core/Auth/Models/Business/Tokenables/OrgUserInviteTokenable.cs @@ -1,6 +1,6 @@ -using Bit.Core.Tokens; -using System.Text.Json.Serialization; +using System.Text.Json.Serialization; using Bit.Core.Entities; +using Bit.Core.Tokens; namespace Bit.Core.Auth.Models.Business.Tokenables; diff --git a/test/Core.Test/Auth/Models/Business/Tokenables/OrgUserInviteTokenableTests.cs b/test/Core.Test/Auth/Models/Business/Tokenables/OrgUserInviteTokenableTests.cs index 0f1df1cd313e..aeeda206fa95 100644 --- a/test/Core.Test/Auth/Models/Business/Tokenables/OrgUserInviteTokenableTests.cs +++ b/test/Core.Test/Auth/Models/Business/Tokenables/OrgUserInviteTokenableTests.cs @@ -2,265 +2,263 @@ using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Entities; using Bit.Core.Tokens; -using System; using Xunit; -namespace Bit.Core.Test.Auth.Models.Business.Tokenables +namespace Bit.Core.Test.Auth.Models.Business.Tokenables; + +// Note: test names follow MethodName_StateUnderTest_ExpectedBehavior pattern. +public class OrgUserInviteTokenableTests { - // Note: test names follow MethodName_StateUnderTest_ExpectedBehavior pattern. - public class OrgUserInviteTokenableTests + // Allow a small tolerance for possible execution delays or clock precision. + private readonly TimeSpan _timeTolerance = TimeSpan.FromMilliseconds(10); + + /// + /// Tests that the default constructor sets the expiration date to the expected duration. + /// + [Fact] + public void Constructor_DefaultInitialization_ExpirationSetToExpectedDuration() { - // Allow a small tolerance for possible execution delays or clock precision. - private readonly TimeSpan _timeTolerance = TimeSpan.FromMilliseconds(10); - - /// - /// Tests that the default constructor sets the expiration date to the expected duration. - /// - [Fact] - public void Constructor_DefaultInitialization_ExpirationSetToExpectedDuration() - { - var token = new OrgUserInviteTokenable(); - var expectedExpiration = DateTime.UtcNow + OrgUserInviteTokenable.GetTokenLifetime(); + var token = new OrgUserInviteTokenable(); + var expectedExpiration = DateTime.UtcNow + OrgUserInviteTokenable.GetTokenLifetime(); - Assert.True(TimesAreCloseEnough(expectedExpiration, token.ExpirationDate, _timeTolerance)); - } + Assert.True(TimesAreCloseEnough(expectedExpiration, token.ExpirationDate, _timeTolerance)); + } - /// - /// Tests that the constructor sets the properties correctly from a valid OrganizationUser object. - /// - [Theory, AutoData] - public void Constructor_ValidOrgUser_PropertiesSetFromOrgUser(OrganizationUser orgUser) - { - var token = new OrgUserInviteTokenable(orgUser); + /// + /// Tests that the constructor sets the properties correctly from a valid OrganizationUser object. + /// + [Theory, AutoData] + public void Constructor_ValidOrgUser_PropertiesSetFromOrgUser(OrganizationUser orgUser) + { + var token = new OrgUserInviteTokenable(orgUser); - Assert.Equal(orgUser.Id, token.OrgUserId); - Assert.Equal(orgUser.Email, token.OrgUserEmail); - } + Assert.Equal(orgUser.Id, token.OrgUserId); + Assert.Equal(orgUser.Email, token.OrgUserEmail); + } - /// - /// Tests that the constructor sets the properties to default values when given a null OrganizationUser object. - /// - [Fact] - public void Constructor_NullOrgUser_PropertiesSetToDefault() - { - var token = new OrgUserInviteTokenable(null); + /// + /// Tests that the constructor sets the properties to default values when given a null OrganizationUser object. + /// + [Fact] + public void Constructor_NullOrgUser_PropertiesSetToDefault() + { + var token = new OrgUserInviteTokenable(null); - Assert.Equal(default, token.OrgUserId); - Assert.Equal(default, token.OrgUserEmail); - } + Assert.Equal(default, token.OrgUserId); + Assert.Equal(default, token.OrgUserEmail); + } - /// - /// Tests that a custom expiration date is preserved after token initialization. - /// - [Fact] - public void Constructor_CustomExpirationDate_ExpirationMatchesProvidedValue() - { - var customExpiration = DateTime.UtcNow.AddHours(3); - var token = new OrgUserInviteTokenable - { - ExpirationDate = customExpiration - }; - - Assert.True(TimesAreCloseEnough(customExpiration, token.ExpirationDate, _timeTolerance)); - } - - /// - /// Tests the validity of a token initialized with a null org user. - /// - [Fact] - public void Valid_NullOrgUser_ReturnsFalse() + /// + /// Tests that a custom expiration date is preserved after token initialization. + /// + [Fact] + public void Constructor_CustomExpirationDate_ExpirationMatchesProvidedValue() + { + var customExpiration = DateTime.UtcNow.AddHours(3); + var token = new OrgUserInviteTokenable { - var token = new OrgUserInviteTokenable(null); + ExpirationDate = customExpiration + }; + + Assert.True(TimesAreCloseEnough(customExpiration, token.ExpirationDate, _timeTolerance)); + } + + /// + /// Tests the validity of a token initialized with a null org user. + /// + [Fact] + public void Valid_NullOrgUser_ReturnsFalse() + { + var token = new OrgUserInviteTokenable(null); - Assert.False(token.Valid); - } + Assert.False(token.Valid); + } - /// - /// Tests the validity of a token with a non-matching identifier. - /// - [Fact] - public void Valid_WrongIdentifier_ReturnsFalse() + /// + /// Tests the validity of a token with a non-matching identifier. + /// + [Fact] + public void Valid_WrongIdentifier_ReturnsFalse() + { + var token = new OrgUserInviteTokenable { - var token = new OrgUserInviteTokenable - { - Identifier = "IncorrectIdentifier" - }; - - Assert.False(token.Valid); - } - - /// - /// Tests the validity of the token when the OrgUserId is set to default. - /// - [Fact] - public void Valid_DefaultOrgUserId_ReturnsFalse() + Identifier = "IncorrectIdentifier" + }; + + Assert.False(token.Valid); + } + + /// + /// Tests the validity of the token when the OrgUserId is set to default. + /// + [Fact] + public void Valid_DefaultOrgUserId_ReturnsFalse() + { + var token = new OrgUserInviteTokenable { - var token = new OrgUserInviteTokenable - { - OrgUserId = default // Guid.Empty - }; - - Assert.False(token.Valid); - } - - /// - /// Tests the validity of the token when the OrgUserEmail is null or empty. - /// - [Theory] - [InlineData(null)] - [InlineData("")] - public void Valid_NullOrEmptyOrgUserEmail_ReturnsFalse(string email) + OrgUserId = default // Guid.Empty + }; + + Assert.False(token.Valid); + } + + /// + /// Tests the validity of the token when the OrgUserEmail is null or empty. + /// + [Theory] + [InlineData(null)] + [InlineData("")] + public void Valid_NullOrEmptyOrgUserEmail_ReturnsFalse(string email) + { + var token = new OrgUserInviteTokenable { - var token = new OrgUserInviteTokenable - { - OrgUserEmail = email - }; + OrgUserEmail = email + }; - Assert.False(token.Valid); - } + Assert.False(token.Valid); + } - /// - /// Tests the validity of the token when the token is expired. - /// - [Fact] - public void Valid_ExpiredToken_ReturnsFalse() + /// + /// Tests the validity of the token when the token is expired. + /// + [Fact] + public void Valid_ExpiredToken_ReturnsFalse() + { + var expiredDate = DateTime.UtcNow.AddHours(-3); + var token = new OrgUserInviteTokenable { - var expiredDate = DateTime.UtcNow.AddHours(-3); - var token = new OrgUserInviteTokenable - { - ExpirationDate = expiredDate - }; + ExpirationDate = expiredDate + }; - Assert.False(token.Valid); - } + Assert.False(token.Valid); + } - /// - /// Tests the TokenIsValid method when given a null OrganizationUser object. - /// - [Fact] - public void TokenIsValid_NullOrgUser_ReturnsFalse() - { - var token = new OrgUserInviteTokenable(null); + /// + /// Tests the TokenIsValid method when given a null OrganizationUser object. + /// + [Fact] + public void TokenIsValid_NullOrgUser_ReturnsFalse() + { + var token = new OrgUserInviteTokenable(null); - Assert.False(token.TokenIsValid(null)); - } + Assert.False(token.TokenIsValid(null)); + } - /// - /// Tests the TokenIsValid method when the OrgUserId does not match. - /// - [Theory, AutoData] - public void TokenIsValid_WrongUserId_ReturnsFalse(OrganizationUser orgUser) - { - var token = new OrgUserInviteTokenable(orgUser) - { - OrgUserId = Guid.NewGuid() // Force a different ID - }; - - Assert.False(token.TokenIsValid(orgUser)); - } - - /// - /// Tests the TokenIsValid method when the OrgUserEmail does not match. - /// - [Theory, AutoData] - public void TokenIsValid_WrongEmail_ReturnsFalse(OrganizationUser orgUser) - { - var token = new OrgUserInviteTokenable(orgUser) - { - OrgUserEmail = "wrongemail@example.com" // Force a different email - }; - - Assert.False(token.TokenIsValid(orgUser)); - } - - /// - /// Tests the TokenIsValid method when both OrgUserId and OrgUserEmail match. - /// - [Theory, AutoData] - public void TokenIsValid_MatchingUserIdAndEmail_ReturnsTrue(OrganizationUser orgUser) + /// + /// Tests the TokenIsValid method when the OrgUserId does not match. + /// + [Theory, AutoData] + public void TokenIsValid_WrongUserId_ReturnsFalse(OrganizationUser orgUser) + { + var token = new OrgUserInviteTokenable(orgUser) { - var token = new OrgUserInviteTokenable(orgUser); + OrgUserId = Guid.NewGuid() // Force a different ID + }; - Assert.True(token.TokenIsValid(orgUser)); - } + Assert.False(token.TokenIsValid(orgUser)); + } - /// - /// Tests the TokenIsValid method to ensure email comparison is case-insensitive. - /// - [Theory, AutoData] - public void TokenIsValid_EmailCaseInsensitiveComparison_ReturnsTrue(OrganizationUser orgUser) + /// + /// Tests the TokenIsValid method when the OrgUserEmail does not match. + /// + [Theory, AutoData] + public void TokenIsValid_WrongEmail_ReturnsFalse(OrganizationUser orgUser) + { + var token = new OrgUserInviteTokenable(orgUser) { - var token = new OrgUserInviteTokenable(orgUser); + OrgUserEmail = "wrongemail@example.com" // Force a different email + }; - // Modify the orgUser's email case - orgUser.Email = orgUser.Email.ToUpperInvariant(); + Assert.False(token.TokenIsValid(orgUser)); + } - Assert.True(token.TokenIsValid(orgUser)); - } + /// + /// Tests the TokenIsValid method when both OrgUserId and OrgUserEmail match. + /// + [Theory, AutoData] + public void TokenIsValid_MatchingUserIdAndEmail_ReturnsTrue(OrganizationUser orgUser) + { + var token = new OrgUserInviteTokenable(orgUser); + Assert.True(token.TokenIsValid(orgUser)); + } - /// - /// Tests the TokenIsValid method when the token is expired. - /// Should return true as TokenIsValid only validates token data -- not token expiration. - /// - [Theory, AutoData] - public void TokenIsValid_ExpiredToken_ReturnsTrue(OrganizationUser orgUser) - { - var expiredDate = DateTime.UtcNow.AddHours(-3); - var token = new OrgUserInviteTokenable(orgUser) - { - ExpirationDate = expiredDate - }; - - Assert.True(token.TokenIsValid(orgUser)); - } - - /// - /// Tests the deserialization of a token to ensure that the ExpirationDate is preserved. - /// - [Theory, AutoData] - public void FromToken_SerializedToken_PreservesExpirationDate(OrganizationUser orgUser) - { - // Arbitrary time for testing - var expectedDateTime = DateTime.UtcNow.AddHours(-3); - var token = new OrgUserInviteTokenable(orgUser) - { - ExpirationDate = expectedDateTime - }; - - var result = Tokenable.FromToken(token.ToToken()); - - Assert.True(TimesAreCloseEnough(expectedDateTime, result.ExpirationDate, _timeTolerance)); - } - - /// - /// Tests the deserialization of a token to ensure that the OrgUserId property is preserved. - /// - [Theory, AutoData] - public void FromToken_SerializedToken_PreservesOrgUserId(OrganizationUser orgUser) - { - var token = new OrgUserInviteTokenable(orgUser); - var result = Tokenable.FromToken(token.ToToken()); - Assert.Equal(orgUser.Id, result.OrgUserId); - } - - /// - /// Tests the deserialization of a token to ensure that the OrgUserEmail property is preserved. - /// - [Theory, AutoData] - public void FromToken_SerializedToken_PreservesOrgUserEmail(OrganizationUser orgUser) + /// + /// Tests the TokenIsValid method to ensure email comparison is case-insensitive. + /// + [Theory, AutoData] + public void TokenIsValid_EmailCaseInsensitiveComparison_ReturnsTrue(OrganizationUser orgUser) + { + var token = new OrgUserInviteTokenable(orgUser); + + // Modify the orgUser's email case + orgUser.Email = orgUser.Email.ToUpperInvariant(); + + Assert.True(token.TokenIsValid(orgUser)); + } + + + /// + /// Tests the TokenIsValid method when the token is expired. + /// Should return true as TokenIsValid only validates token data -- not token expiration. + /// + [Theory, AutoData] + public void TokenIsValid_ExpiredToken_ReturnsTrue(OrganizationUser orgUser) + { + var expiredDate = DateTime.UtcNow.AddHours(-3); + var token = new OrgUserInviteTokenable(orgUser) { - var token = new OrgUserInviteTokenable(orgUser); - var result = Tokenable.FromToken(token.ToToken()); - Assert.Equal(orgUser.Email, result.OrgUserEmail); - } + ExpirationDate = expiredDate + }; + + Assert.True(token.TokenIsValid(orgUser)); + } - private bool TimesAreCloseEnough(DateTime time1, DateTime time2, TimeSpan tolerance) + /// + /// Tests the deserialization of a token to ensure that the ExpirationDate is preserved. + /// + [Theory, AutoData] + public void FromToken_SerializedToken_PreservesExpirationDate(OrganizationUser orgUser) + { + // Arbitrary time for testing + var expectedDateTime = DateTime.UtcNow.AddHours(-3); + var token = new OrgUserInviteTokenable(orgUser) { - return (time1 - time2).Duration() < tolerance; - } + ExpirationDate = expectedDateTime + }; + + var result = Tokenable.FromToken(token.ToToken()); + + Assert.True(TimesAreCloseEnough(expectedDateTime, result.ExpirationDate, _timeTolerance)); + } + + /// + /// Tests the deserialization of a token to ensure that the OrgUserId property is preserved. + /// + [Theory, AutoData] + public void FromToken_SerializedToken_PreservesOrgUserId(OrganizationUser orgUser) + { + var token = new OrgUserInviteTokenable(orgUser); + var result = Tokenable.FromToken(token.ToToken()); + Assert.Equal(orgUser.Id, result.OrgUserId); + } + + /// + /// Tests the deserialization of a token to ensure that the OrgUserEmail property is preserved. + /// + [Theory, AutoData] + public void FromToken_SerializedToken_PreservesOrgUserEmail(OrganizationUser orgUser) + { + var token = new OrgUserInviteTokenable(orgUser); + var result = Tokenable.FromToken(token.ToToken()); + Assert.Equal(orgUser.Email, result.OrgUserEmail); + } + + private bool TimesAreCloseEnough(DateTime time1, DateTime time2, TimeSpan tolerance) + { + return (time1 - time2).Duration() < tolerance; } } diff --git a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommandTests.cs b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommandTests.cs index 4a91c35f0952..d58537571588 100644 --- a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommandTests.cs +++ b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommandTests.cs @@ -2,13 +2,13 @@ using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; +using Bit.Core.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.Repositories; using Bit.Core.Services; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; using Microsoft.AspNetCore.Identity; using NSubstitute; -using Bit.Core.OrganizationFeatures.OrganizationUsers.Interfaces; using NSubstitute.ReturnsExtensions; using Xunit; @@ -86,8 +86,8 @@ public async Task SetInitialMasterPassword_InvalidOrganization_Throws(SutProvide .ReturnsNull(); // Act & Assert - var exception = await Assert.ThrowsAsync(async () => await sutProvider.Sut.SetInitialMasterPasswordAsync(user, masterPassword, key, orgIdentifier)); - Assert.Equal("Organization invalid.", exception.Message); + var exception = await Assert.ThrowsAsync(async () => await sutProvider.Sut.SetInitialMasterPasswordAsync(user, masterPassword, key, orgIdentifier)); + Assert.Equal("Organization invalid.", exception.Message); } [Theory] From 33ba8db898aa72c6e9781da0b95e50cebc520f0b Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Mon, 18 Sep 2023 16:19:18 -0400 Subject: [PATCH 25/80] PM-3275 - AcceptOrgUserCommand.cs - Move private method below where it is used --- .../OrganizationUsers/AcceptOrgUserCommand.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs b/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs index f8577bd7b675..a908cb85af2f 100644 --- a/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs +++ b/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs @@ -45,13 +45,6 @@ public AcceptOrgUserCommand( _orgUserInviteTokenDataFactory = orgUserInviteTokenDataFactory; } - private bool ValidateOrgUserInviteToken(string orgUserInviteToken, OrganizationUser orgUser) - { - return _orgUserInviteTokenDataFactory.TryUnprotect(orgUserInviteToken, out var decryptedToken) - && decryptedToken.Valid - && decryptedToken.TokenIsValid(orgUser); - } - public async Task AcceptOrgUserByTokenAsync(Guid organizationUserId, User user, string token, IUserService userService) { @@ -105,6 +98,13 @@ public async Task AcceptOrgUserByTokenAsync(Guid organizationU return organizationUser; } + private bool ValidateOrgUserInviteToken(string orgUserInviteToken, OrganizationUser orgUser) + { + return _orgUserInviteTokenDataFactory.TryUnprotect(orgUserInviteToken, out var decryptedToken) + && decryptedToken.Valid + && decryptedToken.TokenIsValid(orgUser); + } + public async Task AcceptOrgUserByOrgSsoIdAsync(string orgSsoIdentifier, User user, IUserService userService) { var org = await _organizationRepository.GetByIdentifierAsync(orgSsoIdentifier); From ae29432c3048dcba7b993c7ccb8e598957bdf2af Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Mon, 18 Sep 2023 19:32:28 -0400 Subject: [PATCH 26/80] PM-3275 - ServiceCollectionExtensions.cs - Must register IDataProtectorTokenFactory for new tokenable --- src/SharedWeb/Utilities/ServiceCollectionExtensions.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs index e42e86f2db58..598979459202 100644 --- a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs +++ b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs @@ -150,6 +150,7 @@ public static void AddTokenizers(this IServiceCollection services) serviceProvider.GetDataProtectionProvider(), serviceProvider.GetRequiredService>>()) ); + services.AddSingleton>(serviceProvider => new DataProtectorTokenFactory( HCaptchaTokenable.ClearTextPrefix, @@ -157,18 +158,27 @@ public static void AddTokenizers(this IServiceCollection services) serviceProvider.GetDataProtectionProvider(), serviceProvider.GetRequiredService>>()) ); + services.AddSingleton>(serviceProvider => new DataProtectorTokenFactory( SsoTokenable.ClearTextPrefix, SsoTokenable.DataProtectorPurpose, serviceProvider.GetDataProtectionProvider(), serviceProvider.GetRequiredService>>())); + services.AddSingleton>(serviceProvider => new DataProtectorTokenFactory( SsoEmail2faSessionTokenable.ClearTextPrefix, SsoEmail2faSessionTokenable.DataProtectorPurpose, serviceProvider.GetDataProtectionProvider(), serviceProvider.GetRequiredService>>())); + + services.AddSingleton>(serviceProvider => + new DataProtectorTokenFactory( + OrgUserInviteTokenable.ClearTextPrefix, + OrgUserInviteTokenable.DataProtectorPurpose, + serviceProvider.GetDataProtectionProvider(), + serviceProvider.GetRequiredService>>())); } public static void AddDefaultServices(this IServiceCollection services, GlobalSettings globalSettings) From 8b449777e9b0056cf5618e94ac74a51e8b3fb8e9 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Mon, 18 Sep 2023 19:38:58 -0400 Subject: [PATCH 27/80] PM-3275 - OrgUserInviteTokenable needed access to global settings to set its token lifetime to the _globalSettings.OrganizationInviteExpirationHours value. Creating a factory seemed the most straightforward way to encapsulate the desired creation logic. Unsure if in the correct location in ServiceCollectionExtensions.cs but will figure that out later. --- .../OrgUserInviteTokenableFactory.cs | 23 +++++++++++++++++++ .../Implementations/OrganizationService.cs | 7 ++++-- .../Utilities/ServiceCollectionExtensions.cs | 3 +++ 3 files changed, 31 insertions(+), 2 deletions(-) create mode 100644 src/Core/Auth/Models/Business/Tokenables/OrgUserInviteTokenableFactory.cs diff --git a/src/Core/Auth/Models/Business/Tokenables/OrgUserInviteTokenableFactory.cs b/src/Core/Auth/Models/Business/Tokenables/OrgUserInviteTokenableFactory.cs new file mode 100644 index 000000000000..5ec4502639da --- /dev/null +++ b/src/Core/Auth/Models/Business/Tokenables/OrgUserInviteTokenableFactory.cs @@ -0,0 +1,23 @@ +using Bit.Core.Entities; +using Bit.Core.Settings; + +namespace Bit.Core.Auth.Models.Business.Tokenables; + +public class OrgUserInviteTokenableFactory +{ + private readonly IGlobalSettings _globalSettings; + + public OrgUserInviteTokenableFactory(IGlobalSettings globalSettings) + { + _globalSettings = globalSettings; + } + + public OrgUserInviteTokenable CreateToken(OrganizationUser orgUser) + { + var token = new OrgUserInviteTokenable(orgUser) + { + ExpirationDate = DateTime.UtcNow.Add(TimeSpan.FromHours(_globalSettings.OrganizationInviteExpirationHours)) + }; + return token; + } +} diff --git a/src/Core/Services/Implementations/OrganizationService.cs b/src/Core/Services/Implementations/OrganizationService.cs index e424a3a9a6f1..63d7a9e60fb6 100644 --- a/src/Core/Services/Implementations/OrganizationService.cs +++ b/src/Core/Services/Implementations/OrganizationService.cs @@ -54,6 +54,7 @@ public class OrganizationService : IOrganizationService private readonly IProviderUserRepository _providerUserRepository; private readonly ICountNewSmSeatsRequiredQuery _countNewSmSeatsRequiredQuery; private readonly IUpdateSecretsManagerSubscriptionCommand _updateSecretsManagerSubscriptionCommand; + private readonly OrgUserInviteTokenableFactory _orgUserInviteTokenableFactory; private readonly IDataProtectorTokenFactory _orgUserInviteTokenDataFactory; public OrganizationService( @@ -83,6 +84,7 @@ public OrganizationService( IProviderUserRepository providerUserRepository, ICountNewSmSeatsRequiredQuery countNewSmSeatsRequiredQuery, IUpdateSecretsManagerSubscriptionCommand updateSecretsManagerSubscriptionCommand, + OrgUserInviteTokenableFactory orgUserInviteTokenableFactory, IDataProtectorTokenFactory orgUserInviteTokenDataFactory) { _organizationRepository = organizationRepository; @@ -111,6 +113,7 @@ public OrganizationService( _providerUserRepository = providerUserRepository; _countNewSmSeatsRequiredQuery = countNewSmSeatsRequiredQuery; _updateSecretsManagerSubscriptionCommand = updateSecretsManagerSubscriptionCommand; + _orgUserInviteTokenableFactory = orgUserInviteTokenableFactory; _orgUserInviteTokenDataFactory = orgUserInviteTokenDataFactory; } @@ -1049,7 +1052,7 @@ private async Task SendInvitesAsync(IEnumerable orgUsers, Orga { (OrganizationUser, ExpiringToken) MakeOrgUserExpiringTokenPair(OrganizationUser orgUser) { - var orgUserInviteTokenable = new OrgUserInviteTokenable(orgUser); + var orgUserInviteTokenable = _orgUserInviteTokenableFactory.CreateToken(orgUser); var protectedToken = _orgUserInviteTokenDataFactory.Protect(orgUserInviteTokenable); return (orgUser, new ExpiringToken(protectedToken, orgUserInviteTokenable.ExpirationDate)); } @@ -1065,7 +1068,7 @@ await _mailService.BulkSendOrganizationInviteEmailAsync( private async Task SendInviteAsync(OrganizationUser orgUser, Organization organization, bool initOrganization) { - var orgUserInviteTokenable = new OrgUserInviteTokenable(orgUser); + var orgUserInviteTokenable = _orgUserInviteTokenableFactory.CreateToken(orgUser); var protectedToken = _orgUserInviteTokenDataFactory.Protect(orgUserInviteTokenable); await _mailService.SendOrganizationInviteEmailAsync( organization.Name, diff --git a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs index 598979459202..f8d618077e9c 100644 --- a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs +++ b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs @@ -124,6 +124,9 @@ public static SupportedDatabaseProviders AddDatabaseRepositories(this IServiceCo public static void AddBaseServices(this IServiceCollection services, IGlobalSettings globalSettings) { + // TODO: ask if this is the correct place for this. + services.AddSingleton(); + services.AddScoped(); services.AddUserServices(globalSettings); services.AddOrganizationServices(globalSettings); From 2eeacfc7afcd47cde5c78988a41b5a2fe4b4dc35 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Mon, 18 Sep 2023 19:39:54 -0400 Subject: [PATCH 28/80] PM-3275 - In process work of creating AcceptOrgUserCommandTests.cs --- .../AcceptOrgUserCommandTests.cs | 48 ++++++++++++++----- 1 file changed, 37 insertions(+), 11 deletions(-) diff --git a/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs b/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs index 6071a8d0cb3c..ecee490bfb00 100644 --- a/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs +++ b/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs @@ -4,6 +4,7 @@ using Bit.Core.Repositories; using Bit.Core.Services; using Bit.Core.Settings; +using Bit.Core.Test.AutoFixture.OrganizationFixtures; using Bit.Core.Utilities; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; @@ -13,6 +14,8 @@ namespace Bit.Core.Test.OrganizationFeatures.OrganizationUsers; + +// Note: test names follow MethodName_StateUnderTest_ExpectedBehavior pattern. [SutProviderCustomize] public class AcceptOrgUserCommandTests { @@ -24,27 +27,50 @@ public AcceptOrgUserCommandTests() } [Theory] - [BitAutoData] - public async Task AcceptOrgUser_ByOrgUserId_Success(SutProvider sutProvider, - User user, Guid organizationUserId, string token, OrganizationUser orgUser) + [EphemeralDataProtectionAutoData] + public async Task AcceptOrgUserByToken_OldToken_Success(SutProvider sutProvider, + User user, Guid organizationUserId, OrganizationUser orgUser) { // Arrange + sutProvider.GetDependency().OrganizationInviteExpirationHours.Returns(24); + + user.EmailVerified = false; + orgUser.Status = OrganizationUserStatusType.Invited; + orgUser.Email = user.Email; + + sutProvider.GetDependency() .GetByIdAsync(organizationUserId) .Returns(orgUser); - // TODO: can't mock static methods - // See CoreHelpersTests to figure out how to properly mock valid token - CoreHelpers.UserInviteTokenIsValid(Arg.Any(), token, user.Email, orgUser.Id, - Arg.Any()) - .Returns(true); + var oldToken = CreateOldToken(sutProvider, orgUser); // Act - var result = await sutProvider.Sut.AcceptOrgUserAsync(organizationUserId, user, token, _userService); - + var result = await sutProvider.Sut.AcceptOrgUserByTokenAsync(organizationUserId, user, oldToken, _userService); // Assert - // Assert.Equal(IdentityResult.Success, result); + Assert.NotNull(result); + Assert.Equal(OrganizationUserStatusType.Accepted, result.Status); + Assert.Equal(orgUser.Id, result.Id); + Assert.Null(result.Email); + Assert.Equal(user.Id, result.UserId); + Assert.True(user.EmailVerified); // Verifying the EmailVerified flag. + } + + + + private string CreateOldToken(SutProvider sutProvider, + OrganizationUser organizationUser) + { + + var dataProtector = sutProvider.GetDependency() + .CreateProtector("OrganizationServiceDataProtector"); + + // Token matching the format used in OrganizationService.InviteUserAsync + var oldToken = dataProtector.Protect( + $"OrganizationUserInvite {organizationUser.Id} {organizationUser.Email} {CoreHelpers.ToEpocMilliseconds(DateTime.UtcNow)}"); + + return oldToken; } } From 9c5b8088fd7f97f6f5e4b41ec68288212b4b084b Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Tue, 19 Sep 2023 10:58:13 -0400 Subject: [PATCH 29/80] PM-3275 - Remove no longer relevant AcceptOrgUser tests from OrganizationServiceTests.cs --- .../Services/OrganizationServiceTests.cs | 37 ------------------- 1 file changed, 37 deletions(-) diff --git a/test/Core.Test/Services/OrganizationServiceTests.cs b/test/Core.Test/Services/OrganizationServiceTests.cs index 4907efafcb75..6b4ed692d7c0 100644 --- a/test/Core.Test/Services/OrganizationServiceTests.cs +++ b/test/Core.Test/Services/OrganizationServiceTests.cs @@ -28,7 +28,6 @@ using Bit.Core.Utilities; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; -using Microsoft.AspNetCore.DataProtection; using NSubstitute; using NSubstitute.ExceptionExtensions; using Xunit; @@ -1836,42 +1835,6 @@ public void ValidateSecretsManagerPlan_ValidPlan_NoExceptionThrown( sutProvider.Sut.ValidateSecretsManagerPlan(plan, signup); } - [Theory] - [EphemeralDataProtectionAutoData] - public async Task AcceptUserAsync_Success([OrganizationUser(OrganizationUserStatusType.Invited)] OrganizationUser organizationUser, - User user, SutProvider sutProvider) - { - var token = SetupAcceptUserAsyncTest(sutProvider, user, organizationUser); - var userService = Substitute.For(); - - await sutProvider.Sut.AcceptUserAsync(organizationUser.Id, user, token, userService); - - await sutProvider.GetDependency().Received(1).ReplaceAsync( - Arg.Is(ou => ou.Id == organizationUser.Id && ou.Status == OrganizationUserStatusType.Accepted)); - await sutProvider.GetDependency().Received(1).ReplaceAsync( - Arg.Is(u => u.Id == user.Id && u.Email == user.Email && user.EmailVerified == true)); - } - - private string SetupAcceptUserAsyncTest(SutProvider sutProvider, User user, - OrganizationUser organizationUser) - { - user.Email = organizationUser.Email; - user.EmailVerified = false; - - var dataProtector = sutProvider.GetDependency() - .CreateProtector("OrganizationServiceDataProtector"); - // Token matching the format used in OrganizationService.InviteUserAsync - var token = dataProtector.Protect( - $"OrganizationUserInvite {organizationUser.Id} {organizationUser.Email} {CoreHelpers.ToEpocMilliseconds(DateTime.UtcNow)}"); - - sutProvider.GetDependency().OrganizationInviteExpirationHours.Returns(24); - - sutProvider.GetDependency().GetByIdAsync(organizationUser.Id) - .Returns(organizationUser); - - return token; - } - [Theory] [OrganizationInviteCustomize( InviteeUserType = OrganizationUserType.Owner, From f33e51568da19cea72db0c913b586ec8d255e4b2 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Tue, 19 Sep 2023 11:05:28 -0400 Subject: [PATCH 30/80] PM-3275 - Register OrgUserInviteTokenableFactory alongside tokenizer --- src/SharedWeb/Utilities/ServiceCollectionExtensions.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs index f8d618077e9c..d16feb18cfe0 100644 --- a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs +++ b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs @@ -124,9 +124,6 @@ public static SupportedDatabaseProviders AddDatabaseRepositories(this IServiceCo public static void AddBaseServices(this IServiceCollection services, IGlobalSettings globalSettings) { - // TODO: ask if this is the correct place for this. - services.AddSingleton(); - services.AddScoped(); services.AddUserServices(globalSettings); services.AddOrganizationServices(globalSettings); @@ -176,6 +173,7 @@ public static void AddTokenizers(this IServiceCollection services) serviceProvider.GetDataProtectionProvider(), serviceProvider.GetRequiredService>>())); + services.AddSingleton(); services.AddSingleton>(serviceProvider => new DataProtectorTokenFactory( OrgUserInviteTokenable.ClearTextPrefix, From 3d55d70478162f364ec23a47cc9993b607acb0b7 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Tue, 19 Sep 2023 17:10:47 -0400 Subject: [PATCH 31/80] PM-3275 - AcceptOrgUserCommandTests.cs - AcceptOrgUserAsync basic test suite completed. --- .../AcceptOrgUserCommandTests.cs | 261 +++++++++++++++++- 1 file changed, 252 insertions(+), 9 deletions(-) diff --git a/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs b/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs index ecee490bfb00..b1426179ddab 100644 --- a/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs +++ b/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs @@ -1,5 +1,8 @@ -using Bit.Core.Entities; +using System.Text.Json; +using Bit.Core.Entities; using Bit.Core.Enums; +using Bit.Core.Exceptions; +using Bit.Core.Models.Data.Organizations.OrganizationUsers; using Bit.Core.OrganizationFeatures.OrganizationUsers; using Bit.Core.Repositories; using Bit.Core.Services; @@ -14,18 +17,162 @@ namespace Bit.Core.Test.OrganizationFeatures.OrganizationUsers; - // Note: test names follow MethodName_StateUnderTest_ExpectedBehavior pattern. [SutProviderCustomize] public class AcceptOrgUserCommandTests { - private readonly IUserService _userService; + private readonly IUserService _userService = Substitute.For(); + + [Theory] + [BitAutoData] + public async Task AcceptOrgUser_InvitedUserToSingleOrg_Success( + SutProvider sutProvider, + User user, Organization org, OrganizationUser orgUser, OrganizationUserUserDetails adminUserDetails) + { + // Arrange + SetupCommonAcceptOrgUserMocks(sutProvider, user, org, orgUser, adminUserDetails); + + // Act + var orgUserResult = await sutProvider.Sut.AcceptOrgUserAsync(orgUser, user, _userService); + + // Assert + + // Verify returned org user details + Assert.NotNull(orgUserResult); + Assert.Equal(OrganizationUserStatusType.Accepted, orgUserResult.Status); + Assert.Equal(orgUser.Id, orgUserResult.Id); + Assert.Null(orgUserResult.Email); + + // Verify org repository called with updated orgUser + await sutProvider.GetDependency().Received(1).ReplaceAsync( + Arg.Is(ou => ou.Id == orgUser.Id && ou.Status == OrganizationUserStatusType.Accepted)); + + // Verify emails sent to admin + await sutProvider.GetDependency().Received(1).SendOrganizationAcceptedEmailAsync( + Arg.Is(o => o.Id == org.Id), + Arg.Is(e => e == user.Email), + Arg.Is>(a => a.Contains(adminUserDetails.Email)) + ); + } + + [Theory] + [BitAutoData] + public async Task AcceptOrgUser_WhenUserStatusIsRevoked_ReturnsBadRequest( + SutProvider sutProvider, + User user, Organization org, OrganizationUser orgUser, OrganizationUserUserDetails adminUserDetails) + { + // Common setup + SetupCommonAcceptOrgUserMocks(sutProvider, user, org, orgUser, adminUserDetails); + + // Revoke user status + orgUser.Status = OrganizationUserStatusType.Revoked; + + // Act & Assert + var exception = await Assert.ThrowsAsync(() => + sutProvider.Sut.AcceptOrgUserAsync(orgUser, user, _userService)); + + Assert.Equal("Your organization access has been revoked.", exception.Message); + } - public AcceptOrgUserCommandTests() + [Theory] + [BitAutoData(OrganizationUserStatusType.Accepted)] + [BitAutoData(OrganizationUserStatusType.Confirmed)] + public async Task AcceptOrgUser_WhenUserStatusIsNotInvited_ThrowsBadRequest( + OrganizationUserStatusType orgUserStatus, + SutProvider sutProvider, + User user, Organization org, OrganizationUser orgUser, OrganizationUserUserDetails adminUserDetails) { - _userService = Substitute.For(); + // Arrange + SetupCommonAcceptOrgUserMocks(sutProvider, user, org, orgUser, adminUserDetails); + + // Set status to something other than invited + orgUser.Status = orgUserStatus; + + // Act & Assert + var exception = await Assert.ThrowsAsync(() => + sutProvider.Sut.AcceptOrgUserAsync(orgUser, user, _userService)); + + Assert.Equal("Already accepted.", exception.Message); } + [Theory] + [BitAutoData] + public async Task AcceptOrgUser_UserJoiningOrgWithSingleOrgPolicyWhileInAnotherOrg_ThrowsBadRequest( + SutProvider sutProvider, + User user, Organization org, OrganizationUser orgUser, OrganizationUserUserDetails adminUserDetails) + { + // Arrange + SetupCommonAcceptOrgUserMocks(sutProvider, user, org, orgUser, adminUserDetails); + + // Make user part of another org + var otherOrgUser = new OrganizationUser { UserId = user.Id, OrganizationId = Guid.NewGuid() }; // random org ID + sutProvider.GetDependency() + .GetManyByUserAsync(user.Id) + .Returns(Task.FromResult>(new List { otherOrgUser })); + + // Make organization they are trying to join have the single org policy + var singleOrgPolicy = new OrganizationUserPolicyDetails { OrganizationId = orgUser.OrganizationId }; + sutProvider.GetDependency() + .GetPoliciesApplicableToUserAsync(user.Id, PolicyType.SingleOrg, OrganizationUserStatusType.Invited) + .Returns(Task.FromResult>( + new List { singleOrgPolicy })); + + // Act & Assert + var exception = await Assert.ThrowsAsync(() => + sutProvider.Sut.AcceptOrgUserAsync(orgUser, user, _userService)); + + Assert.Equal("You may not join this organization until you leave or remove all other organizations.", exception.Message); + } + + [Theory] + [BitAutoData] + public async Task AcceptOrgUserAsync_UserInOrgWithSingleOrgPolicyAlready_ThrowsBadRequest( + SutProvider sutProvider, + User user, Organization org, OrganizationUser orgUser, OrganizationUserUserDetails adminUserDetails) + { + // Arrange + SetupCommonAcceptOrgUserMocks(sutProvider, user, org, orgUser, adminUserDetails); + + // Mock that user is part of an org that has the single org policy + sutProvider.GetDependency() + .AnyPoliciesApplicableToUserAsync(user.Id, PolicyType.SingleOrg) + .Returns(true); + + // Act & Assert + var exception = await Assert.ThrowsAsync(() => + sutProvider.Sut.AcceptOrgUserAsync(orgUser, user, _userService)); + + Assert.Equal("You cannot join this organization because you are a member of another organization which forbids it", exception.Message); + } + + + [Theory] + [BitAutoData] + public async Task AcceptOrgUserAsync_UserWithout2FAJoining2FARequiredOrg_ThrowsBadRequest( + SutProvider sutProvider, + User user, Organization org, OrganizationUser orgUser, OrganizationUserUserDetails adminUserDetails) + { + // Arrange + SetupCommonAcceptOrgUserMocks(sutProvider, user, org, orgUser, adminUserDetails); + + // User doesn't have 2FA enabled + _userService.TwoFactorIsEnabledAsync(user).Returns(false); + + // Organization they are trying to join requires 2FA + var twoFactorPolicy = new OrganizationUserPolicyDetails { OrganizationId = orgUser.OrganizationId }; + sutProvider.GetDependency() + .GetPoliciesApplicableToUserAsync(user.Id, PolicyType.TwoFactorAuthentication, OrganizationUserStatusType.Invited) + .Returns(Task.FromResult>( + new List { twoFactorPolicy })); + + // Act & Assert + var exception = await Assert.ThrowsAsync(() => + sutProvider.Sut.AcceptOrgUserAsync(orgUser, user, _userService)); + + Assert.Equal("You cannot join this organization until you enable two-step login on your user account.", exception.Message); + } + + [Theory] [EphemeralDataProtectionAutoData] public async Task AcceptOrgUserByToken_OldToken_Success(SutProvider sutProvider, @@ -39,13 +186,22 @@ public async Task AcceptOrgUserByToken_OldToken_Success(SutProvider() .GetByIdAsync(organizationUserId) - .Returns(orgUser); + .Returns(Task.FromResult(orgUser)); + + sutProvider.GetDependency() + .GetCountByOrganizationAsync(orgUser.OrganizationId, user.Email, true) + .Returns(0); var oldToken = CreateOldToken(sutProvider, orgUser); + // Mock successful call to AcceptOrgUserAsync (will be tested separately) + var orgUserJson = JsonSerializer.Serialize(orgUser); + var acceptedOrgUser = JsonSerializer.Deserialize(orgUserJson); + acceptedOrgUser.Status = OrganizationUserStatusType.Accepted; + + // Act var result = await sutProvider.Sut.AcceptOrgUserByTokenAsync(organizationUserId, user, oldToken, _userService); @@ -55,15 +211,65 @@ public async Task AcceptOrgUserByToken_OldToken_Success(SutProvider sutProvider, User user, Organization org, + OrganizationUser orgUser, OrganizationUserUserDetails adminUserDetails) + { + // Arrange + orgUser.Email = user.Email; + orgUser.Status = OrganizationUserStatusType.Invited; + orgUser.Type = OrganizationUserType.User; + orgUser.OrganizationId = org.Id; + + // User is not part of any other orgs + sutProvider.GetDependency() + .GetManyByUserAsync(user.Id) + .Returns( + Task.FromResult>(new List()) + ); + + // Org they are trying to join does not have single org policy + sutProvider.GetDependency() + .GetPoliciesApplicableToUserAsync(user.Id, PolicyType.SingleOrg, OrganizationUserStatusType.Invited) + .Returns( + Task.FromResult>( + new List() + ) + ); + + // User is not part of any organization that applies the single org policy + sutProvider.GetDependency() + .AnyPoliciesApplicableToUserAsync(user.Id, PolicyType.SingleOrg) + .Returns(false); + + // User doesn't have 2FA enabled + _userService.TwoFactorIsEnabledAsync(user).Returns(false); + + // Org does not require 2FA + sutProvider.GetDependency().GetPoliciesApplicableToUserAsync(user.Id, + PolicyType.TwoFactorAuthentication, OrganizationUserStatusType.Invited) + .Returns(Task.FromResult>( + new List())); + + // Provide at least 1 admin to test email functionality + sutProvider.GetDependency() + .GetManyByMinimumRoleAsync(orgUser.OrganizationId, OrganizationUserType.Admin) + .Returns(Task.FromResult>( + new List() { adminUserDetails } + )); + + // Return org + sutProvider.GetDependency() + .GetByIdAsync(org.Id) + .Returns(Task.FromResult(org)); + } private string CreateOldToken(SutProvider sutProvider, OrganizationUser organizationUser) { - var dataProtector = sutProvider.GetDependency() .CreateProtector("OrganizationServiceDataProtector"); @@ -74,3 +280,40 @@ private string CreateOldToken(SutProvider sutProvider, return oldToken; } } + +// Tests from OrganizationServiceTests.cs +// [Theory] +// [EphemeralDataProtectionAutoData] +// public async Task AcceptUserAsync_Success([OrganizationUser(OrganizationUserStatusType.Invited)] OrganizationUser organizationUser, +// User user, SutProvider sutProvider) +// { +// var token = SetupAcceptUserAsyncTest(sutProvider, user, organizationUser); +// var userService = Substitute.For(); +// +// await sutProvider.Sut.AcceptUserAsync(organizationUser.Id, user, token, userService); +// +// await sutProvider.GetDependency().Received(1).ReplaceAsync( +// Arg.Is(ou => ou.Id == organizationUser.Id && ou.Status == OrganizationUserStatusType.Accepted)); +// await sutProvider.GetDependency().Received(1).ReplaceAsync( +// Arg.Is(u => u.Id == user.Id && u.Email == user.Email && user.EmailVerified == true)); +// } +// +// private string SetupAcceptUserAsyncTest(SutProvider sutProvider, User user, +// OrganizationUser organizationUser) +// { +// user.Email = organizationUser.Email; +// user.EmailVerified = false; +// +// var dataProtector = sutProvider.GetDependency() +// .CreateProtector("OrganizationServiceDataProtector"); +// // Token matching the format used in OrganizationService.InviteUserAsync +// var token = dataProtector.Protect( +// $"OrganizationUserInvite {organizationUser.Id} {organizationUser.Email} {CoreHelpers.ToEpocMilliseconds(DateTime.UtcNow)}"); +// +// sutProvider.GetDependency().OrganizationInviteExpirationHours.Returns(24); +// +// sutProvider.GetDependency().GetByIdAsync(organizationUser.Id) +// .Returns(organizationUser); +// +// return token; +// } From a7a671268a70fbdbb1b7eb5c96573a6e6baf34a8 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Tue, 19 Sep 2023 17:12:03 -0400 Subject: [PATCH 32/80] PM-3275 - AcceptOrgUserCommandTests.cs - tweak test names --- .../OrganizationUsers/AcceptOrgUserCommandTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs b/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs index b1426179ddab..47673ff90df8 100644 --- a/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs +++ b/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs @@ -57,7 +57,7 @@ await sutProvider.GetDependency().Received(1).SendOrganizationAcce [Theory] [BitAutoData] - public async Task AcceptOrgUser_WhenUserStatusIsRevoked_ReturnsBadRequest( + public async Task AcceptOrgUser_OrgUserStatusIsRevoked_ReturnsBadRequest( SutProvider sutProvider, User user, Organization org, OrganizationUser orgUser, OrganizationUserUserDetails adminUserDetails) { @@ -77,7 +77,7 @@ public async Task AcceptOrgUser_WhenUserStatusIsRevoked_ReturnsBadRequest( [Theory] [BitAutoData(OrganizationUserStatusType.Accepted)] [BitAutoData(OrganizationUserStatusType.Confirmed)] - public async Task AcceptOrgUser_WhenUserStatusIsNotInvited_ThrowsBadRequest( + public async Task AcceptOrgUser_OrgUserStatusIsNotInvited_ThrowsBadRequest( OrganizationUserStatusType orgUserStatus, SutProvider sutProvider, User user, Organization org, OrganizationUser orgUser, OrganizationUserUserDetails adminUserDetails) From 62f5f4997a0b37c306506fd514756c2efaee223c Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Tue, 19 Sep 2023 17:29:28 -0400 Subject: [PATCH 33/80] PM-3275 - AcceptOrgUserCommandTests.cs - (1) Remove old tests from OrganizationServiceTests as no longer needed to reference (2) Add summary for SetupCommonAcceptOrgUserMocks (3) Get AcceptOrgUserByToken_OldToken_AcceptsUserAndVerifiesEmail passing --- .../AcceptOrgUserCommandTests.cs | 73 ++++++------------- 1 file changed, 22 insertions(+), 51 deletions(-) diff --git a/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs b/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs index 47673ff90df8..cac81f6983d9 100644 --- a/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs +++ b/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs @@ -175,19 +175,17 @@ public async Task AcceptOrgUserAsync_UserWithout2FAJoining2FARequiredOrg_ThrowsB [Theory] [EphemeralDataProtectionAutoData] - public async Task AcceptOrgUserByToken_OldToken_Success(SutProvider sutProvider, - User user, Guid organizationUserId, OrganizationUser orgUser) + public async Task AcceptOrgUserByToken_OldToken_AcceptsUserAndVerifiesEmail(SutProvider sutProvider, + User user, Organization org, OrganizationUser orgUser, OrganizationUserUserDetails adminUserDetails) { // Arrange - sutProvider.GetDependency().OrganizationInviteExpirationHours.Returns(24); + SetupCommonAcceptOrgUserMocks(sutProvider, user, org, orgUser, adminUserDetails); + sutProvider.GetDependency().OrganizationInviteExpirationHours.Returns(24); user.EmailVerified = false; - orgUser.Status = OrganizationUserStatusType.Invited; - orgUser.Email = user.Email; - sutProvider.GetDependency() - .GetByIdAsync(organizationUserId) + .GetByIdAsync(orgUser.Id) .Returns(Task.FromResult(orgUser)); sutProvider.GetDependency() @@ -196,14 +194,8 @@ public async Task AcceptOrgUserByToken_OldToken_Success(SutProvider(orgUserJson); - acceptedOrgUser.Status = OrganizationUserStatusType.Accepted; - - // Act - var result = await sutProvider.Sut.AcceptOrgUserByTokenAsync(organizationUserId, user, oldToken, _userService); + var result = await sutProvider.Sut.AcceptOrgUserByTokenAsync(orgUser.Id, user, oldToken, _userService); // Assert Assert.NotNull(result); @@ -211,9 +203,24 @@ public async Task AcceptOrgUserByToken_OldToken_Success(SutProvider().Received(1).ReplaceAsync( + Arg.Is(u => u.Id == user.Id && u.Email == user.Email && user.EmailVerified == true)); } + /// + /// Sets up common mock behavior for the AcceptOrgUserAsync tests. + /// This method initializes: + /// - The invited user's email, status, type, and organization ID. + /// - Ensures the user is not part of any other organizations. + /// - Confirms the target organization doesn't have a single org policy. + /// - Ensures the user doesn't belong to an organization with a single org policy. + /// - Assumes the user doesn't have 2FA enabled and the organization doesn't require it. + /// - Provides mock data for an admin to validate email functionality. + /// - Returns the corresponding organization for the given org ID. + /// private void SetupCommonAcceptOrgUserMocks(SutProvider sutProvider, User user, Organization org, OrganizationUser orgUser, OrganizationUserUserDetails adminUserDetails) { @@ -281,39 +288,3 @@ private string CreateOldToken(SutProvider sutProvider, } } -// Tests from OrganizationServiceTests.cs -// [Theory] -// [EphemeralDataProtectionAutoData] -// public async Task AcceptUserAsync_Success([OrganizationUser(OrganizationUserStatusType.Invited)] OrganizationUser organizationUser, -// User user, SutProvider sutProvider) -// { -// var token = SetupAcceptUserAsyncTest(sutProvider, user, organizationUser); -// var userService = Substitute.For(); -// -// await sutProvider.Sut.AcceptUserAsync(organizationUser.Id, user, token, userService); -// -// await sutProvider.GetDependency().Received(1).ReplaceAsync( -// Arg.Is(ou => ou.Id == organizationUser.Id && ou.Status == OrganizationUserStatusType.Accepted)); -// await sutProvider.GetDependency().Received(1).ReplaceAsync( -// Arg.Is(u => u.Id == user.Id && u.Email == user.Email && user.EmailVerified == true)); -// } -// -// private string SetupAcceptUserAsyncTest(SutProvider sutProvider, User user, -// OrganizationUser organizationUser) -// { -// user.Email = organizationUser.Email; -// user.EmailVerified = false; -// -// var dataProtector = sutProvider.GetDependency() -// .CreateProtector("OrganizationServiceDataProtector"); -// // Token matching the format used in OrganizationService.InviteUserAsync -// var token = dataProtector.Protect( -// $"OrganizationUserInvite {organizationUser.Id} {organizationUser.Email} {CoreHelpers.ToEpocMilliseconds(DateTime.UtcNow)}"); -// -// sutProvider.GetDependency().OrganizationInviteExpirationHours.Returns(24); -// -// sutProvider.GetDependency().GetByIdAsync(organizationUser.Id) -// .Returns(organizationUser); -// -// return token; -// } From 97b350386b603478e36a43f79f3fad8606ae6bcd Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Tue, 19 Sep 2023 18:00:03 -0400 Subject: [PATCH 34/80] PM-3275 - Create interface for OrgUserInviteTokenableFactory b/c that's the right thing to do + enables test substitution --- .../Business/Tokenables/IOrgUserInviteTokenableFactory.cs | 8 ++++++++ .../Business/Tokenables/OrgUserInviteTokenableFactory.cs | 2 +- src/Core/Services/Implementations/OrganizationService.cs | 4 ++-- src/SharedWeb/Utilities/ServiceCollectionExtensions.cs | 2 +- 4 files changed, 12 insertions(+), 4 deletions(-) create mode 100644 src/Core/Auth/Models/Business/Tokenables/IOrgUserInviteTokenableFactory.cs diff --git a/src/Core/Auth/Models/Business/Tokenables/IOrgUserInviteTokenableFactory.cs b/src/Core/Auth/Models/Business/Tokenables/IOrgUserInviteTokenableFactory.cs new file mode 100644 index 000000000000..04d3b008b1c5 --- /dev/null +++ b/src/Core/Auth/Models/Business/Tokenables/IOrgUserInviteTokenableFactory.cs @@ -0,0 +1,8 @@ +using Bit.Core.Entities; + +namespace Bit.Core.Auth.Models.Business.Tokenables; + +public interface IOrgUserInviteTokenableFactory +{ + OrgUserInviteTokenable CreateToken(OrganizationUser orgUser); +} diff --git a/src/Core/Auth/Models/Business/Tokenables/OrgUserInviteTokenableFactory.cs b/src/Core/Auth/Models/Business/Tokenables/OrgUserInviteTokenableFactory.cs index 5ec4502639da..9e5a797dbb1e 100644 --- a/src/Core/Auth/Models/Business/Tokenables/OrgUserInviteTokenableFactory.cs +++ b/src/Core/Auth/Models/Business/Tokenables/OrgUserInviteTokenableFactory.cs @@ -3,7 +3,7 @@ namespace Bit.Core.Auth.Models.Business.Tokenables; -public class OrgUserInviteTokenableFactory +public class OrgUserInviteTokenableFactory : IOrgUserInviteTokenableFactory { private readonly IGlobalSettings _globalSettings; diff --git a/src/Core/Services/Implementations/OrganizationService.cs b/src/Core/Services/Implementations/OrganizationService.cs index 63d7a9e60fb6..1842a012216e 100644 --- a/src/Core/Services/Implementations/OrganizationService.cs +++ b/src/Core/Services/Implementations/OrganizationService.cs @@ -54,7 +54,7 @@ public class OrganizationService : IOrganizationService private readonly IProviderUserRepository _providerUserRepository; private readonly ICountNewSmSeatsRequiredQuery _countNewSmSeatsRequiredQuery; private readonly IUpdateSecretsManagerSubscriptionCommand _updateSecretsManagerSubscriptionCommand; - private readonly OrgUserInviteTokenableFactory _orgUserInviteTokenableFactory; + private readonly IOrgUserInviteTokenableFactory _orgUserInviteTokenableFactory; private readonly IDataProtectorTokenFactory _orgUserInviteTokenDataFactory; public OrganizationService( @@ -84,7 +84,7 @@ public OrganizationService( IProviderUserRepository providerUserRepository, ICountNewSmSeatsRequiredQuery countNewSmSeatsRequiredQuery, IUpdateSecretsManagerSubscriptionCommand updateSecretsManagerSubscriptionCommand, - OrgUserInviteTokenableFactory orgUserInviteTokenableFactory, + IOrgUserInviteTokenableFactory orgUserInviteTokenableFactory, IDataProtectorTokenFactory orgUserInviteTokenDataFactory) { _organizationRepository = organizationRepository; diff --git a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs index d16feb18cfe0..d3c3cd9f742d 100644 --- a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs +++ b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs @@ -173,7 +173,7 @@ public static void AddTokenizers(this IServiceCollection services) serviceProvider.GetDataProtectionProvider(), serviceProvider.GetRequiredService>>())); - services.AddSingleton(); + services.AddSingleton(); services.AddSingleton>(serviceProvider => new DataProtectorTokenFactory( OrgUserInviteTokenable.ClearTextPrefix, From d6791ab575204939515cba9bcae1378173b6e7a6 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Tue, 19 Sep 2023 18:57:50 -0400 Subject: [PATCH 35/80] PM-3275 - AcceptOrgUserCommandTests.cs - (1) Start work on AcceptOrgUserByToken_NewToken_AcceptsUserAndVerifiesEmail (2) Create and use SetupCommonAcceptOrgUserByTokenMocks() (3) Create generic FakeDataProtectorTokenFactory for tokenable testing --- .../AcceptOrgUserCommandTests.cs | 154 +++++++++++++++--- 1 file changed, 134 insertions(+), 20 deletions(-) diff --git a/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs b/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs index cac81f6983d9..f37b5c30a21b 100644 --- a/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs +++ b/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs @@ -1,4 +1,5 @@ using System.Text.Json; +using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; @@ -8,20 +9,75 @@ using Bit.Core.Services; using Bit.Core.Settings; using Bit.Core.Test.AutoFixture.OrganizationFixtures; +using Bit.Core.Tokens; using Bit.Core.Utilities; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; using Microsoft.AspNetCore.DataProtection; using NSubstitute; using Xunit; +using Xunit.Sdk; namespace Bit.Core.Test.OrganizationFeatures.OrganizationUsers; + +public class FakeDataProtectorTokenFactory : IDataProtectorTokenFactory where T : Tokenable, new() +{ + // Instead of real encryption, use a simple Dictionary to emulate protection/unprotection + private readonly Dictionary _tokenDatabase = new Dictionary(); + + public string Protect(T data) + { + // Generate a simple token representation + var token = Guid.NewGuid().ToString(); + + // Store the data against the token + _tokenDatabase[token] = data; + + return token; + } + + public T Unprotect(string token) + { + // If the token exists in the dictionary, return the corresponding data + if (_tokenDatabase.TryGetValue(token, out var data)) + { + return data; + } + + // If the token doesn't exist, throw an exception similar to a decryption failure. + throw new Exception("Failed to unprotect token."); + } + + public bool TryUnprotect(string token, out T data) + { + try + { + data = Unprotect(token); + return true; + } + catch + { + data = default; + return false; + } + } + + public bool TokenValid(string token) + { + return _tokenDatabase.ContainsKey(token); + } +} + // Note: test names follow MethodName_StateUnderTest_ExpectedBehavior pattern. [SutProviderCustomize] public class AcceptOrgUserCommandTests { private readonly IUserService _userService = Substitute.For(); + private readonly IOrgUserInviteTokenableFactory _orgUserInviteTokenableFactory = Substitute.For(); + + // private readonly IDataProtectorTokenFactory _orgUserInviteTokenDataFactory = Substitute.For>(); + private readonly IDataProtectorTokenFactory _orgUserInviteTokenDataFactory = new FakeDataProtectorTokenFactory(); [Theory] [BitAutoData] @@ -68,10 +124,10 @@ public async Task AcceptOrgUser_OrgUserStatusIsRevoked_ReturnsBadRequest( orgUser.Status = OrganizationUserStatusType.Revoked; // Act & Assert - var exception = await Assert.ThrowsAsync(() => - sutProvider.Sut.AcceptOrgUserAsync(orgUser, user, _userService)); + var exception = await Assert.ThrowsAsync(() => + sutProvider.Sut.AcceptOrgUserAsync(orgUser, user, _userService)); - Assert.Equal("Your organization access has been revoked.", exception.Message); + Assert.Equal("Your organization access has been revoked.", exception.Message); } [Theory] @@ -121,7 +177,8 @@ public async Task AcceptOrgUser_UserJoiningOrgWithSingleOrgPolicyWhileInAnotherO var exception = await Assert.ThrowsAsync(() => sutProvider.Sut.AcceptOrgUserAsync(orgUser, user, _userService)); - Assert.Equal("You may not join this organization until you leave or remove all other organizations.", exception.Message); + Assert.Equal("You may not join this organization until you leave or remove all other organizations.", + exception.Message); } [Theory] @@ -142,7 +199,9 @@ public async Task AcceptOrgUserAsync_UserInOrgWithSingleOrgPolicyAlready_ThrowsB var exception = await Assert.ThrowsAsync(() => sutProvider.Sut.AcceptOrgUserAsync(orgUser, user, _userService)); - Assert.Equal("You cannot join this organization because you are a member of another organization which forbids it", exception.Message); + Assert.Equal( + "You cannot join this organization because you are a member of another organization which forbids it", + exception.Message); } @@ -161,7 +220,8 @@ public async Task AcceptOrgUserAsync_UserWithout2FAJoining2FARequiredOrg_ThrowsB // Organization they are trying to join requires 2FA var twoFactorPolicy = new OrganizationUserPolicyDetails { OrganizationId = orgUser.OrganizationId }; sutProvider.GetDependency() - .GetPoliciesApplicableToUserAsync(user.Id, PolicyType.TwoFactorAuthentication, OrganizationUserStatusType.Invited) + .GetPoliciesApplicableToUserAsync(user.Id, PolicyType.TwoFactorAuthentication, + OrganizationUserStatusType.Invited) .Returns(Task.FromResult>( new List { twoFactorPolicy })); @@ -169,33 +229,65 @@ public async Task AcceptOrgUserAsync_UserWithout2FAJoining2FARequiredOrg_ThrowsB var exception = await Assert.ThrowsAsync(() => sutProvider.Sut.AcceptOrgUserAsync(orgUser, user, _userService)); - Assert.Equal("You cannot join this organization until you enable two-step login on your user account.", exception.Message); + Assert.Equal("You cannot join this organization until you enable two-step login on your user account.", + exception.Message); } [Theory] [EphemeralDataProtectionAutoData] - public async Task AcceptOrgUserByToken_OldToken_AcceptsUserAndVerifiesEmail(SutProvider sutProvider, + public async Task AcceptOrgUserByToken_OldToken_AcceptsUserAndVerifiesEmail( + SutProvider sutProvider, User user, Organization org, OrganizationUser orgUser, OrganizationUserUserDetails adminUserDetails) { // Arrange SetupCommonAcceptOrgUserMocks(sutProvider, user, org, orgUser, adminUserDetails); + SetupCommonAcceptOrgUserByTokenMocks(sutProvider, user, orgUser); - sutProvider.GetDependency().OrganizationInviteExpirationHours.Returns(24); - user.EmailVerified = false; + var oldToken = CreateOldToken(sutProvider, orgUser); - sutProvider.GetDependency() - .GetByIdAsync(orgUser.Id) - .Returns(Task.FromResult(orgUser)); + // Act + var result = await sutProvider.Sut.AcceptOrgUserByTokenAsync(orgUser.Id, user, oldToken, _userService); - sutProvider.GetDependency() - .GetCountByOrganizationAsync(orgUser.OrganizationId, user.Email, true) - .Returns(0); + // Assert + Assert.NotNull(result); + Assert.Equal(OrganizationUserStatusType.Accepted, result.Status); + Assert.Equal(orgUser.Id, result.Id); + Assert.Null(result.Email); + Assert.Equal(user.Id, result.UserId); - var oldToken = CreateOldToken(sutProvider, orgUser); + // Verify user email verified logic + Assert.True(user.EmailVerified); + await sutProvider.GetDependency().Received(1).ReplaceAsync( + Arg.Is(u => u.Id == user.Id && u.Email == user.Email && user.EmailVerified == true)); + } + + [Theory] + [BitAutoData] + public async Task AcceptOrgUserByToken_NewToken_AcceptsUserAndVerifiesEmail( + SutProvider sutProvider, + User user, Organization org, OrganizationUser orgUser, OrganizationUserUserDetails adminUserDetails) + { + // Arrange + SetupCommonAcceptOrgUserMocks(sutProvider, user, org, orgUser, adminUserDetails); + SetupCommonAcceptOrgUserByTokenMocks(sutProvider, user, orgUser); + + // Mock tokenable factory to return a token that expires in 5 days + _orgUserInviteTokenableFactory.CreateToken(orgUser).Returns(new OrgUserInviteTokenable(orgUser) + { + ExpirationDate = DateTime.UtcNow.Add(TimeSpan.FromDays(5)) + }); + + // TODO: figure out how to do this. Either have to instantiate the command with the fake data protection token factory + // in constructor or maybe create a CustomizedBitAutoDataAttribute + // Command must use fake data protection token factory for token validation so that our created tokens in + // CreateNewToken are seen as valid in the command. + // sutProvider.GetDependency>().Returns(_orgUserInviteTokenDataFactory); + + var newToken = CreateNewToken(orgUser); // Act - var result = await sutProvider.Sut.AcceptOrgUserByTokenAsync(orgUser.Id, user, oldToken, _userService); + var result = await sutProvider.Sut.AcceptOrgUserByTokenAsync(orgUser.Id, user, newToken, _userService); // Assert Assert.NotNull(result); @@ -210,6 +302,20 @@ await sutProvider.GetDependency().Received(1).ReplaceAsync( Arg.Is(u => u.Id == user.Id && u.Email == user.Email && user.EmailVerified == true)); } + private void SetupCommonAcceptOrgUserByTokenMocks(SutProvider sutProvider, User user, OrganizationUser orgUser) + { + sutProvider.GetDependency().OrganizationInviteExpirationHours.Returns(24); + user.EmailVerified = false; + + sutProvider.GetDependency() + .GetByIdAsync(orgUser.Id) + .Returns(Task.FromResult(orgUser)); + + sutProvider.GetDependency() + .GetCountByOrganizationAsync(orgUser.OrganizationId, user.Email, true) + .Returns(0); + } + /// /// Sets up common mock behavior for the AcceptOrgUserAsync tests. /// This method initializes: @@ -221,7 +327,8 @@ await sutProvider.GetDependency().Received(1).ReplaceAsync( /// - Provides mock data for an admin to validate email functionality. /// - Returns the corresponding organization for the given org ID. /// - private void SetupCommonAcceptOrgUserMocks(SutProvider sutProvider, User user, Organization org, + private void SetupCommonAcceptOrgUserMocks(SutProvider sutProvider, User user, + Organization org, OrganizationUser orgUser, OrganizationUserUserDetails adminUserDetails) { // Arrange @@ -286,5 +393,12 @@ private string CreateOldToken(SutProvider sutProvider, return oldToken; } -} + private string CreateNewToken(OrganizationUser orgUser) + { + var orgUserInviteTokenable = _orgUserInviteTokenableFactory.CreateToken(orgUser); + var protectedToken = _orgUserInviteTokenDataFactory.Protect(orgUserInviteTokenable); + + return protectedToken; + } +} From b1b786ff07d7b3b47731e219ca9700c61b099830 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Wed, 20 Sep 2023 11:48:26 -0400 Subject: [PATCH 36/80] PM-3275 - (1) Get AcceptOrgUserByToken_NewToken_AcceptsUserAndVerifiesEmail test passing (2) Move FakeDataProtectorTokenFactory to own file --- .../Fakes/FakeDataProtectorTokenFactory.cs | 55 +++++++++++++++ .../AcceptOrgUserCommandTests.cs | 67 +++---------------- 2 files changed, 63 insertions(+), 59 deletions(-) create mode 100644 test/Common/Fakes/FakeDataProtectorTokenFactory.cs diff --git a/test/Common/Fakes/FakeDataProtectorTokenFactory.cs b/test/Common/Fakes/FakeDataProtectorTokenFactory.cs new file mode 100644 index 000000000000..fe3af320d69a --- /dev/null +++ b/test/Common/Fakes/FakeDataProtectorTokenFactory.cs @@ -0,0 +1,55 @@ +using Bit.Core.Tokens; + +namespace Bit.Test.Common.Fakes; + +/// +/// Used to fake the IDataProtectorTokenFactory for testing purposes. +/// Generalized for use with all Tokenables. +/// +public class FakeDataProtectorTokenFactory : IDataProtectorTokenFactory where T : Tokenable, new() +{ + // Instead of real encryption, use a simple Dictionary to emulate protection/unprotection + private readonly Dictionary _tokenDatabase = new Dictionary(); + + public string Protect(T data) + { + // Generate a simple token representation + var token = Guid.NewGuid().ToString(); + + // Store the data against the token + _tokenDatabase[token] = data; + + return token; + } + + public T Unprotect(string token) + { + // If the token exists in the dictionary, return the corresponding data + if (_tokenDatabase.TryGetValue(token, out var data)) + { + return data; + } + + // If the token doesn't exist, throw an exception similar to a decryption failure. + throw new Exception("Failed to unprotect token."); + } + + public bool TryUnprotect(string token, out T data) + { + try + { + data = Unprotect(token); + return true; + } + catch + { + data = default; + return false; + } + } + + public bool TokenValid(string token) + { + return _tokenDatabase.ContainsKey(token); + } +} diff --git a/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs b/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs index f37b5c30a21b..af9b9a844aa9 100644 --- a/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs +++ b/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs @@ -1,5 +1,4 @@ -using System.Text.Json; -using Bit.Core.Auth.Models.Business.Tokenables; +using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; @@ -13,62 +12,13 @@ using Bit.Core.Utilities; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; +using Bit.Test.Common.Fakes; using Microsoft.AspNetCore.DataProtection; using NSubstitute; using Xunit; -using Xunit.Sdk; namespace Bit.Core.Test.OrganizationFeatures.OrganizationUsers; - -public class FakeDataProtectorTokenFactory : IDataProtectorTokenFactory where T : Tokenable, new() -{ - // Instead of real encryption, use a simple Dictionary to emulate protection/unprotection - private readonly Dictionary _tokenDatabase = new Dictionary(); - - public string Protect(T data) - { - // Generate a simple token representation - var token = Guid.NewGuid().ToString(); - - // Store the data against the token - _tokenDatabase[token] = data; - - return token; - } - - public T Unprotect(string token) - { - // If the token exists in the dictionary, return the corresponding data - if (_tokenDatabase.TryGetValue(token, out var data)) - { - return data; - } - - // If the token doesn't exist, throw an exception similar to a decryption failure. - throw new Exception("Failed to unprotect token."); - } - - public bool TryUnprotect(string token, out T data) - { - try - { - data = Unprotect(token); - return true; - } - catch - { - data = default; - return false; - } - } - - public bool TokenValid(string token) - { - return _tokenDatabase.ContainsKey(token); - } -} - // Note: test names follow MethodName_StateUnderTest_ExpectedBehavior pattern. [SutProviderCustomize] public class AcceptOrgUserCommandTests @@ -76,7 +26,6 @@ public class AcceptOrgUserCommandTests private readonly IUserService _userService = Substitute.For(); private readonly IOrgUserInviteTokenableFactory _orgUserInviteTokenableFactory = Substitute.For(); - // private readonly IDataProtectorTokenFactory _orgUserInviteTokenDataFactory = Substitute.For>(); private readonly IDataProtectorTokenFactory _orgUserInviteTokenDataFactory = new FakeDataProtectorTokenFactory(); [Theory] @@ -269,21 +218,21 @@ public async Task AcceptOrgUserByToken_NewToken_AcceptsUserAndVerifiesEmail( User user, Organization org, OrganizationUser orgUser, OrganizationUserUserDetails adminUserDetails) { // Arrange + // Setup FakeDataProtectorTokenFactory for creating new tokens - this must come first in order + // to avoid resetting mocks + sutProvider.SetDependency(_orgUserInviteTokenDataFactory, "orgUserInviteTokenDataFactory"); + sutProvider.Create(); + SetupCommonAcceptOrgUserMocks(sutProvider, user, org, orgUser, adminUserDetails); SetupCommonAcceptOrgUserByTokenMocks(sutProvider, user, orgUser); + // Must come after common mocks as they mutate the org user. // Mock tokenable factory to return a token that expires in 5 days _orgUserInviteTokenableFactory.CreateToken(orgUser).Returns(new OrgUserInviteTokenable(orgUser) { ExpirationDate = DateTime.UtcNow.Add(TimeSpan.FromDays(5)) }); - // TODO: figure out how to do this. Either have to instantiate the command with the fake data protection token factory - // in constructor or maybe create a CustomizedBitAutoDataAttribute - // Command must use fake data protection token factory for token validation so that our created tokens in - // CreateNewToken are seen as valid in the command. - // sutProvider.GetDependency>().Returns(_orgUserInviteTokenDataFactory); - var newToken = CreateNewToken(orgUser); // Act From bb071abd0b117f57c8126c6cdf74adc0de220ed1 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Wed, 20 Sep 2023 16:18:15 -0400 Subject: [PATCH 37/80] PM-3275 - AcceptOrgUserCommandTests.cs - Finish up tests for AcceptOrgUserByTokenAsync --- .../AcceptOrgUserCommandTests.cs | 171 ++++++++++++++++++ 1 file changed, 171 insertions(+) diff --git a/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs b/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs index af9b9a844aa9..c046e06e1bec 100644 --- a/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs +++ b/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs @@ -251,6 +251,177 @@ await sutProvider.GetDependency().Received(1).ReplaceAsync( Arg.Is(u => u.Id == user.Id && u.Email == user.Email && user.EmailVerified == true)); } + [Theory] + [BitAutoData] + public async Task AcceptOrgUserByToken_NullOrgUser_ThrowsBadRequest( + SutProvider sutProvider, + User user, Guid orgUserId) + { + // Arrange + sutProvider.GetDependency() + .GetByIdAsync(orgUserId).Returns((OrganizationUser)null); + + // Act & Assert + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.AcceptOrgUserByTokenAsync(orgUserId, user, "token", _userService)); + + Assert.Equal("User invalid.", exception.Message); + } + + [Theory] + [BitAutoData] + public async Task AcceptOrgUserByToken_GenericInvalidToken_ThrowsBadRequest( + SutProvider sutProvider, + User user, OrganizationUser orgUser) + { + // Arrange + sutProvider.GetDependency() + .GetByIdAsync(orgUser.Id) + .Returns(Task.FromResult(orgUser)); + + var invalidToken = "invalidToken"; + + // Act & Assert + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.AcceptOrgUserByTokenAsync(orgUser.Id, user, invalidToken, _userService)); + + Assert.Equal("Invalid token.", exception.Message); + } + + [Theory] + [EphemeralDataProtectionAutoData] + public async Task AcceptOrgUserByToken_ExpiredOldToken_ThrowsBadRequest( + SutProvider sutProvider, + User user, Organization org, OrganizationUser orgUser, OrganizationUserUserDetails adminUserDetails) + { + // Arrange + SetupCommonAcceptOrgUserMocks(sutProvider, user, org, orgUser, adminUserDetails); + SetupCommonAcceptOrgUserByTokenMocks(sutProvider, user, orgUser); + + // As the old token simply set a timestamp which was later compared against the + // OrganizationInviteExpirationHours global setting to determine if it was expired or not, + // we can simply set the expiration to 24 hours ago to simulate an expired token. + sutProvider.GetDependency().OrganizationInviteExpirationHours.Returns(-24); + + var oldToken = CreateOldToken(sutProvider, orgUser); + + // Act & Assert + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.AcceptOrgUserByTokenAsync(orgUser.Id, user, oldToken, _userService)); + + Assert.Equal("Invalid token.", exception.Message); + } + + [Theory] + [BitAutoData] + public async Task AcceptOrgUserByToken_ExpiredNewToken_ThrowsBadRequest( + SutProvider sutProvider, + User user, OrganizationUser orgUser) + { + // Arrange + // Setup FakeDataProtectorTokenFactory for creating new tokens - this must come first in order + // to avoid resetting mocks + sutProvider.SetDependency(_orgUserInviteTokenDataFactory, "orgUserInviteTokenDataFactory"); + sutProvider.Create(); + + sutProvider.GetDependency() + .GetByIdAsync(orgUser.Id) + .Returns(Task.FromResult(orgUser)); + + // Must come after common mocks as they mutate the org user. + // Mock tokenable factory to return a token that expired yesterday + _orgUserInviteTokenableFactory.CreateToken(orgUser).Returns(new OrgUserInviteTokenable(orgUser) + { + ExpirationDate = DateTime.UtcNow.Add(TimeSpan.FromDays(-1)) + }); + + var newToken = CreateNewToken(orgUser); + + // Act & Assert + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.AcceptOrgUserByTokenAsync(orgUser.Id, user, newToken, _userService)); + + Assert.Equal("Invalid token.", exception.Message); + + } + + [Theory] + [BitAutoData(OrganizationUserStatusType.Accepted, + "Invitation already accepted. You will receive an email when your organization membership is confirmed.")] + [BitAutoData(OrganizationUserStatusType.Confirmed, + "You are already part of this organization.")] + public async Task AcceptOrgUserByToken_UserAlreadyInOrg_ThrowsBadRequest( + OrganizationUserStatusType statusType, + string expectedErrorMessage, + SutProvider sutProvider, + User user, OrganizationUser orgUser) + { + // Arrange + // Setup FakeDataProtectorTokenFactory for creating new tokens - this must come first in order + // to avoid resetting mocks + sutProvider.SetDependency(_orgUserInviteTokenDataFactory, "orgUserInviteTokenDataFactory"); + sutProvider.Create(); + + sutProvider.GetDependency() + .GetByIdAsync(orgUser.Id) + .Returns(Task.FromResult(orgUser)); + + // Indicate that a user with the given email already exists in the organization + sutProvider.GetDependency() + .GetCountByOrganizationAsync(orgUser.OrganizationId, user.Email, true) + .Returns(1); + + orgUser.Status = statusType; + + // Must come after common mocks as they mutate the org user. + // Mock tokenable factory to return valid, new token that expires in 5 days + _orgUserInviteTokenableFactory.CreateToken(orgUser).Returns(new OrgUserInviteTokenable(orgUser) + { + ExpirationDate = DateTime.UtcNow.Add(TimeSpan.FromDays(5)) + }); + + var newToken = CreateNewToken(orgUser); + + // Act & Assert + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.AcceptOrgUserByTokenAsync(orgUser.Id, user, newToken, _userService)); + + Assert.Equal(expectedErrorMessage, exception.Message); + } + + [Theory] + [BitAutoData] + public async Task AcceptOrgUserByToken_EmailMismatch_ThrowsBadRequest( + SutProvider sutProvider, + User user, Organization org, OrganizationUser orgUser, OrganizationUserUserDetails adminUserDetails) + { + // Arrange + // Setup FakeDataProtectorTokenFactory for creating new tokens - this must come first in order + // to avoid resetting mocks + sutProvider.SetDependency(_orgUserInviteTokenDataFactory, "orgUserInviteTokenDataFactory"); + sutProvider.Create(); + + SetupCommonAcceptOrgUserByTokenMocks(sutProvider, user, orgUser); + + // Modify the orgUser's email to be different from the user's email to simulate the mismatch + orgUser.Email = "mismatchedEmail@example.com"; + + // Must come after common mocks as they mutate the org user. + // Mock tokenable factory to return a token that expires in 5 days + _orgUserInviteTokenableFactory.CreateToken(orgUser).Returns(new OrgUserInviteTokenable(orgUser) + { + ExpirationDate = DateTime.UtcNow.Add(TimeSpan.FromDays(5)) + }); + + var newToken = CreateNewToken(orgUser); + + // Act & Assert + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.AcceptOrgUserByTokenAsync(orgUser.Id, user, newToken, _userService)); + + Assert.Equal("User email does not match invite.", exception.Message); + } + private void SetupCommonAcceptOrgUserByTokenMocks(SutProvider sutProvider, User user, OrganizationUser orgUser) { sutProvider.GetDependency().OrganizationInviteExpirationHours.Returns(24); From 85e531ed78126f313139d559e11bcc3a1ed225bb Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Wed, 20 Sep 2023 16:24:19 -0400 Subject: [PATCH 38/80] PM-3275 - Add pseudo section comments --- .../OrganizationUsers/AcceptOrgUserCommandTests.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs b/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs index c046e06e1bec..1fd3943f10cb 100644 --- a/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs +++ b/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs @@ -28,6 +28,8 @@ public class AcceptOrgUserCommandTests private readonly IDataProtectorTokenFactory _orgUserInviteTokenDataFactory = new FakeDataProtectorTokenFactory(); + // Base AcceptOrgUserAsync method tests ---------------------------------------------------------------------------- + [Theory] [BitAutoData] public async Task AcceptOrgUser_InvitedUserToSingleOrg_Success( @@ -183,6 +185,8 @@ public async Task AcceptOrgUserAsync_UserWithout2FAJoining2FARequiredOrg_ThrowsB } + // AcceptOrgUserByOrgIdAsync tests -------------------------------------------------------------------------------- + [Theory] [EphemeralDataProtectionAutoData] public async Task AcceptOrgUserByToken_OldToken_AcceptsUserAndVerifiesEmail( @@ -422,6 +426,8 @@ public async Task AcceptOrgUserByToken_EmailMismatch_ThrowsBadRequest( Assert.Equal("User email does not match invite.", exception.Message); } + // Private helpers ------------------------------------------------------------------------------------------------- + private void SetupCommonAcceptOrgUserByTokenMocks(SutProvider sutProvider, User user, OrganizationUser orgUser) { sutProvider.GetDependency().OrganizationInviteExpirationHours.Returns(24); From d49f99497b4e0ec95bd8155aded7eed20e09a14a Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Wed, 20 Sep 2023 16:25:07 -0400 Subject: [PATCH 39/80] PM-3275 - Clean up unused params on AcceptOrgUserByToken_EmailMismatch_ThrowsBadRequest test --- .../OrganizationUsers/AcceptOrgUserCommandTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs b/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs index 1fd3943f10cb..7939525229fe 100644 --- a/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs +++ b/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs @@ -397,7 +397,7 @@ public async Task AcceptOrgUserByToken_UserAlreadyInOrg_ThrowsBadRequest( [BitAutoData] public async Task AcceptOrgUserByToken_EmailMismatch_ThrowsBadRequest( SutProvider sutProvider, - User user, Organization org, OrganizationUser orgUser, OrganizationUserUserDetails adminUserDetails) + User user, OrganizationUser orgUser) { // Arrange // Setup FakeDataProtectorTokenFactory for creating new tokens - this must come first in order From a022de8d9f7ee7cc1b92b6d4c765218a423850b0 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Wed, 20 Sep 2023 16:51:02 -0400 Subject: [PATCH 40/80] PM-3275 - (1) Tests written for AcceptOrgUserByOrgSsoIdAsync (2) Refactor happy path assertions into helper function AssertValidAcceptedOrgUser to reduce code duplication --- .../AcceptOrgUserCommandTests.cs | 96 ++++++++++++++++--- 1 file changed, 84 insertions(+), 12 deletions(-) diff --git a/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs b/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs index 7939525229fe..431a160e49aa 100644 --- a/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs +++ b/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs @@ -200,14 +200,10 @@ public async Task AcceptOrgUserByToken_OldToken_AcceptsUserAndVerifiesEmail( var oldToken = CreateOldToken(sutProvider, orgUser); // Act - var result = await sutProvider.Sut.AcceptOrgUserByTokenAsync(orgUser.Id, user, oldToken, _userService); + var resultOrgUser = await sutProvider.Sut.AcceptOrgUserByTokenAsync(orgUser.Id, user, oldToken, _userService); // Assert - Assert.NotNull(result); - Assert.Equal(OrganizationUserStatusType.Accepted, result.Status); - Assert.Equal(orgUser.Id, result.Id); - Assert.Null(result.Email); - Assert.Equal(user.Id, result.UserId); + AssertValidAcceptedOrgUser(resultOrgUser, orgUser, user); // Verify user email verified logic Assert.True(user.EmailVerified); @@ -240,14 +236,10 @@ public async Task AcceptOrgUserByToken_NewToken_AcceptsUserAndVerifiesEmail( var newToken = CreateNewToken(orgUser); // Act - var result = await sutProvider.Sut.AcceptOrgUserByTokenAsync(orgUser.Id, user, newToken, _userService); + var resultOrgUser = await sutProvider.Sut.AcceptOrgUserByTokenAsync(orgUser.Id, user, newToken, _userService); // Assert - Assert.NotNull(result); - Assert.Equal(OrganizationUserStatusType.Accepted, result.Status); - Assert.Equal(orgUser.Id, result.Id); - Assert.Null(result.Email); - Assert.Equal(user.Id, result.UserId); + AssertValidAcceptedOrgUser(resultOrgUser, orgUser, user); // Verify user email verified logic Assert.True(user.EmailVerified); @@ -426,8 +418,88 @@ public async Task AcceptOrgUserByToken_EmailMismatch_ThrowsBadRequest( Assert.Equal("User email does not match invite.", exception.Message); } + + // AcceptOrgUserByOrgSsoIdAsync ----------------------------------------------------------------------------------- + + [Theory] + [BitAutoData] + public async Task AcceptOrgUserByOrgSsoIdAsync_ValidOrgAndUser_AcceptsOrgUser( + SutProvider sutProvider, + User user, Organization org, OrganizationUser orgUser, OrganizationUserUserDetails adminUserDetails) + { + // Arrange + SetupCommonAcceptOrgUserMocks(sutProvider, user, org, orgUser, adminUserDetails); + + sutProvider.GetDependency() + .GetByIdentifierAsync(org.Identifier) + .Returns(org); + + sutProvider.GetDependency() + .GetByOrganizationAsync(org.Id, user.Id) + .Returns(orgUser); + + // Act + var resultOrgUser = await sutProvider.Sut.AcceptOrgUserByOrgSsoIdAsync(org.Identifier, user, _userService); + + // Assert + AssertValidAcceptedOrgUser(resultOrgUser, orgUser, user); + } + + [Theory] + [BitAutoData] + public async Task AcceptOrgUserByOrgSsoIdAsync_InvalidOrg_ThrowsBadRequest(SutProvider sutProvider, + string orgSsoIdentifier, User user) + { + // Arrange + + sutProvider.GetDependency() + .GetByIdentifierAsync(orgSsoIdentifier) + .Returns((Organization)null); + + // Act & Assert + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.AcceptOrgUserByOrgSsoIdAsync(orgSsoIdentifier, user, _userService)); + + Assert.Equal("Organization invalid.", exception.Message); + } + + [Theory] + [BitAutoData] + public async Task AcceptOrgUserByOrgSsoIdAsync_UserNotInOrg_ThrowsBadRequest(SutProvider sutProvider, + Organization org, User user) + { + // Arrange + sutProvider.GetDependency() + .GetByIdentifierAsync(org.Identifier) + .Returns(org); + + sutProvider.GetDependency() + .GetByOrganizationAsync(org.Id, user.Id) + .Returns((OrganizationUser)null); + + // Act & Assert + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.AcceptOrgUserByOrgSsoIdAsync(org.Identifier, user, _userService)); + + Assert.Equal("User not found within organization.", exception.Message); + } + // Private helpers ------------------------------------------------------------------------------------------------- + /// + /// Asserts that the given org user is in the expected state after a successful AcceptOrgUserAsync call. + /// For use in happy path tests. + /// + private void AssertValidAcceptedOrgUser(OrganizationUser resultOrgUser, OrganizationUser expectedOrgUser, User user) + { + Assert.NotNull(resultOrgUser); + Assert.Equal(OrganizationUserStatusType.Accepted, resultOrgUser.Status); + Assert.Equal(expectedOrgUser, resultOrgUser); + Assert.Equal(expectedOrgUser.Id, resultOrgUser.Id); + Assert.Null(resultOrgUser.Email); + Assert.Equal(user.Id, resultOrgUser.UserId); + } + private void SetupCommonAcceptOrgUserByTokenMocks(SutProvider sutProvider, User user, OrganizationUser orgUser) { sutProvider.GetDependency().OrganizationInviteExpirationHours.Returns(24); From 847d6ae8fe73a85ca830850b9b29584e51e755d4 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Wed, 20 Sep 2023 17:05:21 -0400 Subject: [PATCH 41/80] PM-3275 - Finish up testing AcceptOrgUserCommandTests.cs by adding tests for AcceptOrgUserByOrgIdAsync --- .../AcceptOrgUserCommandTests.cs | 67 ++++++++++++++++++- 1 file changed, 66 insertions(+), 1 deletion(-) diff --git a/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs b/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs index 431a160e49aa..eac14bb8c197 100644 --- a/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs +++ b/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs @@ -423,7 +423,7 @@ public async Task AcceptOrgUserByToken_EmailMismatch_ThrowsBadRequest( [Theory] [BitAutoData] - public async Task AcceptOrgUserByOrgSsoIdAsync_ValidOrgAndUser_AcceptsOrgUser( + public async Task AcceptOrgUserByOrgSsoIdAsync_ValidData_AcceptsOrgUser( SutProvider sutProvider, User user, Organization org, OrganizationUser orgUser, OrganizationUserUserDetails adminUserDetails) { @@ -484,6 +484,71 @@ public async Task AcceptOrgUserByOrgSsoIdAsync_UserNotInOrg_ThrowsBadRequest(Sut Assert.Equal("User not found within organization.", exception.Message); } + // AcceptOrgUserByOrgIdAsync --------------------------------------------------------------------------------------- + + [Theory] + [BitAutoData] + public async Task AcceptOrgUserByOrgId_ValidData_AcceptsOrgUser( + SutProvider sutProvider, + User user, Organization org, OrganizationUser orgUser, OrganizationUserUserDetails adminUserDetails) + { + // Arrange + SetupCommonAcceptOrgUserMocks(sutProvider, user, org, orgUser, adminUserDetails); + + sutProvider.GetDependency() + .GetByIdAsync(org.Id) + .Returns(org); + + sutProvider.GetDependency() + .GetByOrganizationAsync(org.Id, user.Id) + .Returns(orgUser); + + // Act + var resultOrgUser = await sutProvider.Sut.AcceptOrgUserByOrgIdAsync(org.Id, user, _userService); + + // Assert + AssertValidAcceptedOrgUser(resultOrgUser, orgUser, user); + } + + [Theory] + [BitAutoData] + public async Task AcceptOrgUserByOrgId_InvalidOrg_ThrowsBadRequest(SutProvider sutProvider, + Guid orgId, User user) + { + // Arrange + + sutProvider.GetDependency() + .GetByIdAsync(orgId) + .Returns((Organization)null); + + // Act & Assert + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.AcceptOrgUserByOrgIdAsync(orgId, user, _userService)); + + Assert.Equal("Organization invalid.", exception.Message); + } + + [Theory] + [BitAutoData] + public async Task AcceptOrgUserByOrgId_UserNotInOrg_ThrowsBadRequest(SutProvider sutProvider, + Organization org, User user) + { + // Arrange + sutProvider.GetDependency() + .GetByIdAsync(org.Id) + .Returns(org); + + sutProvider.GetDependency() + .GetByOrganizationAsync(org.Id, user.Id) + .Returns((OrganizationUser)null); + + // Act & Assert + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.AcceptOrgUserByOrgIdAsync(org.Id, user, _userService)); + + Assert.Equal("User not found within organization.", exception.Message); + } + // Private helpers ------------------------------------------------------------------------------------------------- /// From 359943824e0c2c27c88fc74e10d71c061f040a0f Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Wed, 20 Sep 2023 17:09:22 -0400 Subject: [PATCH 42/80] PM-3275 - Tweaking test naming to ensure consistency. --- .../OrganizationUsers/AcceptOrgUserCommandTests.cs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs b/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs index eac14bb8c197..ea2e4b7fecb3 100644 --- a/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs +++ b/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs @@ -32,7 +32,7 @@ public class AcceptOrgUserCommandTests [Theory] [BitAutoData] - public async Task AcceptOrgUser_InvitedUserToSingleOrg_Success( + public async Task AcceptOrgUser_InvitedUserToSingleOrg_AcceptsOrgUser( SutProvider sutProvider, User user, Organization org, OrganizationUser orgUser, OrganizationUserUserDetails adminUserDetails) { @@ -40,15 +40,11 @@ public async Task AcceptOrgUser_InvitedUserToSingleOrg_Success( SetupCommonAcceptOrgUserMocks(sutProvider, user, org, orgUser, adminUserDetails); // Act - var orgUserResult = await sutProvider.Sut.AcceptOrgUserAsync(orgUser, user, _userService); + var resultOrgUser = await sutProvider.Sut.AcceptOrgUserAsync(orgUser, user, _userService); // Assert - // Verify returned org user details - Assert.NotNull(orgUserResult); - Assert.Equal(OrganizationUserStatusType.Accepted, orgUserResult.Status); - Assert.Equal(orgUser.Id, orgUserResult.Id); - Assert.Null(orgUserResult.Email); + AssertValidAcceptedOrgUser(resultOrgUser, orgUser, user); // Verify org repository called with updated orgUser await sutProvider.GetDependency().Received(1).ReplaceAsync( @@ -563,6 +559,8 @@ private void AssertValidAcceptedOrgUser(OrganizationUser resultOrgUser, Organiza Assert.Equal(expectedOrgUser.Id, resultOrgUser.Id); Assert.Null(resultOrgUser.Email); Assert.Equal(user.Id, resultOrgUser.UserId); + + } private void SetupCommonAcceptOrgUserByTokenMocks(SutProvider sutProvider, User user, OrganizationUser orgUser) From ca95b252ed60da689f8ce145fef66589177e0d71 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Wed, 20 Sep 2023 17:17:57 -0400 Subject: [PATCH 43/80] PM-3275 - Bugfix - OrgUserInviteTokenableFactory implementation required when declaring singleton service in ServiceCollectionExtensions.cs --- src/SharedWeb/Utilities/ServiceCollectionExtensions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs index d3c3cd9f742d..c0948ef9adfa 100644 --- a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs +++ b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs @@ -173,7 +173,7 @@ public static void AddTokenizers(this IServiceCollection services) serviceProvider.GetDataProtectionProvider(), serviceProvider.GetRequiredService>>())); - services.AddSingleton(); + services.AddSingleton(); services.AddSingleton>(serviceProvider => new DataProtectorTokenFactory( OrgUserInviteTokenable.ClearTextPrefix, From e689788daefa7169904f49696a3e23c34c681241 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Wed, 20 Sep 2023 17:50:11 -0400 Subject: [PATCH 44/80] PM-3275 - Resolve failing OrganizationServiceTests.cs --- .../AcceptOrgUserCommandTests.cs | 1 - .../Services/OrganizationServiceTests.cs | 75 +++++++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-) diff --git a/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs b/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs index ea2e4b7fecb3..873973308745 100644 --- a/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs +++ b/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs @@ -25,7 +25,6 @@ public class AcceptOrgUserCommandTests { private readonly IUserService _userService = Substitute.For(); private readonly IOrgUserInviteTokenableFactory _orgUserInviteTokenableFactory = Substitute.For(); - private readonly IDataProtectorTokenFactory _orgUserInviteTokenDataFactory = new FakeDataProtectorTokenFactory(); // Base AcceptOrgUserAsync method tests ---------------------------------------------------------------------------- diff --git a/test/Core.Test/Services/OrganizationServiceTests.cs b/test/Core.Test/Services/OrganizationServiceTests.cs index 6b4ed692d7c0..cb059abb9a32 100644 --- a/test/Core.Test/Services/OrganizationServiceTests.cs +++ b/test/Core.Test/Services/OrganizationServiceTests.cs @@ -2,6 +2,7 @@ using Bit.Core.Auth.Entities; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Models.Business; +using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Auth.Models.Data; using Bit.Core.Auth.Repositories; using Bit.Core.Context; @@ -22,12 +23,14 @@ using Bit.Core.Test.AutoFixture.OrganizationFixtures; using Bit.Core.Test.AutoFixture.OrganizationUserFixtures; using Bit.Core.Test.AutoFixture.PolicyFixtures; +using Bit.Core.Tokens; using Bit.Core.Tools.Enums; using Bit.Core.Tools.Models.Business; using Bit.Core.Tools.Services; using Bit.Core.Utilities; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; +using Bit.Test.Common.Fakes; using NSubstitute; using NSubstitute.ExceptionExtensions; using Xunit; @@ -40,10 +43,16 @@ namespace Bit.Core.Test.Services; [SutProviderCustomize] public class OrganizationServiceTests { + private readonly IDataProtectorTokenFactory _orgUserInviteTokenDataFactory = new FakeDataProtectorTokenFactory(); + [Theory, PaidOrganizationCustomize, BitAutoData] public async Task OrgImportCreateNewUsers(SutProvider sutProvider, Guid userId, Organization org, List existingUsers, List newUsers) { + // Setup FakeDataProtectorTokenFactory for creating new tokens - this must come first in order to avoid resetting mocks + sutProvider.SetDependency(_orgUserInviteTokenDataFactory, "orgUserInviteTokenDataFactory"); + sutProvider.Create(); + org.UseDirectory = true; org.Seats = 10; newUsers.Add(new ImportedOrganizationUser @@ -64,6 +73,16 @@ public async Task OrgImportCreateNewUsers(SutProvider sutPr .Returns(existingUsers.Select(u => new OrganizationUser { Status = OrganizationUserStatusType.Confirmed, Type = OrganizationUserType.Owner, Id = u.Id }).ToList()); sutProvider.GetDependency().ManageUsers(org.Id).Returns(true); + // Mock tokenable factory to return a token that expires in 5 days + sutProvider.GetDependency() + .CreateToken(Arg.Any()) + .Returns( + info => new OrgUserInviteTokenable(info.Arg()) + { + ExpirationDate = DateTime.UtcNow.Add(TimeSpan.FromDays(5)) + } + ); + await sutProvider.Sut.ImportAsync(org.Id, userId, null, newUsers, null, false); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() @@ -95,6 +114,10 @@ public async Task OrgImportCreateNewUsersAndMarryExistingUser(SutProvider existingUsers, List newUsers) { + // Setup FakeDataProtectorTokenFactory for creating new tokens - this must come first in order to avoid resetting mocks + sutProvider.SetDependency(_orgUserInviteTokenDataFactory, "orgUserInviteTokenDataFactory"); + sutProvider.Create(); + org.UseDirectory = true; org.Seats = newUsers.Count + existingUsers.Count + 1; var reInvitedUser = existingUsers.First(); @@ -118,6 +141,16 @@ public async Task OrgImportCreateNewUsersAndMarryExistingUser(SutProvider(); currentContext.ManageUsers(org.Id).Returns(true); + // Mock tokenable factory to return a token that expires in 5 days + sutProvider.GetDependency() + .CreateToken(Arg.Any()) + .Returns( + info => new OrgUserInviteTokenable(info.Arg()) + { + ExpirationDate = DateTime.UtcNow.Add(TimeSpan.FromDays(5)) + } + ); + await sutProvider.Sut.ImportAsync(org.Id, userId, null, newUsers, null, false); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() @@ -334,6 +367,10 @@ public async Task InviteUser_DuplicateEmails_PassesWithoutDuplicates(Organizatio [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, OrganizationUserInvite invite, SutProvider sutProvider) { + // Setup FakeDataProtectorTokenFactory for creating new tokens - this must come first in order to avoid resetting mocks + sutProvider.SetDependency(_orgUserInviteTokenDataFactory, "orgUserInviteTokenDataFactory"); + sutProvider.Create(); + invite.Emails = invite.Emails.Append(invite.Emails.First()); sutProvider.GetDependency().GetByIdAsync(organization.Id).Returns(organization); @@ -343,6 +380,16 @@ public async Task InviteUser_DuplicateEmails_PassesWithoutDuplicates(Organizatio organizationUserRepository.GetManyByOrganizationAsync(organization.Id, OrganizationUserType.Owner) .Returns(new[] { owner }); + // Mock tokenable factory to return a token that expires in 5 days + sutProvider.GetDependency() + .CreateToken(Arg.Any()) + .Returns( + info => new OrgUserInviteTokenable(info.Arg()) + { + ExpirationDate = DateTime.UtcNow.Add(TimeSpan.FromDays(5)) + } + ); + await sutProvider.Sut.InviteUsersAsync(organization.Id, invitor.UserId, new (OrganizationUserInvite, string)[] { (invite, null) }); await sutProvider.GetDependency().Received(1) @@ -571,6 +618,10 @@ public async Task InviteUser_Passes(Organization organization, IEnumerable<(Orga [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, SutProvider sutProvider) { + // Setup FakeDataProtectorTokenFactory for creating new tokens - this must come first in order to avoid resetting mocks + sutProvider.SetDependency(_orgUserInviteTokenDataFactory, "orgUserInviteTokenDataFactory"); + sutProvider.Create(); + invitor.Permissions = JsonSerializer.Serialize(new Permissions() { ManageUsers = true }, new JsonSerializerOptions { @@ -599,6 +650,16 @@ public async Task InviteUser_Passes(Organization organization, IEnumerable<(Orga currentContext.EditAssignedCollections(organization.Id).Returns(true); currentContext.ManageResetPassword(organization.Id).Returns(true); + // Mock tokenable factory to return a token that expires in 5 days + sutProvider.GetDependency() + .CreateToken(Arg.Any()) + .Returns( + info => new OrgUserInviteTokenable(info.Arg()) + { + ExpirationDate = DateTime.UtcNow.Add(TimeSpan.FromDays(5)) + } + ); + await sutProvider.Sut.InviteUsersAsync(organization.Id, invitor.UserId, invites); await sutProvider.GetDependency().Received(1) @@ -618,6 +679,10 @@ public async Task InviteUser_WithEventSystemUser_Passes(Organization organizatio [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, SutProvider sutProvider) { + // Setup FakeDataProtectorTokenFactory for creating new tokens - this must come first in order to avoid resetting mocks + sutProvider.SetDependency(_orgUserInviteTokenDataFactory, "orgUserInviteTokenDataFactory"); + sutProvider.Create(); + invitor.Permissions = JsonSerializer.Serialize(new Permissions() { ManageUsers = true }, new JsonSerializerOptions { @@ -633,6 +698,16 @@ public async Task InviteUser_WithEventSystemUser_Passes(Organization organizatio .Returns(new[] { owner }); currentContext.ManageUsers(organization.Id).Returns(true); + // Mock tokenable factory to return a token that expires in 5 days + sutProvider.GetDependency() + .CreateToken(Arg.Any()) + .Returns( + info => new OrgUserInviteTokenable(info.Arg()) + { + ExpirationDate = DateTime.UtcNow.Add(TimeSpan.FromDays(5)) + } + ); + await sutProvider.Sut.InviteUsersAsync(organization.Id, eventSystemUser, invites); await sutProvider.GetDependency().Received(1) From 7c9c1babc1b818aaefd9243e9ac6072e9ee4767a Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Wed, 20 Sep 2023 17:58:31 -0400 Subject: [PATCH 45/80] dotnet format --- test/Core.Test/Services/OrganizationServiceTests.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/Core.Test/Services/OrganizationServiceTests.cs b/test/Core.Test/Services/OrganizationServiceTests.cs index cb059abb9a32..f82fdddf4d3e 100644 --- a/test/Core.Test/Services/OrganizationServiceTests.cs +++ b/test/Core.Test/Services/OrganizationServiceTests.cs @@ -385,9 +385,9 @@ public async Task InviteUser_DuplicateEmails_PassesWithoutDuplicates(Organizatio .CreateToken(Arg.Any()) .Returns( info => new OrgUserInviteTokenable(info.Arg()) - { - ExpirationDate = DateTime.UtcNow.Add(TimeSpan.FromDays(5)) - } + { + ExpirationDate = DateTime.UtcNow.Add(TimeSpan.FromDays(5)) + } ); await sutProvider.Sut.InviteUsersAsync(organization.Id, invitor.UserId, new (OrganizationUserInvite, string)[] { (invite, null) }); From 675079a4f30c99e9fc3d81d3ebe212b355c244bd Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Thu, 21 Sep 2023 14:38:29 -0400 Subject: [PATCH 46/80] PM-3275 - PoliciesController.cs - GetMasterPasswordPolicy bugfix - for orgs without a MP policy, policy comes back as null and we should return notFound in that case. --- src/Api/Controllers/PoliciesController.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Api/Controllers/PoliciesController.cs b/src/Api/Controllers/PoliciesController.cs index 93f1c0c6d2be..be3492d1205a 100644 --- a/src/Api/Controllers/PoliciesController.cs +++ b/src/Api/Controllers/PoliciesController.cs @@ -137,8 +137,8 @@ public async Task GetMasterPasswordPolicy(Guid orgId) { var userId = _userService.GetProperUserId(User).Value; - var orgUsersByUserId = await _organizationUserRepository.GetManyByUserAsync(userId); - var orgUser = orgUsersByUserId.SingleOrDefault(u => u.OrganizationId == orgId); + var orgUser = await _organizationUserRepository.GetByOrganizationAsync(orgId, userId); + if (orgUser == null) { throw new NotFoundException(); @@ -146,7 +146,7 @@ public async Task GetMasterPasswordPolicy(Guid orgId) var policy = await _policyRepository.GetByOrganizationIdTypeAsync(orgId, PolicyType.MasterPassword); - if (!policy.Enabled) + if (policy == null || !policy.Enabled) { throw new NotFoundException(); } From 3954b65dc85a1a603751033740fafe45b119b2d8 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Thu, 21 Sep 2023 15:23:29 -0400 Subject: [PATCH 47/80] PM-3275 - Add PoliciesControllerTests.cs specifically for new GetMasterPasswordPolicy(...) endpoint. --- .../Controllers/PoliciesControllerTests.cs | 134 ++++++++++++++++++ 1 file changed, 134 insertions(+) create mode 100644 test/Api.Test/Controllers/PoliciesControllerTests.cs diff --git a/test/Api.Test/Controllers/PoliciesControllerTests.cs b/test/Api.Test/Controllers/PoliciesControllerTests.cs new file mode 100644 index 000000000000..9dccabefe4e3 --- /dev/null +++ b/test/Api.Test/Controllers/PoliciesControllerTests.cs @@ -0,0 +1,134 @@ +using System.Security.Claims; +using System.Text.Json; +using Bit.Api.Controllers; +using Bit.Api.Models.Public.Request; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Exceptions; +using Bit.Core.Models.Data.Organizations.Policies; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using NSubstitute; +using Xunit; + +namespace Bit.Api.Test.Controllers; + + +// Note: test names follow MethodName_StateUnderTest_ExpectedBehavior pattern. +[ControllerCustomize(typeof(PoliciesController))] +[SutProviderCustomize] +public class PoliciesControllerTests +{ + + [Theory] + [BitAutoData] + public async Task GetMasterPasswordPolicy_WhenCalled_ReturnsMasterPasswordPolicy( + SutProvider sutProvider, Guid orgId, Guid userId, OrganizationUser orgUser, + Policy policy, MasterPasswordPolicyData mpPolicyData) + { + // Arrange + sutProvider.GetDependency() + .GetProperUserId(Arg.Any()) + .Returns((Guid?)userId); + + sutProvider.GetDependency() + .GetByOrganizationAsync(orgId, userId) + .Returns(orgUser); + + + policy.Type = PolicyType.MasterPassword; + policy.Enabled = true; + // data should be a JSON serialized version of the mpPolicyData object + policy.Data = JsonSerializer.Serialize(mpPolicyData); + + sutProvider.GetDependency() + .GetByOrganizationIdTypeAsync(orgId, PolicyType.MasterPassword) + .Returns(policy); + + // Act + var result = await sutProvider.Sut.GetMasterPasswordPolicy(orgId); + + // Assert + + Assert.NotNull(result); + Assert.Equal(policy.Id, result.Id); + Assert.Equal(policy.Type, result.Type); + Assert.Equal(policy.Enabled, result.Enabled); + + // Assert that the data is deserialized correctly into a Dictionary + // for all MasterPasswordPolicyData properties + Assert.Equal(mpPolicyData.MinComplexity, ((JsonElement)result.Data["MinComplexity"]).GetInt32()); + Assert.Equal(mpPolicyData.MinLength, ((JsonElement)result.Data["MinLength"]).GetInt32()); + Assert.Equal(mpPolicyData.RequireLower, ((JsonElement)result.Data["RequireLower"]).GetBoolean()); + Assert.Equal(mpPolicyData.RequireUpper, ((JsonElement)result.Data["RequireUpper"]).GetBoolean()); + Assert.Equal(mpPolicyData.RequireNumbers, ((JsonElement)result.Data["RequireNumbers"]).GetBoolean()); + Assert.Equal(mpPolicyData.RequireSpecial, ((JsonElement)result.Data["RequireSpecial"]).GetBoolean()); + Assert.Equal(mpPolicyData.EnforceOnLogin, ((JsonElement)result.Data["EnforceOnLogin"]).GetBoolean()); + } + + + [Theory] + [BitAutoData] + public async Task GetMasterPasswordPolicy_OrgUserIsNull_ThrowsNotFoundException( + SutProvider sutProvider, Guid orgId, Guid userId) + { + // Arrange + sutProvider.GetDependency() + .GetProperUserId(Arg.Any()) + .Returns((Guid?)userId); + + sutProvider.GetDependency() + .GetByOrganizationAsync(orgId, userId) + .Returns((OrganizationUser)null); + + // Act & Assert + await Assert.ThrowsAsync(() => sutProvider.Sut.GetMasterPasswordPolicy(orgId)); + } + + [Theory] + [BitAutoData] + public async Task GetMasterPasswordPolicy_PolicyIsNull_ThrowsNotFoundException( + SutProvider sutProvider, Guid orgId, Guid userId, OrganizationUser orgUser) + { + // Arrange + sutProvider.GetDependency() + .GetProperUserId(Arg.Any()) + .Returns((Guid?)userId); + + sutProvider.GetDependency() + .GetByOrganizationAsync(orgId, userId) + .Returns(orgUser); + + sutProvider.GetDependency() + .GetByOrganizationIdTypeAsync(orgId, PolicyType.MasterPassword) + .Returns((Policy)null); + + // Act & Assert + await Assert.ThrowsAsync(() => sutProvider.Sut.GetMasterPasswordPolicy(orgId)); + } + + [Theory] + [BitAutoData] + public async Task GetMasterPasswordPolicy_PolicyNotEnabled_ThrowsNotFoundException( + SutProvider sutProvider, Guid orgId, Guid userId, OrganizationUser orgUser, Policy policy) + { + // Arrange + sutProvider.GetDependency() + .GetProperUserId(Arg.Any()) + .Returns((Guid?)userId); + + sutProvider.GetDependency() + .GetByOrganizationAsync(orgId, userId) + .Returns(orgUser); + + policy.Enabled = false; // Ensuring the policy is not enabled + sutProvider.GetDependency() + .GetByOrganizationIdTypeAsync(orgId, PolicyType.MasterPassword) + .Returns(policy); + + // Act & Assert + await Assert.ThrowsAsync(() => sutProvider.Sut.GetMasterPasswordPolicy(orgId)); + } +} From 151f4a2428ee6837d142433f8f11f2225ec20aaa Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Thu, 21 Sep 2023 16:48:57 -0400 Subject: [PATCH 48/80] PM-3275 - dotnet format PoliciesControllerTests.cs --- test/Api.Test/Controllers/PoliciesControllerTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/test/Api.Test/Controllers/PoliciesControllerTests.cs b/test/Api.Test/Controllers/PoliciesControllerTests.cs index 9dccabefe4e3..7f410dc11def 100644 --- a/test/Api.Test/Controllers/PoliciesControllerTests.cs +++ b/test/Api.Test/Controllers/PoliciesControllerTests.cs @@ -1,7 +1,6 @@ using System.Security.Claims; using System.Text.Json; using Bit.Api.Controllers; -using Bit.Api.Models.Public.Request; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; From db44b97532462d3693e3d3592c0aca4a37ece2a5 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Mon, 25 Sep 2023 14:57:42 -0400 Subject: [PATCH 49/80] PM-3275 - PoliciesController.cs - (1) Add tech debt task number (2) Properly flag endpoint as deprecated --- src/Api/Controllers/PoliciesController.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Api/Controllers/PoliciesController.cs b/src/Api/Controllers/PoliciesController.cs index be3492d1205a..c29e1b9ca904 100644 --- a/src/Api/Controllers/PoliciesController.cs +++ b/src/Api/Controllers/PoliciesController.cs @@ -104,9 +104,8 @@ public async Task> GetByToken(string orgI return new ListResponseModel(responses); } - // TODO: remove GetByInvitedUser once all clients are updated to use the GetMasterPasswordPolicy endpoint below - // TODO: create PM-??? tech debt item to track this work. In theory, target release for removal should be 2024.01.0 - // TODO: figure out how to mark this as deprecated. [Obsolete("Deprecated API", false)]? + // TODO: PM-4097 - remove GetByInvitedUser once all clients are updated to use the GetMasterPasswordPolicy endpoint below + [Obsolete("Deprecated API", false)] [AllowAnonymous] [HttpGet("invited-user")] public async Task> GetByInvitedUser(Guid orgId, [FromQuery] string userId) From 6f6d14cbcfa637532ec954baaeef31b8364e11ff Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Mon, 25 Sep 2023 15:12:51 -0400 Subject: [PATCH 50/80] PM-3275 - Add new hasManageResetPasswordPermission property to ProfileResponseModel.cs primarily for sync so that we can condition client side if TDE user obtains elevated permissions --- src/Api/Controllers/AccountsController.cs | 13 +++++++++-- .../Models/Response/ProfileResponseModel.cs | 4 +++- src/Api/Vault/Controllers/SyncController.cs | 12 ++++++++-- .../Models/Response/SyncResponseModel.cs | 5 ++-- src/Core/Context/CurrentContextExtensions.cs | 23 +++++++++++++++++++ 5 files changed, 50 insertions(+), 7 deletions(-) create mode 100644 src/Core/Context/CurrentContextExtensions.cs diff --git a/src/Api/Controllers/AccountsController.cs b/src/Api/Controllers/AccountsController.cs index d668ebefb5e7..2c088820c37e 100644 --- a/src/Api/Controllers/AccountsController.cs +++ b/src/Api/Controllers/AccountsController.cs @@ -9,6 +9,7 @@ using Bit.Core.Auth.Services; using Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; using Bit.Core.Auth.Utilities; +using Bit.Core.Context; using Bit.Core.Enums; using Bit.Core.Enums.Provider; using Bit.Core.Exceptions; @@ -48,6 +49,8 @@ public class AccountsController : Controller private readonly IPolicyService _policyService; private readonly ISetInitialMasterPasswordCommand _setInitialMasterPasswordCommand; + protected ICurrentContext CurrentContext { get; } + public AccountsController( GlobalSettings globalSettings, ICipherRepository cipherRepository, @@ -62,7 +65,8 @@ public AccountsController( ISendService sendService, ICaptchaValidationService captchaValidationService, IPolicyService policyService, - ISetInitialMasterPasswordCommand setInitialMasterPasswordCommand + ISetInitialMasterPasswordCommand setInitialMasterPasswordCommand, + ICurrentContext currentContext ) { _cipherRepository = cipherRepository; @@ -79,6 +83,7 @@ ISetInitialMasterPasswordCommand setInitialMasterPasswordCommand _captchaValidationService = captchaValidationService; _policyService = policyService; _setInitialMasterPasswordCommand = setInitialMasterPasswordCommand; + CurrentContext = currentContext; } #region DEPRECATED (Moved to Identity Service) @@ -459,8 +464,12 @@ public async Task GetProfile() var providerUserOrganizationDetails = await _providerUserRepository.GetManyOrganizationDetailsByUserAsync(user.Id, ProviderUserStatusType.Confirmed); + + var hasManageResetPasswordPermission = await CurrentContext.AnyOrgUserHasManageResetPasswordPermission(organizationUserDetails); + var response = new ProfileResponseModel(user, organizationUserDetails, providerUserDetails, - providerUserOrganizationDetails, await _userService.TwoFactorIsEnabledAsync(user), await _userService.HasPremiumFromOrganization(user)); + providerUserOrganizationDetails, await _userService.TwoFactorIsEnabledAsync(user), + await _userService.HasPremiumFromOrganization(user), hasManageResetPasswordPermission); return response; } diff --git a/src/Api/Models/Response/ProfileResponseModel.cs b/src/Api/Models/Response/ProfileResponseModel.cs index 1fbf002e2761..009816166995 100644 --- a/src/Api/Models/Response/ProfileResponseModel.cs +++ b/src/Api/Models/Response/ProfileResponseModel.cs @@ -13,7 +13,7 @@ public ProfileResponseModel(User user, IEnumerable providerUserDetails, IEnumerable providerUserOrganizationDetails, bool twoFactorEnabled, - bool premiumFromOrganization) : base("profile") + bool premiumFromOrganization, bool? hasManageResetPasswordPermission = null) : base("profile") { if (user == null) { @@ -33,6 +33,7 @@ public ProfileResponseModel(User user, PrivateKey = user.PrivateKey; SecurityStamp = user.SecurityStamp; ForcePasswordReset = user.ForcePasswordReset; + HasManageResetPasswordPermission = hasManageResetPasswordPermission; UsesKeyConnector = user.UsesKeyConnector; AvatarColor = user.AvatarColor; Organizations = organizationsUserDetails?.Select(o => new ProfileOrganizationResponseModel(o)); @@ -58,6 +59,7 @@ public ProfileResponseModel() : base("profile") public string PrivateKey { get; set; } public string SecurityStamp { get; set; } public bool ForcePasswordReset { get; set; } + public bool? HasManageResetPasswordPermission { get; set; } public bool UsesKeyConnector { get; set; } public string AvatarColor { get; set; } public IEnumerable Organizations { get; set; } diff --git a/src/Api/Vault/Controllers/SyncController.cs b/src/Api/Vault/Controllers/SyncController.cs index ec9ab4f96dd8..75dc0e5f6915 100644 --- a/src/Api/Vault/Controllers/SyncController.cs +++ b/src/Api/Vault/Controllers/SyncController.cs @@ -1,4 +1,5 @@ using Bit.Api.Vault.Models.Response; +using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Enums.Provider; @@ -29,6 +30,8 @@ public class SyncController : Controller private readonly ISendRepository _sendRepository; private readonly GlobalSettings _globalSettings; + protected ICurrentContext CurrentContext { get; } + public SyncController( IUserService userService, IFolderRepository folderRepository, @@ -39,7 +42,8 @@ public SyncController( IProviderUserRepository providerUserRepository, IPolicyRepository policyRepository, ISendRepository sendRepository, - GlobalSettings globalSettings) + GlobalSettings globalSettings, + ICurrentContext currentContext) { _userService = userService; _folderRepository = folderRepository; @@ -51,6 +55,7 @@ public SyncController( _policyRepository = policyRepository; _sendRepository = sendRepository; _globalSettings = globalSettings; + CurrentContext = currentContext; } [HttpGet("")] @@ -70,6 +75,9 @@ public async Task Get([FromQuery] bool excludeDomains = false await _providerUserRepository.GetManyOrganizationDetailsByUserAsync(user.Id, ProviderUserStatusType.Confirmed); var hasEnabledOrgs = organizationUserDetails.Any(o => o.Enabled); + + var hasManageResetPasswordPermission = await CurrentContext.AnyOrgUserHasManageResetPasswordPermission(organizationUserDetails); + var folders = await _folderRepository.GetManyByUserIdAsync(user.Id); var ciphers = await _cipherRepository.GetManyByUserIdAsync(user.Id, hasEnabledOrgs); var sends = await _sendRepository.GetManyByUserIdAsync(user.Id); @@ -89,7 +97,7 @@ await _providerUserRepository.GetManyOrganizationDetailsByUserAsync(user.Id, var userHasPremiumFromOrganization = await _userService.HasPremiumFromOrganization(user); var response = new SyncResponseModel(_globalSettings, user, userTwoFactorEnabled, userHasPremiumFromOrganization, organizationUserDetails, providerUserDetails, providerUserOrganizationDetails, folders, collections, ciphers, - collectionCiphersGroupDict, excludeDomains, policies, sends); + collectionCiphersGroupDict, excludeDomains, policies, sends, hasManageResetPasswordPermission); return response; } } diff --git a/src/Api/Vault/Models/Response/SyncResponseModel.cs b/src/Api/Vault/Models/Response/SyncResponseModel.cs index 386b7445be74..fb80565cb3c8 100644 --- a/src/Api/Vault/Models/Response/SyncResponseModel.cs +++ b/src/Api/Vault/Models/Response/SyncResponseModel.cs @@ -28,11 +28,12 @@ public SyncResponseModel( IDictionary> collectionCiphersDict, bool excludeDomains, IEnumerable policies, - IEnumerable sends) + IEnumerable sends, + bool hasManageResetPasswordPermission) : base("sync") { Profile = new ProfileResponseModel(user, organizationUserDetails, providerUserDetails, - providerUserOrganizationDetails, userTwoFactorEnabled, userHasPremiumFromOrganization); + providerUserOrganizationDetails, userTwoFactorEnabled, userHasPremiumFromOrganization, hasManageResetPasswordPermission); Folders = folders.Select(f => new FolderResponseModel(f)); Ciphers = ciphers.Select(c => new CipherDetailsResponseModel(c, globalSettings, collectionCiphersDict)); Collections = collections?.Select( diff --git a/src/Core/Context/CurrentContextExtensions.cs b/src/Core/Context/CurrentContextExtensions.cs new file mode 100644 index 000000000000..750ddb404d0d --- /dev/null +++ b/src/Core/Context/CurrentContextExtensions.cs @@ -0,0 +1,23 @@ +using Bit.Core.Models.Data.Organizations.OrganizationUsers; + +namespace Bit.Core.Context; + +public static class CurrentContextExtensions +{ + public static async Task AnyOrgUserHasManageResetPasswordPermission(this ICurrentContext currentContext, + ICollection organizationUserDetails + ) + { + var hasManageResetPasswordPermission = false; + // if user has a single org with manage reset password permission, then they have it + foreach (var orgUserDetail in organizationUserDetails) + { + if (await currentContext.ManageResetPassword(orgUserDetail.OrganizationId)) + { + hasManageResetPasswordPermission = true; + break; + } + } + return hasManageResetPasswordPermission; + } +} From 7ff502233b8f7f458379c0bdc70a7130bd32d34b Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Tue, 26 Sep 2023 14:28:06 -0400 Subject: [PATCH 51/80] PM-3275 - Fix AccountsControllerTests.cs --- test/Api.Test/Controllers/AccountsControllerTests.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/Api.Test/Controllers/AccountsControllerTests.cs b/test/Api.Test/Controllers/AccountsControllerTests.cs index a0acc2e8a0d4..8973be15e557 100644 --- a/test/Api.Test/Controllers/AccountsControllerTests.cs +++ b/test/Api.Test/Controllers/AccountsControllerTests.cs @@ -4,6 +4,7 @@ using Bit.Core.Auth.Models.Api.Request.Accounts; using Bit.Core.Auth.Services; using Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; +using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; @@ -38,6 +39,7 @@ public class AccountsControllerTests : IDisposable private readonly ICaptchaValidationService _captchaValidationService; private readonly IPolicyService _policyService; private readonly ISetInitialMasterPasswordCommand _setInitialMasterPasswordCommand; + private readonly ICurrentContext _currentContext; public AccountsControllerTests() { @@ -55,6 +57,7 @@ public AccountsControllerTests() _captchaValidationService = Substitute.For(); _policyService = Substitute.For(); _setInitialMasterPasswordCommand = Substitute.For(); + _currentContext = Substitute.For(); _sut = new AccountsController( _globalSettings, @@ -70,7 +73,8 @@ public AccountsControllerTests() _sendService, _captchaValidationService, _policyService, - _setInitialMasterPasswordCommand + _setInitialMasterPasswordCommand, + _currentContext ); } From cd2c0a9ea279bc4ef83ffb6cadf4a68ebc1052be Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Thu, 28 Sep 2023 16:44:25 -0400 Subject: [PATCH 52/80] PM-3275 - OrgUserInviteTokenable.cs - clarify TODO --- .../Auth/Models/Business/Tokenables/OrgUserInviteTokenable.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Core/Auth/Models/Business/Tokenables/OrgUserInviteTokenable.cs b/src/Core/Auth/Models/Business/Tokenables/OrgUserInviteTokenable.cs index 469e9c31facc..765928808c5e 100644 --- a/src/Core/Auth/Models/Business/Tokenables/OrgUserInviteTokenable.cs +++ b/src/Core/Auth/Models/Business/Tokenables/OrgUserInviteTokenable.cs @@ -7,6 +7,7 @@ namespace Bit.Core.Auth.Models.Business.Tokenables; public class OrgUserInviteTokenable : ExpiringTokenable { // TODO: Ideally this would be internal and only visible to the test project. + // but configuring that is out of scope for these changes. public static TimeSpan GetTokenLifetime() => TimeSpan.FromDays(5); public const string ClearTextPrefix = "BwOrgUserInviteToken_"; From 7cde4272013f24316dfe6cd3c042d3dee48bd566 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Fri, 29 Sep 2023 14:37:22 -0400 Subject: [PATCH 53/80] PM-3275 - AcceptOrgUserCommand.cs - Refactor token validation to use short circuiting to only run old token validation if new token validation fails. --- .../OrganizationUsers/AcceptOrgUserCommand.cs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs b/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs index a908cb85af2f..60d057832182 100644 --- a/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs +++ b/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs @@ -57,14 +57,18 @@ public async Task AcceptOrgUserByTokenAsync(Guid organizationU // Tokens will have been created in two ways in the OrganizationService invite methods: // 1. New way - via OrgUserInviteTokenable // 2. Old way - via manual process using data protector initialized with purpose: "OrganizationServiceDataProtector" - // For backwards compatibility, must check validity of both types of tokens and accept if either is valid - // TODO: remove old token validation logic after 1 release as token are only valid for 5 days - var isNewTokenInvalid = !ValidateOrgUserInviteToken(token, orgUser); - var isOldTokenInvalid = !CoreHelpers.UserInviteTokenIsValid(_dataProtector, token, user.Email, orgUser.Id, _globalSettings); + // TODO: PM-4142 - remove old token validation logic once 3 releases of backwards compatibility are complete + // TODO: update this code to only run old token validation if new token validation fails + var newTokenValid = OrgUserInviteTokenable.ValidateOrgUserInviteStringToken( + _orgUserInviteTokenDataFactory, token, orgUser); + + var tokenValid = newTokenValid || + CoreHelpers.UserInviteTokenIsValid(_dataProtector, token, user.Email, orgUser.Id, + _globalSettings); - if (isNewTokenInvalid && isOldTokenInvalid) + if (!tokenValid) { throw new BadRequestException("Invalid token."); } From 138d4a2aa0edbe76eb94d9f5cff3d20d0f6fb96e Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Fri, 29 Sep 2023 14:38:46 -0400 Subject: [PATCH 54/80] PM-3275 - OrgUserInviteTokenable.cs - (1) Add new static methods to centralize validation logic to avoid repetition (2) Add new token validation method so we can avoid having to pass in a full org user (and hitting the db to do so) --- .../Tokenables/OrgUserInviteTokenable.cs | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/Core/Auth/Models/Business/Tokenables/OrgUserInviteTokenable.cs b/src/Core/Auth/Models/Business/Tokenables/OrgUserInviteTokenable.cs index 765928808c5e..2f6e96b5fa6e 100644 --- a/src/Core/Auth/Models/Business/Tokenables/OrgUserInviteTokenable.cs +++ b/src/Core/Auth/Models/Business/Tokenables/OrgUserInviteTokenable.cs @@ -48,7 +48,37 @@ public bool TokenIsValid(OrganizationUser orgUser) OrgUserEmail.Equals(orgUser.Email, StringComparison.InvariantCultureIgnoreCase); } + public bool TokenIsValid(Guid orgUserId, string orgUserEmail) + { + if (OrgUserId == default || OrgUserEmail == default || orgUserId == default || orgUserEmail == default) + { + return false; + } + + return OrgUserId == orgUserId && + OrgUserEmail.Equals(orgUserEmail, StringComparison.InvariantCultureIgnoreCase); + } + // Validates deserialized protected override bool TokenIsValid() => Identifier == TokenIdentifier && OrgUserId != default && !string.IsNullOrWhiteSpace(OrgUserEmail); + + + public static bool ValidateOrgUserInviteStringToken( + IDataProtectorTokenFactory orgUserInviteTokenDataFactory, + string orgUserInviteToken, OrganizationUser orgUser) + { + return orgUserInviteTokenDataFactory.TryUnprotect(orgUserInviteToken, out var decryptedToken) + && decryptedToken.Valid + && decryptedToken.TokenIsValid(orgUser); + } + + public static bool ValidateOrgUserInviteStringToken( + IDataProtectorTokenFactory orgUserInviteTokenDataFactory, + string orgUserInviteToken, Guid orgUserId, string orgUserEmail) + { + return orgUserInviteTokenDataFactory.TryUnprotect(orgUserInviteToken, out var decryptedToken) + && decryptedToken.Valid + && decryptedToken.TokenIsValid(orgUserId, orgUserEmail); + } } From eff02c0a66d4e2125d3c8210089bfe693f3ac547 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Fri, 29 Sep 2023 14:44:30 -0400 Subject: [PATCH 55/80] PM-3275 - Realized that the old token validation was used in the PoliciesController.cs (existing user clicks invite link in email and goes to log in) and UserService.cs (user clicks invite link in email and registers for a new acct). Added tech debt item for cleaning up backwards compatibility in future. --- src/Api/Controllers/PoliciesController.cs | 31 +++++++++++++------ .../Services/Implementations/UserService.cs | 16 ++++++++-- src/Core/Utilities/CoreHelpers.cs | 1 + test/Core.Test/Services/UserServiceTests.cs | 5 ++- 4 files changed, 39 insertions(+), 14 deletions(-) diff --git a/src/Api/Controllers/PoliciesController.cs b/src/Api/Controllers/PoliciesController.cs index c29e1b9ca904..348c8d585cd9 100644 --- a/src/Api/Controllers/PoliciesController.cs +++ b/src/Api/Controllers/PoliciesController.cs @@ -1,12 +1,15 @@ using Bit.Api.Models.Request; using Bit.Api.Models.Response; +using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Context; +using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.Models.Api.Response; using Bit.Core.Repositories; using Bit.Core.Services; using Bit.Core.Settings; +using Bit.Core.Tokens; using Bit.Core.Utilities; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.DataProtection; @@ -26,6 +29,7 @@ public class PoliciesController : Controller private readonly ICurrentContext _currentContext; private readonly GlobalSettings _globalSettings; private readonly IDataProtector _organizationServiceDataProtector; + private readonly IDataProtectorTokenFactory _orgUserInviteTokenDataFactory; public PoliciesController( IPolicyRepository policyRepository, @@ -35,7 +39,8 @@ public PoliciesController( IUserService userService, ICurrentContext currentContext, GlobalSettings globalSettings, - IDataProtectionProvider dataProtectionProvider) + IDataProtectionProvider dataProtectionProvider, + IDataProtectorTokenFactory orgUserInviteTokenDataFactory) { _policyRepository = policyRepository; _policyService = policyService; @@ -46,6 +51,8 @@ public PoliciesController( _globalSettings = globalSettings; _organizationServiceDataProtector = dataProtectionProvider.CreateProtector( "OrganizationServiceDataProtector"); + + _orgUserInviteTokenDataFactory = orgUserInviteTokenDataFactory; } [HttpGet("{type}")] @@ -81,25 +88,29 @@ public async Task> Get(string orgId) [AllowAnonymous] [HttpGet("token")] - public async Task> GetByToken(string orgId, [FromQuery] string email, - [FromQuery] string token, [FromQuery] string organizationUserId) + public async Task> GetByToken(Guid orgId, [FromQuery] string email, + [FromQuery] string token, [FromQuery] Guid organizationUserId) { - var orgUserId = new Guid(organizationUserId); - var tokenValid = CoreHelpers.UserInviteTokenIsValid(_organizationServiceDataProtector, token, - email, orgUserId, _globalSettings); + // TODO: PM-4142 - remove old token validation logic once 3 releases of backwards compatibility are complete + var newTokenValid = OrgUserInviteTokenable.ValidateOrgUserInviteStringToken( + _orgUserInviteTokenDataFactory, token, organizationUserId, email); + + var tokenValid = newTokenValid || CoreHelpers.UserInviteTokenIsValid( + _organizationServiceDataProtector, token, email, organizationUserId, _globalSettings + ); + if (!tokenValid) { throw new NotFoundException(); } - var orgIdGuid = new Guid(orgId); - var orgUser = await _organizationUserRepository.GetByIdAsync(orgUserId); - if (orgUser == null || orgUser.OrganizationId != orgIdGuid) + var orgUser = await _organizationUserRepository.GetByIdAsync(orgId); + if (orgUser == null || orgUser.OrganizationId != orgId) { throw new NotFoundException(); } - var policies = await _policyRepository.GetManyByOrganizationIdAsync(orgIdGuid); + var policies = await _policyRepository.GetManyByOrganizationIdAsync(orgId); var responses = policies.Where(p => p.Enabled).Select(p => new PolicyResponseModel(p)); return new ListResponseModel(responses); } diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index fe33041c2242..55d357583ef5 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -2,6 +2,7 @@ using System.Text.Json; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Models; +using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Enums; @@ -10,6 +11,7 @@ using Bit.Core.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.Repositories; using Bit.Core.Settings; +using Bit.Core.Tokens; using Bit.Core.Tools.Entities; using Bit.Core.Tools.Enums; using Bit.Core.Tools.Models.Business; @@ -56,6 +58,7 @@ public class UserService : UserManager, IUserService, IDisposable private readonly IAcceptOrgUserCommand _acceptOrgUserCommand; private readonly IProviderUserRepository _providerUserRepository; private readonly IStripeSyncService _stripeSyncService; + private readonly IDataProtectorTokenFactory _orgUserInviteTokenDataFactory; public UserService( IUserRepository userRepository, @@ -86,7 +89,8 @@ public UserService( IGlobalSettings globalSettings, IAcceptOrgUserCommand acceptOrgUserCommand, IProviderUserRepository providerUserRepository, - IStripeSyncService stripeSyncService) + IStripeSyncService stripeSyncService, + IDataProtectorTokenFactory orgUserInviteTokenDataFactory) : base( store, optionsAccessor, @@ -123,6 +127,7 @@ public UserService( _acceptOrgUserCommand = acceptOrgUserCommand; _providerUserRepository = providerUserRepository; _stripeSyncService = stripeSyncService; + _orgUserInviteTokenDataFactory = orgUserInviteTokenDataFactory; } public Guid? GetProperUserId(ClaimsPrincipal principal) @@ -288,8 +293,13 @@ public async Task RegisterUserAsync(User user, string masterPass var tokenValid = false; if (_globalSettings.DisableUserRegistration && !string.IsNullOrWhiteSpace(token) && orgUserId.HasValue) { - tokenValid = CoreHelpers.UserInviteTokenIsValid(_organizationServiceDataProtector, token, - user.Email, orgUserId.Value, _globalSettings); + // TODO: PM-4142 - remove old token validation logic once 3 releases of backwards compatibility are complete + var newTokenValid = OrgUserInviteTokenable.ValidateOrgUserInviteStringToken( + _orgUserInviteTokenDataFactory, token, orgUserId.Value, user.Email); + + tokenValid = newTokenValid || + CoreHelpers.UserInviteTokenIsValid(_organizationServiceDataProtector, token, + user.Email, orgUserId.Value, _globalSettings); } if (_globalSettings.DisableUserRegistration && !tokenValid) diff --git a/src/Core/Utilities/CoreHelpers.cs b/src/Core/Utilities/CoreHelpers.cs index c128fa8e46e5..e148ec103ff1 100644 --- a/src/Core/Utilities/CoreHelpers.cs +++ b/src/Core/Utilities/CoreHelpers.cs @@ -494,6 +494,7 @@ public static string CustomProviderName(TwoFactorProviderType type) return string.Concat("Custom_", type.ToString()); } + // TODO: PM-4142 - remove old token validation logic once 3 releases of backwards compatibility are complete public static bool UserInviteTokenIsValid(IDataProtector protector, string token, string userEmail, Guid orgUserId, IGlobalSettings globalSettings) { diff --git a/test/Core.Test/Services/UserServiceTests.cs b/test/Core.Test/Services/UserServiceTests.cs index 107fd55ebafb..66e3c427b5d1 100644 --- a/test/Core.Test/Services/UserServiceTests.cs +++ b/test/Core.Test/Services/UserServiceTests.cs @@ -1,6 +1,7 @@ using System.Text.Json; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Models; +using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Models.Business; @@ -13,6 +14,7 @@ using Bit.Core.Vault.Repositories; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; +using Bit.Test.Common.Fakes; using Bit.Test.Common.Helpers; using Fido2NetLib; using Microsoft.AspNetCore.DataProtection; @@ -254,7 +256,8 @@ public async Task VerifySecretAsync_Works( sutProvider.GetDependency(), sutProvider.GetDependency(), sutProvider.GetDependency(), - sutProvider.GetDependency()); + sutProvider.GetDependency(), + new FakeDataProtectorTokenFactory()); var actualIsVerified = await sut.VerifySecretAsync(user, secret); From 49c17019e578b24d0d2d6c83c234f016a9cf12d3 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Fri, 29 Sep 2023 15:14:05 -0400 Subject: [PATCH 56/80] dotnet format --- src/Api/Controllers/PoliciesController.cs | 1 - src/Core/Services/Implementations/UserService.cs | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Api/Controllers/PoliciesController.cs b/src/Api/Controllers/PoliciesController.cs index 348c8d585cd9..2457e24adc0b 100644 --- a/src/Api/Controllers/PoliciesController.cs +++ b/src/Api/Controllers/PoliciesController.cs @@ -2,7 +2,6 @@ using Bit.Api.Models.Response; using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Context; -using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.Models.Api.Response; diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index 55d357583ef5..d894eec4081f 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -297,7 +297,7 @@ public async Task RegisterUserAsync(User user, string masterPass var newTokenValid = OrgUserInviteTokenable.ValidateOrgUserInviteStringToken( _orgUserInviteTokenDataFactory, token, orgUserId.Value, user.Email); - tokenValid = newTokenValid || + tokenValid = newTokenValid || CoreHelpers.UserInviteTokenIsValid(_organizationServiceDataProtector, token, user.Email, orgUserId.Value, _globalSettings); } From 0dcc86df02e3715e0acdf51db7587b22aa85ec83 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Tue, 3 Oct 2023 18:40:37 -0400 Subject: [PATCH 57/80] PM-3275 - (1) AccountsController.cs - Update PostSetPasswordAsync SetPasswordRequestModel to allow null keys for the case where we have a TDE user who obtains elevated permissions - they already have a user public and user encrypted private key saved in the db. (2) AccountsControllerTests.cs - test PostSetPasswordAsync scenarios to ensure changes will work as expected. --- .../Accounts/SetPasswordRequestModel.cs | 9 +- src/Api/Controllers/AccountsController.cs | 6 +- .../Controllers/AccountsControllerTests.cs | 119 ++++++++++++++++++ 3 files changed, 129 insertions(+), 5 deletions(-) diff --git a/src/Api/Auth/Models/Request/Accounts/SetPasswordRequestModel.cs b/src/Api/Auth/Models/Request/Accounts/SetPasswordRequestModel.cs index 9c078d7b289d..6339725cd929 100644 --- a/src/Api/Auth/Models/Request/Accounts/SetPasswordRequestModel.cs +++ b/src/Api/Auth/Models/Request/Accounts/SetPasswordRequestModel.cs @@ -1,4 +1,6 @@ -using System.ComponentModel.DataAnnotations; +#nullable enable + +using System.ComponentModel.DataAnnotations; using Bit.Core.Auth.Models.Api.Request.Accounts; using Bit.Core.Entities; using Bit.Core.Enums; @@ -15,8 +17,7 @@ public class SetPasswordRequestModel : IValidatableObject public string Key { get; set; } [StringLength(50)] public string MasterPasswordHint { get; set; } - [Required] - public KeysRequestModel Keys { get; set; } + public KeysRequestModel? Keys { get; set; } [Required] public KdfType Kdf { get; set; } [Required] @@ -33,7 +34,7 @@ public User ToUser(User existingUser) existingUser.KdfMemory = KdfMemory; existingUser.KdfParallelism = KdfParallelism; existingUser.Key = Key; - Keys.ToUser(existingUser); + Keys?.ToUser(existingUser); return existingUser; } diff --git a/src/Api/Controllers/AccountsController.cs b/src/Api/Controllers/AccountsController.cs index 2c088820c37e..30ec55c6fd02 100644 --- a/src/Api/Controllers/AccountsController.cs +++ b/src/Api/Controllers/AccountsController.cs @@ -261,8 +261,12 @@ public async Task PostSetPasswordAsync([FromBody] SetPasswordRequestModel model) throw new UnauthorizedAccessException(); } - var result = await _setInitialMasterPasswordCommand.SetInitialMasterPasswordAsync(model.ToUser(user), model.MasterPasswordHash, model.Key, + var result = await _setInitialMasterPasswordCommand.SetInitialMasterPasswordAsync( + model.ToUser(user), + model.MasterPasswordHash, + model.Key, model.OrgIdentifier); + if (result.Succeeded) { return; diff --git a/test/Api.Test/Controllers/AccountsControllerTests.cs b/test/Api.Test/Controllers/AccountsControllerTests.cs index 8973be15e557..e9a09da3c630 100644 --- a/test/Api.Test/Controllers/AccountsControllerTests.cs +++ b/test/Api.Test/Controllers/AccountsControllerTests.cs @@ -15,6 +15,7 @@ using Bit.Core.Tools.Repositories; using Bit.Core.Tools.Services; using Bit.Core.Vault.Repositories; +using Bit.Test.Common.AutoFixture.Attributes; using Microsoft.AspNetCore.Identity; using NSubstitute; using Xunit; @@ -389,6 +390,124 @@ await Assert.ThrowsAsync( ); } + + [Theory] + [BitAutoData(true, false)] // User has PublicKey and PrivateKey, and Keys in request are NOT null + [BitAutoData(true, true)] // User has PublicKey and PrivateKey, and Keys in request are null + [BitAutoData(false, false)] // User has neither PublicKey nor PrivateKey, and Keys in request are NOT null + [BitAutoData(false, true)] // User has neither PublicKey nor PrivateKey, and Keys in request are null + public async Task PostSetPasswordAsync_WhenUserExistsAndSettingPasswordSucceeds_ShouldHandleKeysCorrectlyAndReturn( + bool hasExistingKeys, + bool shouldSetKeysToNull, + User user, + SetPasswordRequestModel setPasswordRequestModel) + { + // Arrange + const string existingPublicKey = "existingPublicKey"; + const string existingEncryptedPrivateKey = "existingEncryptedPrivateKey"; + + const string newPublicKey = "newPublicKey"; + const string newEncryptedPrivateKey = "newEncryptedPrivateKey"; + + if (hasExistingKeys) + { + user.PublicKey = existingPublicKey; + user.PrivateKey = existingEncryptedPrivateKey; + } + else + { + user.PublicKey = null; + user.PrivateKey = null; + } + + if (shouldSetKeysToNull) + { + setPasswordRequestModel.Keys = null; + } + else + { + setPasswordRequestModel.Keys = new KeysRequestModel() + { + PublicKey = newPublicKey, + EncryptedPrivateKey = newEncryptedPrivateKey + }; + } + + _userService.GetUserByPrincipalAsync(Arg.Any()).Returns(Task.FromResult(user)); + _setInitialMasterPasswordCommand.SetInitialMasterPasswordAsync( + user, + setPasswordRequestModel.MasterPasswordHash, + setPasswordRequestModel.Key, + setPasswordRequestModel.OrgIdentifier) + .Returns(Task.FromResult(IdentityResult.Success)); + + // Act + await _sut.PostSetPasswordAsync(setPasswordRequestModel); + + // Assert + await _setInitialMasterPasswordCommand.Received(1) + .SetInitialMasterPasswordAsync( + Arg.Is(u => u == user), + Arg.Is(s => s == setPasswordRequestModel.MasterPasswordHash), + Arg.Is(s => s == setPasswordRequestModel.Key), + Arg.Is(s => s == setPasswordRequestModel.OrgIdentifier)); + + // Additional Assertions for User object modifications + Assert.Equal(setPasswordRequestModel.MasterPasswordHint, user.MasterPasswordHint); + Assert.Equal(setPasswordRequestModel.Kdf, user.Kdf); + Assert.Equal(setPasswordRequestModel.KdfIterations, user.KdfIterations); + Assert.Equal(setPasswordRequestModel.KdfMemory, user.KdfMemory); + Assert.Equal(setPasswordRequestModel.KdfParallelism, user.KdfParallelism); + Assert.Equal(setPasswordRequestModel.Key, user.Key); + + if (hasExistingKeys) + { + // User Keys should not be modified + Assert.Equal(existingPublicKey, user.PublicKey); + Assert.Equal(existingEncryptedPrivateKey, user.PrivateKey); + } + else if (!shouldSetKeysToNull) + { + // User had no keys so they should be set to the request model's keys + Assert.Equal(setPasswordRequestModel.Keys.PublicKey, user.PublicKey); + Assert.Equal(setPasswordRequestModel.Keys.EncryptedPrivateKey, user.PrivateKey); + } + else + { + // User had no keys and the request model's keys were null, so they should be set to null + Assert.Null(user.PublicKey); + Assert.Null(user.PrivateKey); + } + } + + [Theory] + [BitAutoData] + public async Task PostSetPasswordAsync_WhenUserDoesNotExist_ShouldThrowUnauthorizedAccessException( + SetPasswordRequestModel setPasswordRequestModel) + { + // Arrange + _userService.GetUserByPrincipalAsync(Arg.Any()).Returns(Task.FromResult((User)null)); + + // Act & Assert + await Assert.ThrowsAsync(() => _sut.PostSetPasswordAsync(setPasswordRequestModel)); + } + + [Theory] + [BitAutoData] + public async Task PostSetPasswordAsync_WhenSettingPasswordFails_ShouldThrowBadRequestException( + User user, + SetPasswordRequestModel model) + { + // Arrange + _userService.GetUserByPrincipalAsync(Arg.Any()).Returns(Task.FromResult(user)); + _setInitialMasterPasswordCommand.SetInitialMasterPasswordAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(IdentityResult.Failed(new IdentityError { Description = "Some Error" }))); + + // Act & Assert + await Assert.ThrowsAsync(() => _sut.PostSetPasswordAsync(model)); + } + + // Below are helper functions that currently belong to this // test class, but ultimately may need to be split out into // something greater in order to share common test steps with From 056636d2de99d3a34076a408dfcb793a1f36f24e Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Fri, 6 Oct 2023 11:25:29 -0400 Subject: [PATCH 58/80] PM-3275 - PR review feedback - (1) set CurrentContext to private (2) Refactor GetProfile to use variables to improve clarity and simplify debugging. --- src/Api/Controllers/AccountsController.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/Api/Controllers/AccountsController.cs b/src/Api/Controllers/AccountsController.cs index 30ec55c6fd02..17ab3b6baa42 100644 --- a/src/Api/Controllers/AccountsController.cs +++ b/src/Api/Controllers/AccountsController.cs @@ -49,7 +49,7 @@ public class AccountsController : Controller private readonly IPolicyService _policyService; private readonly ISetInitialMasterPasswordCommand _setInitialMasterPasswordCommand; - protected ICurrentContext CurrentContext { get; } + private ICurrentContext CurrentContext { get; } public AccountsController( GlobalSettings globalSettings, @@ -470,10 +470,12 @@ await _providerUserRepository.GetManyOrganizationDetailsByUserAsync(user.Id, ProviderUserStatusType.Confirmed); var hasManageResetPasswordPermission = await CurrentContext.AnyOrgUserHasManageResetPasswordPermission(organizationUserDetails); + var twoFactorEnabled = await _userService.TwoFactorIsEnabledAsync(user); + var hasPremiumFromOrg = await _userService.HasPremiumFromOrganization(user); var response = new ProfileResponseModel(user, organizationUserDetails, providerUserDetails, - providerUserOrganizationDetails, await _userService.TwoFactorIsEnabledAsync(user), - await _userService.HasPremiumFromOrganization(user), hasManageResetPasswordPermission); + providerUserOrganizationDetails, twoFactorEnabled, + hasPremiumFromOrg, hasManageResetPasswordPermission); return response; } From 1c099a0cd867e386ff57df0bfd20085275be66ee Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Fri, 6 Oct 2023 11:28:33 -0400 Subject: [PATCH 59/80] PM-3275 - SyncController.cs - PR Review Feedback - Set current context as private instead of protected. --- src/Api/Vault/Controllers/SyncController.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Api/Vault/Controllers/SyncController.cs b/src/Api/Vault/Controllers/SyncController.cs index 75dc0e5f6915..b71467638e4d 100644 --- a/src/Api/Vault/Controllers/SyncController.cs +++ b/src/Api/Vault/Controllers/SyncController.cs @@ -29,8 +29,7 @@ public class SyncController : Controller private readonly IPolicyRepository _policyRepository; private readonly ISendRepository _sendRepository; private readonly GlobalSettings _globalSettings; - - protected ICurrentContext CurrentContext { get; } + private ICurrentContext CurrentContext { get; } public SyncController( IUserService userService, From d83da377ce975953785c64a682eb6086b1a204fd Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Fri, 6 Oct 2023 11:29:22 -0400 Subject: [PATCH 60/80] PM-3275 - CurrentContextExtensions.cs - PR Feedback - move parenthesis up from own line. --- src/Core/Context/CurrentContextExtensions.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Core/Context/CurrentContextExtensions.cs b/src/Core/Context/CurrentContextExtensions.cs index 750ddb404d0d..03380bf84e83 100644 --- a/src/Core/Context/CurrentContextExtensions.cs +++ b/src/Core/Context/CurrentContextExtensions.cs @@ -5,8 +5,7 @@ namespace Bit.Core.Context; public static class CurrentContextExtensions { public static async Task AnyOrgUserHasManageResetPasswordPermission(this ICurrentContext currentContext, - ICollection organizationUserDetails - ) + ICollection organizationUserDetails) { var hasManageResetPasswordPermission = false; // if user has a single org with manage reset password permission, then they have it From f651f85e3356385c8e3e17d90a884030b951261f Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Fri, 6 Oct 2023 11:42:41 -0400 Subject: [PATCH 61/80] PM-3275 - SetInitialMasterPasswordCommandTests.cs - Replace unnecessary variable --- .../SetInitialMasterPasswordCommandTests.cs | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommandTests.cs b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommandTests.cs index d58537571588..0ed43ced2ab5 100644 --- a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommandTests.cs +++ b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommandTests.cs @@ -25,11 +25,10 @@ public async Task SetInitialMasterPassword_Success(SutProvider() .UpdatePasswordHash(Arg.Any(), Arg.Any(), true, false) - .Returns(identityResult); + .Returns(IdentityResult.Success); sutProvider.GetDependency() .GetByIdentifierAsync(orgIdentifier) @@ -75,11 +74,10 @@ public async Task SetInitialMasterPassword_InvalidOrganization_Throws(SutProvide { // Arrange user.MasterPassword = null; - var identityResult = IdentityResult.Success; sutProvider.GetDependency() .UpdatePasswordHash(Arg.Any(), Arg.Any(), true, false) - .Returns(identityResult); + .Returns(IdentityResult.Success); sutProvider.GetDependency() .GetByIdentifierAsync(orgIdentifier) @@ -96,11 +94,10 @@ public async Task SetInitialMasterPassword_UserNotFoundInOrganization_Throws(Sut { // Arrange user.MasterPassword = null; - var identityResult = IdentityResult.Success; sutProvider.GetDependency() .UpdatePasswordHash(Arg.Any(), Arg.Any(), true, false) - .Returns(identityResult); + .Returns(IdentityResult.Success); sutProvider.GetDependency() .GetByIdentifierAsync(Arg.Any()) @@ -122,11 +119,10 @@ public async Task SetInitialMasterPassword_ConfirmedOrgUser_DoesNotCallAcceptOrg { // Arrange user.MasterPassword = null; - var identityResult = IdentityResult.Success; sutProvider.GetDependency() .UpdatePasswordHash(Arg.Any(), Arg.Any(), true, false) - .Returns(identityResult); + .Returns(IdentityResult.Success); sutProvider.GetDependency() .GetByIdentifierAsync(orgIdentifier) @@ -153,11 +149,10 @@ public async Task SetInitialMasterPassword_InvitedOrgUser_CallsAcceptOrgUser(Sut { // Arrange user.MasterPassword = null; - var identityResult = IdentityResult.Success; sutProvider.GetDependency() .UpdatePasswordHash(Arg.Any(), Arg.Any(), true, false) - .Returns(identityResult); + .Returns(IdentityResult.Success); sutProvider.GetDependency() .GetByIdentifierAsync(orgIdentifier) From be5ad4930a273619a7354287392dcd7b52378587 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Fri, 6 Oct 2023 11:43:55 -0400 Subject: [PATCH 62/80] PM-3275 - SetInitialMasterPasswordCommandTests.cs - PR Feedback - Add expected outcome statement to test name --- .../UserMasterPassword/SetInitialMasterPasswordCommandTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommandTests.cs b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommandTests.cs index 0ed43ced2ab5..97040b6603de 100644 --- a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommandTests.cs +++ b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommandTests.cs @@ -55,7 +55,7 @@ public async Task SetInitialMasterPassword_UserIsNull_ThrowsArgumentNullExceptio [Theory] [BitAutoData] - public async Task SetInitialMasterPassword_AlreadyHasPassword(SutProvider sutProvider, User user, string masterPassword, string key, string orgIdentifier) + public async Task SetInitialMasterPassword_AlreadyHasPassword_ReturnsFalse(SutProvider sutProvider, User user, string masterPassword, string key, string orgIdentifier) { // Arrange user.MasterPassword = "ExistingPassword"; From 6b92cdeaac54d717a7f37022ee06b5a4cc40ec73 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Fri, 6 Oct 2023 12:50:15 -0400 Subject: [PATCH 63/80] PM-3275 - Set Initial Password command and tests - PR Feedback changes - (1) Rename orgIdentifier --> OrgSsoIdentifier for clarity (2) Update SetInitialMasterPasswordAsync to not allow null orgSsoId with explicit message saying this vs letting null org trigger invalid organization (3) Add test to cover this new scenario. --- .../ISetInitialMasterPasswordCommand.cs | 2 +- .../SetInitialMasterPasswordCommand.cs | 12 +++++++++--- .../SetInitialMasterPasswordCommandTests.cs | 19 +++++++++++++++++++ 3 files changed, 29 insertions(+), 4 deletions(-) diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/ISetInitialMasterPasswordCommand.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/ISetInitialMasterPasswordCommand.cs index 5d6074aaa4e0..89a280fac490 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/ISetInitialMasterPasswordCommand.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/ISetInitialMasterPasswordCommand.cs @@ -6,5 +6,5 @@ namespace Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; public interface ISetInitialMasterPasswordCommand { public Task SetInitialMasterPasswordAsync(User user, string masterPassword, string key, - string orgIdentifier = null); + string orgSsoIdentifier); } diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommand.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommand.cs index a53522733e40..fa82aff6a628 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommand.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommand.cs @@ -51,7 +51,7 @@ public SetInitialMasterPasswordCommand(ILogger } public async Task SetInitialMasterPasswordAsync(User user, string masterPassword, string key, - string orgIdentifier = null) + string orgSsoIdentifier) { if (user == null) { @@ -76,7 +76,13 @@ public async Task SetInitialMasterPasswordAsync(User user, strin await _userRepository.ReplaceAsync(user); await _eventService.LogUserEventAsync(user.Id, EventType.User_ChangedPassword); - var org = await _organizationRepository.GetByIdentifierAsync(orgIdentifier); + + if (string.IsNullOrWhiteSpace(orgSsoIdentifier)) + { + throw new BadRequestException("Organization SSO Identifier required."); + } + + var org = await _organizationRepository.GetByIdentifierAsync(orgSsoIdentifier); if (org == null) { @@ -94,7 +100,7 @@ public async Task SetInitialMasterPasswordAsync(User user, strin // required to set a MP for the first time and we don't want to re-execute the accept logic // as they are already confirmed. // TLDR: only accept post SSO user if they are invited - if (!string.IsNullOrWhiteSpace(orgIdentifier) && orgUser.Status == OrganizationUserStatusType.Invited) + if (orgUser.Status == OrganizationUserStatusType.Invited) { await _acceptOrgUserCommand.AcceptOrgUserAsync(orgUser, user, _userService); } diff --git a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommandTests.cs b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommandTests.cs index 97040b6603de..a352976438f0 100644 --- a/test/Core.Test/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommandTests.cs +++ b/test/Core.Test/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommandTests.cs @@ -67,6 +67,25 @@ public async Task SetInitialMasterPassword_AlreadyHasPassword_ReturnsFalse(SutPr Assert.False(result.Succeeded); } + [Theory] + [BitAutoData] + public async Task SetInitialMasterPassword_NullOrgSsoIdentifier_ThrowsBadRequestException( + SutProvider sutProvider, User user, string masterPassword, string key) + { + // Arrange + user.MasterPassword = null; + string orgSsoIdentifier = null; + + sutProvider.GetDependency() + .UpdatePasswordHash(Arg.Any(), Arg.Any(), true, false) + .Returns(IdentityResult.Success); + + // Act & Assert + var exception = await Assert.ThrowsAsync( + async () => await sutProvider.Sut.SetInitialMasterPasswordAsync(user, masterPassword, key, orgSsoIdentifier)); + Assert.Equal("Organization SSO Identifier required.", exception.Message); + } + [Theory] [BitAutoData] From bc476b7779d24715752d42d122497ac6c7f7fb65 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Fri, 6 Oct 2023 13:10:51 -0400 Subject: [PATCH 64/80] PM-3275 - SetInitialMasterPasswordCommand.cs - Move summary from implementation to interface to better respect standards and the fact that the interface is the more seen piece of code. --- .../Interfaces/ISetInitialMasterPasswordCommand.cs | 9 +++++++++ .../SetInitialMasterPasswordCommand.cs | 9 --------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/ISetInitialMasterPasswordCommand.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/ISetInitialMasterPasswordCommand.cs index 89a280fac490..31dd19d5bfb7 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/ISetInitialMasterPasswordCommand.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/ISetInitialMasterPasswordCommand.cs @@ -3,6 +3,15 @@ namespace Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; +/// +/// Manages the setting of the initial master password for a in an organization. +/// This class is primarily invoked in two scenarios: +/// 1) In organizations configured with Single Sign-On (SSO) and master password decryption: +/// just in time (JIT) provisioned users logging in via SSO are required to set a master password. +/// 2) In organizations configured with SSO and trusted devices decryption: +/// Users who are upgraded to have admin account recovery permissions must set a master password +/// to ensure their ability to reset other users' accounts. +/// public interface ISetInitialMasterPasswordCommand { public Task SetInitialMasterPasswordAsync(User user, string masterPassword, string key, diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommand.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommand.cs index fa82aff6a628..e1c75899e202 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommand.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommand.cs @@ -10,15 +10,6 @@ namespace Bit.Core.Auth.UserFeatures.UserMasterPassword; -/// -/// Manages the setting of the initial master password for a in an organization. -/// This class is primarily invoked in two scenarios: -/// 1) In organizations configured with Single Sign-On (SSO) and master password decryption: -/// just in time (JIT) provisioned users logging in via SSO are required to set a master password. -/// 2) In organizations configured with SSO and trusted devices decryption: -/// Users who are upgraded to have admin account recovery permissions must set a master password -/// to ensure their ability to reset other users' accounts. -/// public class SetInitialMasterPasswordCommand : ISetInitialMasterPasswordCommand { private readonly ILogger _logger; From c891f43ae6405eb3501896a50538a40133bb6921 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Tue, 10 Oct 2023 13:01:07 -0400 Subject: [PATCH 65/80] PM-3275 - AcceptOrgUserCommand.cs - Per PR feedback, rename AcceptOrgUserByTokenAsync -> AcceptOrgUserByEmailTokenAsync + replace generic name token with emailToken --- .../Controllers/OrganizationUsersController.cs | 4 ++-- .../OrganizationUsers/AcceptOrgUserCommand.cs | 6 +++--- .../Interfaces/IAcceptOrgUserCommand.cs | 4 ++-- .../OrganizationUsersControllerTests.cs | 4 ++-- .../AcceptOrgUserCommandTests.cs | 16 ++++++++-------- 5 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/Api/Controllers/OrganizationUsersController.cs b/src/Api/Controllers/OrganizationUsersController.cs index 1398c0e79d61..cb8d494e1022 100644 --- a/src/Api/Controllers/OrganizationUsersController.cs +++ b/src/Api/Controllers/OrganizationUsersController.cs @@ -200,7 +200,7 @@ public async Task AcceptInit(Guid orgId, Guid organizationUserId, [FromBody] Org } await _organizationService.InitPendingOrganization(user.Id, orgId, model.Keys.PublicKey, model.Keys.EncryptedPrivateKey, model.CollectionName); - await _acceptOrgUserCommand.AcceptOrgUserByTokenAsync(organizationUserId, user, model.Token, _userService); + await _acceptOrgUserCommand.AcceptOrgUserByEmailTokenAsync(organizationUserId, user, model.Token, _userService); await _organizationService.ConfirmUserAsync(orgId, organizationUserId, model.Key, user.Id, _userService); } @@ -222,7 +222,7 @@ public async Task Accept(Guid orgId, Guid organizationUserId, [FromBody] Organiz throw new BadRequestException(string.Empty, "Master Password reset is required, but not provided."); } - await _acceptOrgUserCommand.AcceptOrgUserByTokenAsync(organizationUserId, user, model.Token, _userService); + await _acceptOrgUserCommand.AcceptOrgUserByEmailTokenAsync(organizationUserId, user, model.Token, _userService); if (useMasterPasswordPolicy) { diff --git a/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs b/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs index 60d057832182..331d71d73cc5 100644 --- a/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs +++ b/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs @@ -45,7 +45,7 @@ public AcceptOrgUserCommand( _orgUserInviteTokenDataFactory = orgUserInviteTokenDataFactory; } - public async Task AcceptOrgUserByTokenAsync(Guid organizationUserId, User user, string token, + public async Task AcceptOrgUserByEmailTokenAsync(Guid organizationUserId, User user, string emailToken, IUserService userService) { var orgUser = await _organizationUserRepository.GetByIdAsync(organizationUserId); @@ -62,10 +62,10 @@ public async Task AcceptOrgUserByTokenAsync(Guid organizationU // TODO: PM-4142 - remove old token validation logic once 3 releases of backwards compatibility are complete // TODO: update this code to only run old token validation if new token validation fails var newTokenValid = OrgUserInviteTokenable.ValidateOrgUserInviteStringToken( - _orgUserInviteTokenDataFactory, token, orgUser); + _orgUserInviteTokenDataFactory, emailToken, orgUser); var tokenValid = newTokenValid || - CoreHelpers.UserInviteTokenIsValid(_dataProtector, token, user.Email, orgUser.Id, + CoreHelpers.UserInviteTokenIsValid(_dataProtector, emailToken, user.Email, orgUser.Id, _globalSettings); if (!tokenValid) diff --git a/src/Core/OrganizationFeatures/OrganizationUsers/Interfaces/IAcceptOrgUserCommand.cs b/src/Core/OrganizationFeatures/OrganizationUsers/Interfaces/IAcceptOrgUserCommand.cs index 551901aa1085..14cabda52137 100644 --- a/src/Core/OrganizationFeatures/OrganizationUsers/Interfaces/IAcceptOrgUserCommand.cs +++ b/src/Core/OrganizationFeatures/OrganizationUsers/Interfaces/IAcceptOrgUserCommand.cs @@ -9,9 +9,9 @@ public interface IAcceptOrgUserCommand /// Moves an OrganizationUser into the Accepted status and marks their email as verified. /// This method is used where the user has clicked the invitation link sent by email. /// - /// The token embedded in the email invitation link + /// The token embedded in the email invitation link /// The accepted OrganizationUser. - Task AcceptOrgUserByTokenAsync(Guid organizationUserId, User user, string token, IUserService userService); + Task AcceptOrgUserByEmailTokenAsync(Guid organizationUserId, User user, string emailToken, IUserService userService); Task AcceptOrgUserByOrgSsoIdAsync(string orgIdentifier, User user, IUserService userService); Task AcceptOrgUserByOrgIdAsync(Guid organizationId, User user, IUserService userService); Task AcceptOrgUserAsync(OrganizationUser orgUser, User user, IUserService userService); diff --git a/test/Api.Test/Controllers/OrganizationUsersControllerTests.cs b/test/Api.Test/Controllers/OrganizationUsersControllerTests.cs index f394d91eac12..6c96eae04c63 100644 --- a/test/Api.Test/Controllers/OrganizationUsersControllerTests.cs +++ b/test/Api.Test/Controllers/OrganizationUsersControllerTests.cs @@ -65,7 +65,7 @@ public async Task Accept_NoMasterPasswordReset(Guid orgId, Guid orgUserId, await sutProvider.Sut.Accept(orgId, orgUserId, model); await sutProvider.GetDependency().Received(1) - .AcceptOrgUserByTokenAsync(orgUserId, user, model.Token, sutProvider.GetDependency()); + .AcceptOrgUserByEmailTokenAsync(orgUserId, user, model.Token, sutProvider.GetDependency()); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() .UpdateUserResetPasswordEnrollmentAsync(default, default, default, default); } @@ -87,7 +87,7 @@ public async Task Accept_RequireMasterPasswordReset(Guid orgId, Guid orgUserId, await sutProvider.Sut.Accept(orgId, orgUserId, model); await sutProvider.GetDependency().Received(1) - .AcceptOrgUserByTokenAsync(orgUserId, user, model.Token, sutProvider.GetDependency()); + .AcceptOrgUserByEmailTokenAsync(orgUserId, user, model.Token, sutProvider.GetDependency()); await sutProvider.GetDependency().Received(1) .UpdateUserResetPasswordEnrollmentAsync(orgId, user.Id, model.ResetPasswordKey, user.Id); } diff --git a/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs b/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs index 873973308745..33ad63d02e0c 100644 --- a/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs +++ b/test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs @@ -195,7 +195,7 @@ public async Task AcceptOrgUserByToken_OldToken_AcceptsUserAndVerifiesEmail( var oldToken = CreateOldToken(sutProvider, orgUser); // Act - var resultOrgUser = await sutProvider.Sut.AcceptOrgUserByTokenAsync(orgUser.Id, user, oldToken, _userService); + var resultOrgUser = await sutProvider.Sut.AcceptOrgUserByEmailTokenAsync(orgUser.Id, user, oldToken, _userService); // Assert AssertValidAcceptedOrgUser(resultOrgUser, orgUser, user); @@ -231,7 +231,7 @@ public async Task AcceptOrgUserByToken_NewToken_AcceptsUserAndVerifiesEmail( var newToken = CreateNewToken(orgUser); // Act - var resultOrgUser = await sutProvider.Sut.AcceptOrgUserByTokenAsync(orgUser.Id, user, newToken, _userService); + var resultOrgUser = await sutProvider.Sut.AcceptOrgUserByEmailTokenAsync(orgUser.Id, user, newToken, _userService); // Assert AssertValidAcceptedOrgUser(resultOrgUser, orgUser, user); @@ -254,7 +254,7 @@ public async Task AcceptOrgUserByToken_NullOrgUser_ThrowsBadRequest( // Act & Assert var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.AcceptOrgUserByTokenAsync(orgUserId, user, "token", _userService)); + () => sutProvider.Sut.AcceptOrgUserByEmailTokenAsync(orgUserId, user, "token", _userService)); Assert.Equal("User invalid.", exception.Message); } @@ -274,7 +274,7 @@ public async Task AcceptOrgUserByToken_GenericInvalidToken_ThrowsBadRequest( // Act & Assert var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.AcceptOrgUserByTokenAsync(orgUser.Id, user, invalidToken, _userService)); + () => sutProvider.Sut.AcceptOrgUserByEmailTokenAsync(orgUser.Id, user, invalidToken, _userService)); Assert.Equal("Invalid token.", exception.Message); } @@ -298,7 +298,7 @@ public async Task AcceptOrgUserByToken_ExpiredOldToken_ThrowsBadRequest( // Act & Assert var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.AcceptOrgUserByTokenAsync(orgUser.Id, user, oldToken, _userService)); + () => sutProvider.Sut.AcceptOrgUserByEmailTokenAsync(orgUser.Id, user, oldToken, _userService)); Assert.Equal("Invalid token.", exception.Message); } @@ -330,7 +330,7 @@ public async Task AcceptOrgUserByToken_ExpiredNewToken_ThrowsBadRequest( // Act & Assert var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.AcceptOrgUserByTokenAsync(orgUser.Id, user, newToken, _userService)); + () => sutProvider.Sut.AcceptOrgUserByEmailTokenAsync(orgUser.Id, user, newToken, _userService)); Assert.Equal("Invalid token.", exception.Message); @@ -375,7 +375,7 @@ public async Task AcceptOrgUserByToken_UserAlreadyInOrg_ThrowsBadRequest( // Act & Assert var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.AcceptOrgUserByTokenAsync(orgUser.Id, user, newToken, _userService)); + () => sutProvider.Sut.AcceptOrgUserByEmailTokenAsync(orgUser.Id, user, newToken, _userService)); Assert.Equal(expectedErrorMessage, exception.Message); } @@ -408,7 +408,7 @@ public async Task AcceptOrgUserByToken_EmailMismatch_ThrowsBadRequest( // Act & Assert var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.AcceptOrgUserByTokenAsync(orgUser.Id, user, newToken, _userService)); + () => sutProvider.Sut.AcceptOrgUserByEmailTokenAsync(orgUser.Id, user, newToken, _userService)); Assert.Equal("User email does not match invite.", exception.Message); } From 07c537448a85951d038906978470a2a99632984e Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Fri, 13 Oct 2023 10:26:32 -0400 Subject: [PATCH 66/80] PM-3275 - OrganizationService.cs - Per PR feedback, remove dupe line --- src/Core/Services/Implementations/OrganizationService.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Core/Services/Implementations/OrganizationService.cs b/src/Core/Services/Implementations/OrganizationService.cs index 1842a012216e..ebe3ca7f7b5d 100644 --- a/src/Core/Services/Implementations/OrganizationService.cs +++ b/src/Core/Services/Implementations/OrganizationService.cs @@ -114,7 +114,6 @@ public OrganizationService( _countNewSmSeatsRequiredQuery = countNewSmSeatsRequiredQuery; _updateSecretsManagerSubscriptionCommand = updateSecretsManagerSubscriptionCommand; _orgUserInviteTokenableFactory = orgUserInviteTokenableFactory; - _orgUserInviteTokenDataFactory = orgUserInviteTokenDataFactory; } public async Task ReplacePaymentMethodAsync(Guid organizationId, string paymentToken, From 0ab5825949f1337626c516500b6d7b58a16560a7 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Fri, 13 Oct 2023 10:29:15 -0400 Subject: [PATCH 67/80] PM-3275 - AcceptOrgUserCommand.cs - Per PR feedback, remove new lines in error messages for consistency. --- .../OrganizationUsers/AcceptOrgUserCommand.cs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs b/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs index 331d71d73cc5..b2efe4680d3f 100644 --- a/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs +++ b/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs @@ -178,8 +178,7 @@ public async Task AcceptOrgUserAsync(OrganizationUser orgUser, if (hasOtherOrgs && invitedSingleOrgPolicies.Any(p => p.OrganizationId == orgUser.OrganizationId)) { - throw new BadRequestException("You may not join this organization until you leave or remove " + - "all other organizations."); + throw new BadRequestException("You may not join this organization until you leave or remove all other organizations."); } // Enforce Single Organization Policy of other organizations user is a member of @@ -187,8 +186,7 @@ public async Task AcceptOrgUserAsync(OrganizationUser orgUser, PolicyType.SingleOrg); if (anySingleOrgPolicies) { - throw new BadRequestException("You cannot join this organization because you are a member of " + - "another organization which forbids it"); + throw new BadRequestException("You cannot join this organization because you are a member of another organization which forbids it"); } // Enforce Two Factor Authentication Policy of organization user is trying to join @@ -198,8 +196,7 @@ public async Task AcceptOrgUserAsync(OrganizationUser orgUser, PolicyType.TwoFactorAuthentication, OrganizationUserStatusType.Invited); if (invitedTwoFactorPolicies.Any(p => p.OrganizationId == orgUser.OrganizationId)) { - throw new BadRequestException("You cannot join this organization until you enable " + - "two-step login on your user account."); + throw new BadRequestException("You cannot join this organization until you enable two-step login on your user account."); } } From 9b78c53dea802b45563746ac74dd97fd6bcf2127 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Fri, 13 Oct 2023 10:31:19 -0400 Subject: [PATCH 68/80] PM-3275 - SetInitialMasterPasswordCommand.cs - Per PR feedback, adjust formatting of constructor for improved readability. --- .../UserMasterPassword/SetInitialMasterPasswordCommand.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Core/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommand.cs b/src/Core/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommand.cs index e1c75899e202..32966f571064 100644 --- a/src/Core/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommand.cs +++ b/src/Core/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommand.cs @@ -22,7 +22,8 @@ public class SetInitialMasterPasswordCommand : ISetInitialMasterPasswordCommand private readonly IOrganizationRepository _organizationRepository; - public SetInitialMasterPasswordCommand(ILogger logger, + public SetInitialMasterPasswordCommand( + ILogger logger, IdentityErrorDescriber identityErrorDescriber, IUserService userService, IUserRepository userRepository, From f3bda4288a3f828dd4f52b60122d6bb951c279fd Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Fri, 13 Oct 2023 10:38:44 -0400 Subject: [PATCH 69/80] PM-3275 - CurrentContextExtensions.cs - Refactor AnyOrgUserHasManageResetPasswordPermission per PR feedback to remove unnecessary var. --- src/Core/Context/CurrentContextExtensions.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Core/Context/CurrentContextExtensions.cs b/src/Core/Context/CurrentContextExtensions.cs index 03380bf84e83..5038e3481520 100644 --- a/src/Core/Context/CurrentContextExtensions.cs +++ b/src/Core/Context/CurrentContextExtensions.cs @@ -7,16 +7,14 @@ public static class CurrentContextExtensions public static async Task AnyOrgUserHasManageResetPasswordPermission(this ICurrentContext currentContext, ICollection organizationUserDetails) { - var hasManageResetPasswordPermission = false; // if user has a single org with manage reset password permission, then they have it foreach (var orgUserDetail in organizationUserDetails) { if (await currentContext.ManageResetPassword(orgUserDetail.OrganizationId)) { - hasManageResetPasswordPermission = true; - break; + return true; } } - return hasManageResetPasswordPermission; + return false; } } From 44bc574d4f2ee7f34e303c59203cd8f58fc97702 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Fri, 13 Oct 2023 10:52:59 -0400 Subject: [PATCH 70/80] PM-3275 - AcceptOrgUserCommand.cs - Per PR feedback, remove completed TODO --- .../OrganizationUsers/AcceptOrgUserCommand.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs b/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs index b2efe4680d3f..c0f2a5f95ddf 100644 --- a/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs +++ b/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs @@ -60,7 +60,6 @@ public async Task AcceptOrgUserByEmailTokenAsync(Guid organiza // For backwards compatibility, must check validity of both types of tokens and accept if either is valid // TODO: PM-4142 - remove old token validation logic once 3 releases of backwards compatibility are complete - // TODO: update this code to only run old token validation if new token validation fails var newTokenValid = OrgUserInviteTokenable.ValidateOrgUserInviteStringToken( _orgUserInviteTokenDataFactory, emailToken, orgUser); From a569fe714025f481d0c7e9dd201c738bfea626af Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Fri, 13 Oct 2023 10:54:36 -0400 Subject: [PATCH 71/80] PM-3275 - PoliciesController.cs - Per PR feedback, update GetByInvitedUser param to be guid instead of string. --- src/Api/Controllers/PoliciesController.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Api/Controllers/PoliciesController.cs b/src/Api/Controllers/PoliciesController.cs index 2457e24adc0b..f3c5a8f298db 100644 --- a/src/Api/Controllers/PoliciesController.cs +++ b/src/Api/Controllers/PoliciesController.cs @@ -118,9 +118,9 @@ public async Task> GetByToken(Guid orgId, [Obsolete("Deprecated API", false)] [AllowAnonymous] [HttpGet("invited-user")] - public async Task> GetByInvitedUser(Guid orgId, [FromQuery] string userId) + public async Task> GetByInvitedUser(Guid orgId, [FromQuery] Guid userId) { - var user = await _userService.GetUserByIdAsync(new Guid(userId)); + var user = await _userService.GetUserByIdAsync(userId); if (user == null) { throw new UnauthorizedAccessException(); From 011fda97a3ccee2b63e38ed1cec5f7b1d41a26ee Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Fri, 13 Oct 2023 11:11:29 -0400 Subject: [PATCH 72/80] PM-3275 - OrgUserInviteTokenable.cs - per PR feedback, add tech debt item info. --- .../Auth/Models/Business/Tokenables/OrgUserInviteTokenable.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Core/Auth/Models/Business/Tokenables/OrgUserInviteTokenable.cs b/src/Core/Auth/Models/Business/Tokenables/OrgUserInviteTokenable.cs index 2f6e96b5fa6e..95c84ad3b53d 100644 --- a/src/Core/Auth/Models/Business/Tokenables/OrgUserInviteTokenable.cs +++ b/src/Core/Auth/Models/Business/Tokenables/OrgUserInviteTokenable.cs @@ -6,7 +6,7 @@ namespace Bit.Core.Auth.Models.Business.Tokenables; public class OrgUserInviteTokenable : ExpiringTokenable { - // TODO: Ideally this would be internal and only visible to the test project. + // TODO: PM-4317 - Ideally this would be internal and only visible to the test project. // but configuring that is out of scope for these changes. public static TimeSpan GetTokenLifetime() => TimeSpan.FromDays(5); From 9c142bf0edfc01373e354282186c454c0ec38b4a Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Fri, 13 Oct 2023 11:26:25 -0400 Subject: [PATCH 73/80] PM-3275 - AcceptOrgUserCommand.cs - Per PR feedback, use const purpose from tokenable instead of magic string. --- .../OrganizationUsers/AcceptOrgUserCommand.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs b/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs index c0f2a5f95ddf..5baff54634ea 100644 --- a/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs +++ b/src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs @@ -35,7 +35,7 @@ public AcceptOrgUserCommand( { // TODO: remove data protector when old token validation removed - _dataProtector = dataProtectionProvider.CreateProtector("OrganizationServiceDataProtector"); + _dataProtector = dataProtectionProvider.CreateProtector(OrgUserInviteTokenable.DataProtectorPurpose); _globalSettings = globalSettings; _organizationUserRepository = organizationUserRepository; _organizationRepository = organizationRepository; From 2841b9654f9a73a76f3501c7005a6ad66874bd10 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Fri, 13 Oct 2023 13:54:50 -0400 Subject: [PATCH 74/80] PM-3275 - Restore non duplicate line to fix tests --- src/Core/Services/Implementations/OrganizationService.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Core/Services/Implementations/OrganizationService.cs b/src/Core/Services/Implementations/OrganizationService.cs index 74c3b793ca48..f9a42e2add96 100644 --- a/src/Core/Services/Implementations/OrganizationService.cs +++ b/src/Core/Services/Implementations/OrganizationService.cs @@ -114,6 +114,7 @@ public OrganizationService( _countNewSmSeatsRequiredQuery = countNewSmSeatsRequiredQuery; _updateSecretsManagerSubscriptionCommand = updateSecretsManagerSubscriptionCommand; _orgUserInviteTokenableFactory = orgUserInviteTokenableFactory; + _orgUserInviteTokenDataFactory = orgUserInviteTokenDataFactory; } public async Task ReplacePaymentMethodAsync(Guid organizationId, string paymentToken, From 36c8d4bd02a64e0491ac185bd2937ff96aee0fe4 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Mon, 16 Oct 2023 13:41:08 -0400 Subject: [PATCH 75/80] PM-3275 - Per PR feedback, revert all sync controller changes as the ProfileResponseModel.organizations array has org objects which have permissions which have the ManageResetPassword permission. So, I have the information that I need clientside already to determine if the user has the ManageResetPassword in any org. --- src/Api/Controllers/AccountsController.cs | 9 ++------- .../Models/Response/ProfileResponseModel.cs | 4 +--- src/Api/Vault/Controllers/SyncController.cs | 10 ++-------- .../Models/Response/SyncResponseModel.cs | 5 ++--- src/Core/Context/CurrentContextExtensions.cs | 20 ------------------- .../Controllers/AccountsControllerTests.cs | 6 +----- 6 files changed, 8 insertions(+), 46 deletions(-) delete mode 100644 src/Core/Context/CurrentContextExtensions.cs diff --git a/src/Api/Controllers/AccountsController.cs b/src/Api/Controllers/AccountsController.cs index 17ab3b6baa42..11c0b26fa08b 100644 --- a/src/Api/Controllers/AccountsController.cs +++ b/src/Api/Controllers/AccountsController.cs @@ -9,7 +9,6 @@ using Bit.Core.Auth.Services; using Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; using Bit.Core.Auth.Utilities; -using Bit.Core.Context; using Bit.Core.Enums; using Bit.Core.Enums.Provider; using Bit.Core.Exceptions; @@ -49,7 +48,6 @@ public class AccountsController : Controller private readonly IPolicyService _policyService; private readonly ISetInitialMasterPasswordCommand _setInitialMasterPasswordCommand; - private ICurrentContext CurrentContext { get; } public AccountsController( GlobalSettings globalSettings, @@ -65,8 +63,7 @@ public AccountsController( ISendService sendService, ICaptchaValidationService captchaValidationService, IPolicyService policyService, - ISetInitialMasterPasswordCommand setInitialMasterPasswordCommand, - ICurrentContext currentContext + ISetInitialMasterPasswordCommand setInitialMasterPasswordCommand ) { _cipherRepository = cipherRepository; @@ -83,7 +80,6 @@ ICurrentContext currentContext _captchaValidationService = captchaValidationService; _policyService = policyService; _setInitialMasterPasswordCommand = setInitialMasterPasswordCommand; - CurrentContext = currentContext; } #region DEPRECATED (Moved to Identity Service) @@ -469,13 +465,12 @@ public async Task GetProfile() await _providerUserRepository.GetManyOrganizationDetailsByUserAsync(user.Id, ProviderUserStatusType.Confirmed); - var hasManageResetPasswordPermission = await CurrentContext.AnyOrgUserHasManageResetPasswordPermission(organizationUserDetails); var twoFactorEnabled = await _userService.TwoFactorIsEnabledAsync(user); var hasPremiumFromOrg = await _userService.HasPremiumFromOrganization(user); var response = new ProfileResponseModel(user, organizationUserDetails, providerUserDetails, providerUserOrganizationDetails, twoFactorEnabled, - hasPremiumFromOrg, hasManageResetPasswordPermission); + hasPremiumFromOrg); return response; } diff --git a/src/Api/Models/Response/ProfileResponseModel.cs b/src/Api/Models/Response/ProfileResponseModel.cs index 009816166995..1fbf002e2761 100644 --- a/src/Api/Models/Response/ProfileResponseModel.cs +++ b/src/Api/Models/Response/ProfileResponseModel.cs @@ -13,7 +13,7 @@ public ProfileResponseModel(User user, IEnumerable providerUserDetails, IEnumerable providerUserOrganizationDetails, bool twoFactorEnabled, - bool premiumFromOrganization, bool? hasManageResetPasswordPermission = null) : base("profile") + bool premiumFromOrganization) : base("profile") { if (user == null) { @@ -33,7 +33,6 @@ public ProfileResponseModel(User user, PrivateKey = user.PrivateKey; SecurityStamp = user.SecurityStamp; ForcePasswordReset = user.ForcePasswordReset; - HasManageResetPasswordPermission = hasManageResetPasswordPermission; UsesKeyConnector = user.UsesKeyConnector; AvatarColor = user.AvatarColor; Organizations = organizationsUserDetails?.Select(o => new ProfileOrganizationResponseModel(o)); @@ -59,7 +58,6 @@ public ProfileResponseModel() : base("profile") public string PrivateKey { get; set; } public string SecurityStamp { get; set; } public bool ForcePasswordReset { get; set; } - public bool? HasManageResetPasswordPermission { get; set; } public bool UsesKeyConnector { get; set; } public string AvatarColor { get; set; } public IEnumerable Organizations { get; set; } diff --git a/src/Api/Vault/Controllers/SyncController.cs b/src/Api/Vault/Controllers/SyncController.cs index b71467638e4d..c2928f934fe2 100644 --- a/src/Api/Vault/Controllers/SyncController.cs +++ b/src/Api/Vault/Controllers/SyncController.cs @@ -1,5 +1,4 @@ using Bit.Api.Vault.Models.Response; -using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Enums.Provider; @@ -29,7 +28,6 @@ public class SyncController : Controller private readonly IPolicyRepository _policyRepository; private readonly ISendRepository _sendRepository; private readonly GlobalSettings _globalSettings; - private ICurrentContext CurrentContext { get; } public SyncController( IUserService userService, @@ -41,8 +39,7 @@ public SyncController( IProviderUserRepository providerUserRepository, IPolicyRepository policyRepository, ISendRepository sendRepository, - GlobalSettings globalSettings, - ICurrentContext currentContext) + GlobalSettings globalSettings) { _userService = userService; _folderRepository = folderRepository; @@ -54,7 +51,6 @@ public SyncController( _policyRepository = policyRepository; _sendRepository = sendRepository; _globalSettings = globalSettings; - CurrentContext = currentContext; } [HttpGet("")] @@ -75,8 +71,6 @@ await _providerUserRepository.GetManyOrganizationDetailsByUserAsync(user.Id, ProviderUserStatusType.Confirmed); var hasEnabledOrgs = organizationUserDetails.Any(o => o.Enabled); - var hasManageResetPasswordPermission = await CurrentContext.AnyOrgUserHasManageResetPasswordPermission(organizationUserDetails); - var folders = await _folderRepository.GetManyByUserIdAsync(user.Id); var ciphers = await _cipherRepository.GetManyByUserIdAsync(user.Id, hasEnabledOrgs); var sends = await _sendRepository.GetManyByUserIdAsync(user.Id); @@ -96,7 +90,7 @@ await _providerUserRepository.GetManyOrganizationDetailsByUserAsync(user.Id, var userHasPremiumFromOrganization = await _userService.HasPremiumFromOrganization(user); var response = new SyncResponseModel(_globalSettings, user, userTwoFactorEnabled, userHasPremiumFromOrganization, organizationUserDetails, providerUserDetails, providerUserOrganizationDetails, folders, collections, ciphers, - collectionCiphersGroupDict, excludeDomains, policies, sends, hasManageResetPasswordPermission); + collectionCiphersGroupDict, excludeDomains, policies, sends); return response; } } diff --git a/src/Api/Vault/Models/Response/SyncResponseModel.cs b/src/Api/Vault/Models/Response/SyncResponseModel.cs index fb80565cb3c8..386b7445be74 100644 --- a/src/Api/Vault/Models/Response/SyncResponseModel.cs +++ b/src/Api/Vault/Models/Response/SyncResponseModel.cs @@ -28,12 +28,11 @@ public SyncResponseModel( IDictionary> collectionCiphersDict, bool excludeDomains, IEnumerable policies, - IEnumerable sends, - bool hasManageResetPasswordPermission) + IEnumerable sends) : base("sync") { Profile = new ProfileResponseModel(user, organizationUserDetails, providerUserDetails, - providerUserOrganizationDetails, userTwoFactorEnabled, userHasPremiumFromOrganization, hasManageResetPasswordPermission); + providerUserOrganizationDetails, userTwoFactorEnabled, userHasPremiumFromOrganization); Folders = folders.Select(f => new FolderResponseModel(f)); Ciphers = ciphers.Select(c => new CipherDetailsResponseModel(c, globalSettings, collectionCiphersDict)); Collections = collections?.Select( diff --git a/src/Core/Context/CurrentContextExtensions.cs b/src/Core/Context/CurrentContextExtensions.cs deleted file mode 100644 index 5038e3481520..000000000000 --- a/src/Core/Context/CurrentContextExtensions.cs +++ /dev/null @@ -1,20 +0,0 @@ -using Bit.Core.Models.Data.Organizations.OrganizationUsers; - -namespace Bit.Core.Context; - -public static class CurrentContextExtensions -{ - public static async Task AnyOrgUserHasManageResetPasswordPermission(this ICurrentContext currentContext, - ICollection organizationUserDetails) - { - // if user has a single org with manage reset password permission, then they have it - foreach (var orgUserDetail in organizationUserDetails) - { - if (await currentContext.ManageResetPassword(orgUserDetail.OrganizationId)) - { - return true; - } - } - return false; - } -} diff --git a/test/Api.Test/Controllers/AccountsControllerTests.cs b/test/Api.Test/Controllers/AccountsControllerTests.cs index e9a09da3c630..d413c1ab8a53 100644 --- a/test/Api.Test/Controllers/AccountsControllerTests.cs +++ b/test/Api.Test/Controllers/AccountsControllerTests.cs @@ -4,7 +4,6 @@ using Bit.Core.Auth.Models.Api.Request.Accounts; using Bit.Core.Auth.Services; using Bit.Core.Auth.UserFeatures.UserMasterPassword.Interfaces; -using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; @@ -40,7 +39,6 @@ public class AccountsControllerTests : IDisposable private readonly ICaptchaValidationService _captchaValidationService; private readonly IPolicyService _policyService; private readonly ISetInitialMasterPasswordCommand _setInitialMasterPasswordCommand; - private readonly ICurrentContext _currentContext; public AccountsControllerTests() { @@ -58,7 +56,6 @@ public AccountsControllerTests() _captchaValidationService = Substitute.For(); _policyService = Substitute.For(); _setInitialMasterPasswordCommand = Substitute.For(); - _currentContext = Substitute.For(); _sut = new AccountsController( _globalSettings, @@ -74,8 +71,7 @@ public AccountsControllerTests() _sendService, _captchaValidationService, _policyService, - _setInitialMasterPasswordCommand, - _currentContext + _setInitialMasterPasswordCommand ); } From eaf319719a7c66b91880aa891ca1e76ab7acdfd7 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Wed, 18 Oct 2023 18:39:05 -0400 Subject: [PATCH 76/80] PM-3275 - PoliciesControllerTests.cs - Update imports as the PoliciesController was moved under the admin console team's domain. --- test/Api.Test/Controllers/PoliciesControllerTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Api.Test/Controllers/PoliciesControllerTests.cs b/test/Api.Test/Controllers/PoliciesControllerTests.cs index 7f410dc11def..0d2f004b1722 100644 --- a/test/Api.Test/Controllers/PoliciesControllerTests.cs +++ b/test/Api.Test/Controllers/PoliciesControllerTests.cs @@ -1,6 +1,6 @@ using System.Security.Claims; using System.Text.Json; -using Bit.Api.Controllers; +using Bit.Api.AdminConsole.Controllers; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; From 6806047a64135d98d324dd3cf44dee052286e77a Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Mon, 30 Oct 2023 18:02:55 -0400 Subject: [PATCH 77/80] PM-3275 - Resolve issues from merge conflict resolutions to get solution building. --- src/Api/AdminConsole/Controllers/OrganizationUsersController.cs | 1 + .../OrganizationServiceCollectionExtensions.cs | 2 ++ src/Core/Services/Implementations/UserService.cs | 1 - 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs b/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs index a8cf6a638711..73140cea4eda 100644 --- a/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs +++ b/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs @@ -11,6 +11,7 @@ using Bit.Core.Models.Data.Organizations.OrganizationUsers; using Bit.Core.Models.Data.Organizations.Policies; using Bit.Core.OrganizationFeatures.OrganizationSubscriptions.Interface; +using Bit.Core.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.Repositories; using Bit.Core.Services; using Microsoft.AspNetCore.Authorization; diff --git a/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs b/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs index d50ef3de7875..64874e590be1 100644 --- a/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs +++ b/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs @@ -21,6 +21,8 @@ using Bit.Core.OrganizationFeatures.OrganizationSponsorships.FamiliesForEnterprise.SelfHosted; using Bit.Core.OrganizationFeatures.OrganizationSubscriptions; using Bit.Core.OrganizationFeatures.OrganizationSubscriptions.Interface; +using Bit.Core.OrganizationFeatures.OrganizationUsers; +using Bit.Core.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.Services; using Bit.Core.Settings; using Bit.Core.Tokens; diff --git a/src/Core/Services/Implementations/UserService.cs b/src/Core/Services/Implementations/UserService.cs index a70b462e8a2b..abe01d2e9a72 100644 --- a/src/Core/Services/Implementations/UserService.cs +++ b/src/Core/Services/Implementations/UserService.cs @@ -96,7 +96,6 @@ public UserService( IProviderUserRepository providerUserRepository, IStripeSyncService stripeSyncService, IDataProtectorTokenFactory orgUserInviteTokenDataFactory, - IStripeSyncService stripeSyncService, IWebAuthnCredentialRepository webAuthnRepository, IDataProtectorTokenFactory webAuthnLoginTokenizer) : base( From c5468974024a8670817ba11d1be03d8d81b869cf Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Mon, 30 Oct 2023 18:27:03 -0400 Subject: [PATCH 78/80] PM-3275 / PM-4633 - PoliciesController.cs - use orgUserId to look up user instead of orgId. Oops. --- src/Api/AdminConsole/Controllers/PoliciesController.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Api/AdminConsole/Controllers/PoliciesController.cs b/src/Api/AdminConsole/Controllers/PoliciesController.cs index a3cf74c3cbae..60257814e93f 100644 --- a/src/Api/AdminConsole/Controllers/PoliciesController.cs +++ b/src/Api/AdminConsole/Controllers/PoliciesController.cs @@ -103,7 +103,7 @@ public async Task> GetByToken(Guid orgId, throw new NotFoundException(); } - var orgUser = await _organizationUserRepository.GetByIdAsync(orgId); + var orgUser = await _organizationUserRepository.GetByIdAsync(organizationUserId); if (orgUser == null || orgUser.OrganizationId != orgId) { throw new NotFoundException(); From c8aa95397b2579cb76a5ab87965303220d1ad420 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Mon, 30 Oct 2023 19:11:08 -0400 Subject: [PATCH 79/80] Fix user service tests --- test/Core.Test/Services/UserServiceTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/test/Core.Test/Services/UserServiceTests.cs b/test/Core.Test/Services/UserServiceTests.cs index fe9f656d23b1..cbedea4a7a71 100644 --- a/test/Core.Test/Services/UserServiceTests.cs +++ b/test/Core.Test/Services/UserServiceTests.cs @@ -278,7 +278,6 @@ public async Task VerifySecretAsync_Works( sutProvider.GetDependency(), sutProvider.GetDependency(), new FakeDataProtectorTokenFactory(), - sutProvider.GetDependency(), sutProvider.GetDependency(), sutProvider.GetDependency>() ); From 050b691b3cc9f769d250590b4fe32fa7145f7073 Mon Sep 17 00:00:00 2001 From: Jared Snider Date: Wed, 1 Nov 2023 18:19:46 -0400 Subject: [PATCH 80/80] Resolve merge conflict --- src/Core/Services/Implementations/OrganizationService.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Core/Services/Implementations/OrganizationService.cs b/src/Core/Services/Implementations/OrganizationService.cs index 70aebcbb0f84..73eb98cd0c58 100644 --- a/src/Core/Services/Implementations/OrganizationService.cs +++ b/src/Core/Services/Implementations/OrganizationService.cs @@ -87,7 +87,6 @@ public OrganizationService( IProviderOrganizationRepository providerOrganizationRepository, IProviderUserRepository providerUserRepository, ICountNewSmSeatsRequiredQuery countNewSmSeatsRequiredQuery, - IUpdateSecretsManagerSubscriptionCommand updateSecretsManagerSubscriptionCommand, IOrgUserInviteTokenableFactory orgUserInviteTokenableFactory, IDataProtectorTokenFactory orgUserInviteTokenDataFactory, IUpdateSecretsManagerSubscriptionCommand updateSecretsManagerSubscriptionCommand,