merge: Allow user-initiated object lookups (/ap/show endpoint) to follow cross-domain redirects (resolves #820) (!878)

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

Closes #820

Approved-by: dakkar <dakkar@thenautilus.net>
Approved-by: Marie <github@yuugi.dev>

3 files changed, 44 insertions, 7 deletions

diff --git a/packages/backend/src/core/activitypub/ApRequestService.ts b/packages/backend/src/core/activitypub/ApRequestService.ts
index 8036c9638f..aca322e745 100644
--- a/packages/backend/src/core/activitypub/ApRequestService.ts
+++ b/packages/backend/src/core/activitypub/ApRequestService.ts
@@ -185,7 +185,7 @@ export class ApRequestService {
 	 * @param url URL to fetch
 	 */
 	@bindThis
-	public async signedGet(url: string, user: { id: MiUser['id'] }, followAlternate?: boolean): Promise<unknown> {
+	public async signedGet(url: string, user: { id: MiUser['id'] }, followAlternate?: boolean): Promise<object> {
 		const _followAlternate = followAlternate ?? true;
 		const keypair = await this.userKeypairService.getUserKeypair(user.id);
 
@@ -239,7 +239,18 @@ export class ApRequestService {
 			try {
 				document.documentElement.innerHTML = html;
 
-				const alternate = document.querySelector('head > link[rel="alternate"][type="application/activity+json"]');
+				// Search for any matching value in priority order:
+				// 1. Type=AP > Type=none > Type=anything
+				// 2. Alternate > Canonical
+				// 3. Page order (fallback)
+				const alternate =
+					document.querySelector('head > link[href][rel="alternate"][type="application/activity+json"]') ??
+					document.querySelector('head > link[href][rel="canonical"][type="application/activity+json"]') ??
+					document.querySelector('head > link[href][rel="alternate"]:not([type])') ??
+					document.querySelector('head > link[href][rel="canonical"]:not([type])') ??
+					document.querySelector('head > link[href][rel="alternate"]') ??
+					document.querySelector('head > link[href][rel="canonical"]');
+
 				if (alternate) {
 					const href = alternate.getAttribute('href');
 					if (href) {
diff --git a/packages/backend/src/core/activitypub/type.ts b/packages/backend/src/core/activitypub/type.ts
index 119a9d8ccb..08bd224700 100644
--- a/packages/backend/src/core/activitypub/type.ts
+++ b/packages/backend/src/core/activitypub/type.ts
@@ -57,9 +57,16 @@ export function getOneApId(value: ApObject): string {
 }
 
 /**
+ * Minimal AP payload - just an object with optional ID.
+ */
+export interface ObjectWithId {
+	id?: string;
+}
+
+/**
  * Get ActivityStreams Object id
  */
-export function getApId(value: string | IObject | [string | IObject]): string {
+export function getApId(value: string | ObjectWithId | [string | ObjectWithId]): string {
 	// eslint-disable-next-line no-param-reassign
 	value = fromTuple(value);
 
@@ -71,7 +78,7 @@ export function getApId(value: string | IObject | [string | IObject]): string {
 /**
  * Get ActivityStreams Object id, or null if not present
  */
-export function getNullableApId(value: string | IObject | [string | IObject]): string | null {
+export function getNullableApId(value: string | ObjectWithId | [string | ObjectWithId]): string | null {
 	// eslint-disable-next-line no-param-reassign
 	value = fromTuple(value);
 
diff --git a/packages/backend/src/server/api/endpoints/ap/show.ts b/packages/backend/src/server/api/endpoints/ap/show.ts
index 616a77e337..4904e3cb87 100644
--- a/packages/backend/src/server/api/endpoints/ap/show.ts
+++ b/packages/backend/src/server/api/endpoints/ap/show.ts
@@ -4,11 +4,10 @@
  */
 
 import { Inject, Injectable } from '@nestjs/common';
-import ms from 'ms';
 import { Endpoint } from '@/server/api/endpoint-base.js';
 import type { MiNote } from '@/models/Note.js';
 import type { MiLocalUser, MiUser } from '@/models/User.js';
-import { isActor, isPost, getApId } from '@/core/activitypub/type.js';
+import { isActor, isPost, getApId, getNullableApId, ObjectWithId } from '@/core/activitypub/type.js';
 import type { SchemaType } from '@/misc/json-schema.js';
 import { ApResolverService } from '@/core/activitypub/ApResolverService.js';
 import { ApDbResolverService } from '@/core/activitypub/ApDbResolverService.js';
@@ -18,6 +17,8 @@ import { UserEntityService } from '@/core/entities/UserEntityService.js';
 import { NoteEntityService } from '@/core/entities/NoteEntityService.js';
 import { UtilityService } from '@/core/UtilityService.js';
 import { bindThis } from '@/decorators.js';
+import { ApRequestService } from '@/core/activitypub/ApRequestService.js';
+import { InstanceActorService } from '@/core/InstanceActorService.js';
 import { ApiError } from '../../error.js';
 
 export const meta = {
@@ -26,9 +27,10 @@ export const meta = {
 	requireCredential: true,
 	kind: 'read:account',
 
+	// Up to 30 calls, then 1 per 1/2 second
 	limit: {
-		duration: ms('1minute'),
 		max: 30,
+		dripRate: 500,
 	},
 
 	errors: {
@@ -94,6 +96,8 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
 		private apDbResolverService: ApDbResolverService,
 		private apPersonService: ApPersonService,
 		private apNoteService: ApNoteService,
+		private readonly apRequestService: ApRequestService,
+		private readonly instanceActorService: InstanceActorService,
 	) {
 		super(meta, paramDef, async (ps, me) => {
 			const object = await this.fetchAny(ps.uri, me);
@@ -118,6 +122,12 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
 		]));
 		if (local != null) return local;
 
+		// No local object found with that uri.
+		// Before we fetch, resolve the URI in case it has a cross-origin redirect or anything like that.
+		// Resolver.resolve() uses strict verification, which is overly paranoid for a user-provided lookup.
+		uri = await this.resolveCanonicalUri(uri); // eslint-disable-line no-param-reassign
+		if (!this.utilityService.isFederationAllowedUri(uri)) return null;
+
 		const host = this.utilityService.extractDbHost(uri);
 
 		// local object, not found in db? fail
@@ -167,4 +177,13 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
 
 		return null;
 	}
+
+	/**
+	 * Resolves an arbitrary URI to its canonical, post-redirect form.
+	 */
+	private async resolveCanonicalUri(uri: string): Promise<string> {
+		const user = await this.instanceActorService.getInstanceActor();
+		const res = await this.apRequestService.signedGet(uri, user, true) as ObjectWithId;
+		return getNullableApId(res) ?? uri;
+	}
 }