From 4b596afae56a2ed82a5c06249b555f268acbcc68 Mon Sep 17 00:00:00 2001 From: Jannis Braun <151788261+TheZwiss@users.noreply.github.com> Date: Wed, 1 Apr 2026 03:49:31 +0200 Subject: [PATCH] fix(federation): allow homeward relay in attribution check Client-federation users (e.g., youruser@nova logged into orbit) send DMs on the remote server. The S2S relay forwards these back to the author's home instance, but verifyAttribution rejected them because the author's homeInstance didn't match the sourceInstance. Now also accepts when the author's home matches the receiving instance (getOurOrigin()), covering the homeward relay case. --- .../src/routes/federation.attribution.test.ts | 47 +++++++++++++++++++ packages/server/src/routes/federation.ts | 17 +++++-- 2 files changed, 61 insertions(+), 3 deletions(-) create mode 100644 packages/server/src/routes/federation.attribution.test.ts diff --git a/packages/server/src/routes/federation.attribution.test.ts b/packages/server/src/routes/federation.attribution.test.ts new file mode 100644 index 00000000..0302cc70 --- /dev/null +++ b/packages/server/src/routes/federation.attribution.test.ts @@ -0,0 +1,47 @@ +import { describe, it, expect, vi } from 'vitest'; + +// Mock federationAuth before importing the module under test +vi.mock('../utils/federationAuth.js', () => ({ + getOurOrigin: () => 'https://nova.ddns.net', +})); + +import { verifyAttribution, extractDomain } from './federation.js'; + +describe('verifyAttribution', () => { + it('accepts when author home matches source instance (standard S2S)', () => { + expect(verifyAttribution('orbit.ddns.net', 'https://orbit.ddns.net')).toBe(true); + }); + + it('accepts when author home matches source with bare domain', () => { + expect(verifyAttribution('nova.ddns.net', 'https://nova.ddns.net')).toBe(true); + }); + + it('rejects when author home matches neither source nor receiver', () => { + expect(verifyAttribution('evil.net', 'https://orbit.ddns.net')).toBe(false); + }); + + it('accepts homeward relay — author home matches receiving instance', () => { + // Author from nova (home), source is orbit → relay going HOME → accept + // getOurOrigin() returns 'https://nova.ddns.net' (mocked above) + expect(verifyAttribution('nova.ddns.net', 'https://orbit.ddns.net')).toBe(true); + }); + + it('accepts homeward relay with full URL homeInstance', () => { + expect(verifyAttribution('https://nova.ddns.net', 'https://orbit.ddns.net')).toBe(true); + }); +}); + +describe('extractDomain', () => { + it('strips https:// prefix', () => { + expect(extractDomain('https://nova.ddns.net')).toBe('nova.ddns.net'); + }); + + it('returns bare domain unchanged', () => { + expect(extractDomain('nova.ddns.net')).toBe('nova.ddns.net'); + }); + + it('strips http:// prefix and port via URL constructor', () => { + // URL.hostname strips port — extractDomain returns bare hostname + expect(extractDomain('http://localhost:3000')).toBe('localhost'); + }); +}); diff --git a/packages/server/src/routes/federation.ts b/packages/server/src/routes/federation.ts index 7ab20634..d32727bc 100644 --- a/packages/server/src/routes/federation.ts +++ b/packages/server/src/routes/federation.ts @@ -1244,12 +1244,23 @@ export function extractDomain(homeInstance: string): string { } /** - * Verify that an acting user's homeInstance matches the source instance (X-Federation-Origin). - * In direct S2S federation, a peer should only send events for its own users. + * Verify that an acting user's homeInstance is legitimate for this relay. + * + * Two valid cases: + * 1. **Direct**: author is from the source instance (standard S2S — peer sends events for its own users). + * 2. **Homeward relay**: author is from the *receiving* instance. This happens when a client-federation + * user (e.g., youruser@nova logged into orbit) sends a message on a remote server, and the + * S2S relay forwards it back to the author's home instance. The trusted peer is just the messenger. + * * Both sides are normalized to bare domain before comparison. */ export function verifyAttribution(actingUserHomeInstance: string, sourceInstance: string): boolean { - return extractDomain(actingUserHomeInstance) === extractDomain(sourceInstance); + const authorDomain = extractDomain(actingUserHomeInstance); + // Case 1: author belongs to the source peer + if (authorDomain === extractDomain(sourceInstance)) return true; + // Case 2: homeward relay — author belongs to THIS (receiving) instance + if (authorDomain === extractDomain(getOurOrigin())) return true; + return false; } /**