merge: look inside `url` when checking activity origin - #512 (!521)

View MR for information: https://activitypub.software/TransFem-org/Sharkey/-/merge_requests/521

Closes #512

Approved-by: Marie <marie@kaifa.ch>
Approved-by: fEmber <acomputerdog@gmail.com>

packages/backend/src/core/activitypub/misc/check-against-url.ts

@@ -4,16 +4,24 @@
  */
 import type { IObject } from '../type.js';
 
+function getHrefFrom(one: IObject|string): string | undefined {
+	if (typeof(one) === 'string') return one;
+	return one.href;
+}
+
 export function assertActivityMatchesUrls(activity: IObject, urls: string[]) {
 	const idOk = activity.id !== undefined && urls.includes(activity.id);
+	if (idOk) return;
 
-	// technically `activity.url` could be an `ApObject = IObject |
-	// string | (IObject | string)[]`, but if it's a complicated thing
-	// and the `activity.id` doesn't match, I think we're fine
-	// rejecting the activity
-	const urlOk = typeof(activity.url) === 'string' && urls.includes(activity.url);
+	const url = activity.url;
+	if (url) {
+		// `activity.url` can be an `ApObject = IObject | string | (IObject
+		// | string)[]`, we have to look inside it
+		const activityUrls = Array.isArray(url) ? url.map(getHrefFrom) : [getHrefFrom(url)];
+		const goodUrl = activityUrls.find(u => u && urls.includes(u));
 
-	if (!idOk && !urlOk) {
-		throw new Error(`bad Activity: neither id(${activity?.id}) nor url(${activity?.url}) match location(${urls})`);
+		if (goodUrl) return;
 	}
+
+	throw new Error(`bad Activity: neither id(${activity?.id}) nor url(${JSON.stringify(activity?.url)}) match location(${urls})`);
 }

packages/backend/test/unit/misc/check-against-url.ts

@@ -0,0 +1,51 @@
+/*
+ * SPDX-FileCopyrightText: dakkar and sharkey-project
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+
+import type { IObject } from '@/core/activitypub/type.js';
+import { describe, expect, test } from '@jest/globals';
+import { assertActivityMatchesUrls } from '@/core/activitypub/misc/check-against-url.js';
+
+function assertOne(activity: IObject) {
+	// return a function so we can use `.toThrow`
+	return () => assertActivityMatchesUrls(activity, ['good']);
+}
+
+describe('assertActivityMatchesUrls', () => {
+	test('id', () => {
+		expect(assertOne({ id: 'bad' })).toThrow(/bad Activity/);
+		expect(assertOne({ id: 'good' })).not.toThrow();
+	});
+
+	test('simple url', () => {
+		expect(assertOne({ url: 'bad' })).toThrow(/bad Activity/);
+		expect(assertOne({ url: 'good' })).not.toThrow();
+	});
+
+	test('array of urls', () => {
+		expect(assertOne({ url: ['bad'] })).toThrow(/bad Activity/);
+		expect(assertOne({ url: ['bad', 'other'] })).toThrow(/bad Activity/);
+		expect(assertOne({ url: ['good'] })).not.toThrow();
+		expect(assertOne({ url: ['bad', 'good'] })).not.toThrow();
+	});
+
+	test('array of objects', () => {
+		expect(assertOne({ url: [{ href: 'bad' }] })).toThrow(/bad Activity/);
+		expect(assertOne({ url: [{ href: 'bad' }, { href: 'other' }] })).toThrow(/bad Activity/);
+		expect(assertOne({ url: [{ href: 'good' }] })).not.toThrow();
+		expect(assertOne({ url: [{ href: 'bad' }, { href: 'good' }] })).not.toThrow();
+	});
+
+	test('mixed array', () => {
+		expect(assertOne({ url: [{ href: 'bad' }, 'other'] })).toThrow(/bad Activity/);
+		expect(assertOne({ url: [{ href: 'bad' }, 'good'] })).not.toThrow();
+		expect(assertOne({ url: ['bad', { href: 'good' }] })).not.toThrow();
+	});
+
+	test('id and url', () => {
+		expect(assertOne({ id: 'other', url: 'bad' })).toThrow(/bad Activity/);
+		expect(assertOne({ id: 'bad', url: 'good' })).not.toThrow();
+		expect(assertOne({ id: 'good', url: 'bad' })).not.toThrow();
+	});
+});