From bf9cc0145944338e15484893679bad43a1780bf8 Mon Sep 17 00:00:00 2001 From: Brandon Treston Date: Fri, 13 Feb 2026 11:50:12 -0500 Subject: [PATCH] [PM-26379] Implement auto confirm push notification (#6980) * implement auto confirm push notification * fix test * fix test * simplify LINQ --- .../OrganizationUsers/AcceptOrgUserCommand.cs | 11 +- .../IPushAutoConfirmNotificationCommand.cs | 6 + .../PushAutoConfirmNotificationCommand.cs | 65 ++++ src/Core/Models/PushNotification.cs | 18 ++ ...OrganizationServiceCollectionExtensions.cs | 1 + src/Core/Platform/Push/PushType.cs | 3 + src/Notifications/HubHelpers.cs | 12 + .../AcceptOrgUserCommandTests.cs | 45 +++ ...PushAutoConfirmNotificationCommandTests.cs | 286 ++++++++++++++++++ 9 files changed, 446 insertions(+), 1 deletion(-) create mode 100644 src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Interfaces/IPushAutoConfirmNotificationCommand.cs create mode 100644 src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/PushAutoConfirmNotificationCommand.cs create mode 100644 test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/PushAutoConfirmNotificationCommandTests.cs diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs index 50f194b578..ac532be9bf 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs @@ -2,6 +2,7 @@ #nullable disable using Bit.Core.AdminConsole.Enums; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.AdminConsole.OrganizationFeatures.Policies; using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Enforcement.AutoConfirm; using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; @@ -36,6 +37,7 @@ public class AcceptOrgUserCommand : IAcceptOrgUserCommand private readonly IFeatureService _featureService; private readonly IPolicyRequirementQuery _policyRequirementQuery; private readonly IAutomaticUserConfirmationPolicyEnforcementValidator _automaticUserConfirmationPolicyEnforcementValidator; + private readonly IPushAutoConfirmNotificationCommand _pushAutoConfirmNotificationCommand; public AcceptOrgUserCommand( IDataProtectionProvider dataProtectionProvider, @@ -49,7 +51,8 @@ public class AcceptOrgUserCommand : IAcceptOrgUserCommand IDataProtectorTokenFactory orgUserInviteTokenDataFactory, IFeatureService featureService, IPolicyRequirementQuery policyRequirementQuery, - IAutomaticUserConfirmationPolicyEnforcementValidator automaticUserConfirmationPolicyEnforcementValidator) + IAutomaticUserConfirmationPolicyEnforcementValidator automaticUserConfirmationPolicyEnforcementValidator, + IPushAutoConfirmNotificationCommand pushAutoConfirmNotificationCommand) { // TODO: remove data protector when old token validation removed _dataProtector = dataProtectionProvider.CreateProtector(OrgUserInviteTokenable.DataProtectorPurpose); @@ -64,6 +67,7 @@ public class AcceptOrgUserCommand : IAcceptOrgUserCommand _featureService = featureService; _policyRequirementQuery = policyRequirementQuery; _automaticUserConfirmationPolicyEnforcementValidator = automaticUserConfirmationPolicyEnforcementValidator; + _pushAutoConfirmNotificationCommand = pushAutoConfirmNotificationCommand; } public async Task AcceptOrgUserByEmailTokenAsync(Guid organizationUserId, User user, string emailToken, @@ -233,6 +237,11 @@ public class AcceptOrgUserCommand : IAcceptOrgUserCommand await _mailService.SendOrganizationAcceptedEmailAsync(organization, user.Email, adminEmails); } + if (_featureService.IsEnabled(FeatureFlagKeys.AutomaticConfirmUsers)) + { + await _pushAutoConfirmNotificationCommand.PushAsync(user.Id, orgUser.OrganizationId); + } + return orgUser; } diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Interfaces/IPushAutoConfirmNotificationCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Interfaces/IPushAutoConfirmNotificationCommand.cs new file mode 100644 index 0000000000..be70f4544f --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Interfaces/IPushAutoConfirmNotificationCommand.cs @@ -0,0 +1,6 @@ +namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; + +public interface IPushAutoConfirmNotificationCommand +{ + Task PushAsync(Guid userId, Guid organizationId); +} diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/PushAutoConfirmNotificationCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/PushAutoConfirmNotificationCommand.cs new file mode 100644 index 0000000000..88afc57e7b --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/PushAutoConfirmNotificationCommand.cs @@ -0,0 +1,65 @@ +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; +using Bit.Core.Enums; +using Bit.Core.Models; +using Bit.Core.Platform.Push; +using Bit.Core.Repositories; + +namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers; + +public class PushAutoConfirmNotificationCommand : IPushAutoConfirmNotificationCommand +{ + private readonly IOrganizationUserRepository _organizationUserRepository; + private readonly IPushNotificationService _pushNotificationService; + + public PushAutoConfirmNotificationCommand( + IOrganizationUserRepository organizationUserRepository, + IPushNotificationService pushNotificationService) + { + _organizationUserRepository = organizationUserRepository; + _pushNotificationService = pushNotificationService; + } + + public async Task PushAsync(Guid userId, Guid organizationId) + { + var organizationUser = await _organizationUserRepository.GetByOrganizationAsync(organizationId, userId); + if (organizationUser == null) + { + throw new Exception("Organization user not found"); + } + + var admins = await _organizationUserRepository.GetManyByMinimumRoleAsync( + organizationId, + OrganizationUserType.Admin); + + var customUsersWithManagePermission = (await _organizationUserRepository.GetManyDetailsByRoleAsync( + organizationId, + OrganizationUserType.Custom)) + .Where(c => c.GetPermissions()?.ManageUsers == true) + .Select(c => c.UserId); + + var userIds = admins + .Select(a => a.UserId) + .Concat(customUsersWithManagePermission) + .Where(id => id.HasValue) + .Select(id => id!.Value) + .Distinct(); + + foreach (var adminUserId in userIds) + { + await _pushNotificationService.PushAsync( + new PushNotification + { + Target = NotificationTarget.User, + TargetId = adminUserId, + Type = PushType.AutoConfirm, + Payload = new AutoConfirmPushNotification + { + UserId = adminUserId, + OrganizationId = organizationId, + TargetUserId = organizationUser.Id + }, + ExcludeCurrentContext = false, + }); + } + } +} diff --git a/src/Core/Models/PushNotification.cs b/src/Core/Models/PushNotification.cs index ec39c495aa..18245faf48 100644 --- a/src/Core/Models/PushNotification.cs +++ b/src/Core/Models/PushNotification.cs @@ -110,3 +110,21 @@ public class SyncPolicyPushNotification public Guid OrganizationId { get; set; } public required Policy Policy { get; set; } } + +public class AutoConfirmPushNotification +{ + /// + /// The admin/owner receiving this notification + /// + public Guid UserId { get; set; } + + /// + /// The organization the user accepted an invite to + /// + public Guid OrganizationId { get; set; } + + /// + /// The user who accepted the organization invite (will be auto-confirmed) + /// + public Guid TargetUserId { get; set; } +} diff --git a/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs b/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs index c1ebc65d44..6fb1145e7c 100644 --- a/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs +++ b/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs @@ -197,6 +197,7 @@ public static class OrganizationServiceCollectionExtensions { services.AddScoped(); services.AddScoped(); + services.AddScoped(); services.AddScoped(); services.AddScoped(); diff --git a/src/Core/Platform/Push/PushType.cs b/src/Core/Platform/Push/PushType.cs index 9a601ab0d3..b086195304 100644 --- a/src/Core/Platform/Push/PushType.cs +++ b/src/Core/Platform/Push/PushType.cs @@ -99,4 +99,7 @@ public enum PushType : byte [NotificationInfo("@bitwarden/team-admin-console-dev", typeof(Models.SyncPolicyPushNotification))] PolicyChanged = 25, + + [NotificationInfo("@bitwarden/team-admin-console-dev", typeof(Models.AutoConfirmPushNotification))] + AutoConfirm = 26, } diff --git a/src/Notifications/HubHelpers.cs b/src/Notifications/HubHelpers.cs index bc03bb46df..f9956a5de4 100644 --- a/src/Notifications/HubHelpers.cs +++ b/src/Notifications/HubHelpers.cs @@ -234,6 +234,18 @@ public class HubHelpers case PushType.PolicyChanged: await policyChangedNotificationHandler(notificationJson, cancellationToken); break; + case PushType.AutoConfirm: + var autoConfirmNotification = + JsonSerializer.Deserialize>( + notificationJson, _deserializerOptions); + if (autoConfirmNotification is null) + { + break; + } + + await _hubContext.Clients.User(autoConfirmNotification.Payload.UserId.ToString()) + .SendAsync(_receiveMessageMethod, autoConfirmNotification, cancellationToken); + break; default: _logger.LogWarning("Notification type '{NotificationType}' has not been registered in HubHelpers and will not be pushed as as result", notification.Type); break; diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs index 82d4eceaed..2acbd67465 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs @@ -2,6 +2,7 @@ using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.AutoConfirmUser; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.AdminConsole.OrganizationFeatures.Policies; using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Enforcement.AutoConfirm; using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; @@ -767,6 +768,50 @@ public class AcceptOrgUserCommandTests } + + [Theory] + [BitAutoData] + public async Task AcceptOrgUser_WithAutoConfirmFeatureFlagEnabled_SendsPushNotification( + SutProvider sutProvider, + User user, Organization org, OrganizationUser orgUser, OrganizationUserUserDetails adminUserDetails) + { + SetupCommonAcceptOrgUserMocks(sutProvider, user, org, orgUser, adminUserDetails); + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.AutomaticConfirmUsers) + .Returns(true); + + sutProvider.GetDependency() + .IsCompliantAsync(Arg.Any()) + .Returns(Valid(new AutomaticUserConfirmationPolicyEnforcementRequest(org.Id, [orgUser], user))); + + await sutProvider.Sut.AcceptOrgUserAsync(orgUser, user, _userService); + + await sutProvider.GetDependency() + .Received(1) + .PushAsync(user.Id, orgUser.OrganizationId); + } + + [Theory] + [BitAutoData] + public async Task AcceptOrgUser_WithAutoConfirmFeatureFlagDisabled_DoesNotSendPushNotification( + SutProvider sutProvider, + User user, Organization org, OrganizationUser orgUser, OrganizationUserUserDetails adminUserDetails) + { + SetupCommonAcceptOrgUserMocks(sutProvider, user, org, orgUser, adminUserDetails); + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.AutomaticConfirmUsers) + .Returns(false); + + await sutProvider.Sut.AcceptOrgUserAsync(orgUser, user, _userService); + + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .PushAsync(Arg.Any(), Arg.Any()); + } + + private void SetupCommonAcceptOrgUserByTokenMocks(SutProvider sutProvider, User user, OrganizationUser orgUser) { sutProvider.GetDependency().OrganizationInviteExpirationHours.Returns(24); diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/PushAutoConfirmNotificationCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/PushAutoConfirmNotificationCommandTests.cs new file mode 100644 index 0000000000..a5cfa7a602 --- /dev/null +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/PushAutoConfirmNotificationCommandTests.cs @@ -0,0 +1,286 @@ +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Models; +using Bit.Core.Models.Data.Organizations.OrganizationUsers; +using Bit.Core.Platform.Push; +using Bit.Core.Repositories; +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 PushAutoConfirmNotificationCommandTests +{ + [Theory] + [BitAutoData] + public async Task PushAsync_SendsNotificationToAdminsAndOwners( + SutProvider sutProvider, + Guid userId, + Guid organizationId, + OrganizationUser orgUser, + List admins) + { + foreach (var admin in admins) + { + admin.UserId = Guid.NewGuid(); + } + + orgUser.Id = Guid.NewGuid(); + + sutProvider.GetDependency() + .GetByOrganizationAsync(organizationId, userId) + .Returns(orgUser); + + sutProvider.GetDependency() + .GetManyByMinimumRoleAsync(organizationId, OrganizationUserType.Admin) + .Returns(admins); + + sutProvider.GetDependency() + .GetManyDetailsByRoleAsync(organizationId, OrganizationUserType.Custom) + .Returns(new List()); + + await sutProvider.Sut.PushAsync(userId, organizationId); + + await sutProvider.GetDependency() + .Received(admins.Count) + .PushAsync(Arg.Is>(pn => + pn.Type == PushType.AutoConfirm && + pn.Target == NotificationTarget.User && + pn.Payload.OrganizationId == organizationId && + pn.Payload.TargetUserId == orgUser.Id && + pn.ExcludeCurrentContext == false)); + } + + [Theory] + [BitAutoData] + public async Task PushAsync_SendsNotificationToCustomUsersWithManageUsersPermission( + SutProvider sutProvider, + Guid userId, + Guid organizationId, + OrganizationUser orgUser, + List customUsers) + { + foreach (var customUser in customUsers) + { + customUser.UserId = Guid.NewGuid(); + customUser.Permissions = "{\"manageUsers\":true}"; + } + + orgUser.Id = Guid.NewGuid(); + + sutProvider.GetDependency() + .GetByOrganizationAsync(organizationId, userId) + .Returns(orgUser); + + sutProvider.GetDependency() + .GetManyByMinimumRoleAsync(organizationId, OrganizationUserType.Admin) + .Returns(new List()); + + sutProvider.GetDependency() + .GetManyDetailsByRoleAsync(organizationId, OrganizationUserType.Custom) + .Returns(customUsers); + + await sutProvider.Sut.PushAsync(userId, organizationId); + + await sutProvider.GetDependency() + .Received(customUsers.Count) + .PushAsync(Arg.Is>(pn => + pn.Type == PushType.AutoConfirm && + pn.Target == NotificationTarget.User && + pn.Payload.OrganizationId == organizationId && + pn.Payload.TargetUserId == orgUser.Id && + pn.ExcludeCurrentContext == false)); + } + + [Theory] + [BitAutoData] + public async Task PushAsync_DoesNotSendToCustomUsersWithoutManageUsersPermission( + SutProvider sutProvider, + Guid userId, + Guid organizationId, + OrganizationUser orgUser, + List customUsers) + { + foreach (var customUser in customUsers) + { + customUser.UserId = Guid.NewGuid(); + customUser.Permissions = "{\"manageUsers\":false}"; + } + + orgUser.Id = Guid.NewGuid(); + + sutProvider.GetDependency() + .GetByOrganizationAsync(organizationId, userId) + .Returns(orgUser); + + sutProvider.GetDependency() + .GetManyByMinimumRoleAsync(organizationId, OrganizationUserType.Admin) + .Returns(new List()); + + sutProvider.GetDependency() + .GetManyDetailsByRoleAsync(organizationId, OrganizationUserType.Custom) + .Returns(customUsers); + + await sutProvider.Sut.PushAsync(userId, organizationId); + + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .PushAsync(Arg.Any>()); + } + + [Theory] + [BitAutoData] + public async Task PushAsync_SendsToAdminsAndCustomUsersWithManageUsers( + SutProvider sutProvider, + Guid userId, + Guid organizationId, + OrganizationUser orgUser, + List admins, + List customUsersWithPermission, + List customUsersWithoutPermission) + { + foreach (var admin in admins) + { + admin.UserId = Guid.NewGuid(); + } + + foreach (var customUser in customUsersWithPermission) + { + customUser.UserId = Guid.NewGuid(); + customUser.Permissions = "{\"manageUsers\":true}"; + } + + foreach (var customUser in customUsersWithoutPermission) + { + customUser.UserId = Guid.NewGuid(); + customUser.Permissions = "{\"manageUsers\":false}"; + } + + orgUser.Id = Guid.NewGuid(); + + var allCustomUsers = customUsersWithPermission.Concat(customUsersWithoutPermission).ToList(); + + sutProvider.GetDependency() + .GetByOrganizationAsync(organizationId, userId) + .Returns(orgUser); + + sutProvider.GetDependency() + .GetManyByMinimumRoleAsync(organizationId, OrganizationUserType.Admin) + .Returns(admins); + + sutProvider.GetDependency() + .GetManyDetailsByRoleAsync(organizationId, OrganizationUserType.Custom) + .Returns(allCustomUsers); + + await sutProvider.Sut.PushAsync(userId, organizationId); + + var expectedNotificationCount = admins.Count + customUsersWithPermission.Count; + await sutProvider.GetDependency() + .Received(expectedNotificationCount) + .PushAsync(Arg.Is>(pn => + pn.Type == PushType.AutoConfirm && + pn.Target == NotificationTarget.User && + pn.Payload.OrganizationId == organizationId && + pn.Payload.TargetUserId == orgUser.Id && + pn.ExcludeCurrentContext == false)); + } + + [Theory] + [BitAutoData] + public async Task PushAsync_SkipsUsersWithoutUserId( + SutProvider sutProvider, + Guid userId, + Guid organizationId, + OrganizationUser orgUser, + List admins) + { + admins[0].UserId = Guid.NewGuid(); + admins[1].UserId = null; + admins[2].UserId = Guid.NewGuid(); + + orgUser.Id = Guid.NewGuid(); + + sutProvider.GetDependency() + .GetByOrganizationAsync(organizationId, userId) + .Returns(orgUser); + + sutProvider.GetDependency() + .GetManyByMinimumRoleAsync(organizationId, OrganizationUserType.Admin) + .Returns(admins); + + sutProvider.GetDependency() + .GetManyDetailsByRoleAsync(organizationId, OrganizationUserType.Custom) + .Returns(new List()); + + await sutProvider.Sut.PushAsync(userId, organizationId); + + await sutProvider.GetDependency() + .Received(2) + .PushAsync(Arg.Is>(pn => + pn.Type == PushType.AutoConfirm)); + } + + [Theory] + [BitAutoData] + public async Task PushAsync_DeduplicatesUserIds( + SutProvider sutProvider, + Guid userId, + Guid organizationId, + OrganizationUser orgUser, + Guid duplicateUserId) + { + var admin1 = new OrganizationUserUserDetails { UserId = duplicateUserId }; + var admin2 = new OrganizationUserUserDetails { UserId = duplicateUserId }; + var customUser = new OrganizationUserUserDetails + { + UserId = duplicateUserId, + Permissions = "{\"manageUsers\":true}" + }; + + orgUser.Id = Guid.NewGuid(); + + sutProvider.GetDependency() + .GetByOrganizationAsync(organizationId, userId) + .Returns(orgUser); + + sutProvider.GetDependency() + .GetManyByMinimumRoleAsync(organizationId, OrganizationUserType.Admin) + .Returns(new List { admin1, admin2 }); + + sutProvider.GetDependency() + .GetManyDetailsByRoleAsync(organizationId, OrganizationUserType.Custom) + .Returns(new List { customUser }); + + await sutProvider.Sut.PushAsync(userId, organizationId); + + await sutProvider.GetDependency() + .Received(1) + .PushAsync(Arg.Is>(pn => + pn.TargetId == duplicateUserId)); + } + + [Theory] + [BitAutoData] + public async Task PushAsync_OrganizationUserNotFound_ThrowsException( + SutProvider sutProvider, + Guid userId, + Guid organizationId) + { + sutProvider.GetDependency() + .GetByOrganizationAsync(organizationId, userId) + .Returns((OrganizationUser)null); + + var exception = await Assert.ThrowsAsync(() => + sutProvider.Sut.PushAsync(userId, organizationId)); + + Assert.Equal("Organization user not found", exception.Message); + + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .PushAsync(Arg.Any>()); + } +}