From 4de9a00612faa39b7b0bdd08f9e5c8318b1166b7 Mon Sep 17 00:00:00 2001 From: Thomas Rittson Date: Wed, 27 Nov 2024 15:02:08 +1000 Subject: [PATCH] Remove PolicyService.SaveAsync and use command instead --- .../Controllers/PoliciesController.cs | 29 +- .../Models/Request/PolicyRequestModel.cs | 25 +- .../Public/Controllers/PoliciesController.cs | 20 +- .../Request/PolicyUpdateRequestModel.cs | 27 +- .../VerifyOrganizationDomainCommand.cs | 21 +- .../Policies/ISavePolicyCommand.cs | 5 +- .../Implementations/SavePolicyCommand.cs | 4 +- .../AdminConsole/Services/IPolicyService.cs | 5 +- .../Services/Implementations/PolicyService.cs | 276 +------ .../Implementations/SsoConfigService.cs | 47 +- src/Core/Models/Commands/CommandResult.cs | 2 +- .../VerifyOrganizationDomainCommandTests.cs | 28 +- .../Services/PolicyServiceTests.cs | 701 ------------------ .../Auth/Services/SsoConfigServiceTests.cs | 25 +- 14 files changed, 123 insertions(+), 1092 deletions(-) diff --git a/src/Api/AdminConsole/Controllers/PoliciesController.cs b/src/Api/AdminConsole/Controllers/PoliciesController.cs index ee48cdd5d..1167d7a86 100644 --- a/src/Api/AdminConsole/Controllers/PoliciesController.cs +++ b/src/Api/AdminConsole/Controllers/PoliciesController.cs @@ -6,8 +6,8 @@ using Bit.Core; using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationDomains.Interfaces; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies; using Bit.Core.AdminConsole.Repositories; -using Bit.Core.AdminConsole.Services; using Bit.Core.Auth.Models.Business.Tokenables; using Bit.Core.Context; using Bit.Core.Enums; @@ -28,7 +28,6 @@ namespace Bit.Api.AdminConsole.Controllers; public class PoliciesController : Controller { private readonly IPolicyRepository _policyRepository; - private readonly IPolicyService _policyService; private readonly IOrganizationUserRepository _organizationUserRepository; private readonly IUserService _userService; private readonly ICurrentContext _currentContext; @@ -37,10 +36,10 @@ public class PoliciesController : Controller private readonly IDataProtectorTokenFactory _orgUserInviteTokenDataFactory; private readonly IFeatureService _featureService; private readonly IOrganizationHasVerifiedDomainsQuery _organizationHasVerifiedDomainsQuery; + private readonly ISavePolicyCommand _savePolicyCommand; public PoliciesController( IPolicyRepository policyRepository, - IPolicyService policyService, IOrganizationUserRepository organizationUserRepository, IUserService userService, ICurrentContext currentContext, @@ -48,10 +47,10 @@ public class PoliciesController : Controller IDataProtectionProvider dataProtectionProvider, IDataProtectorTokenFactory orgUserInviteTokenDataFactory, IFeatureService featureService, - IOrganizationHasVerifiedDomainsQuery organizationHasVerifiedDomainsQuery) + IOrganizationHasVerifiedDomainsQuery organizationHasVerifiedDomainsQuery, + ISavePolicyCommand savePolicyCommand) { _policyRepository = policyRepository; - _policyService = policyService; _organizationUserRepository = organizationUserRepository; _userService = userService; _currentContext = currentContext; @@ -62,6 +61,7 @@ public class PoliciesController : Controller _orgUserInviteTokenDataFactory = orgUserInviteTokenDataFactory; _featureService = featureService; _organizationHasVerifiedDomainsQuery = organizationHasVerifiedDomainsQuery; + _savePolicyCommand = savePolicyCommand; } [HttpGet("{type}")] @@ -178,25 +178,20 @@ public class PoliciesController : Controller } [HttpPut("{type}")] - public async Task Put(string orgId, int type, [FromBody] PolicyRequestModel model) + public async Task Put(Guid orgId, PolicyType type, [FromBody] PolicyRequestModel model) { - var orgIdGuid = new Guid(orgId); - if (!await _currentContext.ManagePolicies(orgIdGuid)) + if (!await _currentContext.ManagePolicies(orgId)) { throw new NotFoundException(); } - var policy = await _policyRepository.GetByOrganizationIdTypeAsync(new Guid(orgId), (PolicyType)type); - if (policy == null) + + if (type != model.Type) { - policy = model.ToPolicy(orgIdGuid); - } - else - { - policy = model.ToPolicy(policy); + throw new BadRequestException("Mismatched policy type"); } - var userId = _userService.GetProperUserId(User); - await _policyService.SaveAsync(policy, userId); + var policyUpdate = await model.ToPolicyUpdateAsync(orgId, _currentContext); + var policy = await _savePolicyCommand.SaveAsync(policyUpdate); return new PolicyResponseModel(policy); } } diff --git a/src/Api/AdminConsole/Models/Request/PolicyRequestModel.cs b/src/Api/AdminConsole/Models/Request/PolicyRequestModel.cs index db191194d..a243f46b2 100644 --- a/src/Api/AdminConsole/Models/Request/PolicyRequestModel.cs +++ b/src/Api/AdminConsole/Models/Request/PolicyRequestModel.cs @@ -1,7 +1,9 @@ using System.ComponentModel.DataAnnotations; using System.Text.Json; -using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Enums; +using Bit.Core.AdminConsole.Models.Data; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Models; +using Bit.Core.Context; namespace Bit.Api.AdminConsole.Models.Request; @@ -13,19 +15,12 @@ public class PolicyRequestModel public bool? Enabled { get; set; } public Dictionary Data { get; set; } - public Policy ToPolicy(Guid orgId) + public async Task ToPolicyUpdateAsync(Guid organizationId, ICurrentContext currentContext) => new() { - return ToPolicy(new Policy - { - Type = Type.Value, - OrganizationId = orgId - }); - } - - public Policy ToPolicy(Policy existingPolicy) - { - existingPolicy.Enabled = Enabled.GetValueOrDefault(); - existingPolicy.Data = Data != null ? JsonSerializer.Serialize(Data) : null; - return existingPolicy; - } + Type = Type!.Value, + OrganizationId = organizationId, + Data = Data != null ? JsonSerializer.Serialize(Data) : null, + Enabled = Enabled.GetValueOrDefault(), + PerformedBy = new StandardUser(currentContext.UserId!.Value, await currentContext.OrganizationOwner(organizationId)) + }; } diff --git a/src/Api/AdminConsole/Public/Controllers/PoliciesController.cs b/src/Api/AdminConsole/Public/Controllers/PoliciesController.cs index f2e7c35d2..a22c05ed6 100644 --- a/src/Api/AdminConsole/Public/Controllers/PoliciesController.cs +++ b/src/Api/AdminConsole/Public/Controllers/PoliciesController.cs @@ -3,6 +3,7 @@ using Bit.Api.AdminConsole.Public.Models.Request; using Bit.Api.AdminConsole.Public.Models.Response; using Bit.Api.Models.Public.Response; using Bit.Core.AdminConsole.Enums; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies; using Bit.Core.AdminConsole.Repositories; using Bit.Core.AdminConsole.Services; using Bit.Core.Context; @@ -18,15 +19,18 @@ public class PoliciesController : Controller private readonly IPolicyRepository _policyRepository; private readonly IPolicyService _policyService; private readonly ICurrentContext _currentContext; + private readonly ISavePolicyCommand _savePolicyCommand; public PoliciesController( IPolicyRepository policyRepository, IPolicyService policyService, - ICurrentContext currentContext) + ICurrentContext currentContext, + ISavePolicyCommand savePolicyCommand) { _policyRepository = policyRepository; _policyService = policyService; _currentContext = currentContext; + _savePolicyCommand = savePolicyCommand; } /// @@ -80,17 +84,9 @@ public class PoliciesController : Controller [ProducesResponseType((int)HttpStatusCode.NotFound)] public async Task Put(PolicyType type, [FromBody] PolicyUpdateRequestModel model) { - var policy = await _policyRepository.GetByOrganizationIdTypeAsync( - _currentContext.OrganizationId.Value, type); - if (policy == null) - { - policy = model.ToPolicy(_currentContext.OrganizationId.Value, type); - } - else - { - policy = model.ToPolicy(policy); - } - await _policyService.SaveAsync(policy, null); + var policyUpdate = model.ToPolicyUpdate(_currentContext.OrganizationId!.Value, type); + var policy = await _savePolicyCommand.SaveAsync(policyUpdate); + var response = new PolicyResponseModel(policy); return new JsonResult(response); } diff --git a/src/Api/AdminConsole/Public/Models/Request/PolicyUpdateRequestModel.cs b/src/Api/AdminConsole/Public/Models/Request/PolicyUpdateRequestModel.cs index f859686b8..eb5669046 100644 --- a/src/Api/AdminConsole/Public/Models/Request/PolicyUpdateRequestModel.cs +++ b/src/Api/AdminConsole/Public/Models/Request/PolicyUpdateRequestModel.cs @@ -1,26 +1,19 @@ using System.Text.Json; -using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Enums; +using Bit.Core.AdminConsole.Models.Data; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Models; +using Bit.Core.Enums; namespace Bit.Api.AdminConsole.Public.Models.Request; public class PolicyUpdateRequestModel : PolicyBaseModel { - public Policy ToPolicy(Guid orgId, PolicyType type) + public PolicyUpdate ToPolicyUpdate(Guid organizationId, PolicyType type) => new() { - return ToPolicy(new Policy - { - OrganizationId = orgId, - Enabled = Enabled.GetValueOrDefault(), - Data = Data != null ? JsonSerializer.Serialize(Data) : null, - Type = type - }); - } - - public virtual Policy ToPolicy(Policy existingPolicy) - { - existingPolicy.Enabled = Enabled.GetValueOrDefault(); - existingPolicy.Data = Data != null ? JsonSerializer.Serialize(Data) : null; - return existingPolicy; - } + Type = type, + OrganizationId = organizationId, + Data = Data != null ? JsonSerializer.Serialize(Data) : null, + Enabled = Enabled.GetValueOrDefault(), + PerformedBy = new SystemUser(EventSystemUser.PublicApi) + }; } diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationDomains/VerifyOrganizationDomainCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationDomains/VerifyOrganizationDomainCommand.cs index dd6add669..375c6326e 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationDomains/VerifyOrganizationDomainCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationDomains/VerifyOrganizationDomainCommand.cs @@ -1,8 +1,8 @@ -using Bit.Core.AdminConsole.Entities; -using Bit.Core.AdminConsole.Enums; +using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.Models.Data; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationDomains.Interfaces; -using Bit.Core.AdminConsole.Services; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Models; using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Enums; @@ -19,9 +19,9 @@ public class VerifyOrganizationDomainCommand( IDnsResolverService dnsResolverService, IEventService eventService, IGlobalSettings globalSettings, - IPolicyService policyService, IFeatureService featureService, ICurrentContext currentContext, + ISavePolicyCommand savePolicyCommand, ILogger logger) : IVerifyOrganizationDomainCommand { @@ -125,10 +125,15 @@ public class VerifyOrganizationDomainCommand( { if (featureService.IsEnabled(FeatureFlagKeys.AccountDeprovisioning)) { - await policyService.SaveAsync( - new Policy { OrganizationId = organizationId, Type = PolicyType.SingleOrg, Enabled = true }, - savingUserId: actingUser is StandardUser standardUser ? standardUser.UserId : null, - eventSystemUser: actingUser is SystemUser systemUser ? systemUser.SystemUserType : null); + var policyUpdate = new PolicyUpdate + { + OrganizationId = organizationId, + Type = PolicyType.SingleOrg, + Enabled = true, + PerformedBy = actingUser + }; + + await savePolicyCommand.SaveAsync(policyUpdate); } } } diff --git a/src/Core/AdminConsole/OrganizationFeatures/Policies/ISavePolicyCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/Policies/ISavePolicyCommand.cs index 5bfdfc6aa..6ca842686 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/Policies/ISavePolicyCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/Policies/ISavePolicyCommand.cs @@ -1,8 +1,9 @@ -using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Models; +using Bit.Core.AdminConsole.Entities; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Models; namespace Bit.Core.AdminConsole.OrganizationFeatures.Policies; public interface ISavePolicyCommand { - Task SaveAsync(PolicyUpdate policy); + Task SaveAsync(PolicyUpdate policy); } diff --git a/src/Core/AdminConsole/OrganizationFeatures/Policies/Implementations/SavePolicyCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/Policies/Implementations/SavePolicyCommand.cs index f193aeabd..cf332e689 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/Policies/Implementations/SavePolicyCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/Policies/Implementations/SavePolicyCommand.cs @@ -42,7 +42,7 @@ public class SavePolicyCommand : ISavePolicyCommand _policyValidators = policyValidatorsDict; } - public async Task SaveAsync(PolicyUpdate policyUpdate) + public async Task SaveAsync(PolicyUpdate policyUpdate) { var org = await _applicationCacheService.GetOrganizationAbilityAsync(policyUpdate.OrganizationId); if (org == null) @@ -74,6 +74,8 @@ public class SavePolicyCommand : ISavePolicyCommand await _policyRepository.UpsertAsync(policy); await _eventService.LogPolicyEventAsync(policy, EventType.Policy_Updated); + + return policy; } private async Task RunValidatorAsync(IPolicyValidator validator, PolicyUpdate policyUpdate) diff --git a/src/Core/AdminConsole/Services/IPolicyService.cs b/src/Core/AdminConsole/Services/IPolicyService.cs index 715c3a34d..4f9a25f90 100644 --- a/src/Core/AdminConsole/Services/IPolicyService.cs +++ b/src/Core/AdminConsole/Services/IPolicyService.cs @@ -1,5 +1,4 @@ -using Bit.Core.AdminConsole.Entities; -using Bit.Core.AdminConsole.Enums; +using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; using Bit.Core.Entities; using Bit.Core.Enums; @@ -9,8 +8,6 @@ namespace Bit.Core.AdminConsole.Services; public interface IPolicyService { - Task SaveAsync(Policy policy, Guid? savingUserId, EventSystemUser? eventSystemUser = null); - /// /// Get the combined master password policy options for the specified user. /// diff --git a/src/Core/AdminConsole/Services/Implementations/PolicyService.cs b/src/Core/AdminConsole/Services/Implementations/PolicyService.cs index 69ec27fd8..c3eb2272d 100644 --- a/src/Core/AdminConsole/Services/Implementations/PolicyService.cs +++ b/src/Core/AdminConsole/Services/Implementations/PolicyService.cs @@ -1,19 +1,8 @@ -using Bit.Core.AdminConsole.Entities; -using Bit.Core.AdminConsole.Enums; -using Bit.Core.AdminConsole.Models.Data; +using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; -using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationDomains.Interfaces; -using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; -using Bit.Core.AdminConsole.OrganizationFeatures.Policies; -using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Models; using Bit.Core.AdminConsole.Repositories; -using Bit.Core.Auth.Enums; -using Bit.Core.Auth.Repositories; -using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; -using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Enums; -using Bit.Core.Exceptions; using Bit.Core.Models.Data.Organizations.OrganizationUsers; using Bit.Core.Repositories; using Bit.Core.Services; @@ -24,107 +13,20 @@ namespace Bit.Core.AdminConsole.Services.Implementations; public class PolicyService : IPolicyService { private readonly IApplicationCacheService _applicationCacheService; - private readonly IEventService _eventService; - private readonly IOrganizationRepository _organizationRepository; private readonly IOrganizationUserRepository _organizationUserRepository; private readonly IPolicyRepository _policyRepository; - private readonly ISsoConfigRepository _ssoConfigRepository; - private readonly IMailService _mailService; private readonly GlobalSettings _globalSettings; - private readonly ITwoFactorIsEnabledQuery _twoFactorIsEnabledQuery; - private readonly IFeatureService _featureService; - private readonly ISavePolicyCommand _savePolicyCommand; - private readonly IRemoveOrganizationUserCommand _removeOrganizationUserCommand; - private readonly IOrganizationHasVerifiedDomainsQuery _organizationHasVerifiedDomainsQuery; - private readonly ICurrentContext _currentContext; public PolicyService( IApplicationCacheService applicationCacheService, - IEventService eventService, - IOrganizationRepository organizationRepository, IOrganizationUserRepository organizationUserRepository, IPolicyRepository policyRepository, - ISsoConfigRepository ssoConfigRepository, - IMailService mailService, - GlobalSettings globalSettings, - ITwoFactorIsEnabledQuery twoFactorIsEnabledQuery, - IFeatureService featureService, - ISavePolicyCommand savePolicyCommand, - IRemoveOrganizationUserCommand removeOrganizationUserCommand, - IOrganizationHasVerifiedDomainsQuery organizationHasVerifiedDomainsQuery, - ICurrentContext currentContext) + GlobalSettings globalSettings) { _applicationCacheService = applicationCacheService; - _eventService = eventService; - _organizationRepository = organizationRepository; _organizationUserRepository = organizationUserRepository; _policyRepository = policyRepository; - _ssoConfigRepository = ssoConfigRepository; - _mailService = mailService; _globalSettings = globalSettings; - _twoFactorIsEnabledQuery = twoFactorIsEnabledQuery; - _featureService = featureService; - _savePolicyCommand = savePolicyCommand; - _removeOrganizationUserCommand = removeOrganizationUserCommand; - _organizationHasVerifiedDomainsQuery = organizationHasVerifiedDomainsQuery; - _currentContext = currentContext; - } - - public async Task SaveAsync(Policy policy, Guid? savingUserId, EventSystemUser? eventSystemUser = null) - { - if (_featureService.IsEnabled(FeatureFlagKeys.Pm13322AddPolicyDefinitions)) - { - // Transitional mapping - this will be moved to callers once the feature flag is removed - // TODO make sure to populate with SystemUser if not an actual user - var policyUpdate = new PolicyUpdate - { - OrganizationId = policy.OrganizationId, - Type = policy.Type, - Enabled = policy.Enabled, - Data = policy.Data, - PerformedBy = savingUserId.HasValue - ? new StandardUser(savingUserId.Value, await _currentContext.OrganizationOwner(policy.OrganizationId)) - : new SystemUser(eventSystemUser ?? EventSystemUser.Unknown) - }; - - await _savePolicyCommand.SaveAsync(policyUpdate); - return; - } - - var org = await _organizationRepository.GetByIdAsync(policy.OrganizationId); - if (org == null) - { - throw new BadRequestException("Organization not found"); - } - - if (!org.UsePolicies) - { - throw new BadRequestException("This organization cannot use policies."); - } - - // FIXME: This method will throw a bunch of errors based on if the - // policy that is being applied requires some other policy that is - // not enabled. It may be advisable to refactor this into a domain - // object and get this kind of stuff out of the service. - await HandleDependentPoliciesAsync(policy, org); - - var now = DateTime.UtcNow; - if (policy.Id == default(Guid)) - { - policy.CreationDate = now; - } - - policy.RevisionDate = now; - - // We can exit early for disable operations, because they are - // simpler. - if (!policy.Enabled) - { - await SetPolicyConfiguration(policy); - return; - } - - await EnablePolicyAsync(policy, org, savingUserId); } public async Task GetMasterPasswordPolicyForUserAsync(User user) @@ -190,178 +92,4 @@ public class PolicyService : IPolicyService return new[] { OrganizationUserType.Owner, OrganizationUserType.Admin }; } - - private async Task DependsOnSingleOrgAsync(Organization org) - { - var singleOrg = await _policyRepository.GetByOrganizationIdTypeAsync(org.Id, PolicyType.SingleOrg); - if (singleOrg?.Enabled != true) - { - throw new BadRequestException("Single Organization policy not enabled."); - } - } - - private async Task RequiredBySsoAsync(Organization org) - { - var requireSso = await _policyRepository.GetByOrganizationIdTypeAsync(org.Id, PolicyType.RequireSso); - if (requireSso?.Enabled == true) - { - throw new BadRequestException("Single Sign-On Authentication policy is enabled."); - } - } - - private async Task RequiredByKeyConnectorAsync(Organization org) - { - var ssoConfig = await _ssoConfigRepository.GetByOrganizationIdAsync(org.Id); - if (ssoConfig?.GetData()?.MemberDecryptionType == MemberDecryptionType.KeyConnector) - { - throw new BadRequestException("Key Connector is enabled."); - } - } - - private async Task RequiredByAccountRecoveryAsync(Organization org) - { - var requireSso = await _policyRepository.GetByOrganizationIdTypeAsync(org.Id, PolicyType.ResetPassword); - if (requireSso?.Enabled == true) - { - throw new BadRequestException("Account recovery policy is enabled."); - } - } - - private async Task RequiredByVaultTimeoutAsync(Organization org) - { - var vaultTimeout = await _policyRepository.GetByOrganizationIdTypeAsync(org.Id, PolicyType.MaximumVaultTimeout); - if (vaultTimeout?.Enabled == true) - { - throw new BadRequestException("Maximum Vault Timeout policy is enabled."); - } - } - - private async Task RequiredBySsoTrustedDeviceEncryptionAsync(Organization org) - { - var ssoConfig = await _ssoConfigRepository.GetByOrganizationIdAsync(org.Id); - if (ssoConfig?.GetData()?.MemberDecryptionType == MemberDecryptionType.TrustedDeviceEncryption) - { - throw new BadRequestException("Trusted device encryption is on and requires this policy."); - } - } - - private async Task HandleDependentPoliciesAsync(Policy policy, Organization org) - { - switch (policy.Type) - { - case PolicyType.SingleOrg: - if (!policy.Enabled) - { - await HasVerifiedDomainsAsync(org); - await RequiredBySsoAsync(org); - await RequiredByVaultTimeoutAsync(org); - await RequiredByKeyConnectorAsync(org); - await RequiredByAccountRecoveryAsync(org); - } - break; - - case PolicyType.RequireSso: - if (policy.Enabled) - { - await DependsOnSingleOrgAsync(org); - } - else - { - await RequiredByKeyConnectorAsync(org); - await RequiredBySsoTrustedDeviceEncryptionAsync(org); - } - break; - - case PolicyType.ResetPassword: - if (!policy.Enabled || policy.GetDataModel()?.AutoEnrollEnabled == false) - { - await RequiredBySsoTrustedDeviceEncryptionAsync(org); - } - - if (policy.Enabled) - { - await DependsOnSingleOrgAsync(org); - } - break; - - case PolicyType.MaximumVaultTimeout: - if (policy.Enabled) - { - await DependsOnSingleOrgAsync(org); - } - break; - } - } - - private async Task HasVerifiedDomainsAsync(Organization org) - { - if (_featureService.IsEnabled(FeatureFlagKeys.AccountDeprovisioning) - && await _organizationHasVerifiedDomainsQuery.HasVerifiedDomainsAsync(org.Id)) - { - throw new BadRequestException("The Single organization policy is required for organizations that have enabled domain verification."); - } - } - - private async Task SetPolicyConfiguration(Policy policy) - { - await _policyRepository.UpsertAsync(policy); - await _eventService.LogPolicyEventAsync(policy, EventType.Policy_Updated); - } - - private async Task EnablePolicyAsync(Policy policy, Organization org, Guid? savingUserId) - { - var currentPolicy = await _policyRepository.GetByIdAsync(policy.Id); - if (!currentPolicy?.Enabled ?? true) - { - var orgUsers = await _organizationUserRepository.GetManyDetailsByOrganizationAsync(policy.OrganizationId); - var organizationUsersTwoFactorEnabled = await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(orgUsers); - 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)) - { - var userTwoFactorEnabled = organizationUsersTwoFactorEnabled.FirstOrDefault(u => u.user.Id == orgUser.Id).twoFactorIsEnabled; - if (!userTwoFactorEnabled) - { - 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 _removeOrganizationUserCommand.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 _removeOrganizationUserCommand.RemoveUserAsync(policy.OrganizationId, orgUser.Id, - savingUserId); - await _mailService.SendOrganizationUserRemovedForPolicySingleOrgEmailAsync( - org.DisplayName(), orgUser.Email); - } - } - break; - default: - break; - } - } - - await SetPolicyConfiguration(policy); - } } diff --git a/src/Core/Auth/Services/Implementations/SsoConfigService.cs b/src/Core/Auth/Services/Implementations/SsoConfigService.cs index 532f00039..bf7e2d56f 100644 --- a/src/Core/Auth/Services/Implementations/SsoConfigService.cs +++ b/src/Core/Auth/Services/Implementations/SsoConfigService.cs @@ -1,8 +1,9 @@ using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Models; using Bit.Core.AdminConsole.Repositories; -using Bit.Core.AdminConsole.Services; using Bit.Core.Auth.Entities; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Repositories; @@ -17,25 +18,25 @@ public class SsoConfigService : ISsoConfigService { private readonly ISsoConfigRepository _ssoConfigRepository; private readonly IPolicyRepository _policyRepository; - private readonly IPolicyService _policyService; private readonly IOrganizationRepository _organizationRepository; private readonly IOrganizationUserRepository _organizationUserRepository; private readonly IEventService _eventService; + private readonly ISavePolicyCommand _savePolicyCommand; public SsoConfigService( ISsoConfigRepository ssoConfigRepository, IPolicyRepository policyRepository, - IPolicyService policyService, IOrganizationRepository organizationRepository, IOrganizationUserRepository organizationUserRepository, - IEventService eventService) + IEventService eventService, + ISavePolicyCommand savePolicyCommand) { _ssoConfigRepository = ssoConfigRepository; _policyRepository = policyRepository; - _policyService = policyService; _organizationRepository = organizationRepository; _organizationUserRepository = organizationUserRepository; _eventService = eventService; + _savePolicyCommand = savePolicyCommand; } public async Task SaveAsync(SsoConfig config, Organization organization) @@ -63,25 +64,29 @@ public class SsoConfigService : ISsoConfigService // Automatically enable account recovery, SSO required, and single org policies if trusted device encryption is selected if (config.GetData().MemberDecryptionType == MemberDecryptionType.TrustedDeviceEncryption) { - var singleOrgPolicy = await _policyRepository.GetByOrganizationIdTypeAsync(config.OrganizationId, PolicyType.SingleOrg) ?? - new Policy { OrganizationId = config.OrganizationId, Type = PolicyType.SingleOrg }; - singleOrgPolicy.Enabled = true; + await _savePolicyCommand.SaveAsync(new() + { + OrganizationId = config.OrganizationId, + Type = PolicyType.SingleOrg, + Enabled = true + }); - await _policyService.SaveAsync(singleOrgPolicy, null); + var resetPasswordPolicy = new PolicyUpdate + { + OrganizationId = config.OrganizationId, + Type = PolicyType.ResetPassword, + Enabled = true, + }; + resetPasswordPolicy.SetDataModel(new ResetPasswordDataModel { AutoEnrollEnabled = true }); + await _savePolicyCommand.SaveAsync(resetPasswordPolicy); - 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, 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, null); + await _savePolicyCommand.SaveAsync(new() + { + OrganizationId = config.OrganizationId, + Type = PolicyType.RequireSso, + Enabled = true + }); } await LogEventsAsync(config, oldConfig); diff --git a/src/Core/Models/Commands/CommandResult.cs b/src/Core/Models/Commands/CommandResult.cs index 4ac2d6249..e8a756c7c 100644 --- a/src/Core/Models/Commands/CommandResult.cs +++ b/src/Core/Models/Commands/CommandResult.cs @@ -8,5 +8,5 @@ public class CommandResult(IEnumerable errors) public bool HasErrors => ErrorMessages.Count > 0; public List ErrorMessages { get; } = errors.ToList(); - public CommandResult() : this([]) { } + public CommandResult() : this((IEnumerable)[]) { } } diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationDomains/VerifyOrganizationDomainCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationDomains/VerifyOrganizationDomainCommandTests.cs index 8dbe53313..700df88d5 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationDomains/VerifyOrganizationDomainCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationDomains/VerifyOrganizationDomainCommandTests.cs @@ -1,7 +1,8 @@ -using Bit.Core.AdminConsole.Entities; -using Bit.Core.AdminConsole.Enums; +using Bit.Core.AdminConsole.Enums; +using Bit.Core.AdminConsole.Models.Data; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationDomains; -using Bit.Core.AdminConsole.Services; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Models; using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Enums; @@ -182,9 +183,13 @@ public class VerifyOrganizationDomainCommandTests _ = await sutProvider.Sut.UserVerifyOrganizationDomainAsync(domain); - await sutProvider.GetDependency() + await sutProvider.GetDependency() .Received(1) - .SaveAsync(Arg.Is(x => x.Type == PolicyType.SingleOrg && x.OrganizationId == domain.OrganizationId && x.Enabled), userId); + .SaveAsync(Arg.Is(x => x.Type == PolicyType.SingleOrg && + x.OrganizationId == domain.OrganizationId && + x.Enabled && + x.PerformedBy is StandardUser && + x.PerformedBy.UserId == userId)); } [Theory, BitAutoData] @@ -208,9 +213,9 @@ public class VerifyOrganizationDomainCommandTests _ = await sutProvider.Sut.UserVerifyOrganizationDomainAsync(domain); - await sutProvider.GetDependency() + await sutProvider.GetDependency() .DidNotReceive() - .SaveAsync(Arg.Any(), null); + .SaveAsync(Arg.Any()); } [Theory, BitAutoData] @@ -234,10 +239,9 @@ public class VerifyOrganizationDomainCommandTests _ = await sutProvider.Sut.UserVerifyOrganizationDomainAsync(domain); - await sutProvider.GetDependency() + await sutProvider.GetDependency() .DidNotReceive() - .SaveAsync(Arg.Any(), null); - + .SaveAsync(Arg.Any()); } [Theory, BitAutoData] @@ -261,8 +265,8 @@ public class VerifyOrganizationDomainCommandTests _ = await sutProvider.Sut.UserVerifyOrganizationDomainAsync(domain); - await sutProvider.GetDependency() + await sutProvider.GetDependency() .DidNotReceive() - .SaveAsync(Arg.Any(), null); + .SaveAsync(Arg.Any()); } } diff --git a/test/Core.Test/AdminConsole/Services/PolicyServiceTests.cs b/test/Core.Test/AdminConsole/Services/PolicyServiceTests.cs index 68f36e37c..62ab584c4 100644 --- a/test/Core.Test/AdminConsole/Services/PolicyServiceTests.cs +++ b/test/Core.Test/AdminConsole/Services/PolicyServiceTests.cs @@ -1,25 +1,13 @@ using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Enums; -using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; -using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationDomains.Interfaces; -using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; -using Bit.Core.AdminConsole.Repositories; using Bit.Core.AdminConsole.Services.Implementations; -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; using Bit.Core.Repositories; -using Bit.Core.Services; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; using NSubstitute; using Xunit; -using AdminConsoleFixtures = Bit.Core.Test.AdminConsole.AutoFixture; using GlobalSettings = Bit.Core.Settings.GlobalSettings; namespace Bit.Core.Test.AdminConsole.Services; @@ -27,667 +15,6 @@ namespace Bit.Core.Test.AdminConsole.Services; [SutProviderCustomize] public class PolicyServiceTests { - [Theory, BitAutoData] - public async Task SaveAsync_OrganizationDoesNotExist_ThrowsBadRequest( - [AdminConsoleFixtures.Policy(PolicyType.DisableSend)] Policy policy, SutProvider sutProvider) - { - SetupOrg(sutProvider, policy.OrganizationId, null); - - var badRequestException = await Assert.ThrowsAsync( - () => sutProvider.Sut.SaveAsync(policy, - Guid.NewGuid())); - - Assert.Contains("Organization not found", badRequestException.Message, StringComparison.OrdinalIgnoreCase); - - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .UpsertAsync(default); - - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .LogPolicyEventAsync(default, default, default); - } - - [Theory, BitAutoData] - public async Task SaveAsync_OrganizationCannotUsePolicies_ThrowsBadRequest( - [AdminConsoleFixtures.Policy(PolicyType.DisableSend)] Policy policy, SutProvider sutProvider) - { - var orgId = Guid.NewGuid(); - - SetupOrg(sutProvider, policy.OrganizationId, new Organization - { - UsePolicies = false, - }); - - var badRequestException = await Assert.ThrowsAsync( - () => sutProvider.Sut.SaveAsync(policy, - Guid.NewGuid())); - - Assert.Contains("cannot use policies", badRequestException.Message, StringComparison.OrdinalIgnoreCase); - - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .UpsertAsync(default); - - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .LogPolicyEventAsync(default, default, default); - } - - [Theory, BitAutoData] - public async Task SaveAsync_SingleOrg_RequireSsoEnabled_ThrowsBadRequest( - [AdminConsoleFixtures.Policy(PolicyType.SingleOrg)] Policy policy, SutProvider sutProvider) - { - policy.Enabled = false; - - SetupOrg(sutProvider, policy.OrganizationId, new Organization - { - Id = policy.OrganizationId, - UsePolicies = true, - }); - - sutProvider.GetDependency() - .GetByOrganizationIdTypeAsync(policy.OrganizationId, PolicyType.RequireSso) - .Returns(Task.FromResult(new Policy { Enabled = true })); - - var badRequestException = await Assert.ThrowsAsync( - () => sutProvider.Sut.SaveAsync(policy, - Guid.NewGuid())); - - Assert.Contains("Single Sign-On Authentication policy is enabled.", badRequestException.Message, StringComparison.OrdinalIgnoreCase); - - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .UpsertAsync(default); - - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .LogPolicyEventAsync(default, default, default); - } - - [Theory, BitAutoData] - public async Task SaveAsync_SingleOrg_VaultTimeoutEnabled_ThrowsBadRequest([AdminConsoleFixtures.Policy(PolicyType.SingleOrg)] Policy policy, SutProvider sutProvider) - { - policy.Enabled = false; - - SetupOrg(sutProvider, policy.OrganizationId, new Organization - { - Id = policy.OrganizationId, - UsePolicies = true, - }); - - sutProvider.GetDependency() - .GetByOrganizationIdTypeAsync(policy.OrganizationId, PolicyType.MaximumVaultTimeout) - .Returns(new Policy { Enabled = true }); - - var badRequestException = await Assert.ThrowsAsync( - () => sutProvider.Sut.SaveAsync(policy, - Guid.NewGuid())); - - Assert.Contains("Maximum Vault Timeout policy is enabled.", badRequestException.Message, StringComparison.OrdinalIgnoreCase); - - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .UpsertAsync(default); - } - - [Theory] - [BitAutoData(PolicyType.SingleOrg)] - [BitAutoData(PolicyType.RequireSso)] - public async Task SaveAsync_PolicyRequiredByKeyConnector_DisablePolicy_ThrowsBadRequest( - PolicyType policyType, - Policy policy, - SutProvider sutProvider) - { - policy.Enabled = false; - policy.Type = policyType; - - SetupOrg(sutProvider, policy.OrganizationId, new Organization - { - Id = policy.OrganizationId, - UsePolicies = true, - }); - - var ssoConfig = new SsoConfig { Enabled = true }; - var data = new SsoConfigurationData { MemberDecryptionType = MemberDecryptionType.KeyConnector }; - ssoConfig.SetData(data); - - sutProvider.GetDependency() - .GetByOrganizationIdAsync(policy.OrganizationId) - .Returns(ssoConfig); - - var badRequestException = await Assert.ThrowsAsync( - () => sutProvider.Sut.SaveAsync(policy, - Guid.NewGuid())); - - Assert.Contains("Key Connector is enabled.", badRequestException.Message, StringComparison.OrdinalIgnoreCase); - - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .UpsertAsync(default); - } - - [Theory, BitAutoData] - public async Task SaveAsync_RequireSsoPolicy_NotEnabled_ThrowsBadRequestAsync( - [AdminConsoleFixtures.Policy(PolicyType.RequireSso)] Policy policy, SutProvider sutProvider) - { - policy.Enabled = true; - - SetupOrg(sutProvider, policy.OrganizationId, new Organization - { - Id = policy.OrganizationId, - UsePolicies = true, - }); - - sutProvider.GetDependency() - .GetByOrganizationIdTypeAsync(policy.OrganizationId, PolicyType.SingleOrg) - .Returns(Task.FromResult(new Policy { Enabled = false })); - - var badRequestException = await Assert.ThrowsAsync( - () => sutProvider.Sut.SaveAsync(policy, - Guid.NewGuid())); - - Assert.Contains("Single Organization policy not enabled.", badRequestException.Message, StringComparison.OrdinalIgnoreCase); - - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .UpsertAsync(default); - - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .LogPolicyEventAsync(default, default, default); - } - - [Theory, BitAutoData] - public async Task SaveAsync_NewPolicy_Created( - [AdminConsoleFixtures.Policy(PolicyType.ResetPassword)] Policy policy, SutProvider sutProvider) - { - policy.Id = default; - policy.Data = null; - - SetupOrg(sutProvider, policy.OrganizationId, new Organization - { - Id = policy.OrganizationId, - UsePolicies = true, - }); - - sutProvider.GetDependency() - .GetByOrganizationIdTypeAsync(policy.OrganizationId, PolicyType.SingleOrg) - .Returns(Task.FromResult(new Policy { Enabled = true })); - - var utcNow = DateTime.UtcNow; - - await sutProvider.Sut.SaveAsync(policy, Guid.NewGuid()); - - await sutProvider.GetDependency().Received() - .LogPolicyEventAsync(policy, EventType.Policy_Updated); - - await sutProvider.GetDependency().Received() - .UpsertAsync(policy); - - Assert.True(policy.CreationDate - utcNow < TimeSpan.FromSeconds(1)); - Assert.True(policy.RevisionDate - utcNow < TimeSpan.FromSeconds(1)); - } - - [Theory, BitAutoData] - public async Task SaveAsync_VaultTimeoutPolicy_NotEnabled_ThrowsBadRequestAsync( - [AdminConsoleFixtures.Policy(PolicyType.MaximumVaultTimeout)] Policy policy, SutProvider sutProvider) - { - policy.Enabled = true; - - SetupOrg(sutProvider, policy.OrganizationId, new Organization - { - Id = policy.OrganizationId, - UsePolicies = true, - }); - - sutProvider.GetDependency() - .GetByOrganizationIdTypeAsync(policy.OrganizationId, PolicyType.SingleOrg) - .Returns(Task.FromResult(new Policy { Enabled = false })); - - var badRequestException = await Assert.ThrowsAsync( - () => sutProvider.Sut.SaveAsync(policy, - Guid.NewGuid())); - - Assert.Contains("Single Organization policy not enabled.", badRequestException.Message, StringComparison.OrdinalIgnoreCase); - - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .UpsertAsync(default); - - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .LogPolicyEventAsync(default, default, default); - } - - [Theory, BitAutoData] - public async Task SaveAsync_ExistingPolicy_UpdateTwoFactor( - Organization organization, - [AdminConsoleFixtures.Policy(PolicyType.TwoFactorAuthentication)] Policy policy, - SutProvider sutProvider) - { - // If the policy that this is updating isn't enabled then do some work now that the current one is enabled - - organization.UsePolicies = true; - policy.OrganizationId = organization.Id; - - SetupOrg(sutProvider, organization.Id, organization); - - sutProvider.GetDependency() - .GetByIdAsync(policy.Id) - .Returns(new Policy - { - Id = policy.Id, - Type = PolicyType.TwoFactorAuthentication, - Enabled = false - }); - - var orgUserDetailUserInvited = new OrganizationUserUserDetails - { - Id = Guid.NewGuid(), - Status = OrganizationUserStatusType.Invited, - Type = OrganizationUserType.User, - // Needs to be different from what is passed in as the savingUserId to Sut.SaveAsync - Email = "user1@test.com", - Name = "TEST", - UserId = Guid.NewGuid(), - HasMasterPassword = false - }; - var orgUserDetailUserAcceptedWith2FA = new OrganizationUserUserDetails - { - Id = Guid.NewGuid(), - Status = OrganizationUserStatusType.Accepted, - Type = OrganizationUserType.User, - // Needs to be different from what is passed in as the savingUserId to Sut.SaveAsync - Email = "user2@test.com", - Name = "TEST", - UserId = Guid.NewGuid(), - HasMasterPassword = true - }; - var orgUserDetailUserAcceptedWithout2FA = new OrganizationUserUserDetails - { - Id = Guid.NewGuid(), - Status = OrganizationUserStatusType.Accepted, - Type = OrganizationUserType.User, - // Needs to be different from what is passed in as the savingUserId to Sut.SaveAsync - Email = "user3@test.com", - Name = "TEST", - UserId = Guid.NewGuid(), - HasMasterPassword = true - }; - var orgUserDetailAdmin = new OrganizationUserUserDetails - { - Id = Guid.NewGuid(), - Status = OrganizationUserStatusType.Confirmed, - Type = OrganizationUserType.Admin, - // Needs to be different from what is passed in as the savingUserId to Sut.SaveAsync - Email = "admin@test.com", - Name = "ADMIN", - UserId = Guid.NewGuid(), - HasMasterPassword = false - }; - - sutProvider.GetDependency() - .GetManyDetailsByOrganizationAsync(policy.OrganizationId) - .Returns(new List - { - orgUserDetailUserInvited, - orgUserDetailUserAcceptedWith2FA, - orgUserDetailUserAcceptedWithout2FA, - orgUserDetailAdmin - }); - - sutProvider.GetDependency() - .TwoFactorIsEnabledAsync(Arg.Any>()) - .Returns(new List<(OrganizationUserUserDetails user, bool hasTwoFactor)>() - { - (orgUserDetailUserInvited, false), - (orgUserDetailUserAcceptedWith2FA, true), - (orgUserDetailUserAcceptedWithout2FA, false), - (orgUserDetailAdmin, false), - }); - - var removeOrganizationUserCommand = sutProvider.GetDependency(); - - var utcNow = DateTime.UtcNow; - - var savingUserId = Guid.NewGuid(); - - await sutProvider.Sut.SaveAsync(policy, savingUserId); - - await removeOrganizationUserCommand.Received() - .RemoveUserAsync(policy.OrganizationId, orgUserDetailUserAcceptedWithout2FA.Id, savingUserId); - await sutProvider.GetDependency().Received() - .SendOrganizationUserRemovedForPolicyTwoStepEmailAsync(organization.DisplayName(), orgUserDetailUserAcceptedWithout2FA.Email); - - await removeOrganizationUserCommand.DidNotReceive() - .RemoveUserAsync(policy.OrganizationId, orgUserDetailUserInvited.Id, savingUserId); - await sutProvider.GetDependency().DidNotReceive() - .SendOrganizationUserRemovedForPolicyTwoStepEmailAsync(organization.DisplayName(), orgUserDetailUserInvited.Email); - await removeOrganizationUserCommand.DidNotReceive() - .RemoveUserAsync(policy.OrganizationId, orgUserDetailUserAcceptedWith2FA.Id, savingUserId); - await sutProvider.GetDependency().DidNotReceive() - .SendOrganizationUserRemovedForPolicyTwoStepEmailAsync(organization.DisplayName(), orgUserDetailUserAcceptedWith2FA.Email); - await removeOrganizationUserCommand.DidNotReceive() - .RemoveUserAsync(policy.OrganizationId, orgUserDetailAdmin.Id, savingUserId); - await sutProvider.GetDependency().DidNotReceive() - .SendOrganizationUserRemovedForPolicyTwoStepEmailAsync(organization.DisplayName(), orgUserDetailAdmin.Email); - - await sutProvider.GetDependency().Received() - .LogPolicyEventAsync(policy, EventType.Policy_Updated); - - await sutProvider.GetDependency().Received() - .UpsertAsync(policy); - - Assert.True(policy.CreationDate - utcNow < TimeSpan.FromSeconds(1)); - Assert.True(policy.RevisionDate - utcNow < TimeSpan.FromSeconds(1)); - } - - [Theory, BitAutoData] - public async Task SaveAsync_EnableTwoFactor_WithoutMasterPasswordOr2FA_ThrowsBadRequest( - Organization organization, - [AdminConsoleFixtures.Policy(PolicyType.TwoFactorAuthentication)] Policy policy, - SutProvider sutProvider) - { - organization.UsePolicies = true; - policy.OrganizationId = organization.Id; - - SetupOrg(sutProvider, organization.Id, organization); - - var orgUserDetailUserWith2FAAndMP = new OrganizationUserUserDetails - { - Id = Guid.NewGuid(), - Status = OrganizationUserStatusType.Confirmed, - Type = OrganizationUserType.User, - // Needs to be different from what is passed in as the savingUserId to Sut.SaveAsync - Email = "user1@test.com", - Name = "TEST", - UserId = Guid.NewGuid(), - HasMasterPassword = true - }; - var orgUserDetailUserWith2FANoMP = new OrganizationUserUserDetails - { - Id = Guid.NewGuid(), - Status = OrganizationUserStatusType.Confirmed, - Type = OrganizationUserType.User, - // Needs to be different from what is passed in as the savingUserId to Sut.SaveAsync - Email = "user2@test.com", - Name = "TEST", - UserId = Guid.NewGuid(), - HasMasterPassword = false - }; - var orgUserDetailUserWithout2FA = new OrganizationUserUserDetails - { - Id = Guid.NewGuid(), - Status = OrganizationUserStatusType.Confirmed, - Type = OrganizationUserType.User, - // Needs to be different from what is passed in as the savingUserId to Sut.SaveAsync - Email = "user3@test.com", - Name = "TEST", - UserId = Guid.NewGuid(), - HasMasterPassword = false - }; - var orgUserDetailAdmin = new OrganizationUserUserDetails - { - Id = Guid.NewGuid(), - Status = OrganizationUserStatusType.Confirmed, - Type = OrganizationUserType.Admin, - // Needs to be different from what is passed in as the savingUserId to Sut.SaveAsync - Email = "admin@test.com", - Name = "ADMIN", - UserId = Guid.NewGuid(), - HasMasterPassword = false - }; - - sutProvider.GetDependency() - .GetManyDetailsByOrganizationAsync(policy.OrganizationId) - .Returns(new List - { - orgUserDetailUserWith2FAAndMP, - orgUserDetailUserWith2FANoMP, - orgUserDetailUserWithout2FA, - orgUserDetailAdmin - }); - - 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), - }); - - var removeOrganizationUserCommand = sutProvider.GetDependency(); - - var savingUserId = Guid.NewGuid(); - - var badRequestException = await Assert.ThrowsAsync( - () => sutProvider.Sut.SaveAsync(policy, 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); - - await removeOrganizationUserCommand.DidNotReceiveWithAnyArgs() - .RemoveUserAsync(organizationId: default, organizationUserId: default, deletingUserId: default); - - await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() - .SendOrganizationUserRemovedForPolicyTwoStepEmailAsync(default, default); - - await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() - .LogPolicyEventAsync(default, default); - - await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() - .UpsertAsync(default); - } - - [Theory, BitAutoData] - public async Task SaveAsync_ExistingPolicy_UpdateSingleOrg( - [AdminConsoleFixtures.Policy(PolicyType.TwoFactorAuthentication)] Policy policy, SutProvider sutProvider) - { - // If the policy that this is updating isn't enabled then do some work now that the current one is enabled - - var org = new Organization - { - Id = policy.OrganizationId, - UsePolicies = true, - Name = "TEST", - }; - - SetupOrg(sutProvider, policy.OrganizationId, org); - - sutProvider.GetDependency() - .GetByIdAsync(policy.Id) - .Returns(new Policy - { - Id = policy.Id, - Type = PolicyType.SingleOrg, - Enabled = false, - }); - - var orgUserDetail = new Core.Models.Data.Organizations.OrganizationUsers.OrganizationUserUserDetails - { - Id = Guid.NewGuid(), - Status = OrganizationUserStatusType.Accepted, - Type = OrganizationUserType.User, - // Needs to be different from what is passed in as the savingUserId to Sut.SaveAsync - Email = "test@bitwarden.com", - Name = "TEST", - UserId = Guid.NewGuid(), - HasMasterPassword = true - }; - - sutProvider.GetDependency() - .GetManyDetailsByOrganizationAsync(policy.OrganizationId) - .Returns(new List - { - orgUserDetail, - }); - - sutProvider.GetDependency() - .TwoFactorIsEnabledAsync(Arg.Is>(ids => ids.Contains(orgUserDetail.UserId.Value))) - .Returns(new List<(Guid userId, bool hasTwoFactor)>() - { - (orgUserDetail.UserId.Value, false), - }); - - var utcNow = DateTime.UtcNow; - - var savingUserId = Guid.NewGuid(); - - await sutProvider.Sut.SaveAsync(policy, savingUserId); - - await sutProvider.GetDependency().Received() - .LogPolicyEventAsync(policy, EventType.Policy_Updated); - - await sutProvider.GetDependency().Received() - .UpsertAsync(policy); - - Assert.True(policy.CreationDate - utcNow < TimeSpan.FromSeconds(1)); - Assert.True(policy.RevisionDate - utcNow < TimeSpan.FromSeconds(1)); - } - - [Theory] - [BitAutoData(true, false)] - [BitAutoData(false, true)] - [BitAutoData(false, false)] - public async Task SaveAsync_ResetPasswordPolicyRequiredByTrustedDeviceEncryption_DisablePolicyOrDisableAutomaticEnrollment_ThrowsBadRequest( - bool policyEnabled, - bool autoEnrollEnabled, - [AdminConsoleFixtures.Policy(PolicyType.ResetPassword)] Policy policy, - SutProvider sutProvider) - { - policy.Enabled = policyEnabled; - policy.SetDataModel(new ResetPasswordDataModel - { - AutoEnrollEnabled = autoEnrollEnabled - }); - - SetupOrg(sutProvider, policy.OrganizationId, new Organization - { - Id = policy.OrganizationId, - UsePolicies = true, - }); - - var ssoConfig = new SsoConfig { Enabled = true }; - ssoConfig.SetData(new SsoConfigurationData { MemberDecryptionType = MemberDecryptionType.TrustedDeviceEncryption }); - - sutProvider.GetDependency() - .GetByOrganizationIdAsync(policy.OrganizationId) - .Returns(ssoConfig); - - var badRequestException = await Assert.ThrowsAsync( - () => sutProvider.Sut.SaveAsync(policy, - Guid.NewGuid())); - - Assert.Contains("Trusted device encryption is on and requires this policy.", badRequestException.Message, StringComparison.OrdinalIgnoreCase); - - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .UpsertAsync(default); - - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .LogPolicyEventAsync(default, default, default); - } - - [Theory, BitAutoData] - public async Task SaveAsync_RequireSsoPolicyRequiredByTrustedDeviceEncryption_DisablePolicy_ThrowsBadRequest( - [AdminConsoleFixtures.Policy(PolicyType.RequireSso)] Policy policy, - SutProvider sutProvider) - { - policy.Enabled = false; - - SetupOrg(sutProvider, policy.OrganizationId, new Organization - { - Id = policy.OrganizationId, - UsePolicies = true, - }); - - var ssoConfig = new SsoConfig { Enabled = true }; - ssoConfig.SetData(new SsoConfigurationData { MemberDecryptionType = MemberDecryptionType.TrustedDeviceEncryption }); - - sutProvider.GetDependency() - .GetByOrganizationIdAsync(policy.OrganizationId) - .Returns(ssoConfig); - - var badRequestException = await Assert.ThrowsAsync( - () => sutProvider.Sut.SaveAsync(policy, - Guid.NewGuid())); - - Assert.Contains("Trusted device encryption is on and requires this policy.", badRequestException.Message, StringComparison.OrdinalIgnoreCase); - - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .UpsertAsync(default); - - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .LogPolicyEventAsync(default, default, default); - } - - [Theory, BitAutoData] - public async Task SaveAsync_PolicyRequiredForAccountRecovery_NotEnabled_ThrowsBadRequestAsync( - [AdminConsoleFixtures.Policy(PolicyType.ResetPassword)] Policy policy, SutProvider sutProvider) - { - policy.Enabled = true; - policy.SetDataModel(new ResetPasswordDataModel()); - - SetupOrg(sutProvider, policy.OrganizationId, new Organization - { - Id = policy.OrganizationId, - UsePolicies = true, - }); - - sutProvider.GetDependency() - .GetByOrganizationIdTypeAsync(policy.OrganizationId, PolicyType.SingleOrg) - .Returns(Task.FromResult(new Policy { Enabled = false })); - - var badRequestException = await Assert.ThrowsAsync( - () => sutProvider.Sut.SaveAsync(policy, - Guid.NewGuid())); - - Assert.Contains("Single Organization policy not enabled.", badRequestException.Message, StringComparison.OrdinalIgnoreCase); - - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .UpsertAsync(default); - - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .LogPolicyEventAsync(default, default, default); - } - - - [Theory, BitAutoData] - public async Task SaveAsync_SingleOrg_AccountRecoveryEnabled_ThrowsBadRequest( - [AdminConsoleFixtures.Policy(PolicyType.SingleOrg)] Policy policy, SutProvider sutProvider) - { - policy.Enabled = false; - - SetupOrg(sutProvider, policy.OrganizationId, new Organization - { - Id = policy.OrganizationId, - UsePolicies = true, - }); - - sutProvider.GetDependency() - .GetByOrganizationIdTypeAsync(policy.OrganizationId, PolicyType.ResetPassword) - .Returns(new Policy { Enabled = true }); - - var badRequestException = await Assert.ThrowsAsync( - () => sutProvider.Sut.SaveAsync(policy, - Guid.NewGuid())); - - Assert.Contains("Account recovery policy is enabled.", badRequestException.Message, StringComparison.OrdinalIgnoreCase); - - await sutProvider.GetDependency() - .DidNotReceiveWithAnyArgs() - .UpsertAsync(default); - } - [Theory, BitAutoData] public async Task GetPoliciesApplicableToUserAsync_WithRequireSsoTypeFilter_WithDefaultOrganizationUserStatusFilter_ReturnsNoPolicies(Guid userId, SutProvider sutProvider) { @@ -816,32 +143,4 @@ public class PolicyServiceTests new() { OrganizationId = Guid.NewGuid(), PolicyType = PolicyType.DisableSend, PolicyEnabled = true, OrganizationUserType = OrganizationUserType.User, OrganizationUserStatus = OrganizationUserStatusType.Invited, IsProvider = true } }); } - - - [Theory, BitAutoData] - public async Task SaveAsync_GivenOrganizationUsingPoliciesAndHasVerifiedDomains_WhenSingleOrgPolicyIsDisabled_ThenAnErrorShouldBeThrownOrganizationHasVerifiedDomains( - [AdminConsoleFixtures.Policy(PolicyType.SingleOrg)] Policy policy, Organization org, SutProvider sutProvider) - { - org.Id = policy.OrganizationId; - org.UsePolicies = true; - - policy.Enabled = false; - - sutProvider.GetDependency() - .IsEnabled(FeatureFlagKeys.AccountDeprovisioning) - .Returns(true); - - sutProvider.GetDependency() - .GetByIdAsync(policy.OrganizationId) - .Returns(org); - - sutProvider.GetDependency() - .HasVerifiedDomainsAsync(org.Id) - .Returns(true); - - var badRequestException = await Assert.ThrowsAsync( - () => sutProvider.Sut.SaveAsync(policy, null)); - - Assert.Equal("The Single organization policy is required for organizations that have enabled domain verification.", badRequestException.Message); - } } diff --git a/test/Core.Test/Auth/Services/SsoConfigServiceTests.cs b/test/Core.Test/Auth/Services/SsoConfigServiceTests.cs index e397c838c..7beb772b9 100644 --- a/test/Core.Test/Auth/Services/SsoConfigServiceTests.cs +++ b/test/Core.Test/Auth/Services/SsoConfigServiceTests.cs @@ -1,8 +1,9 @@ using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies; +using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Models; 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.Data; @@ -338,16 +339,26 @@ public class SsoConfigServiceTests await sutProvider.Sut.SaveAsync(ssoConfig, organization); - await sutProvider.GetDependency().Received(1) + await sutProvider.GetDependency().Received(1) .SaveAsync( - Arg.Is(t => t.Type == PolicyType.SingleOrg), - null + Arg.Is(t => t.Type == PolicyType.SingleOrg && + t.OrganizationId == organization.Id && + t.Enabled) ); - await sutProvider.GetDependency().Received(1) + await sutProvider.GetDependency().Received(1) .SaveAsync( - Arg.Is(t => t.Type == PolicyType.ResetPassword && t.GetDataModel().AutoEnrollEnabled), - null + Arg.Is(t => t.Type == PolicyType.ResetPassword && + t.GetDataModel().AutoEnrollEnabled && + t.OrganizationId == organization.Id && + t.Enabled) + ); + + await sutProvider.GetDependency().Received(1) + .SaveAsync( + Arg.Is(t => t.Type == PolicyType.RequireSso && + t.OrganizationId == organization.Id && + t.Enabled) ); await sutProvider.GetDependency().ReceivedWithAnyArgs()