Add WebHookSubscription2021 support

[joachimvh]

📁 Related issues

Some ideas in here based on #1388

✍️ Description

An addition to #1468 that also adds webhooks. There was a demand for webhook notification support besides websockets, and I thought it was also good to see how easy it is to add a new notification type to the architecture proposed in #1468. Result is that only minor changes were necessary and only a few new classes.

[joachimvh]

There seems to be a memory issue with the node 18 integration test. Will have to check if I can reproduce locally.

[joachimvh]

There seems to be a memory issue with the node 18 integration test. Will have to check if I can reproduce locally.

This has been fixed now but weirdly enough the node v14 integration test seems to fail now on what seems to be a race condition. Will look into that later. It's not even the implementation of this PR but of the websocket notifications so might actually also be present in #1468.

The above fix doesn't really fit in this PR but is necessary since this is the PR that seems to have triggered the problem. Some more information in #1496

[RubenVerborgh]

Great one, good test of the architecture!

[RubenVerborgh]

Have we reported this?

[joachimvh]

No. Issue is also that the webhook "spec" currently is a markdown document that is still much in flux and is not and does not even have a published version yet: https://github.com/solid/notifications/blob/main/webhook-subscription-2021.md

[RubenVerborgh]

Isn't that, like, super dangerous?

[RubenVerborgh]

If not, tell us why we're allowed to make that assumption.

[joachimvh]

Only the person that made the subscription should have access to that identifier (and thus the unsubscribe URL), we don't expose it anywhere. Could have generated a new value just for the unsubscribe URL but thought we could just reuse the ID. Currently the spec doesn't say the unsubscribe request should be authenticated:

If the client wishes to unsubscribe from the webhook, it can make a DELETE request to the provided unsubscribe_endpoint.

[RubenVerborgh]

Ah okay, so it is a server-generated identifier? That's important context. So we'd deed to write then indeed who is responsible for creating the URLs.

My preference is here that we encode that knowledge in one separate place. So have one class/module/utility (can be embedded in this file) which has createWebHookUrl(id) and getWebHookId(url) or whatever is appropriate. Or just two member methods that are next to each other. But should not be implicitly distributed.

[joachimvh]

Yea, currently the WebHookSubscription2021.ts, that generates the response, has a unsubscribe_endpoint: '${this.unsubscribePath}/${encodeURI(info.id)}' line. Could have a WebHookUnsubscribeGenerator or something like that with a get(id: string): string and parse(url: string): string functions.

Note that the WebSockets solution had a similar case where one class generates the WebSocket URL, and another class parses the URL of a connection to extract the ID from it:

CommunitySolidServer/src/server/notifications/WebSocketSubscription2021/WebSocketSubscription2021.ts

Line 51 in 37ba404

source: `ws${this.path.slice('http'.length)}?auth=${encodeURI(info.id)}`,

CommunitySolidServer/src/server/notifications/WebSocketSubscription2021/WebSocket2021Listener.ts

Line 29 in 37ba404

const { pathname, searchParams } = new URL(upgradeRequest.url ?? '', 'http://example.com');

[RubenVerborgh]

Yeah even if just utility functions. And same for the sockets indeed.