1
0
mirror of https://github.com/bitwarden/server synced 2026-01-06 02:23:51 +00:00

Apply suggestions from code review

Co-authored-by: Todd Martin <106564991+trmartin4@users.noreply.github.com>
This commit is contained in:
Justin Baur
2025-09-03 15:41:11 -04:00
committed by GitHub
parent ffbe368daf
commit c3181d581e
2 changed files with 23 additions and 20 deletions

View File

@@ -29,7 +29,7 @@ await pushNotificationService.PushAsync(new PushNotification<MyPayload>
## Extending
If you want to extend this framework for sending your own notification type you do so by adding a
new enum member the [`PushType`](./PushType.cs) enum. Assign a number to it that is 1 above the next
new enum member to the [`PushType`](./PushType.cs) enum. Assign a number to it that is 1 above the next
highest value. You must then annotate that enum member with a
[`[NotificationInfo]`](./NotificationInfoAttribute.cs) attribute to inform others who the owning
team and expected payload type are. Then you may inject
@@ -40,30 +40,33 @@ You should NOT add tests for your specific notification type in any of the `IPus
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.
Since notifications are relayed through our cloud instance for self hosted users (whom opt-in) it's
Since notifications are relayed through our cloud instance for self hosted users (if they opt in) it's
important to try and keep the information in the notification payload minimal. It's generally best
to send a notification with various ID's that mean nothing to our cloud but can then be used to get
to send a notification with IDs for any entities involved, which mean nothing to our cloud but can then be used to get
more detailed information once the notification is received on the device.
## Implementations
The implementation of push notifications scatters push notification requests to all `IPushEngine`'s
The implementation of push notifications scatters push notification requests to all `IPushEngine`s
that have been registered in DI for the current application. In release builds, this service does
NOT await the underlying engines to make sure that the notification has gotten to its destination
NOT await the underlying engines to make sure that the notification has arrived at its destination
before its returned task completes.
### Azure Notification Hub
Used when the application is hosted by Bitwarden in the cloud. This sends the notification to the
configured Azure Notification Hub which we currently rely on it for sending the notification to our
configured Azure Notification Hub, which we currently rely on for sending notifications to:
- Our mobile clients, through the Notification Hub federation with mobile app notification systems, and
- Our clients configured to use Web Push (currently the Chrome Extension).
mobile clients and any clients configured to use Web Push (currently Chrome Extension).
This implementation is always assumed to have available configuration when running in the cloud.
### Azure Queue
Used when the application is hosted by Bitwarden in the cloud. This sends notifications to a Azure
Queue, that queue is then consumed in our Notifications service and the notifications are then sent
Used when the application is hosted by Bitwarden in the cloud, to send the notification over web sockets (SignalR). This sends the notification to a Azure
Queue. That queue is then consumed in our Notifications service, where the notification is sent
to a SignalR hub so that our clients connected through a persistent web socket to our notifications
service get the notification.
@@ -74,7 +77,7 @@ to a value.
Used when the application is being self-hosted. This relays a notification from the self-hosted
instance to a cloud instance. The notification is recieved by our cloud and then relayed to
Azure Notification Hub. This is because self-hosted instance aren't able to directly send
Azure Notification Hub. This is necessary because self-hosted instance aren't able to directly send
notifications to mobile devices.
This instance is registered in DI when `GlobalSettings:PushRelayBaseUri` and
@@ -83,9 +86,9 @@ This instance is registered in DI when `GlobalSettings:PushRelayBaseUri` and
### Notifications API
Used when the application is being self-hosted. This sends a API request to the self-hosted instance
of the Notifications service. The notifications service receives the request and then sends the
of the Notifications service. The Notifications service receives the request and then sends the
notification through the SignalR hub. This is very similar to cloud using an Azure Queue but it
doesn't require the self-hosted infrastructure to run their own queuing infrastructure.
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

View File

@@ -20,23 +20,23 @@ so that we can keep push registration working correctly.
### Azure Notification Hub
Used when the application is hosted by Bitwarden in the cloud. This registers the device and
associated metadata with Azure Notification Hub. This is necessary so that when a notification
associated metadata with Azure Notification Hub (ANH). This is necessary so that when a notification
is sent ANH will be able to get the notification to that device.
Since Azure Notification Hubs has a limit on the amount of devices per hub we have begun to shard
Since Azure Notification Hub has a limit on the amount of devices per hub we have begun to shard
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 of a device is pulled out of the devices ID and that date is used to find a hub that spans
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.
When we register a device with Azure Notificaiton Hub we include tags, which are data that can later
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 id's of
organizations the user is a confirmed member of, 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's important
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
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.
### Relay