1
0
mirror of https://github.com/bitwarden/server synced 2025-12-15 15:53:59 +00:00

[PM-19659] Clean up Notifications code (#6244)

* Move PushType to Platform Folder

- Move the PushType next to the rest of push notification code
- Specifically exclude it from needing Platform code review
- Add tests establishing rules Platform has for usage of this enum, making it safe to have no owner

* Move NotificationHub code into Platform/Push directory

* Update NotificationHub namespace imports

* Add attribute for storing push type metadata

* Rename Push Engines to have PushEngine suffix

* Move Push Registration items to their own directory

* Push code move

* Add expected usage comment

* Add Push feature registration method

- Make method able to be called multipes times with no ill effects

* Add Push Registration service entrypoint and tests

* Use new service entrypoints

* Test changes
This commit is contained in:
Justin Baur
2025-08-26 13:30:37 -04:00
committed by GitHub
parent 7a63ae6315
commit e5159a3ba2
51 changed files with 849 additions and 205 deletions

View File

@@ -22,18 +22,18 @@ using Microsoft.Extensions.Time.Testing;
using NSubstitute;
using Xunit;
namespace Bit.Core.Test.Platform.Push.Services;
namespace Bit.Core.Test.Platform.Push.Engines;
[QueueClientCustomize]
[SutProviderCustomize]
public class AzureQueuePushNotificationServiceTests
public class AzureQueuePushEngineTests
{
private static readonly Guid _deviceId = Guid.Parse("c4730f80-caaa-4772-97bd-5c0d23a2baa3");
private static readonly string _deviceIdentifier = "test_device_identifier";
private readonly FakeTimeProvider _fakeTimeProvider;
private readonly Core.Settings.GlobalSettings _globalSettings = new();
public AzureQueuePushNotificationServiceTests()
public AzureQueuePushEngineTests()
{
_fakeTimeProvider = new();
_fakeTimeProvider.SetUtcNow(DateTime.UtcNow);
@@ -771,12 +771,11 @@ public class AzureQueuePushNotificationServiceTests
var globalSettings = new Core.Settings.GlobalSettings();
var sut = new AzureQueuePushNotificationService(
var sut = new AzureQueuePushEngine(
queueClient,
httpContextAccessor,
globalSettings,
NullLogger<AzureQueuePushNotificationService>.Instance,
_fakeTimeProvider
NullLogger<AzureQueuePushEngine>.Instance
);
await test(new EngineWrapper(sut, _fakeTimeProvider, _globalSettings.Installation.Id));

View File

@@ -2,16 +2,16 @@
using Bit.Core.AdminConsole.Entities;
using Bit.Core.Auth.Entities;
using Bit.Core.NotificationCenter.Entities;
using Bit.Core.Platform.Push;
using Bit.Core.Platform.Push.Internal;
using Bit.Core.Tools.Entities;
using Bit.Core.Vault.Entities;
using Microsoft.Extensions.Logging.Abstractions;
namespace Bit.Core.Test.Platform.Push.Services;
namespace Bit.Core.Test.Platform.Push.Engines;
public class NotificationsApiPushNotificationServiceTests : PushTestBase
public class NotificationsApiPushEngineTests : PushTestBase
{
public NotificationsApiPushNotificationServiceTests()
public NotificationsApiPushEngineTests()
{
GlobalSettings.BaseServiceUri.InternalNotifications = "https://localhost:7777";
GlobalSettings.BaseServiceUri.InternalIdentity = "https://localhost:8888";
@@ -21,11 +21,11 @@ public class NotificationsApiPushNotificationServiceTests : PushTestBase
protected override IPushEngine CreateService()
{
return new NotificationsApiPushNotificationService(
return new NotificationsApiPushEngine(
HttpClientFactory,
GlobalSettings,
HttpContextAccessor,
NullLogger<NotificationsApiPushNotificationService>.Instance
NullLogger<NotificationsApiPushEngine>.Instance
);
}

View File

@@ -10,6 +10,7 @@ using Bit.Core.Enums;
using Bit.Core.NotificationCenter.Entities;
using Bit.Core.NotificationCenter.Enums;
using Bit.Core.Platform.Push;
using Bit.Core.Platform.Push.Internal;
using Bit.Core.Settings;
using Bit.Core.Tools.Entities;
using Bit.Core.Vault.Entities;
@@ -22,6 +23,8 @@ using NSubstitute;
using RichardSzalay.MockHttp;
using Xunit;
namespace Bit.Core.Test.Platform.Push.Engines;
public class EngineWrapper(IPushEngine pushEngine, FakeTimeProvider fakeTimeProvider, Guid installationId) : IPushNotificationService
{
public Guid InstallationId { get; } = installationId;

View File

@@ -5,7 +5,6 @@ using Bit.Core.AdminConsole.Entities;
using Bit.Core.Auth.Entities;
using Bit.Core.Entities;
using Bit.Core.NotificationCenter.Entities;
using Bit.Core.Platform.Push;
using Bit.Core.Platform.Push.Internal;
using Bit.Core.Repositories;
using Bit.Core.Settings;
@@ -15,7 +14,7 @@ using Microsoft.Extensions.Logging.Abstractions;
using Microsoft.Extensions.Time.Testing;
using NSubstitute;
namespace Bit.Core.Test.Platform.Push.Services;
namespace Bit.Core.Test.Platform.Push.Engines;
public class RelayPushNotificationServiceTests : PushTestBase
{
@@ -39,12 +38,12 @@ public class RelayPushNotificationServiceTests : PushTestBase
protected override IPushEngine CreateService()
{
return new RelayPushNotificationService(
return new RelayPushEngine(
HttpClientFactory,
_deviceRepository,
GlobalSettings,
HttpContextAccessor,
NullLogger<RelayPushNotificationService>.Instance
NullLogger<RelayPushEngine>.Instance
);
}

View File

@@ -0,0 +1,57 @@
using Bit.Core.Enums;
using Bit.Core.Platform.Push;
using Bit.Core.Platform.Push.Internal;
using Bit.Core.Settings;
using Microsoft.Extensions.Logging.Abstractions;
using Microsoft.Extensions.Time.Testing;
using NSubstitute;
using Xunit;
namespace Bit.Core.Test.Platform.Push;
public class MultiServicePushNotificationServiceTests
{
private readonly IPushEngine _fakeEngine1;
private readonly IPushEngine _fakeEngine2;
private readonly MultiServicePushNotificationService _sut;
public MultiServicePushNotificationServiceTests()
{
_fakeEngine1 = Substitute.For<IPushEngine>();
_fakeEngine2 = Substitute.For<IPushEngine>();
_sut = new MultiServicePushNotificationService(
[_fakeEngine1, _fakeEngine2],
NullLogger<MultiServicePushNotificationService>.Instance,
new GlobalSettings(),
new FakeTimeProvider()
);
}
#if DEBUG // This test requires debug code in the sut to work properly
[Fact]
public async Task PushAsync_CallsAllEngines()
{
var notification = new PushNotification<object>
{
Target = NotificationTarget.User,
TargetId = Guid.NewGuid(),
Type = PushType.AuthRequest,
Payload = new { },
ExcludeCurrentContext = false,
};
await _sut.PushAsync(notification);
await _fakeEngine1
.Received(1)
.PushAsync(Arg.Is<PushNotification<object>>(n => ReferenceEquals(n, notification)));
await _fakeEngine2
.Received(1)
.PushAsync(Arg.Is<PushNotification<object>>(n => ReferenceEquals(n, notification)));
}
#endif
}

View File

@@ -1,9 +1,9 @@
using Bit.Core.NotificationHub;
using Bit.Core.Platform.Push.Internal;
using Bit.Core.Settings;
using Bit.Core.Utilities;
using Xunit;
namespace Bit.Core.Test.NotificationHub;
namespace Bit.Core.Test.Platform.Push.NotificationHub;
public class NotificationHubConnectionTests
{

View File

@@ -1,4 +1,4 @@
using Bit.Core.NotificationHub;
using Bit.Core.Platform.Push.Internal;
using Bit.Core.Settings;
using Bit.Core.Utilities;
using Microsoft.Extensions.Logging;
@@ -6,7 +6,7 @@ using NSubstitute;
using Xunit;
using static Bit.Core.Settings.GlobalSettings;
namespace Bit.Core.Test.NotificationHub;
namespace Bit.Core.Test.Platform.Push.NotificationHub;
public class NotificationHubPoolTests
{

View File

@@ -1,11 +1,11 @@
using AutoFixture;
using Bit.Core.NotificationHub;
using Bit.Core.Platform.Push.Internal;
using Bit.Test.Common.AutoFixture;
using Microsoft.Azure.NotificationHubs;
using NSubstitute;
using Xunit;
namespace Bit.Core.Test.NotificationHub;
namespace Bit.Core.Test.Platform.Push.NotificationHub;
public class NotificationHubProxyTests
{

View File

@@ -1,5 +1,4 @@
#nullable enable
using System.Text.Json;
using System.Text.Json;
using System.Text.Json.Nodes;
using Bit.Core.Auth.Entities;
using Bit.Core.Context;
@@ -7,10 +6,11 @@ using Bit.Core.Enums;
using Bit.Core.Models;
using Bit.Core.NotificationCenter.Entities;
using Bit.Core.NotificationCenter.Enums;
using Bit.Core.NotificationHub;
using Bit.Core.Platform.Push;
using Bit.Core.Platform.Push.Internal;
using Bit.Core.Repositories;
using Bit.Core.Test.NotificationCenter.AutoFixture;
using Bit.Core.Test.Platform.Push.Engines;
using Bit.Core.Tools.Entities;
using Bit.Core.Vault.Entities;
using Bit.Test.Common.AutoFixture;
@@ -22,7 +22,7 @@ using Microsoft.Extensions.Time.Testing;
using NSubstitute;
using Xunit;
namespace Bit.Core.Test.NotificationHub;
namespace Bit.Core.Test.Platform.Push.NotificationHub;
[SutProviderCustomize]
[NotificationStatusCustomize]
@@ -621,11 +621,11 @@ public class NotificationHubPushNotificationServiceTests
fakeTimeProvider.SetUtcNow(_now);
var sut = new NotificationHubPushNotificationService(
var sut = new NotificationHubPushEngine(
installationDeviceRepository,
notificationHubPool,
httpContextAccessor,
NullLogger<NotificationHubPushNotificationService>.Instance,
NullLogger<NotificationHubPushEngine>.Instance,
globalSettings
);
@@ -676,7 +676,7 @@ public class NotificationHubPushNotificationServiceTests
};
private static async Task AssertSendTemplateNotificationAsync(
SutProvider<NotificationHubPushNotificationService> sutProvider, PushType type, object payload, string tag)
SutProvider<NotificationHubPushEngine> sutProvider, PushType type, object payload, string tag)
{
await sutProvider.GetDependency<INotificationHubPool>()
.Received(1)

View File

@@ -0,0 +1,198 @@
using Bit.Core.Auth.Models.Data;
using Bit.Core.Entities;
using Bit.Core.KeyManagement.UserKey;
using Bit.Core.Platform.Push;
using Bit.Core.Platform.Push.Internal;
using Bit.Core.Repositories;
using Bit.Core.Repositories.Noop;
using Bit.Core.Settings;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.DependencyInjection.Extensions;
using Xunit;
namespace Bit.Core.Test.Platform.Push;
public class PushServiceCollectionExtensionsTests
{
[Fact]
public void AddPush_SelfHosted_NoConfig_NoEngines()
{
var services = Build(new Dictionary<string, string?>
{
{ "GlobalSettings:SelfHosted", "true" },
{ "GlobalSettings:Installation:Id", Guid.NewGuid().ToString() },
});
_ = services.GetRequiredService<IPushNotificationService>();
var engines = services.GetServices<IPushEngine>();
Assert.Empty(engines);
}
[Fact]
public void AddPush_SelfHosted_ConfiguredForRelay_RelayEngineAdded()
{
var services = Build(new Dictionary<string, string?>
{
{ "GlobalSettings:SelfHosted", "true" },
{ "GlobalSettings:Installation:Id", Guid.NewGuid().ToString() },
{ "GlobalSettings:Installation:Key", "some_key"},
{ "GlobalSettings:PushRelayBaseUri", "https://example.com" },
});
_ = services.GetRequiredService<IPushNotificationService>();
var engines = services.GetServices<IPushEngine>();
var engine = Assert.Single(engines);
Assert.IsType<RelayPushEngine>(engine);
}
[Fact]
public void AddPush_SelfHosted_ConfiguredForApi_ApiEngineAdded()
{
var services = Build(new Dictionary<string, string?>
{
{ "GlobalSettings:SelfHosted", "true" },
{ "GlobalSettings:Installation:Id", Guid.NewGuid().ToString() },
{ "GlobalSettings:InternalIdentityKey", "some_key"},
{ "GlobalSettings:BaseServiceUri", "https://example.com" },
});
_ = services.GetRequiredService<IPushNotificationService>();
var engines = services.GetServices<IPushEngine>();
var engine = Assert.Single(engines);
Assert.IsType<NotificationsApiPushEngine>(engine);
}
[Fact]
public void AddPush_SelfHosted_ConfiguredForRelayAndApi_TwoEnginesAdded()
{
var services = Build(new Dictionary<string, string?>
{
{ "GlobalSettings:SelfHosted", "true" },
{ "GlobalSettings:Installation:Id", Guid.NewGuid().ToString() },
{ "GlobalSettings:Installation:Key", "some_key"},
{ "GlobalSettings:PushRelayBaseUri", "https://example.com" },
{ "GlobalSettings:InternalIdentityKey", "some_key"},
{ "GlobalSettings:BaseServiceUri", "https://example.com" },
});
_ = services.GetRequiredService<IPushNotificationService>();
var engines = services.GetServices<IPushEngine>();
Assert.Collection(
engines,
e => Assert.IsType<RelayPushEngine>(e),
e => Assert.IsType<NotificationsApiPushEngine>(e)
);
}
[Fact]
public void AddPush_Cloud_NoConfig_AddsNotificationHub()
{
var services = Build(new Dictionary<string, string?>
{
{ "GlobalSettings:SelfHosted", "false" },
});
_ = services.GetRequiredService<IPushNotificationService>();
var engines = services.GetServices<IPushEngine>();
var engine = Assert.Single(engines);
Assert.IsType<NotificationHubPushEngine>(engine);
}
[Fact]
public void AddPush_Cloud_HasNotificationConnectionString_TwoEngines()
{
var services = Build(new Dictionary<string, string?>
{
{ "GlobalSettings:SelfHosted", "false" },
{ "GlobalSettings:Notifications:ConnectionString", "UseDevelopmentStorage=true" },
});
_ = services.GetRequiredService<IPushNotificationService>();
var engines = services.GetServices<IPushEngine>();
Assert.Collection(
engines,
e => Assert.IsType<NotificationHubPushEngine>(e),
e => Assert.IsType<AzureQueuePushEngine>(e)
);
}
[Fact]
public void AddPush_Cloud_CalledTwice_DoesNotAddServicesTwice()
{
var services = new ServiceCollection();
var config = new Dictionary<string, string?>
{
{ "GlobalSettings:SelfHosted", "false" },
{ "GlobalSettings:Notifications:ConnectionString", "UseDevelopmentStorage=true" },
};
AddServices(services, config);
var initialCount = services.Count;
// Add services again
AddServices(services, config);
Assert.Equal(initialCount, services.Count);
}
private static ServiceProvider Build(Dictionary<string, string?> initialData)
{
var services = new ServiceCollection();
AddServices(services, initialData);
return services.BuildServiceProvider();
}
private static void AddServices(IServiceCollection services, Dictionary<string, string?> initialData)
{
// A minimal service collection is always expected to have logging, config, and global settings
// pre-registered.
services.AddLogging();
var config = new ConfigurationBuilder()
.AddInMemoryCollection(initialData)
.Build();
services.TryAddSingleton(config);
var globalSettings = new GlobalSettings();
config.GetSection("GlobalSettings").Bind(globalSettings);
services.TryAddSingleton(globalSettings);
services.TryAddSingleton<IGlobalSettings>(globalSettings);
// Temporary until AddPush can add it themselves directly.
services.TryAddSingleton<IDeviceRepository, StubDeviceRepository>();
// Temporary until AddPush can add it themselves directly.
services.TryAddSingleton<IInstallationDeviceRepository, InstallationDeviceRepository>();
services.AddPush(globalSettings);
}
private class StubDeviceRepository : IDeviceRepository
{
public Task ClearPushTokenAsync(Guid id) => throw new NotImplementedException();
public Task<Device> CreateAsync(Device obj) => throw new NotImplementedException();
public Task DeleteAsync(Device obj) => throw new NotImplementedException();
public Task<Device?> GetByIdAsync(Guid id, Guid userId) => throw new NotImplementedException();
public Task<Device?> GetByIdAsync(Guid id) => throw new NotImplementedException();
public Task<Device?> GetByIdentifierAsync(string identifier) => throw new NotImplementedException();
public Task<Device?> GetByIdentifierAsync(string identifier, Guid userId) => throw new NotImplementedException();
public Task<ICollection<Device>> GetManyByUserIdAsync(Guid userId) => throw new NotImplementedException();
public Task<ICollection<DeviceAuthDetails>> GetManyByUserIdWithDeviceAuth(Guid userId) => throw new NotImplementedException();
public Task ReplaceAsync(Device obj) => throw new NotImplementedException();
public UpdateEncryptedDataForKeyRotation UpdateKeysForRotationAsync(Guid userId, IEnumerable<Device> devices) => throw new NotImplementedException();
public Task UpsertAsync(Device obj) => throw new NotImplementedException();
}
}

View File

@@ -0,0 +1,64 @@
using System.Diagnostics;
using System.Reflection;
using Bit.Core.Enums;
using Bit.Core.Platform.Push;
using Xunit;
namespace Bit.Core.Test.Platform.Push;
public class PushTypeTests
{
[Fact]
public void AllEnumMembersHaveUniqueValue()
{
// No enum member should use the same value as another named member.
var usedNumbers = new HashSet<byte>();
var enumMembers = Enum.GetValues<PushType>();
foreach (var enumMember in enumMembers)
{
if (!usedNumbers.Add((byte)enumMember))
{
Assert.Fail($"Enum number value ({(byte)enumMember}) on {enumMember} is already in use.");
}
}
}
[Fact]
public void AllEnumMembersHaveNotificationInfoAttribute()
{
// Every enum member should be annotated with [NotificationInfo]
foreach (var member in typeof(PushType).GetMembers(BindingFlags.Public | BindingFlags.Static))
{
var notificationInfoAttribute = member.GetCustomAttribute<NotificationInfoAttribute>();
if (notificationInfoAttribute is null)
{
Assert.Fail($"PushType.{member.Name} is missing a required [NotificationInfo(\"team-name\", typeof(MyType))] attribute.");
}
}
}
[Fact]
public void AllEnumValuesAreInSequence()
{
// There should not be any gaps in the numbers defined for an enum, that being if someone last defined 22
// the next number used should be 23 not 24 or any other number.
var sortedValues = Enum.GetValues<PushType>()
.Order()
.ToArray();
Debug.Assert(sortedValues.Length > 0);
var lastValue = sortedValues[0];
foreach (var value in sortedValues[1..])
{
var expectedValue = ++lastValue;
Assert.Equal(expectedValue, value);
}
}
}

View File

@@ -1,8 +0,0 @@
#nullable enable
namespace Bit.Core.Test.Platform.Push.Services;
public class MultiServicePushNotificationServiceTests
{
// TODO: Can add a couple tests here
}

View File

@@ -1,6 +1,8 @@
#nullable enable
using Bit.Core.Enums;
using Bit.Core.NotificationHub;
using Bit.Core.Platform.Push.Internal;
using Bit.Core.Platform.PushRegistration;
using Bit.Core.Platform.PushRegistration.Internal;
using Bit.Core.Utilities;
using Bit.Test.Common.AutoFixture;
using Bit.Test.Common.AutoFixture.Attributes;

View File

@@ -0,0 +1,108 @@
using Bit.Core.Platform.Push;
using Bit.Core.Platform.PushRegistration.Internal;
using Bit.Core.Repositories;
using Bit.Core.Repositories.Noop;
using Bit.Core.Settings;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.DependencyInjection.Extensions;
using Xunit;
namespace Bit.Core.Test.Platform.PushRegistration;
public class PushRegistrationServiceCollectionExtensionsTests
{
[Fact]
public void AddPushRegistration_Cloud_CreatesNotificationHubRegistrationService()
{
var services = Build(new Dictionary<string, string?>
{
{ "GlobalSettings:SelfHosted", "false" },
});
var pushRegistrationService = services.GetRequiredService<IPushRegistrationService>();
Assert.IsType<NotificationHubPushRegistrationService>(pushRegistrationService);
}
[Fact]
public void AddPushRegistration_SelfHosted_NoOtherConfig_ReturnsNoopRegistrationService()
{
var services = Build(new Dictionary<string, string?>
{
{ "GlobalSettings:SelfHosted", "true" },
});
var pushRegistrationService = services.GetRequiredService<IPushRegistrationService>();
Assert.IsType<NoopPushRegistrationService>(pushRegistrationService);
}
[Fact]
public void AddPushRegistration_SelfHosted_RelayConfig_ReturnsRelayRegistrationService()
{
var services = Build(new Dictionary<string, string?>
{
{ "GlobalSettings:SelfHosted", "true" },
{ "GlobalSettings:PushRelayBaseUri", "https://example.com" },
{ "GlobalSettings:Installation:Key", "some_key" },
});
var pushRegistrationService = services.GetRequiredService<IPushRegistrationService>();
Assert.IsType<RelayPushRegistrationService>(pushRegistrationService);
}
[Fact]
public void AddPushRegistration_MultipleTimes_NoAdditionalServices()
{
var services = new ServiceCollection();
var config = new Dictionary<string, string?>
{
{ "GlobalSettings:SelfHosted", "true" },
{ "GlobalSettings:PushRelayBaseUri", "https://example.com" },
{ "GlobalSettings:Installation:Key", "some_key" },
};
AddServices(services, config);
// Add services again
services.AddPushRegistration();
var provider = services.BuildServiceProvider();
Assert.Single(provider.GetServices<IPushRegistrationService>());
}
private static ServiceProvider Build(Dictionary<string, string?> initialData)
{
var services = new ServiceCollection();
AddServices(services, initialData);
return services.BuildServiceProvider();
}
private static void AddServices(IServiceCollection services, Dictionary<string, string?> initialData)
{
// A minimal service collection is always expected to have logging, config, and global settings
// pre-registered.
services.AddLogging();
var config = new ConfigurationBuilder()
.AddInMemoryCollection(initialData)
.Build();
services.TryAddSingleton(config);
var globalSettings = new GlobalSettings();
config.GetSection("GlobalSettings").Bind(globalSettings);
services.TryAddSingleton(globalSettings);
services.TryAddSingleton<IGlobalSettings>(globalSettings);
// Temporary until AddPushRegistration can add it themselves directly.
services.TryAddSingleton<IInstallationDeviceRepository, InstallationDeviceRepository>();
services.AddPushRegistration();
}
}

View File

@@ -1,4 +1,4 @@
using Bit.Core.Platform.Push.Internal;
using Bit.Core.Platform.PushRegistration.Internal;
using Bit.Core.Settings;
using Microsoft.Extensions.Logging;
using NSubstitute;

View File

@@ -4,8 +4,8 @@ using Bit.Core.Entities;
using Bit.Core.Enums;
using Bit.Core.Exceptions;
using Bit.Core.Models.Data.Organizations.OrganizationUsers;
using Bit.Core.NotificationHub;
using Bit.Core.Platform.Push;
using Bit.Core.Platform.PushRegistration;
using Bit.Core.Repositories;
using Bit.Core.Services;
using Bit.Core.Settings;