From 58c6f09629700f995e6c1da0006b40cc55ae9dcd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Tom=C3=A9?= <108268980+r-tome@users.noreply.github.com> Date: Wed, 9 Oct 2024 15:32:49 +0100 Subject: [PATCH] [PM-12684] Remove Members Bulk 2FA feature flag logic (#4864) --- .../Scim/Controllers/v2/UsersController.cs | 5 +- .../src/Scim/Users/PatchUserCommand.cs | 5 +- .../Scim.Test/Users/PatchUserCommandTests.cs | 6 +- .../Controllers/OrganizationsController.cs | 2 +- src/Admin/Controllers/UsersController.cs | 27 +- .../OrganizationUsersController.cs | 56 +---- .../Controllers/PoliciesController.cs | 2 +- .../Public/Controllers/MembersController.cs | 27 +- .../Public/Controllers/PoliciesController.cs | 5 +- .../Services/IOrganizationService.cs | 9 +- .../AdminConsole/Services/IPolicyService.cs | 3 +- .../Implementations/OrganizationService.cs | 209 ++-------------- .../Services/Implementations/PolicyService.cs | 72 +----- .../Implementations/SsoConfigService.cs | 9 +- .../Services/OrganizationServiceTests.cs | 231 +++--------------- .../Services/PolicyServiceTests.cs | 64 ++--- .../Auth/Services/SsoConfigServiceTests.cs | 2 - 17 files changed, 125 insertions(+), 609 deletions(-) diff --git a/bitwarden_license/src/Scim/Controllers/v2/UsersController.cs b/bitwarden_license/src/Scim/Controllers/v2/UsersController.cs index 1429fc387..5e73505b9 100644 --- a/bitwarden_license/src/Scim/Controllers/v2/UsersController.cs +++ b/bitwarden_license/src/Scim/Controllers/v2/UsersController.cs @@ -17,7 +17,6 @@ namespace Bit.Scim.Controllers.v2; [ExceptionHandlerFilter] public class UsersController : Controller { - private readonly IUserService _userService; private readonly IOrganizationUserRepository _organizationUserRepository; private readonly IOrganizationService _organizationService; private readonly IGetUsersListQuery _getUsersListQuery; @@ -27,7 +26,6 @@ public class UsersController : Controller private readonly ILogger _logger; public UsersController( - IUserService userService, IOrganizationUserRepository organizationUserRepository, IOrganizationService organizationService, IGetUsersListQuery getUsersListQuery, @@ -36,7 +34,6 @@ public class UsersController : Controller IPostUserCommand postUserCommand, ILogger logger) { - _userService = userService; _organizationUserRepository = organizationUserRepository; _organizationService = organizationService; _getUsersListQuery = getUsersListQuery; @@ -98,7 +95,7 @@ public class UsersController : Controller if (model.Active && orgUser.Status == OrganizationUserStatusType.Revoked) { - await _organizationService.RestoreUserAsync(orgUser, EventSystemUser.SCIM, _userService); + await _organizationService.RestoreUserAsync(orgUser, EventSystemUser.SCIM); } else if (!model.Active && orgUser.Status != OrganizationUserStatusType.Revoked) { diff --git a/bitwarden_license/src/Scim/Users/PatchUserCommand.cs b/bitwarden_license/src/Scim/Users/PatchUserCommand.cs index 075807a58..f4445354c 100644 --- a/bitwarden_license/src/Scim/Users/PatchUserCommand.cs +++ b/bitwarden_license/src/Scim/Users/PatchUserCommand.cs @@ -9,18 +9,15 @@ namespace Bit.Scim.Users; public class PatchUserCommand : IPatchUserCommand { - private readonly IUserService _userService; private readonly IOrganizationUserRepository _organizationUserRepository; private readonly IOrganizationService _organizationService; private readonly ILogger _logger; public PatchUserCommand( - IUserService userService, IOrganizationUserRepository organizationUserRepository, IOrganizationService organizationService, ILogger logger) { - _userService = userService; _organizationUserRepository = organizationUserRepository; _organizationService = organizationService; _logger = logger; @@ -74,7 +71,7 @@ public class PatchUserCommand : IPatchUserCommand { if (active && orgUser.Status == OrganizationUserStatusType.Revoked) { - await _organizationService.RestoreUserAsync(orgUser, EventSystemUser.SCIM, _userService); + await _organizationService.RestoreUserAsync(orgUser, EventSystemUser.SCIM); return true; } else if (!active && orgUser.Status != OrganizationUserStatusType.Revoked) diff --git a/bitwarden_license/test/Scim.Test/Users/PatchUserCommandTests.cs b/bitwarden_license/test/Scim.Test/Users/PatchUserCommandTests.cs index 977011b35..6e9c985b8 100644 --- a/bitwarden_license/test/Scim.Test/Users/PatchUserCommandTests.cs +++ b/bitwarden_license/test/Scim.Test/Users/PatchUserCommandTests.cs @@ -43,7 +43,7 @@ public class PatchUserCommandTests await sutProvider.Sut.PatchUserAsync(organizationUser.OrganizationId, organizationUser.Id, scimPatchModel); - await sutProvider.GetDependency().Received(1).RestoreUserAsync(organizationUser, EventSystemUser.SCIM, Arg.Any()); + await sutProvider.GetDependency().Received(1).RestoreUserAsync(organizationUser, EventSystemUser.SCIM); } [Theory] @@ -71,7 +71,7 @@ public class PatchUserCommandTests await sutProvider.Sut.PatchUserAsync(organizationUser.OrganizationId, organizationUser.Id, scimPatchModel); - await sutProvider.GetDependency().Received(1).RestoreUserAsync(organizationUser, EventSystemUser.SCIM, Arg.Any()); + await sutProvider.GetDependency().Received(1).RestoreUserAsync(organizationUser, EventSystemUser.SCIM); } [Theory] @@ -147,7 +147,7 @@ public class PatchUserCommandTests await sutProvider.Sut.PatchUserAsync(organizationUser.OrganizationId, organizationUser.Id, scimPatchModel); - await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().RestoreUserAsync(default, EventSystemUser.SCIM, default); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().RestoreUserAsync(default, EventSystemUser.SCIM); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().RevokeUserAsync(default, EventSystemUser.SCIM); } diff --git a/src/Admin/AdminConsole/Controllers/OrganizationsController.cs b/src/Admin/AdminConsole/Controllers/OrganizationsController.cs index 4dc7ec56d..78c8785c7 100644 --- a/src/Admin/AdminConsole/Controllers/OrganizationsController.cs +++ b/src/Admin/AdminConsole/Controllers/OrganizationsController.cs @@ -473,7 +473,7 @@ public class OrganizationsController : Controller await Task.WhenAll(policies.Select(async policy => { policy.Enabled = false; - await _policyService.SaveAsync(policy, _userService, _organizationService, null); + await _policyService.SaveAsync(policy, _organizationService, null); })); } } diff --git a/src/Admin/Controllers/UsersController.cs b/src/Admin/Controllers/UsersController.cs index 842abaea6..2842efcdb 100644 --- a/src/Admin/Controllers/UsersController.cs +++ b/src/Admin/Controllers/UsersController.cs @@ -4,7 +4,6 @@ using Bit.Admin.Enums; using Bit.Admin.Models; using Bit.Admin.Services; using Bit.Admin.Utilities; -using Bit.Core; using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; using Bit.Core.Repositories; using Bit.Core.Services; @@ -25,8 +24,6 @@ public class UsersController : Controller private readonly GlobalSettings _globalSettings; private readonly IAccessControlService _accessControlService; private readonly ITwoFactorIsEnabledQuery _twoFactorIsEnabledQuery; - private readonly IFeatureService _featureService; - private readonly IUserService _userService; public UsersController( IUserRepository userRepository, @@ -34,9 +31,7 @@ public class UsersController : Controller IPaymentService paymentService, GlobalSettings globalSettings, IAccessControlService accessControlService, - ITwoFactorIsEnabledQuery twoFactorIsEnabledQuery, - IFeatureService featureService, - IUserService userService) + ITwoFactorIsEnabledQuery twoFactorIsEnabledQuery) { _userRepository = userRepository; _cipherRepository = cipherRepository; @@ -44,8 +39,6 @@ public class UsersController : Controller _globalSettings = globalSettings; _accessControlService = accessControlService; _twoFactorIsEnabledQuery = twoFactorIsEnabledQuery; - _featureService = featureService; - _userService = userService; } [RequirePermission(Permission.User_List_View)] @@ -64,22 +57,8 @@ public class UsersController : Controller var skip = (page - 1) * count; var users = await _userRepository.SearchAsync(email, skip, count); - var userModels = new List(); - - if (_featureService.IsEnabled(FeatureFlagKeys.MembersTwoFAQueryOptimization)) - { - var twoFactorAuthLookup = (await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(users.Select(u => u.Id))).ToList(); - - userModels = UserViewModel.MapViewModels(users, twoFactorAuthLookup).ToList(); - } - else - { - foreach (var user in users) - { - var isTwoFactorEnabled = await _userService.TwoFactorIsEnabledAsync(user); - userModels.Add(UserViewModel.MapViewModel(user, isTwoFactorEnabled)); - } - } + var twoFactorAuthLookup = (await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(users.Select(u => u.Id))).ToList(); + var userModels = UserViewModel.MapViewModels(users, twoFactorAuthLookup).ToList(); return View(new UsersModel { diff --git a/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs b/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs index cd6bdd6fa..83beb7e07 100644 --- a/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs +++ b/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs @@ -48,13 +48,11 @@ public class OrganizationUsersController : Controller private readonly IAcceptOrgUserCommand _acceptOrgUserCommand; private readonly IAuthorizationService _authorizationService; private readonly IApplicationCacheService _applicationCacheService; - private readonly IFeatureService _featureService; private readonly ISsoConfigRepository _ssoConfigRepository; private readonly IOrganizationUserUserDetailsQuery _organizationUserUserDetailsQuery; private readonly ITwoFactorIsEnabledQuery _twoFactorIsEnabledQuery; private readonly IDeleteManagedOrganizationUserAccountCommand _deleteManagedOrganizationUserAccountCommand; - public OrganizationUsersController( IOrganizationRepository organizationRepository, IOrganizationUserRepository organizationUserRepository, @@ -70,7 +68,6 @@ public class OrganizationUsersController : Controller IAcceptOrgUserCommand acceptOrgUserCommand, IAuthorizationService authorizationService, IApplicationCacheService applicationCacheService, - IFeatureService featureService, ISsoConfigRepository ssoConfigRepository, IOrganizationUserUserDetailsQuery organizationUserUserDetailsQuery, ITwoFactorIsEnabledQuery twoFactorIsEnabledQuery, @@ -90,7 +87,6 @@ public class OrganizationUsersController : Controller _acceptOrgUserCommand = acceptOrgUserCommand; _authorizationService = authorizationService; _applicationCacheService = applicationCacheService; - _featureService = featureService; _ssoConfigRepository = ssoConfigRepository; _organizationUserUserDetailsQuery = organizationUserUserDetailsQuery; _twoFactorIsEnabledQuery = twoFactorIsEnabledQuery; @@ -142,11 +138,6 @@ public class OrganizationUsersController : Controller throw new NotFoundException(); } - if (_featureService.IsEnabled(FeatureFlagKeys.MembersTwoFAQueryOptimization)) - { - return await Get_vNext(orgId, includeGroups, includeCollections); - } - var organizationUsers = await _organizationUserUserDetailsQuery.GetOrganizationUserUserDetails( new OrganizationUserUserDetailsQueryRequest { @@ -155,17 +146,15 @@ public class OrganizationUsersController : Controller IncludeCollections = includeCollections } ); - - var responseTasks = organizationUsers - .Select(async o => + var organizationUsersTwoFactorEnabled = await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(organizationUsers); + var responses = organizationUsers + .Select(o => { - var orgUser = new OrganizationUserUserDetailsResponseModel(o, - await _userService.TwoFactorIsEnabledAsync(o)); + var userTwoFactorEnabled = organizationUsersTwoFactorEnabled.FirstOrDefault(u => u.user.Id == o.Id).twoFactorIsEnabled; + var orgUser = new OrganizationUserUserDetailsResponseModel(o, userTwoFactorEnabled); return orgUser; }); - var responses = await Task.WhenAll(responseTasks); - return new ListResponseModel(responses); } @@ -296,7 +285,7 @@ public class OrganizationUsersController : Controller await _organizationService.InitPendingOrganization(user.Id, orgId, organizationUserId, model.Keys.PublicKey, model.Keys.EncryptedPrivateKey, model.CollectionName); await _acceptOrgUserCommand.AcceptOrgUserByEmailTokenAsync(organizationUserId, user, model.Token, _userService); - await _organizationService.ConfirmUserAsync(orgId, organizationUserId, model.Key, user.Id, _userService); + await _organizationService.ConfirmUserAsync(orgId, organizationUserId, model.Key, user.Id); } [HttpPost("{organizationUserId}/accept")] @@ -335,8 +324,7 @@ public class OrganizationUsersController : Controller } var userId = _userService.GetProperUserId(User); - var result = await _organizationService.ConfirmUserAsync(orgGuidId, new Guid(id), model.Key, userId.Value, - _userService); + var result = await _organizationService.ConfirmUserAsync(orgGuidId, new Guid(id), model.Key, userId.Value); } [HttpPost("confirm")] @@ -350,10 +338,7 @@ public class OrganizationUsersController : Controller } var userId = _userService.GetProperUserId(User); - var results = _featureService.IsEnabled(FeatureFlagKeys.MembersTwoFAQueryOptimization) - ? await _organizationService.ConfirmUsersAsync_vNext(orgGuidId, model.ToDictionary(), userId.Value) - : await _organizationService.ConfirmUsersAsync(orgGuidId, model.ToDictionary(), userId.Value, - _userService); + var results = await _organizationService.ConfirmUsersAsync(orgGuidId, model.ToDictionary(), userId.Value); return new ListResponseModel(results.Select(r => new OrganizationUserBulkResponseModel(r.Item1.Id, r.Item2))); @@ -616,7 +601,7 @@ public class OrganizationUsersController : Controller [HttpPut("{id}/restore")] public async Task RestoreAsync(Guid orgId, Guid id) { - await RestoreOrRevokeUserAsync(orgId, id, (orgUser, userId) => _organizationService.RestoreUserAsync(orgUser, userId, _userService)); + await RestoreOrRevokeUserAsync(orgId, id, (orgUser, userId) => _organizationService.RestoreUserAsync(orgUser, userId)); } [HttpPatch("restore")] @@ -696,27 +681,4 @@ public class OrganizationUsersController : Controller return new ListResponseModel(result.Select(r => new OrganizationUserBulkResponseModel(r.Item1.Id, r.Item2))); } - - private async Task> Get_vNext(Guid orgId, - bool includeGroups = false, bool includeCollections = false) - { - var organizationUsers = await _organizationUserUserDetailsQuery.GetOrganizationUserUserDetails( - new OrganizationUserUserDetailsQueryRequest - { - OrganizationId = orgId, - IncludeGroups = includeGroups, - IncludeCollections = includeCollections - } - ); - var organizationUsersTwoFactorEnabled = await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(organizationUsers); - var responses = organizationUsers - .Select(o => - { - var userTwoFactorEnabled = organizationUsersTwoFactorEnabled.FirstOrDefault(u => u.user.Id == o.Id).twoFactorIsEnabled; - var orgUser = new OrganizationUserUserDetailsResponseModel(o, userTwoFactorEnabled); - - return orgUser; - }); - return new ListResponseModel(responses); - } } diff --git a/src/Api/AdminConsole/Controllers/PoliciesController.cs b/src/Api/AdminConsole/Controllers/PoliciesController.cs index 5668031d2..b3be852db 100644 --- a/src/Api/AdminConsole/Controllers/PoliciesController.cs +++ b/src/Api/AdminConsole/Controllers/PoliciesController.cs @@ -185,7 +185,7 @@ public class PoliciesController : Controller } var userId = _userService.GetProperUserId(User); - await _policyService.SaveAsync(policy, _userService, _organizationService, userId); + await _policyService.SaveAsync(policy, _organizationService, userId); return new PolicyResponseModel(policy); } } diff --git a/src/Api/AdminConsole/Public/Controllers/MembersController.cs b/src/Api/AdminConsole/Public/Controllers/MembersController.cs index a737f0b49..0eb5409ee 100644 --- a/src/Api/AdminConsole/Public/Controllers/MembersController.cs +++ b/src/Api/AdminConsole/Public/Controllers/MembersController.cs @@ -2,12 +2,10 @@ using Bit.Api.AdminConsole.Public.Models.Request; using Bit.Api.AdminConsole.Public.Models.Response; using Bit.Api.Models.Public.Response; -using Bit.Core; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.AdminConsole.Repositories; using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; using Bit.Core.Context; -using Bit.Core.Models.Data.Organizations.OrganizationUsers; using Bit.Core.Repositories; using Bit.Core.Services; using Microsoft.AspNetCore.Authorization; @@ -29,7 +27,6 @@ public class MembersController : Controller private readonly IApplicationCacheService _applicationCacheService; private readonly IPaymentService _paymentService; private readonly IOrganizationRepository _organizationRepository; - private readonly IFeatureService _featureService; private readonly ITwoFactorIsEnabledQuery _twoFactorIsEnabledQuery; public MembersController( @@ -43,7 +40,6 @@ public class MembersController : Controller IApplicationCacheService applicationCacheService, IPaymentService paymentService, IOrganizationRepository organizationRepository, - IFeatureService featureService, ITwoFactorIsEnabledQuery twoFactorIsEnabledQuery) { _organizationUserRepository = organizationUserRepository; @@ -56,7 +52,6 @@ public class MembersController : Controller _applicationCacheService = applicationCacheService; _paymentService = paymentService; _organizationRepository = organizationRepository; - _featureService = featureService; _twoFactorIsEnabledQuery = twoFactorIsEnabledQuery; } @@ -120,16 +115,11 @@ public class MembersController : Controller var organizationUserUserDetails = await _organizationUserRepository.GetManyDetailsByOrganizationAsync(_currentContext.OrganizationId.Value); // TODO: Get all CollectionUser associations for the organization and marry them up here for the response. - if (_featureService.IsEnabled(FeatureFlagKeys.MembersTwoFAQueryOptimization)) + var orgUsersTwoFactorIsEnabled = await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(organizationUserUserDetails); + var memberResponses = organizationUserUserDetails.Select(u => { - return await List_vNext(organizationUserUserDetails); - } - - var memberResponsesTasks = organizationUserUserDetails.Select(async u => - { - return new MemberResponseModel(u, await _userService.TwoFactorIsEnabledAsync(u), null); + return new MemberResponseModel(u, orgUsersTwoFactorIsEnabled.FirstOrDefault(tuple => tuple.user == u).twoFactorIsEnabled, null); }); - var memberResponses = await Task.WhenAll(memberResponsesTasks); var response = new ListResponseModel(memberResponses); return new JsonResult(response); } @@ -268,15 +258,4 @@ public class MembersController : Controller await _organizationService.ResendInviteAsync(_currentContext.OrganizationId.Value, null, id); return new OkResult(); } - - private async Task List_vNext(ICollection organizationUserUserDetails) - { - var orgUsersTwoFactorIsEnabled = await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(organizationUserUserDetails); - var memberResponses = organizationUserUserDetails.Select(u => - { - return new MemberResponseModel(u, orgUsersTwoFactorIsEnabled.FirstOrDefault(tuple => tuple.user == u).twoFactorIsEnabled, null); - }); - var response = new ListResponseModel(memberResponses); - return new JsonResult(response); - } } diff --git a/src/Api/AdminConsole/Public/Controllers/PoliciesController.cs b/src/Api/AdminConsole/Public/Controllers/PoliciesController.cs index 6af83e57d..2d83bd705 100644 --- a/src/Api/AdminConsole/Public/Controllers/PoliciesController.cs +++ b/src/Api/AdminConsole/Public/Controllers/PoliciesController.cs @@ -18,20 +18,17 @@ public class PoliciesController : Controller { private readonly IPolicyRepository _policyRepository; private readonly IPolicyService _policyService; - private readonly IUserService _userService; private readonly IOrganizationService _organizationService; private readonly ICurrentContext _currentContext; public PoliciesController( IPolicyRepository policyRepository, IPolicyService policyService, - IUserService userService, IOrganizationService organizationService, ICurrentContext currentContext) { _policyRepository = policyRepository; _policyService = policyService; - _userService = userService; _organizationService = organizationService; _currentContext = currentContext; } @@ -99,7 +96,7 @@ public class PoliciesController : Controller { policy = model.ToPolicy(policy); } - await _policyService.SaveAsync(policy, _userService, _organizationService, null); + await _policyService.SaveAsync(policy, _organizationService, null); var response = new PolicyResponseModel(policy); return new JsonResult(response); } diff --git a/src/Core/AdminConsole/Services/IOrganizationService.cs b/src/Core/AdminConsole/Services/IOrganizationService.cs index aaa2f86c8..a406d75fb 100644 --- a/src/Core/AdminConsole/Services/IOrganizationService.cs +++ b/src/Core/AdminConsole/Services/IOrganizationService.cs @@ -49,11 +49,8 @@ public interface IOrganizationService IEnumerable<(OrganizationUserInvite invite, string externalId)> invites); Task>> ResendInvitesAsync(Guid organizationId, Guid? invitingUserId, IEnumerable organizationUsersId); Task ResendInviteAsync(Guid organizationId, Guid? invitingUserId, Guid organizationUserId, bool initOrganization = false); - Task ConfirmUserAsync(Guid organizationId, Guid organizationUserId, string key, - Guid confirmingUserId, IUserService userService); + Task ConfirmUserAsync(Guid organizationId, Guid organizationUserId, string key, Guid confirmingUserId); Task>> ConfirmUsersAsync(Guid organizationId, Dictionary keys, - Guid confirmingUserId, IUserService userService); - Task>> ConfirmUsersAsync_vNext(Guid organizationId, Dictionary keys, Guid confirmingUserId); [Obsolete("IRemoveOrganizationUserCommand should be used instead. To be removed by EC-607.")] Task RemoveUserAsync(Guid organizationId, Guid organizationUserId, Guid? deletingUserId); @@ -73,8 +70,8 @@ public interface IOrganizationService Task RevokeUserAsync(OrganizationUser organizationUser, EventSystemUser systemUser); Task>> RevokeUsersAsync(Guid organizationId, IEnumerable organizationUserIds, Guid? revokingUserId); - Task RestoreUserAsync(OrganizationUser organizationUser, Guid? restoringUserId, IUserService userService); - Task RestoreUserAsync(OrganizationUser organizationUser, EventSystemUser systemUser, IUserService userService); + Task RestoreUserAsync(OrganizationUser organizationUser, Guid? restoringUserId); + Task RestoreUserAsync(OrganizationUser organizationUser, EventSystemUser systemUser); Task>> RestoreUsersAsync(Guid organizationId, IEnumerable organizationUserIds, Guid? restoringUserId, IUserService userService); Task CreatePendingOrganization(Organization organization, string ownerEmail, ClaimsPrincipal user, IUserService userService, bool salesAssistedTrialStarted); diff --git a/src/Core/AdminConsole/Services/IPolicyService.cs b/src/Core/AdminConsole/Services/IPolicyService.cs index e2f2fa794..6d92a3a4f 100644 --- a/src/Core/AdminConsole/Services/IPolicyService.cs +++ b/src/Core/AdminConsole/Services/IPolicyService.cs @@ -10,8 +10,7 @@ namespace Bit.Core.AdminConsole.Services; public interface IPolicyService { - Task SaveAsync(Policy policy, IUserService userService, IOrganizationService organizationService, - Guid? savingUserId); + Task SaveAsync(Policy policy, IOrganizationService organizationService, Guid? savingUserId); /// /// Get the combined master password policy options for the specified user. diff --git a/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs b/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs index 5c3f81cee..86854503c 100644 --- a/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs +++ b/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs @@ -1323,13 +1323,12 @@ public class OrganizationService : IOrganizationService } public async Task ConfirmUserAsync(Guid organizationId, Guid organizationUserId, string key, - Guid confirmingUserId, IUserService userService) + Guid confirmingUserId) { - var result = _featureService.IsEnabled(FeatureFlagKeys.MembersTwoFAQueryOptimization) - ? await ConfirmUsersAsync_vNext(organizationId, new Dictionary() { { organizationUserId, key } }, - confirmingUserId) - : await ConfirmUsersAsync(organizationId, new Dictionary() { { organizationUserId, key } }, - confirmingUserId, userService); + var result = await ConfirmUsersAsync( + organizationId, + new Dictionary() { { organizationUserId, key } }, + confirmingUserId); if (!result.Any()) { @@ -1345,75 +1344,6 @@ public class OrganizationService : IOrganizationService } public async Task>> ConfirmUsersAsync(Guid organizationId, Dictionary keys, - Guid confirmingUserId, IUserService userService) - { - var organizationUsers = await _organizationUserRepository.GetManyAsync(keys.Keys); - var validOrganizationUsers = organizationUsers - .Where(u => u.Status == OrganizationUserStatusType.Accepted && u.OrganizationId == organizationId && u.UserId != null) - .ToList(); - - if (!validOrganizationUsers.Any()) - { - return new List>(); - } - - var validOrganizationUserIds = validOrganizationUsers.Select(u => u.UserId.Value).ToList(); - - var organization = await GetOrgById(organizationId); - var usersOrgs = await _organizationUserRepository.GetManyByManyUsersAsync(validOrganizationUserIds); - var users = await _userRepository.GetManyAsync(validOrganizationUserIds); - - var keyedFilteredUsers = validOrganizationUsers.ToDictionary(u => u.UserId.Value, u => u); - var keyedOrganizationUsers = usersOrgs.GroupBy(u => u.UserId.Value) - .ToDictionary(u => u.Key, u => u.ToList()); - - var succeededUsers = new List(); - var result = new List>(); - - foreach (var user in users) - { - if (!keyedFilteredUsers.ContainsKey(user.Id)) - { - continue; - } - var orgUser = keyedFilteredUsers[user.Id]; - var orgUsers = keyedOrganizationUsers.GetValueOrDefault(user.Id, new List()); - try - { - if (organization.PlanType == PlanType.Free && (orgUser.Type == OrganizationUserType.Admin - || orgUser.Type == OrganizationUserType.Owner)) - { - // Since free organizations only supports a few users there is not much point in avoiding N+1 queries for this. - var adminCount = await _organizationUserRepository.GetCountByFreeOrganizationAdminUserAsync(user.Id); - if (adminCount > 0) - { - throw new BadRequestException("User can only be an admin of one free organization."); - } - } - - await CheckPolicies(organizationId, user, orgUsers, userService); - orgUser.Status = OrganizationUserStatusType.Confirmed; - orgUser.Key = keys[orgUser.Id]; - orgUser.Email = null; - - await _eventService.LogOrganizationUserEventAsync(orgUser, EventType.OrganizationUser_Confirmed); - await _mailService.SendOrganizationConfirmedEmailAsync(organization.DisplayName(), user.Email, orgUser.AccessSecretsManager); - await DeleteAndPushUserRegistrationAsync(organizationId, user.Id); - succeededUsers.Add(orgUser); - result.Add(Tuple.Create(orgUser, "")); - } - catch (BadRequestException e) - { - result.Add(Tuple.Create(orgUser, e.Message)); - } - } - - await _organizationUserRepository.ReplaceManyAsync(succeededUsers); - - return result; - } - - public async Task>> ConfirmUsersAsync_vNext(Guid organizationId, Dictionary keys, Guid confirmingUserId) { var selectedOrganizationUsers = await _organizationUserRepository.GetManyAsync(keys.Keys); @@ -1430,7 +1360,7 @@ public class OrganizationService : IOrganizationService var organization = await GetOrgById(organizationId); var allUsersOrgs = await _organizationUserRepository.GetManyByManyUsersAsync(validSelectedUserIds); - var users = await _userRepository.GetManyWithCalculatedPremiumAsync(validSelectedUserIds); + var users = await _userRepository.GetManyAsync(validSelectedUserIds); var usersTwoFactorEnabled = await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(validSelectedUserIds); var keyedFilteredUsers = validSelectedOrganizationUsers.ToDictionary(u => u.UserId.Value, u => u); @@ -1462,7 +1392,7 @@ public class OrganizationService : IOrganizationService } var twoFactorEnabled = usersTwoFactorEnabled.FirstOrDefault(tuple => tuple.userId == user.Id).twoFactorIsEnabled; - await CheckPolicies_vNext(organizationId, user, orgUsers, twoFactorEnabled); + await CheckPoliciesAsync(organizationId, user, orgUsers, twoFactorEnabled); orgUser.Status = OrganizationUserStatusType.Confirmed; orgUser.Key = keys[orgUser.Id]; orgUser.Email = null; @@ -1567,33 +1497,7 @@ public class OrganizationService : IOrganizationService } } - private async Task CheckPolicies(Guid organizationId, User user, - ICollection userOrgs, IUserService userService) - { - // Enforce Two Factor Authentication Policy for this organization - var orgRequiresTwoFactor = (await _policyService.GetPoliciesApplicableToUserAsync(user.Id, PolicyType.TwoFactorAuthentication)).Any(p => p.OrganizationId == organizationId); - if (orgRequiresTwoFactor && !await userService.TwoFactorIsEnabledAsync(user)) - { - throw new BadRequestException("User does not have two-step login enabled."); - } - - var hasOtherOrgs = userOrgs.Any(ou => ou.OrganizationId != organizationId); - var singleOrgPolicies = await _policyService.GetPoliciesApplicableToUserAsync(user.Id, PolicyType.SingleOrg); - var otherSingleOrgPolicies = - singleOrgPolicies.Where(p => p.OrganizationId != organizationId); - // Enforce Single Organization Policy for this organization - if (hasOtherOrgs && singleOrgPolicies.Any(p => p.OrganizationId == organizationId)) - { - throw new BadRequestException("Cannot confirm this member to the organization until they leave or remove all other organizations."); - } - // Enforce Single Organization Policy of other organizations user is a member of - if (otherSingleOrgPolicies.Any()) - { - throw new BadRequestException("Cannot confirm this member to the organization because they are in another organization which forbids it."); - } - } - - private async Task CheckPolicies_vNext(Guid organizationId, UserWithCalculatedPremium user, + private async Task CheckPoliciesAsync(Guid organizationId, User user, ICollection userOrgs, bool twoFactorEnabled) { // Enforce Two Factor Authentication Policy for this organization @@ -2438,8 +2342,7 @@ public class OrganizationService : IOrganizationService return result; } - public async Task RestoreUserAsync(OrganizationUser organizationUser, Guid? restoringUserId, - IUserService userService) + public async Task RestoreUserAsync(OrganizationUser organizationUser, Guid? restoringUserId) { if (restoringUserId.HasValue && organizationUser.UserId == restoringUserId.Value) { @@ -2452,18 +2355,17 @@ public class OrganizationService : IOrganizationService throw new BadRequestException("Only owners can restore other owners."); } - await RepositoryRestoreUserAsync(organizationUser, userService); + await RepositoryRestoreUserAsync(organizationUser); await _eventService.LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Restored); } - public async Task RestoreUserAsync(OrganizationUser organizationUser, EventSystemUser systemUser, - IUserService userService) + public async Task RestoreUserAsync(OrganizationUser organizationUser, EventSystemUser systemUser) { - await RepositoryRestoreUserAsync(organizationUser, userService); + await RepositoryRestoreUserAsync(organizationUser); await _eventService.LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Restored, systemUser); } - private async Task RepositoryRestoreUserAsync(OrganizationUser organizationUser, IUserService userService) + private async Task RepositoryRestoreUserAsync(OrganizationUser organizationUser) { if (organizationUser.Status != OrganizationUserStatusType.Revoked) { @@ -2478,21 +2380,14 @@ public class OrganizationService : IOrganizationService await AutoAddSeatsAsync(organization, 1); } - if (_featureService.IsEnabled(FeatureFlagKeys.MembersTwoFAQueryOptimization)) + var userTwoFactorIsEnabled = false; + // Only check Two Factor Authentication status if the user is linked to a user account + if (organizationUser.UserId.HasValue) { - var userTwoFactorIsEnabled = false; - // Only check Two Factor Authentication status if the user is linked to a user account - if (organizationUser.UserId.HasValue) - { - userTwoFactorIsEnabled = (await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(new[] { organizationUser.UserId.Value })).FirstOrDefault().twoFactorIsEnabled; - } + userTwoFactorIsEnabled = (await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(new[] { organizationUser.UserId.Value })).FirstOrDefault().twoFactorIsEnabled; + } - await CheckPoliciesBeforeRestoreAsync_vNext(organizationUser, userTwoFactorIsEnabled); - } - else - { - await CheckPoliciesBeforeRestoreAsync(organizationUser, userService); - } + await CheckPoliciesBeforeRestoreAsync(organizationUser, userTwoFactorIsEnabled); var status = GetPriorActiveOrganizationUserStatusType(organizationUser); @@ -2526,11 +2421,7 @@ public class OrganizationService : IOrganizationService // Query Two Factor Authentication status for all users in the organization // This is an optimization to avoid querying the Two Factor Authentication status for each user individually - IEnumerable<(Guid userId, bool twoFactorIsEnabled)> organizationUsersTwoFactorEnabled = null; - if (_featureService.IsEnabled(FeatureFlagKeys.MembersTwoFAQueryOptimization)) - { - organizationUsersTwoFactorEnabled = await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(filteredUsers.Select(ou => ou.UserId.Value)); - } + var organizationUsersTwoFactorEnabled = await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(filteredUsers.Select(ou => ou.UserId.Value)); var result = new List>(); @@ -2553,15 +2444,8 @@ public class OrganizationService : IOrganizationService throw new BadRequestException("Only owners can restore other owners."); } - if (_featureService.IsEnabled(FeatureFlagKeys.MembersTwoFAQueryOptimization)) - { - var twoFactorIsEnabled = organizationUsersTwoFactorEnabled.FirstOrDefault(ou => ou.userId == organizationUser.UserId.Value).twoFactorIsEnabled; - await CheckPoliciesBeforeRestoreAsync_vNext(organizationUser, twoFactorIsEnabled); - } - else - { - await CheckPoliciesBeforeRestoreAsync(organizationUser, userService); - } + var twoFactorIsEnabled = organizationUsersTwoFactorEnabled.FirstOrDefault(ou => ou.userId == organizationUser.UserId.Value).twoFactorIsEnabled; + await CheckPoliciesBeforeRestoreAsync(organizationUser, twoFactorIsEnabled); var status = GetPriorActiveOrganizationUserStatusType(organizationUser); @@ -2580,54 +2464,7 @@ public class OrganizationService : IOrganizationService return result; } - private async Task CheckPoliciesBeforeRestoreAsync(OrganizationUser orgUser, IUserService userService) - { - // An invited OrganizationUser isn't linked with a user account yet, so these checks are irrelevant - // The user will be subject to the same checks when they try to accept the invite - if (GetPriorActiveOrganizationUserStatusType(orgUser) == OrganizationUserStatusType.Invited) - { - return; - } - - var userId = orgUser.UserId.Value; - - // Enforce Single Organization Policy of organization user is being restored to - var allOrgUsers = await _organizationUserRepository.GetManyByUserAsync(userId); - var hasOtherOrgs = allOrgUsers.Any(ou => ou.OrganizationId != orgUser.OrganizationId); - var singleOrgPoliciesApplyingToRevokedUsers = await _policyService.GetPoliciesApplicableToUserAsync(userId, - PolicyType.SingleOrg, OrganizationUserStatusType.Revoked); - var singleOrgPolicyApplies = singleOrgPoliciesApplyingToRevokedUsers.Any(p => p.OrganizationId == orgUser.OrganizationId); - - if (hasOtherOrgs && singleOrgPolicyApplies) - { - throw new BadRequestException("You cannot restore this user until " + - "they leave or remove all other organizations."); - } - - // Enforce Single Organization Policy of other organizations user is a member of - var anySingleOrgPolicies = await _policyService.AnyPoliciesApplicableToUserAsync(userId, - PolicyType.SingleOrg); - if (anySingleOrgPolicies) - { - throw new BadRequestException("You cannot restore this user because they are a member of " + - "another organization which forbids it"); - } - - // Enforce Two Factor Authentication Policy of organization user is trying to join - var user = await _userRepository.GetByIdAsync(userId); - if (!await userService.TwoFactorIsEnabledAsync(user)) - { - var invitedTwoFactorPolicies = await _policyService.GetPoliciesApplicableToUserAsync(userId, - PolicyType.TwoFactorAuthentication, OrganizationUserStatusType.Invited); - if (invitedTwoFactorPolicies.Any(p => p.OrganizationId == orgUser.OrganizationId)) - { - throw new BadRequestException("You cannot restore this user until they enable " + - "two-step login on their user account."); - } - } - } - - private async Task CheckPoliciesBeforeRestoreAsync_vNext(OrganizationUser orgUser, bool userHasTwoFactorEnabled) + private async Task CheckPoliciesBeforeRestoreAsync(OrganizationUser orgUser, bool userHasTwoFactorEnabled) { // An invited OrganizationUser isn't linked with a user account yet, so these checks are irrelevant // The user will be subject to the same checks when they try to accept the invite diff --git a/src/Core/AdminConsole/Services/Implementations/PolicyService.cs b/src/Core/AdminConsole/Services/Implementations/PolicyService.cs index 1ffa2a0e2..fab32aaff 100644 --- a/src/Core/AdminConsole/Services/Implementations/PolicyService.cs +++ b/src/Core/AdminConsole/Services/Implementations/PolicyService.cs @@ -25,7 +25,6 @@ public class PolicyService : IPolicyService private readonly ISsoConfigRepository _ssoConfigRepository; private readonly IMailService _mailService; private readonly GlobalSettings _globalSettings; - private readonly IFeatureService _featureService; private readonly ITwoFactorIsEnabledQuery _twoFactorIsEnabledQuery; public PolicyService( @@ -37,7 +36,6 @@ public class PolicyService : IPolicyService ISsoConfigRepository ssoConfigRepository, IMailService mailService, GlobalSettings globalSettings, - IFeatureService featureService, ITwoFactorIsEnabledQuery twoFactorIsEnabledQuery) { _applicationCacheService = applicationCacheService; @@ -48,12 +46,10 @@ public class PolicyService : IPolicyService _ssoConfigRepository = ssoConfigRepository; _mailService = mailService; _globalSettings = globalSettings; - _featureService = featureService; _twoFactorIsEnabledQuery = twoFactorIsEnabledQuery; } - public async Task SaveAsync(Policy policy, IUserService userService, IOrganizationService organizationService, - Guid? savingUserId) + public async Task SaveAsync(Policy policy, IOrganizationService organizationService, Guid? savingUserId) { var org = await _organizationRepository.GetByIdAsync(policy.OrganizationId); if (org == null) @@ -88,14 +84,7 @@ public class PolicyService : IPolicyService return; } - if (_featureService.IsEnabled(FeatureFlagKeys.MembersTwoFAQueryOptimization)) - { - await EnablePolicy_vNext(policy, org, organizationService, savingUserId); - return; - } - - await EnablePolicy(policy, org, userService, organizationService, savingUserId); - return; + await EnablePolicyAsync(policy, org, organizationService, savingUserId); } public async Task GetMasterPasswordPolicyForUserAsync(User user) @@ -269,62 +258,7 @@ public class PolicyService : IPolicyService await _eventService.LogPolicyEventAsync(policy, EventType.Policy_Updated); } - private async Task EnablePolicy(Policy policy, Organization org, IUserService userService, IOrganizationService organizationService, Guid? savingUserId) - { - var currentPolicy = await _policyRepository.GetByIdAsync(policy.Id); - if (!currentPolicy?.Enabled ?? true) - { - var orgUsers = await _organizationUserRepository.GetManyDetailsByOrganizationAsync(policy.OrganizationId); - var removableOrgUsers = orgUsers.Where(ou => - ou.Status != OrganizationUserStatusType.Invited && ou.Status != OrganizationUserStatusType.Revoked && - ou.Type != OrganizationUserType.Owner && ou.Type != OrganizationUserType.Admin && - ou.UserId != savingUserId); - switch (policy.Type) - { - case PolicyType.TwoFactorAuthentication: - // Reorder by HasMasterPassword to prioritize checking users without a master if they have 2FA enabled - foreach (var orgUser in removableOrgUsers.OrderBy(ou => ou.HasMasterPassword)) - { - if (!await userService.TwoFactorIsEnabledAsync(orgUser)) - { - if (!orgUser.HasMasterPassword) - { - throw new BadRequestException( - "Policy could not be enabled. Non-compliant members will lose access to their accounts. Identify members without two-step login from the policies column in the members page."); - } - - await organizationService.RemoveUserAsync(policy.OrganizationId, orgUser.Id, - savingUserId); - await _mailService.SendOrganizationUserRemovedForPolicyTwoStepEmailAsync( - org.DisplayName(), orgUser.Email); - } - } - break; - case PolicyType.SingleOrg: - var userOrgs = await _organizationUserRepository.GetManyByManyUsersAsync( - removableOrgUsers.Select(ou => ou.UserId.Value)); - foreach (var orgUser in removableOrgUsers) - { - if (userOrgs.Any(ou => ou.UserId == orgUser.UserId - && ou.OrganizationId != org.Id - && ou.Status != OrganizationUserStatusType.Invited)) - { - await organizationService.RemoveUserAsync(policy.OrganizationId, orgUser.Id, - savingUserId); - await _mailService.SendOrganizationUserRemovedForPolicySingleOrgEmailAsync( - org.DisplayName(), orgUser.Email); - } - } - break; - default: - break; - } - } - - await SetPolicyConfiguration(policy); - } - - private async Task EnablePolicy_vNext(Policy policy, Organization org, IOrganizationService organizationService, Guid? savingUserId) + private async Task EnablePolicyAsync(Policy policy, Organization org, IOrganizationService organizationService, Guid? savingUserId) { var currentPolicy = await _policyRepository.GetByIdAsync(policy.Id); if (!currentPolicy?.Enabled ?? true) diff --git a/src/Core/Auth/Services/Implementations/SsoConfigService.cs b/src/Core/Auth/Services/Implementations/SsoConfigService.cs index 62c828495..fdf7e278e 100644 --- a/src/Core/Auth/Services/Implementations/SsoConfigService.cs +++ b/src/Core/Auth/Services/Implementations/SsoConfigService.cs @@ -20,7 +20,6 @@ public class SsoConfigService : ISsoConfigService private readonly IPolicyService _policyService; private readonly IOrganizationRepository _organizationRepository; private readonly IOrganizationUserRepository _organizationUserRepository; - private readonly IUserService _userService; private readonly IOrganizationService _organizationService; private readonly IEventService _eventService; @@ -30,7 +29,6 @@ public class SsoConfigService : ISsoConfigService IPolicyService policyService, IOrganizationRepository organizationRepository, IOrganizationUserRepository organizationUserRepository, - IUserService userService, IOrganizationService organizationService, IEventService eventService) { @@ -39,7 +37,6 @@ public class SsoConfigService : ISsoConfigService _policyService = policyService; _organizationRepository = organizationRepository; _organizationUserRepository = organizationUserRepository; - _userService = userService; _organizationService = organizationService; _eventService = eventService; } @@ -74,20 +71,20 @@ public class SsoConfigService : ISsoConfigService singleOrgPolicy.Enabled = true; - await _policyService.SaveAsync(singleOrgPolicy, _userService, _organizationService, null); + await _policyService.SaveAsync(singleOrgPolicy, _organizationService, null); var resetPolicy = await _policyRepository.GetByOrganizationIdTypeAsync(config.OrganizationId, PolicyType.ResetPassword) ?? new Policy { OrganizationId = config.OrganizationId, Type = PolicyType.ResetPassword, }; resetPolicy.Enabled = true; resetPolicy.SetDataModel(new ResetPasswordDataModel { AutoEnrollEnabled = true }); - await _policyService.SaveAsync(resetPolicy, _userService, _organizationService, null); + await _policyService.SaveAsync(resetPolicy, _organizationService, null); var ssoRequiredPolicy = await _policyRepository.GetByOrganizationIdTypeAsync(config.OrganizationId, PolicyType.RequireSso) ?? new Policy { OrganizationId = config.OrganizationId, Type = PolicyType.RequireSso, }; ssoRequiredPolicy.Enabled = true; - await _policyService.SaveAsync(ssoRequiredPolicy, _userService, _organizationService, null); + await _policyService.SaveAsync(ssoRequiredPolicy, _organizationService, null); } await LogEventsAsync(config, oldConfig); diff --git a/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs b/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs index 1193b2de8..357222669 100644 --- a/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs +++ b/test/Core.Test/AdminConsole/Services/OrganizationServiceTests.cs @@ -7,7 +7,6 @@ using Bit.Core.AdminConsole.Repositories; using Bit.Core.AdminConsole.Services; using Bit.Core.Auth.Entities; using Bit.Core.Auth.Enums; -using Bit.Core.Auth.Models; using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Auth.Models.Data; using Bit.Core.Auth.Repositories; @@ -1378,12 +1377,11 @@ OrganizationUserInvite invite, SutProvider sutProvider) SutProvider sutProvider) { var organizationUserRepository = sutProvider.GetDependency(); - var userService = Substitute.For(); organizationUserRepository.GetByIdAsync(orgUser.Id).Returns(orgUser); var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.ConfirmUserAsync(orgUser.OrganizationId, orgUser.Id, key, confirmingUser.Id, userService)); + () => sutProvider.Sut.ConfirmUserAsync(orgUser.OrganizationId, orgUser.Id, key, confirmingUser.Id)); Assert.Contains("User not valid.", exception.Message); } @@ -1393,12 +1391,11 @@ OrganizationUserInvite invite, SutProvider sutProvider) SutProvider sutProvider) { var organizationUserRepository = sutProvider.GetDependency(); - var userService = Substitute.For(); organizationUserRepository.GetByIdAsync(orgUser.Id).Returns(orgUser); var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.ConfirmUserAsync(confirmingUser.OrganizationId, orgUser.Id, key, confirmingUser.Id, userService)); + () => sutProvider.Sut.ConfirmUserAsync(confirmingUser.OrganizationId, orgUser.Id, key, confirmingUser.Id)); Assert.Contains("User not valid.", exception.Message); } @@ -1411,7 +1408,6 @@ OrganizationUserInvite invite, SutProvider sutProvider) { var organizationUserRepository = sutProvider.GetDependency(); var organizationRepository = sutProvider.GetDependency(); - var userService = Substitute.For(); var userRepository = sutProvider.GetDependency(); org.PlanType = PlanType.Free; @@ -1424,7 +1420,7 @@ OrganizationUserInvite invite, SutProvider sutProvider) userRepository.GetManyAsync(default).ReturnsForAnyArgs(new[] { user }); var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.ConfirmUserAsync(orgUser.OrganizationId, orgUser.Id, key, confirmingUser.Id, userService)); + () => sutProvider.Sut.ConfirmUserAsync(orgUser.OrganizationId, orgUser.Id, key, confirmingUser.Id)); Assert.Contains("User can only be an admin of one free organization.", exception.Message); } @@ -1465,7 +1461,6 @@ OrganizationUserInvite invite, SutProvider sutProvider) { var organizationUserRepository = sutProvider.GetDependency(); var organizationRepository = sutProvider.GetDependency(); - var userService = Substitute.For(); var userRepository = sutProvider.GetDependency(); org.PlanType = planType; @@ -1478,7 +1473,7 @@ OrganizationUserInvite invite, SutProvider sutProvider) organizationRepository.GetByIdAsync(org.Id).Returns(org); userRepository.GetManyAsync(default).ReturnsForAnyArgs(new[] { user }); - await sutProvider.Sut.ConfirmUserAsync(orgUser.OrganizationId, orgUser.Id, key, confirmingUser.Id, userService); + await sutProvider.Sut.ConfirmUserAsync(orgUser.OrganizationId, orgUser.Id, key, confirmingUser.Id); await sutProvider.GetDependency().Received(1).LogOrganizationUserEventAsync(orgUser, EventType.OrganizationUser_Confirmed); await sutProvider.GetDependency().Received(1).SendOrganizationConfirmedEmailAsync(org.DisplayName(), user.Email); @@ -1496,7 +1491,6 @@ OrganizationUserInvite invite, SutProvider sutProvider) var organizationRepository = sutProvider.GetDependency(); var userRepository = sutProvider.GetDependency(); var policyService = sutProvider.GetDependency(); - var userService = Substitute.For(); org.PlanType = PlanType.EnterpriseAnnually; orgUser.Status = OrganizationUserStatusType.Accepted; @@ -1510,7 +1504,7 @@ OrganizationUserInvite invite, SutProvider sutProvider) policyService.GetPoliciesApplicableToUserAsync(user.Id, PolicyType.SingleOrg).Returns(new[] { singleOrgPolicy }); var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.ConfirmUserAsync(orgUser.OrganizationId, orgUser.Id, key, confirmingUser.Id, userService)); + () => sutProvider.Sut.ConfirmUserAsync(orgUser.OrganizationId, orgUser.Id, key, confirmingUser.Id)); Assert.Contains("Cannot confirm this member to the organization until they leave or remove all other organizations.", exception.Message); } @@ -1524,7 +1518,6 @@ OrganizationUserInvite invite, SutProvider sutProvider) var organizationRepository = sutProvider.GetDependency(); var userRepository = sutProvider.GetDependency(); var policyService = sutProvider.GetDependency(); - var userService = Substitute.For(); org.PlanType = PlanType.EnterpriseAnnually; orgUser.Status = OrganizationUserStatusType.Accepted; @@ -1538,7 +1531,7 @@ OrganizationUserInvite invite, SutProvider sutProvider) policyService.GetPoliciesApplicableToUserAsync(user.Id, PolicyType.SingleOrg).Returns(new[] { singleOrgPolicy }); var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.ConfirmUserAsync(orgUser.OrganizationId, orgUser.Id, key, confirmingUser.Id, userService)); + () => sutProvider.Sut.ConfirmUserAsync(orgUser.OrganizationId, orgUser.Id, key, confirmingUser.Id)); Assert.Contains("Cannot confirm this member to the organization because they are in another organization which forbids it.", exception.Message); } @@ -1554,7 +1547,6 @@ OrganizationUserInvite invite, SutProvider sutProvider) var organizationUserRepository = sutProvider.GetDependency(); var organizationRepository = sutProvider.GetDependency(); var userRepository = sutProvider.GetDependency(); - var userService = Substitute.For(); org.PlanType = PlanType.EnterpriseAnnually; orgUser.Type = userType; @@ -1567,7 +1559,7 @@ OrganizationUserInvite invite, SutProvider sutProvider) organizationRepository.GetByIdAsync(org.Id).Returns(org); userRepository.GetManyAsync(default).ReturnsForAnyArgs(new[] { user }); - await sutProvider.Sut.ConfirmUserAsync(orgUser.OrganizationId, orgUser.Id, key, confirmingUser.Id, userService); + await sutProvider.Sut.ConfirmUserAsync(orgUser.OrganizationId, orgUser.Id, key, confirmingUser.Id); await sutProvider.GetDependency().Received(1).LogOrganizationUserEventAsync(orgUser, EventType.OrganizationUser_Confirmed); await sutProvider.GetDependency().Received(1).SendOrganizationConfirmedEmailAsync(org.DisplayName(), user.Email, true); @@ -1585,7 +1577,7 @@ OrganizationUserInvite invite, SutProvider sutProvider) var organizationRepository = sutProvider.GetDependency(); var userRepository = sutProvider.GetDependency(); var policyService = sutProvider.GetDependency(); - var userService = Substitute.For(); + var twoFactorIsEnabledQuery = sutProvider.GetDependency(); org.PlanType = PlanType.EnterpriseAnnually; orgUser.OrganizationId = confirmingUser.OrganizationId = org.Id; @@ -1596,9 +1588,11 @@ OrganizationUserInvite invite, SutProvider sutProvider) userRepository.GetManyAsync(default).ReturnsForAnyArgs(new[] { user }); twoFactorPolicy.OrganizationId = org.Id; policyService.GetPoliciesApplicableToUserAsync(user.Id, PolicyType.TwoFactorAuthentication).Returns(new[] { twoFactorPolicy }); + twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(Arg.Is>(ids => ids.Contains(user.Id))) + .Returns(new List<(Guid userId, bool twoFactorIsEnabled)>() { (user.Id, false) }); var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.ConfirmUserAsync(orgUser.OrganizationId, orgUser.Id, key, confirmingUser.Id, userService)); + () => sutProvider.Sut.ConfirmUserAsync(orgUser.OrganizationId, orgUser.Id, key, confirmingUser.Id)); Assert.Contains("User does not have two-step login enabled.", exception.Message); } @@ -1612,7 +1606,7 @@ OrganizationUserInvite invite, SutProvider sutProvider) var organizationRepository = sutProvider.GetDependency(); var userRepository = sutProvider.GetDependency(); var policyService = sutProvider.GetDependency(); - var userService = Substitute.For(); + var twoFactorIsEnabledQuery = sutProvider.GetDependency(); org.PlanType = PlanType.EnterpriseAnnually; orgUser.OrganizationId = confirmingUser.OrganizationId = org.Id; @@ -1622,71 +1616,10 @@ OrganizationUserInvite invite, SutProvider sutProvider) userRepository.GetManyAsync(default).ReturnsForAnyArgs(new[] { user }); twoFactorPolicy.OrganizationId = org.Id; policyService.GetPoliciesApplicableToUserAsync(user.Id, PolicyType.TwoFactorAuthentication).Returns(new[] { twoFactorPolicy }); - userService.TwoFactorIsEnabledAsync(user).Returns(true); - - await sutProvider.Sut.ConfirmUserAsync(orgUser.OrganizationId, orgUser.Id, key, confirmingUser.Id, userService); - } - - [Theory, BitAutoData] - public async Task ConfirmUser_vNext_TwoFactorPolicy_NotEnabled_Throws(Organization org, OrganizationUser confirmingUser, - [OrganizationUser(OrganizationUserStatusType.Accepted)] OrganizationUser orgUser, UserWithCalculatedPremium user, - OrganizationUser orgUserAnotherOrg, - [OrganizationUserPolicyDetails(PolicyType.TwoFactorAuthentication)] OrganizationUserPolicyDetails twoFactorPolicy, - string key, SutProvider sutProvider) - { - sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.MembersTwoFAQueryOptimization).Returns(true); - - var organizationUserRepository = sutProvider.GetDependency(); - var organizationRepository = sutProvider.GetDependency(); - var userRepository = sutProvider.GetDependency(); - var policyService = sutProvider.GetDependency(); - var userService = Substitute.For(); - var twoFactorIsEnabledQuery = sutProvider.GetDependency(); - - org.PlanType = PlanType.EnterpriseAnnually; - orgUser.OrganizationId = confirmingUser.OrganizationId = org.Id; - orgUser.UserId = orgUserAnotherOrg.UserId = user.Id; - organizationUserRepository.GetManyAsync(default).ReturnsForAnyArgs(new[] { orgUser }); - organizationUserRepository.GetManyByManyUsersAsync(default).ReturnsForAnyArgs(new[] { orgUserAnotherOrg }); - organizationRepository.GetByIdAsync(org.Id).Returns(org); - userRepository.GetManyWithCalculatedPremiumAsync(default).ReturnsForAnyArgs(new[] { user }); - twoFactorPolicy.OrganizationId = org.Id; - policyService.GetPoliciesApplicableToUserAsync(user.Id, PolicyType.TwoFactorAuthentication).Returns(new[] { twoFactorPolicy }); - twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(Arg.Is>(ids => ids.Contains(user.Id))) - .Returns(new List<(Guid userId, bool twoFactorIsEnabled)>() { (user.Id, false) }); - - var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.ConfirmUserAsync(orgUser.OrganizationId, orgUser.Id, key, confirmingUser.Id, userService)); - Assert.Contains("User does not have two-step login enabled.", exception.Message); - } - - [Theory, BitAutoData] - public async Task ConfirmUser_vNext_TwoFactorPolicy_Enabled_Success(Organization org, OrganizationUser confirmingUser, - [OrganizationUser(OrganizationUserStatusType.Accepted)] OrganizationUser orgUser, UserWithCalculatedPremium user, - [OrganizationUserPolicyDetails(PolicyType.TwoFactorAuthentication)] OrganizationUserPolicyDetails twoFactorPolicy, - string key, SutProvider sutProvider) - { - sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.MembersTwoFAQueryOptimization).Returns(true); - - var organizationUserRepository = sutProvider.GetDependency(); - var organizationRepository = sutProvider.GetDependency(); - var userRepository = sutProvider.GetDependency(); - var policyService = sutProvider.GetDependency(); - var userService = Substitute.For(); - var twoFactorIsEnabledQuery = sutProvider.GetDependency(); - - org.PlanType = PlanType.EnterpriseAnnually; - orgUser.OrganizationId = confirmingUser.OrganizationId = org.Id; - orgUser.UserId = user.Id; - organizationUserRepository.GetManyAsync(default).ReturnsForAnyArgs(new[] { orgUser }); - organizationRepository.GetByIdAsync(org.Id).Returns(org); - userRepository.GetManyWithCalculatedPremiumAsync(default).ReturnsForAnyArgs(new[] { user }); - twoFactorPolicy.OrganizationId = org.Id; - policyService.GetPoliciesApplicableToUserAsync(user.Id, PolicyType.TwoFactorAuthentication).Returns(new[] { twoFactorPolicy }); twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(Arg.Is>(ids => ids.Contains(user.Id))) .Returns(new List<(Guid userId, bool twoFactorIsEnabled)>() { (user.Id, true) }); - await sutProvider.Sut.ConfirmUserAsync(orgUser.OrganizationId, orgUser.Id, key, confirmingUser.Id, userService); + await sutProvider.Sut.ConfirmUserAsync(orgUser.OrganizationId, orgUser.Id, key, confirmingUser.Id); } [Theory, BitAutoData] @@ -1704,52 +1637,6 @@ OrganizationUserInvite invite, SutProvider sutProvider) var organizationRepository = sutProvider.GetDependency(); var userRepository = sutProvider.GetDependency(); var policyService = sutProvider.GetDependency(); - var userService = Substitute.For(); - - org.PlanType = PlanType.EnterpriseAnnually; - orgUser1.OrganizationId = orgUser2.OrganizationId = orgUser3.OrganizationId = confirmingUser.OrganizationId = org.Id; - orgUser1.UserId = user1.Id; - orgUser2.UserId = user2.Id; - orgUser3.UserId = user3.Id; - anotherOrgUser.UserId = user3.Id; - var orgUsers = new[] { orgUser1, orgUser2, orgUser3 }; - organizationUserRepository.GetManyAsync(default).ReturnsForAnyArgs(orgUsers); - organizationRepository.GetByIdAsync(org.Id).Returns(org); - userRepository.GetManyAsync(default).ReturnsForAnyArgs(new[] { user1, user2, user3 }); - twoFactorPolicy.OrganizationId = org.Id; - policyService.GetPoliciesApplicableToUserAsync(Arg.Any(), PolicyType.TwoFactorAuthentication).Returns(new[] { twoFactorPolicy }); - userService.TwoFactorIsEnabledAsync(user1).Returns(true); - userService.TwoFactorIsEnabledAsync(user2).Returns(false); - userService.TwoFactorIsEnabledAsync(user3).Returns(true); - singleOrgPolicy.OrganizationId = org.Id; - policyService.GetPoliciesApplicableToUserAsync(user3.Id, PolicyType.SingleOrg) - .Returns(new[] { singleOrgPolicy }); - organizationUserRepository.GetManyByManyUsersAsync(default) - .ReturnsForAnyArgs(new[] { orgUser1, orgUser2, orgUser3, anotherOrgUser }); - - var keys = orgUsers.ToDictionary(ou => ou.Id, _ => key); - var result = await sutProvider.Sut.ConfirmUsersAsync(confirmingUser.OrganizationId, keys, confirmingUser.Id, userService); - Assert.Contains("", result[0].Item2); - Assert.Contains("User does not have two-step login enabled.", result[1].Item2); - Assert.Contains("Cannot confirm this member to the organization until they leave or remove all other organizations.", result[2].Item2); - } - - [Theory, BitAutoData] - public async Task ConfirmUsers_vNext_Success(Organization org, - OrganizationUser confirmingUser, - [OrganizationUser(OrganizationUserStatusType.Accepted)] OrganizationUser orgUser1, - [OrganizationUser(OrganizationUserStatusType.Accepted)] OrganizationUser orgUser2, - [OrganizationUser(OrganizationUserStatusType.Accepted)] OrganizationUser orgUser3, - OrganizationUser anotherOrgUser, UserWithCalculatedPremium user1, UserWithCalculatedPremium user2, UserWithCalculatedPremium user3, - [OrganizationUserPolicyDetails(PolicyType.TwoFactorAuthentication)] OrganizationUserPolicyDetails twoFactorPolicy, - [OrganizationUserPolicyDetails(PolicyType.SingleOrg)] OrganizationUserPolicyDetails singleOrgPolicy, - string key, SutProvider sutProvider) - { - var organizationUserRepository = sutProvider.GetDependency(); - var organizationRepository = sutProvider.GetDependency(); - var userRepository = sutProvider.GetDependency(); - var policyService = sutProvider.GetDependency(); - var userService = Substitute.For(); var twoFactorIsEnabledQuery = sutProvider.GetDependency(); org.PlanType = PlanType.EnterpriseAnnually; @@ -1761,7 +1648,7 @@ OrganizationUserInvite invite, SutProvider sutProvider) var orgUsers = new[] { orgUser1, orgUser2, orgUser3 }; organizationUserRepository.GetManyAsync(default).ReturnsForAnyArgs(orgUsers); organizationRepository.GetByIdAsync(org.Id).Returns(org); - userRepository.GetManyWithCalculatedPremiumAsync(default).ReturnsForAnyArgs(new[] { user1, user2, user3 }); + userRepository.GetManyAsync(default).ReturnsForAnyArgs(new[] { user1, user2, user3 }); twoFactorPolicy.OrganizationId = org.Id; policyService.GetPoliciesApplicableToUserAsync(Arg.Any(), PolicyType.TwoFactorAuthentication).Returns(new[] { twoFactorPolicy }); twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(Arg.Is>(ids => ids.Contains(user1.Id) && ids.Contains(user2.Id) && ids.Contains(user3.Id))) @@ -1778,7 +1665,7 @@ OrganizationUserInvite invite, SutProvider sutProvider) .ReturnsForAnyArgs(new[] { orgUser1, orgUser2, orgUser3, anotherOrgUser }); var keys = orgUsers.ToDictionary(ou => ou.Id, _ => key); - var result = await sutProvider.Sut.ConfirmUsersAsync_vNext(confirmingUser.OrganizationId, keys, confirmingUser.Id); + var result = await sutProvider.Sut.ConfirmUsersAsync(confirmingUser.OrganizationId, keys, confirmingUser.Id); Assert.Contains("", result[0].Item2); Assert.Contains("User does not have two-step login enabled.", result[1].Item2); Assert.Contains("Cannot confirm this member to the organization until they leave or remove all other organizations.", result[2].Item2); @@ -2019,11 +1906,10 @@ OrganizationUserInvite invite, SutProvider sutProvider) [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, SutProvider sutProvider) { RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); - var userService = Substitute.For(); var organizationUserRepository = sutProvider.GetDependency(); var eventService = sutProvider.GetDependency(); - await sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id, userService); + await sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id); await organizationUserRepository.Received().RestoreAsync(organizationUser.Id, OrganizationUserStatusType.Invited); await eventService.Received() @@ -2035,11 +1921,10 @@ OrganizationUserInvite invite, SutProvider sutProvider) [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, EventSystemUser eventSystemUser, SutProvider sutProvider) { RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); - var userService = Substitute.For(); var organizationUserRepository = sutProvider.GetDependency(); var eventService = sutProvider.GetDependency(); - await sutProvider.Sut.RestoreUserAsync(organizationUser, eventSystemUser, userService); + await sutProvider.Sut.RestoreUserAsync(organizationUser, eventSystemUser); await organizationUserRepository.Received().RestoreAsync(organizationUser.Id, OrganizationUserStatusType.Invited); await eventService.Received() @@ -2052,12 +1937,11 @@ OrganizationUserInvite invite, SutProvider sutProvider) { organizationUser.UserId = owner.Id; RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); - var userService = Substitute.For(); var organizationUserRepository = sutProvider.GetDependency(); var eventService = sutProvider.GetDependency(); var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id, userService)); + () => sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id)); Assert.Contains("you cannot restore yourself", exception.Message.ToLowerInvariant()); @@ -2074,12 +1958,11 @@ OrganizationUserInvite invite, SutProvider sutProvider) { restoringUser.Type = restoringUserType; RestoreRevokeUser_Setup(organization, restoringUser, organizationUser, sutProvider, OrganizationUserType.Admin); - var userService = Substitute.For(); var organizationUserRepository = sutProvider.GetDependency(); var eventService = sutProvider.GetDependency(); var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.RestoreUserAsync(organizationUser, restoringUser.Id, userService)); + () => sutProvider.Sut.RestoreUserAsync(organizationUser, restoringUser.Id)); Assert.Contains("only owners can restore other owners", exception.Message.ToLowerInvariant()); @@ -2097,12 +1980,11 @@ OrganizationUserInvite invite, SutProvider sutProvider) { organizationUser.Status = userStatus; RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); - var userService = Substitute.For(); var organizationUserRepository = sutProvider.GetDependency(); var eventService = sutProvider.GetDependency(); var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id, userService)); + () => sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id)); Assert.Contains("already active", exception.Message.ToLowerInvariant()); @@ -2111,37 +1993,6 @@ OrganizationUserInvite invite, SutProvider sutProvider) .LogOrganizationUserEventAsync(Arg.Any(), Arg.Any(), Arg.Any()); } - [Theory, BitAutoData] - public async Task RestoreUser_WithSingleOrgPolicyEnabled_Fails( - Organization organization, - [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, - [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, - [OrganizationUser(OrganizationUserStatusType.Accepted)] OrganizationUser secondOrganizationUser, - SutProvider sutProvider) - { - organizationUser.Email = null; // this is required to mock that the user as had already been confirmed before the revoke - secondOrganizationUser.UserId = organizationUser.UserId; - RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); - var userService = Substitute.For(); - var organizationUserRepository = sutProvider.GetDependency(); - var eventService = sutProvider.GetDependency(); - - organizationUserRepository.GetManyByUserAsync(organizationUser.UserId.Value).Returns(new[] { organizationUser, secondOrganizationUser }); - sutProvider.GetDependency() - .GetPoliciesApplicableToUserAsync(organizationUser.UserId.Value, PolicyType.SingleOrg, Arg.Any()) - .Returns(new[] { new OrganizationUserPolicyDetails { OrganizationId = organizationUser.OrganizationId, PolicyType = PolicyType.SingleOrg } }); - - var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id, userService)); - - Assert.Contains("you cannot restore this user until " + - "they leave or remove all other organizations.", exception.Message.ToLowerInvariant()); - - await organizationUserRepository.DidNotReceiveWithAnyArgs().RestoreAsync(Arg.Any(), Arg.Any()); - await eventService.DidNotReceiveWithAnyArgs() - .LogOrganizationUserEventAsync(Arg.Any(), Arg.Any(), Arg.Any()); - } - [Theory, BitAutoData] public async Task RestoreUser_WithOtherOrganizationSingleOrgPolicyEnabled_Fails( Organization organization, @@ -2151,7 +2002,6 @@ OrganizationUserInvite invite, SutProvider sutProvider) { organizationUser.Email = null; // this is required to mock that the user as had already been confirmed before the revoke RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); - var userService = Substitute.For(); var organizationUserRepository = sutProvider.GetDependency(); var eventService = sutProvider.GetDependency(); @@ -2160,7 +2010,7 @@ OrganizationUserInvite invite, SutProvider sutProvider) .Returns(true); var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id, userService)); + () => sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id)); Assert.Contains("you cannot restore this user because they are a member of " + "another organization which forbids it", exception.Message.ToLowerInvariant()); @@ -2182,16 +2032,16 @@ OrganizationUserInvite invite, SutProvider sutProvider) .GetPoliciesApplicableToUserAsync(organizationUser.UserId.Value, PolicyType.TwoFactorAuthentication, Arg.Any()) .Returns(new[] { new OrganizationUserPolicyDetails { OrganizationId = organizationUser.OrganizationId, PolicyType = PolicyType.TwoFactorAuthentication } }); + sutProvider.GetDependency() + .TwoFactorIsEnabledAsync(Arg.Is>(i => i.Contains(organizationUser.UserId.Value))) + .Returns(new List<(Guid userId, bool twoFactorIsEnabled)>() { (organizationUser.UserId.Value, false) }); + RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); - var userService = Substitute.For(); var organizationUserRepository = sutProvider.GetDependency(); var eventService = sutProvider.GetDependency(); - - userService.TwoFactorIsEnabledAsync(Arg.Any()).Returns(false); - var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id, userService)); + () => sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id)); Assert.Contains("you cannot restore this user until they enable " + "two-step login on their user account.", exception.Message.ToLowerInvariant()); @@ -2210,16 +2060,17 @@ OrganizationUserInvite invite, SutProvider sutProvider) { organizationUser.Email = null; // this is required to mock that the user as had already been confirmed before the revoke RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); - var userService = Substitute.For(); var organizationUserRepository = sutProvider.GetDependency(); var eventService = sutProvider.GetDependency(); sutProvider.GetDependency() .GetPoliciesApplicableToUserAsync(organizationUser.UserId.Value, PolicyType.TwoFactorAuthentication, Arg.Any()) .Returns(new[] { new OrganizationUserPolicyDetails { OrganizationId = organizationUser.OrganizationId, PolicyType = PolicyType.TwoFactorAuthentication } }); - userService.TwoFactorIsEnabledAsync(Arg.Any()).Returns(true); + sutProvider.GetDependency() + .TwoFactorIsEnabledAsync(Arg.Is>(i => i.Contains(organizationUser.UserId.Value))) + .Returns(new List<(Guid userId, bool twoFactorIsEnabled)>() { (organizationUser.UserId.Value, true) }); - await sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id, userService); + await sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id); await organizationUserRepository.Received().RestoreAsync(organizationUser.Id, OrganizationUserStatusType.Confirmed); await eventService.Received() @@ -2227,19 +2078,16 @@ OrganizationUserInvite invite, SutProvider sutProvider) } [Theory, BitAutoData] - public async Task RestoreUser_vNext_WithSingleOrgPolicyEnabled_Fails( + public async Task RestoreUser_WithSingleOrgPolicyEnabled_Fails( Organization organization, [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser owner, [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, [OrganizationUser(OrganizationUserStatusType.Accepted)] OrganizationUser secondOrganizationUser, SutProvider sutProvider) { - sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.MembersTwoFAQueryOptimization).Returns(true); - organizationUser.Email = null; // this is required to mock that the user as had already been confirmed before the revoke secondOrganizationUser.UserId = organizationUser.UserId; RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); - var userService = Substitute.For(); var organizationUserRepository = sutProvider.GetDependency(); var eventService = sutProvider.GetDependency(); @@ -2252,7 +2100,7 @@ OrganizationUserInvite invite, SutProvider sutProvider) }); var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id, userService)); + () => sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id)); Assert.Contains("you cannot restore this user until " + "they leave or remove all other organizations.", exception.Message.ToLowerInvariant()); @@ -2270,12 +2118,9 @@ OrganizationUserInvite invite, SutProvider sutProvider) [OrganizationUser(OrganizationUserStatusType.Accepted)] OrganizationUser secondOrganizationUser, SutProvider sutProvider) { - sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.MembersTwoFAQueryOptimization).Returns(true); - organizationUser.Email = null; // this is required to mock that the user as had already been confirmed before the revoke secondOrganizationUser.UserId = organizationUser.UserId; RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); - var userService = Substitute.For(); var organizationUserRepository = sutProvider.GetDependency(); var eventService = sutProvider.GetDependency(); var twoFactorIsEnabledQuery = sutProvider.GetDependency(); @@ -2289,7 +2134,7 @@ OrganizationUserInvite invite, SutProvider sutProvider) .Returns(true); var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id, userService)); + () => sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id)); Assert.Contains("you cannot restore this user because they are a member of " + "another organization which forbids it", exception.Message.ToLowerInvariant()); @@ -2306,20 +2151,17 @@ OrganizationUserInvite invite, SutProvider sutProvider) [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, SutProvider sutProvider) { - sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.MembersTwoFAQueryOptimization).Returns(true); - organizationUser.Email = null; sutProvider.GetDependency() .GetPoliciesApplicableToUserAsync(organizationUser.UserId.Value, PolicyType.TwoFactorAuthentication, Arg.Any()) .Returns(new[] { new OrganizationUserPolicyDetails { OrganizationId = organizationUser.OrganizationId, PolicyType = PolicyType.TwoFactorAuthentication } }); RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); - var userService = Substitute.For(); var organizationUserRepository = sutProvider.GetDependency(); var eventService = sutProvider.GetDependency(); var exception = await Assert.ThrowsAsync( - () => sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id, userService)); + () => sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id)); Assert.Contains("you cannot restore this user until they enable " + "two-step login on their user account.", exception.Message.ToLowerInvariant()); @@ -2336,11 +2178,8 @@ OrganizationUserInvite invite, SutProvider sutProvider) [OrganizationUser(OrganizationUserStatusType.Revoked)] OrganizationUser organizationUser, SutProvider sutProvider) { - sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.MembersTwoFAQueryOptimization).Returns(true); - organizationUser.Email = null; // this is required to mock that the user as had already been confirmed before the revoke RestoreRevokeUser_Setup(organization, owner, organizationUser, sutProvider); - var userService = Substitute.For(); var organizationUserRepository = sutProvider.GetDependency(); var eventService = sutProvider.GetDependency(); var twoFactorIsEnabledQuery = sutProvider.GetDependency(); @@ -2353,7 +2192,7 @@ OrganizationUserInvite invite, SutProvider sutProvider) .TwoFactorIsEnabledAsync(Arg.Is>(i => i.Contains(organizationUser.UserId.Value))) .Returns(new List<(Guid userId, bool twoFactorIsEnabled)>() { (organizationUser.UserId.Value, true) }); - await sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id, userService); + await sutProvider.Sut.RestoreUserAsync(organizationUser, owner.Id); await organizationUserRepository.Received().RestoreAsync(organizationUser.Id, OrganizationUserStatusType.Confirmed); await eventService.Received() diff --git a/test/Core.Test/AdminConsole/Services/PolicyServiceTests.cs b/test/Core.Test/AdminConsole/Services/PolicyServiceTests.cs index fd7597a74..81b3fd459 100644 --- a/test/Core.Test/AdminConsole/Services/PolicyServiceTests.cs +++ b/test/Core.Test/AdminConsole/Services/PolicyServiceTests.cs @@ -7,6 +7,7 @@ using Bit.Core.Auth.Entities; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Models.Data; using Bit.Core.Auth.Repositories; +using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.Models.Data.Organizations.OrganizationUsers; @@ -32,7 +33,6 @@ public class PolicyServiceTests var badRequestException = await Assert.ThrowsAsync( () => sutProvider.Sut.SaveAsync(policy, - Substitute.For(), Substitute.For(), Guid.NewGuid())); @@ -60,7 +60,6 @@ public class PolicyServiceTests var badRequestException = await Assert.ThrowsAsync( () => sutProvider.Sut.SaveAsync(policy, - Substitute.For(), Substitute.For(), Guid.NewGuid())); @@ -93,7 +92,6 @@ public class PolicyServiceTests var badRequestException = await Assert.ThrowsAsync( () => sutProvider.Sut.SaveAsync(policy, - Substitute.For(), Substitute.For(), Guid.NewGuid())); @@ -125,7 +123,6 @@ public class PolicyServiceTests var badRequestException = await Assert.ThrowsAsync( () => sutProvider.Sut.SaveAsync(policy, - Substitute.For(), Substitute.For(), Guid.NewGuid())); @@ -163,7 +160,6 @@ public class PolicyServiceTests var badRequestException = await Assert.ThrowsAsync( () => sutProvider.Sut.SaveAsync(policy, - Substitute.For(), Substitute.For(), Guid.NewGuid())); @@ -192,7 +188,6 @@ public class PolicyServiceTests var badRequestException = await Assert.ThrowsAsync( () => sutProvider.Sut.SaveAsync(policy, - Substitute.For(), Substitute.For(), Guid.NewGuid())); @@ -226,7 +221,7 @@ public class PolicyServiceTests var utcNow = DateTime.UtcNow; - await sutProvider.Sut.SaveAsync(policy, Substitute.For(), Substitute.For(), Guid.NewGuid()); + await sutProvider.Sut.SaveAsync(policy, Substitute.For(), Guid.NewGuid()); await sutProvider.GetDependency().Received() .LogPolicyEventAsync(policy, EventType.Policy_Updated); @@ -256,7 +251,6 @@ public class PolicyServiceTests var badRequestException = await Assert.ThrowsAsync( () => sutProvider.Sut.SaveAsync(policy, - Substitute.For(), Substitute.For(), Guid.NewGuid())); @@ -348,19 +342,23 @@ public class PolicyServiceTests orgUserDetailAdmin }); - var userService = Substitute.For(); - var organizationService = Substitute.For(); + sutProvider.GetDependency() + .TwoFactorIsEnabledAsync(Arg.Any>()) + .Returns(new List<(OrganizationUserUserDetails user, bool hasTwoFactor)>() + { + (orgUserDetailUserInvited, false), + (orgUserDetailUserAcceptedWith2FA, true), + (orgUserDetailUserAcceptedWithout2FA, false), + (orgUserDetailAdmin, false), + }); - userService.TwoFactorIsEnabledAsync(orgUserDetailUserInvited).Returns(false); - userService.TwoFactorIsEnabledAsync(orgUserDetailUserAcceptedWith2FA).Returns(true); - userService.TwoFactorIsEnabledAsync(orgUserDetailUserAcceptedWithout2FA).Returns(false); - userService.TwoFactorIsEnabledAsync(orgUserDetailAdmin).Returns(false); + var organizationService = Substitute.For(); var utcNow = DateTime.UtcNow; var savingUserId = Guid.NewGuid(); - await sutProvider.Sut.SaveAsync(policy, userService, organizationService, savingUserId); + await sutProvider.Sut.SaveAsync(policy, organizationService, savingUserId); await organizationService.Received() .RemoveUserAsync(policy.OrganizationId, orgUserDetailUserAcceptedWithout2FA.Id, savingUserId); @@ -456,17 +454,24 @@ public class PolicyServiceTests orgUserDetailAdmin }); - var userService = Substitute.For(); - var organizationService = Substitute.For(); + sutProvider.GetDependency() + .TwoFactorIsEnabledAsync(Arg.Is>(ids => + ids.Contains(orgUserDetailUserWith2FANoMP.UserId.Value) + && ids.Contains(orgUserDetailUserWithout2FA.UserId.Value) + && ids.Contains(orgUserDetailAdmin.UserId.Value))) + .Returns(new List<(Guid userId, bool hasTwoFactor)>() + { + (orgUserDetailUserWith2FANoMP.UserId.Value, true), + (orgUserDetailUserWithout2FA.UserId.Value, false), + (orgUserDetailAdmin.UserId.Value, false), + }); - userService.TwoFactorIsEnabledAsync(orgUserDetailUserWith2FANoMP).Returns(true); - userService.TwoFactorIsEnabledAsync(orgUserDetailUserWithout2FA).Returns(false); - userService.TwoFactorIsEnabledAsync(orgUserDetailAdmin).Returns(false); + var organizationService = Substitute.For(); var savingUserId = Guid.NewGuid(); var badRequestException = await Assert.ThrowsAsync( - () => sutProvider.Sut.SaveAsync(policy, userService, organizationService, savingUserId)); + () => sutProvider.Sut.SaveAsync(policy, organizationService, savingUserId)); Assert.Contains("Policy could not be enabled. Non-compliant members will lose access to their accounts. Identify members without two-step login from the policies column in the members page.", badRequestException.Message, StringComparison.OrdinalIgnoreCase); @@ -526,17 +531,20 @@ public class PolicyServiceTests orgUserDetail, }); - var userService = Substitute.For(); - var organizationService = Substitute.For(); + sutProvider.GetDependency() + .TwoFactorIsEnabledAsync(Arg.Is>(ids => ids.Contains(orgUserDetail.UserId.Value))) + .Returns(new List<(Guid userId, bool hasTwoFactor)>() + { + (orgUserDetail.UserId.Value, false), + }); - userService.TwoFactorIsEnabledAsync(orgUserDetail) - .Returns(false); + var organizationService = Substitute.For(); var utcNow = DateTime.UtcNow; var savingUserId = Guid.NewGuid(); - await sutProvider.Sut.SaveAsync(policy, userService, organizationService, savingUserId); + await sutProvider.Sut.SaveAsync(policy, organizationService, savingUserId); await sutProvider.GetDependency().Received() .LogPolicyEventAsync(policy, EventType.Policy_Updated); @@ -579,7 +587,6 @@ public class PolicyServiceTests var badRequestException = await Assert.ThrowsAsync( () => sutProvider.Sut.SaveAsync(policy, - Substitute.For(), Substitute.For(), Guid.NewGuid())); @@ -616,7 +623,6 @@ public class PolicyServiceTests var badRequestException = await Assert.ThrowsAsync( () => sutProvider.Sut.SaveAsync(policy, - Substitute.For(), Substitute.For(), Guid.NewGuid())); @@ -650,7 +656,6 @@ public class PolicyServiceTests var badRequestException = await Assert.ThrowsAsync( () => sutProvider.Sut.SaveAsync(policy, - Substitute.For(), Substitute.For(), Guid.NewGuid())); @@ -684,7 +689,6 @@ public class PolicyServiceTests var badRequestException = await Assert.ThrowsAsync( () => sutProvider.Sut.SaveAsync(policy, - Substitute.For(), Substitute.For(), Guid.NewGuid())); diff --git a/test/Core.Test/Auth/Services/SsoConfigServiceTests.cs b/test/Core.Test/Auth/Services/SsoConfigServiceTests.cs index 9c63c6b01..fb566537a 100644 --- a/test/Core.Test/Auth/Services/SsoConfigServiceTests.cs +++ b/test/Core.Test/Auth/Services/SsoConfigServiceTests.cs @@ -342,7 +342,6 @@ public class SsoConfigServiceTests await sutProvider.GetDependency().Received(1) .SaveAsync( Arg.Is(t => t.Type == PolicyType.SingleOrg), - Arg.Any(), Arg.Any(), null ); @@ -350,7 +349,6 @@ public class SsoConfigServiceTests await sutProvider.GetDependency().Received(1) .SaveAsync( Arg.Is(t => t.Type == PolicyType.ResetPassword && t.GetDataModel().AutoEnrollEnabled), - Arg.Any(), Arg.Any(), null );