Add notes about notification target and signalR

src/Core/Platform/Push/NotificationTarget.cs

@@ -0,0 +1,24 @@
+namespace Bit.Core.Platform.Push;
+
+/// <summary>
+/// Contains constants for all the available targets for a given notification.
+/// </summary>
+/// <remarks>
+/// Please reach out to the Platform team if you need a new target added.
+/// </remarks>
+public enum NotificationTarget
+{
+    /// <summary>
+    /// The target for the notification is a single user.
+    /// </summary>
+    User,
+    /// <summary>
+    /// The target for the notification are all the users in an organization.
+    /// </summary>
+    Organization,
+    /// <summary>
+    /// The target for the notification are all the organizations, 
+    /// and all the users in that organization for a installation.
+    /// </summary>
+    Installation,
+}

src/Core/Platform/Push/PushNotification.cs

@@ -1,31 +1,7 @@
-#nullable enable
-using Bit.Core.Enums;
+using Bit.Core.Enums;
 
 namespace Bit.Core.Platform.Push;
 
-/// <summary>
-/// Contains constants for all the available targets for a given notification.
-/// </summary>
-/// <remarks>
-/// Please reach out to the Platform team if you need a new target added.
-/// </remarks>
-public enum NotificationTarget
-{
-    /// <summary>
-    /// The target for the notification is a single user.
-    /// </summary>
-    User,
-    /// <summary>
-    /// The target for the notification are all the users in an organization.
-    /// </summary>
-    Organization,
-    /// <summary>
-    /// The target for the notification are all the organizations, 
-    /// and all the users in that organization for a installation.
-    /// </summary>
-    Installation,
-}
-
 /// <summary>
 /// An object containing all the information required for getting a notification
 /// to an end users device and the information you want available to that device.

src/Core/Platform/Push/README.md

@@ -36,6 +36,9 @@ team and expected payload type are. Then you may inject
 [`IPushNotificationService`](./IPushNotificationService.cs) into your own service and call its
 `PushAsync` method.
 
+You also need to add code to [`HubHelpers`](../../../Notifications/HubHelpers.cs) to read your
+payload body and select the appropriate group or user to send the notification to.
+
 You should NOT add tests for your specific notification type in any of the `IPushEngine`
 implementations. They do currently have tests for many of the notification types but those will
 eventually be deleted and no new ones need to be added.
@@ -92,3 +95,20 @@ doesn't require the self-hosted customer to run their own queuing infrastructure
 This instance is registered when `GlobalSettings:InternalIdentityKey` and
 `GlobalSettings:BaseServiceUri:InternalNotifications` are set. Both of these settings are usually
 set automatically in supported Bitwarden setups.
+
+## Adding new notification targets
+
+[`NotificationTarget`](./NotificationTarget.cs) is an enum that defines the possible targets for a
+notification, `IPushEngine` implementations may or may not need to be aware and have special
+considerations for each notification target type. For that reason adding a new target is NOT as easy
+as adding a new enum member. The ANH implementation uses it to build it's tag query. A new target
+also needs to be something that is targettable through the tag query. For example, say a team wants
+a notification target for users in an org with a verified email. Today this target would not be
+expressable through a target because when we register a device with ANH we do not include whether
+that user has a verified email. It would be possible to start including that information but the
+effort of tracking that information and updating push registrations when a user verifies their email
+needs to be weighed with the amount this notification is sent. What might instead be worth it would
+be for the team that wants such a target to instead do a query to find users who match that query
+and send a notification for each user individually using `NotificationTarget.User`. If there are
+enough requests like that though we may want to consider adding a `BulkPushAsync` method to
+`IPushNotificationService`.

src/Core/Platform/PushRegistration/README.md

@@ -3,7 +3,7 @@
 ## About
 
 Push Registration is a feature for managing devices that should receive push notifications. The main
-entrypoint for this feature is `IPushRegistrationService`. 
+entrypoint for this feature is `IPushRegistrationService`.
 
 ## Usage
 
@@ -27,15 +27,15 @@ Since Azure Notification Hub has a limit on the amount of devices per hub we hav
 devices across multiple hubs. Multiple hubs can be configured through configuration and each one can
 have a `RegistrationStartDate` and `RegistrationEndDate`. If the start date is `null` no devices
 will be registered against that given hub. A `null` end date is treated as no known expiry. The
-creation date for a device is retrieved by the device's ID, and that date is used to find a hub that spans
-during it's creation date.
+creation date for a device is retrieved by the device's ID, and that date is used to find a hub that
+spans uring it's creation date.
 
 When we register a device with Azure Notification Hub we include tags, which are data that can later
 be used to specifically target that device with a notification. We send the ID of the user this
 device belongs to, the type of the client (Web, Desktop, Mobile, etc), all the organization IDs of
-organizations of which the user is a confirmed member, the ID of the self-hosted installation if this
-device was relayed to us, and the device identifier, which is a random GUID generated on the device.
-Most of this data is considered immutable after the creation of a device, except for the
+organizations of which the user is a confirmed member, the ID of the self-hosted installation if 
+this device was relayed to us, and the device identifier, which is a random GUID generated on the 
+device. Most of this data is considered immutable after the creation of a device, except for the
 organization memberships of a user. If a user is added/removed from an organization, it is important
 that `CreateOrUpdateRegistrationAsync` is called with the new memberships.
 
@@ -44,3 +44,10 @@ that `CreateOrUpdateRegistrationAsync` is called with the new memberships.
 Used when the application is self-hosted. This sends a API request to the configured cloud instance
 and which will then use [Azure Notification Hub](#azure-notification-hub) but will associate the
 installation as the self-hosted installation id instead of using the cloud one.
+
+### SignalR
+
+While not an implementation of `IPushRegistrationService` the SignalR hub adds users to various
+groups in [`NotificationsHub.OnConnectedAsync`](../../../Notifications/NotificationsHub.cs) method.
+It utilizes a manual build of `ICurrentContext` where it reads the claims provided from the access
+token.