From c4ab5f31f5979319d4b7a9d3189117edaca3304c Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Mon, 25 Nov 2024 15:12:04 +0100 Subject: [PATCH 1/6] [deps] Tools: Update aws-sdk-net monorepo (#5065) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- src/Core/Core.csproj | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Core/Core.csproj b/src/Core/Core.csproj index 2f40fb7b23b0..d16624898dee 100644 --- a/src/Core/Core.csproj +++ b/src/Core/Core.csproj @@ -21,8 +21,8 @@ - - + + From 07592e22b9f4102e118d817c6384a7c4da604195 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Mon, 25 Nov 2024 16:17:59 +0100 Subject: [PATCH 2/6] [deps]: Update Microsoft.NET.Test.Sdk to 17.12.0 (#5067) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Daniel James Smith <2670567+djsmith85@users.noreply.github.com> --- .../Infrastructure.Dapper.Test.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Infrastructure.Dapper.Test/Infrastructure.Dapper.Test.csproj b/test/Infrastructure.Dapper.Test/Infrastructure.Dapper.Test.csproj index dfc8951cc38b..82d63bd3c19d 100644 --- a/test/Infrastructure.Dapper.Test/Infrastructure.Dapper.Test.csproj +++ b/test/Infrastructure.Dapper.Test/Infrastructure.Dapper.Test.csproj @@ -10,7 +10,7 @@ - + runtime; build; native; contentfiles; analyzers; buildtransitive From fd7ff2ac63f40cbee8457af1aa44e6bc9a1c0070 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Mon, 25 Nov 2024 14:30:02 -0500 Subject: [PATCH 3/6] [deps] Billing: Update FluentAssertions to 6.12.2 (#5015) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Alex Morask <144709477+amorask-bitwarden@users.noreply.github.com> --- test/Billing.Test/Billing.Test.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Billing.Test/Billing.Test.csproj b/test/Billing.Test/Billing.Test.csproj index 3bbda52ded40..c6a7ef48e0ab 100644 --- a/test/Billing.Test/Billing.Test.csproj +++ b/test/Billing.Test/Billing.Test.csproj @@ -6,7 +6,7 @@ - + From b974899127ee0154b20fb8ad8d254b17194ee523 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Mon, 25 Nov 2024 14:30:32 -0500 Subject: [PATCH 4/6] [deps] Billing: Update Braintree to 5.28.0 (#5019) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Alex Morask <144709477+amorask-bitwarden@users.noreply.github.com> --- src/Core/Core.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Core/Core.csproj b/src/Core/Core.csproj index d16624898dee..5970629caa00 100644 --- a/src/Core/Core.csproj +++ b/src/Core/Core.csproj @@ -54,7 +54,7 @@ - + From 8f703a29ac4dd46d8f5eacee09b9fbb73ede9031 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Tue, 26 Nov 2024 13:20:42 -0500 Subject: [PATCH 5/6] [deps] DbOps: Update Microsoft.Azure.Cosmos to 3.46.0 (#5066) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- src/Core/Core.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Core/Core.csproj b/src/Core/Core.csproj index 5970629caa00..fd4d8cc7e124 100644 --- a/src/Core/Core.csproj +++ b/src/Core/Core.csproj @@ -36,7 +36,7 @@ - + From 1b75e35c319f588d9e8a5fe7fd9ccd6591813cf1 Mon Sep 17 00:00:00 2001 From: Jared McCannon Date: Tue, 26 Nov 2024 16:37:12 -0600 Subject: [PATCH 6/6] [PM-10319] - Revoke Non Complaint Users for 2FA and Single Org Policy Enablement (#5037) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Revoking users when enabling single org and 2fa policies. - Updated emails sent when users are revoked via 2FA or Single Organization policy enablement Co-authored-by: Matt Bishop Co-authored-by: Rui Tomé <108268980+r-tome@users.noreply.github.com> --- .../AdminConsole/Enums/EventSystemUser.cs | 1 + .../AdminConsole/Models/Data/IActingUser.cs | 10 + .../AdminConsole/Models/Data/StandardUser.cs | 16 ++ .../AdminConsole/Models/Data/SystemUser.cs | 16 ++ .../VerifyOrganizationDomainCommand.cs | 93 +++++---- ...vokeNonCompliantOrganizationUserCommand.cs | 9 + .../Requests/RevokeOrganizationUserRequest.cs | 13 ++ ...vokeNonCompliantOrganizationUserCommand.cs | 112 +++++++++++ .../Policies/Models/PolicyUpdate.cs | 2 + .../SingleOrgPolicyValidator.cs | 70 ++++++- .../TwoFactorAuthenticationPolicyValidator.cs | 69 ++++++- .../IOrganizationUserRepository.cs | 2 + .../AdminConsole/Services/IPolicyService.cs | 2 +- .../Services/Implementations/PolicyService.cs | 15 +- ...tionUserRevokedForSingleOrgPolicy.html.hbs | 14 ++ ...tionUserRevokedForSingleOrgPolicy.text.hbs | 5 + ...tionUserRevokedForTwoFactorPolicy.html.hbs | 15 ++ ...tionUserRevokedForTwoFactorPolicy.text.hbs | 7 + src/Core/Models/Commands/CommandResult.cs | 12 ++ ...nUserRevokedForPolicySingleOrgViewModel.cs | 6 + ...nUserRevokedForPolicyTwoFactorViewModel.cs | 6 + ...OrganizationServiceCollectionExtensions.cs | 1 + src/Core/Services/IMailService.cs | 2 + .../Implementations/HandlebarsMailService.cs | 31 ++- .../NoopImplementations/NoopMailService.cs | 6 + .../OrganizationUserRepository.cs | 10 + .../OrganizationUserRepository.cs | 12 ++ ...OrganizationUser_SetStatusForUsersById.sql | 29 +++ .../AutoFixture/PolicyUpdateFixtures.cs | 4 +- .../VerifyOrganizationDomainCommandTests.cs | 38 +++- ...onCompliantOrganizationUserCommandTests.cs | 185 ++++++++++++++++++ .../SingleOrgPolicyValidatorTests.cs | 151 ++++++++++++++ ...actorAuthenticationPolicyValidatorTests.cs | 150 +++++++++++++- .../Policies/SavePolicyCommandTests.cs | 4 +- .../Services/EventServiceTests.cs | 1 - .../2024-11-26-00_OrgUserSetStatusBulk.sql | 28 +++ 36 files changed, 1074 insertions(+), 73 deletions(-) create mode 100644 src/Core/AdminConsole/Models/Data/IActingUser.cs create mode 100644 src/Core/AdminConsole/Models/Data/StandardUser.cs create mode 100644 src/Core/AdminConsole/Models/Data/SystemUser.cs create mode 100644 src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Interfaces/IRevokeNonCompliantOrganizationUserCommand.cs create mode 100644 src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Requests/RevokeOrganizationUserRequest.cs create mode 100644 src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RevokeNonCompliantOrganizationUserCommand.cs create mode 100644 src/Core/MailTemplates/Handlebars/AdminConsole/OrganizationUserRevokedForSingleOrgPolicy.html.hbs create mode 100644 src/Core/MailTemplates/Handlebars/AdminConsole/OrganizationUserRevokedForSingleOrgPolicy.text.hbs create mode 100644 src/Core/MailTemplates/Handlebars/AdminConsole/OrganizationUserRevokedForTwoFactorPolicy.html.hbs create mode 100644 src/Core/MailTemplates/Handlebars/AdminConsole/OrganizationUserRevokedForTwoFactorPolicy.text.hbs create mode 100644 src/Core/Models/Commands/CommandResult.cs create mode 100644 src/Core/Models/Mail/OrganizationUserRevokedForPolicySingleOrgViewModel.cs create mode 100644 src/Core/Models/Mail/OrganizationUserRevokedForPolicyTwoFactorViewModel.cs create mode 100644 src/Sql/dbo/Stored Procedures/OrganizationUser_SetStatusForUsersById.sql create mode 100644 test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RevokeNonCompliantOrganizationUserCommandTests.cs create mode 100644 util/Migrator/DbScripts/2024-11-26-00_OrgUserSetStatusBulk.sql diff --git a/src/Core/AdminConsole/Enums/EventSystemUser.cs b/src/Core/AdminConsole/Enums/EventSystemUser.cs index df9be9a350b7..c3e13705dd9f 100644 --- a/src/Core/AdminConsole/Enums/EventSystemUser.cs +++ b/src/Core/AdminConsole/Enums/EventSystemUser.cs @@ -2,6 +2,7 @@ public enum EventSystemUser : byte { + Unknown = 0, SCIM = 1, DomainVerification = 2, PublicApi = 3, diff --git a/src/Core/AdminConsole/Models/Data/IActingUser.cs b/src/Core/AdminConsole/Models/Data/IActingUser.cs new file mode 100644 index 000000000000..f97235f34cc4 --- /dev/null +++ b/src/Core/AdminConsole/Models/Data/IActingUser.cs @@ -0,0 +1,10 @@ +using Bit.Core.Enums; + +namespace Bit.Core.AdminConsole.Models.Data; + +public interface IActingUser +{ + Guid? UserId { get; } + bool IsOrganizationOwnerOrProvider { get; } + EventSystemUser? SystemUserType { get; } +} diff --git a/src/Core/AdminConsole/Models/Data/StandardUser.cs b/src/Core/AdminConsole/Models/Data/StandardUser.cs new file mode 100644 index 000000000000..f21a41db7c0e --- /dev/null +++ b/src/Core/AdminConsole/Models/Data/StandardUser.cs @@ -0,0 +1,16 @@ +using Bit.Core.Enums; + +namespace Bit.Core.AdminConsole.Models.Data; + +public class StandardUser : IActingUser +{ + public StandardUser(Guid userId, bool isOrganizationOwner) + { + UserId = userId; + IsOrganizationOwnerOrProvider = isOrganizationOwner; + } + + public Guid? UserId { get; } + public bool IsOrganizationOwnerOrProvider { get; } + public EventSystemUser? SystemUserType => throw new Exception($"{nameof(StandardUser)} does not have a {nameof(SystemUserType)}"); +} diff --git a/src/Core/AdminConsole/Models/Data/SystemUser.cs b/src/Core/AdminConsole/Models/Data/SystemUser.cs new file mode 100644 index 000000000000..c4859f928fa5 --- /dev/null +++ b/src/Core/AdminConsole/Models/Data/SystemUser.cs @@ -0,0 +1,16 @@ +using Bit.Core.Enums; + +namespace Bit.Core.AdminConsole.Models.Data; + +public class SystemUser : IActingUser +{ + public SystemUser(EventSystemUser systemUser) + { + SystemUserType = systemUser; + } + + public Guid? UserId => throw new Exception($"{nameof(SystemUserType)} does not have a {nameof(UserId)}."); + + public bool IsOrganizationOwnerOrProvider => false; + public EventSystemUser? SystemUserType { get; } +} diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationDomains/VerifyOrganizationDomainCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationDomains/VerifyOrganizationDomainCommand.cs index 870fa72aa709..dd6add669f2d 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationDomains/VerifyOrganizationDomainCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationDomains/VerifyOrganizationDomainCommand.cs @@ -1,7 +1,9 @@ using Bit.Core.AdminConsole.Entities; 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.Context; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; @@ -12,124 +14,121 @@ namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationDomains; -public class VerifyOrganizationDomainCommand : IVerifyOrganizationDomainCommand +public class VerifyOrganizationDomainCommand( + IOrganizationDomainRepository organizationDomainRepository, + IDnsResolverService dnsResolverService, + IEventService eventService, + IGlobalSettings globalSettings, + IPolicyService policyService, + IFeatureService featureService, + ICurrentContext currentContext, + ILogger logger) + : IVerifyOrganizationDomainCommand { - private readonly IOrganizationDomainRepository _organizationDomainRepository; - private readonly IDnsResolverService _dnsResolverService; - private readonly IEventService _eventService; - private readonly IGlobalSettings _globalSettings; - private readonly IPolicyService _policyService; - private readonly IFeatureService _featureService; - private readonly ILogger _logger; - - public VerifyOrganizationDomainCommand( - IOrganizationDomainRepository organizationDomainRepository, - IDnsResolverService dnsResolverService, - IEventService eventService, - IGlobalSettings globalSettings, - IPolicyService policyService, - IFeatureService featureService, - ILogger logger) - { - _organizationDomainRepository = organizationDomainRepository; - _dnsResolverService = dnsResolverService; - _eventService = eventService; - _globalSettings = globalSettings; - _policyService = policyService; - _featureService = featureService; - _logger = logger; - } public async Task UserVerifyOrganizationDomainAsync(OrganizationDomain organizationDomain) { - var domainVerificationResult = await VerifyOrganizationDomainAsync(organizationDomain); + if (currentContext.UserId is null) + { + throw new InvalidOperationException( + $"{nameof(UserVerifyOrganizationDomainAsync)} can only be called by a user. " + + $"Please call {nameof(SystemVerifyOrganizationDomainAsync)} for system users."); + } - await _eventService.LogOrganizationDomainEventAsync(domainVerificationResult, + var actingUser = new StandardUser(currentContext.UserId.Value, await currentContext.OrganizationOwner(organizationDomain.OrganizationId)); + + var domainVerificationResult = await VerifyOrganizationDomainAsync(organizationDomain, actingUser); + + await eventService.LogOrganizationDomainEventAsync(domainVerificationResult, domainVerificationResult.VerifiedDate != null ? EventType.OrganizationDomain_Verified : EventType.OrganizationDomain_NotVerified); - await _organizationDomainRepository.ReplaceAsync(domainVerificationResult); + await organizationDomainRepository.ReplaceAsync(domainVerificationResult); return domainVerificationResult; } public async Task SystemVerifyOrganizationDomainAsync(OrganizationDomain organizationDomain) { + var actingUser = new SystemUser(EventSystemUser.DomainVerification); + organizationDomain.SetJobRunCount(); - var domainVerificationResult = await VerifyOrganizationDomainAsync(organizationDomain); + var domainVerificationResult = await VerifyOrganizationDomainAsync(organizationDomain, actingUser); if (domainVerificationResult.VerifiedDate is not null) { - _logger.LogInformation(Constants.BypassFiltersEventId, "Successfully validated domain"); + logger.LogInformation(Constants.BypassFiltersEventId, "Successfully validated domain"); - await _eventService.LogOrganizationDomainEventAsync(domainVerificationResult, + await eventService.LogOrganizationDomainEventAsync(domainVerificationResult, EventType.OrganizationDomain_Verified, EventSystemUser.DomainVerification); } else { - domainVerificationResult.SetNextRunDate(_globalSettings.DomainVerification.VerificationInterval); + domainVerificationResult.SetNextRunDate(globalSettings.DomainVerification.VerificationInterval); - await _eventService.LogOrganizationDomainEventAsync(domainVerificationResult, + await eventService.LogOrganizationDomainEventAsync(domainVerificationResult, EventType.OrganizationDomain_NotVerified, EventSystemUser.DomainVerification); - _logger.LogInformation(Constants.BypassFiltersEventId, + logger.LogInformation(Constants.BypassFiltersEventId, "Verification for organization {OrgId} with domain {Domain} failed", domainVerificationResult.OrganizationId, domainVerificationResult.DomainName); } - await _organizationDomainRepository.ReplaceAsync(domainVerificationResult); + await organizationDomainRepository.ReplaceAsync(domainVerificationResult); return domainVerificationResult; } - private async Task VerifyOrganizationDomainAsync(OrganizationDomain domain) + private async Task VerifyOrganizationDomainAsync(OrganizationDomain domain, IActingUser actingUser) { domain.SetLastCheckedDate(); if (domain.VerifiedDate is not null) { - await _organizationDomainRepository.ReplaceAsync(domain); + await organizationDomainRepository.ReplaceAsync(domain); throw new ConflictException("Domain has already been verified."); } var claimedDomain = - await _organizationDomainRepository.GetClaimedDomainsByDomainNameAsync(domain.DomainName); + await organizationDomainRepository.GetClaimedDomainsByDomainNameAsync(domain.DomainName); if (claimedDomain.Count > 0) { - await _organizationDomainRepository.ReplaceAsync(domain); + await organizationDomainRepository.ReplaceAsync(domain); throw new ConflictException("The domain is not available to be claimed."); } try { - if (await _dnsResolverService.ResolveAsync(domain.DomainName, domain.Txt)) + if (await dnsResolverService.ResolveAsync(domain.DomainName, domain.Txt)) { domain.SetVerifiedDate(); - await EnableSingleOrganizationPolicyAsync(domain.OrganizationId); + await EnableSingleOrganizationPolicyAsync(domain.OrganizationId, actingUser); } } catch (Exception e) { - _logger.LogError("Error verifying Organization domain: {domain}. {errorMessage}", + logger.LogError("Error verifying Organization domain: {domain}. {errorMessage}", domain.DomainName, e.Message); } return domain; } - private async Task EnableSingleOrganizationPolicyAsync(Guid organizationId) + private async Task EnableSingleOrganizationPolicyAsync(Guid organizationId, IActingUser actingUser) { - if (_featureService.IsEnabled(FeatureFlagKeys.AccountDeprovisioning)) + if (featureService.IsEnabled(FeatureFlagKeys.AccountDeprovisioning)) { - await _policyService.SaveAsync( - new Policy { OrganizationId = organizationId, Type = PolicyType.SingleOrg, Enabled = true }, null); + 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); } } } diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Interfaces/IRevokeNonCompliantOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Interfaces/IRevokeNonCompliantOrganizationUserCommand.cs new file mode 100644 index 000000000000..c9768a890576 --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Interfaces/IRevokeNonCompliantOrganizationUserCommand.cs @@ -0,0 +1,9 @@ +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Requests; +using Bit.Core.Models.Commands; + +namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; + +public interface IRevokeNonCompliantOrganizationUserCommand +{ + Task RevokeNonCompliantOrganizationUsersAsync(RevokeOrganizationUsersRequest request); +} diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Requests/RevokeOrganizationUserRequest.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Requests/RevokeOrganizationUserRequest.cs new file mode 100644 index 000000000000..88f1dc8aa1f0 --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Requests/RevokeOrganizationUserRequest.cs @@ -0,0 +1,13 @@ +using Bit.Core.AdminConsole.Models.Data; +using Bit.Core.Models.Data.Organizations.OrganizationUsers; + +namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Requests; + +public record RevokeOrganizationUsersRequest( + Guid OrganizationId, + IEnumerable OrganizationUsers, + IActingUser ActionPerformedBy) +{ + public RevokeOrganizationUsersRequest(Guid organizationId, OrganizationUserUserDetails organizationUser, IActingUser actionPerformedBy) + : this(organizationId, [organizationUser], actionPerformedBy) { } +} diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RevokeNonCompliantOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RevokeNonCompliantOrganizationUserCommand.cs new file mode 100644 index 000000000000..971ed02b29e9 --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RevokeNonCompliantOrganizationUserCommand.cs @@ -0,0 +1,112 @@ +using Bit.Core.AdminConsole.Models.Data; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Requests; +using Bit.Core.Enums; +using Bit.Core.Models.Commands; +using Bit.Core.Models.Data.Organizations.OrganizationUsers; +using Bit.Core.Repositories; +using Bit.Core.Services; + +namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers; + +public class RevokeNonCompliantOrganizationUserCommand(IOrganizationUserRepository organizationUserRepository, + IEventService eventService, + IHasConfirmedOwnersExceptQuery confirmedOwnersExceptQuery, + TimeProvider timeProvider) : IRevokeNonCompliantOrganizationUserCommand +{ + public const string ErrorCannotRevokeSelf = "You cannot revoke yourself."; + public const string ErrorOnlyOwnersCanRevokeOtherOwners = "Only owners can revoke other owners."; + public const string ErrorUserAlreadyRevoked = "User is already revoked."; + public const string ErrorOrgMustHaveAtLeastOneOwner = "Organization must have at least one confirmed owner."; + public const string ErrorInvalidUsers = "Invalid users."; + public const string ErrorRequestedByWasNotValid = "Action was performed by an unexpected type."; + + public async Task RevokeNonCompliantOrganizationUsersAsync(RevokeOrganizationUsersRequest request) + { + var validationResult = await ValidateAsync(request); + + if (validationResult.HasErrors) + { + return validationResult; + } + + await organizationUserRepository.RevokeManyByIdAsync(request.OrganizationUsers.Select(x => x.Id)); + + var now = timeProvider.GetUtcNow(); + + switch (request.ActionPerformedBy) + { + case StandardUser: + await eventService.LogOrganizationUserEventsAsync( + request.OrganizationUsers.Select(x => GetRevokedUserEventTuple(x, now))); + break; + case SystemUser { SystemUserType: not null } loggableSystem: + await eventService.LogOrganizationUserEventsAsync( + request.OrganizationUsers.Select(x => + GetRevokedUserEventBySystemUserTuple(x, loggableSystem.SystemUserType.Value, now))); + break; + } + + return validationResult; + } + + private static (OrganizationUserUserDetails organizationUser, EventType eventType, DateTime? time) GetRevokedUserEventTuple( + OrganizationUserUserDetails organizationUser, DateTimeOffset dateTimeOffset) => + new(organizationUser, EventType.OrganizationUser_Revoked, dateTimeOffset.UtcDateTime); + + private static (OrganizationUserUserDetails organizationUser, EventType eventType, EventSystemUser eventSystemUser, DateTime? time) GetRevokedUserEventBySystemUserTuple( + OrganizationUserUserDetails organizationUser, EventSystemUser systemUser, DateTimeOffset dateTimeOffset) => new(organizationUser, + EventType.OrganizationUser_Revoked, systemUser, dateTimeOffset.UtcDateTime); + + private async Task ValidateAsync(RevokeOrganizationUsersRequest request) + { + if (!PerformedByIsAnExpectedType(request.ActionPerformedBy)) + { + return new CommandResult(ErrorRequestedByWasNotValid); + } + + if (request.ActionPerformedBy is StandardUser user + && request.OrganizationUsers.Any(x => x.UserId == user.UserId)) + { + return new CommandResult(ErrorCannotRevokeSelf); + } + + if (request.OrganizationUsers.Any(x => x.OrganizationId != request.OrganizationId)) + { + return new CommandResult(ErrorInvalidUsers); + } + + if (!await confirmedOwnersExceptQuery.HasConfirmedOwnersExceptAsync( + request.OrganizationId, + request.OrganizationUsers.Select(x => x.Id))) + { + return new CommandResult(ErrorOrgMustHaveAtLeastOneOwner); + } + + return request.OrganizationUsers.Aggregate(new CommandResult(), (result, userToRevoke) => + { + if (IsAlreadyRevoked(userToRevoke)) + { + result.ErrorMessages.Add($"{ErrorUserAlreadyRevoked} Id: {userToRevoke.Id}"); + return result; + } + + if (NonOwnersCannotRevokeOwners(userToRevoke, request.ActionPerformedBy)) + { + result.ErrorMessages.Add($"{ErrorOnlyOwnersCanRevokeOtherOwners}"); + return result; + } + + return result; + }); + } + + private static bool PerformedByIsAnExpectedType(IActingUser entity) => entity is SystemUser or StandardUser; + + private static bool IsAlreadyRevoked(OrganizationUserUserDetails organizationUser) => + organizationUser is { Status: OrganizationUserStatusType.Revoked }; + + private static bool NonOwnersCannotRevokeOwners(OrganizationUserUserDetails organizationUser, + IActingUser actingUser) => + actingUser is StandardUser { IsOrganizationOwnerOrProvider: false } && organizationUser.Type == OrganizationUserType.Owner; +} diff --git a/src/Core/AdminConsole/OrganizationFeatures/Policies/Models/PolicyUpdate.cs b/src/Core/AdminConsole/OrganizationFeatures/Policies/Models/PolicyUpdate.cs index 117a7ec73396..d1a52f008011 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/Policies/Models/PolicyUpdate.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/Policies/Models/PolicyUpdate.cs @@ -1,6 +1,7 @@ #nullable enable using Bit.Core.AdminConsole.Enums; +using Bit.Core.AdminConsole.Models.Data; using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; using Bit.Core.Utilities; @@ -15,6 +16,7 @@ public record PolicyUpdate public PolicyType Type { get; set; } public string? Data { get; set; } public bool Enabled { get; set; } + public IActingUser? PerformedBy { get; set; } public T GetDataModel() where T : IPolicyDataModel, new() { diff --git a/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyValidators/SingleOrgPolicyValidator.cs b/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyValidators/SingleOrgPolicyValidator.cs index cc6971f9462a..050949ee7f91 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyValidators/SingleOrgPolicyValidator.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyValidators/SingleOrgPolicyValidator.cs @@ -2,8 +2,10 @@ using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Enums; +using Bit.Core.AdminConsole.Models.Data; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationDomains.Interfaces; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Requests; using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Models; using Bit.Core.Auth.Enums; using Bit.Core.Auth.Repositories; @@ -18,6 +20,8 @@ namespace Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyValidators; public class SingleOrgPolicyValidator : IPolicyValidator { public PolicyType Type => PolicyType.SingleOrg; + private const string OrganizationNotFoundErrorMessage = "Organization not found."; + private const string ClaimedDomainSingleOrganizationRequiredErrorMessage = "The Single organization policy is required for organizations that have enabled domain verification."; private readonly IOrganizationUserRepository _organizationUserRepository; private readonly IMailService _mailService; @@ -27,6 +31,7 @@ public class SingleOrgPolicyValidator : IPolicyValidator private readonly IFeatureService _featureService; private readonly IRemoveOrganizationUserCommand _removeOrganizationUserCommand; private readonly IOrganizationHasVerifiedDomainsQuery _organizationHasVerifiedDomainsQuery; + private readonly IRevokeNonCompliantOrganizationUserCommand _revokeNonCompliantOrganizationUserCommand; public SingleOrgPolicyValidator( IOrganizationUserRepository organizationUserRepository, @@ -36,7 +41,8 @@ public SingleOrgPolicyValidator( ICurrentContext currentContext, IFeatureService featureService, IRemoveOrganizationUserCommand removeOrganizationUserCommand, - IOrganizationHasVerifiedDomainsQuery organizationHasVerifiedDomainsQuery) + IOrganizationHasVerifiedDomainsQuery organizationHasVerifiedDomainsQuery, + IRevokeNonCompliantOrganizationUserCommand revokeNonCompliantOrganizationUserCommand) { _organizationUserRepository = organizationUserRepository; _mailService = mailService; @@ -46,6 +52,7 @@ public SingleOrgPolicyValidator( _featureService = featureService; _removeOrganizationUserCommand = removeOrganizationUserCommand; _organizationHasVerifiedDomainsQuery = organizationHasVerifiedDomainsQuery; + _revokeNonCompliantOrganizationUserCommand = revokeNonCompliantOrganizationUserCommand; } public IEnumerable RequiredPolicies => []; @@ -54,8 +61,52 @@ public async Task OnSaveSideEffectsAsync(PolicyUpdate policyUpdate, Policy? curr { if (currentPolicy is not { Enabled: true } && policyUpdate is { Enabled: true }) { - await RemoveNonCompliantUsersAsync(policyUpdate.OrganizationId); + if (_featureService.IsEnabled(FeatureFlagKeys.AccountDeprovisioning)) + { + var currentUser = _currentContext.UserId ?? Guid.Empty; + var isOwnerOrProvider = await _currentContext.OrganizationOwner(policyUpdate.OrganizationId); + await RevokeNonCompliantUsersAsync(policyUpdate.OrganizationId, policyUpdate.PerformedBy ?? new StandardUser(currentUser, isOwnerOrProvider)); + } + else + { + await RemoveNonCompliantUsersAsync(policyUpdate.OrganizationId); + } + } + } + + private async Task RevokeNonCompliantUsersAsync(Guid organizationId, IActingUser performedBy) + { + var organization = await _organizationRepository.GetByIdAsync(organizationId); + + if (organization is null) + { + throw new NotFoundException(OrganizationNotFoundErrorMessage); } + + var currentActiveRevocableOrganizationUsers = + (await _organizationUserRepository.GetManyDetailsByOrganizationAsync(organizationId)) + .Where(ou => ou.Status != OrganizationUserStatusType.Invited && + ou.Status != OrganizationUserStatusType.Revoked && + ou.Type != OrganizationUserType.Owner && + ou.Type != OrganizationUserType.Admin && + !(performedBy is StandardUser stdUser && stdUser.UserId == ou.UserId)) + .ToList(); + + if (currentActiveRevocableOrganizationUsers.Count == 0) + { + return; + } + + var commandResult = await _revokeNonCompliantOrganizationUserCommand.RevokeNonCompliantOrganizationUsersAsync( + new RevokeOrganizationUsersRequest(organizationId, currentActiveRevocableOrganizationUsers, performedBy)); + + if (commandResult.HasErrors) + { + throw new BadRequestException(string.Join(", ", commandResult.ErrorMessages)); + } + + await Task.WhenAll(currentActiveRevocableOrganizationUsers.Select(x => + _mailService.SendOrganizationUserRevokedForPolicySingleOrgEmailAsync(organization.DisplayName(), x.Email))); } private async Task RemoveNonCompliantUsersAsync(Guid organizationId) @@ -67,7 +118,7 @@ private async Task RemoveNonCompliantUsersAsync(Guid organizationId) var org = await _organizationRepository.GetByIdAsync(organizationId); if (org == null) { - throw new NotFoundException("Organization not found."); + throw new NotFoundException(OrganizationNotFoundErrorMessage); } var removableOrgUsers = orgUsers.Where(ou => @@ -76,18 +127,17 @@ private async Task RemoveNonCompliantUsersAsync(Guid organizationId) ou.Type != OrganizationUserType.Owner && ou.Type != OrganizationUserType.Admin && ou.UserId != savingUserId - ).ToList(); + ).ToList(); var userOrgs = await _organizationUserRepository.GetManyByManyUsersAsync( - removableOrgUsers.Select(ou => ou.UserId!.Value)); + 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)) + && ou.OrganizationId != org.Id + && ou.Status != OrganizationUserStatusType.Invited)) { - await _removeOrganizationUserCommand.RemoveUserAsync(organizationId, orgUser.Id, - savingUserId); + await _removeOrganizationUserCommand.RemoveUserAsync(organizationId, orgUser.Id, savingUserId); await _mailService.SendOrganizationUserRemovedForPolicySingleOrgEmailAsync( org.DisplayName(), orgUser.Email); @@ -111,7 +161,7 @@ public async Task ValidateAsync(PolicyUpdate policyUpdate, Policy? curre if (_featureService.IsEnabled(FeatureFlagKeys.AccountDeprovisioning) && await _organizationHasVerifiedDomainsQuery.HasVerifiedDomainsAsync(policyUpdate.OrganizationId)) { - return "The Single organization policy is required for organizations that have enabled domain verification."; + return ClaimedDomainSingleOrganizationRequiredErrorMessage; } } diff --git a/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyValidators/TwoFactorAuthenticationPolicyValidator.cs b/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyValidators/TwoFactorAuthenticationPolicyValidator.cs index ef896bbb9b57..c2dd8cff91c8 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyValidators/TwoFactorAuthenticationPolicyValidator.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyValidators/TwoFactorAuthenticationPolicyValidator.cs @@ -2,12 +2,15 @@ using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Enums; +using Bit.Core.AdminConsole.Models.Data; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Requests; using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Models; using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; using Bit.Core.Context; using Bit.Core.Enums; using Bit.Core.Exceptions; +using Bit.Core.Models.Data.Organizations.OrganizationUsers; using Bit.Core.Repositories; using Bit.Core.Services; @@ -21,6 +24,10 @@ public class TwoFactorAuthenticationPolicyValidator : IPolicyValidator private readonly ICurrentContext _currentContext; private readonly ITwoFactorIsEnabledQuery _twoFactorIsEnabledQuery; private readonly IRemoveOrganizationUserCommand _removeOrganizationUserCommand; + private readonly IFeatureService _featureService; + private readonly IRevokeNonCompliantOrganizationUserCommand _revokeNonCompliantOrganizationUserCommand; + + public const string NonCompliantMembersWillLoseAccessMessage = "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."; public PolicyType Type => PolicyType.TwoFactorAuthentication; public IEnumerable RequiredPolicies => []; @@ -31,7 +38,9 @@ public TwoFactorAuthenticationPolicyValidator( IOrganizationRepository organizationRepository, ICurrentContext currentContext, ITwoFactorIsEnabledQuery twoFactorIsEnabledQuery, - IRemoveOrganizationUserCommand removeOrganizationUserCommand) + IRemoveOrganizationUserCommand removeOrganizationUserCommand, + IFeatureService featureService, + IRevokeNonCompliantOrganizationUserCommand revokeNonCompliantOrganizationUserCommand) { _organizationUserRepository = organizationUserRepository; _mailService = mailService; @@ -39,14 +48,63 @@ public TwoFactorAuthenticationPolicyValidator( _currentContext = currentContext; _twoFactorIsEnabledQuery = twoFactorIsEnabledQuery; _removeOrganizationUserCommand = removeOrganizationUserCommand; + _featureService = featureService; + _revokeNonCompliantOrganizationUserCommand = revokeNonCompliantOrganizationUserCommand; } public async Task OnSaveSideEffectsAsync(PolicyUpdate policyUpdate, Policy? currentPolicy) { if (currentPolicy is not { Enabled: true } && policyUpdate is { Enabled: true }) { - await RemoveNonCompliantUsersAsync(policyUpdate.OrganizationId); + if (_featureService.IsEnabled(FeatureFlagKeys.AccountDeprovisioning)) + { + var currentUser = _currentContext.UserId ?? Guid.Empty; + var isOwnerOrProvider = await _currentContext.OrganizationOwner(policyUpdate.OrganizationId); + await RevokeNonCompliantUsersAsync(policyUpdate.OrganizationId, policyUpdate.PerformedBy ?? new StandardUser(currentUser, isOwnerOrProvider)); + } + else + { + await RemoveNonCompliantUsersAsync(policyUpdate.OrganizationId); + } + } + } + + private async Task RevokeNonCompliantUsersAsync(Guid organizationId, IActingUser performedBy) + { + var organization = await _organizationRepository.GetByIdAsync(organizationId); + + var currentActiveRevocableOrganizationUsers = + (await _organizationUserRepository.GetManyDetailsByOrganizationAsync(organizationId)) + .Where(ou => ou.Status != OrganizationUserStatusType.Invited && + ou.Status != OrganizationUserStatusType.Revoked && + ou.Type != OrganizationUserType.Owner && + ou.Type != OrganizationUserType.Admin && + !(performedBy is StandardUser stdUser && stdUser.UserId == ou.UserId)) + .ToList(); + + if (currentActiveRevocableOrganizationUsers.Count == 0) + { + return; + } + + var organizationUsersTwoFactorEnabled = + await _twoFactorIsEnabledQuery.TwoFactorIsEnabledAsync(currentActiveRevocableOrganizationUsers); + + if (NonCompliantMembersWillLoseAccess(currentActiveRevocableOrganizationUsers, organizationUsersTwoFactorEnabled)) + { + throw new BadRequestException(NonCompliantMembersWillLoseAccessMessage); } + + var commandResult = await _revokeNonCompliantOrganizationUserCommand.RevokeNonCompliantOrganizationUsersAsync( + new RevokeOrganizationUsersRequest(organizationId, currentActiveRevocableOrganizationUsers, performedBy)); + + if (commandResult.HasErrors) + { + throw new BadRequestException(string.Join(", ", commandResult.ErrorMessages)); + } + + await Task.WhenAll(currentActiveRevocableOrganizationUsers.Select(x => + _mailService.SendOrganizationUserRevokedForPolicySingleOrgEmailAsync(organization.DisplayName(), x.Email))); } private async Task RemoveNonCompliantUsersAsync(Guid organizationId) @@ -83,5 +141,12 @@ await _mailService.SendOrganizationUserRemovedForPolicyTwoStepEmailAsync( } } + private static bool NonCompliantMembersWillLoseAccess( + IEnumerable orgUserDetails, + IEnumerable<(OrganizationUserUserDetails user, bool isTwoFactorEnabled)> organizationUsersTwoFactorEnabled) => + orgUserDetails.Any(x => + !x.HasMasterPassword && !organizationUsersTwoFactorEnabled.FirstOrDefault(u => u.user.Id == x.Id) + .isTwoFactorEnabled); + public Task ValidateAsync(PolicyUpdate policyUpdate, Policy? currentPolicy) => Task.FromResult(""); } diff --git a/src/Core/AdminConsole/Repositories/IOrganizationUserRepository.cs b/src/Core/AdminConsole/Repositories/IOrganizationUserRepository.cs index cb540c212b5b..516b4614af49 100644 --- a/src/Core/AdminConsole/Repositories/IOrganizationUserRepository.cs +++ b/src/Core/AdminConsole/Repositories/IOrganizationUserRepository.cs @@ -58,4 +58,6 @@ UpdateEncryptedDataForKeyRotation UpdateForKeyRotation(Guid userId, /// Returns a list of OrganizationUsers with email domains that match one of the Organization's claimed domains. /// Task> GetManyByOrganizationWithClaimedDomainsAsync(Guid organizationId); + + Task RevokeManyByIdAsync(IEnumerable organizationUserIds); } diff --git a/src/Core/AdminConsole/Services/IPolicyService.cs b/src/Core/AdminConsole/Services/IPolicyService.cs index 16ff2f4fa11c..715c3a34d934 100644 --- a/src/Core/AdminConsole/Services/IPolicyService.cs +++ b/src/Core/AdminConsole/Services/IPolicyService.cs @@ -9,7 +9,7 @@ namespace Bit.Core.AdminConsole.Services; public interface IPolicyService { - Task SaveAsync(Policy policy, Guid? savingUserId); + 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 42655040a3c1..69ec27fd8a2a 100644 --- a/src/Core/AdminConsole/Services/Implementations/PolicyService.cs +++ b/src/Core/AdminConsole/Services/Implementations/PolicyService.cs @@ -1,5 +1,6 @@ using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Enums; +using Bit.Core.AdminConsole.Models.Data; using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationDomains.Interfaces; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; @@ -9,6 +10,7 @@ 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; @@ -34,6 +36,7 @@ public class PolicyService : IPolicyService private readonly ISavePolicyCommand _savePolicyCommand; private readonly IRemoveOrganizationUserCommand _removeOrganizationUserCommand; private readonly IOrganizationHasVerifiedDomainsQuery _organizationHasVerifiedDomainsQuery; + private readonly ICurrentContext _currentContext; public PolicyService( IApplicationCacheService applicationCacheService, @@ -48,7 +51,8 @@ public PolicyService( IFeatureService featureService, ISavePolicyCommand savePolicyCommand, IRemoveOrganizationUserCommand removeOrganizationUserCommand, - IOrganizationHasVerifiedDomainsQuery organizationHasVerifiedDomainsQuery) + IOrganizationHasVerifiedDomainsQuery organizationHasVerifiedDomainsQuery, + ICurrentContext currentContext) { _applicationCacheService = applicationCacheService; _eventService = eventService; @@ -63,19 +67,24 @@ public PolicyService( _savePolicyCommand = savePolicyCommand; _removeOrganizationUserCommand = removeOrganizationUserCommand; _organizationHasVerifiedDomainsQuery = organizationHasVerifiedDomainsQuery; + _currentContext = currentContext; } - public async Task SaveAsync(Policy policy, Guid? savingUserId) + 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 + Data = policy.Data, + PerformedBy = savingUserId.HasValue + ? new StandardUser(savingUserId.Value, await _currentContext.OrganizationOwner(policy.OrganizationId)) + : new SystemUser(eventSystemUser ?? EventSystemUser.Unknown) }; await _savePolicyCommand.SaveAsync(policyUpdate); diff --git a/src/Core/MailTemplates/Handlebars/AdminConsole/OrganizationUserRevokedForSingleOrgPolicy.html.hbs b/src/Core/MailTemplates/Handlebars/AdminConsole/OrganizationUserRevokedForSingleOrgPolicy.html.hbs new file mode 100644 index 000000000000..d04abe86c935 --- /dev/null +++ b/src/Core/MailTemplates/Handlebars/AdminConsole/OrganizationUserRevokedForSingleOrgPolicy.html.hbs @@ -0,0 +1,14 @@ +{{#>FullHtmlLayout}} + + + + + + + +
+ Your user account has been revoked from the {{OrganizationName}} organization because your account is part of multiple organizations. Before you can re-join {{OrganizationName}}, you must first leave all other organizations. +
+ To leave an organization, first log into the web app, select the three dot menu next to the organization name, and select Leave. +
+{{/FullHtmlLayout}} diff --git a/src/Core/MailTemplates/Handlebars/AdminConsole/OrganizationUserRevokedForSingleOrgPolicy.text.hbs b/src/Core/MailTemplates/Handlebars/AdminConsole/OrganizationUserRevokedForSingleOrgPolicy.text.hbs new file mode 100644 index 000000000000..f933e8cf625e --- /dev/null +++ b/src/Core/MailTemplates/Handlebars/AdminConsole/OrganizationUserRevokedForSingleOrgPolicy.text.hbs @@ -0,0 +1,5 @@ +{{#>BasicTextLayout}} +Your user account has been revoked from the {{OrganizationName}} organization because your account is part of multiple organizations. Before you can rejoin {{OrganizationName}}, you must first leave all other organizations. + +To leave an organization, first log in the web app (https://vault.bitwarden.com/#/login), select the three dot menu next to the organization name, and select Leave. +{{/BasicTextLayout}} diff --git a/src/Core/MailTemplates/Handlebars/AdminConsole/OrganizationUserRevokedForTwoFactorPolicy.html.hbs b/src/Core/MailTemplates/Handlebars/AdminConsole/OrganizationUserRevokedForTwoFactorPolicy.html.hbs new file mode 100644 index 000000000000..cf38632a9e1f --- /dev/null +++ b/src/Core/MailTemplates/Handlebars/AdminConsole/OrganizationUserRevokedForTwoFactorPolicy.html.hbs @@ -0,0 +1,15 @@ +{{#>FullHtmlLayout}} + + + + + + + +
+ Your user account has been revoked from the {{OrganizationName}} organization because you do not have two-step login configured. Before you can re-join {{OrganizationName}}, you need to set up two-step login on your user account. +
+ Learn how to enable two-step login on your user account at + https://help.bitwarden.com/article/setup-two-step-login/ +
+{{/FullHtmlLayout}} diff --git a/src/Core/MailTemplates/Handlebars/AdminConsole/OrganizationUserRevokedForTwoFactorPolicy.text.hbs b/src/Core/MailTemplates/Handlebars/AdminConsole/OrganizationUserRevokedForTwoFactorPolicy.text.hbs new file mode 100644 index 000000000000..f197f37f00e5 --- /dev/null +++ b/src/Core/MailTemplates/Handlebars/AdminConsole/OrganizationUserRevokedForTwoFactorPolicy.text.hbs @@ -0,0 +1,7 @@ +{{#>BasicTextLayout}} + Your user account has been removed from the {{OrganizationName}} organization because you do not have two-step login + configured. Before you can re-join this organization you need to set up two-step login on your user account. + + Learn how to enable two-step login on your user account at + https://help.bitwarden.com/article/setup-two-step-login/ +{{/BasicTextLayout}} diff --git a/src/Core/Models/Commands/CommandResult.cs b/src/Core/Models/Commands/CommandResult.cs new file mode 100644 index 000000000000..4ac2d6249918 --- /dev/null +++ b/src/Core/Models/Commands/CommandResult.cs @@ -0,0 +1,12 @@ +namespace Bit.Core.Models.Commands; + +public class CommandResult(IEnumerable errors) +{ + public CommandResult(string error) : this([error]) { } + + public bool Success => ErrorMessages.Count == 0; + public bool HasErrors => ErrorMessages.Count > 0; + public List ErrorMessages { get; } = errors.ToList(); + + public CommandResult() : this([]) { } +} diff --git a/src/Core/Models/Mail/OrganizationUserRevokedForPolicySingleOrgViewModel.cs b/src/Core/Models/Mail/OrganizationUserRevokedForPolicySingleOrgViewModel.cs new file mode 100644 index 000000000000..27c784bd15dd --- /dev/null +++ b/src/Core/Models/Mail/OrganizationUserRevokedForPolicySingleOrgViewModel.cs @@ -0,0 +1,6 @@ +namespace Bit.Core.Models.Mail; + +public class OrganizationUserRevokedForPolicySingleOrgViewModel : BaseMailModel +{ + public string OrganizationName { get; set; } +} diff --git a/src/Core/Models/Mail/OrganizationUserRevokedForPolicyTwoFactorViewModel.cs b/src/Core/Models/Mail/OrganizationUserRevokedForPolicyTwoFactorViewModel.cs new file mode 100644 index 000000000000..9286ee74b3d2 --- /dev/null +++ b/src/Core/Models/Mail/OrganizationUserRevokedForPolicyTwoFactorViewModel.cs @@ -0,0 +1,6 @@ +namespace Bit.Core.Models.Mail; + +public class OrganizationUserRevokedForPolicyTwoFactorViewModel : BaseMailModel +{ + public string OrganizationName { get; set; } +} diff --git a/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs b/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs index d11da2119aa5..96fdefcfad44 100644 --- a/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs +++ b/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs @@ -91,6 +91,7 @@ private static void AddOrganizationSponsorshipCommands(this IServiceCollection s private static void AddOrganizationUserCommands(this IServiceCollection services) { services.AddScoped(); + services.AddScoped(); services.AddScoped(); services.AddScoped(); services.AddScoped(); diff --git a/src/Core/Services/IMailService.cs b/src/Core/Services/IMailService.cs index 5514cd507d32..bc8d1440f155 100644 --- a/src/Core/Services/IMailService.cs +++ b/src/Core/Services/IMailService.cs @@ -35,6 +35,8 @@ Task SendTrialInitiationSignupEmailAsync( Task SendOrganizationAcceptedEmailAsync(Organization organization, string userIdentifier, IEnumerable adminEmails, bool hasAccessSecretsManager = false); Task SendOrganizationConfirmedEmailAsync(string organizationName, string email, bool hasAccessSecretsManager = false); Task SendOrganizationUserRemovedForPolicyTwoStepEmailAsync(string organizationName, string email); + Task SendOrganizationUserRevokedForTwoFactoryPolicyEmailAsync(string organizationName, string email); + Task SendOrganizationUserRevokedForPolicySingleOrgEmailAsync(string organizationName, string email); Task SendPasswordlessSignInAsync(string returnUrl, string token, string email); Task SendInvoiceUpcoming( string email, diff --git a/src/Core/Services/Implementations/HandlebarsMailService.cs b/src/Core/Services/Implementations/HandlebarsMailService.cs index e1943b0e3c78..acc729e53ce0 100644 --- a/src/Core/Services/Implementations/HandlebarsMailService.cs +++ b/src/Core/Services/Implementations/HandlebarsMailService.cs @@ -25,8 +25,7 @@ public class HandlebarsMailService : IMailService private readonly GlobalSettings _globalSettings; private readonly IMailDeliveryService _mailDeliveryService; private readonly IMailEnqueuingService _mailEnqueuingService; - private readonly Dictionary> _templateCache = - new Dictionary>(); + private readonly Dictionary> _templateCache = new(); private bool _registeredHelpersAndPartials = false; @@ -295,6 +294,20 @@ public async Task SendOrganizationUserRemovedForPolicyTwoStepEmailAsync(string o await _mailDeliveryService.SendEmailAsync(message); } + public async Task SendOrganizationUserRevokedForTwoFactoryPolicyEmailAsync(string organizationName, string email) + { + var message = CreateDefaultMessage($"You have been revoked from {organizationName}", email); + var model = new OrganizationUserRevokedForPolicyTwoFactorViewModel + { + OrganizationName = CoreHelpers.SanitizeForEmail(organizationName, false), + WebVaultUrl = _globalSettings.BaseServiceUri.VaultWithHash, + SiteName = _globalSettings.SiteName + }; + await AddMessageContentAsync(message, "AdminConsole.OrganizationUserRevokedForTwoFactorPolicy", model); + message.Category = "OrganizationUserRevokedForTwoFactorPolicy"; + await _mailDeliveryService.SendEmailAsync(message); + } + public async Task SendWelcomeEmailAsync(User user) { var message = CreateDefaultMessage("Welcome to Bitwarden!", user.Email); @@ -496,6 +509,20 @@ public async Task SendOrganizationUserRemovedForPolicySingleOrgEmailAsync(string await _mailDeliveryService.SendEmailAsync(message); } + public async Task SendOrganizationUserRevokedForPolicySingleOrgEmailAsync(string organizationName, string email) + { + var message = CreateDefaultMessage($"You have been revoked from {organizationName}", email); + var model = new OrganizationUserRevokedForPolicySingleOrgViewModel + { + OrganizationName = CoreHelpers.SanitizeForEmail(organizationName, false), + WebVaultUrl = _globalSettings.BaseServiceUri.VaultWithHash, + SiteName = _globalSettings.SiteName + }; + await AddMessageContentAsync(message, "AdminConsole.OrganizationUserRevokedForSingleOrgPolicy", model); + message.Category = "OrganizationUserRevokedForSingleOrgPolicy"; + await _mailDeliveryService.SendEmailAsync(message); + } + public async Task SendEnqueuedMailMessageAsync(IMailQueueMessage queueMessage) { var message = CreateDefaultMessage(queueMessage.Subject, queueMessage.ToEmails); diff --git a/src/Core/Services/NoopImplementations/NoopMailService.cs b/src/Core/Services/NoopImplementations/NoopMailService.cs index a56858fb96f5..399874eee73d 100644 --- a/src/Core/Services/NoopImplementations/NoopMailService.cs +++ b/src/Core/Services/NoopImplementations/NoopMailService.cs @@ -79,6 +79,12 @@ public Task SendOrganizationUserRemovedForPolicyTwoStepEmailAsync(string organiz return Task.FromResult(0); } + public Task SendOrganizationUserRevokedForTwoFactoryPolicyEmailAsync(string organizationName, string email) => + Task.CompletedTask; + + public Task SendOrganizationUserRevokedForPolicySingleOrgEmailAsync(string organizationName, string email) => + Task.CompletedTask; + public Task SendTwoFactorEmailAsync(string email, string token) { return Task.FromResult(0); diff --git a/src/Infrastructure.Dapper/AdminConsole/Repositories/OrganizationUserRepository.cs b/src/Infrastructure.Dapper/AdminConsole/Repositories/OrganizationUserRepository.cs index d5bdd3b6a270..42f79852f3c8 100644 --- a/src/Infrastructure.Dapper/AdminConsole/Repositories/OrganizationUserRepository.cs +++ b/src/Infrastructure.Dapper/AdminConsole/Repositories/OrganizationUserRepository.cs @@ -557,4 +557,14 @@ public async Task> GetManyByOrganizationWithClaime return results.ToList(); } } + + public async Task RevokeManyByIdAsync(IEnumerable organizationUserIds) + { + await using var connection = new SqlConnection(ConnectionString); + + await connection.ExecuteAsync( + "[dbo].[OrganizationUser_SetStatusForUsersById]", + new { OrganizationUserIds = JsonSerializer.Serialize(organizationUserIds), Status = OrganizationUserStatusType.Revoked }, + commandType: CommandType.StoredProcedure); + } } diff --git a/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationUserRepository.cs b/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationUserRepository.cs index a64c19704de2..007ff1a7ff6e 100644 --- a/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationUserRepository.cs +++ b/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationUserRepository.cs @@ -721,4 +721,16 @@ public UpdateEncryptedDataForKeyRotation UpdateForKeyRotation( return data; } } + + public async Task RevokeManyByIdAsync(IEnumerable organizationUserIds) + { + using var scope = ServiceScopeFactory.CreateScope(); + + var dbContext = GetDatabaseContext(scope); + + await dbContext.OrganizationUsers.Where(x => organizationUserIds.Contains(x.Id)) + .ExecuteUpdateAsync(s => s.SetProperty(x => x.Status, OrganizationUserStatusType.Revoked)); + + await dbContext.UserBumpAccountRevisionDateByOrganizationUserIdsAsync(organizationUserIds); + } } diff --git a/src/Sql/dbo/Stored Procedures/OrganizationUser_SetStatusForUsersById.sql b/src/Sql/dbo/Stored Procedures/OrganizationUser_SetStatusForUsersById.sql new file mode 100644 index 000000000000..95ed5a3155d0 --- /dev/null +++ b/src/Sql/dbo/Stored Procedures/OrganizationUser_SetStatusForUsersById.sql @@ -0,0 +1,29 @@ +CREATE PROCEDURE [dbo].[OrganizationUser_SetStatusForUsersById] + @OrganizationUserIds AS NVARCHAR(MAX), + @Status SMALLINT +AS +BEGIN + SET NOCOUNT ON + + -- Declare a table variable to hold the parsed JSON data + DECLARE @ParsedIds TABLE (Id UNIQUEIDENTIFIER); + + -- Parse the JSON input into the table variable + INSERT INTO @ParsedIds (Id) + SELECT value + FROM OPENJSON(@OrganizationUserIds); + + -- Check if the input table is empty + IF (SELECT COUNT(1) FROM @ParsedIds) < 1 + BEGIN + RETURN(-1); + END + + UPDATE + [dbo].[OrganizationUser] + SET [Status] = @Status + WHERE [Id] IN (SELECT Id from @ParsedIds) + + EXEC [dbo].[User_BumpAccountRevisionDateByOrganizationUserIds] @OrganizationUserIds +END + diff --git a/test/Core.Test/AdminConsole/AutoFixture/PolicyUpdateFixtures.cs b/test/Core.Test/AdminConsole/AutoFixture/PolicyUpdateFixtures.cs index dff9b571782c..794f6fddf351 100644 --- a/test/Core.Test/AdminConsole/AutoFixture/PolicyUpdateFixtures.cs +++ b/test/Core.Test/AdminConsole/AutoFixture/PolicyUpdateFixtures.cs @@ -2,6 +2,7 @@ using AutoFixture; using AutoFixture.Xunit2; using Bit.Core.AdminConsole.Enums; +using Bit.Core.AdminConsole.Models.Data; using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Models; namespace Bit.Core.Test.AdminConsole.AutoFixture; @@ -12,7 +13,8 @@ public void Customize(IFixture fixture) { fixture.Customize(composer => composer .With(o => o.Type, type) - .With(o => o.Enabled, enabled)); + .With(o => o.Enabled, enabled) + .With(o => o.PerformedBy, new StandardUser(Guid.NewGuid(), false))); } } diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationDomains/VerifyOrganizationDomainCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationDomains/VerifyOrganizationDomainCommandTests.cs index 2fcaf8134c80..8dbe5331317c 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationDomains/VerifyOrganizationDomainCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationDomains/VerifyOrganizationDomainCommandTests.cs @@ -2,6 +2,7 @@ using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationDomains; using Bit.Core.AdminConsole.Services; +using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; @@ -28,7 +29,12 @@ public async Task UserVerifyOrganizationDomainAsync_ShouldThrowConflict_WhenDoma DomainName = "Test Domain", Txt = "btw+test18383838383" }; + + sutProvider.GetDependency() + .UserId.Returns(Guid.NewGuid()); + expected.SetVerifiedDate(); + sutProvider.GetDependency() .GetByIdAsync(id) .Returns(expected); @@ -53,6 +59,10 @@ public async Task UserVerifyOrganizationDomainAsync_ShouldThrowConflict_WhenDoma sutProvider.GetDependency() .GetByIdAsync(id) .Returns(expected); + + sutProvider.GetDependency() + .UserId.Returns(Guid.NewGuid()); + sutProvider.GetDependency() .GetClaimedDomainsByDomainNameAsync(expected.DomainName) .Returns(new List { expected }); @@ -77,9 +87,14 @@ public async Task UserVerifyOrganizationDomainAsync_ShouldVerifyDomainUpdateAndL sutProvider.GetDependency() .GetByIdAsync(id) .Returns(expected); + + sutProvider.GetDependency() + .UserId.Returns(Guid.NewGuid()); + sutProvider.GetDependency() .GetClaimedDomainsByDomainNameAsync(expected.DomainName) .Returns(new List()); + sutProvider.GetDependency() .ResolveAsync(expected.DomainName, Arg.Any()) .Returns(true); @@ -107,9 +122,14 @@ public async Task UserVerifyOrganizationDomainAsync_ShouldNotSetVerifiedDate_Whe sutProvider.GetDependency() .GetByIdAsync(id) .Returns(expected); + + sutProvider.GetDependency() + .UserId.Returns(Guid.NewGuid()); + sutProvider.GetDependency() .GetClaimedDomainsByDomainNameAsync(expected.DomainName) .Returns(new List()); + sutProvider.GetDependency() .ResolveAsync(expected.DomainName, Arg.Any()) .Returns(false); @@ -143,7 +163,7 @@ await sutProvider.GetDependency().ReceivedWithAnyArgs(1) [Theory, BitAutoData] public async Task UserVerifyOrganizationDomainAsync_GivenOrganizationDomainWithAccountDeprovisioningEnabled_WhenDomainIsVerified_ThenSingleOrgPolicyShouldBeEnabled( - OrganizationDomain domain, SutProvider sutProvider) + OrganizationDomain domain, Guid userId, SutProvider sutProvider) { sutProvider.GetDependency() .GetClaimedDomainsByDomainNameAsync(domain.DomainName) @@ -157,11 +177,14 @@ public async Task UserVerifyOrganizationDomainAsync_GivenOrganizationDomainWithA .IsEnabled(FeatureFlagKeys.AccountDeprovisioning) .Returns(true); + sutProvider.GetDependency() + .UserId.Returns(userId); + _ = await sutProvider.Sut.UserVerifyOrganizationDomainAsync(domain); await sutProvider.GetDependency() .Received(1) - .SaveAsync(Arg.Is(x => x.Type == PolicyType.SingleOrg && x.OrganizationId == domain.OrganizationId && x.Enabled), null); + .SaveAsync(Arg.Is(x => x.Type == PolicyType.SingleOrg && x.OrganizationId == domain.OrganizationId && x.Enabled), userId); } [Theory, BitAutoData] @@ -176,6 +199,9 @@ public async Task UserVerifyOrganizationDomainAsync_GivenOrganizationDomainWithA .ResolveAsync(domain.DomainName, domain.Txt) .Returns(true); + sutProvider.GetDependency() + .UserId.Returns(Guid.NewGuid()); + sutProvider.GetDependency() .IsEnabled(FeatureFlagKeys.AccountDeprovisioning) .Returns(false); @@ -189,7 +215,7 @@ await sutProvider.GetDependency() [Theory, BitAutoData] public async Task UserVerifyOrganizationDomainAsync_GivenOrganizationDomainWithAccountDeprovisioningEnabled_WhenDomainIsNotVerified_ThenSingleOrgPolicyShouldNotBeEnabled( - OrganizationDomain domain, SutProvider sutProvider) + OrganizationDomain domain, SutProvider sutProvider) { sutProvider.GetDependency() .GetClaimedDomainsByDomainNameAsync(domain.DomainName) @@ -199,6 +225,9 @@ public async Task UserVerifyOrganizationDomainAsync_GivenOrganizationDomainWithA .ResolveAsync(domain.DomainName, domain.Txt) .Returns(false); + sutProvider.GetDependency() + .UserId.Returns(Guid.NewGuid()); + sutProvider.GetDependency() .IsEnabled(FeatureFlagKeys.AccountDeprovisioning) .Returns(true); @@ -223,6 +252,9 @@ public async Task UserVerifyOrganizationDomainAsync_GivenOrganizationDomainWithA .ResolveAsync(domain.DomainName, domain.Txt) .Returns(false); + sutProvider.GetDependency() + .UserId.Returns(Guid.NewGuid()); + sutProvider.GetDependency() .IsEnabled(FeatureFlagKeys.AccountDeprovisioning) .Returns(true); diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RevokeNonCompliantOrganizationUserCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RevokeNonCompliantOrganizationUserCommandTests.cs new file mode 100644 index 000000000000..3653cd27d7df --- /dev/null +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/RevokeNonCompliantOrganizationUserCommandTests.cs @@ -0,0 +1,185 @@ +using Bit.Core.AdminConsole.Models.Data; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Requests; +using Bit.Core.Enums; +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; + +namespace Bit.Core.Test.AdminConsole.OrganizationFeatures.OrganizationUsers; + +[SutProviderCustomize] +public class RevokeNonCompliantOrganizationUserCommandTests +{ + [Theory, BitAutoData] + public async Task RevokeNonCompliantOrganizationUsersAsync_GivenUnrecognizedUserType_WhenAttemptingToRevoke_ThenErrorShouldBeReturned( + Guid organizationId, SutProvider sutProvider) + { + var command = new RevokeOrganizationUsersRequest(organizationId, [], new InvalidUser()); + + var result = await sutProvider.Sut.RevokeNonCompliantOrganizationUsersAsync(command); + + Assert.True(result.HasErrors); + Assert.Contains(RevokeNonCompliantOrganizationUserCommand.ErrorRequestedByWasNotValid, result.ErrorMessages); + } + + [Theory, BitAutoData] + public async Task RevokeNonCompliantOrganizationUsersAsync_GivenPopulatedRequest_WhenUserAttemptsToRevokeThemselves_ThenErrorShouldBeReturned( + Guid organizationId, OrganizationUserUserDetails revokingUser, + SutProvider sutProvider) + { + var command = new RevokeOrganizationUsersRequest(organizationId, revokingUser, + new StandardUser(revokingUser?.UserId ?? Guid.NewGuid(), true)); + + var result = await sutProvider.Sut.RevokeNonCompliantOrganizationUsersAsync(command); + + Assert.True(result.HasErrors); + Assert.Contains(RevokeNonCompliantOrganizationUserCommand.ErrorCannotRevokeSelf, result.ErrorMessages); + } + + [Theory, BitAutoData] + public async Task RevokeNonCompliantOrganizationUsersAsync_GivenPopulatedRequest_WhenUserAttemptsToRevokeOrgUsersFromAnotherOrg_ThenErrorShouldBeReturned( + Guid organizationId, OrganizationUserUserDetails userFromAnotherOrg, + SutProvider sutProvider) + { + userFromAnotherOrg.OrganizationId = Guid.NewGuid(); + + var command = new RevokeOrganizationUsersRequest(organizationId, userFromAnotherOrg, + new StandardUser(Guid.NewGuid(), true)); + + var result = await sutProvider.Sut.RevokeNonCompliantOrganizationUsersAsync(command); + + Assert.True(result.HasErrors); + Assert.Contains(RevokeNonCompliantOrganizationUserCommand.ErrorInvalidUsers, result.ErrorMessages); + } + + [Theory, BitAutoData] + public async Task RevokeNonCompliantOrganizationUsersAsync_GivenPopulatedRequest_WhenUserAttemptsToRevokeAllOwnersFromOrg_ThenErrorShouldBeReturned( + Guid organizationId, OrganizationUserUserDetails userToRevoke, + SutProvider sutProvider) + { + userToRevoke.OrganizationId = organizationId; + + var command = new RevokeOrganizationUsersRequest(organizationId, userToRevoke, + new StandardUser(Guid.NewGuid(), true)); + + sutProvider.GetDependency() + .HasConfirmedOwnersExceptAsync(organizationId, Arg.Any>()) + .Returns(false); + + var result = await sutProvider.Sut.RevokeNonCompliantOrganizationUsersAsync(command); + + Assert.True(result.HasErrors); + Assert.Contains(RevokeNonCompliantOrganizationUserCommand.ErrorOrgMustHaveAtLeastOneOwner, result.ErrorMessages); + } + + [Theory, BitAutoData] + public async Task RevokeNonCompliantOrganizationUsersAsync_GivenPopulatedRequest_WhenUserAttemptsToRevokeOwnerWhenNotAnOwner_ThenErrorShouldBeReturned( + Guid organizationId, OrganizationUserUserDetails userToRevoke, + SutProvider sutProvider) + { + userToRevoke.OrganizationId = organizationId; + userToRevoke.Type = OrganizationUserType.Owner; + + var command = new RevokeOrganizationUsersRequest(organizationId, userToRevoke, + new StandardUser(Guid.NewGuid(), false)); + + sutProvider.GetDependency() + .HasConfirmedOwnersExceptAsync(organizationId, Arg.Any>()) + .Returns(true); + + var result = await sutProvider.Sut.RevokeNonCompliantOrganizationUsersAsync(command); + + Assert.True(result.HasErrors); + Assert.Contains(RevokeNonCompliantOrganizationUserCommand.ErrorOnlyOwnersCanRevokeOtherOwners, result.ErrorMessages); + } + + [Theory, BitAutoData] + public async Task RevokeNonCompliantOrganizationUsersAsync_GivenPopulatedRequest_WhenUserAttemptsToRevokeUserWhoIsAlreadyRevoked_ThenErrorShouldBeReturned( + Guid organizationId, OrganizationUserUserDetails userToRevoke, + SutProvider sutProvider) + { + userToRevoke.OrganizationId = organizationId; + userToRevoke.Status = OrganizationUserStatusType.Revoked; + + var command = new RevokeOrganizationUsersRequest(organizationId, userToRevoke, + new StandardUser(Guid.NewGuid(), true)); + + sutProvider.GetDependency() + .HasConfirmedOwnersExceptAsync(organizationId, Arg.Any>()) + .Returns(true); + + var result = await sutProvider.Sut.RevokeNonCompliantOrganizationUsersAsync(command); + + Assert.True(result.HasErrors); + Assert.Contains($"{RevokeNonCompliantOrganizationUserCommand.ErrorUserAlreadyRevoked} Id: {userToRevoke.Id}", result.ErrorMessages); + } + + [Theory, BitAutoData] + public async Task RevokeNonCompliantOrganizationUsersAsync_GivenPopulatedRequest_WhenUserHasMultipleInvalidUsers_ThenErrorShouldBeReturned( + Guid organizationId, IEnumerable usersToRevoke, + SutProvider sutProvider) + { + var revocableUsers = usersToRevoke.ToList(); + revocableUsers.ForEach(user => user.OrganizationId = organizationId); + revocableUsers[0].Type = OrganizationUserType.Owner; + revocableUsers[1].Status = OrganizationUserStatusType.Revoked; + + var command = new RevokeOrganizationUsersRequest(organizationId, revocableUsers, + new StandardUser(Guid.NewGuid(), false)); + + sutProvider.GetDependency() + .HasConfirmedOwnersExceptAsync(organizationId, Arg.Any>()) + .Returns(true); + + var result = await sutProvider.Sut.RevokeNonCompliantOrganizationUsersAsync(command); + + Assert.True(result.HasErrors); + Assert.True(result.ErrorMessages.Count > 1); + } + + [Theory, BitAutoData] + public async Task RevokeNonCompliantOrganizationUsersAsync_GivenValidPopulatedRequest_WhenUserAttemptsToRevokeAUser_ThenUserShouldBeRevoked( + Guid organizationId, OrganizationUserUserDetails userToRevoke, + SutProvider sutProvider) + { + userToRevoke.OrganizationId = organizationId; + userToRevoke.Type = OrganizationUserType.Admin; + + var command = new RevokeOrganizationUsersRequest(organizationId, userToRevoke, + new StandardUser(Guid.NewGuid(), false)); + + sutProvider.GetDependency() + .HasConfirmedOwnersExceptAsync(organizationId, Arg.Any>()) + .Returns(true); + + var result = await sutProvider.Sut.RevokeNonCompliantOrganizationUsersAsync(command); + + await sutProvider.GetDependency() + .Received(1) + .RevokeManyByIdAsync(Arg.Any>()); + + Assert.True(result.Success); + + await sutProvider.GetDependency() + .Received(1) + .LogOrganizationUserEventsAsync( + Arg.Is>( + x => x.Any(y => + y.organizationUser.Id == userToRevoke.Id && y.eventType == EventType.OrganizationUser_Revoked) + )); + } + + public class InvalidUser : IActingUser + { + public Guid? UserId => Guid.Empty; + public bool IsOrganizationOwnerOrProvider => false; + public EventSystemUser? SystemUserType => null; + } +} diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyValidators/SingleOrgPolicyValidatorTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyValidators/SingleOrgPolicyValidatorTests.cs index 76ee5748403f..0731920757a6 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyValidators/SingleOrgPolicyValidatorTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyValidators/SingleOrgPolicyValidatorTests.cs @@ -1,6 +1,7 @@ using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Requests; using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Models; using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyValidators; using Bit.Core.Auth.Entities; @@ -10,6 +11,7 @@ using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Enums; +using Bit.Core.Models.Commands; using Bit.Core.Models.Data.Organizations.OrganizationUsers; using Bit.Core.Repositories; using Bit.Core.Services; @@ -61,6 +63,79 @@ public async Task ValidateAsync_DisablingPolicy_KeyConnectorNotEnabled_Success( Assert.True(string.IsNullOrEmpty(result)); } + [Theory, BitAutoData] + public async Task OnSaveSideEffectsAsync_RevokesNonCompliantUsers( + [PolicyUpdate(PolicyType.SingleOrg)] PolicyUpdate policyUpdate, + [Policy(PolicyType.SingleOrg, false)] Policy policy, + Guid savingUserId, + Guid nonCompliantUserId, + Organization organization, SutProvider sutProvider) + { + policy.OrganizationId = organization.Id = policyUpdate.OrganizationId; + + var compliantUser1 = new OrganizationUserUserDetails + { + OrganizationId = organization.Id, + Type = OrganizationUserType.User, + Status = OrganizationUserStatusType.Confirmed, + UserId = new Guid(), + Email = "user1@example.com" + }; + + var compliantUser2 = new OrganizationUserUserDetails + { + OrganizationId = organization.Id, + Type = OrganizationUserType.User, + Status = OrganizationUserStatusType.Confirmed, + UserId = new Guid(), + Email = "user2@example.com" + }; + + var nonCompliantUser = new OrganizationUserUserDetails + { + OrganizationId = organization.Id, + Type = OrganizationUserType.User, + Status = OrganizationUserStatusType.Confirmed, + UserId = nonCompliantUserId, + Email = "user3@example.com" + }; + + sutProvider.GetDependency() + .GetManyDetailsByOrganizationAsync(policyUpdate.OrganizationId) + .Returns([compliantUser1, compliantUser2, nonCompliantUser]); + + var otherOrganizationUser = new OrganizationUser + { + OrganizationId = new Guid(), + UserId = nonCompliantUserId, + Status = OrganizationUserStatusType.Confirmed + }; + + sutProvider.GetDependency() + .GetManyByManyUsersAsync(Arg.Is>(ids => ids.Contains(nonCompliantUserId))) + .Returns([otherOrganizationUser]); + + sutProvider.GetDependency().UserId.Returns(savingUserId); + sutProvider.GetDependency().GetByIdAsync(policyUpdate.OrganizationId).Returns(organization); + + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.AccountDeprovisioning) + .Returns(true); + + sutProvider.GetDependency() + .RevokeNonCompliantOrganizationUsersAsync(Arg.Any()) + .Returns(new CommandResult()); + + await sutProvider.Sut.OnSaveSideEffectsAsync(policyUpdate, policy); + + await sutProvider.GetDependency() + .Received(1) + .RevokeNonCompliantOrganizationUsersAsync(Arg.Any()); + await sutProvider.GetDependency() + .Received(1) + .SendOrganizationUserRevokedForPolicySingleOrgEmailAsync(organization.DisplayName(), + "user3@example.com"); + } + [Theory, BitAutoData] public async Task OnSaveSideEffectsAsync_RemovesNonCompliantUsers( [PolicyUpdate(PolicyType.SingleOrg)] PolicyUpdate policyUpdate, @@ -116,6 +191,13 @@ public async Task OnSaveSideEffectsAsync_RemovesNonCompliantUsers( sutProvider.GetDependency().UserId.Returns(savingUserId); sutProvider.GetDependency().GetByIdAsync(policyUpdate.OrganizationId).Returns(organization); + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.AccountDeprovisioning) + .Returns(false); + + sutProvider.GetDependency() + .RevokeNonCompliantOrganizationUsersAsync(Arg.Any()) + .Returns(new CommandResult()); + await sutProvider.Sut.OnSaveSideEffectsAsync(policyUpdate, policy); await sutProvider.GetDependency() @@ -126,4 +208,73 @@ await sutProvider.GetDependency() .SendOrganizationUserRemovedForPolicySingleOrgEmailAsync(organization.DisplayName(), "user3@example.com"); } + + [Theory, BitAutoData] + public async Task OnSaveSideEffectsAsync_WhenAccountDeprovisioningIsEnabled_ThenUsersAreRevoked( + [PolicyUpdate(PolicyType.SingleOrg)] PolicyUpdate policyUpdate, + [Policy(PolicyType.SingleOrg, false)] Policy policy, + Guid savingUserId, + Guid nonCompliantUserId, + Organization organization, SutProvider sutProvider) + { + policy.OrganizationId = organization.Id = policyUpdate.OrganizationId; + + var compliantUser1 = new OrganizationUserUserDetails + { + OrganizationId = organization.Id, + Type = OrganizationUserType.User, + Status = OrganizationUserStatusType.Confirmed, + UserId = new Guid(), + Email = "user1@example.com" + }; + + var compliantUser2 = new OrganizationUserUserDetails + { + OrganizationId = organization.Id, + Type = OrganizationUserType.User, + Status = OrganizationUserStatusType.Confirmed, + UserId = new Guid(), + Email = "user2@example.com" + }; + + var nonCompliantUser = new OrganizationUserUserDetails + { + OrganizationId = organization.Id, + Type = OrganizationUserType.User, + Status = OrganizationUserStatusType.Confirmed, + UserId = nonCompliantUserId, + Email = "user3@example.com" + }; + + sutProvider.GetDependency() + .GetManyDetailsByOrganizationAsync(policyUpdate.OrganizationId) + .Returns([compliantUser1, compliantUser2, nonCompliantUser]); + + var otherOrganizationUser = new OrganizationUser + { + OrganizationId = new Guid(), + UserId = nonCompliantUserId, + Status = OrganizationUserStatusType.Confirmed + }; + + sutProvider.GetDependency() + .GetManyByManyUsersAsync(Arg.Is>(ids => ids.Contains(nonCompliantUserId))) + .Returns([otherOrganizationUser]); + + sutProvider.GetDependency().UserId.Returns(savingUserId); + sutProvider.GetDependency().GetByIdAsync(policyUpdate.OrganizationId) + .Returns(organization); + + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.AccountDeprovisioning).Returns(true); + + sutProvider.GetDependency() + .RevokeNonCompliantOrganizationUsersAsync(Arg.Any()) + .Returns(new CommandResult()); + + await sutProvider.Sut.OnSaveSideEffectsAsync(policyUpdate, policy); + + await sutProvider.GetDependency() + .Received() + .RevokeNonCompliantOrganizationUsersAsync(Arg.Any()); + } } diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyValidators/TwoFactorAuthenticationPolicyValidatorTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyValidators/TwoFactorAuthenticationPolicyValidatorTests.cs index 4dce13174934..4e5f1816a5a4 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyValidators/TwoFactorAuthenticationPolicyValidatorTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyValidators/TwoFactorAuthenticationPolicyValidatorTests.cs @@ -1,12 +1,14 @@ using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Requests; using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Models; using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyValidators; using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; using Bit.Core.Context; using Bit.Core.Enums; using Bit.Core.Exceptions; +using Bit.Core.Models.Commands; using Bit.Core.Models.Data.Organizations.OrganizationUsers; using Bit.Core.Repositories; using Bit.Core.Services; @@ -176,6 +178,10 @@ public async Task OnSaveSideEffectsAsync_UsersToBeRemovedDontHaveMasterPasswords HasMasterPassword = false }; + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.AccountDeprovisioning) + .Returns(false); + sutProvider.GetDependency() .GetManyDetailsByOrganizationAsync(policy.OrganizationId) .Returns(new List @@ -201,9 +207,151 @@ public async Task OnSaveSideEffectsAsync_UsersToBeRemovedDontHaveMasterPasswords var badRequestException = await Assert.ThrowsAsync( () => sutProvider.Sut.OnSaveSideEffectsAsync(policyUpdate, policy)); - 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); + Assert.Equal(TwoFactorAuthenticationPolicyValidator.NonCompliantMembersWillLoseAccessMessage, badRequestException.Message); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() .RemoveUserAsync(organizationId: default, organizationUserId: default, deletingUserId: default); } + + [Theory, BitAutoData] + public async Task OnSaveSideEffectsAsync_GivenUpdateTo2faPolicy_WhenAccountProvisioningIsDisabled_ThenRevokeUserCommandShouldNotBeCalled( + Organization organization, + [PolicyUpdate(PolicyType.TwoFactorAuthentication)] + PolicyUpdate policyUpdate, + [Policy(PolicyType.TwoFactorAuthentication, false)] + Policy policy, + SutProvider sutProvider) + { + policy.OrganizationId = organization.Id = policyUpdate.OrganizationId; + sutProvider.GetDependency().GetByIdAsync(organization.Id).Returns(organization); + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.AccountDeprovisioning) + .Returns(false); + + var orgUserDetailUserAcceptedWithout2Fa = new OrganizationUserUserDetails + { + Id = Guid.NewGuid(), + Status = OrganizationUserStatusType.Accepted, + Type = OrganizationUserType.User, + Email = "user3@test.com", + Name = "TEST", + UserId = Guid.NewGuid(), + HasMasterPassword = true + }; + + sutProvider.GetDependency() + .GetManyDetailsByOrganizationAsync(policyUpdate.OrganizationId) + .Returns(new List + { + orgUserDetailUserAcceptedWithout2Fa + }); + + + sutProvider.GetDependency() + .TwoFactorIsEnabledAsync(Arg.Any>()) + .Returns(new List<(OrganizationUserUserDetails user, bool hasTwoFactor)>() + { + (orgUserDetailUserAcceptedWithout2Fa, false), + }); + + await sutProvider.Sut.OnSaveSideEffectsAsync(policyUpdate, policy); + + await sutProvider.GetDependency() + .DidNotReceive() + .RevokeNonCompliantOrganizationUsersAsync(Arg.Any()); + } + + [Theory, BitAutoData] + public async Task OnSaveSideEffectsAsync_GivenUpdateTo2faPolicy_WhenAccountProvisioningIsEnabledAndUserDoesNotHaveMasterPassword_ThenNonCompliantMembersErrorMessageWillReturn( + Organization organization, + [PolicyUpdate(PolicyType.TwoFactorAuthentication)] PolicyUpdate policyUpdate, + [Policy(PolicyType.TwoFactorAuthentication, false)] Policy policy, + SutProvider sutProvider) + { + policy.OrganizationId = organization.Id = policyUpdate.OrganizationId; + sutProvider.GetDependency().GetByIdAsync(organization.Id).Returns(organization); + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.AccountDeprovisioning) + .Returns(true); + + var orgUserDetailUserWithout2Fa = new OrganizationUserUserDetails + { + Id = Guid.NewGuid(), + Status = OrganizationUserStatusType.Confirmed, + Type = OrganizationUserType.User, + Email = "user3@test.com", + Name = "TEST", + UserId = Guid.NewGuid(), + HasMasterPassword = false + }; + + sutProvider.GetDependency() + .GetManyDetailsByOrganizationAsync(policyUpdate.OrganizationId) + .Returns([orgUserDetailUserWithout2Fa]); + + sutProvider.GetDependency() + .TwoFactorIsEnabledAsync(Arg.Any>()) + .Returns(new List<(OrganizationUserUserDetails user, bool hasTwoFactor)>() + { + (orgUserDetailUserWithout2Fa, false), + }); + + var exception = await Assert.ThrowsAsync(() => sutProvider.Sut.OnSaveSideEffectsAsync(policyUpdate, policy)); + + Assert.Equal(TwoFactorAuthenticationPolicyValidator.NonCompliantMembersWillLoseAccessMessage, exception.Message); + } + + [Theory, BitAutoData] + public async Task OnSaveSideEffectsAsync_WhenAccountProvisioningIsEnabledAndUserHasMasterPassword_ThenUserWillBeRevoked( + Organization organization, + [PolicyUpdate(PolicyType.TwoFactorAuthentication)] PolicyUpdate policyUpdate, + [Policy(PolicyType.TwoFactorAuthentication, false)] Policy policy, + SutProvider sutProvider) + { + policy.OrganizationId = organization.Id = policyUpdate.OrganizationId; + sutProvider.GetDependency().GetByIdAsync(organization.Id).Returns(organization); + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.AccountDeprovisioning) + .Returns(true); + + var orgUserDetailUserWithout2Fa = new OrganizationUserUserDetails + { + Id = Guid.NewGuid(), + Status = OrganizationUserStatusType.Confirmed, + Type = OrganizationUserType.User, + Email = "user3@test.com", + Name = "TEST", + UserId = Guid.NewGuid(), + HasMasterPassword = true + }; + + sutProvider.GetDependency() + .GetManyDetailsByOrganizationAsync(policyUpdate.OrganizationId) + .Returns([orgUserDetailUserWithout2Fa]); + + sutProvider.GetDependency() + .TwoFactorIsEnabledAsync(Arg.Any>()) + .Returns(new List<(OrganizationUserUserDetails user, bool hasTwoFactor)>() + { + (orgUserDetailUserWithout2Fa, true), + }); + + sutProvider.GetDependency() + .RevokeNonCompliantOrganizationUsersAsync(Arg.Any()) + .Returns(new CommandResult()); + + await sutProvider.Sut.OnSaveSideEffectsAsync(policyUpdate, policy); + + await sutProvider.GetDependency() + .Received(1) + .RevokeNonCompliantOrganizationUsersAsync(Arg.Any()); + + await sutProvider.GetDependency() + .Received(1) + .SendOrganizationUserRevokedForPolicySingleOrgEmailAsync(organization.DisplayName(), + "user3@test.com"); + } } diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/SavePolicyCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/SavePolicyCommandTests.cs index 342ede9c829d..3ca7004e70ba 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/SavePolicyCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/SavePolicyCommandTests.cs @@ -100,7 +100,7 @@ public void Constructor_DuplicatePolicyValidators_Throws() } [Theory, BitAutoData] - public async Task SaveAsync_OrganizationDoesNotExist_ThrowsBadRequest(PolicyUpdate policyUpdate) + public async Task SaveAsync_OrganizationDoesNotExist_ThrowsBadRequest([PolicyUpdate(PolicyType.ActivateAutofill)] PolicyUpdate policyUpdate) { var sutProvider = SutProviderFactory(); sutProvider.GetDependency() @@ -115,7 +115,7 @@ public async Task SaveAsync_OrganizationDoesNotExist_ThrowsBadRequest(PolicyUpda } [Theory, BitAutoData] - public async Task SaveAsync_OrganizationCannotUsePolicies_ThrowsBadRequest(PolicyUpdate policyUpdate) + public async Task SaveAsync_OrganizationCannotUsePolicies_ThrowsBadRequest([PolicyUpdate(PolicyType.ActivateAutofill)] PolicyUpdate policyUpdate) { var sutProvider = SutProviderFactory(); sutProvider.GetDependency() diff --git a/test/Core.Test/AdminConsole/Services/EventServiceTests.cs b/test/Core.Test/AdminConsole/Services/EventServiceTests.cs index 18f5371b4943..d064fce2ecd8 100644 --- a/test/Core.Test/AdminConsole/Services/EventServiceTests.cs +++ b/test/Core.Test/AdminConsole/Services/EventServiceTests.cs @@ -169,7 +169,6 @@ public async Task LogOrganizationUserEvent_WithEventSystemUser_LogsRequiredInfo( new EventMessage() { IpAddress = ipAddress, - DeviceType = DeviceType.Server, OrganizationId = orgUser.OrganizationId, UserId = orgUser.UserId, OrganizationUserId = orgUser.Id, diff --git a/util/Migrator/DbScripts/2024-11-26-00_OrgUserSetStatusBulk.sql b/util/Migrator/DbScripts/2024-11-26-00_OrgUserSetStatusBulk.sql new file mode 100644 index 000000000000..95151f90ded5 --- /dev/null +++ b/util/Migrator/DbScripts/2024-11-26-00_OrgUserSetStatusBulk.sql @@ -0,0 +1,28 @@ +CREATE PROCEDURE [dbo].[OrganizationUser_SetStatusForUsersById] + @OrganizationUserIds AS NVARCHAR(MAX), + @Status SMALLINT +AS +BEGIN + SET NOCOUNT ON + + -- Declare a table variable to hold the parsed JSON data + DECLARE @ParsedIds TABLE (Id UNIQUEIDENTIFIER); + + -- Parse the JSON input into the table variable + INSERT INTO @ParsedIds (Id) + SELECT value + FROM OPENJSON(@OrganizationUserIds); + + -- Check if the input table is empty + IF (SELECT COUNT(1) FROM @ParsedIds) < 1 + BEGIN + RETURN(-1); + END + + UPDATE + [dbo].[OrganizationUser] + SET [Status] = @Status + WHERE [Id] IN (SELECT Id from @ParsedIds) + + EXEC [dbo].[User_BumpAccountRevisionDateByOrganizationUserIds] @OrganizationUserIds +END