Rich push notifications not shown for certain instances

[sorin-davidoi]

self.addEventListener('push', event => {
  event.waitUntil((async () => {
    const data = event.data.json()
    const { origin } = new URL(data.icon)

    try {
      const notification = await get(`${origin}/api/v1/notifications/${data.notification_id}`, {
        Authorization: `Bearer ${data.access_token}`
      }, { timeout: 2000 })

      await showRichNotification(data, notification)
    } catch (e) {
      await showSimpleNotification(data)
    }
  })())
})

The issue is const { origin } = new URL(data.icon) - this used to work fine when the icon was hosted on toot.cafe, but now it's hosted on assets.toot.cafe, so the request will fail. I think we can use event.target.origin instead.

[sorin-davidoi]

This is why clicking on all notifications on toot.cafe will open the notifications page instead of the page for that particular notification.

[sorin-davidoi]

I was wrong - event.target.origin is the origin of the pinafore instance. Not sure how to get the origin of the server doing the push 😕

[nolanlawson]

Reopening!

Seems like maybe we need to solve the whole "register push notifications for multiple instances at once" so that we can keep track of which instance is pushing which notifications? @sorin-davidoi

[sorin-davidoi]

I'm not sure that would help.

There must be a way to identify, given a PushEvent, which server sent it. Either I can't figure it out or it's not possible (in which case I think it's a serious omision from the spec).

If we figure out "register push notifications for multiple instances at once", we would still not be able to tell to which instance to switch to without being able to infer which instance triggered the push.

A workaround for now could be to query IndexedDB for the active instance and assume the push came from it.

[tbroyer]

For the case where people use Pinafore with a single instance, maybe just use that? (and multiple instances would still get the "simple" notification until a reliable solution is found)

Also, the simple notification could use the badge (should it also use the notification_id as tag like the rich notification?)

[tbroyer]

In #2296, I'm using the instanceName if there's only a single known instance, and directly fallback to a "simple" notification otherwise. Would solve the issue at least for a bunch of users.
(I also added the badge and tag to the simple notifications)
Currently waiting for a notification on the Vercel-deployed preview to actually test it.

[nolanlawson]

The issue is const { origin } = new URL(data.icon) - this used to work fine when the icon was hosted on toot.cafe, but now it's hosted on assets.toot.cafe, so the request will fail. I think we can use event.target.origin instead.

Re-reading this, I don't understand this at all. Why does the request fail? At what point does it fail? Does it fall back to the catch block in the try/catch (i.e. showing a simple notification instead of a rich one)? What does this have to do with knowing which instance the notification came from?

[tbroyer]

We receive an event with a notification_id and we'd like to go get the notification details to show a rich notification. The problem is we need to know the URL of the API, i.e. get the instance that sent the notification so we can call its API.

Currently event.target is the ServiceWorkerGlobalScope, which doesn't even have an origin property, so the request goes to undefined/api/v1/notifications/<notification id>, fails, and it falls back to a simple notification.
The previous code used the origin of the icon property from the PushEvent's data, which works if the icon is served from the same origin as the API, but fails otherwise (as explained above).

Unfortunately, there's nothing in either the PushEvent (from the browser) or its data (from the server) that could be used to determine the instance's API URL.

Here's a comparison of the notification I receive from pinafore.social (bottom) and from the preview deployment of #2296 (top):

And for comparison, the one from my instance, a Mastodon v4.0.2 server:

[nolanlawson]

That is really bizarre. It feels like Mastodon ought to include the URL in the push event somewhere.

[nolanlawson]

I confirmed through local testing that there is no way to figure out the instance origin (that I can see). FWIW you also can't figure out the username, so even if Pinafore supported multiple usernames on the same instance, we couldn't support push notifications for each.

I filed an issue on Mastodon: mastodon/mastodon#22183

[tbroyer]

AFAICT, the access_token being sent in the PushEvent (i.e. specific to the notification, not the user/subscription), we don't even need the username (as long as the push is only for using Web Notifications and won't update the locally-cached data)

[nolanlawson]

I saw the access_token but it didn't seem to correspond to the access token stored for the instance, so I wasn't sure we could use to it figure out which instance the push event came from.

we don't even need the username

Yeah well Pinafore also doesn't support multiple usernames on the same instance anyway. 🙂 #39

[tbroyer]

Fwiw, Mastodon too uses the access_token from the push event to retrieve the notification details: https://github.com/mastodon/mastodon/blob/ad568924c064c41e056dfec651217dbe7bb637fc/app/javascript/mastodon/service_worker/web_push_notifications.js#L79-L83
This doesn't allow us to tell which instance sent the push, but it means that if there were multiple usernames on the same instance, we wouldn't need to first determine which one the notification is linked to to then get the appropriate username+instance access_token; we can just call the API to get the notification details and display that using only the information in the push event; no need for a username property in the push event if (when) it gives us the instance URL (I suppose a username would allow us to derive the instance URL, so we'd either one or the other).

[nolanlawson]

@tbroyer Hm are you saying we could just try calling that API with the access token until a server responds with something other than an error?

That sounds doable, although it potentially opens a security vulnerability because we'd be communicating the access token from one instance to another one. 😬

[tbroyer]

That's certainly not what I'm suggesting. All I'm saying is that the push event is missing an instance URL, we don't need a username in addition to the instance URL.

(if the push event was also used to update the local cache, then we'd probably need the username to target the correct cache, but not for displaying the notification using the Web Notification API)

[nolanlawson]

OK, maybe I wasn't clear. I was just thinking that for the issue I filed on Mastodon (mastodon/mastodon#22183), they should probably include the username too since some clients support multiple accounts on the same instance.