merge: Fix activity verification after key rotation (!691)

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

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

packages/backend/src/core/activitypub/ApDbResolverService.ts

@@ -16,6 +16,7 @@ import type { MiLocalUser, MiRemoteUser } from '@/models/User.js';
 import { getApId } from './type.js';
 import { ApPersonService } from './models/ApPersonService.js';
 import type { IObject } from './type.js';
+import { ApLoggerService } from '@/core/activitypub/ApLoggerService.js';
 
 export type UriParseResult = {
 	/** wether the URI was generated by us */
@@ -53,6 +54,7 @@ export class ApDbResolverService implements OnApplicationShutdown {
 
 		private cacheService: CacheService,
 		private apPersonService: ApPersonService,
+		private apLoggerService: ApLoggerService,
 	) {
 		this.publicKeyCache = new MemoryKVCache<MiUserPublickey | null>(1000 * 60 * 60 * 12); // 12h
 		this.publicKeyByUserIdCache = new MemoryKVCache<MiUserPublickey | null>(1000 * 60 * 60 * 12); // 12h
@@ -174,10 +176,16 @@ export class ApDbResolverService implements OnApplicationShutdown {
 	 */
 	@bindThis
 	public async refetchPublicKeyForApId(user: MiRemoteUser): Promise<MiUserPublickey | null> {
+		this.apLoggerService.logger.debug('Re-fetching public key for user', { userId: user.id, uri: user.uri });
 		await this.apPersonService.updatePerson(user.uri);
+
 		const key = await this.userPublickeysRepository.findOneBy({ userId: user.id });
-		if (key != null) {
+		this.publicKeyByUserIdCache.set(user.id, key);
-			await this.publicKeyByUserIdCache.set(user.id, key);
+
+		if (key) {
+			this.apLoggerService.logger.info('Re-fetched public key for user', { userId: user.id, uri: user.uri });
+		} else {
+			this.apLoggerService.logger.warn('Failed to re-fetch key for user', { userId: user.id, uri: user.uri });
 		}
 		return key;
 	}

packages/backend/src/queue/processors/InboxProcessorService.ts

@@ -118,19 +118,16 @@ export class InboxProcessorService implements OnApplicationShutdown {
 		// HTTP-Signatureの検証
 		let httpSignatureValidated = httpSignature.verifySignature(signature, authUser.key.keyPem);
 
-		// また、signatureのsignerは、activity.actorと一致する必要がある
+		// maybe they changed their key? refetch it
-		if (!httpSignatureValidated || authUser.user.uri !== activity.actor) {
+		if (!httpSignatureValidated) {
-			let renewKeyFailed = true;
+			authUser.key = await this.apDbResolverService.refetchPublicKeyForApId(authUser.user);
-
+			if (authUser.key != null) {
-			if (!httpSignatureValidated) {
+				httpSignatureValidated = httpSignature.verifySignature(signature, authUser.key.keyPem);
-				authUser.key = await this.apDbResolverService.refetchPublicKeyForApId(authUser.user);
-
-				if (authUser.key != null) {
-					httpSignatureValidated = httpSignature.verifySignature(signature, authUser.key.keyPem);
-					renewKeyFailed = false;
-				}
 			}
+		}
 
+		// また、signatureのsignerは、activity.actorと一致する必要がある
+		if (!httpSignatureValidated || authUser.user.uri !== getApId(activity.actor)) {
 			// 一致しなくても、でもLD-Signatureがありそうならそっちも見る
 			const ldSignature = activity.signature;
 			if (ldSignature) {

packages/backend/src/server/ActivityPubServerService.ts

@@ -205,15 +205,11 @@ export class ActivityPubServerService {
 
 		let httpSignatureValidated = httpSignature.verifySignature(signature, authUser.key.keyPem);
 
+		// maybe they changed their key? refetch it
 		if (!httpSignatureValidated) {
-			this.authlogger.info(`${logPrefix} failed to validate signature, re-fetching the key for ${authUser.user.uri}`);
-			// maybe they changed their key? refetch it
 			authUser.key = await this.apDbResolverService.refetchPublicKeyForApId(authUser.user);
-
 			if (authUser.key != null) {
 				httpSignatureValidated = httpSignature.verifySignature(signature, authUser.key.keyPem);
-			} else {
-				this.authlogger.warn(`${logPrefix} failed to re-fetch key for ${authUser.user}`);
 			}
 		}