From cbbb10afa1f186a973d98a0ce55429cb5aee5a3a Mon Sep 17 00:00:00 2001 From: Joachim Van Herwegen Date: Fri, 3 Feb 2023 15:32:34 +0100 Subject: [PATCH] feat: Use URLs for channel identifiers In the future these can potentially be used to dereference them --- .../http/notifications/webhooks/routes.json | 10 ++----- .../notifications/webhooks/subscription.json | 1 - src/server/notifications/BaseChannelType.ts | 7 +++-- .../WebHook2021Util.ts | 20 -------------- .../WebHookSubscription2021.ts | 10 +++---- .../WebHookUnsubscriber.ts | 3 +-- .../WebHookSubscription2021.test.ts | 14 ++++++++++ .../WebSocketSubscription2021.test.ts | 27 ++++++++++++++++++- .../notifications/BaseChannelType.test.ts | 2 +- .../WebHook2021Util.test.ts | 18 ------------- .../WebHookSubscription2021.test.ts | 8 +++--- .../WebHookUnsubscriber.test.ts | 4 +-- .../WebSocketSubscription2021.test.ts | 2 +- 13 files changed, 58 insertions(+), 68 deletions(-) delete mode 100644 src/server/notifications/WebHookSubscription2021/WebHook2021Util.ts delete mode 100644 test/unit/server/notifications/WebHookSubscription2021/WebHook2021Util.test.ts diff --git a/config/http/notifications/webhooks/routes.json b/config/http/notifications/webhooks/routes.json index 97c8ff4198..5e294c33aa 100644 --- a/config/http/notifications/webhooks/routes.json +++ b/config/http/notifications/webhooks/routes.json @@ -7,12 +7,6 @@ "base": { "@id": "urn:solid-server:default:NotificationRoute" }, "relativePath": "/WebHookSubscription2021/" }, - { - "@id": "urn:solid-server:default:WebHookUnsubscribeRoute", - "@type": "RelativePathInteractionRoute", - "base": { "@id": "urn:solid-server:default:WebHookRoute" }, - "relativePath": "/unsubscribe/" - }, { "@id": "urn:solid-server:default:WebHookWebIdRoute", "@type": "RelativePathInteractionRoute", @@ -27,7 +21,7 @@ "@type": "OperationRouterHandler", "baseUrl": { "@id": "urn:solid-server:default:variable:baseUrl" }, "allowedMethods": [ "DELETE" ], - "allowedPathNames": [ "/WebHookSubscription2021/unsubscribe/" ], + "allowedPathNames": [ "/WebHookSubscription2021/" ], "handler": { "@type": "WebHookUnsubscriber", "credentialsExtractor": { "@id": "urn:solid-server:default:CredentialsExtractor" }, @@ -39,7 +33,7 @@ "@id": "urn:solid-server:default:WebHookWebId", "@type": "OperationRouterHandler", "baseUrl": { "@id": "urn:solid-server:default:variable:baseUrl" }, - "allowedPathNames": [ "/WebHookSubscription2021/webId" ], + "allowedPathNames": [ "/WebHookSubscription2021/webId$" ], "handler": { "@type": "WebHookWebId", "baseUrl": { "@id": "urn:solid-server:default:variable:baseUrl" } diff --git a/config/http/notifications/webhooks/subscription.json b/config/http/notifications/webhooks/subscription.json index dd0f22bf25..0f1b87d7e6 100644 --- a/config/http/notifications/webhooks/subscription.json +++ b/config/http/notifications/webhooks/subscription.json @@ -24,7 +24,6 @@ "@type": "WebHookSubscription2021", "route": { "@id": "urn:solid-server:default:WebHookRoute" }, "webIdRoute": { "@id": "urn:solid-server:default:WebHookWebIdRoute" }, - "unsubscribeRoute": { "@id": "urn:solid-server:default:WebHookUnsubscribeRoute" }, "stateHandler": { "@type": "BaseStateHandler", "handler": { "@id": "urn:solid-server:default:WebHookNotificationHandler" }, diff --git a/src/server/notifications/BaseChannelType.ts b/src/server/notifications/BaseChannelType.ts index a3a7234f67..0a750240da 100644 --- a/src/server/notifications/BaseChannelType.ts +++ b/src/server/notifications/BaseChannelType.ts @@ -14,6 +14,7 @@ import type { InteractionRoute } from '../../identity/interaction/routing/Intera import { ContextDocumentLoader } from '../../storage/conversion/ConversionUtil'; import { UnprocessableEntityHttpError } from '../../util/errors/UnprocessableEntityHttpError'; import { IdentifierSetMultiMap } from '../../util/map/IdentifierMap'; +import { joinUrl } from '../../util/PathUtil'; import { readableToQuads } from '../../util/StreamUtil'; import { msToDuration } from '../../util/StringUtil'; import { NOTIFY, RDF, XSD } from '../../util/Vocabularies'; @@ -201,7 +202,9 @@ export abstract class BaseChannelType implements NotificationChannelType { /** * Converts a set of quads to a {@link NotificationChannel}. - * Assumes the data is valid, so this should be called after {@link validateSubscription} + * Assumes the data is valid, so this should be called after {@link validateSubscription}. + * + * The generated identifier will be a URL made by combining the base URL of the channel type with a unique identifier. * * The values of the default features will be added to the resulting channel, * subclasses with additional features that need to be added are responsible for parsing those quads. @@ -216,7 +219,7 @@ export abstract class BaseChannelType implements NotificationChannelType { const type = data.getObjects(subject, RDF.terms.type, null)[0] as NamedNode; const channel: NotificationChannel = { - id: `${v4()}:${topic.value}`, + id: joinUrl(this.path, v4()), type: type.value, topic: topic.value, }; diff --git a/src/server/notifications/WebHookSubscription2021/WebHook2021Util.ts b/src/server/notifications/WebHookSubscription2021/WebHook2021Util.ts deleted file mode 100644 index bfc4c4ca66..0000000000 --- a/src/server/notifications/WebHookSubscription2021/WebHook2021Util.ts +++ /dev/null @@ -1,20 +0,0 @@ -import { joinUrl } from '../../../util/PathUtil'; - -/** - * Generates a specific unsubscribe URL for a WebHookSubscription2021 - * by combining the default unsubscribe URL with the given identifier. - * @param url - The default unsubscribe URL. - * @param id - The identifier. - */ -export function generateWebHookUnsubscribeUrl(url: string, id: string): string { - return joinUrl(url, encodeURIComponent(id)); -} - -/** - * Parses a WebHookSubscription2021 unsubscribe URL to extract the identifier. - * @param url - The unsubscribe URL that is being called. - */ -export function parseWebHookUnsubscribeUrl(url: string): string { - // Split always returns an array of at least length 1 so result can not be undefined - return decodeURIComponent(url.split(/\//u).pop()!); -} diff --git a/src/server/notifications/WebHookSubscription2021/WebHookSubscription2021.ts b/src/server/notifications/WebHookSubscription2021/WebHookSubscription2021.ts index e0d29562d5..67eed3ddf0 100644 --- a/src/server/notifications/WebHookSubscription2021/WebHookSubscription2021.ts +++ b/src/server/notifications/WebHookSubscription2021/WebHookSubscription2021.ts @@ -9,7 +9,6 @@ import { BaseChannelType, DEFAULT_NOTIFICATION_FEATURES } from '../BaseChannelTy import type { NotificationChannel } from '../NotificationChannel'; import type { SubscriptionService } from '../NotificationChannelType'; import type { StateHandler } from '../StateHandler'; -import { generateWebHookUnsubscribeUrl } from './WebHook2021Util'; /** * A {@link NotificationChannel} containing the necessary fields for a WebHookSubscription2021 channel. @@ -57,19 +56,17 @@ export function isWebHook2021Channel(channel: NotificationChannel): channel is W export class WebHookSubscription2021 extends BaseChannelType { protected readonly logger = getLoggerFor(this); - private readonly unsubscribePath: string; private readonly stateHandler: StateHandler; private readonly webId: string; /** * @param route - The route corresponding to the URL of the subscription service of this channel type. * @param webIdRoute - The route to the WebID that needs to be used when generating DPoP tokens for notifications. - * @param unsubscribeRoute - The route where the request needs to be sent to unsubscribe. * @param stateHandler - The {@link StateHandler} that will be called after a successful subscription. * @param features - The features that need to be enabled for this channel type. */ - public constructor(route: InteractionRoute, webIdRoute: InteractionRoute, unsubscribeRoute: InteractionRoute, - stateHandler: StateHandler, features: string[] = DEFAULT_NOTIFICATION_FEATURES) { + public constructor(route: InteractionRoute, webIdRoute: InteractionRoute, stateHandler: StateHandler, + features: string[] = DEFAULT_NOTIFICATION_FEATURES) { super(NOTIFY.terms.WebHookSubscription2021, route, [ ...features, NOTIFY.webhookAuth ], @@ -80,7 +77,6 @@ export class WebHookSubscription2021 extends BaseChannelType { // which would make this more annoying so we are lenient here. // Could change in the future once this field is updated and part of the context. [{ path: NOTIFY.target, minCount: 1, maxCount: 1 }]); - this.unsubscribePath = unsubscribeRoute.getPath(); this.stateHandler = stateHandler; this.webId = webIdRoute.getPath(); } @@ -114,7 +110,7 @@ export class WebHookSubscription2021 extends BaseChannelType { webId, target: target.value, // eslint-disable-next-line @typescript-eslint/naming-convention - unsubscribe_endpoint: generateWebHookUnsubscribeUrl(this.unsubscribePath, channel.id), + unsubscribe_endpoint: channel.id, }; } diff --git a/src/server/notifications/WebHookSubscription2021/WebHookUnsubscriber.ts b/src/server/notifications/WebHookSubscription2021/WebHookUnsubscriber.ts index 2b57516ec7..7ee8825823 100644 --- a/src/server/notifications/WebHookSubscription2021/WebHookUnsubscriber.ts +++ b/src/server/notifications/WebHookSubscription2021/WebHookUnsubscriber.ts @@ -7,7 +7,6 @@ import { NotFoundHttpError } from '../../../util/errors/NotFoundHttpError'; import type { OperationHttpHandlerInput } from '../../OperationHttpHandler'; import { OperationHttpHandler } from '../../OperationHttpHandler'; import type { NotificationChannelStorage } from '../NotificationChannelStorage'; -import { parseWebHookUnsubscribeUrl } from './WebHook2021Util'; import { isWebHook2021Channel } from './WebHookSubscription2021'; /** @@ -27,7 +26,7 @@ export class WebHookUnsubscriber extends OperationHttpHandler { } public async handle({ operation, request }: OperationHttpHandlerInput): Promise { - const id = parseWebHookUnsubscribeUrl(operation.target.path); + const id = operation.target.path; const channel = await this.storage.get(id); if (!channel || !isWebHook2021Channel(channel)) { throw new NotFoundHttpError(); diff --git a/test/integration/WebHookSubscription2021.test.ts b/test/integration/WebHookSubscription2021.test.ts index 0e927988ce..07b6d4bc32 100644 --- a/test/integration/WebHookSubscription2021.test.ts +++ b/test/integration/WebHookSubscription2021.test.ts @@ -5,6 +5,9 @@ import { createRemoteJWKSet, jwtVerify } from 'jose'; import type { NamedNode } from 'n3'; import { DataFactory, Parser, Store } from 'n3'; import type { App } from '../../src/init/App'; +import type { + WebHookSubscription2021Channel, +} from '../../src/server/notifications/WebHookSubscription2021/WebHookSubscription2021'; import { matchesAuthorizationScheme } from '../../src/util/HeaderUtil'; import { joinUrl, trimTrailingSlashes } from '../../src/util/PathUtil'; import { readJsonStream } from '../../src/util/StreamUtil'; @@ -181,4 +184,15 @@ describe.each(stores)('A server supporting WebHookSubscription2021 using %s', (n // Close the connection so the server can shut down response.end(); }); + + it('allows a user to unsubscribe.', async(): Promise => { + const json = await subscribe(notificationType, webId, subscriptionUrl, topic, { [NOTIFY.target]: target }); + const unsubscribeUrl = (json as WebHookSubscription2021Channel).unsubscribe_endpoint; + let response = await fetch(unsubscribeUrl, { method: 'DELETE' }); + expect(response.status).toBe(403); + response = await fetch(unsubscribeUrl, { method: 'DELETE', headers: { authorization: `WebID ${webId}` }}); + expect(response.status).toBe(205); + response = await fetch(joinUrl(subscriptionUrl, 'abc'), { method: 'DELETE' }); + expect(response.status).toBe(404); + }); }); diff --git a/test/integration/WebSocketSubscription2021.test.ts b/test/integration/WebSocketSubscription2021.test.ts index 11945bd705..1d414ce4d8 100644 --- a/test/integration/WebSocketSubscription2021.test.ts +++ b/test/integration/WebSocketSubscription2021.test.ts @@ -6,7 +6,7 @@ import { BasicRepresentation } from '../../src/http/representation/BasicRepresen import type { App } from '../../src/init/App'; import type { ResourceStore } from '../../src/storage/ResourceStore'; import { joinUrl } from '../../src/util/PathUtil'; -import { NOTIFY } from '../../src/util/Vocabularies'; +import { NOTIFY, RDF } from '../../src/util/Vocabularies'; import { expectNotification, subscribe } from '../util/NotificationUtil'; import { getPort } from '../util/Util'; import { @@ -223,4 +223,29 @@ describe.each(stores)('A server supporting WebSocketSubscription2021 using %s', const message = (await messagePromise).toString(); expect(message).toBe('Notification channel has expired'); }); + + it('can use other RDF formats and content negotiation when creating a channel.', async(): Promise => { + const turtleChannel = ` + _:id <${RDF.type}> <${notificationType}> ; + <${topic}>. + `; + + const response = await fetch(subscriptionUrl, { + method: 'POST', + headers: { + authorization: `WebID ${webId}`, + 'content-type': 'text/turtle', + accept: 'text/turtle', + }, + body: turtleChannel, + }); + expect(response.status).toBe(200); + expect(response.headers.get('content-type')).toBe('text/turtle'); + + const parser = new Parser({ baseIRI: subscriptionUrl }); + const quads = new Store(parser.parse(await response.text())); + + expect(quads.getObjects(null, RDF.terms.type, null)).toEqual([ NOTIFY.terms.WebSocketSubscription2021 ]); + expect(quads.getObjects(null, NOTIFY.terms.topic, null)).toEqual([ namedNode(topic) ]); + }); }); diff --git a/test/unit/server/notifications/BaseChannelType.test.ts b/test/unit/server/notifications/BaseChannelType.test.ts index 8ee4baa878..c57274afef 100644 --- a/test/unit/server/notifications/BaseChannelType.test.ts +++ b/test/unit/server/notifications/BaseChannelType.test.ts @@ -29,7 +29,7 @@ class DummyChannelType extends BaseChannelType { } describe('A BaseChannelType', (): void => { - const id = '4c9b88c1-7502-4107-bb79-2a3a590c7aa3:https://storage.example/resource'; + const id = 'http://example.com/DummyType/4c9b88c1-7502-4107-bb79-2a3a590c7aa3'; const credentials: Credentials = {}; const channelType = new DummyChannelType(); diff --git a/test/unit/server/notifications/WebHookSubscription2021/WebHook2021Util.test.ts b/test/unit/server/notifications/WebHookSubscription2021/WebHook2021Util.test.ts deleted file mode 100644 index 42845a7f36..0000000000 --- a/test/unit/server/notifications/WebHookSubscription2021/WebHook2021Util.test.ts +++ /dev/null @@ -1,18 +0,0 @@ -import { - generateWebHookUnsubscribeUrl, parseWebHookUnsubscribeUrl, -} from '../../../../../src/server/notifications/WebHookSubscription2021/WebHook2021Util'; - -describe('WebHook2021Util', (): void => { - describe('#generateWebHookUnsubscribeUrl', (): void => { - it('generates the URL with the identifier.', async(): Promise => { - expect(generateWebHookUnsubscribeUrl('http://example.com/unsubscribe', '123$456')) - .toBe('http://example.com/unsubscribe/123%24456'); - }); - }); - - describe('#parseWebHookUnsubscribeUrl', (): void => { - it('returns the parsed identifier from the URL.', async(): Promise => { - expect(parseWebHookUnsubscribeUrl('http://example.com/unsubscribe/123%24456')).toBe('123$456'); - }); - }); -}); diff --git a/test/unit/server/notifications/WebHookSubscription2021/WebHookSubscription2021.test.ts b/test/unit/server/notifications/WebHookSubscription2021/WebHookSubscription2021.test.ts index 59e0486ab8..ab0bd5c4ab 100644 --- a/test/unit/server/notifications/WebHookSubscription2021/WebHookSubscription2021.test.ts +++ b/test/unit/server/notifications/WebHookSubscription2021/WebHookSubscription2021.test.ts @@ -18,7 +18,6 @@ import { isWebHook2021Channel, WebHookSubscription2021, } from '../../../../../src/server/notifications/WebHookSubscription2021/WebHookSubscription2021'; -import { joinUrl } from '../../../../../src/util/PathUtil'; import { NOTIFY, RDF } from '../../../../../src/util/Vocabularies'; import quad = DataFactory.quad; import blankNode = DataFactory.blankNode; @@ -41,7 +40,6 @@ describe('A WebHookSubscription2021', (): void => { let channel: WebHookSubscription2021Channel; const route = new AbsolutePathInteractionRoute('http://example.com/webhooks/'); const webIdRoute = new RelativePathInteractionRoute(route, '/webid'); - const unsubscribeRoute = new RelativePathInteractionRoute(route, '/unsubscribe'); let stateHandler: jest.Mocked; let channelType: WebHookSubscription2021; @@ -51,7 +49,7 @@ describe('A WebHookSubscription2021', (): void => { data.addQuad(quad(subject, NOTIFY.terms.topic, namedNode(topic))); data.addQuad(quad(subject, NOTIFY.terms.target, namedNode(target))); - const id = '4c9b88c1-7502-4107-bb79-2a3a590c7aa3:https://storage.example/resource'; + const id = 'http://example.com/webhooks/4c9b88c1-7502-4107-bb79-2a3a590c7aa3'; channel = { id, type: NOTIFY.WebHookSubscription2021, @@ -59,14 +57,14 @@ describe('A WebHookSubscription2021', (): void => { target, webId: 'http://example.org/alice', // eslint-disable-next-line @typescript-eslint/naming-convention - unsubscribe_endpoint: joinUrl(unsubscribeRoute.getPath(), encodeURIComponent(id)), + unsubscribe_endpoint: id, }; stateHandler = { handleSafe: jest.fn(), } as any; - channelType = new WebHookSubscription2021(route, webIdRoute, unsubscribeRoute, stateHandler); + channelType = new WebHookSubscription2021(route, webIdRoute, stateHandler); }); it('exposes a utility function to verify if a channel is a webhook channel.', async(): Promise => { diff --git a/test/unit/server/notifications/WebHookSubscription2021/WebHookUnsubscriber.test.ts b/test/unit/server/notifications/WebHookSubscription2021/WebHookUnsubscriber.test.ts index eb11ee95be..6a5df42d79 100644 --- a/test/unit/server/notifications/WebHookSubscription2021/WebHookUnsubscriber.test.ts +++ b/test/unit/server/notifications/WebHookSubscription2021/WebHookUnsubscriber.test.ts @@ -25,7 +25,7 @@ describe('A WebHookUnsubscriber', (): void => { beforeEach(async(): Promise => { operation = { method: 'DELETE', - target: { path: 'http://example.com/.notifications/webhooks/unsubscribe/134' }, + target: { path: 'http://example.com/.notifications/webhooks/134' }, preferences: {}, body: new BasicRepresentation(), }; @@ -58,6 +58,6 @@ describe('A WebHookUnsubscriber', (): void => { await expect(unsubscriber.handle({ operation, request, response })) .resolves.toEqual(new ResetResponseDescription()); expect(storage.delete).toHaveBeenCalledTimes(1); - expect(storage.delete).toHaveBeenLastCalledWith('134'); + expect(storage.delete).toHaveBeenLastCalledWith('http://example.com/.notifications/webhooks/134'); }); }); diff --git a/test/unit/server/notifications/WebSocketSubscription2021/WebSocketSubscription2021.test.ts b/test/unit/server/notifications/WebSocketSubscription2021/WebSocketSubscription2021.test.ts index 5c9b6992c1..967c2f73d1 100644 --- a/test/unit/server/notifications/WebSocketSubscription2021/WebSocketSubscription2021.test.ts +++ b/test/unit/server/notifications/WebSocketSubscription2021/WebSocketSubscription2021.test.ts @@ -34,7 +34,7 @@ describe('A WebSocketSubscription2021', (): void => { data.addQuad(quad(subject, RDF.terms.type, NOTIFY.terms.WebSocketSubscription2021)); data.addQuad(quad(subject, NOTIFY.terms.topic, namedNode(topic))); - const id = '4c9b88c1-7502-4107-bb79-2a3a590c7aa3:https://storage.example/resource'; + const id = 'http://example.com/foo/4c9b88c1-7502-4107-bb79-2a3a590c7aa3'; channel = { id, type: NOTIFY.WebSocketSubscription2021,