From 787996bc8a88ac7f08121842d6dfbb8a0ca81137 Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Mon, 1 Jul 2024 19:15:54 -0700 Subject: [PATCH 01/31] Allow for binning of comb IDs by date and value --- src/Core/Utilities/CoreHelpers.cs | 33 ++++++++++++++++++++ test/Core.Test/Utilities/CoreHelpersTests.cs | 25 +++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/src/Core/Utilities/CoreHelpers.cs b/src/Core/Utilities/CoreHelpers.cs index c263ccdbe1e2..abb3aa061273 100644 --- a/src/Core/Utilities/CoreHelpers.cs +++ b/src/Core/Utilities/CoreHelpers.cs @@ -76,6 +76,39 @@ internal static Guid GenerateComb(Guid startingGuid, DateTime time) return new Guid(guidArray); } + internal static DateTime DateFromComb(Guid combGuid) + { + var guidArray = combGuid.ToByteArray(); + var daysArray = new byte[4]; + var msecsArray = new byte[4]; + + Array.Copy(guidArray, guidArray.Length - 6, daysArray, 2, 2); + Array.Copy(guidArray, guidArray.Length - 4, msecsArray, 0, 4); + + Array.Reverse(daysArray); + Array.Reverse(msecsArray); + + var days = BitConverter.ToInt32(daysArray, 0); + var msecs = BitConverter.ToInt32(msecsArray, 0); + + var time = new TimeSpan(days, 0, 0, 0, (int)(msecs * 3.333333)); + return new DateTime(_baseDateTicks + time.Ticks, DateTimeKind.Utc); + } + + internal static double BinForComb(Guid combGuid, int binCount) + { + // From System.Web.Util.HashCodeCombiner + uint CombineHashCodes(uint h1, byte h2) + { + return (uint)(((h1 << 5) + h1) ^ h2); + } + var guidArray = combGuid.ToByteArray(); + var randomArray = new byte[10]; + Array.Copy(guidArray, 0, randomArray, 0, 10); + var hash = randomArray.Aggregate((uint)randomArray.Length, CombineHashCodes); + return hash % binCount; + } + public static string CleanCertificateThumbprint(string thumbprint) { // Clean possible garbage characters from thumbprint copy/paste diff --git a/test/Core.Test/Utilities/CoreHelpersTests.cs b/test/Core.Test/Utilities/CoreHelpersTests.cs index af115679893a..ac02234e3453 100644 --- a/test/Core.Test/Utilities/CoreHelpersTests.cs +++ b/test/Core.Test/Utilities/CoreHelpersTests.cs @@ -71,6 +71,31 @@ public void GenerateComb_WithInputs_Success(Guid inputGuid, DateTime inputTime, Assert.Equal(expectedComb, comb); } + [Theory] + [MemberData(nameof(GenerateCombCases))] + public void DateFromComb_WithComb_Success(Guid inputGuid, DateTime inputTime) + { + var comb = CoreHelpers.GenerateComb(inputGuid, inputTime); + var inverseComb = CoreHelpers.DateFromComb(comb); + + Assert.Equal(inputTime, inverseComb, TimeSpan.FromMilliseconds(4)); + } + + [Theory] + [InlineData("00000000-0000-0000-0000-000000000000", 1, 0)] + [InlineData("00000000-0000-0000-0000-000000000001", 1, 0)] + [InlineData("00000000-0000-0000-0000-000000000000", 500, 430)] + [InlineData("00000000-0000-0000-0000-000000000001", 500, 430)] + [InlineData("10000000-0000-0000-0000-000000000001", 500, 454)] + [InlineData("00000000-0000-0100-0000-000000000001", 500, 19)] + public void BinForComb_Success(string guidString, int nbins, int expectedBin) + { + var guid = Guid.Parse(guidString); + var bin = CoreHelpers.BinForComb(guid, nbins); + + Assert.Equal(expectedBin, bin); + } + /* [Fact] public void ToGuidIdArrayTVP_Success() From aa8372052e433b6bb0583cb542c7d30fb6726d58 Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Tue, 2 Jul 2024 10:29:30 -0700 Subject: [PATCH 02/31] Introduce notification hub pool --- .../INotificationHubClientProxy.cs | 11 + .../NotificationHub/INotificationHubPool.cs | 9 + .../NotificationHubClientProxy.cs | 27 +++ .../NotificationHubConnection.cs | 120 ++++++++++ .../NotificationHub/NotificationHubPool.cs | 58 +++++ .../NotificationHubPushNotificationService.cs | 3 +- .../NotificationHubPushRegistrationService.cs | 6 +- .../MultiServicePushNotificationService.cs | 1 + src/Core/Settings/GlobalSettings.cs | 32 ++- src/Core/Utilities/CoreHelpers.cs | 2 +- .../Utilities/ServiceCollectionExtensions.cs | 1 + .../NotificationHubConnectionTests.cs | 205 ++++++++++++++++++ .../NotificationHubPoolTests.cs | 156 +++++++++++++ .../NotificationHubProxyTests.cs | 52 +++++ ...ficationHubPushNotificationServiceTests.cs | 5 +- ...ficationHubPushRegistrationServiceTests.cs | 9 +- test/Core.Test/Utilities/CoreHelpersTests.cs | 37 ++-- 17 files changed, 703 insertions(+), 31 deletions(-) create mode 100644 src/Core/NotificationHub/INotificationHubClientProxy.cs create mode 100644 src/Core/NotificationHub/INotificationHubPool.cs create mode 100644 src/Core/NotificationHub/NotificationHubClientProxy.cs create mode 100644 src/Core/NotificationHub/NotificationHubConnection.cs create mode 100644 src/Core/NotificationHub/NotificationHubPool.cs rename src/Core/{Services/Implementations => NotificationHub}/NotificationHubPushNotificationService.cs (99%) rename src/Core/{Services/Implementations => NotificationHub}/NotificationHubPushRegistrationService.cs (97%) create mode 100644 test/Core.Test/NotificationHub/NotificationHubConnectionTests.cs create mode 100644 test/Core.Test/NotificationHub/NotificationHubPoolTests.cs create mode 100644 test/Core.Test/NotificationHub/NotificationHubProxyTests.cs rename test/Core.Test/{Services => NotificationHub}/NotificationHubPushNotificationServiceTests.cs (92%) rename test/Core.Test/{Services => NotificationHub}/NotificationHubPushRegistrationServiceTests.cs (82%) diff --git a/src/Core/NotificationHub/INotificationHubClientProxy.cs b/src/Core/NotificationHub/INotificationHubClientProxy.cs new file mode 100644 index 000000000000..5e8dbb2c29e2 --- /dev/null +++ b/src/Core/NotificationHub/INotificationHubClientProxy.cs @@ -0,0 +1,11 @@ +using Microsoft.Azure.NotificationHubs; + +namespace Bit.Core.NotificationHub; + +public interface INotificationHubProxy +{ + Task DeleteInstallationAsync(string installationId); + Task DeleteInstallationAsync(string installationId, CancellationToken cancellationToken); + Task PatchInstallationAsync(string installationId, IList operations); + Task PatchInstallationAsync(string installationId, IList operations, CancellationToken cancellationToken); +} diff --git a/src/Core/NotificationHub/INotificationHubPool.cs b/src/Core/NotificationHub/INotificationHubPool.cs new file mode 100644 index 000000000000..8ae33297b5dd --- /dev/null +++ b/src/Core/NotificationHub/INotificationHubPool.cs @@ -0,0 +1,9 @@ +using Microsoft.Azure.NotificationHubs; + +namespace Bit.Core.NotificationHub; + +public interface INotificationHubPool +{ + NotificationHubClient ClientFor(Guid comb); + +} diff --git a/src/Core/NotificationHub/NotificationHubClientProxy.cs b/src/Core/NotificationHub/NotificationHubClientProxy.cs new file mode 100644 index 000000000000..99e19d61cff5 --- /dev/null +++ b/src/Core/NotificationHub/NotificationHubClientProxy.cs @@ -0,0 +1,27 @@ +using Microsoft.Azure.NotificationHubs; + +namespace Bit.Core.NotificationHub; + +public class NotificationHubClientProxy : INotificationHubProxy +{ + private readonly IEnumerable _clients; + + public NotificationHubClientProxy(IEnumerable clients) + { + _clients = clients; + } + + private async Task ApplyToAllClientsAsync(Func action) + { + var tasks = _clients.Select(async c => await action(c)); + await Task.WhenAll(tasks); + } + + // partial INotificationHubClient implementation + // Note: Any other methods that are needed can simply be delegated as done here. + public async Task DeleteInstallationAsync(string installationId) => await ApplyToAllClientsAsync((c) => c.DeleteInstallationAsync(installationId)); + public async Task DeleteInstallationAsync(string installationId, CancellationToken cancellationToken) => await ApplyToAllClientsAsync(c => c.DeleteInstallationAsync(installationId, cancellationToken)); + public async Task PatchInstallationAsync(string installationId, IList operations) => await ApplyToAllClientsAsync(c => c.PatchInstallationAsync(installationId, operations)); + public async Task PatchInstallationAsync(string installationId, IList operations, CancellationToken cancellationToken) => await ApplyToAllClientsAsync(c => c.PatchInstallationAsync(installationId, operations, cancellationToken)); + +} diff --git a/src/Core/NotificationHub/NotificationHubConnection.cs b/src/Core/NotificationHub/NotificationHubConnection.cs new file mode 100644 index 000000000000..eed38e8b27db --- /dev/null +++ b/src/Core/NotificationHub/NotificationHubConnection.cs @@ -0,0 +1,120 @@ +using Bit.Core.Settings; +using Bit.Core.Utilities; +using Microsoft.Azure.NotificationHubs; + +class NotificationHubConnection +{ + public string HubName { get; init; } + public string ConnectionString { get; init; } + public bool EnableSendTracing { get; init; } + private NotificationHubClient _hubClient; + /// + /// Gets the NotificationHubClient for this connection. + /// + /// If the client is null, it will be initialized. + /// + /// Exception if the connection is invalid. + /// + public NotificationHubClient HubClient + { + get + { + if (_hubClient == null) + { + if (!IsValid) + { + throw new Exception("Invalid notification hub settings"); + } + Init(); + } + return _hubClient; + } + private set + { + _hubClient = value; + } + } + /// + /// Gets the start date for registration. + /// + /// If null, registration is always disabled. + /// + public DateTime? RegistrationStartDate { get; init; } + /// + /// Gets the end date for registration. + /// + /// If null, registration has no end date. + /// + public DateTime? RegistrationEndDate { get; init; } + /// + /// Gets whether all data needed to generate a connection to Notification Hub is present. + /// + public bool IsValid + { + get + { + { + var invalid = string.IsNullOrWhiteSpace(HubName) || string.IsNullOrWhiteSpace(ConnectionString); + return !invalid; + } + } + } + + /// + /// Gets whether registration is enabled for the given comb ID. + /// This is based off of the generation time encoded in the comb ID. + /// + /// + /// + public bool RegistrationEnabled(Guid comb) + { + var combTime = CoreHelpers.DateFromComb(comb); + return RegistrationEnabled(combTime); + } + + /// + /// Gets whether registration is enabled for the given time. + /// + /// The time to check + /// + public bool RegistrationEnabled(DateTime queryTime) + { + if (queryTime >= RegistrationEndDate || RegistrationStartDate == null) + { + return false; + } + + return RegistrationStartDate <= queryTime; + } + + private NotificationHubConnection() { } + + /// + /// Creates a new NotificationHubConnection from the given settings. + /// + /// + /// + public static NotificationHubConnection From(GlobalSettings.NotificationHubSettings settings) + { + return new() + { + HubName = settings.HubName, + ConnectionString = settings.ConnectionString, + EnableSendTracing = settings.EnableSendTracing, + // Comb time is not precise enough for millisecond accuracy + RegistrationStartDate = settings.RegistrationStartDate.HasValue ? Truncate(settings.RegistrationStartDate.Value, TimeSpan.FromMilliseconds(10)) : null, + RegistrationEndDate = settings.RegistrationEndDate + }; + } + + private NotificationHubConnection Init() + { + HubClient = NotificationHubClient.CreateClientFromConnectionString(ConnectionString, HubName, EnableSendTracing); + return this; + } + + private static DateTime Truncate(DateTime dateTime, TimeSpan resolution) + { + return dateTime.AddTicks(-(dateTime.Ticks % resolution.Ticks)); + } +} diff --git a/src/Core/NotificationHub/NotificationHubPool.cs b/src/Core/NotificationHub/NotificationHubPool.cs new file mode 100644 index 000000000000..32b8188227f8 --- /dev/null +++ b/src/Core/NotificationHub/NotificationHubPool.cs @@ -0,0 +1,58 @@ +using Bit.Core.Settings; +using Bit.Core.Utilities; +using Microsoft.Azure.NotificationHubs; +using Microsoft.Extensions.Logging; + +namespace Bit.Core.NotificationHub; + +public class NotificationHubPool : INotificationHubPool +{ + private List _connections { get; } + private readonly IEnumerable _clients; + private readonly ILogger _logger; + public NotificationHubPool(ILogger logger, GlobalSettings globalSettings) + { + _logger = logger; + _connections = filterInvalidHubs(globalSettings.NotificationHubPool.NotificationHubSettings); + _clients = _connections.Select(c => c.HubClient); + } + + private List filterInvalidHubs(IEnumerable hubs) + { + List result = new(); + foreach (var hub in hubs) + { + var connection = NotificationHubConnection.From(hub); + if (!connection.IsValid) + { + _logger.LogWarning("Invalid notification hub settings: {0}", hub.HubName ?? "hub name missing"); + continue; + } + result.Add(connection); + } + + return result; + } + + + /// + /// Gets the NotificationHubClient for the given comb ID. + /// + /// + /// + /// + public NotificationHubClient ClientFor(Guid comb) + { + var possibleConnections = _connections.Where(c => c.RegistrationEnabled(comb)).ToArray(); + if (possibleConnections.Length == 0) + { + throw new InvalidOperationException($"No valid notification hubs are available for the given comb ({comb}).\n" + + $"The comb's datetime is {CoreHelpers.DateFromComb(comb)}." + + $"Hub start and end times are configured as follows:\n" + + string.Join("\n", _connections.Select(c => $"Hub {c.HubName} - Start: {c.RegistrationStartDate}, End: {c.RegistrationEndDate}"))); + } + return possibleConnections[CoreHelpers.BinForComb(comb, possibleConnections.Length)].HubClient; + } + + public INotificationHubProxy AllClients { get { return new NotificationHubClientProxy(_clients); } } +} diff --git a/src/Core/Services/Implementations/NotificationHubPushNotificationService.cs b/src/Core/NotificationHub/NotificationHubPushNotificationService.cs similarity index 99% rename from src/Core/Services/Implementations/NotificationHubPushNotificationService.cs rename to src/Core/NotificationHub/NotificationHubPushNotificationService.cs index 480f0dfa9ef8..c764424bc79d 100644 --- a/src/Core/Services/Implementations/NotificationHubPushNotificationService.cs +++ b/src/Core/NotificationHub/NotificationHubPushNotificationService.cs @@ -6,6 +6,7 @@ using Bit.Core.Models; using Bit.Core.Models.Data; using Bit.Core.Repositories; +using Bit.Core.Services; using Bit.Core.Settings; using Bit.Core.Tools.Entities; using Bit.Core.Vault.Entities; @@ -13,7 +14,7 @@ using Microsoft.Azure.NotificationHubs; using Microsoft.Extensions.Logging; -namespace Bit.Core.Services; +namespace Bit.Core.NotificationHub; public class NotificationHubPushNotificationService : IPushNotificationService { diff --git a/src/Core/Services/Implementations/NotificationHubPushRegistrationService.cs b/src/Core/NotificationHub/NotificationHubPushRegistrationService.cs similarity index 97% rename from src/Core/Services/Implementations/NotificationHubPushRegistrationService.cs rename to src/Core/NotificationHub/NotificationHubPushRegistrationService.cs index 87df60e8e3ce..6373a1fd2d6a 100644 --- a/src/Core/Services/Implementations/NotificationHubPushRegistrationService.cs +++ b/src/Core/NotificationHub/NotificationHubPushRegistrationService.cs @@ -1,17 +1,19 @@ using Bit.Core.Enums; using Bit.Core.Models.Data; using Bit.Core.Repositories; +using Bit.Core.Services; using Bit.Core.Settings; using Microsoft.Azure.NotificationHubs; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; -namespace Bit.Core.Services; +namespace Bit.Core.NotificationHub; public class NotificationHubPushRegistrationService : IPushRegistrationService { private readonly IInstallationDeviceRepository _installationDeviceRepository; private readonly GlobalSettings _globalSettings; + private readonly INotificationHubPool _notificationHubPool; private readonly IServiceProvider _serviceProvider; private readonly ILogger _logger; private Dictionary _clients = []; @@ -19,11 +21,13 @@ public class NotificationHubPushRegistrationService : IPushRegistrationService public NotificationHubPushRegistrationService( IInstallationDeviceRepository installationDeviceRepository, GlobalSettings globalSettings, + INotificationHubPool notificationHubPool, IServiceProvider serviceProvider, ILogger logger) { _installationDeviceRepository = installationDeviceRepository; _globalSettings = globalSettings; + _notificationHubPool = notificationHubPool; _serviceProvider = serviceProvider; _logger = logger; diff --git a/src/Core/Services/Implementations/MultiServicePushNotificationService.cs b/src/Core/Services/Implementations/MultiServicePushNotificationService.cs index 92e29908f50b..46db400719bf 100644 --- a/src/Core/Services/Implementations/MultiServicePushNotificationService.cs +++ b/src/Core/Services/Implementations/MultiServicePushNotificationService.cs @@ -1,5 +1,6 @@ using Bit.Core.Auth.Entities; using Bit.Core.Enums; +using Bit.Core.NotificationHub; using Bit.Core.Repositories; using Bit.Core.Settings; using Bit.Core.Tools.Entities; diff --git a/src/Core/Settings/GlobalSettings.cs b/src/Core/Settings/GlobalSettings.cs index 42e3f2bdc9c9..1817191c8296 100644 --- a/src/Core/Settings/GlobalSettings.cs +++ b/src/Core/Settings/GlobalSettings.cs @@ -65,7 +65,9 @@ public virtual string LicenseDirectory public virtual SentrySettings Sentry { get; set; } = new SentrySettings(); public virtual SyslogSettings Syslog { get; set; } = new SyslogSettings(); public virtual ILogLevelSettings MinLogLevel { get; set; } = new LogLevelSettings(); + // TODO MDG: delete this property public virtual List NotificationHubs { get; set; } = new(); + public virtual NotificationHubPoolSettings NotificationHubPool { get; set; } = new(); public virtual YubicoSettings Yubico { get; set; } = new YubicoSettings(); public virtual DuoSettings Duo { get; set; } = new DuoSettings(); public virtual BraintreeSettings Braintree { get; set; } = new BraintreeSettings(); @@ -417,7 +419,7 @@ public class NotificationHubSettings public string ConnectionString { get => _connectionString; - set => _connectionString = value.Trim('"'); + set => _connectionString = value?.Trim('"'); } public string HubName { get; set; } /// @@ -425,13 +427,37 @@ public string ConnectionString /// Enabling this will result in delayed responses because the Hub must wait on delivery to the PNS. This should ONLY be enabled in a non-production environment, as results are throttled. /// public bool EnableSendTracing { get; set; } = false; + public bool EnableRegistration { get; set; } /// - /// At least one hub configuration should have registration enabled, preferably the General hub as a safety net. + /// The date and time at which registration will be enabled. + /// + /// **This value should not be updated once set, as it is used to determine installation location of devices.** + /// + /// If null, registration is disabled. + /// /// - public bool EnableRegistration { get; set; } + public DateTime? RegistrationStartDate { get; set; } + /// + /// The date and time at which registration will be disabled. + /// + /// **This value should not be updated once set, as it is used to determine installation location of devices.** + /// + /// If null, hub registration has no yet known expiry. + /// + public DateTime? RegistrationEndDate { get; set; } public NotificationHubType HubType { get; set; } } + public class NotificationHubPoolSettings + { + /// + /// List of Notification Hub settings to use for sending push notifications. + /// + /// Note that hubs on the same namespace share active device limits, so multiple namespaces should be used to increase capacity. + /// + public List NotificationHubSettings { get; set; } = new(); + } + public class YubicoSettings { public string ClientId { get; set; } diff --git a/src/Core/Utilities/CoreHelpers.cs b/src/Core/Utilities/CoreHelpers.cs index abb3aa061273..215c3f658792 100644 --- a/src/Core/Utilities/CoreHelpers.cs +++ b/src/Core/Utilities/CoreHelpers.cs @@ -95,7 +95,7 @@ internal static DateTime DateFromComb(Guid combGuid) return new DateTime(_baseDateTicks + time.Ticks, DateTimeKind.Utc); } - internal static double BinForComb(Guid combGuid, int binCount) + internal static long BinForComb(Guid combGuid, int binCount) { // From System.Web.Util.HashCodeCombiner uint CombineHashCodes(uint h1, byte h2) diff --git a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs index f38130574578..fa4b2cba102a 100644 --- a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs +++ b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs @@ -21,6 +21,7 @@ using Bit.Core.HostedServices; using Bit.Core.Identity; using Bit.Core.IdentityServer; +using Bit.Core.NotificationHub; using Bit.Core.OrganizationFeatures; using Bit.Core.Repositories; using Bit.Core.Resources; diff --git a/test/Core.Test/NotificationHub/NotificationHubConnectionTests.cs b/test/Core.Test/NotificationHub/NotificationHubConnectionTests.cs new file mode 100644 index 000000000000..0d7382b3cc0e --- /dev/null +++ b/test/Core.Test/NotificationHub/NotificationHubConnectionTests.cs @@ -0,0 +1,205 @@ +using Bit.Core.Settings; +using Bit.Core.Utilities; +using Xunit; + +namespace Bit.Core.Test.NotificationHub; + +public class NotificationHubConnectionTests +{ + [Fact] + public void IsValid_ConnectionStringIsNull_ReturnsFalse() + { + // Arrange + var hub = new GlobalSettings.NotificationHubSettings() + { + ConnectionString = null, + HubName = "hub", + RegistrationStartDate = DateTime.UtcNow, + RegistrationEndDate = DateTime.UtcNow.AddDays(1) + }; + + // Act + var connection = NotificationHubConnection.From(hub); + + // Assert + Assert.False(connection.IsValid); + } + + [Fact] + public void IsValid_HubNameIsNull_ReturnsFalse() + { + // Arrange + var hub = new GlobalSettings.NotificationHubSettings() + { + ConnectionString = "Endpoint=sb://example.servicebus.windows.net/;", + HubName = null, + RegistrationStartDate = DateTime.UtcNow, + RegistrationEndDate = DateTime.UtcNow.AddDays(1) + }; + + // Act + var connection = NotificationHubConnection.From(hub); + + // Assert + Assert.False(connection.IsValid); + } + + [Fact] + public void IsValid_ConnectionStringAndHubNameAreNotNull_ReturnsTrue() + { + // Arrange + var hub = new GlobalSettings.NotificationHubSettings() + { + ConnectionString = "connection", + HubName = "hub", + RegistrationStartDate = DateTime.UtcNow, + RegistrationEndDate = DateTime.UtcNow.AddDays(1) + }; + + // Act + var connection = NotificationHubConnection.From(hub); + + // Assert + Assert.True(connection.IsValid); + } + + [Fact] + public void RegistrationEnabled_QueryTimeIsBeforeStartDate_ReturnsFalse() + { + // Arrange + var hub = new GlobalSettings.NotificationHubSettings() + { + ConnectionString = "connection", + HubName = "hub", + RegistrationStartDate = DateTime.UtcNow.AddDays(1), + RegistrationEndDate = DateTime.UtcNow.AddDays(2) + }; + var connection = NotificationHubConnection.From(hub); + + // Act + var result = connection.RegistrationEnabled(DateTime.UtcNow); + + // Assert + Assert.False(result); + } + + [Fact] + public void RegistrationEnabled_QueryTimeIsAfterEndDate_ReturnsFalse() + { + // Arrange + var hub = new GlobalSettings.NotificationHubSettings() + { + ConnectionString = "connection", + HubName = "hub", + RegistrationStartDate = DateTime.UtcNow, + RegistrationEndDate = DateTime.UtcNow.AddDays(1) + }; + var connection = NotificationHubConnection.From(hub); + + // Act + var result = connection.RegistrationEnabled(DateTime.UtcNow.AddDays(2)); + + // Assert + Assert.False(result); + } + + [Fact] + public void RegistrationEnabled_NullStartDate_ReturnsFalse() + { + // Arrange + var hub = new GlobalSettings.NotificationHubSettings() + { + ConnectionString = "connection", + HubName = "hub", + RegistrationStartDate = null, + RegistrationEndDate = DateTime.UtcNow.AddDays(1) + }; + var connection = NotificationHubConnection.From(hub); + + // Act + var result = connection.RegistrationEnabled(DateTime.UtcNow); + + // Assert + Assert.False(result); + } + + [Fact] + public void RegistrationEnabled_QueryTimeIsBetweenStartDateAndEndDate_ReturnsTrue() + { + // Arrange + var hub = new GlobalSettings.NotificationHubSettings() + { + ConnectionString = "connection", + HubName = "hub", + RegistrationStartDate = DateTime.UtcNow, + RegistrationEndDate = DateTime.UtcNow.AddDays(1) + }; + var connection = NotificationHubConnection.From(hub); + + // Act + var result = connection.RegistrationEnabled(DateTime.UtcNow.AddHours(1)); + + // Assert + Assert.True(result); + } + + [Fact] + public void RegistrationEnabled_CombTimeIsBeforeStartDate_ReturnsFalse() + { + // Arrange + var hub = new GlobalSettings.NotificationHubSettings() + { + ConnectionString = "connection", + HubName = "hub", + RegistrationStartDate = DateTime.UtcNow.AddDays(1), + RegistrationEndDate = DateTime.UtcNow.AddDays(2) + }; + var connection = NotificationHubConnection.From(hub); + + // Act + var result = connection.RegistrationEnabled(CoreHelpers.GenerateComb(Guid.NewGuid(), DateTime.UtcNow)); + + // Assert + Assert.False(result); + } + + [Fact] + public void RegistrationEnabled_CombTimeIsAfterEndDate_ReturnsFalse() + { + // Arrange + var hub = new GlobalSettings.NotificationHubSettings() + { + ConnectionString = "connection", + HubName = "hub", + RegistrationStartDate = DateTime.UtcNow, + RegistrationEndDate = DateTime.UtcNow.AddDays(1) + }; + var connection = NotificationHubConnection.From(hub); + + // Act + var result = connection.RegistrationEnabled(CoreHelpers.GenerateComb(Guid.NewGuid(), DateTime.UtcNow.AddDays(2))); + + // Assert + Assert.False(result); + } + + [Fact] + public void RegistrationEnabled_CombTimeIsBetweenStartDateAndEndDate_ReturnsTrue() + { + // Arrange + var hub = new GlobalSettings.NotificationHubSettings() + { + ConnectionString = "connection", + HubName = "hub", + RegistrationStartDate = DateTime.UtcNow, + RegistrationEndDate = DateTime.UtcNow.AddDays(1) + }; + var connection = NotificationHubConnection.From(hub); + + // Act + var result = connection.RegistrationEnabled(CoreHelpers.GenerateComb(Guid.NewGuid(), DateTime.UtcNow.AddHours(1))); + + // Assert + Assert.True(result); + } +} diff --git a/test/Core.Test/NotificationHub/NotificationHubPoolTests.cs b/test/Core.Test/NotificationHub/NotificationHubPoolTests.cs new file mode 100644 index 000000000000..c3cceeb26b81 --- /dev/null +++ b/test/Core.Test/NotificationHub/NotificationHubPoolTests.cs @@ -0,0 +1,156 @@ +using Bit.Core.NotificationHub; +using Bit.Core.Settings; +using Bit.Core.Utilities; +using Microsoft.Extensions.Logging; +using NSubstitute; +using Xunit; +using static Bit.Core.Settings.GlobalSettings; + +namespace Bit.Core.Test.NotificationHub; + +public class NotificationHubPoolTests +{ + [Fact] + public void NotificationHubPool_WarnsOnMissingConnectionString() + { + // Arrange + var globalSettings = new GlobalSettings() + { + NotificationHubPool = new NotificationHubPoolSettings() + { + NotificationHubSettings = new() { + new() { + ConnectionString = null, + HubName = "hub", + RegistrationStartDate = DateTime.UtcNow, + RegistrationEndDate = DateTime.UtcNow.AddDays(1) + } + } + } + }; + var logger = Substitute.For>(); + + // Act + var sut = new NotificationHubPool(logger, globalSettings); + + // Assert + logger.Received().Log(LogLevel.Warning, Arg.Any(), + Arg.Is(o => o.ToString() == "Invalid notification hub settings: hub"), + null, + Arg.Any>()); + } + + [Fact] + public void NotificationHubPool_WarnsOnMissingHubName() + { + // Arrange + var globalSettings = new GlobalSettings() + { + NotificationHubPool = new NotificationHubPoolSettings() + { + NotificationHubSettings = new() { + new() { + ConnectionString = "connection", + HubName = null, + RegistrationStartDate = DateTime.UtcNow, + RegistrationEndDate = DateTime.UtcNow.AddDays(1) + } + } + } + }; + var logger = Substitute.For>(); + + // Act + var sut = new NotificationHubPool(logger, globalSettings); + + // Assert + logger.Received().Log(LogLevel.Warning, Arg.Any(), + Arg.Is(o => o.ToString() == "Invalid notification hub settings: hub name missing"), + null, + Arg.Any>()); + } + + [Fact] + public void NotificationHubPool_ClientFor_ThrowsOnNoValidHubs() + { + // Arrange + var globalSettings = new GlobalSettings() + { + NotificationHubPool = new NotificationHubPoolSettings() + { + NotificationHubSettings = new() { + new() { + ConnectionString = "connection", + HubName = "hub", + RegistrationStartDate = null, + RegistrationEndDate = null, + } + } + } + }; + var logger = Substitute.For>(); + var sut = new NotificationHubPool(logger, globalSettings); + + // Act + Action act = () => sut.ClientFor(Guid.NewGuid()); + + // Assert + Assert.Throws(act); + } + + [Fact] + public void NotificationHubPool_ClientFor_ReturnsClient() + { + // Arrange + var globalSettings = new GlobalSettings() + { + NotificationHubPool = new NotificationHubPoolSettings() + { + NotificationHubSettings = new() { + new() { + ConnectionString = "Endpoint=sb://example.servicebus.windows.net/;SharedAccessKey=example///example=", + HubName = "hub", + RegistrationStartDate = DateTime.UtcNow, + RegistrationEndDate = DateTime.UtcNow.AddDays(1), + } + } + } + }; + var logger = Substitute.For>(); + var sut = new NotificationHubPool(logger, globalSettings); + + // Act + var client = sut.ClientFor(CoreHelpers.GenerateComb(Guid.NewGuid(), DateTime.UtcNow)); + + // Assert + Assert.NotNull(client); + } + + [Fact] + public void NotificationHubPool_AllClients_ReturnsProxy() + { + // Arrange + var globalSettings = new GlobalSettings() + { + NotificationHubPool = new NotificationHubPoolSettings() + { + NotificationHubSettings = new() { + new() { + ConnectionString = "connection", + HubName = "hub", + RegistrationStartDate = DateTime.UtcNow, + RegistrationEndDate = DateTime.UtcNow.AddDays(1), + } + } + } + }; + var logger = Substitute.For>(); + var sut = new NotificationHubPool(logger, globalSettings); + + // Act + var proxy = sut.AllClients; + + // Assert + Assert.NotNull(proxy); + } +} diff --git a/test/Core.Test/NotificationHub/NotificationHubProxyTests.cs b/test/Core.Test/NotificationHub/NotificationHubProxyTests.cs new file mode 100644 index 000000000000..5cdd10d8977f --- /dev/null +++ b/test/Core.Test/NotificationHub/NotificationHubProxyTests.cs @@ -0,0 +1,52 @@ +using AutoFixture; +using Bit.Core.NotificationHub; +using Bit.Test.Common.AutoFixture; +using Microsoft.Azure.NotificationHubs; +using NSubstitute; +using Xunit; + +namespace Bit.Core.Test.NotificationHub; + +public class NotificationHubProxyTests +{ + private readonly IEnumerable _clients; + public NotificationHubProxyTests() + { + _clients = new Fixture().WithAutoNSubstitutions().CreateMany(); + } + + public static IEnumerable ClientMethods = + [ + [ + (NotificationHubClientProxy c) => c.DeleteInstallationAsync("test"), + (INotificationHubClient c) => c.DeleteInstallationAsync("test"), + ], + [ + (NotificationHubClientProxy c) => c.DeleteInstallationAsync("test", default), + (INotificationHubClient c) => c.DeleteInstallationAsync("test", default), + ], + [ + (NotificationHubClientProxy c) => c.PatchInstallationAsync("test", new List()), + (INotificationHubClient c) => c.PatchInstallationAsync("test", Arg.Any>()), + ], + [ + (NotificationHubClientProxy c) => c.PatchInstallationAsync("test", new List(), default), + (INotificationHubClient c) => c.PatchInstallationAsync("test", Arg.Any>(), default) + ] + ]; + + [Theory] + [MemberData(nameof(ClientMethods))] + public async void CallsAllClients(Func proxyMethod, Func clientMethod) + { + var clients = _clients.ToArray(); + var proxy = new NotificationHubClientProxy(clients); + + await proxyMethod(proxy); + + foreach (var client in clients) + { + await clientMethod(client.Received()); + } + } +} diff --git a/test/Core.Test/Services/NotificationHubPushNotificationServiceTests.cs b/test/Core.Test/NotificationHub/NotificationHubPushNotificationServiceTests.cs similarity index 92% rename from test/Core.Test/Services/NotificationHubPushNotificationServiceTests.cs rename to test/Core.Test/NotificationHub/NotificationHubPushNotificationServiceTests.cs index 82594445a6e1..af525e0a1c80 100644 --- a/test/Core.Test/Services/NotificationHubPushNotificationServiceTests.cs +++ b/test/Core.Test/NotificationHub/NotificationHubPushNotificationServiceTests.cs @@ -1,4 +1,5 @@ -using Bit.Core.Repositories; +using Bit.Core.NotificationHub; +using Bit.Core.Repositories; using Bit.Core.Services; using Bit.Core.Settings; using Microsoft.AspNetCore.Http; @@ -6,7 +7,7 @@ using NSubstitute; using Xunit; -namespace Bit.Core.Test.Services; +namespace Bit.Core.Test.NotificationHub; public class NotificationHubPushNotificationServiceTests { diff --git a/test/Core.Test/Services/NotificationHubPushRegistrationServiceTests.cs b/test/Core.Test/NotificationHub/NotificationHubPushRegistrationServiceTests.cs similarity index 82% rename from test/Core.Test/Services/NotificationHubPushRegistrationServiceTests.cs rename to test/Core.Test/NotificationHub/NotificationHubPushRegistrationServiceTests.cs index a8dd536b87d7..c5851f279148 100644 --- a/test/Core.Test/Services/NotificationHubPushRegistrationServiceTests.cs +++ b/test/Core.Test/NotificationHub/NotificationHubPushRegistrationServiceTests.cs @@ -1,11 +1,11 @@ -using Bit.Core.Repositories; -using Bit.Core.Services; +using Bit.Core.NotificationHub; +using Bit.Core.Repositories; using Bit.Core.Settings; using Microsoft.Extensions.Logging; using NSubstitute; using Xunit; -namespace Bit.Core.Test.Services; +namespace Bit.Core.Test.NotificationHub; public class NotificationHubPushRegistrationServiceTests { @@ -15,6 +15,7 @@ public class NotificationHubPushRegistrationServiceTests private readonly IServiceProvider _serviceProvider; private readonly ILogger _logger; private readonly GlobalSettings _globalSettings; + private readonly INotificationHubPool _notificationHubPool; public NotificationHubPushRegistrationServiceTests() { @@ -22,10 +23,12 @@ public NotificationHubPushRegistrationServiceTests() _serviceProvider = Substitute.For(); _logger = Substitute.For>(); _globalSettings = new GlobalSettings(); + _notificationHubPool = Substitute.For(); _sut = new NotificationHubPushRegistrationService( _installationDeviceRepository, _globalSettings, + _notificationHubPool, _serviceProvider, _logger ); diff --git a/test/Core.Test/Utilities/CoreHelpersTests.cs b/test/Core.Test/Utilities/CoreHelpersTests.cs index ac02234e3453..2cce276fcb8a 100644 --- a/test/Core.Test/Utilities/CoreHelpersTests.cs +++ b/test/Core.Test/Utilities/CoreHelpersTests.cs @@ -34,33 +34,30 @@ public void GenerateComb_Success() // the comb are working properly } - public static IEnumerable GenerateCombCases = new[] - { - new object[] - { + public static IEnumerable GuidSeedCases = [ + [ Guid.Parse("a58db474-43d8-42f1-b4ee-0c17647cd0c0"), // Input Guid new DateTime(2022, 3, 12, 12, 12, 0, DateTimeKind.Utc), // Input Time - Guid.Parse("a58db474-43d8-42f1-b4ee-ae5600c90cc1"), // Expected Comb - }, - new object[] - { + ], + [ Guid.Parse("f776e6ee-511f-4352-bb28-88513002bdeb"), new DateTime(2021, 5, 10, 10, 52, 0, DateTimeKind.Utc), - Guid.Parse("f776e6ee-511f-4352-bb28-ad2400b313c1"), - }, - new object[] - { + ], + [ Guid.Parse("51a25fc7-3cad-497d-8e2f-8d77011648a1"), new DateTime(1999, 2, 26, 16, 53, 13, DateTimeKind.Utc), - Guid.Parse("51a25fc7-3cad-497d-8e2f-8d77011649cd"), - }, - new object[] - { + ], + [ Guid.Parse("bfb8f353-3b32-4a9e-bef6-24fe0b54bfb0"), new DateTime(2024, 10, 20, 1, 32, 16, DateTimeKind.Utc), - Guid.Parse("bfb8f353-3b32-4a9e-bef6-b20f00195780"), - } - }; + ] + ]; + public static IEnumerable GenerateCombCases = GuidSeedCases.Zip([ + Guid.Parse("a58db474-43d8-42f1-b4ee-ae5600c90cc1"), // Expected Comb for each Guid Seed case + Guid.Parse("f776e6ee-511f-4352-bb28-ad2400b313c1"), + Guid.Parse("51a25fc7-3cad-497d-8e2f-8d77011649cd"), + Guid.Parse("bfb8f353-3b32-4a9e-bef6-b20f00195780"), + ]).Select((zip) => new object[] { zip.Item1[0], zip.Item1[1], zip.Item2 }); [Theory] [MemberData(nameof(GenerateCombCases))] @@ -72,7 +69,7 @@ public void GenerateComb_WithInputs_Success(Guid inputGuid, DateTime inputTime, } [Theory] - [MemberData(nameof(GenerateCombCases))] + [MemberData(nameof(GuidSeedCases))] public void DateFromComb_WithComb_Success(Guid inputGuid, DateTime inputTime) { var comb = CoreHelpers.GenerateComb(inputGuid, inputTime); From abd67e8ec63d2e2a9996f7ee4038693755075b13 Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Tue, 2 Jul 2024 10:53:31 -0700 Subject: [PATCH 03/31] Replace device type sharding with comb + range sharding --- src/Api/Controllers/PushController.cs | 6 +- .../Api/Request/PushDeviceRequestModel.cs | 1 - .../Api/Request/PushUpdateRequestModel.cs | 5 +- .../Models/Data/InstallationDeviceEntity.cs | 19 ++++ .../NotificationHub/INotificationHubPool.cs | 2 +- .../NotificationHubPushRegistrationService.cs | 107 +++++------------- src/Core/Services/IPushRegistrationService.cs | 6 +- .../RelayPushRegistrationService.cs | 15 ++- .../NoopPushRegistrationService.cs | 6 +- 9 files changed, 67 insertions(+), 100 deletions(-) diff --git a/src/Api/Controllers/PushController.cs b/src/Api/Controllers/PushController.cs index c83eb200b8df..38398051060e 100644 --- a/src/Api/Controllers/PushController.cs +++ b/src/Api/Controllers/PushController.cs @@ -46,7 +46,7 @@ await _pushRegistrationService.CreateOrUpdateRegistrationAsync(model.PushToken, public async Task PostDelete([FromBody] PushDeviceRequestModel model) { CheckUsage(); - await _pushRegistrationService.DeleteRegistrationAsync(Prefix(model.Id), model.Type); + await _pushRegistrationService.DeleteRegistrationAsync(Prefix(model.Id)); } [HttpPut("add-organization")] @@ -54,7 +54,7 @@ public async Task PutAddOrganization([FromBody] PushUpdateRequestModel model) { CheckUsage(); await _pushRegistrationService.AddUserRegistrationOrganizationAsync( - model.Devices.Select(d => new KeyValuePair(Prefix(d.Id), d.Type)), + model.Devices.Select(d => Prefix(d.Id)), Prefix(model.OrganizationId)); } @@ -63,7 +63,7 @@ public async Task PutDeleteOrganization([FromBody] PushUpdateRequestModel model) { CheckUsage(); await _pushRegistrationService.DeleteUserRegistrationOrganizationAsync( - model.Devices.Select(d => new KeyValuePair(Prefix(d.Id), d.Type)), + model.Devices.Select(d => Prefix(d.Id)), Prefix(model.OrganizationId)); } diff --git a/src/Core/Models/Api/Request/PushDeviceRequestModel.cs b/src/Core/Models/Api/Request/PushDeviceRequestModel.cs index e1866b6f2735..c90603d21f6d 100644 --- a/src/Core/Models/Api/Request/PushDeviceRequestModel.cs +++ b/src/Core/Models/Api/Request/PushDeviceRequestModel.cs @@ -7,6 +7,5 @@ public class PushDeviceRequestModel { [Required] public string Id { get; set; } - [Required] public DeviceType Type { get; set; } } diff --git a/src/Core/Models/Api/Request/PushUpdateRequestModel.cs b/src/Core/Models/Api/Request/PushUpdateRequestModel.cs index 9f7ed5f28851..f8c2d296fd4a 100644 --- a/src/Core/Models/Api/Request/PushUpdateRequestModel.cs +++ b/src/Core/Models/Api/Request/PushUpdateRequestModel.cs @@ -1,5 +1,4 @@ using System.ComponentModel.DataAnnotations; -using Bit.Core.Enums; namespace Bit.Core.Models.Api; @@ -8,9 +7,9 @@ public class PushUpdateRequestModel public PushUpdateRequestModel() { } - public PushUpdateRequestModel(IEnumerable> devices, string organizationId) + public PushUpdateRequestModel(IEnumerable deviceIds, string organizationId) { - Devices = devices.Select(d => new PushDeviceRequestModel { Id = d.Key, Type = d.Value }); + Devices = deviceIds.Select(d => new PushDeviceRequestModel { Id = d }); OrganizationId = organizationId; } diff --git a/src/Core/Models/Data/InstallationDeviceEntity.cs b/src/Core/Models/Data/InstallationDeviceEntity.cs index 3186efc661c7..dcd2ae2759a7 100644 --- a/src/Core/Models/Data/InstallationDeviceEntity.cs +++ b/src/Core/Models/Data/InstallationDeviceEntity.cs @@ -37,4 +37,23 @@ public static bool IsInstallationDeviceId(string deviceId) { return deviceId != null && deviceId.Length == 73 && deviceId[36] == '_'; } + public static bool TrySplit(string deviceId, out Guid installationId, out Guid deviceIdGuid) + { + installationId = Guid.Empty; + deviceIdGuid = Guid.Empty; + if (!IsInstallationDeviceId(deviceId)) + { + return false; + } + var parts = deviceId.Split("_"); + if (parts.Length < 2) + { + return false; + } + if (!Guid.TryParse(parts[0], out installationId) || !Guid.TryParse(parts[1], out deviceIdGuid)) + { + return false; + } + return true; + } } diff --git a/src/Core/NotificationHub/INotificationHubPool.cs b/src/Core/NotificationHub/INotificationHubPool.cs index 8ae33297b5dd..7c383d7b9649 100644 --- a/src/Core/NotificationHub/INotificationHubPool.cs +++ b/src/Core/NotificationHub/INotificationHubPool.cs @@ -5,5 +5,5 @@ namespace Bit.Core.NotificationHub; public interface INotificationHubPool { NotificationHubClient ClientFor(Guid comb); - + INotificationHubProxy AllClients { get; } } diff --git a/src/Core/NotificationHub/NotificationHubPushRegistrationService.cs b/src/Core/NotificationHub/NotificationHubPushRegistrationService.cs index 6373a1fd2d6a..a3051815c625 100644 --- a/src/Core/NotificationHub/NotificationHubPushRegistrationService.cs +++ b/src/Core/NotificationHub/NotificationHubPushRegistrationService.cs @@ -16,7 +16,6 @@ public class NotificationHubPushRegistrationService : IPushRegistrationService private readonly INotificationHubPool _notificationHubPool; private readonly IServiceProvider _serviceProvider; private readonly ILogger _logger; - private Dictionary _clients = []; public NotificationHubPushRegistrationService( IInstallationDeviceRepository installationDeviceRepository, @@ -30,25 +29,6 @@ public NotificationHubPushRegistrationService( _notificationHubPool = notificationHubPool; _serviceProvider = serviceProvider; _logger = logger; - - // Is this dirty to do in the ctor? - void addHub(NotificationHubType type) - { - var hubRegistration = globalSettings.NotificationHubs.FirstOrDefault( - h => h.HubType == type && h.EnableRegistration); - if (hubRegistration != null) - { - var client = NotificationHubClient.CreateClientFromConnectionString( - hubRegistration.ConnectionString, - hubRegistration.HubName, - hubRegistration.EnableSendTracing); - _clients.Add(type, client); - } - } - - addHub(NotificationHubType.General); - addHub(NotificationHubType.iOS); - addHub(NotificationHubType.Android); } public async Task CreateOrUpdateRegistrationAsync(string pushToken, string deviceId, string userId, @@ -121,7 +101,7 @@ public async Task CreateOrUpdateRegistrationAsync(string pushToken, string devic BuildInstallationTemplate(installation, "badgeMessage", badgeMessageTemplate ?? messageTemplate, userId, identifier); - await GetClient(type).CreateOrUpdateInstallationAsync(installation); + await ClientFor(GetComb(deviceId)).CreateOrUpdateInstallationAsync(installation); if (InstallationDeviceEntity.IsInstallationDeviceId(deviceId)) { await _installationDeviceRepository.UpsertAsync(new InstallationDeviceEntity(deviceId)); @@ -156,11 +136,11 @@ private void BuildInstallationTemplate(Installation installation, string templat installation.Templates.Add(fullTemplateId, template); } - public async Task DeleteRegistrationAsync(string deviceId, DeviceType deviceType) + public async Task DeleteRegistrationAsync(string deviceId) { try { - await GetClient(deviceType).DeleteInstallationAsync(deviceId); + await ClientFor(GetComb(deviceId)).DeleteInstallationAsync(deviceId); if (InstallationDeviceEntity.IsInstallationDeviceId(deviceId)) { await _installationDeviceRepository.DeleteAsync(new InstallationDeviceEntity(deviceId)); @@ -172,31 +152,31 @@ public async Task DeleteRegistrationAsync(string deviceId, DeviceType deviceType } } - public async Task AddUserRegistrationOrganizationAsync(IEnumerable> devices, string organizationId) + public async Task AddUserRegistrationOrganizationAsync(IEnumerable deviceIds, string organizationId) { - await PatchTagsForUserDevicesAsync(devices, UpdateOperationType.Add, $"organizationId:{organizationId}"); - if (devices.Any() && InstallationDeviceEntity.IsInstallationDeviceId(devices.First().Key)) + await PatchTagsForUserDevicesAsync(deviceIds, UpdateOperationType.Add, $"organizationId:{organizationId}"); + if (deviceIds.Any() && InstallationDeviceEntity.IsInstallationDeviceId(deviceIds.First())) { - var entities = devices.Select(e => new InstallationDeviceEntity(e.Key)); + var entities = deviceIds.Select(e => new InstallationDeviceEntity(e)); await _installationDeviceRepository.UpsertManyAsync(entities.ToList()); } } - public async Task DeleteUserRegistrationOrganizationAsync(IEnumerable> devices, string organizationId) + public async Task DeleteUserRegistrationOrganizationAsync(IEnumerable deviceIds, string organizationId) { - await PatchTagsForUserDevicesAsync(devices, UpdateOperationType.Remove, + await PatchTagsForUserDevicesAsync(deviceIds, UpdateOperationType.Remove, $"organizationId:{organizationId}"); - if (devices.Any() && InstallationDeviceEntity.IsInstallationDeviceId(devices.First().Key)) + if (deviceIds.Any() && InstallationDeviceEntity.IsInstallationDeviceId(deviceIds.First())) { - var entities = devices.Select(e => new InstallationDeviceEntity(e.Key)); + var entities = deviceIds.Select(e => new InstallationDeviceEntity(e)); await _installationDeviceRepository.UpsertManyAsync(entities.ToList()); } } - private async Task PatchTagsForUserDevicesAsync(IEnumerable> devices, UpdateOperationType op, + private async Task PatchTagsForUserDevicesAsync(IEnumerable deviceIds, UpdateOperationType op, string tag) { - if (!devices.Any()) + if (!deviceIds.Any()) { return; } @@ -216,11 +196,11 @@ private async Task PatchTagsForUserDevicesAsync(IEnumerable { operation }); + await ClientFor(GetComb(deviceId)).PatchInstallationAsync(deviceId, new List { operation }); } catch (Exception e) when (e.InnerException == null || !e.InnerException.Message.Contains("(404) Not Found")) { @@ -229,53 +209,24 @@ private async Task PatchTagsForUserDevicesAsync(IEnumerable> devices, string organizationId); - Task DeleteUserRegistrationOrganizationAsync(IEnumerable> devices, string organizationId); + Task DeleteRegistrationAsync(string deviceId); + Task AddUserRegistrationOrganizationAsync(IEnumerable deviceIds, string organizationId); + Task DeleteUserRegistrationOrganizationAsync(IEnumerable deviceIds, string organizationId); } diff --git a/src/Core/Services/Implementations/RelayPushRegistrationService.cs b/src/Core/Services/Implementations/RelayPushRegistrationService.cs index d9df7d04dcbc..d0f7736e984b 100644 --- a/src/Core/Services/Implementations/RelayPushRegistrationService.cs +++ b/src/Core/Services/Implementations/RelayPushRegistrationService.cs @@ -38,37 +38,36 @@ public async Task CreateOrUpdateRegistrationAsync(string pushToken, string devic await SendAsync(HttpMethod.Post, "push/register", requestModel); } - public async Task DeleteRegistrationAsync(string deviceId, DeviceType type) + public async Task DeleteRegistrationAsync(string deviceId) { var requestModel = new PushDeviceRequestModel { Id = deviceId, - Type = type, }; await SendAsync(HttpMethod.Post, "push/delete", requestModel); } public async Task AddUserRegistrationOrganizationAsync( - IEnumerable> devices, string organizationId) + IEnumerable deviceIds, string organizationId) { - if (!devices.Any()) + if (!deviceIds.Any()) { return; } - var requestModel = new PushUpdateRequestModel(devices, organizationId); + var requestModel = new PushUpdateRequestModel(deviceIds, organizationId); await SendAsync(HttpMethod.Put, "push/add-organization", requestModel); } public async Task DeleteUserRegistrationOrganizationAsync( - IEnumerable> devices, string organizationId) + IEnumerable deviceIds, string organizationId) { - if (!devices.Any()) + if (!deviceIds.Any()) { return; } - var requestModel = new PushUpdateRequestModel(devices, organizationId); + var requestModel = new PushUpdateRequestModel(deviceIds, organizationId); await SendAsync(HttpMethod.Put, "push/delete-organization", requestModel); } } diff --git a/src/Core/Services/NoopImplementations/NoopPushRegistrationService.cs b/src/Core/Services/NoopImplementations/NoopPushRegistrationService.cs index fcd0889248a3..f6279c946798 100644 --- a/src/Core/Services/NoopImplementations/NoopPushRegistrationService.cs +++ b/src/Core/Services/NoopImplementations/NoopPushRegistrationService.cs @@ -4,7 +4,7 @@ namespace Bit.Core.Services; public class NoopPushRegistrationService : IPushRegistrationService { - public Task AddUserRegistrationOrganizationAsync(IEnumerable> devices, string organizationId) + public Task AddUserRegistrationOrganizationAsync(IEnumerable deviceIds, string organizationId) { return Task.FromResult(0); } @@ -15,12 +15,12 @@ public Task CreateOrUpdateRegistrationAsync(string pushToken, string deviceId, s return Task.FromResult(0); } - public Task DeleteRegistrationAsync(string deviceId, DeviceType deviceType) + public Task DeleteRegistrationAsync(string deviceId) { return Task.FromResult(0); } - public Task DeleteUserRegistrationOrganizationAsync(IEnumerable> devices, string organizationId) + public Task DeleteUserRegistrationOrganizationAsync(IEnumerable deviceIds, string organizationId) { return Task.FromResult(0); } From 709851543e999bfa0b025e670b7b38c48ec4ce78 Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Tue, 2 Jul 2024 12:03:22 -0700 Subject: [PATCH 04/31] Fix proxy interface --- .../INotificationHubClientProxy.cs | 5 +---- .../NotificationHubClientProxy.cs | 17 ++++++++--------- .../Services/Implementations/DeviceService.cs | 4 ++-- .../NotificationHubProxyTests.cs | 16 ++-------------- 4 files changed, 13 insertions(+), 29 deletions(-) diff --git a/src/Core/NotificationHub/INotificationHubClientProxy.cs b/src/Core/NotificationHub/INotificationHubClientProxy.cs index 5e8dbb2c29e2..ab499872dcfb 100644 --- a/src/Core/NotificationHub/INotificationHubClientProxy.cs +++ b/src/Core/NotificationHub/INotificationHubClientProxy.cs @@ -4,8 +4,5 @@ namespace Bit.Core.NotificationHub; public interface INotificationHubProxy { - Task DeleteInstallationAsync(string installationId); - Task DeleteInstallationAsync(string installationId, CancellationToken cancellationToken); - Task PatchInstallationAsync(string installationId, IList operations); - Task PatchInstallationAsync(string installationId, IList operations, CancellationToken cancellationToken); + Task<(INotificationHubClient client, NotificationOutcome outcome)[]> SendTemplateNotificationAsync(IDictionary properties, string tagExpression); } diff --git a/src/Core/NotificationHub/NotificationHubClientProxy.cs b/src/Core/NotificationHub/NotificationHubClientProxy.cs index 99e19d61cff5..81638587e959 100644 --- a/src/Core/NotificationHub/NotificationHubClientProxy.cs +++ b/src/Core/NotificationHub/NotificationHubClientProxy.cs @@ -11,17 +11,16 @@ public NotificationHubClientProxy(IEnumerable clients) _clients = clients; } - private async Task ApplyToAllClientsAsync(Func action) + private async Task<(INotificationHubClient, T)[]> ApplyToAllClientsAsync(Func> action) { - var tasks = _clients.Select(async c => await action(c)); - await Task.WhenAll(tasks); + var tasks = _clients.Select(async c => (c, await action(c))); + return await Task.WhenAll(tasks); } - // partial INotificationHubClient implementation + // partial proxy of INotificationHubClient implementation // Note: Any other methods that are needed can simply be delegated as done here. - public async Task DeleteInstallationAsync(string installationId) => await ApplyToAllClientsAsync((c) => c.DeleteInstallationAsync(installationId)); - public async Task DeleteInstallationAsync(string installationId, CancellationToken cancellationToken) => await ApplyToAllClientsAsync(c => c.DeleteInstallationAsync(installationId, cancellationToken)); - public async Task PatchInstallationAsync(string installationId, IList operations) => await ApplyToAllClientsAsync(c => c.PatchInstallationAsync(installationId, operations)); - public async Task PatchInstallationAsync(string installationId, IList operations, CancellationToken cancellationToken) => await ApplyToAllClientsAsync(c => c.PatchInstallationAsync(installationId, operations, cancellationToken)); - + public async Task<(INotificationHubClient client, NotificationOutcome outcome)[]> SendTemplateNotificationAsync(IDictionary properties, string tagExpression) + { + return await ApplyToAllClientsAsync(async c => await c.SendTemplateNotificationAsync(properties, tagExpression)); + } } diff --git a/src/Core/Services/Implementations/DeviceService.cs b/src/Core/Services/Implementations/DeviceService.cs index 9d8315f691ba..5b1e4b0f0171 100644 --- a/src/Core/Services/Implementations/DeviceService.cs +++ b/src/Core/Services/Implementations/DeviceService.cs @@ -38,13 +38,13 @@ await _pushRegistrationService.CreateOrUpdateRegistrationAsync(device.PushToken, public async Task ClearTokenAsync(Device device) { await _deviceRepository.ClearPushTokenAsync(device.Id); - await _pushRegistrationService.DeleteRegistrationAsync(device.Id.ToString(), device.Type); + await _pushRegistrationService.DeleteRegistrationAsync(device.Id.ToString()); } public async Task DeleteAsync(Device device) { await _deviceRepository.DeleteAsync(device); - await _pushRegistrationService.DeleteRegistrationAsync(device.Id.ToString(), device.Type); + await _pushRegistrationService.DeleteRegistrationAsync(device.Id.ToString()); } public async Task UpdateDevicesTrustAsync(string currentDeviceIdentifier, diff --git a/test/Core.Test/NotificationHub/NotificationHubProxyTests.cs b/test/Core.Test/NotificationHub/NotificationHubProxyTests.cs index 5cdd10d8977f..704519caf111 100644 --- a/test/Core.Test/NotificationHub/NotificationHubProxyTests.cs +++ b/test/Core.Test/NotificationHub/NotificationHubProxyTests.cs @@ -18,21 +18,9 @@ public NotificationHubProxyTests() public static IEnumerable ClientMethods = [ [ - (NotificationHubClientProxy c) => c.DeleteInstallationAsync("test"), - (INotificationHubClient c) => c.DeleteInstallationAsync("test"), + (NotificationHubClientProxy c) => c.SendTemplateNotificationAsync(new Dictionary() { { "key", "value" } }, "tag"), + (INotificationHubClient c) => c.SendTemplateNotificationAsync(new Dictionary() { { "key", "value" } }, "tag"), ], - [ - (NotificationHubClientProxy c) => c.DeleteInstallationAsync("test", default), - (INotificationHubClient c) => c.DeleteInstallationAsync("test", default), - ], - [ - (NotificationHubClientProxy c) => c.PatchInstallationAsync("test", new List()), - (INotificationHubClient c) => c.PatchInstallationAsync("test", Arg.Any>()), - ], - [ - (NotificationHubClientProxy c) => c.PatchInstallationAsync("test", new List(), default), - (INotificationHubClient c) => c.PatchInstallationAsync("test", Arg.Any>(), default) - ] ]; [Theory] From 5bc4945bc703fa74be2bf8a02fc81cca60cbdfc8 Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Tue, 2 Jul 2024 12:46:14 -0700 Subject: [PATCH 05/31] Use enumerable services for multiServiceNotificationHub --- .../NotificationHubPushNotificationService.cs | 42 +++++----------- .../MultiServicePushNotificationService.cs | 48 ++----------------- .../Utilities/ServiceCollectionExtensions.cs | 27 ++++++++--- 3 files changed, 39 insertions(+), 78 deletions(-) diff --git a/src/Core/NotificationHub/NotificationHubPushNotificationService.cs b/src/Core/NotificationHub/NotificationHubPushNotificationService.cs index c764424bc79d..2bb765f48266 100644 --- a/src/Core/NotificationHub/NotificationHubPushNotificationService.cs +++ b/src/Core/NotificationHub/NotificationHubPushNotificationService.cs @@ -23,11 +23,13 @@ public class NotificationHubPushNotificationService : IPushNotificationService private readonly IHttpContextAccessor _httpContextAccessor; private readonly List _clients = []; private readonly bool _enableTracing = false; + private readonly INotificationHubPool _notificationHubPool; private readonly ILogger _logger; public NotificationHubPushNotificationService( IInstallationDeviceRepository installationDeviceRepository, GlobalSettings globalSettings, + INotificationHubPool notificationHubPool, IHttpContextAccessor httpContextAccessor, ILogger logger) { @@ -35,17 +37,6 @@ public NotificationHubPushNotificationService( _globalSettings = globalSettings; _httpContextAccessor = httpContextAccessor; - foreach (var hub in globalSettings.NotificationHubs) - { - var client = NotificationHubClient.CreateClientFromConnectionString( - hub.ConnectionString, - hub.HubName, - hub.EnableSendTracing); - _clients.Add(client); - - _enableTracing = _enableTracing || hub.EnableSendTracing; - } - _logger = logger; } @@ -265,30 +256,23 @@ private string BuildTag(string tag, string identifier) private async Task SendPayloadAsync(string tag, PushType type, object payload) { - var tasks = new List>(); - foreach (var client in _clients) - { - var task = client.SendTemplateNotificationAsync( - new Dictionary - { - { "type", ((byte)type).ToString() }, - { "payload", JsonSerializer.Serialize(payload) } - }, tag); - tasks.Add(task); - } - - await Task.WhenAll(tasks); + var results = await _notificationHubPool.AllClients.SendTemplateNotificationAsync( + new Dictionary + { + { "type", ((byte)type).ToString() }, + { "payload", JsonSerializer.Serialize(payload) } + }, tag); if (_enableTracing) { - for (var i = 0; i < tasks.Count; i++) + foreach (var (client, outcome) in results) { - if (_clients[i].EnableTestSend) + if (!client.EnableTestSend) { - var outcome = await tasks[i]; - _logger.LogInformation("Azure Notification Hub Tracking ID: {id} | {type} push notification with {success} successes and {failure} failures with a payload of {@payload} and result of {@results}", - outcome.TrackingId, type, outcome.Success, outcome.Failure, payload, outcome.Results); + continue; } + _logger.LogInformation("Azure Notification Hub Tracking ID: {id} | {type} push notification with {success} successes and {failure} failures with a payload of {@payload} and result of {@results}", + outcome.TrackingId, type, outcome.Success, outcome.Failure, payload, outcome.Results); } } } diff --git a/src/Core/Services/Implementations/MultiServicePushNotificationService.cs b/src/Core/Services/Implementations/MultiServicePushNotificationService.cs index 46db400719bf..8da0839c392f 100644 --- a/src/Core/Services/Implementations/MultiServicePushNotificationService.cs +++ b/src/Core/Services/Implementations/MultiServicePushNotificationService.cs @@ -1,12 +1,8 @@ using Bit.Core.Auth.Entities; using Bit.Core.Enums; -using Bit.Core.NotificationHub; -using Bit.Core.Repositories; -using Bit.Core.Settings; using Bit.Core.Tools.Entities; -using Bit.Core.Utilities; using Bit.Core.Vault.Entities; -using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; namespace Bit.Core.Services; @@ -17,44 +13,10 @@ public class MultiServicePushNotificationService : IPushNotificationService private readonly ILogger _logger; public MultiServicePushNotificationService( - IHttpClientFactory httpFactory, - IDeviceRepository deviceRepository, - IInstallationDeviceRepository installationDeviceRepository, - GlobalSettings globalSettings, - IHttpContextAccessor httpContextAccessor, - ILogger logger, - ILogger relayLogger, - ILogger hubLogger) - { - if (globalSettings.SelfHosted) - { - if (CoreHelpers.SettingHasValue(globalSettings.PushRelayBaseUri) && - globalSettings.Installation?.Id != null && - CoreHelpers.SettingHasValue(globalSettings.Installation?.Key)) - { - _services.Add(new RelayPushNotificationService(httpFactory, deviceRepository, globalSettings, - httpContextAccessor, relayLogger)); - } - if (CoreHelpers.SettingHasValue(globalSettings.InternalIdentityKey) && - CoreHelpers.SettingHasValue(globalSettings.BaseServiceUri.InternalNotifications)) - { - _services.Add(new NotificationsApiPushNotificationService( - httpFactory, globalSettings, httpContextAccessor, hubLogger)); - } - } - else - { - var generalHub = globalSettings.NotificationHubs?.FirstOrDefault(h => h.HubType == NotificationHubType.General); - if (CoreHelpers.SettingHasValue(generalHub?.ConnectionString)) - { - _services.Add(new NotificationHubPushNotificationService(installationDeviceRepository, - globalSettings, httpContextAccessor, hubLogger)); - } - if (CoreHelpers.SettingHasValue(globalSettings.Notifications?.ConnectionString)) - { - _services.Add(new AzureQueuePushNotificationService(globalSettings, httpContextAccessor)); - } - } + [FromKeyedServices("pushNotification")] IEnumerable services, + ILogger logger) + { + _services = services.ToList(); _logger = logger; } diff --git a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs index fa4b2cba102a..e3d3c0b7e14b 100644 --- a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs +++ b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs @@ -290,22 +290,37 @@ public static void AddDefaultServices(this IServiceCollection services, GlobalSe services.AddSingleton(); } - services.AddSingleton(); - if (globalSettings.SelfHosted && - CoreHelpers.SettingHasValue(globalSettings.PushRelayBaseUri) && - globalSettings.Installation?.Id != null && - CoreHelpers.SettingHasValue(globalSettings.Installation?.Key)) + if (globalSettings.SelfHosted) { - services.AddSingleton(); + if (CoreHelpers.SettingHasValue(globalSettings.PushRelayBaseUri) && + globalSettings.Installation?.Id != null && + CoreHelpers.SettingHasValue(globalSettings.Installation?.Key)) + { + services.AddKeyedSingleton("pushNotification"); + services.AddSingleton(); + } + if (CoreHelpers.SettingHasValue(globalSettings.InternalIdentityKey) && + CoreHelpers.SettingHasValue(globalSettings.BaseServiceUri.InternalNotifications)) + { + services.AddKeyedSingleton("pushNotification"); + } } else if (!globalSettings.SelfHosted) { + services.AddSingleton(); services.AddSingleton(); + services.AddKeyedSingleton("pushNotification"); + if (CoreHelpers.SettingHasValue(globalSettings.Notifications?.ConnectionString)) + { + services.AddSingleton(); + } } else { services.AddSingleton(); } + // Must be last registered IPushNotificationService + services.AddSingleton(); if (!globalSettings.SelfHosted && CoreHelpers.SettingHasValue(globalSettings.Mail.ConnectionString)) { From c7fd6bfe9459712e59c517c64a1558ea2e527123 Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Tue, 2 Jul 2024 12:49:00 -0700 Subject: [PATCH 06/31] Fix push interface usage --- .../Services/Implementations/OrganizationService.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs b/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs index 9bcefa3c7472..cb766cfb9bdd 100644 --- a/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs +++ b/src/Core/AdminConsole/Services/Implementations/OrganizationService.cs @@ -1952,12 +1952,12 @@ await _pushRegistrationService.DeleteUserRegistrationOrganizationAsync(devices, } - private async Task>> GetUserDeviceIdsAsync(Guid userId) + private async Task> GetUserDeviceIdsAsync(Guid userId) { var devices = await _deviceRepository.GetManyByUserIdAsync(userId); return devices .Where(d => !string.IsNullOrWhiteSpace(d.PushToken)) - .Select(d => new KeyValuePair(d.Id.ToString(), d.Type)); + .Select(d => d.Id.ToString()); } public async Task ReplaceAndUpdateCacheAsync(Organization org, EventType? orgEvent = null) From 15c0490630f4312bc78a6b7b594f50eb4c00eed4 Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Tue, 2 Jul 2024 13:39:30 -0700 Subject: [PATCH 07/31] Fix push notification service dependencies --- .../NotificationHubPushNotificationService.cs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/Core/NotificationHub/NotificationHubPushNotificationService.cs b/src/Core/NotificationHub/NotificationHubPushNotificationService.cs index 2bb765f48266..1992a8a3dd6f 100644 --- a/src/Core/NotificationHub/NotificationHubPushNotificationService.cs +++ b/src/Core/NotificationHub/NotificationHubPushNotificationService.cs @@ -7,11 +7,9 @@ using Bit.Core.Models.Data; using Bit.Core.Repositories; using Bit.Core.Services; -using Bit.Core.Settings; using Bit.Core.Tools.Entities; using Bit.Core.Vault.Entities; using Microsoft.AspNetCore.Http; -using Microsoft.Azure.NotificationHubs; using Microsoft.Extensions.Logging; namespace Bit.Core.NotificationHub; @@ -19,24 +17,20 @@ namespace Bit.Core.NotificationHub; public class NotificationHubPushNotificationService : IPushNotificationService { private readonly IInstallationDeviceRepository _installationDeviceRepository; - private readonly GlobalSettings _globalSettings; private readonly IHttpContextAccessor _httpContextAccessor; - private readonly List _clients = []; private readonly bool _enableTracing = false; private readonly INotificationHubPool _notificationHubPool; private readonly ILogger _logger; public NotificationHubPushNotificationService( IInstallationDeviceRepository installationDeviceRepository, - GlobalSettings globalSettings, INotificationHubPool notificationHubPool, IHttpContextAccessor httpContextAccessor, ILogger logger) { _installationDeviceRepository = installationDeviceRepository; - _globalSettings = globalSettings; _httpContextAccessor = httpContextAccessor; - + _notificationHubPool = notificationHubPool; _logger = logger; } From a84b486a4ea7707e2393fb1f1d3a5798cef8dc92 Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Tue, 2 Jul 2024 14:30:47 -0700 Subject: [PATCH 08/31] Fix push notification keys --- .../MultiServicePushNotificationService.cs | 4 ++-- .../Utilities/ServiceCollectionExtensions.cs | 11 +++++------ 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/Core/Services/Implementations/MultiServicePushNotificationService.cs b/src/Core/Services/Implementations/MultiServicePushNotificationService.cs index 8da0839c392f..c10037837d93 100644 --- a/src/Core/Services/Implementations/MultiServicePushNotificationService.cs +++ b/src/Core/Services/Implementations/MultiServicePushNotificationService.cs @@ -13,10 +13,10 @@ public class MultiServicePushNotificationService : IPushNotificationService private readonly ILogger _logger; public MultiServicePushNotificationService( - [FromKeyedServices("pushNotification")] IEnumerable services, + [FromKeyedServices("implementation")] IEnumerable services, ILogger logger) { - _services = services.ToList(); + _services.AddRange(services); _logger = logger; } diff --git a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs index e3d3c0b7e14b..997fd6020989 100644 --- a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs +++ b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs @@ -290,37 +290,36 @@ public static void AddDefaultServices(this IServiceCollection services, GlobalSe services.AddSingleton(); } + services.AddSingleton(); if (globalSettings.SelfHosted) { if (CoreHelpers.SettingHasValue(globalSettings.PushRelayBaseUri) && globalSettings.Installation?.Id != null && CoreHelpers.SettingHasValue(globalSettings.Installation?.Key)) { - services.AddKeyedSingleton("pushNotification"); + services.AddKeyedSingleton("implementation"); services.AddSingleton(); } if (CoreHelpers.SettingHasValue(globalSettings.InternalIdentityKey) && CoreHelpers.SettingHasValue(globalSettings.BaseServiceUri.InternalNotifications)) { - services.AddKeyedSingleton("pushNotification"); + services.AddKeyedSingleton("implementation"); } } else if (!globalSettings.SelfHosted) { services.AddSingleton(); services.AddSingleton(); - services.AddKeyedSingleton("pushNotification"); + services.AddKeyedSingleton("implementation"); if (CoreHelpers.SettingHasValue(globalSettings.Notifications?.ConnectionString)) { - services.AddSingleton(); + services.AddKeyedSingleton("implementation"); } } else { services.AddSingleton(); } - // Must be last registered IPushNotificationService - services.AddSingleton(); if (!globalSettings.SelfHosted && CoreHelpers.SettingHasValue(globalSettings.Mail.ConnectionString)) { From b3785d33c0dba8f7a5f2488b285c45c36e9e5889 Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Tue, 2 Jul 2024 15:13:10 -0700 Subject: [PATCH 09/31] Fixup documentation --- src/Core/NotificationHub/NotificationHubPool.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Core/NotificationHub/NotificationHubPool.cs b/src/Core/NotificationHub/NotificationHubPool.cs index 32b8188227f8..eba81a4b5a36 100644 --- a/src/Core/NotificationHub/NotificationHubPool.cs +++ b/src/Core/NotificationHub/NotificationHubPool.cs @@ -40,7 +40,7 @@ private List filterInvalidHubs(IEnumerable /// /// - /// + /// Thrown when no notification hub is found for a given comb. public NotificationHubClient ClientFor(Guid comb) { var possibleConnections = _connections.Where(c => c.RegistrationEnabled(comb)).ToArray(); From 2ce81b1bda8c37c3790f381490e2f09ae1228d17 Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Tue, 2 Jul 2024 15:13:53 -0700 Subject: [PATCH 10/31] Remove deprecated settings --- src/Core/Settings/GlobalSettings.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/Core/Settings/GlobalSettings.cs b/src/Core/Settings/GlobalSettings.cs index 1817191c8296..7d3b323974d9 100644 --- a/src/Core/Settings/GlobalSettings.cs +++ b/src/Core/Settings/GlobalSettings.cs @@ -65,8 +65,6 @@ public virtual string LicenseDirectory public virtual SentrySettings Sentry { get; set; } = new SentrySettings(); public virtual SyslogSettings Syslog { get; set; } = new SyslogSettings(); public virtual ILogLevelSettings MinLogLevel { get; set; } = new LogLevelSettings(); - // TODO MDG: delete this property - public virtual List NotificationHubs { get; set; } = new(); public virtual NotificationHubPoolSettings NotificationHubPool { get; set; } = new(); public virtual YubicoSettings Yubico { get; set; } = new YubicoSettings(); public virtual DuoSettings Duo { get; set; } = new DuoSettings(); @@ -427,7 +425,6 @@ public string ConnectionString /// Enabling this will result in delayed responses because the Hub must wait on delivery to the PNS. This should ONLY be enabled in a non-production environment, as results are throttled. /// public bool EnableSendTracing { get; set; } = false; - public bool EnableRegistration { get; set; } /// /// The date and time at which registration will be enabled. /// From c53037ea1c3ee034e4a586fad6cbed718118aae3 Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Tue, 2 Jul 2024 15:14:25 -0700 Subject: [PATCH 11/31] Fix tests --- .../NotificationHubProxyTests.cs | 2 +- ...ficationHubPushNotificationServiceTests.cs | 7 +++-- ...ultiServicePushNotificationServiceTests.cs | 27 +++++-------------- 3 files changed, 10 insertions(+), 26 deletions(-) diff --git a/test/Core.Test/NotificationHub/NotificationHubProxyTests.cs b/test/Core.Test/NotificationHub/NotificationHubProxyTests.cs index 704519caf111..b2e9c4f9f330 100644 --- a/test/Core.Test/NotificationHub/NotificationHubProxyTests.cs +++ b/test/Core.Test/NotificationHub/NotificationHubProxyTests.cs @@ -19,7 +19,7 @@ public NotificationHubProxyTests() [ [ (NotificationHubClientProxy c) => c.SendTemplateNotificationAsync(new Dictionary() { { "key", "value" } }, "tag"), - (INotificationHubClient c) => c.SendTemplateNotificationAsync(new Dictionary() { { "key", "value" } }, "tag"), + (INotificationHubClient c) => c.SendTemplateNotificationAsync(Arg.Is>((a) => a.Keys.Count == 1 && a.ContainsKey("key") && a["key"] == "value"), "tag"), ], ]; diff --git a/test/Core.Test/NotificationHub/NotificationHubPushNotificationServiceTests.cs b/test/Core.Test/NotificationHub/NotificationHubPushNotificationServiceTests.cs index af525e0a1c80..ea9ce54131e8 100644 --- a/test/Core.Test/NotificationHub/NotificationHubPushNotificationServiceTests.cs +++ b/test/Core.Test/NotificationHub/NotificationHubPushNotificationServiceTests.cs @@ -1,7 +1,6 @@ using Bit.Core.NotificationHub; using Bit.Core.Repositories; using Bit.Core.Services; -using Bit.Core.Settings; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.Logging; using NSubstitute; @@ -14,20 +13,20 @@ public class NotificationHubPushNotificationServiceTests private readonly NotificationHubPushNotificationService _sut; private readonly IInstallationDeviceRepository _installationDeviceRepository; - private readonly GlobalSettings _globalSettings; + private readonly INotificationHubPool _notificationHubPool; private readonly IHttpContextAccessor _httpContextAccessor; private readonly ILogger _logger; public NotificationHubPushNotificationServiceTests() { _installationDeviceRepository = Substitute.For(); - _globalSettings = new GlobalSettings(); _httpContextAccessor = Substitute.For(); + _notificationHubPool = Substitute.For(); _logger = Substitute.For>(); _sut = new NotificationHubPushNotificationService( _installationDeviceRepository, - _globalSettings, + _notificationHubPool, _httpContextAccessor, _logger ); diff --git a/test/Core.Test/Services/MultiServicePushNotificationServiceTests.cs b/test/Core.Test/Services/MultiServicePushNotificationServiceTests.cs index b1876f1dda7f..335d492c2e9f 100644 --- a/test/Core.Test/Services/MultiServicePushNotificationServiceTests.cs +++ b/test/Core.Test/Services/MultiServicePushNotificationServiceTests.cs @@ -1,7 +1,6 @@ -using Bit.Core.Repositories; +using AutoFixture; using Bit.Core.Services; -using Bit.Core.Settings; -using Microsoft.AspNetCore.Http; +using Bit.Test.Common.AutoFixture; using Microsoft.Extensions.Logging; using NSubstitute; using Xunit; @@ -12,35 +11,21 @@ public class MultiServicePushNotificationServiceTests { private readonly MultiServicePushNotificationService _sut; - private readonly IHttpClientFactory _httpFactory; - private readonly IDeviceRepository _deviceRepository; - private readonly IInstallationDeviceRepository _installationDeviceRepository; - private readonly GlobalSettings _globalSettings; - private readonly IHttpContextAccessor _httpContextAccessor; private readonly ILogger _logger; private readonly ILogger _relayLogger; private readonly ILogger _hubLogger; + private readonly IEnumerable _services; public MultiServicePushNotificationServiceTests() { - _httpFactory = Substitute.For(); - _deviceRepository = Substitute.For(); - _installationDeviceRepository = Substitute.For(); - _globalSettings = new GlobalSettings(); - _httpContextAccessor = Substitute.For(); _logger = Substitute.For>(); _relayLogger = Substitute.For>(); _hubLogger = Substitute.For>(); + _services = new Fixture().WithAutoNSubstitutions().CreateMany(); _sut = new MultiServicePushNotificationService( - _httpFactory, - _deviceRepository, - _installationDeviceRepository, - _globalSettings, - _httpContextAccessor, - _logger, - _relayLogger, - _hubLogger + _services, + _logger ); } From 6fbcb1abcac584b6e9066a35963a979b733a64d1 Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Wed, 3 Jul 2024 10:49:17 -0700 Subject: [PATCH 12/31] PascalCase method names --- src/Core/NotificationHub/NotificationHubPool.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Core/NotificationHub/NotificationHubPool.cs b/src/Core/NotificationHub/NotificationHubPool.cs index eba81a4b5a36..0837a0079a72 100644 --- a/src/Core/NotificationHub/NotificationHubPool.cs +++ b/src/Core/NotificationHub/NotificationHubPool.cs @@ -13,11 +13,11 @@ public class NotificationHubPool : INotificationHubPool public NotificationHubPool(ILogger logger, GlobalSettings globalSettings) { _logger = logger; - _connections = filterInvalidHubs(globalSettings.NotificationHubPool.NotificationHubSettings); + _connections = FilterInvalidHubs(globalSettings.NotificationHubPool.NotificationHubSettings); _clients = _connections.Select(c => c.HubClient); } - private List filterInvalidHubs(IEnumerable hubs) + private List FilterInvalidHubs(IEnumerable hubs) { List result = new(); foreach (var hub in hubs) From 78c6bd6192f58afc2f13fc336b327f16d06604a2 Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Wed, 3 Jul 2024 11:15:22 -0700 Subject: [PATCH 13/31] Remove unused request model properties --- src/Core/Models/Api/Request/PushDeviceRequestModel.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Core/Models/Api/Request/PushDeviceRequestModel.cs b/src/Core/Models/Api/Request/PushDeviceRequestModel.cs index c90603d21f6d..8b97dcc3600f 100644 --- a/src/Core/Models/Api/Request/PushDeviceRequestModel.cs +++ b/src/Core/Models/Api/Request/PushDeviceRequestModel.cs @@ -1,5 +1,4 @@ using System.ComponentModel.DataAnnotations; -using Bit.Core.Enums; namespace Bit.Core.Models.Api; @@ -7,5 +6,4 @@ public class PushDeviceRequestModel { [Required] public string Id { get; set; } - public DeviceType Type { get; set; } } From 7c7f92b4ac9d3bc6e3821d2c6367d35378e361b2 Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Wed, 3 Jul 2024 16:53:53 -0700 Subject: [PATCH 14/31] Remove unused setting --- src/Core/Settings/GlobalSettings.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Core/Settings/GlobalSettings.cs b/src/Core/Settings/GlobalSettings.cs index 7d3b323974d9..d83748db4928 100644 --- a/src/Core/Settings/GlobalSettings.cs +++ b/src/Core/Settings/GlobalSettings.cs @@ -1,5 +1,4 @@ using Bit.Core.Auth.Settings; -using Bit.Core.Enums; using Bit.Core.Settings.LoggingSettings; namespace Bit.Core.Settings; @@ -442,7 +441,6 @@ public string ConnectionString /// If null, hub registration has no yet known expiry. /// public DateTime? RegistrationEndDate { get; set; } - public NotificationHubType HubType { get; set; } } public class NotificationHubPoolSettings From 14ff31cde3892fc8ef34ab3660836b2695c58902 Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Mon, 8 Jul 2024 08:00:23 -0700 Subject: [PATCH 15/31] Improve DateFromComb precision --- src/Core/Utilities/CoreHelpers.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Core/Utilities/CoreHelpers.cs b/src/Core/Utilities/CoreHelpers.cs index 215c3f658792..f7f2de48ba2c 100644 --- a/src/Core/Utilities/CoreHelpers.cs +++ b/src/Core/Utilities/CoreHelpers.cs @@ -91,7 +91,7 @@ internal static DateTime DateFromComb(Guid combGuid) var days = BitConverter.ToInt32(daysArray, 0); var msecs = BitConverter.ToInt32(msecsArray, 0); - var time = new TimeSpan(days, 0, 0, 0, (int)(msecs * 3.333333)); + var time = TimeSpan.FromDays(days) + TimeSpan.FromMilliseconds(msecs * 3.333333); return new DateTime(_baseDateTicks + time.Ticks, DateTimeKind.Utc); } From 1445d01b46c009012569d48a0b4d32f432a4af78 Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Tue, 9 Jul 2024 06:21:46 -0700 Subject: [PATCH 16/31] Prefer readonly service enumerable --- .../Implementations/MultiServicePushNotificationService.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Core/Services/Implementations/MultiServicePushNotificationService.cs b/src/Core/Services/Implementations/MultiServicePushNotificationService.cs index c10037837d93..86639db66f56 100644 --- a/src/Core/Services/Implementations/MultiServicePushNotificationService.cs +++ b/src/Core/Services/Implementations/MultiServicePushNotificationService.cs @@ -9,14 +9,14 @@ namespace Bit.Core.Services; public class MultiServicePushNotificationService : IPushNotificationService { - private readonly List _services = new List(); + private readonly IEnumerable _services; private readonly ILogger _logger; public MultiServicePushNotificationService( [FromKeyedServices("implementation")] IEnumerable services, ILogger logger) { - _services.AddRange(services); + _services = services; _logger = logger; } From 640e9a03e74fed466bb3ce742a6cbcc6f5dd06df Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Tue, 9 Jul 2024 06:22:44 -0700 Subject: [PATCH 17/31] Pascal case template holes --- src/Core/NotificationHub/NotificationHubPool.cs | 2 +- .../NotificationHub/NotificationHubPushNotificationService.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Core/NotificationHub/NotificationHubPool.cs b/src/Core/NotificationHub/NotificationHubPool.cs index 0837a0079a72..b1ee9fe196ad 100644 --- a/src/Core/NotificationHub/NotificationHubPool.cs +++ b/src/Core/NotificationHub/NotificationHubPool.cs @@ -25,7 +25,7 @@ private List FilterInvalidHubs(IEnumerable Date: Tue, 9 Jul 2024 06:33:44 -0700 Subject: [PATCH 18/31] Name TryParse methods TryParse --- src/Core/Models/Data/InstallationDeviceEntity.cs | 8 +++++--- .../NotificationHubPushRegistrationService.cs | 9 +++++++-- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/Core/Models/Data/InstallationDeviceEntity.cs b/src/Core/Models/Data/InstallationDeviceEntity.cs index dcd2ae2759a7..a3d960b242c0 100644 --- a/src/Core/Models/Data/InstallationDeviceEntity.cs +++ b/src/Core/Models/Data/InstallationDeviceEntity.cs @@ -37,10 +37,11 @@ public static bool IsInstallationDeviceId(string deviceId) { return deviceId != null && deviceId.Length == 73 && deviceId[36] == '_'; } - public static bool TrySplit(string deviceId, out Guid installationId, out Guid deviceIdGuid) + public static bool TryParse(string deviceId, out InstallationDeviceEntity installationDeviceEntity) { - installationId = Guid.Empty; - deviceIdGuid = Guid.Empty; + installationDeviceEntity = null; + var installationId = Guid.Empty; + var deviceIdGuid = Guid.Empty; if (!IsInstallationDeviceId(deviceId)) { return false; @@ -54,6 +55,7 @@ public static bool TrySplit(string deviceId, out Guid installationId, out Guid d { return false; } + installationDeviceEntity = new InstallationDeviceEntity(installationId, deviceIdGuid); return true; } } diff --git a/src/Core/NotificationHub/NotificationHubPushRegistrationService.cs b/src/Core/NotificationHub/NotificationHubPushRegistrationService.cs index a3051815c625..ae32babf4477 100644 --- a/src/Core/NotificationHub/NotificationHubPushRegistrationService.cs +++ b/src/Core/NotificationHub/NotificationHubPushRegistrationService.cs @@ -216,11 +216,16 @@ private NotificationHubClient ClientFor(Guid deviceId) private Guid GetComb(string deviceId) { + var deviceIdString = deviceId; + InstallationDeviceEntity installationDeviceEntity; Guid deviceIdGuid; - if (InstallationDeviceEntity.TrySplit(deviceId, out _, out deviceIdGuid)) + if (InstallationDeviceEntity.TryParse(deviceIdString, out installationDeviceEntity)) { + // Strip off the installation id (PartitionId). RowKey is the ID in the Installation's table. + deviceIdString = installationDeviceEntity.RowKey; } - else if (Guid.TryParse(deviceId, out deviceIdGuid)) + + if (Guid.TryParse(deviceIdString, out deviceIdGuid)) { } else From 3a6b1285a644f958a7400295429360ff0dec61d3 Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Tue, 9 Jul 2024 09:35:30 -0400 Subject: [PATCH 19/31] Apply suggestions from code review Co-authored-by: Justin Baur <19896123+justindbaur@users.noreply.github.com> --- src/Core/NotificationHub/INotificationHubClientProxy.cs | 2 +- src/Core/NotificationHub/NotificationHubClientProxy.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Core/NotificationHub/INotificationHubClientProxy.cs b/src/Core/NotificationHub/INotificationHubClientProxy.cs index ab499872dcfb..82b4d3959107 100644 --- a/src/Core/NotificationHub/INotificationHubClientProxy.cs +++ b/src/Core/NotificationHub/INotificationHubClientProxy.cs @@ -4,5 +4,5 @@ namespace Bit.Core.NotificationHub; public interface INotificationHubProxy { - Task<(INotificationHubClient client, NotificationOutcome outcome)[]> SendTemplateNotificationAsync(IDictionary properties, string tagExpression); + Task<(INotificationHubClient Client, NotificationOutcome Outcome)[]> SendTemplateNotificationAsync(IDictionary properties, string tagExpression); } diff --git a/src/Core/NotificationHub/NotificationHubClientProxy.cs b/src/Core/NotificationHub/NotificationHubClientProxy.cs index 81638587e959..815ac8836393 100644 --- a/src/Core/NotificationHub/NotificationHubClientProxy.cs +++ b/src/Core/NotificationHub/NotificationHubClientProxy.cs @@ -19,7 +19,7 @@ public NotificationHubClientProxy(IEnumerable clients) // partial proxy of INotificationHubClient implementation // Note: Any other methods that are needed can simply be delegated as done here. - public async Task<(INotificationHubClient client, NotificationOutcome outcome)[]> SendTemplateNotificationAsync(IDictionary properties, string tagExpression) + public async Task<(INotificationHubClient Client, NotificationOutcome Outcome)[]> SendTemplateNotificationAsync(IDictionary properties, string tagExpression) { return await ApplyToAllClientsAsync(async c => await c.SendTemplateNotificationAsync(properties, tagExpression)); } From e9a10bc7c0f5ddc88818b5b7f56c23316a50470f Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Fri, 19 Jul 2024 07:48:50 -0700 Subject: [PATCH 20/31] AllClients is a set of clients and must be deduplicated --- src/Core/NotificationHub/NotificationHubPool.cs | 4 ++-- src/Core/Settings/GlobalSettings.cs | 2 +- .../NotificationHub/NotificationHubPoolTests.cs | 10 +++++----- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Core/NotificationHub/NotificationHubPool.cs b/src/Core/NotificationHub/NotificationHubPool.cs index b1ee9fe196ad..3b1dbb786a96 100644 --- a/src/Core/NotificationHub/NotificationHubPool.cs +++ b/src/Core/NotificationHub/NotificationHubPool.cs @@ -13,8 +13,8 @@ public class NotificationHubPool : INotificationHubPool public NotificationHubPool(ILogger logger, GlobalSettings globalSettings) { _logger = logger; - _connections = FilterInvalidHubs(globalSettings.NotificationHubPool.NotificationHubSettings); - _clients = _connections.Select(c => c.HubClient); + _connections = FilterInvalidHubs(globalSettings.NotificationHubPool.NotificationHubs); + _clients = _connections.GroupBy(c => c.ConnectionString).Select(g => g.First().HubClient); } private List FilterInvalidHubs(IEnumerable hubs) diff --git a/src/Core/Settings/GlobalSettings.cs b/src/Core/Settings/GlobalSettings.cs index d83748db4928..78de02bc4f44 100644 --- a/src/Core/Settings/GlobalSettings.cs +++ b/src/Core/Settings/GlobalSettings.cs @@ -450,7 +450,7 @@ public class NotificationHubPoolSettings /// /// Note that hubs on the same namespace share active device limits, so multiple namespaces should be used to increase capacity. /// - public List NotificationHubSettings { get; set; } = new(); + public List NotificationHubs { get; set; } = new(); } public class YubicoSettings diff --git a/test/Core.Test/NotificationHub/NotificationHubPoolTests.cs b/test/Core.Test/NotificationHub/NotificationHubPoolTests.cs index c3cceeb26b81..d542b51bfaad 100644 --- a/test/Core.Test/NotificationHub/NotificationHubPoolTests.cs +++ b/test/Core.Test/NotificationHub/NotificationHubPoolTests.cs @@ -18,7 +18,7 @@ public void NotificationHubPool_WarnsOnMissingConnectionString() { NotificationHubPool = new NotificationHubPoolSettings() { - NotificationHubSettings = new() { + NotificationHubs = new() { new() { ConnectionString = null, HubName = "hub", @@ -48,7 +48,7 @@ public void NotificationHubPool_WarnsOnMissingHubName() { NotificationHubPool = new NotificationHubPoolSettings() { - NotificationHubSettings = new() { + NotificationHubs = new() { new() { ConnectionString = "connection", HubName = null, @@ -78,7 +78,7 @@ public void NotificationHubPool_ClientFor_ThrowsOnNoValidHubs() { NotificationHubPool = new NotificationHubPoolSettings() { - NotificationHubSettings = new() { + NotificationHubs = new() { new() { ConnectionString = "connection", HubName = "hub", @@ -106,7 +106,7 @@ public void NotificationHubPool_ClientFor_ReturnsClient() { NotificationHubPool = new NotificationHubPoolSettings() { - NotificationHubSettings = new() { + NotificationHubs = new() { new() { ConnectionString = "Endpoint=sb://example.servicebus.windows.net/;SharedAccessKey=example///example=", HubName = "hub", @@ -134,7 +134,7 @@ public void NotificationHubPool_AllClients_ReturnsProxy() { NotificationHubPool = new NotificationHubPoolSettings() { - NotificationHubSettings = new() { + NotificationHubs = new() { new() { ConnectionString = "connection", HubName = "hub", From 8f36b4a3e14fcb247cdb12c8c58d87591baa01ab Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Thu, 8 Aug 2024 14:31:01 -0700 Subject: [PATCH 21/31] Fix registration start time --- src/Core/NotificationHub/NotificationHubConnection.cs | 2 +- test/Core.Test/NotificationHub/NotificationHubPoolTests.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Core/NotificationHub/NotificationHubConnection.cs b/src/Core/NotificationHub/NotificationHubConnection.cs index eed38e8b27db..cd5144f219e2 100644 --- a/src/Core/NotificationHub/NotificationHubConnection.cs +++ b/src/Core/NotificationHub/NotificationHubConnection.cs @@ -84,7 +84,7 @@ public bool RegistrationEnabled(DateTime queryTime) return false; } - return RegistrationStartDate <= queryTime; + return RegistrationStartDate < queryTime; } private NotificationHubConnection() { } diff --git a/test/Core.Test/NotificationHub/NotificationHubPoolTests.cs b/test/Core.Test/NotificationHub/NotificationHubPoolTests.cs index d542b51bfaad..dd9afb867ee1 100644 --- a/test/Core.Test/NotificationHub/NotificationHubPoolTests.cs +++ b/test/Core.Test/NotificationHub/NotificationHubPoolTests.cs @@ -110,7 +110,7 @@ public void NotificationHubPool_ClientFor_ReturnsClient() new() { ConnectionString = "Endpoint=sb://example.servicebus.windows.net/;SharedAccessKey=example///example=", HubName = "hub", - RegistrationStartDate = DateTime.UtcNow, + RegistrationStartDate = DateTime.UtcNow.AddMinutes(-1), RegistrationEndDate = DateTime.UtcNow.AddDays(1), } } From 3ae86e01df29a8c3d7e222e41516c56ceb65ab52 Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Wed, 18 Sep 2024 12:37:30 -0700 Subject: [PATCH 22/31] Add logging to initialization of a notification hub --- src/Core/NotificationHub/NotificationHubConnection.cs | 8 ++++++++ src/Core/NotificationHub/NotificationHubPool.cs | 1 + 2 files changed, 9 insertions(+) diff --git a/src/Core/NotificationHub/NotificationHubConnection.cs b/src/Core/NotificationHub/NotificationHubConnection.cs index cd5144f219e2..3a1437f70c52 100644 --- a/src/Core/NotificationHub/NotificationHubConnection.cs +++ b/src/Core/NotificationHub/NotificationHubConnection.cs @@ -60,6 +60,14 @@ public bool IsValid } } + public string LogString + { + get + { + return $"HubName: {HubName}, EnableSendTracing: {EnableSendTracing}, RegistrationStartDate: {RegistrationStartDate}, RegistrationEndDate: {RegistrationEndDate}"; + } + } + /// /// Gets whether registration is enabled for the given comb ID. /// This is based off of the generation time encoded in the comb ID. diff --git a/src/Core/NotificationHub/NotificationHubPool.cs b/src/Core/NotificationHub/NotificationHubPool.cs index 3b1dbb786a96..81ac798c471d 100644 --- a/src/Core/NotificationHub/NotificationHubPool.cs +++ b/src/Core/NotificationHub/NotificationHubPool.cs @@ -28,6 +28,7 @@ private List FilterInvalidHubs(IEnumerable Date: Wed, 18 Sep 2024 13:52:36 -0700 Subject: [PATCH 23/31] more logging --- src/Core/NotificationHub/NotificationHubPool.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Core/NotificationHub/NotificationHubPool.cs b/src/Core/NotificationHub/NotificationHubPool.cs index 81ac798c471d..3b865171c5a4 100644 --- a/src/Core/NotificationHub/NotificationHubPool.cs +++ b/src/Core/NotificationHub/NotificationHubPool.cs @@ -20,6 +20,7 @@ public NotificationHubPool(ILogger logger, GlobalSettings g private List FilterInvalidHubs(IEnumerable hubs) { List result = new(); + _logger.LogDebug("Filtering {HubCount} notification hubs", hubs.Count()); foreach (var hub in hubs) { var connection = NotificationHubConnection.From(hub); From 2fc81ad30063b71055eb328c3514f5fc2935dc80 Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Thu, 19 Sep 2024 08:51:35 -0700 Subject: [PATCH 24/31] Add lower level logging for hub settings --- .../MultiServicePushNotificationService.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/Core/Services/Implementations/MultiServicePushNotificationService.cs b/src/Core/Services/Implementations/MultiServicePushNotificationService.cs index 86639db66f56..00be72c980e0 100644 --- a/src/Core/Services/Implementations/MultiServicePushNotificationService.cs +++ b/src/Core/Services/Implementations/MultiServicePushNotificationService.cs @@ -1,5 +1,6 @@ using Bit.Core.Auth.Entities; using Bit.Core.Enums; +using Bit.Core.Settings; using Bit.Core.Tools.Entities; using Bit.Core.Vault.Entities; using Microsoft.Extensions.DependencyInjection; @@ -14,11 +15,17 @@ public class MultiServicePushNotificationService : IPushNotificationService public MultiServicePushNotificationService( [FromKeyedServices("implementation")] IEnumerable services, - ILogger logger) + ILogger logger, + GlobalSettings globalSettings) { _services = services; _logger = logger; + _logger.LogInformation("Hub services: {Services}", _services.Count()); + globalSettings?.NotificationHubPool?.NotificationHubs?.ForEach(hub => + { + _logger.LogInformation("HubName: {HubName}, EnableSendTracing: {EnableSendTracing}, RegistrationStartDate: {RegistrationStartDate}, RegistrationEndDate: {RegistrationEndDate}", hub.HubName, hub.EnableSendTracing, hub.RegistrationStartDate, hub.RegistrationEndDate); + }); } public Task PushSyncCipherCreateAsync(Cipher cipher, IEnumerable collectionIds) From 5e581741df17e4a321ce8746a2b3ac8a2869edbb Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Thu, 19 Sep 2024 10:54:56 -0700 Subject: [PATCH 25/31] Log when connection is resolved --- src/Core/NotificationHub/NotificationHubPool.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Core/NotificationHub/NotificationHubPool.cs b/src/Core/NotificationHub/NotificationHubPool.cs index 3b865171c5a4..e2ccc7b3f255 100644 --- a/src/Core/NotificationHub/NotificationHubPool.cs +++ b/src/Core/NotificationHub/NotificationHubPool.cs @@ -53,7 +53,9 @@ public NotificationHubClient ClientFor(Guid comb) $"Hub start and end times are configured as follows:\n" + string.Join("\n", _connections.Select(c => $"Hub {c.HubName} - Start: {c.RegistrationStartDate}, End: {c.RegistrationEndDate}"))); } - return possibleConnections[CoreHelpers.BinForComb(comb, possibleConnections.Length)].HubClient; + var resolvedConnection = possibleConnections[CoreHelpers.BinForComb(comb, possibleConnections.Length)]; + _logger.LogDebug("Resolved notification hub for comb {Comb}\n{ConnectionInfo}", comb, resolvedConnection.LogString); + return resolvedConnection.HubClient; } public INotificationHubProxy AllClients { get { return new NotificationHubClientProxy(_clients); } } From d81e4121b6321cf2cdd665ab5078daecd8581ad3 Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Thu, 19 Sep 2024 11:07:24 -0700 Subject: [PATCH 26/31] Improve log message --- src/Core/NotificationHub/NotificationHubPool.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Core/NotificationHub/NotificationHubPool.cs b/src/Core/NotificationHub/NotificationHubPool.cs index e2ccc7b3f255..fd851051b534 100644 --- a/src/Core/NotificationHub/NotificationHubPool.cs +++ b/src/Core/NotificationHub/NotificationHubPool.cs @@ -54,7 +54,7 @@ public NotificationHubClient ClientFor(Guid comb) string.Join("\n", _connections.Select(c => $"Hub {c.HubName} - Start: {c.RegistrationStartDate}, End: {c.RegistrationEndDate}"))); } var resolvedConnection = possibleConnections[CoreHelpers.BinForComb(comb, possibleConnections.Length)]; - _logger.LogDebug("Resolved notification hub for comb {Comb}\n{ConnectionInfo}", comb, resolvedConnection.LogString); + _logger.LogDebug("Resolved notification hub for comb {Comb} out of {HubCount} hubs.\n{ConnectionInfo}", comb, possibleConnections.Length, resolvedConnection.LogString); return resolvedConnection.HubClient; } From ac0371d260e3a3c9214bc5448c022cf9793fe555 Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Thu, 19 Sep 2024 12:07:09 -0700 Subject: [PATCH 27/31] Log pushes to notification hub --- .../NotificationHub/NotificationHubPushNotificationService.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Core/NotificationHub/NotificationHubPushNotificationService.cs b/src/Core/NotificationHub/NotificationHubPushNotificationService.cs index 6143676deffb..a102878f88f4 100644 --- a/src/Core/NotificationHub/NotificationHubPushNotificationService.cs +++ b/src/Core/NotificationHub/NotificationHubPushNotificationService.cs @@ -256,6 +256,7 @@ private async Task SendPayloadAsync(string tag, PushType type, object payload) { "type", ((byte)type).ToString() }, { "payload", JsonSerializer.Serialize(payload) } }, tag); + _logger.LogDebug("Pushed notifications to {Tag} to all hubs", tag); if (_enableTracing) { From dd9339c1f50a010c1efd2d1c8faf4d207f21fb00 Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Thu, 19 Sep 2024 13:22:53 -0700 Subject: [PATCH 28/31] temporarily elevate log messages for visibility --- src/Core/NotificationHub/NotificationHubPool.cs | 6 +++--- .../NotificationHubPushNotificationService.cs | 2 +- .../Implementations/MultiServicePushNotificationService.cs | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Core/NotificationHub/NotificationHubPool.cs b/src/Core/NotificationHub/NotificationHubPool.cs index fd851051b534..d16d762159b3 100644 --- a/src/Core/NotificationHub/NotificationHubPool.cs +++ b/src/Core/NotificationHub/NotificationHubPool.cs @@ -20,7 +20,7 @@ public NotificationHubPool(ILogger logger, GlobalSettings g private List FilterInvalidHubs(IEnumerable hubs) { List result = new(); - _logger.LogDebug("Filtering {HubCount} notification hubs", hubs.Count()); + _logger.LogError("Filtering {HubCount} notification hubs", hubs.Count()); foreach (var hub in hubs) { var connection = NotificationHubConnection.From(hub); @@ -29,7 +29,7 @@ private List FilterInvalidHubs(IEnumerable $"Hub {c.HubName} - Start: {c.RegistrationStartDate}, End: {c.RegistrationEndDate}"))); } var resolvedConnection = possibleConnections[CoreHelpers.BinForComb(comb, possibleConnections.Length)]; - _logger.LogDebug("Resolved notification hub for comb {Comb} out of {HubCount} hubs.\n{ConnectionInfo}", comb, possibleConnections.Length, resolvedConnection.LogString); + _logger.LogError("Resolved notification hub for comb {Comb} out of {HubCount} hubs.\n{ConnectionInfo}", comb, possibleConnections.Length, resolvedConnection.LogString); return resolvedConnection.HubClient; } diff --git a/src/Core/NotificationHub/NotificationHubPushNotificationService.cs b/src/Core/NotificationHub/NotificationHubPushNotificationService.cs index a102878f88f4..cdd9ea13dabf 100644 --- a/src/Core/NotificationHub/NotificationHubPushNotificationService.cs +++ b/src/Core/NotificationHub/NotificationHubPushNotificationService.cs @@ -256,7 +256,7 @@ private async Task SendPayloadAsync(string tag, PushType type, object payload) { "type", ((byte)type).ToString() }, { "payload", JsonSerializer.Serialize(payload) } }, tag); - _logger.LogDebug("Pushed notifications to {Tag} to all hubs", tag); + _logger.LogError("Pushed notifications to {Tag} to all hubs", tag); if (_enableTracing) { diff --git a/src/Core/Services/Implementations/MultiServicePushNotificationService.cs b/src/Core/Services/Implementations/MultiServicePushNotificationService.cs index 00be72c980e0..2777c85a991f 100644 --- a/src/Core/Services/Implementations/MultiServicePushNotificationService.cs +++ b/src/Core/Services/Implementations/MultiServicePushNotificationService.cs @@ -21,10 +21,10 @@ public MultiServicePushNotificationService( _services = services; _logger = logger; - _logger.LogInformation("Hub services: {Services}", _services.Count()); + _logger.LogError("Hub services: {Services}", _services.Count()); globalSettings?.NotificationHubPool?.NotificationHubs?.ForEach(hub => { - _logger.LogInformation("HubName: {HubName}, EnableSendTracing: {EnableSendTracing}, RegistrationStartDate: {RegistrationStartDate}, RegistrationEndDate: {RegistrationEndDate}", hub.HubName, hub.EnableSendTracing, hub.RegistrationStartDate, hub.RegistrationEndDate); + _logger.LogError("HubName: {HubName}, EnableSendTracing: {EnableSendTracing}, RegistrationStartDate: {RegistrationStartDate}, RegistrationEndDate: {RegistrationEndDate}", hub.HubName, hub.EnableSendTracing, hub.RegistrationStartDate, hub.RegistrationEndDate); }); } From ed5183b84d90beec3a952b0f7bcb0d468ae83136 Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Thu, 19 Sep 2024 14:40:07 -0700 Subject: [PATCH 29/31] Log in multi-service when relaying to another push service --- .../NotificationHub/NotificationHubPushNotificationService.cs | 1 + src/Core/Services/IPushNotificationService.cs | 1 + .../Implementations/AzureQueuePushNotificationService.cs | 1 + .../Implementations/MultiServicePushNotificationService.cs | 2 ++ .../Implementations/NotificationsApiPushNotificationService.cs | 1 + .../Services/Implementations/RelayPushNotificationService.cs | 1 + .../Services/NoopImplementations/NoopPushNotificationService.cs | 2 ++ 7 files changed, 9 insertions(+) diff --git a/src/Core/NotificationHub/NotificationHubPushNotificationService.cs b/src/Core/NotificationHub/NotificationHubPushNotificationService.cs index cdd9ea13dabf..a46240aa983f 100644 --- a/src/Core/NotificationHub/NotificationHubPushNotificationService.cs +++ b/src/Core/NotificationHub/NotificationHubPushNotificationService.cs @@ -21,6 +21,7 @@ public class NotificationHubPushNotificationService : IPushNotificationService private readonly bool _enableTracing = false; private readonly INotificationHubPool _notificationHubPool; private readonly ILogger _logger; + public string PushServiceType => "NotificationHub"; public NotificationHubPushNotificationService( IInstallationDeviceRepository installationDeviceRepository, diff --git a/src/Core/Services/IPushNotificationService.cs b/src/Core/Services/IPushNotificationService.cs index 29a20239d1ac..a83e6ea5a9f7 100644 --- a/src/Core/Services/IPushNotificationService.cs +++ b/src/Core/Services/IPushNotificationService.cs @@ -7,6 +7,7 @@ namespace Bit.Core.Services; public interface IPushNotificationService { + string PushServiceType { get; } Task PushSyncCipherCreateAsync(Cipher cipher, IEnumerable collectionIds); Task PushSyncCipherUpdateAsync(Cipher cipher, IEnumerable collectionIds); Task PushSyncCipherDeleteAsync(Cipher cipher); diff --git a/src/Core/Services/Implementations/AzureQueuePushNotificationService.cs b/src/Core/Services/Implementations/AzureQueuePushNotificationService.cs index 1e4a7314c457..4ecdac9ae77c 100644 --- a/src/Core/Services/Implementations/AzureQueuePushNotificationService.cs +++ b/src/Core/Services/Implementations/AzureQueuePushNotificationService.cs @@ -17,6 +17,7 @@ public class AzureQueuePushNotificationService : IPushNotificationService private readonly QueueClient _queueClient; private readonly GlobalSettings _globalSettings; private readonly IHttpContextAccessor _httpContextAccessor; + public string PushServiceType => "AzureQueue"; public AzureQueuePushNotificationService( GlobalSettings globalSettings, diff --git a/src/Core/Services/Implementations/MultiServicePushNotificationService.cs b/src/Core/Services/Implementations/MultiServicePushNotificationService.cs index 2777c85a991f..5040036ebbce 100644 --- a/src/Core/Services/Implementations/MultiServicePushNotificationService.cs +++ b/src/Core/Services/Implementations/MultiServicePushNotificationService.cs @@ -12,6 +12,7 @@ public class MultiServicePushNotificationService : IPushNotificationService { private readonly IEnumerable _services; private readonly ILogger _logger; + public string PushServiceType => "MultiService"; public MultiServicePushNotificationService( [FromKeyedServices("implementation")] IEnumerable services, @@ -150,6 +151,7 @@ private void PushToServices(Func pushFunc) { foreach (var service in _services) { + _logger.LogError("Relaying push to service: {ServiceType}", service.PushServiceType); pushFunc(service); } } diff --git a/src/Core/Services/Implementations/NotificationsApiPushNotificationService.cs b/src/Core/Services/Implementations/NotificationsApiPushNotificationService.cs index 9ec1eb31d4d1..bb6e6abccb9a 100644 --- a/src/Core/Services/Implementations/NotificationsApiPushNotificationService.cs +++ b/src/Core/Services/Implementations/NotificationsApiPushNotificationService.cs @@ -14,6 +14,7 @@ public class NotificationsApiPushNotificationService : BaseIdentityClientService { private readonly GlobalSettings _globalSettings; private readonly IHttpContextAccessor _httpContextAccessor; + public string PushServiceType => "NotificationsApi"; public NotificationsApiPushNotificationService( IHttpClientFactory httpFactory, diff --git a/src/Core/Services/Implementations/RelayPushNotificationService.cs b/src/Core/Services/Implementations/RelayPushNotificationService.cs index 6cfc0c0a6153..c467c6e09e3b 100644 --- a/src/Core/Services/Implementations/RelayPushNotificationService.cs +++ b/src/Core/Services/Implementations/RelayPushNotificationService.cs @@ -17,6 +17,7 @@ public class RelayPushNotificationService : BaseIdentityClientService, IPushNoti { private readonly IDeviceRepository _deviceRepository; private readonly IHttpContextAccessor _httpContextAccessor; + public string PushServiceType => "Relay"; public RelayPushNotificationService( IHttpClientFactory httpFactory, diff --git a/src/Core/Services/NoopImplementations/NoopPushNotificationService.cs b/src/Core/Services/NoopImplementations/NoopPushNotificationService.cs index d4eff93ef65e..c7a6f03c0a3f 100644 --- a/src/Core/Services/NoopImplementations/NoopPushNotificationService.cs +++ b/src/Core/Services/NoopImplementations/NoopPushNotificationService.cs @@ -7,6 +7,8 @@ namespace Bit.Core.Services; public class NoopPushNotificationService : IPushNotificationService { + public string PushServiceType => "Noop"; + public Task PushSyncCipherCreateAsync(Cipher cipher, IEnumerable collectionIds) { return Task.FromResult(0); From 1885e6b78519a177f40e2e0ed891f5d2ee0918ef Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Wed, 16 Oct 2024 14:30:51 -0700 Subject: [PATCH 30/31] Revert to more reasonable logging free of user information --- src/Core/NotificationHub/NotificationHubPool.cs | 6 +++--- .../NotificationHubPushNotificationService.cs | 2 -- src/Core/Services/IPushNotificationService.cs | 1 - .../Implementations/AzureQueuePushNotificationService.cs | 1 - .../Implementations/MultiServicePushNotificationService.cs | 6 ++---- .../NotificationsApiPushNotificationService.cs | 1 - .../Implementations/RelayPushNotificationService.cs | 1 - .../NoopImplementations/NoopPushNotificationService.cs | 2 -- test/Common/AutoFixture/ControllerCustomization.cs | 1 - 9 files changed, 5 insertions(+), 16 deletions(-) diff --git a/src/Core/NotificationHub/NotificationHubPool.cs b/src/Core/NotificationHub/NotificationHubPool.cs index d16d762159b3..7448aad5bda1 100644 --- a/src/Core/NotificationHub/NotificationHubPool.cs +++ b/src/Core/NotificationHub/NotificationHubPool.cs @@ -20,7 +20,7 @@ public NotificationHubPool(ILogger logger, GlobalSettings g private List FilterInvalidHubs(IEnumerable hubs) { List result = new(); - _logger.LogError("Filtering {HubCount} notification hubs", hubs.Count()); + _logger.LogDebug("Filtering {HubCount} notification hubs", hubs.Count()); foreach (var hub in hubs) { var connection = NotificationHubConnection.From(hub); @@ -29,7 +29,7 @@ private List FilterInvalidHubs(IEnumerable $"Hub {c.HubName} - Start: {c.RegistrationStartDate}, End: {c.RegistrationEndDate}"))); } var resolvedConnection = possibleConnections[CoreHelpers.BinForComb(comb, possibleConnections.Length)]; - _logger.LogError("Resolved notification hub for comb {Comb} out of {HubCount} hubs.\n{ConnectionInfo}", comb, possibleConnections.Length, resolvedConnection.LogString); + _logger.LogTrace("Resolved notification hub for comb {Comb} out of {HubCount} hubs.\n{ConnectionInfo}", comb, possibleConnections.Length, resolvedConnection.LogString); return resolvedConnection.HubClient; } diff --git a/src/Core/NotificationHub/NotificationHubPushNotificationService.cs b/src/Core/NotificationHub/NotificationHubPushNotificationService.cs index a46240aa983f..6143676deffb 100644 --- a/src/Core/NotificationHub/NotificationHubPushNotificationService.cs +++ b/src/Core/NotificationHub/NotificationHubPushNotificationService.cs @@ -21,7 +21,6 @@ public class NotificationHubPushNotificationService : IPushNotificationService private readonly bool _enableTracing = false; private readonly INotificationHubPool _notificationHubPool; private readonly ILogger _logger; - public string PushServiceType => "NotificationHub"; public NotificationHubPushNotificationService( IInstallationDeviceRepository installationDeviceRepository, @@ -257,7 +256,6 @@ private async Task SendPayloadAsync(string tag, PushType type, object payload) { "type", ((byte)type).ToString() }, { "payload", JsonSerializer.Serialize(payload) } }, tag); - _logger.LogError("Pushed notifications to {Tag} to all hubs", tag); if (_enableTracing) { diff --git a/src/Core/Services/IPushNotificationService.cs b/src/Core/Services/IPushNotificationService.cs index a83e6ea5a9f7..29a20239d1ac 100644 --- a/src/Core/Services/IPushNotificationService.cs +++ b/src/Core/Services/IPushNotificationService.cs @@ -7,7 +7,6 @@ namespace Bit.Core.Services; public interface IPushNotificationService { - string PushServiceType { get; } Task PushSyncCipherCreateAsync(Cipher cipher, IEnumerable collectionIds); Task PushSyncCipherUpdateAsync(Cipher cipher, IEnumerable collectionIds); Task PushSyncCipherDeleteAsync(Cipher cipher); diff --git a/src/Core/Services/Implementations/AzureQueuePushNotificationService.cs b/src/Core/Services/Implementations/AzureQueuePushNotificationService.cs index 4ecdac9ae77c..1e4a7314c457 100644 --- a/src/Core/Services/Implementations/AzureQueuePushNotificationService.cs +++ b/src/Core/Services/Implementations/AzureQueuePushNotificationService.cs @@ -17,7 +17,6 @@ public class AzureQueuePushNotificationService : IPushNotificationService private readonly QueueClient _queueClient; private readonly GlobalSettings _globalSettings; private readonly IHttpContextAccessor _httpContextAccessor; - public string PushServiceType => "AzureQueue"; public AzureQueuePushNotificationService( GlobalSettings globalSettings, diff --git a/src/Core/Services/Implementations/MultiServicePushNotificationService.cs b/src/Core/Services/Implementations/MultiServicePushNotificationService.cs index 5040036ebbce..00be72c980e0 100644 --- a/src/Core/Services/Implementations/MultiServicePushNotificationService.cs +++ b/src/Core/Services/Implementations/MultiServicePushNotificationService.cs @@ -12,7 +12,6 @@ public class MultiServicePushNotificationService : IPushNotificationService { private readonly IEnumerable _services; private readonly ILogger _logger; - public string PushServiceType => "MultiService"; public MultiServicePushNotificationService( [FromKeyedServices("implementation")] IEnumerable services, @@ -22,10 +21,10 @@ public MultiServicePushNotificationService( _services = services; _logger = logger; - _logger.LogError("Hub services: {Services}", _services.Count()); + _logger.LogInformation("Hub services: {Services}", _services.Count()); globalSettings?.NotificationHubPool?.NotificationHubs?.ForEach(hub => { - _logger.LogError("HubName: {HubName}, EnableSendTracing: {EnableSendTracing}, RegistrationStartDate: {RegistrationStartDate}, RegistrationEndDate: {RegistrationEndDate}", hub.HubName, hub.EnableSendTracing, hub.RegistrationStartDate, hub.RegistrationEndDate); + _logger.LogInformation("HubName: {HubName}, EnableSendTracing: {EnableSendTracing}, RegistrationStartDate: {RegistrationStartDate}, RegistrationEndDate: {RegistrationEndDate}", hub.HubName, hub.EnableSendTracing, hub.RegistrationStartDate, hub.RegistrationEndDate); }); } @@ -151,7 +150,6 @@ private void PushToServices(Func pushFunc) { foreach (var service in _services) { - _logger.LogError("Relaying push to service: {ServiceType}", service.PushServiceType); pushFunc(service); } } diff --git a/src/Core/Services/Implementations/NotificationsApiPushNotificationService.cs b/src/Core/Services/Implementations/NotificationsApiPushNotificationService.cs index bb6e6abccb9a..9ec1eb31d4d1 100644 --- a/src/Core/Services/Implementations/NotificationsApiPushNotificationService.cs +++ b/src/Core/Services/Implementations/NotificationsApiPushNotificationService.cs @@ -14,7 +14,6 @@ public class NotificationsApiPushNotificationService : BaseIdentityClientService { private readonly GlobalSettings _globalSettings; private readonly IHttpContextAccessor _httpContextAccessor; - public string PushServiceType => "NotificationsApi"; public NotificationsApiPushNotificationService( IHttpClientFactory httpFactory, diff --git a/src/Core/Services/Implementations/RelayPushNotificationService.cs b/src/Core/Services/Implementations/RelayPushNotificationService.cs index c467c6e09e3b..6cfc0c0a6153 100644 --- a/src/Core/Services/Implementations/RelayPushNotificationService.cs +++ b/src/Core/Services/Implementations/RelayPushNotificationService.cs @@ -17,7 +17,6 @@ public class RelayPushNotificationService : BaseIdentityClientService, IPushNoti { private readonly IDeviceRepository _deviceRepository; private readonly IHttpContextAccessor _httpContextAccessor; - public string PushServiceType => "Relay"; public RelayPushNotificationService( IHttpClientFactory httpFactory, diff --git a/src/Core/Services/NoopImplementations/NoopPushNotificationService.cs b/src/Core/Services/NoopImplementations/NoopPushNotificationService.cs index c7a6f03c0a3f..d4eff93ef65e 100644 --- a/src/Core/Services/NoopImplementations/NoopPushNotificationService.cs +++ b/src/Core/Services/NoopImplementations/NoopPushNotificationService.cs @@ -7,8 +7,6 @@ namespace Bit.Core.Services; public class NoopPushNotificationService : IPushNotificationService { - public string PushServiceType => "Noop"; - public Task PushSyncCipherCreateAsync(Cipher cipher, IEnumerable collectionIds) { return Task.FromResult(0); diff --git a/test/Common/AutoFixture/ControllerCustomization.cs b/test/Common/AutoFixture/ControllerCustomization.cs index f695f86b5503..9faffa446102 100644 --- a/test/Common/AutoFixture/ControllerCustomization.cs +++ b/test/Common/AutoFixture/ControllerCustomization.cs @@ -1,6 +1,5 @@ using AutoFixture; using Microsoft.AspNetCore.Mvc; -using Org.BouncyCastle.Security; namespace Bit.Test.Common.AutoFixture; From 09dbb0843c0aea23cd58f836306a024c583d476a Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Wed, 16 Oct 2024 14:44:25 -0700 Subject: [PATCH 31/31] Fixup merge Deleting user was extracted to a command in #4803, this updates that work to use just the device ids as I did elsewhere in abd67e8ec --- .../OrganizationUsers/RemoveOrganizationUserCommand.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RemoveOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RemoveOrganizationUserCommand.cs index 09444306e6e5..e6d56ea878a2 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RemoveOrganizationUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/RemoveOrganizationUserCommand.cs @@ -162,12 +162,12 @@ private async Task RepositoryDeleteUserAsync(OrganizationUser orgUser, Guid? del } } - private async Task>> GetUserDeviceIdsAsync(Guid userId) + private async Task> GetUserDeviceIdsAsync(Guid userId) { var devices = await _deviceRepository.GetManyByUserIdAsync(userId); return devices .Where(d => !string.IsNullOrWhiteSpace(d.PushToken)) - .Select(d => new KeyValuePair(d.Id.ToString(), d.Type)); + .Select(d => d.Id.ToString()); } private async Task DeleteAndPushUserRegistrationAsync(Guid organizationId, Guid userId)