diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index 01fc40562d2d..d7e1ecc94bc1 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -255,6 +255,7 @@ public static class FeatureFlagKeys public const string WebAuthnRelatedOrigins = "pm-30529-webauthn-related-origins"; public const string ElectronStorageCache = "pm-32783-electron-storage-cache"; public const string AttachmentUploadProgress = "pm-34410-attachment-upload-progress"; + public const string OrgCipherPushFanout = "pm-35168-org-cipher-push-fanout"; /* Tools Team */ /// diff --git a/src/Core/Platform/Push/Engines/AzureQueuePushEngine.cs b/src/Core/Platform/Push/Engines/AzureQueuePushEngine.cs index e8c8790c64c9..5eb416ae5ef7 100644 --- a/src/Core/Platform/Push/Engines/AzureQueuePushEngine.cs +++ b/src/Core/Platform/Push/Engines/AzureQueuePushEngine.cs @@ -5,7 +5,6 @@ using Bit.Core.Models; using Bit.Core.Settings; using Bit.Core.Utilities; -using Bit.Core.Vault.Entities; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; @@ -31,33 +30,6 @@ public AzureQueuePushEngine( } } - public async Task PushCipherAsync(Cipher cipher, PushType type, IEnumerable? collectionIds) - { - if (cipher.OrganizationId.HasValue) - { - var message = new SyncCipherPushNotification - { - Id = cipher.Id, - OrganizationId = cipher.OrganizationId, - RevisionDate = cipher.RevisionDate, - CollectionIds = collectionIds, - }; - - await SendMessageAsync(type, message, true); - } - else if (cipher.UserId.HasValue) - { - var message = new SyncCipherPushNotification - { - Id = cipher.Id, - UserId = cipher.UserId, - RevisionDate = cipher.RevisionDate, - }; - - await SendMessageAsync(type, message, true); - } - } - private async Task SendMessageAsync(PushType type, T payload, bool excludeCurrentContext) { var contextId = GetContextIdentifier(excludeCurrentContext); diff --git a/src/Core/Platform/Push/Engines/MultiServicePushNotificationService.cs b/src/Core/Platform/Push/Engines/MultiServicePushNotificationService.cs index 1dbd2c83e5ec..f7e2703cf595 100644 --- a/src/Core/Platform/Push/Engines/MultiServicePushNotificationService.cs +++ b/src/Core/Platform/Push/Engines/MultiServicePushNotificationService.cs @@ -1,6 +1,4 @@ -using Bit.Core.Enums; -using Bit.Core.Settings; -using Bit.Core.Vault.Entities; +using Bit.Core.Settings; using Microsoft.Extensions.Logging; namespace Bit.Core.Platform.Push.Internal; @@ -66,10 +64,6 @@ private Task PushToServices(Func pushFunc) #endif } - public Task PushCipherAsync(Cipher cipher, PushType pushType, IEnumerable? collectionIds) - { - return PushToServices((s) => s.PushCipherAsync(cipher, pushType, collectionIds)); - } public Task PushAsync(PushNotification pushNotification) where T : class { return PushToServices((s) => s.PushAsync(pushNotification)); diff --git a/src/Core/Platform/Push/Engines/NoopPushEngine.cs b/src/Core/Platform/Push/Engines/NoopPushEngine.cs index 029d6dd556da..4318cd3c989f 100644 --- a/src/Core/Platform/Push/Engines/NoopPushEngine.cs +++ b/src/Core/Platform/Push/Engines/NoopPushEngine.cs @@ -1,11 +1,6 @@ -using Bit.Core.Enums; -using Bit.Core.Vault.Entities; - -namespace Bit.Core.Platform.Push.Internal; +namespace Bit.Core.Platform.Push.Internal; internal class NoopPushEngine : IPushEngine { - public Task PushCipherAsync(Cipher cipher, PushType pushType, IEnumerable? collectionIds) => Task.CompletedTask; - public Task PushAsync(PushNotification pushNotification) where T : class => Task.CompletedTask; } diff --git a/src/Core/Platform/Push/Engines/NotificationsApiPushEngine.cs b/src/Core/Platform/Push/Engines/NotificationsApiPushEngine.cs index add53278ff33..f57031af39ab 100644 --- a/src/Core/Platform/Push/Engines/NotificationsApiPushEngine.cs +++ b/src/Core/Platform/Push/Engines/NotificationsApiPushEngine.cs @@ -3,7 +3,6 @@ using Bit.Core.Models; using Bit.Core.Services; using Bit.Core.Settings; -using Bit.Core.Vault.Entities; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.Logging; @@ -35,34 +34,6 @@ public NotificationsApiPushEngine( _httpContextAccessor = httpContextAccessor; } - public async Task PushCipherAsync(Cipher cipher, PushType type, IEnumerable? collectionIds) - { - if (cipher.OrganizationId.HasValue) - { - var message = new SyncCipherPushNotification - { - Id = cipher.Id, - OrganizationId = cipher.OrganizationId, - RevisionDate = cipher.RevisionDate, - CollectionIds = collectionIds, - }; - - await SendMessageAsync(type, message, true); - } - else if (cipher.UserId.HasValue) - { - var message = new SyncCipherPushNotification - { - Id = cipher.Id, - UserId = cipher.UserId, - RevisionDate = cipher.RevisionDate, - CollectionIds = collectionIds, - }; - - await SendMessageAsync(type, message, true); - } - } - private async Task SendMessageAsync(PushType type, T payload, bool excludeCurrentContext) { var contextId = GetContextIdentifier(excludeCurrentContext); diff --git a/src/Core/Platform/Push/Engines/RelayPushEngine.cs b/src/Core/Platform/Push/Engines/RelayPushEngine.cs index cff077c8504e..61a2f9624853 100644 --- a/src/Core/Platform/Push/Engines/RelayPushEngine.cs +++ b/src/Core/Platform/Push/Engines/RelayPushEngine.cs @@ -1,12 +1,9 @@ using Bit.Core.Auth.IdentityServer; using Bit.Core.Context; -using Bit.Core.Enums; -using Bit.Core.Models; using Bit.Core.Models.Api; using Bit.Core.Repositories; using Bit.Core.Services; using Bit.Core.Settings; -using Bit.Core.Vault.Entities; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; @@ -43,41 +40,14 @@ public RelayPushEngine( _httpContextAccessor = httpContextAccessor; } - public async Task PushCipherAsync(Cipher cipher, PushType type, IEnumerable? collectionIds) + public async Task PushAsync(PushNotification pushNotification) + where T : class { - if (cipher.OrganizationId.HasValue) - { - // We cannot send org pushes since access logic is much more complicated than just the fact that they belong - // to the organization. Potentially we could blindly send to just users that have the access all permission - // device registration needs to be more granular to handle that appropriately. A more brute force approach could - // me to send "full sync" push to all org users, but that has the potential to DDOS the API in bursts. - - // await SendPayloadToOrganizationAsync(cipher.OrganizationId.Value, type, message, true); - } - else if (cipher.UserId.HasValue) + if (pushNotification.NonMobileOnly == true) { - var message = new SyncCipherPushNotification - { - Id = cipher.Id, - UserId = cipher.UserId, - OrganizationId = cipher.OrganizationId, - RevisionDate = cipher.RevisionDate, - }; - - await PushAsync(new PushNotification - { - Type = type, - Target = NotificationTarget.User, - TargetId = cipher.UserId.Value, - Payload = message, - ExcludeCurrentContext = true, - }); + return; } - } - public async Task PushAsync(PushNotification pushNotification) - where T : class - { var deviceIdentifier = _httpContextAccessor.HttpContext ?.RequestServices.GetService() ?.DeviceIdentifier; diff --git a/src/Core/Platform/Push/IPushEngine.cs b/src/Core/Platform/Push/IPushEngine.cs index ca00dae3ad86..c38c14a43f80 100644 --- a/src/Core/Platform/Push/IPushEngine.cs +++ b/src/Core/Platform/Push/IPushEngine.cs @@ -1,12 +1,7 @@ -using Bit.Core.Enums; -using Bit.Core.Vault.Entities; - -namespace Bit.Core.Platform.Push.Internal; +namespace Bit.Core.Platform.Push.Internal; public interface IPushEngine { - Task PushCipherAsync(Cipher cipher, PushType pushType, IEnumerable? collectionIds); - Task PushAsync(PushNotification pushNotification) where T : class; } diff --git a/src/Core/Platform/Push/IPushNotificationService.cs b/src/Core/Platform/Push/IPushNotificationService.cs index 125057be1ff8..056a200664ce 100644 --- a/src/Core/Platform/Push/IPushNotificationService.cs +++ b/src/Core/Platform/Push/IPushNotificationService.cs @@ -33,15 +33,6 @@ public interface IPushNotificationService ILogger Logger { get; } #region Legacy method, to be removed soon. - Task PushSyncCipherCreateAsync(Cipher cipher, IEnumerable collectionIds) - => PushCipherAsync(cipher, PushType.SyncCipherCreate, collectionIds); - - Task PushSyncCipherUpdateAsync(Cipher cipher, IEnumerable collectionIds) - => PushCipherAsync(cipher, PushType.SyncCipherUpdate, collectionIds); - - Task PushSyncCipherDeleteAsync(Cipher cipher) - => PushCipherAsync(cipher, PushType.SyncLoginDelete, null); - Task PushSyncFolderCreateAsync(Folder folder) => PushAsync(new PushNotification { @@ -431,8 +422,6 @@ Task PushRefreshSecurityTasksAsync(Guid userId) }); #endregion - Task PushCipherAsync(Cipher cipher, PushType pushType, IEnumerable? collectionIds); - /// /// Pushes a notification to devices based on the settings given to us in . /// diff --git a/src/Core/Platform/Push/NotificationHub/NotificationHubPushEngine.cs b/src/Core/Platform/Push/NotificationHub/NotificationHubPushEngine.cs index 1d1eb2ef706b..dd0355079236 100644 --- a/src/Core/Platform/Push/NotificationHub/NotificationHubPushEngine.cs +++ b/src/Core/Platform/Push/NotificationHub/NotificationHubPushEngine.cs @@ -2,11 +2,9 @@ using System.Text.RegularExpressions; using Bit.Core.Context; using Bit.Core.Enums; -using Bit.Core.Models; using Bit.Core.Models.Data; using Bit.Core.Repositories; using Bit.Core.Settings; -using Bit.Core.Vault.Entities; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.Logging; @@ -42,39 +40,6 @@ public NotificationHubPushEngine( } } - public async Task PushCipherAsync(Cipher cipher, PushType type, IEnumerable? collectionIds) - { - if (cipher.OrganizationId.HasValue) - { - // We cannot send org pushes since access logic is much more complicated than just the fact that they belong - // to the organization. Potentially we could blindly send to just users that have the access all permission - // device registration needs to be more granular to handle that appropriately. A more brute force approach could - // me to send "full sync" push to all org users, but that has the potential to DDOS the API in bursts. - - // await SendPayloadToOrganizationAsync(cipher.OrganizationId.Value, type, message, true); - } - else if (cipher.UserId.HasValue) - { - var message = new SyncCipherPushNotification - { - Id = cipher.Id, - UserId = cipher.UserId, - OrganizationId = cipher.OrganizationId, - RevisionDate = cipher.RevisionDate, - CollectionIds = collectionIds, - }; - - await PushAsync(new PushNotification - { - Type = type, - Target = NotificationTarget.User, - TargetId = cipher.UserId.Value, - Payload = message, - ExcludeCurrentContext = true, - }); - } - } - private string? GetContextIdentifier(bool excludeCurrentContext) { if (!excludeCurrentContext) @@ -105,6 +70,11 @@ private string BuildTag(string tag, string? identifier, ClientType? clientType) public async Task PushAsync(PushNotification pushNotification) where T : class { + if (pushNotification.NonMobileOnly == true) + { + return; + } + var initialTag = pushNotification.Target switch { NotificationTarget.User => $"template:payload_userId:{pushNotification.TargetId}", diff --git a/src/Core/Platform/Push/PushNotification.cs b/src/Core/Platform/Push/PushNotification.cs index 397e9abe232a..c60cc1dc3f68 100644 --- a/src/Core/Platform/Push/PushNotification.cs +++ b/src/Core/Platform/Push/PushNotification.cs @@ -13,7 +13,7 @@ public record PushNotification /// /// The to be associated with the notification. This is used to route /// the notification to the correct handler on the client side. Be sure to use the correct payload - /// type for the associated . + /// type for the associated . /// public required PushType Type { get; init; } @@ -21,9 +21,9 @@ public record PushNotification /// The target entity type for the notification. /// /// - /// When the target type is the + /// When the target type is the /// property is expected to be a users ID. When it is - /// it should be an organizations id. When it is a + /// it should be an organizations id. When it is a /// it should be an installation id. /// public required NotificationTarget Target { get; init; } @@ -45,11 +45,17 @@ public record PushNotification public required bool ExcludeCurrentContext { get; init; } /// - /// The type of clients the notification should be sent to, if then + /// The type of clients the notification should be sent to, if then /// is inferred. /// public ClientType? ClientType { get; init; } + + /// + /// When true, only non-mobile engines (SignalR/web/desktop) will deliver this notification. When null or false, all engines process it. + /// + public bool? NonMobileOnly { get; init; } + internal Guid? GetTargetWhen(NotificationTarget notificationTarget) { return Target == notificationTarget ? TargetId : null; diff --git a/src/Core/Platform/Push/PushServiceCollectionExtensions.cs b/src/Core/Platform/Push/PushServiceCollectionExtensions.cs index b54ae64c0817..f91fc9e146ca 100644 --- a/src/Core/Platform/Push/PushServiceCollectionExtensions.cs +++ b/src/Core/Platform/Push/PushServiceCollectionExtensions.cs @@ -14,7 +14,7 @@ public static class PushServiceCollectionExtensions { /// /// Adds a to the services that can be used to send push notifications to - /// end user devices. This method is safe to be ran multiple time provided does not + /// end user devices. This method is safe to be ran multiple time provided does not /// change between calls. /// /// The to add services to. diff --git a/src/Core/Repositories/ICollectionCipherRepository.cs b/src/Core/Repositories/ICollectionCipherRepository.cs index f7a4081b73e4..eb4de0518b1d 100644 --- a/src/Core/Repositories/ICollectionCipherRepository.cs +++ b/src/Core/Repositories/ICollectionCipherRepository.cs @@ -15,6 +15,17 @@ public interface ICollectionCipherRepository Task UpdateCollectionsForCiphersAsync(IEnumerable cipherIds, Guid userId, Guid organizationId, IEnumerable collectionIds); + /// + /// Returns the collection IDs that the specified cipher belongs to. + /// + Task> GetCollectionIdsByCipherIdAsync(Guid cipherId); + + /// + /// Returns the distinct user IDs of all confirmed organization members who have access to any of the specified + /// collections, either directly via CollectionUser or via group membership. + /// + Task> GetUserIdsByCollectionIdsAsync(IEnumerable collectionIds); + /// /// Add the specified collections to the specified ciphers. If a cipher already belongs to a requested collection, /// no action is taken. diff --git a/src/Core/Repositories/Noop/CollectionCipherRepository.cs b/src/Core/Repositories/Noop/CollectionCipherRepository.cs new file mode 100644 index 000000000000..9b16b523e6d9 --- /dev/null +++ b/src/Core/Repositories/Noop/CollectionCipherRepository.cs @@ -0,0 +1,41 @@ +using Bit.Core.Entities; + +#nullable enable + +namespace Bit.Core.Repositories.Noop; + +public class CollectionCipherRepository : ICollectionCipherRepository +{ + public Task> GetManyByUserIdAsync(Guid userId) + => Task.FromResult>([]); + + public Task> GetManyByOrganizationIdAsync(Guid organizationId) + => Task.FromResult>([]); + + public Task> GetManySharedByOrganizationIdAsync(Guid organizationId) + => Task.FromResult>([]); + + public Task> GetManyByUserIdCipherIdAsync(Guid userId, Guid cipherId) + => Task.FromResult>([]); + + public Task UpdateCollectionsAsync(Guid cipherId, Guid userId, IEnumerable collectionIds) + => Task.CompletedTask; + + public Task UpdateCollectionsForAdminAsync(Guid cipherId, Guid organizationId, IEnumerable collectionIds) + => Task.CompletedTask; + + public Task UpdateCollectionsForCiphersAsync(IEnumerable cipherIds, Guid userId, Guid organizationId, IEnumerable collectionIds) + => Task.CompletedTask; + + public Task> GetCollectionIdsByCipherIdAsync(Guid cipherId) + => Task.FromResult>([]); + + public Task> GetUserIdsByCollectionIdsAsync(IEnumerable collectionIds) + => Task.FromResult>([]); + + public Task AddCollectionsForManyCiphersAsync(Guid organizationId, IEnumerable cipherIds, IEnumerable collectionIds) + => Task.CompletedTask; + + public Task RemoveCollectionsForManyCiphersAsync(Guid organizationId, IEnumerable cipherIds, IEnumerable collectionIds) + => Task.CompletedTask; +} diff --git a/src/Core/Vault/Services/ICipherService.cs b/src/Core/Vault/Services/ICipherService.cs index 2fe9ac4e0332..74c6f6236472 100644 --- a/src/Core/Vault/Services/ICipherService.cs +++ b/src/Core/Vault/Services/ICipherService.cs @@ -1,16 +1,13 @@ -// FIXME: Update this file to be null safe and then delete the line below -#nullable disable - -using Bit.Core.Vault.Entities; +using Bit.Core.Vault.Entities; using Bit.Core.Vault.Models.Data; namespace Bit.Core.Vault.Services; public interface ICipherService { - Task SaveAsync(Cipher cipher, Guid savingUserId, DateTime? lastKnownRevisionDate, IEnumerable collectionIds = null, + Task SaveAsync(Cipher cipher, Guid savingUserId, DateTime? lastKnownRevisionDate, IEnumerable? collectionIds = null, bool skipPermissionCheck = false, bool limitCollectionScope = true); Task SaveDetailsAsync(CipherDetails cipher, Guid savingUserId, DateTime? lastKnownRevisionDate, - IEnumerable collectionIds = null, bool skipPermissionCheck = false); + IEnumerable? collectionIds = null, bool skipPermissionCheck = false); Task<(string attachmentId, string uploadUrl)> CreateAttachmentForDelayedUploadAsync(Cipher cipher, string key, string fileName, long fileSize, bool adminRequest, Guid savingUserId, DateTime? lastKnownRevisionDate = null); Task CreateAttachmentAsync(Cipher cipher, Stream stream, string fileName, string key, @@ -34,7 +31,7 @@ Task> ShareManyAsync(IEnumerable<(CipherDetails ciphe Task RestoreAsync(CipherDetails cipherDetails, Guid restoringUserId, bool orgAdmin = false); Task> RestoreManyAsync(IEnumerable cipherIds, Guid restoringUserId, Guid? organizationId = null, bool orgAdmin = false); Task UploadFileForExistingAttachmentAsync(Stream stream, Cipher cipher, CipherAttachment.MetaData attachmentId, Guid savingUserId, bool orgAdmin = false); - Task GetAttachmentDownloadDataAsync(Cipher cipher, string attachmentId); + Task GetAttachmentDownloadDataAsync(Cipher? cipher, string attachmentId); Task ValidateCipherAttachmentFile(Cipher cipher, CipherAttachment.MetaData attachmentData); Task ValidateBulkCollectionAssignmentAsync(IEnumerable collectionIds, IEnumerable cipherIds, Guid userId); Task ValidateCipherEditForAttachmentAsync(Cipher cipher, Guid savingUserId, bool orgAdmin, long requestLength); diff --git a/src/Core/Vault/Services/ICipherSyncPushService.cs b/src/Core/Vault/Services/ICipherSyncPushService.cs new file mode 100644 index 000000000000..8e276dbbcc77 --- /dev/null +++ b/src/Core/Vault/Services/ICipherSyncPushService.cs @@ -0,0 +1,10 @@ +using Bit.Core.Vault.Entities; + +namespace Bit.Core.Vault.Services; + +public interface ICipherSyncPushService +{ + Task PushSyncCipherCreateAsync(Cipher cipher, IEnumerable collectionIds); + Task PushSyncCipherUpdateAsync(Cipher cipher, IEnumerable collectionIds); + Task PushSyncCipherDeleteAsync(Cipher cipher, IEnumerable? collectionIds = null); +} diff --git a/src/Core/Vault/Services/Implementations/CipherService.cs b/src/Core/Vault/Services/Implementations/CipherService.cs index f0d99745691f..e38be6ef15b0 100644 --- a/src/Core/Vault/Services/Implementations/CipherService.cs +++ b/src/Core/Vault/Services/Implementations/CipherService.cs @@ -1,7 +1,4 @@ -// FIXME: Update this file to be null safe and then delete the line below -#nullable disable - -using System.Text.Json; +using System.Text.Json; using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.OrganizationFeatures.Policies; using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; @@ -34,6 +31,7 @@ public class CipherService : ICipherService private readonly ICollectionCipherRepository _collectionCipherRepository; private readonly ISecurityTaskRepository _securityTaskRepository; private readonly IPushNotificationService _pushService; + private readonly ICipherSyncPushService _cipherSyncPushService; private readonly IAttachmentStorageService _attachmentStorageService; private readonly IEventService _eventService; private readonly IUserService _userService; @@ -54,6 +52,7 @@ public CipherService( ICollectionCipherRepository collectionCipherRepository, ISecurityTaskRepository securityTaskRepository, IPushNotificationService pushService, + ICipherSyncPushService cipherSyncPushService, IAttachmentStorageService attachmentStorageService, IEventService eventService, IUserService userService, @@ -72,6 +71,7 @@ public CipherService( _collectionCipherRepository = collectionCipherRepository; _securityTaskRepository = securityTaskRepository; _pushService = pushService; + _cipherSyncPushService = cipherSyncPushService; _attachmentStorageService = attachmentStorageService; _eventService = eventService; _userService = userService; @@ -84,7 +84,7 @@ public CipherService( } public async Task SaveAsync(Cipher cipher, Guid savingUserId, DateTime? lastKnownRevisionDate, - IEnumerable collectionIds = null, bool skipPermissionCheck = false, bool limitCollectionScope = true) + IEnumerable? collectionIds = null, bool skipPermissionCheck = false, bool limitCollectionScope = true) { if (!skipPermissionCheck && !(await UserCanEditAsync(cipher, savingUserId))) { @@ -109,7 +109,7 @@ public async Task SaveAsync(Cipher cipher, Guid savingUserId, DateTime? lastKnow await _eventService.LogCipherEventAsync(cipher, Bit.Core.Enums.EventType.Cipher_Created); // push - await _pushService.PushSyncCipherCreateAsync(cipher, null); + await _cipherSyncPushService.PushSyncCipherCreateAsync(cipher, collectionIds ?? Array.Empty()); } else { @@ -119,12 +119,12 @@ public async Task SaveAsync(Cipher cipher, Guid savingUserId, DateTime? lastKnow await _eventService.LogCipherEventAsync(cipher, Bit.Core.Enums.EventType.Cipher_Updated); // push - await _pushService.PushSyncCipherUpdateAsync(cipher, collectionIds); + await _cipherSyncPushService.PushSyncCipherUpdateAsync(cipher, collectionIds ?? Array.Empty()); } } public async Task SaveDetailsAsync(CipherDetails cipher, Guid savingUserId, DateTime? lastKnownRevisionDate, - IEnumerable collectionIds = null, bool skipPermissionCheck = false) + IEnumerable? collectionIds = null, bool skipPermissionCheck = false) { if (!skipPermissionCheck && !(await UserCanEditAsync(cipher, savingUserId))) { @@ -159,11 +159,14 @@ public async Task SaveDetailsAsync(CipherDetails cipher, Guid savingUserId, Date if (cipher.OrganizationId.HasValue) { var org = await _organizationRepository.GetByIdAsync(cipher.OrganizationId.Value); - cipher.OrganizationUseTotp = org.UseTotp; + if (org != null) + { + cipher.OrganizationUseTotp = org.UseTotp; + } } // push - await _pushService.PushSyncCipherCreateAsync(cipher, null); + await _cipherSyncPushService.PushSyncCipherCreateAsync(cipher, collectionIds ?? Array.Empty()); } else { @@ -174,19 +177,19 @@ public async Task SaveDetailsAsync(CipherDetails cipher, Guid savingUserId, Date await _eventService.LogCipherEventAsync(cipher, Bit.Core.Enums.EventType.Cipher_Updated); // push - await _pushService.PushSyncCipherUpdateAsync(cipher, collectionIds); + await _cipherSyncPushService.PushSyncCipherUpdateAsync(cipher, collectionIds ?? Array.Empty()); } } public async Task UploadFileForExistingAttachmentAsync(Stream stream, Cipher cipher, CipherAttachment.MetaData attachment, Guid savingUserId, bool orgAdmin = false) { - await ValidateCipherEditForAttachmentAsync(cipher, savingUserId, orgAdmin, attachment.Size); - if (attachment == null) { throw new BadRequestException("Cipher attachment does not exist"); } + await ValidateCipherEditForAttachmentAsync(cipher, savingUserId, orgAdmin, attachment.Size); + await _attachmentStorageService.UploadNewAttachmentAsync(stream, cipher, attachment); if (!await ValidateCipherAttachmentFile(cipher, attachment)) @@ -229,7 +232,7 @@ await _cipherRepository.UpdateAttachmentAsync(new CipherAttachment cipher.RevisionDate = DateTime.UtcNow; await _cipherRepository.ReplaceAsync((CipherDetails)cipher); - await _pushService.PushSyncCipherUpdateAsync(cipher, null); + await _cipherSyncPushService.PushSyncCipherUpdateAsync(cipher, Array.Empty()); return (attachmentId, uploadUrl); } @@ -285,7 +288,7 @@ public async Task CreateAttachmentAsync(Cipher cipher, Stream stream, string fil await _cipherRepository.ReplaceAsync((CipherDetails)cipher); // push - await _pushService.PushSyncCipherUpdateAsync(cipher, null); + await _cipherSyncPushService.PushSyncCipherUpdateAsync(cipher, Array.Empty()); } public async Task CreateAttachmentShareAsync(Cipher cipher, Stream stream, string fileName, string key, @@ -377,7 +380,7 @@ public async Task ValidateCipherAttachmentFile(Cipher cipher, CipherAttach return false; } // Update Send data if necessary - if (realSize != attachmentData.Size) + if (realSize.HasValue && realSize != attachmentData.Size) { attachmentData.Size = realSize.Value; } @@ -398,9 +401,14 @@ public async Task ValidateCipherAttachmentFile(Cipher cipher, CipherAttach return valid; } - public async Task GetAttachmentDownloadDataAsync(Cipher cipher, string attachmentId) + public async Task GetAttachmentDownloadDataAsync(Cipher? cipher, string attachmentId) { - var attachments = cipher?.GetAttachments() ?? new Dictionary(); + if (cipher == null) + { + throw new NotFoundException(); + } + + var attachments = cipher.GetAttachments() ?? new Dictionary(); if (!attachments.TryGetValue(attachmentId, out var data)) { @@ -427,12 +435,14 @@ public async Task DeleteAsync(CipherDetails cipherDetails, Guid deletingUserId, throw new BadRequestException("You do not have permissions to delete this."); } + var collectionIds = await GetCollectionIdsForPushAsync(cipherDetails); + await _cipherRepository.DeleteAsync(cipherDetails); await _attachmentStorageService.DeleteAttachmentsForCipherAsync(cipherDetails.Id); await _eventService.LogCipherEventAsync(cipherDetails, EventType.Cipher_Deleted); // push - await _pushService.PushSyncCipherDeleteAsync(cipherDetails); + await _cipherSyncPushService.PushSyncCipherDeleteAsync(cipherDetails, collectionIds); } public async Task DeleteManyAsync(IEnumerable cipherIds, Guid deletingUserId, Guid? organizationId = null, bool orgAdmin = false) @@ -484,7 +494,8 @@ public async Task DeleteAttachmentAsync(Cipher cip throw new NotFoundException(); } - return await DeleteAttachmentAsync(cipher, cipher.GetAttachments()[attachmentId], orgAdmin); + return await DeleteAttachmentAsync(cipher, cipher.GetAttachments()[attachmentId], orgAdmin) + ?? throw new NotFoundException(); } public async Task PurgeAsync(Guid organizationId) @@ -560,11 +571,11 @@ public async Task DeleteFolderAsync(Folder folder) public async Task ShareAsync(Cipher originalCipher, Cipher cipher, Guid organizationId, IEnumerable collectionIds, Guid sharingUserId, DateTime? lastKnownRevisionDate) { - var attachments = cipher.GetAttachments(); - var hasOldAttachments = attachments?.Values?.Any(a => a.Key == null) ?? false; + var attachments = cipher.GetAttachments() ?? new Dictionary(); + var hasOldAttachments = attachments.Values.Any(a => a.Key == null); var updatedCipher = false; var migratedAttachments = false; - var originalAttachments = CoreHelpers.CloneObject(originalCipher.GetAttachments()); + var originalAttachments = CoreHelpers.CloneObject(originalCipher.GetAttachments()) ?? new Dictionary(); try { @@ -578,7 +589,7 @@ public async Task ShareAsync(Cipher originalCipher, Cipher cipher, Guid organiza if (hasOldAttachments) { - var attachmentsWithUpdatedMetadata = originalCipher.GetAttachments(); + var attachmentsWithUpdatedMetadata = originalCipher.GetAttachments() ?? new Dictionary(); var attachmentsToUpdateMetadata = CoreHelpers.CloneObject(attachments); foreach (var updatedMetadata in attachmentsWithUpdatedMetadata.Where(a => a.Value?.TempMetadata != null)) { @@ -601,7 +612,7 @@ public async Task ShareAsync(Cipher originalCipher, Cipher cipher, Guid organiza if (hasOldAttachments) { // migrate old attachments - foreach (var attachment in attachments.Values.Where(a => a.TempMetadata != null).Select(a => a.TempMetadata)) + foreach (var attachment in attachments.Values.Where(a => a.TempMetadata != null).Select(a => a.TempMetadata!)) { await _attachmentStorageService.StartShareAttachmentAsync(cipher.Id, organizationId, attachment); @@ -656,7 +667,7 @@ await _attachmentStorageService.RollbackShareAttachmentAsync(cipher.Id, organiza } // push - await _pushService.PushSyncCipherUpdateAsync(cipher, collectionIds); + await _cipherSyncPushService.PushSyncCipherUpdateAsync(cipher, collectionIds); } public async Task> ShareManyAsync(IEnumerable<(CipherDetails cipher, DateTime? lastKnownRevisionDate)> cipherInfos, @@ -724,7 +735,7 @@ await _collectionCipherRepository.UpdateCollectionsForAdminAsync(cipher.Id, await _eventService.LogCipherEventAsync(cipher, EventType.Cipher_UpdatedCollections); // push - await _pushService.PushSyncCipherUpdateAsync(cipher, collectionIds); + await _cipherSyncPushService.PushSyncCipherUpdateAsync(cipher, collectionIds); } public async Task SoftDeleteAsync(CipherDetails cipherDetails, Guid deletingUserId, bool orgAdmin = false) @@ -747,7 +758,7 @@ public async Task SoftDeleteAsync(CipherDetails cipherDetails, Guid deletingUser await _eventService.LogCipherEventAsync(cipherDetails, EventType.Cipher_SoftDeleted); // push - await _pushService.PushSyncCipherUpdateAsync(cipherDetails, null); + await _cipherSyncPushService.PushSyncCipherUpdateAsync(cipherDetails, Array.Empty()); } public async Task SoftDeleteManyAsync(IEnumerable cipherIds, Guid deletingUserId, Guid? organizationId, bool orgAdmin) @@ -802,7 +813,7 @@ public async Task RestoreAsync(CipherDetails cipherDetails, Guid restoringUserId await _eventService.LogCipherEventAsync(cipherDetails, EventType.Cipher_Restored); // push - await _pushService.PushSyncCipherUpdateAsync(cipherDetails, null); + await _cipherSyncPushService.PushSyncCipherUpdateAsync(cipherDetails, Array.Empty()); } public async Task> RestoreManyAsync(IEnumerable cipherIds, Guid restoringUserId, Guid? organizationId = null, bool orgAdmin = false) @@ -852,6 +863,10 @@ public async Task ValidateBulkCollectionAssignmentAsync(IEnumerable collec foreach (var cipherId in cipherIds) { var cipher = await _cipherRepository.GetByIdAsync(cipherId); + if (cipher == null) + { + throw new NotFoundException(); + } await ValidateChangeInCollectionsAsync(cipher, collectionIds, userId); } } @@ -899,7 +914,7 @@ private void ValidateCipherLastKnownRevisionDate(Cipher cipher, DateTime? lastKn } } - private async Task DeleteAttachmentAsync(Cipher cipher, CipherAttachment.MetaData attachmentData, bool orgAdmin) + private async Task DeleteAttachmentAsync(Cipher cipher, CipherAttachment.MetaData attachmentData, bool orgAdmin) { if (attachmentData == null || string.IsNullOrWhiteSpace(attachmentData.AttachmentId)) { @@ -923,7 +938,7 @@ private async Task DeleteAttachmentAsync(Cipher ci } // push - await _pushService.PushSyncCipherUpdateAsync(cipher, null); + await _cipherSyncPushService.PushSyncCipherUpdateAsync(cipher, Array.Empty()); return new DeleteAttachmentResponseData(cipher); } @@ -955,6 +970,11 @@ private async Task StorageBytesRemainingForCipherAsync(Cipher cipher) if (cipher.UserId.HasValue) { var user = await _userRepository.GetByIdAsync(cipher.UserId.Value); + if (user == null) + { + throw new NotFoundException(); + } + if (!(await _userService.CanAccessPremium(user))) { throw new BadRequestException("You must have premium status to use attachments."); @@ -984,6 +1004,11 @@ private async Task StorageBytesRemainingForCipherAsync(Cipher cipher) else if (cipher.OrganizationId.HasValue) { var org = await _organizationRepository.GetByIdAsync(cipher.OrganizationId.Value); + if (org == null) + { + throw new NotFoundException(); + } + if (!org.MaxStorageGb.HasValue) { throw new BadRequestException("This organization cannot use attachments."); @@ -995,6 +1020,16 @@ private async Task StorageBytesRemainingForCipherAsync(Cipher cipher) return storageBytesRemaining; } + private async Task?> GetCollectionIdsForPushAsync(Cipher cipher) + { + if (!cipher.OrganizationId.HasValue) + { + return null; + } + + return await _collectionCipherRepository.GetCollectionIdsByCipherIdAsync(cipher.Id); + } + private async Task ValidateCipherCanBeShared( Cipher cipher, Guid sharingUserId, @@ -1063,7 +1098,7 @@ private async Task IgnoreStorageLimitsOnMigrationAsync(Guid userId, Organi } // Validates that a cipher is not being added to a default collection when it is only currently only in shared collections - private async Task ValidateChangeInCollectionsAsync(Cipher updatedCipher, IEnumerable newCollectionIds, Guid userId) + private async Task ValidateChangeInCollectionsAsync(Cipher updatedCipher, IEnumerable? newCollectionIds, Guid userId) { if (updatedCipher.Id == Guid.Empty || !updatedCipher.OrganizationId.HasValue) @@ -1071,6 +1106,11 @@ private async Task ValidateChangeInCollectionsAsync(Cipher updatedCipher, IEnume return; } + if (newCollectionIds == null) + { + return; + } + var currentCollectionsForCipher = await _collectionCipherRepository.GetManyByUserIdCipherIdAsync(userId, updatedCipher.Id); if (!currentCollectionsForCipher.Any()) @@ -1120,14 +1160,14 @@ private CipherData DeserializeCipherData(Cipher cipher) { return cipher.Type switch { - CipherType.Login => JsonSerializer.Deserialize(cipher.Data), - CipherType.Identity => JsonSerializer.Deserialize(cipher.Data), - CipherType.Card => JsonSerializer.Deserialize(cipher.Data), - CipherType.SecureNote => JsonSerializer.Deserialize(cipher.Data), - CipherType.SSHKey => JsonSerializer.Deserialize(cipher.Data), - CipherType.BankAccount => JsonSerializer.Deserialize(cipher.Data), - CipherType.DriversLicense => JsonSerializer.Deserialize(cipher.Data), - CipherType.Passport => JsonSerializer.Deserialize(cipher.Data), + CipherType.Login => JsonSerializer.Deserialize(cipher.Data)!, + CipherType.Identity => JsonSerializer.Deserialize(cipher.Data)!, + CipherType.Card => JsonSerializer.Deserialize(cipher.Data)!, + CipherType.SecureNote => JsonSerializer.Deserialize(cipher.Data)!, + CipherType.SSHKey => JsonSerializer.Deserialize(cipher.Data)!, + CipherType.BankAccount => JsonSerializer.Deserialize(cipher.Data)!, + CipherType.DriversLicense => JsonSerializer.Deserialize(cipher.Data)!, + CipherType.Passport => JsonSerializer.Deserialize(cipher.Data)!, _ => throw new ArgumentException("Unsupported cipher type.", nameof(cipher)) }; } diff --git a/src/Core/Vault/Services/Implementations/CipherSyncPushService.cs b/src/Core/Vault/Services/Implementations/CipherSyncPushService.cs new file mode 100644 index 000000000000..2cc21ae15954 --- /dev/null +++ b/src/Core/Vault/Services/Implementations/CipherSyncPushService.cs @@ -0,0 +1,122 @@ +using Bit.Core.Enums; +using Bit.Core.Models; +using Bit.Core.Platform.Push; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Bit.Core.Vault.Entities; +using Microsoft.Extensions.Logging; + +namespace Bit.Core.Vault.Services; + +public class CipherSyncPushService : ICipherSyncPushService +{ + private readonly IPushNotificationService _pushNotificationService; + private readonly ICollectionCipherRepository _collectionCipherRepository; + private readonly IFeatureService _featureService; + private readonly ILogger _logger; + + public CipherSyncPushService( + IPushNotificationService pushNotificationService, + ICollectionCipherRepository collectionCipherRepository, + IFeatureService featureService, + ILogger logger) + { + _pushNotificationService = pushNotificationService; + _collectionCipherRepository = collectionCipherRepository; + _featureService = featureService; + _logger = logger; + } + + public Task PushSyncCipherCreateAsync(Cipher cipher, IEnumerable collectionIds) + => PushCipherAsync(cipher, PushType.SyncCipherCreate, collectionIds); + + public Task PushSyncCipherUpdateAsync(Cipher cipher, IEnumerable collectionIds) + => PushCipherAsync(cipher, PushType.SyncCipherUpdate, collectionIds); + + public Task PushSyncCipherDeleteAsync(Cipher cipher, IEnumerable? collectionIds = null) + => PushCipherAsync(cipher, PushType.SyncLoginDelete, collectionIds); + + private async Task PushCipherAsync(Cipher cipher, PushType pushType, IEnumerable? collectionIds) + { + if (cipher.OrganizationId.HasValue) + { + if (!_featureService.IsEnabled(FeatureFlagKeys.OrgCipherPushFanout)) + { + // Device registrations in Notification Hub and Relay are not collection-aware, so we cannot + // safely fan out to individual users on those mobile engines. Restrict to the non-mobile + // (SignalR) path, which routes by organizationId on the receiving end. + await _pushNotificationService.PushAsync(new PushNotification + { + Type = pushType, + Target = NotificationTarget.Organization, + TargetId = cipher.OrganizationId.Value, + Payload = new SyncCipherPushNotification + { + Id = cipher.Id, + OrganizationId = cipher.OrganizationId, + RevisionDate = cipher.RevisionDate, + }, + ExcludeCurrentContext = true, + NonMobileOnly = true, + }); + return; + } + + var collectionIdList = collectionIds?.Distinct().ToList() ?? []; + if (collectionIdList.Count == 0) + { + collectionIdList = [.. await _collectionCipherRepository.GetCollectionIdsByCipherIdAsync(cipher.Id)]; + if (collectionIdList.Count == 0) + { + _logger.LogWarning( + "Skipping push notification for organization cipher {CipherId} in organization {OrganizationId} because no collection IDs were provided or found.", + cipher.Id, + cipher.OrganizationId.Value); + return; + } + } + + var userIds = await _collectionCipherRepository.GetUserIdsByCollectionIdsAsync(collectionIdList); + var pushTasks = userIds.Select(userId => + _pushNotificationService.PushAsync(new PushNotification + { + Type = pushType, + Target = NotificationTarget.User, + TargetId = userId, + Payload = new SyncCipherPushNotification + { + Id = cipher.Id, + UserId = userId, + OrganizationId = cipher.OrganizationId, + RevisionDate = cipher.RevisionDate, + CollectionIds = collectionIdList, + }, + ExcludeCurrentContext = true, + })); + + await Task.WhenAll(pushTasks); + return; + } + + if (!cipher.UserId.HasValue) + { + return; + } + + await _pushNotificationService.PushAsync(new PushNotification + { + Type = pushType, + Target = NotificationTarget.User, + TargetId = cipher.UserId.Value, + Payload = new SyncCipherPushNotification + { + Id = cipher.Id, + UserId = cipher.UserId, + OrganizationId = cipher.OrganizationId, + RevisionDate = cipher.RevisionDate, + CollectionIds = collectionIds, + }, + ExcludeCurrentContext = true, + }); + } +} diff --git a/src/Infrastructure.Dapper/Repositories/CollectionCipherRepository.cs b/src/Infrastructure.Dapper/Repositories/CollectionCipherRepository.cs index 64b1a74072f8..874b98f3fef1 100644 --- a/src/Infrastructure.Dapper/Repositories/CollectionCipherRepository.cs +++ b/src/Infrastructure.Dapper/Repositories/CollectionCipherRepository.cs @@ -71,6 +71,32 @@ public async Task> GetManyByUserIdCipherIdAsync(Gu } } + public async Task> GetCollectionIdsByCipherIdAsync(Guid cipherId) + { + using (var connection = new SqlConnection(ConnectionString)) + { + var results = await connection.QueryAsync( + "[dbo].[CollectionCipher_ReadCollectionIdsByCipherId]", + new { CipherId = cipherId }, + commandType: CommandType.StoredProcedure); + + return results.ToList(); + } + } + + public async Task> GetUserIdsByCollectionIdsAsync(IEnumerable collectionIds) + { + using (var connection = new SqlConnection(ConnectionString)) + { + var results = await connection.QueryAsync( + "[dbo].[CollectionCipher_ReadUserIdsByCollectionIds]", + new { CollectionIds = collectionIds.ToGuidIdArrayTVP() }, + commandType: CommandType.StoredProcedure); + + return results.ToList(); + } + } + public async Task UpdateCollectionsAsync(Guid cipherId, Guid userId, IEnumerable collectionIds) { using (var connection = new SqlConnection(ConnectionString)) diff --git a/src/Infrastructure.EntityFramework/Repositories/CollectionCipherRepository.cs b/src/Infrastructure.EntityFramework/Repositories/CollectionCipherRepository.cs index 13fc146c0ffb..7696847fafef 100644 --- a/src/Infrastructure.EntityFramework/Repositories/CollectionCipherRepository.cs +++ b/src/Infrastructure.EntityFramework/Repositories/CollectionCipherRepository.cs @@ -88,6 +88,54 @@ public async Task> GetManyByUserIdCipherIdAsync(Gu } } + public async Task> GetCollectionIdsByCipherIdAsync(Guid cipherId) + { + using (var scope = ServiceScopeFactory.CreateScope()) + { + var dbContext = GetDatabaseContext(scope); + return await dbContext.CollectionCiphers + .Where(cc => cc.CipherId == cipherId) + .Select(cc => cc.CollectionId) + .ToListAsync(); + } + } + + public async Task> GetUserIdsByCollectionIdsAsync(IEnumerable collectionIds) + { + var collectionIdList = collectionIds.ToList(); + using (var scope = ServiceScopeFactory.CreateScope()) + { + var dbContext = GetDatabaseContext(scope); + + var directUserIds = from cu in dbContext.CollectionUsers + where collectionIdList.Contains(cu.CollectionId) + join ou in dbContext.OrganizationUsers on cu.OrganizationUserId equals ou.Id + where ou.Status == Core.Enums.OrganizationUserStatusType.Confirmed && ou.UserId != null + select ou.UserId!.Value; + + var groupUserIds = from cg in dbContext.CollectionGroups + where collectionIdList.Contains(cg.CollectionId) + join gu in dbContext.GroupUsers on cg.GroupId equals gu.GroupId + join ou in dbContext.OrganizationUsers on gu.OrganizationUserId equals ou.Id + where ou.Status == Core.Enums.OrganizationUserStatusType.Confirmed && ou.UserId != null + select ou.UserId!.Value; + + var orgLevelUserIds = from cc in dbContext.CollectionCiphers + where collectionIdList.Contains(cc.CollectionId) + join c in dbContext.Collections on cc.CollectionId equals c.Id + join o in dbContext.Organizations on c.OrganizationId equals o.Id + join ou in dbContext.OrganizationUsers on c.OrganizationId equals ou.OrganizationId + where ou.Status == Core.Enums.OrganizationUserStatusType.Confirmed + && ou.UserId != null + && (ou.Type == OrganizationUserType.Owner + || ou.Type == OrganizationUserType.Admin) + && o.AllowAdminAccessToAllCollectionItems + select ou.UserId!.Value; + + return await directUserIds.Union(groupUserIds).Union(orgLevelUserIds).ToListAsync(); + } + } + public async Task UpdateCollectionsAsync(Guid cipherId, Guid userId, IEnumerable collectionIds) { using (var scope = ServiceScopeFactory.CreateScope()) diff --git a/src/Notifications/HubHelpers.cs b/src/Notifications/HubHelpers.cs index aaa64cb80c67..8ebe5883e097 100644 --- a/src/Notifications/HubHelpers.cs +++ b/src/Notifications/HubHelpers.cs @@ -54,12 +54,8 @@ public async Task SendNotificationToHubAsync(string notificationJson, Cancellati await _hubContext.Clients.User(cipherNotification.Payload.UserId.Value.ToString()) .SendAsync(_receiveMessageMethod, cipherNotification, cancellationToken); } - else if (cipherNotification.Payload.OrganizationId.HasValue) - { - await _hubContext.Clients - .Group(NotificationsHub.GetOrganizationGroup(cipherNotification.Payload.OrganizationId.Value)) - .SendAsync(_receiveMessageMethod, cipherNotification, cancellationToken); - } + // OrganizationId-only cipher notifications are no longer sent via push engines; + // MultiServicePushNotificationService fans out per-user before the message is queued. break; case PushType.SyncFolderUpdate: diff --git a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs index c780c33c9b3e..f909d215d3d5 100644 --- a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs +++ b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs @@ -159,6 +159,7 @@ public static void AddTestPlayIdTracking(this IServiceCollection services, Globa public static void AddBaseServices(this IServiceCollection services, IGlobalSettings globalSettings) { services.AddScoped(); + services.TryAddScoped(); services.AddUserServices(globalSettings); services.AddTrialInitiationServices(); services.AddOrganizationServices(globalSettings); diff --git a/src/Sql/dbo/Stored Procedures/CollectionCipher_ReadCollectionIdsByCipherId.sql b/src/Sql/dbo/Stored Procedures/CollectionCipher_ReadCollectionIdsByCipherId.sql new file mode 100644 index 000000000000..ec3a942535cc --- /dev/null +++ b/src/Sql/dbo/Stored Procedures/CollectionCipher_ReadCollectionIdsByCipherId.sql @@ -0,0 +1,13 @@ +CREATE PROCEDURE [dbo].[CollectionCipher_ReadCollectionIdsByCipherId] + @CipherId UNIQUEIDENTIFIER +AS +BEGIN + SET NOCOUNT ON + + SELECT + [CollectionId] + FROM + [dbo].[CollectionCipher] + WHERE + [CipherId] = @CipherId +END diff --git a/src/Sql/dbo/Stored Procedures/CollectionCipher_ReadUserIdsByCollectionIds.sql b/src/Sql/dbo/Stored Procedures/CollectionCipher_ReadUserIdsByCollectionIds.sql new file mode 100644 index 000000000000..7023caea0aa7 --- /dev/null +++ b/src/Sql/dbo/Stored Procedures/CollectionCipher_ReadUserIdsByCollectionIds.sql @@ -0,0 +1,50 @@ +CREATE PROCEDURE [dbo].[CollectionCipher_ReadUserIdsByCollectionIds] + @CollectionIds AS [dbo].[GuidIdArray] READONLY +AS +BEGIN + SET NOCOUNT ON + + -- Users with direct collection access + SELECT DISTINCT + OU.[UserId] + FROM + [dbo].[CollectionUser] CU + INNER JOIN + [dbo].[OrganizationUser] OU ON OU.[Id] = CU.[OrganizationUserId] + WHERE + CU.[CollectionId] IN (SELECT [Id] FROM @CollectionIds) + AND OU.[Status] = 2 -- Confirmed + + UNION + + -- Users with group-based collection access + SELECT DISTINCT + OU.[UserId] + FROM + [dbo].[CollectionGroup] CG + INNER JOIN + [dbo].[GroupUser] GU ON GU.[GroupId] = CG.[GroupId] + INNER JOIN + [dbo].[OrganizationUser] OU ON OU.[Id] = GU.[OrganizationUserId] + WHERE + CG.[CollectionId] IN (SELECT [Id] FROM @CollectionIds) + AND OU.[Status] = 2 -- Confirmed + + UNION + + -- Users with org-level access (owners/admins with AllowAdminAccessToAllCollectionItems enabled) + SELECT DISTINCT + OU.[UserId] + FROM + [dbo].[OrganizationUser] OU + INNER JOIN + [dbo].[CollectionCipher] CC ON CC.[CollectionId] IN (SELECT [Id] FROM @CollectionIds) + INNER JOIN + [dbo].[Collection] COL ON COL.[Id] = CC.[CollectionId] + INNER JOIN + [dbo].[Organization] O ON O.[Id] = COL.[OrganizationId] + WHERE OU.[OrganizationId] = COL.[OrganizationId] + AND OU.[Status] = 2 -- Confirmed + AND OU.[Type] IN (0, 1) -- Owner/Admin + AND O.[AllowAdminAccessToAllCollectionItems] = 1 +END diff --git a/test/Core.Test/Platform/Push/Engines/AzureQueuePushEngineTests.cs b/test/Core.Test/Platform/Push/Engines/AzureQueuePushEngineTests.cs index 3f31f1fad458..f5466ad0dc92 100644 --- a/test/Core.Test/Platform/Push/Engines/AzureQueuePushEngineTests.cs +++ b/test/Core.Test/Platform/Push/Engines/AzureQueuePushEngineTests.cs @@ -39,18 +39,17 @@ public AzureQueuePushEngineTests() _fakeTimeProvider.SetUtcNow(DateTime.UtcNow); } - [Theory] - [InlineData("6a5bbe1b-cf16-49a6-965f-5c2eac56a531", null)] - [InlineData(null, "b9a3fcb4-2447-45c1-aad2-24de43c88c44")] - public async Task PushSyncCipherCreateAsync_SendsExpectedResponse(string? userId, string? organizationId) + [Fact] + public async Task PushSyncCipherCreateAsync_PersonalCipher_SendsExpectedResponse() { + var userId = Guid.Parse("6a5bbe1b-cf16-49a6-965f-5c2eac56a531"); var collectionId = Guid.NewGuid(); var cipher = new Cipher { Id = Guid.NewGuid(), - UserId = userId != null ? Guid.Parse(userId) : null, - OrganizationId = organizationId != null ? Guid.Parse(organizationId) : null, + UserId = userId, + OrganizationId = null, RevisionDate = DateTime.UtcNow, }; @@ -61,42 +60,43 @@ public async Task PushSyncCipherCreateAsync_SendsExpectedResponse(string? userId { ["Id"] = cipher.Id, ["UserId"] = cipher.UserId, - ["OrganizationId"] = cipher.OrganizationId, ["CollectionIds"] = new JsonArray(collectionId), ["RevisionDate"] = cipher.RevisionDate, }, ["ContextId"] = _deviceIdentifier, }; - if (!cipher.UserId.HasValue) - { - expectedPayload["Payload"]!.AsObject().Remove("UserId"); - } - - if (!cipher.OrganizationId.HasValue) - { - expectedPayload["Payload"]!.AsObject().Remove("OrganizationId"); - expectedPayload["Payload"]!.AsObject().Remove("CollectionIds"); - } - await VerifyNotificationAsync( - async sut => await sut.PushSyncCipherCreateAsync(cipher, [collectionId]), + async sut => await sut.PushAsync(new PushNotification + { + Type = PushType.SyncCipherCreate, + Target = NotificationTarget.User, + TargetId = userId, + Payload = new SyncCipherPushNotification + { + Id = cipher.Id, + UserId = cipher.UserId, + OrganizationId = cipher.OrganizationId, + RevisionDate = cipher.RevisionDate, + CollectionIds = [collectionId], + }, + ExcludeCurrentContext = true, + }), expectedPayload ); } - [Theory] - [InlineData("6a5bbe1b-cf16-49a6-965f-5c2eac56a531", null)] - [InlineData(null, "b9a3fcb4-2447-45c1-aad2-24de43c88c44")] - public async Task PushSyncCipherUpdateAsync_SendsExpectedResponse(string? userId, string? organizationId) + [Fact] + public async Task PushSyncCipherUpdateAsync_PersonalCipher_SendsExpectedResponse() { + var userId = Guid.Parse("6a5bbe1b-cf16-49a6-965f-5c2eac56a531"); var collectionId = Guid.NewGuid(); var cipher = new Cipher { Id = Guid.NewGuid(), - UserId = userId != null ? Guid.Parse(userId) : null, - OrganizationId = organizationId != null ? Guid.Parse(organizationId) : null, + UserId = userId, + OrganizationId = null, RevisionDate = DateTime.UtcNow, }; @@ -107,26 +107,28 @@ public async Task PushSyncCipherUpdateAsync_SendsExpectedResponse(string? userId { ["Id"] = cipher.Id, ["UserId"] = cipher.UserId, - ["OrganizationId"] = cipher.OrganizationId, ["CollectionIds"] = new JsonArray(collectionId), ["RevisionDate"] = cipher.RevisionDate, }, ["ContextId"] = _deviceIdentifier, }; - if (!cipher.UserId.HasValue) - { - expectedPayload["Payload"]!.AsObject().Remove("UserId"); - } - - if (!cipher.OrganizationId.HasValue) - { - expectedPayload["Payload"]!.AsObject().Remove("OrganizationId"); - expectedPayload["Payload"]!.AsObject().Remove("CollectionIds"); - } - await VerifyNotificationAsync( - async sut => await sut.PushSyncCipherUpdateAsync(cipher, [collectionId]), + async sut => await sut.PushAsync(new PushNotification + { + Type = PushType.SyncCipherUpdate, + Target = NotificationTarget.User, + TargetId = userId, + Payload = new SyncCipherPushNotification + { + Id = cipher.Id, + UserId = cipher.UserId, + OrganizationId = cipher.OrganizationId, + RevisionDate = cipher.RevisionDate, + CollectionIds = [collectionId], + }, + ExcludeCurrentContext = true, + }), expectedPayload ); } @@ -134,8 +136,6 @@ await VerifyNotificationAsync( [Fact] public async Task PushSyncCipherDeleteAsync_SendsExpectedResponse() { - var collectionId = Guid.NewGuid(); - var cipher = new Cipher { Id = Guid.NewGuid(), @@ -157,7 +157,21 @@ public async Task PushSyncCipherDeleteAsync_SendsExpectedResponse() }; await VerifyNotificationAsync( - async sut => await sut.PushSyncCipherDeleteAsync(cipher), + async sut => await sut.PushAsync(new PushNotification + { + Type = PushType.SyncLoginDelete, + Target = NotificationTarget.User, + TargetId = cipher.UserId!.Value, + Payload = new SyncCipherPushNotification + { + Id = cipher.Id, + UserId = cipher.UserId, + OrganizationId = cipher.OrganizationId, + RevisionDate = cipher.RevisionDate, + CollectionIds = null, + }, + ExcludeCurrentContext = true, + }), expectedPayload ); } @@ -734,6 +748,40 @@ await VerifyNotificationAsync( // ); // } + private async Task VerifyNoNotificationAsync(Func test) + { + var queueClient = Substitute.For(); + + var httpContextAccessor = Substitute.For(); + + var httpContext = new DefaultHttpContext(); + + var serviceCollection = new ServiceCollection(); + var currentContext = Substitute.For(); + currentContext.DeviceIdentifier = _deviceIdentifier; + serviceCollection.AddSingleton(currentContext); + + httpContext.RequestServices = serviceCollection.BuildServiceProvider(); + + httpContextAccessor.HttpContext + .Returns(httpContext); + + var globalSettings = new Core.Settings.GlobalSettings(); + + var sut = new AzureQueuePushEngine( + queueClient, + httpContextAccessor, + globalSettings, + NullLogger.Instance + ); + + await test(new EngineWrapper(sut, _fakeTimeProvider, _globalSettings.Installation.Id)); + + await queueClient + .Received(0) + .SendMessageAsync(Arg.Any()); + } + private async Task VerifyNotificationAsync(Func test, JsonNode expectedMessage) { var queueClient = Substitute.For(); diff --git a/test/Core.Test/Platform/Push/Engines/PushTestBase.cs b/test/Core.Test/Platform/Push/Engines/PushTestBase.cs index c0037f57aa8b..c126ddb16b63 100644 --- a/test/Core.Test/Platform/Push/Engines/PushTestBase.cs +++ b/test/Core.Test/Platform/Push/Engines/PushTestBase.cs @@ -7,6 +7,7 @@ using Bit.Core.Auth.Entities; using Bit.Core.Context; using Bit.Core.Enums; +using Bit.Core.Models; using Bit.Core.NotificationCenter.Entities; using Bit.Core.NotificationCenter.Enums; using Bit.Core.Platform.Push; @@ -35,9 +36,6 @@ public class EngineWrapper(IPushEngine pushEngine, FakeTimeProvider fakeTimeProv public Task PushAsync(PushNotification pushNotification) where T : class => pushEngine.PushAsync(pushNotification); - - public Task PushCipherAsync(Cipher cipher, PushType pushType, IEnumerable? collectionIds) - => pushEngine.PushCipherAsync(cipher, pushType, collectionIds); } public abstract class PushTestBase @@ -113,7 +111,21 @@ public async Task PushSyncCipherCreateAsync_SendsExpectedResponse() }; await VerifyNotificationAsync( - async sut => await sut.PushSyncCipherCreateAsync(cipher, [collectionId]), + async sut => await sut.PushAsync(new PushNotification + { + Type = PushType.SyncCipherCreate, + Target = NotificationTarget.User, + TargetId = cipher.UserId!.Value, + Payload = new SyncCipherPushNotification + { + Id = cipher.Id, + UserId = cipher.UserId, + OrganizationId = cipher.OrganizationId, + RevisionDate = cipher.RevisionDate, + CollectionIds = [collectionId], + }, + ExcludeCurrentContext = true, + }), GetPushSyncCipherCreatePayload(cipher, collectionId) ); } @@ -132,7 +144,21 @@ public async Task PushSyncCipherUpdateAsync_SendsExpectedResponse() }; await VerifyNotificationAsync( - async sut => await sut.PushSyncCipherUpdateAsync(cipher, [collectionId]), + async sut => await sut.PushAsync(new PushNotification + { + Type = PushType.SyncCipherUpdate, + Target = NotificationTarget.User, + TargetId = cipher.UserId!.Value, + Payload = new SyncCipherPushNotification + { + Id = cipher.Id, + UserId = cipher.UserId, + OrganizationId = cipher.OrganizationId, + RevisionDate = cipher.RevisionDate, + CollectionIds = [collectionId], + }, + ExcludeCurrentContext = true, + }), GetPushSyncCipherUpdatePayload(cipher, collectionId) ); } @@ -140,8 +166,6 @@ await VerifyNotificationAsync( [Fact] public async Task PushSyncCipherDeleteAsync_SendsExpectedResponse() { - var collectionId = Guid.NewGuid(); - var cipher = new Cipher { Id = Guid.NewGuid(), @@ -151,7 +175,21 @@ public async Task PushSyncCipherDeleteAsync_SendsExpectedResponse() }; await VerifyNotificationAsync( - async sut => await sut.PushSyncCipherDeleteAsync(cipher), + async sut => await sut.PushAsync(new PushNotification + { + Type = PushType.SyncLoginDelete, + Target = NotificationTarget.User, + TargetId = cipher.UserId!.Value, + Payload = new SyncCipherPushNotification + { + Id = cipher.Id, + UserId = cipher.UserId, + OrganizationId = cipher.OrganizationId, + RevisionDate = cipher.RevisionDate, + CollectionIds = null, + }, + ExcludeCurrentContext = true, + }), GetPushSyncCipherDeletePayload(cipher) ); } diff --git a/test/Core.Test/Platform/Push/MultiServicePushNotificationServiceTests.cs b/test/Core.Test/Platform/Push/MultiServicePushNotificationServiceTests.cs index f0143bae51aa..aff17c0c4bb7 100644 --- a/test/Core.Test/Platform/Push/MultiServicePushNotificationServiceTests.cs +++ b/test/Core.Test/Platform/Push/MultiServicePushNotificationServiceTests.cs @@ -29,7 +29,7 @@ public MultiServicePushNotificationServiceTests() ); } -#if DEBUG // This test requires debug code in the sut to work properly +#if DEBUG // These tests require debug code in the sut to work properly [Fact] public async Task PushAsync_CallsAllEngines() { diff --git a/test/Core.Test/Platform/Push/NotificationHub/NotificationHubPushEngineTests.cs b/test/Core.Test/Platform/Push/NotificationHub/NotificationHubPushEngineTests.cs index f5f257c741fa..5852b76d41a7 100644 --- a/test/Core.Test/Platform/Push/NotificationHub/NotificationHubPushEngineTests.cs +++ b/test/Core.Test/Platform/Push/NotificationHub/NotificationHubPushEngineTests.cs @@ -56,7 +56,21 @@ public async Task PushSyncCipherCreateAsync_SendExpectedData() }; await VerifyNotificationAsync( - async sut => await sut.PushSyncCipherCreateAsync(cipher, [collectionId]), + async sut => await sut.PushAsync(new PushNotification + { + Type = PushType.SyncCipherCreate, + Target = NotificationTarget.User, + TargetId = userId, + Payload = new SyncCipherPushNotification + { + Id = cipher.Id, + UserId = cipher.UserId, + OrganizationId = cipher.OrganizationId, + RevisionDate = cipher.RevisionDate, + CollectionIds = [collectionId], + }, + ExcludeCurrentContext = true, + }), PushType.SyncCipherCreate, expectedPayload, $"(template:payload_userId:{userId} && !deviceIdentifier:{_deviceIdentifier})" @@ -87,7 +101,21 @@ public async Task PushSyncCipherUpdateAsync_SendExpectedData() }; await VerifyNotificationAsync( - async sut => await sut.PushSyncCipherUpdateAsync(cipher, [collectionId]), + async sut => await sut.PushAsync(new PushNotification + { + Type = PushType.SyncCipherUpdate, + Target = NotificationTarget.User, + TargetId = userId, + Payload = new SyncCipherPushNotification + { + Id = cipher.Id, + UserId = cipher.UserId, + OrganizationId = cipher.OrganizationId, + RevisionDate = cipher.RevisionDate, + CollectionIds = [collectionId], + }, + ExcludeCurrentContext = true, + }), PushType.SyncCipherUpdate, expectedPayload, $"(template:payload_userId:{userId} && !deviceIdentifier:{_deviceIdentifier})" @@ -116,7 +144,21 @@ public async Task PushSyncCipherDeleteAsync_SendExpectedData() }; await VerifyNotificationAsync( - async sut => await sut.PushSyncCipherDeleteAsync(cipher), + async sut => await sut.PushAsync(new PushNotification + { + Type = PushType.SyncLoginDelete, + Target = NotificationTarget.User, + TargetId = userId, + Payload = new SyncCipherPushNotification + { + Id = cipher.Id, + UserId = cipher.UserId, + OrganizationId = cipher.OrganizationId, + RevisionDate = cipher.RevisionDate, + CollectionIds = null, + }, + ExcludeCurrentContext = true, + }), PushType.SyncLoginDelete, expectedPayload, $"(template:payload_userId:{userId} && !deviceIdentifier:{_deviceIdentifier})" diff --git a/test/Core.Test/Vault/Services/CipherServiceTests.cs b/test/Core.Test/Vault/Services/CipherServiceTests.cs index 319f5667f9fc..6efc75b007a4 100644 --- a/test/Core.Test/Vault/Services/CipherServiceTests.cs +++ b/test/Core.Test/Vault/Services/CipherServiceTests.cs @@ -325,7 +325,7 @@ public async Task ShareAsync_HasV0Attachments_ReplaceAttachmentMetadataWithNewOn var cipherRepository = sutProvider.GetDependency(); cipherRepository.ReplaceAsync(cipher, collectionIds).Returns(true); sutProvider.GetDependency().GetByIdAsync(organization.Id).Returns(organization); - var pushNotificationService = sutProvider.GetDependency(); + var cipherSyncPushService = sutProvider.GetDependency(); var v0AttachmentId = Guid.NewGuid().ToString(); var anotherAttachmentId = Guid.NewGuid().ToString(); @@ -383,7 +383,7 @@ await cipherRepository.Received().ReplaceAsync(Arg.Is(c => c.GetAttachments()[v0AttachmentId].FileName == "AFileNameRe-EncryptedWithOrgKey") , collectionIds); - await pushNotificationService.Received(1).PushSyncCipherUpdateAsync(cipher, collectionIds); + await cipherSyncPushService.Received(1).PushSyncCipherUpdateAsync(cipher, collectionIds); } [Theory] @@ -804,7 +804,7 @@ public async Task RestoreAsync_WithAlreadyRestoredCipher_SkipsOperation( await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().UpsertAsync(default); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().LogCipherEventAsync(default, default); - await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().PushSyncCipherUpdateAsync(default, default); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().PushSyncCipherUpdateAsync(default, default); } [Theory] @@ -828,7 +828,7 @@ public async Task RestoreAsync_WithPersonalCipherBelongingToDifferentUser_Throws Assert.Contains("do not have permissions", exception.Message); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().UpsertAsync(default); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().LogCipherEventAsync(default, default); - await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().PushSyncCipherUpdateAsync(default, default); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().PushSyncCipherUpdateAsync(default, default); } [Theory] @@ -845,7 +845,7 @@ public async Task RestoreAsync_WithOrgAdminOverride_RestoresCipher( Assert.NotEqual(DateTime.UtcNow, cipherDetails.RevisionDate); await sutProvider.GetDependency().Received(1).UpsertAsync(cipherDetails); await sutProvider.GetDependency().Received(1).LogCipherEventAsync(cipherDetails, EventType.Cipher_Restored); - await sutProvider.GetDependency().Received(1).PushSyncCipherUpdateAsync(cipherDetails, null); + await sutProvider.GetDependency().Received(1).PushSyncCipherUpdateAsync(cipherDetails, Array.Empty()); } [Theory] @@ -876,7 +876,7 @@ public async Task RestoreAsync_WithManagePermission_RestoresCipher( Assert.NotEqual(DateTime.UtcNow, cipherDetails.RevisionDate); await sutProvider.GetDependency().Received(1).UpsertAsync(cipherDetails); await sutProvider.GetDependency().Received(1).LogCipherEventAsync(cipherDetails, EventType.Cipher_Restored); - await sutProvider.GetDependency().Received(1).PushSyncCipherUpdateAsync(cipherDetails, null); + await sutProvider.GetDependency().Received(1).PushSyncCipherUpdateAsync(cipherDetails, Array.Empty()); } [Theory] @@ -907,7 +907,7 @@ public async Task RestoreAsync_WithoutManagePermission_ThrowsBadRequestException Assert.Contains("do not have permissions", exception.Message); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().UpsertAsync(default); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().LogCipherEventAsync(default, default); - await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().PushSyncCipherUpdateAsync(default, default); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().PushSyncCipherUpdateAsync(default, default); } @@ -1314,7 +1314,8 @@ public async Task DeleteAsync_WithPersonalCipherOwner_DeletesCipher( await sutProvider.GetDependency().Received(1).DeleteAsync(cipherDetails); await sutProvider.GetDependency().Received(1).DeleteAttachmentsForCipherAsync(cipherDetails.Id); await sutProvider.GetDependency().Received(1).LogCipherEventAsync(cipherDetails, EventType.Cipher_Deleted); - await sutProvider.GetDependency().Received(1).PushSyncCipherDeleteAsync(cipherDetails); + await sutProvider.GetDependency().Received(1) + .PushSyncCipherDeleteAsync(cipherDetails, null); } @@ -1341,9 +1342,11 @@ public async Task DeleteAsync_WithPersonalCipherBelongingToDifferentUser_ThrowsB await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().DeleteAsync(default); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().DeleteAttachmentsForCipherAsync(default); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().LogCipherEventAsync(default, default); - await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().PushSyncCipherDeleteAsync(default); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .PushSyncCipherDeleteAsync(default, default); } + [Theory] [OrganizationCipherCustomize] [BitAutoData] @@ -1355,7 +1358,8 @@ public async Task DeleteAsync_WithOrgAdminOverride_DeletesCipher( await sutProvider.GetDependency().Received(1).DeleteAsync(cipherDetails); await sutProvider.GetDependency().Received(1).DeleteAttachmentsForCipherAsync(cipherDetails.Id); await sutProvider.GetDependency().Received(1).LogCipherEventAsync(cipherDetails, EventType.Cipher_Deleted); - await sutProvider.GetDependency().Received(1).PushSyncCipherDeleteAsync(cipherDetails); + await sutProvider.GetDependency().Received(1) + .PushSyncCipherDeleteAsync(cipherDetails, Arg.Any>()); } [Theory] @@ -1384,7 +1388,8 @@ public async Task DeleteAsync_WithManagePermission_DeletesCipher( await sutProvider.GetDependency().Received(1).DeleteAsync(cipherDetails); await sutProvider.GetDependency().Received(1).DeleteAttachmentsForCipherAsync(cipherDetails.Id); await sutProvider.GetDependency().Received(1).LogCipherEventAsync(cipherDetails, EventType.Cipher_Deleted); - await sutProvider.GetDependency().Received(1).PushSyncCipherDeleteAsync(cipherDetails); + await sutProvider.GetDependency().Received(1) + .PushSyncCipherDeleteAsync(cipherDetails, Arg.Any>()); } [Theory] @@ -1415,7 +1420,8 @@ public async Task DeleteAsync_WithoutManagePermission_ThrowsBadRequestException( await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().DeleteAsync(default); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().DeleteAttachmentsForCipherAsync(default); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().LogCipherEventAsync(default, default); - await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().PushSyncCipherDeleteAsync(default); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .PushSyncCipherDeleteAsync(default, default); } [Theory] @@ -1734,7 +1740,7 @@ public async Task SoftDeleteAsync_WithPersonalCipherOwner_SoftDeletesCipher( Assert.Equal(cipherDetails.RevisionDate, cipherDetails.DeletedDate); await sutProvider.GetDependency().Received(1).UpsertAsync(cipherDetails); await sutProvider.GetDependency().Received(1).LogCipherEventAsync(cipherDetails, EventType.Cipher_SoftDeleted); - await sutProvider.GetDependency().Received(1).PushSyncCipherUpdateAsync(cipherDetails, null); + await sutProvider.GetDependency().Received(1).PushSyncCipherUpdateAsync(cipherDetails, Array.Empty()); } @@ -1781,7 +1787,7 @@ public async Task SoftDeleteAsync_WithAlreadySoftDeletedCipher_SkipsOperation( await sutProvider.GetDependency().DidNotReceive().UpsertAsync(Arg.Any()); await sutProvider.GetDependency().DidNotReceive().LogCipherEventAsync(Arg.Any(), Arg.Any()); - await sutProvider.GetDependency().DidNotReceive().PushSyncCipherUpdateAsync(Arg.Any(), Arg.Any>()); + await sutProvider.GetDependency().DidNotReceive().PushSyncCipherUpdateAsync(Arg.Any(), Arg.Any>()); } [Theory] @@ -1796,7 +1802,7 @@ public async Task SoftDeleteAsync_WithOrgAdminOverride_SoftDeletesCipher( await sutProvider.GetDependency().Received(1).UpsertAsync(cipherDetails); await sutProvider.GetDependency().Received(1).LogCipherEventAsync(cipherDetails, EventType.Cipher_SoftDeleted); - await sutProvider.GetDependency().Received(1).PushSyncCipherUpdateAsync(cipherDetails, null); + await sutProvider.GetDependency().Received(1).PushSyncCipherUpdateAsync(cipherDetails, Array.Empty()); } [Theory] @@ -1827,7 +1833,7 @@ public async Task SoftDeleteAsync_WithManagePermission_SoftDeletesCipher( Assert.Equal(cipherDetails.RevisionDate, cipherDetails.DeletedDate); await sutProvider.GetDependency().Received(1).UpsertAsync(cipherDetails); await sutProvider.GetDependency().Received(1).LogCipherEventAsync(cipherDetails, EventType.Cipher_SoftDeleted); - await sutProvider.GetDependency().Received(1).PushSyncCipherUpdateAsync(cipherDetails, null); + await sutProvider.GetDependency().Received(1).PushSyncCipherUpdateAsync(cipherDetails, Array.Empty()); } [Theory] @@ -1858,7 +1864,7 @@ public async Task SoftDeleteAsync_WithoutManagePermission_ThrowsBadRequestExcept Assert.Contains("do not have permissions", exception.Message); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().UpsertAsync(default); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().LogCipherEventAsync(default, default); - await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().PushSyncCipherUpdateAsync(default, default); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().PushSyncCipherUpdateAsync(default, default); } [Theory] diff --git a/test/Core.Test/Vault/Services/CipherSyncPushServiceTests.cs b/test/Core.Test/Vault/Services/CipherSyncPushServiceTests.cs new file mode 100644 index 000000000000..dbf50e1bfb5b --- /dev/null +++ b/test/Core.Test/Vault/Services/CipherSyncPushServiceTests.cs @@ -0,0 +1,277 @@ +using Bit.Core.Enums; +using Bit.Core.Models; +using Bit.Core.Platform.Push; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Bit.Core.Vault.Entities; +using Bit.Core.Vault.Services; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using NSubstitute; +using Xunit; + +namespace Bit.Core.Test.Vault.Services; + +[SutProviderCustomize] +public class CipherSyncPushServiceTests +{ + [Theory, BitAutoData] + public async Task PushSyncCipherCreateAsync_PersonalCipher_PushesCorrectNotification( + SutProvider sutProvider, Cipher cipher, List collectionIds) + { + cipher.OrganizationId = null; + cipher.UserId = Guid.NewGuid(); + + await sutProvider.Sut.PushSyncCipherCreateAsync(cipher, collectionIds); + + await sutProvider.GetDependency().Received(1) + .PushAsync(Arg.Is>(n => + n.Type == PushType.SyncCipherCreate && + n.Target == NotificationTarget.User && + n.TargetId == cipher.UserId.Value && + n.Payload.Id == cipher.Id && + n.Payload.UserId == cipher.UserId && + n.ExcludeCurrentContext)); + } + + [Theory, BitAutoData] + public async Task PushSyncCipherUpdateAsync_PersonalCipher_PushesCorrectNotification( + SutProvider sutProvider, Cipher cipher) + { + cipher.OrganizationId = null; + cipher.UserId = Guid.NewGuid(); + + await sutProvider.Sut.PushSyncCipherUpdateAsync(cipher, []); + + await sutProvider.GetDependency().Received(1) + .PushAsync(Arg.Is>(n => + n.Type == PushType.SyncCipherUpdate && + n.Target == NotificationTarget.User && + n.TargetId == cipher.UserId.Value && + n.Payload.Id == cipher.Id)); + } + + [Theory, BitAutoData] + public async Task PushSyncCipherDeleteAsync_PersonalCipher_PushesCorrectNotification( + SutProvider sutProvider, Cipher cipher) + { + cipher.OrganizationId = null; + cipher.UserId = Guid.NewGuid(); + + await sutProvider.Sut.PushSyncCipherDeleteAsync(cipher); + + await sutProvider.GetDependency().Received(1) + .PushAsync(Arg.Is>(n => + n.Type == PushType.SyncLoginDelete && + n.Target == NotificationTarget.User && + n.TargetId == cipher.UserId.Value && + n.Payload.Id == cipher.Id)); + } + + [Theory, BitAutoData] + public async Task PushSyncCipherCreateAsync_PersonalCipher_NoUserId_NoPush( + SutProvider sutProvider, Cipher cipher) + { + cipher.OrganizationId = null; + cipher.UserId = null; + + await sutProvider.Sut.PushSyncCipherCreateAsync(cipher, []); + + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .PushAsync(Arg.Any>()); + } + + [Theory, BitAutoData] + public async Task PushSyncCipherCreateAsync_OrgCipher_FlagOff_SendsNonMobileOrgBroadcast( + SutProvider sutProvider, Cipher cipher, Guid collectionId) + { + cipher.OrganizationId = Guid.NewGuid(); + cipher.UserId = null; + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.OrgCipherPushFanout) + .Returns(false); + + await sutProvider.Sut.PushSyncCipherCreateAsync(cipher, [collectionId]); + + await sutProvider.GetDependency() + .Received(1) + .PushAsync(Arg.Is>(n => + n.Type == PushType.SyncCipherCreate && + n.Target == NotificationTarget.Organization && + n.TargetId == cipher.OrganizationId!.Value && + n.NonMobileOnly == true && + n.Payload.Id == cipher.Id && + n.Payload.OrganizationId == cipher.OrganizationId)); + } + + [Theory, BitAutoData] + public async Task PushSyncCipherCreateAsync_OrgCipher_FlagOn_FansOutPerUser( + SutProvider sutProvider, Cipher cipher, Guid collectionId, Guid userId1, Guid userId2) + { + cipher.OrganizationId = Guid.NewGuid(); + cipher.UserId = null; + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.OrgCipherPushFanout) + .Returns(true); + + sutProvider.GetDependency() + .GetUserIdsByCollectionIdsAsync(Arg.Is>(ids => ids.Contains(collectionId))) + .Returns([userId1, userId2]); + + await sutProvider.Sut.PushSyncCipherCreateAsync(cipher, [collectionId]); + + await sutProvider.GetDependency() + .Received(1) + .PushAsync(Arg.Is>(n => + n.Type == PushType.SyncCipherCreate && + n.Target == NotificationTarget.User && + n.TargetId == userId1 && + n.Payload.UserId == userId1 && + n.Payload.OrganizationId == cipher.OrganizationId && + n.ExcludeCurrentContext)); + + await sutProvider.GetDependency() + .Received(1) + .PushAsync(Arg.Is>(n => + n.Type == PushType.SyncCipherCreate && + n.Target == NotificationTarget.User && + n.TargetId == userId2 && + n.Payload.UserId == userId2 && + n.Payload.OrganizationId == cipher.OrganizationId)); + } + + [Theory, BitAutoData] + public async Task PushSyncCipherDeleteAsync_OrgCipher_FlagOn_NoCollectionIds_ResolvesFromRepo( + SutProvider sutProvider, Cipher cipher, Guid collectionId, Guid userId) + { + cipher.OrganizationId = Guid.NewGuid(); + cipher.UserId = null; + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.OrgCipherPushFanout) + .Returns(true); + + sutProvider.GetDependency() + .GetCollectionIdsByCipherIdAsync(cipher.Id) + .Returns([collectionId]); + + sutProvider.GetDependency() + .GetUserIdsByCollectionIdsAsync(Arg.Is>(ids => ids.Contains(collectionId))) + .Returns([userId]); + + await sutProvider.Sut.PushSyncCipherDeleteAsync(cipher); + + await sutProvider.GetDependency() + .Received(1) + .GetCollectionIdsByCipherIdAsync(cipher.Id); + + await sutProvider.GetDependency() + .Received(1) + .PushAsync(Arg.Is>(n => + n.Type == PushType.SyncLoginDelete && + n.Target == NotificationTarget.User && + n.TargetId == userId && + n.Payload.OrganizationId == cipher.OrganizationId && + n.Payload.CollectionIds != null && + n.Payload.CollectionIds.Contains(collectionId))); + } + + [Theory, BitAutoData] + public async Task PushSyncCipherCreateAsync_OrgCipher_FlagOn_NoCollectionIdsFound_LogsWarning_NoPush( + SutProvider sutProvider, Cipher cipher) + { + cipher.OrganizationId = Guid.NewGuid(); + cipher.UserId = null; + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.OrgCipherPushFanout) + .Returns(true); + + sutProvider.GetDependency() + .GetCollectionIdsByCipherIdAsync(cipher.Id) + .Returns([]); + + await sutProvider.Sut.PushSyncCipherCreateAsync(cipher, []); + + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .PushAsync(Arg.Any>()); + } + + [Theory, BitAutoData] + public async Task PushSyncCipherUpdateAsync_OrgCipher_FlagOn_FansOutPerUser( + SutProvider sutProvider, Cipher cipher, Guid collectionId, Guid userId1, Guid userId2) + { + cipher.OrganizationId = Guid.NewGuid(); + cipher.UserId = null; + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.OrgCipherPushFanout) + .Returns(true); + + sutProvider.GetDependency() + .GetUserIdsByCollectionIdsAsync(Arg.Is>(ids => ids.Contains(collectionId))) + .Returns([userId1, userId2]); + + await sutProvider.Sut.PushSyncCipherUpdateAsync(cipher, [collectionId]); + + await sutProvider.GetDependency() + .Received(1) + .PushAsync(Arg.Is>(n => + n.Type == PushType.SyncCipherUpdate && + n.Target == NotificationTarget.User && + n.TargetId == userId1 && + n.Payload.UserId == userId1 && + n.Payload.OrganizationId == cipher.OrganizationId && + n.ExcludeCurrentContext)); + + await sutProvider.GetDependency() + .Received(1) + .PushAsync(Arg.Is>(n => + n.Type == PushType.SyncCipherUpdate && + n.Target == NotificationTarget.User && + n.TargetId == userId2 && + n.Payload.UserId == userId2 && + n.Payload.OrganizationId == cipher.OrganizationId)); + } + + [Theory, BitAutoData] + public async Task PushSyncCipherUpdateAsync_OrgCipher_FlagOn_EmptyCollectionIds_FallsBackToRepo( + SutProvider sutProvider, Cipher cipher, Guid collectionId, Guid userId) + { + cipher.OrganizationId = Guid.NewGuid(); + cipher.UserId = null; + + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.OrgCipherPushFanout) + .Returns(true); + + sutProvider.GetDependency() + .GetCollectionIdsByCipherIdAsync(cipher.Id) + .Returns([collectionId]); + + sutProvider.GetDependency() + .GetUserIdsByCollectionIdsAsync(Arg.Is>(ids => ids.Contains(collectionId))) + .Returns([userId]); + + // Simulates SoftDeleteAsync / RestoreAsync which pass Array.Empty() + await sutProvider.Sut.PushSyncCipherUpdateAsync(cipher, Array.Empty()); + + await sutProvider.GetDependency() + .Received(1) + .GetCollectionIdsByCipherIdAsync(cipher.Id); + + await sutProvider.GetDependency() + .Received(1) + .PushAsync(Arg.Is>(n => + n.Type == PushType.SyncCipherUpdate && + n.Target == NotificationTarget.User && + n.TargetId == userId && + n.Payload.OrganizationId == cipher.OrganizationId && + n.Payload.CollectionIds != null && + n.Payload.CollectionIds.Contains(collectionId))); + } +} diff --git a/test/Infrastructure.IntegrationTest/Vault/Repositories/CollectionCipherRepositoryTests.cs b/test/Infrastructure.IntegrationTest/Vault/Repositories/CollectionCipherRepositoryTests.cs index c1bcde6f02bc..9caa42a1a580 100644 --- a/test/Infrastructure.IntegrationTest/Vault/Repositories/CollectionCipherRepositoryTests.cs +++ b/test/Infrastructure.IntegrationTest/Vault/Repositories/CollectionCipherRepositoryTests.cs @@ -74,4 +74,98 @@ await collectionCipherRepository.AddCollectionsForManyCiphersAsync( Assert.Equal(sharedCollection.Id, result.First().CollectionId); Assert.DoesNotContain(result, cc => cc.CollectionId == defaultUserCollection.Id); } + + [Theory, DatabaseData] + public async Task GetUserIdsByCollectionIdsAsync_IncludesOrganizationLevelUsers( + IUserRepository userRepository, + IOrganizationRepository organizationRepository, + IOrganizationUserRepository organizationUserRepository, + ICollectionRepository collectionRepository, + ICipherRepository cipherRepository, + ICollectionCipherRepository collectionCipherRepository) + { + // Arrange + var ownerUser = await userRepository.CreateAsync(new User + { + Name = "Owner User", + Email = $"owner+{Guid.NewGuid()}@email.com", + ApiKey = "TEST", + SecurityStamp = "stamp", + }); + + var adminUser = await userRepository.CreateAsync(new User + { + Name = "Admin User", + Email = $"admin+{Guid.NewGuid()}@email.com", + ApiKey = "TEST", + SecurityStamp = "stamp", + }); + + var regularUser = await userRepository.CreateAsync(new User + { + Name = "Regular User", + Email = $"regular+{Guid.NewGuid()}@email.com", + ApiKey = "TEST", + SecurityStamp = "stamp", + }); + + var organization = await organizationRepository.CreateAsync(new Organization + { + Name = "Test Org", + PlanType = PlanType.EnterpriseAnnually, + Plan = "Enterprise", + BillingEmail = "billing@example.com", + AllowAdminAccessToAllCollectionItems = true, + }); + + _ = await organizationUserRepository.CreateAsync(new OrganizationUser + { + UserId = ownerUser.Id, + OrganizationId = organization.Id, + Status = OrganizationUserStatusType.Confirmed, + Type = OrganizationUserType.Owner, + }); + + _ = await organizationUserRepository.CreateAsync(new OrganizationUser + { + UserId = adminUser.Id, + OrganizationId = organization.Id, + Status = OrganizationUserStatusType.Confirmed, + Type = OrganizationUserType.Admin, + }); + + _ = await organizationUserRepository.CreateAsync(new OrganizationUser + { + UserId = regularUser.Id, + OrganizationId = organization.Id, + Status = OrganizationUserStatusType.Confirmed, + Type = OrganizationUserType.User, + }); + + var collection = await collectionRepository.CreateAsync(new Collection + { + Name = "Test Collection", + OrganizationId = organization.Id, + }); + + var cipher = await cipherRepository.CreateAsync(new Cipher + { + Type = CipherType.Login, + OrganizationId = organization.Id, + Data = "", + }); + + await collectionCipherRepository.AddCollectionsForManyCiphersAsync( + organization.Id, + new[] { cipher.Id }, + new[] { collection.Id }); + + // Act + var result = await collectionCipherRepository.GetUserIdsByCollectionIdsAsync(new[] { collection.Id }); + + // Assert + Assert.Contains(ownerUser.Id, result); + Assert.Contains(adminUser.Id, result); + Assert.DoesNotContain(regularUser.Id, result); + } } diff --git a/util/Migrator/DbScripts/2026-04-29_00_CollectionCipher_ReadCollectionIdsByCipherId.sql b/util/Migrator/DbScripts/2026-04-29_00_CollectionCipher_ReadCollectionIdsByCipherId.sql new file mode 100644 index 000000000000..b59db752b3d8 --- /dev/null +++ b/util/Migrator/DbScripts/2026-04-29_00_CollectionCipher_ReadCollectionIdsByCipherId.sql @@ -0,0 +1,13 @@ +CREATE OR ALTER PROCEDURE [dbo].[CollectionCipher_ReadCollectionIdsByCipherId] + @CipherId UNIQUEIDENTIFIER +AS +BEGIN + SET NOCOUNT ON + + SELECT + [CollectionId] + FROM + [dbo].[CollectionCipher] + WHERE + [CipherId] = @CipherId +END diff --git a/util/Migrator/DbScripts/2026-04-29_00_CollectionCipher_ReadUserIdsByCollectionIds.sql b/util/Migrator/DbScripts/2026-04-29_00_CollectionCipher_ReadUserIdsByCollectionIds.sql new file mode 100644 index 000000000000..448d004ec7c4 --- /dev/null +++ b/util/Migrator/DbScripts/2026-04-29_00_CollectionCipher_ReadUserIdsByCollectionIds.sql @@ -0,0 +1,50 @@ +CREATE OR ALTER PROCEDURE [dbo].[CollectionCipher_ReadUserIdsByCollectionIds] + @CollectionIds AS [dbo].[GuidIdArray] READONLY +AS +BEGIN + SET NOCOUNT ON + + -- Users with direct collection access + SELECT DISTINCT + OU.[UserId] + FROM + [dbo].[CollectionUser] CU + INNER JOIN + [dbo].[OrganizationUser] OU ON OU.[Id] = CU.[OrganizationUserId] + WHERE + CU.[CollectionId] IN (SELECT [Id] FROM @CollectionIds) + AND OU.[Status] = 2 -- Confirmed + + UNION + + -- Users with group-based collection access + SELECT DISTINCT + OU.[UserId] + FROM + [dbo].[CollectionGroup] CG + INNER JOIN + [dbo].[GroupUser] GU ON GU.[GroupId] = CG.[GroupId] + INNER JOIN + [dbo].[OrganizationUser] OU ON OU.[Id] = GU.[OrganizationUserId] + WHERE + CG.[CollectionId] IN (SELECT [Id] FROM @CollectionIds) + AND OU.[Status] = 2 -- Confirmed + + UNION + + -- Users with org-level access (owners/admins with AllowAdminAccessToAllCollectionItems enabled) + SELECT DISTINCT + OU.[UserId] + FROM + [dbo].[OrganizationUser] OU + INNER JOIN + [dbo].[CollectionCipher] CC ON CC.[CollectionId] IN (SELECT [Id] FROM @CollectionIds) + INNER JOIN + [dbo].[Collection] COL ON COL.[Id] = CC.[CollectionId] + INNER JOIN + [dbo].[Organization] O ON O.[Id] = COL.[OrganizationId] + WHERE OU.[OrganizationId] = COL.[OrganizationId] + AND OU.[Status] = 2 -- Confirmed + AND OU.[Type] IN (0, 1) -- Owner/Admin + AND O.[AllowAdminAccessToAllCollectionItems] = 1 +END