Skip to content

Commit cbf6784

Browse files
authored
Merge pull request #435 from dahlia/refactor/remove-quote-verification-workarounds
Verify quotes with Fedify 2.5 APIs
2 parents b44225e + 952ba02 commit cbf6784

12 files changed

Lines changed: 436 additions & 146 deletions

‎federation/inbox/dispatch.test.ts‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,82 @@ import {
2929
} from "./dispatch.ts";
3030

3131
describe("transactional inbox dispatch", () => {
32+
it("rejects redirected quote authorization IDs before accepting or persisting them", async () => {
33+
await withRollback(async (tx) => {
34+
const author = await insertRemoteActor(tx, {
35+
username: "redirectedquoteowner",
36+
name: "Redirected Quote Owner",
37+
host: "remote.example",
38+
});
39+
const target = await insertRemotePost(tx, {
40+
actorId: author.id,
41+
contentHtml: "<p>Redirect target</p>",
42+
});
43+
const quoter = await insertAccountWithActor(tx, {
44+
username: "redirectedquoter",
45+
name: "Redirected Quoter",
46+
email: "redirectedquoter@example.com",
47+
});
48+
const { post: quote } = await insertNotePost(tx, {
49+
account: quoter.account,
50+
content: "Pending quote",
51+
quotedPostId: target.id,
52+
});
53+
const requestIri = new URL("#quote-request/redirect", quote.iri).href;
54+
await tx.insert(quoteRequestTable).values({
55+
id: generateUuidV7(),
56+
iri: requestIri,
57+
quotePostId: quote.id,
58+
quotedPostId: target.id,
59+
});
60+
const expectedId = "https://remote.example/authorizations/expected";
61+
const fetchedId = "https://remote.example/authorizations/different";
62+
const authorization = new QuoteAuthorization({
63+
id: new URL(fetchedId),
64+
attribution: new URL(author.iri),
65+
interactingObject: new URL(quote.iri),
66+
interactionTarget: new URL(target.iri),
67+
});
68+
const accept = new Accept({
69+
id: new URL("https://remote.example/accept"),
70+
actor: new URL(author.iri),
71+
object: new URL(requestIri),
72+
result: new URL(expectedId),
73+
});
74+
const fedCtx = {
75+
...createFedCtx(tx),
76+
async documentLoader(url: string) {
77+
if (url === requestIri) throw new Error("Use the stored request");
78+
assert.equal(url, expectedId);
79+
return {
80+
contextUrl: null,
81+
documentUrl: fetchedId,
82+
document: await authorization.toJsonLd(),
83+
};
84+
},
85+
async sendActivity() {
86+
assert.fail("An invalid authorization must not publish an update");
87+
},
88+
} as unknown as InboxContext<ContextData>;
89+
await onAccepted(fedCtx, accept);
90+
// The real getter cached the fetched ID; verification used the signed one.
91+
assert.equal(accept.resultId?.href, fetchedId);
92+
const storedQuote = await tx.query.postTable.findFirst({
93+
where: { id: quote.id },
94+
});
95+
assert.equal(storedQuote?.quoteAuthorizationIri, null);
96+
const request = await tx.query.quoteRequestTable.findFirst({
97+
where: { iri: requestIri },
98+
});
99+
assert.equal(request?.accepted, null);
100+
assert.equal(
101+
await tx.query.quoteAuthorizationTable.findFirst({
102+
where: { iri: fetchedId },
103+
}),
104+
undefined,
105+
);
106+
});
107+
});
32108
it("accepts Mastodon quote approvals with an unresolvable request fragment", async () => {
33109
await withRollback(async (tx) => {
34110
const remoteActor = await insertRemoteActor(tx, {

‎federation/inbox/dispatch.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@ export async function onAccepted(
2424
fedCtx: InboxContext<ContextData>,
2525
accept: Accept,
2626
): Promise<void> {
27-
if (accept.resultId == null) {
27+
const resultId = accept.resultId;
28+
if (resultId == null) {
2829
const handledByIri = await withInboxTransaction(
2930
fedCtx,
3031
async (txCtx) =>
@@ -58,7 +59,9 @@ export async function onAccepted(
5859
]);
5960

6061
const handled = await withInboxTransaction(fedCtx, async (txCtx) => {
61-
if (await onQuoteRequestAccepted(txCtx, accept, { object, result })) {
62+
if (
63+
await onQuoteRequestAccepted(txCtx, accept, { object, result, resultId })
64+
) {
6265
return true;
6366
}
6467
return await onRelayFollowAccepted(txCtx, accept);

‎federation/inbox/quote.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1521,6 +1521,7 @@ test("unknown quote approvals honor the state read after locking", async () => {
15211521
await onQuoteRequestAccepted(fedCtx, accept, {
15221522
object: request,
15231523
result: authorization,
1524+
resultId: new URL(authorizationIri),
15241525
}),
15251526
true,
15261527
);

‎federation/inbox/quote.ts‎

Lines changed: 26 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -323,37 +323,28 @@ async function getValidQuoteRequestInstrument(
323323
instrument.id.origin !== request.actorId.origin
324324
)
325325
return;
326-
// Preserve quoteUrl and multi-attribution compatibility while letting the
327-
// helper validate the normalized request. The original object is persisted.
328-
const matchingTarget = [instrument.quoteId, instrument.quoteUrl].find(
329-
(id) => id?.href === request.objectId?.href,
330-
);
331-
const normalized = instrument.clone({
332-
quote: matchingTarget ?? instrument.quoteId,
333-
quoteUrl: matchingTarget ?? instrument.quoteUrl,
334-
attributions: instrument.attributionIds.some(
335-
(id) => id.href === request.actorId!.href,
336-
)
337-
? [request.actorId]
338-
: instrument.attributionIds,
339-
});
340326
// The local target was resolved from the DB, including share wrappers;
341-
// embed its requested ID so verification never fetches our own public URL.
327+
// compare the instrument against the requested IRI, while authorizations
328+
// continue to refer to the original post behind a share.
342329
const result = await quoteInteraction.verifyRequest(fedCtx, {
343-
request: request.clone({
344-
object: new Note({ id: request.objectId }),
345-
instrument: normalized,
346-
}),
330+
request,
331+
resolvedInteractionTarget: new Note({ id: request.objectId }),
332+
resolvedInteractingObject: instrument,
333+
quoteReference: "any",
334+
attribution: "any",
335+
missingAttribution: "reject",
347336
});
348337
return result.verified ? { instrument, instrumentIri } : undefined;
349338
}
350339

351340
export async function onQuoteRequestAccepted(
352341
fedCtx: InboxContext<ContextData>,
353342
accept: Accept,
354-
resolved?: Readonly<{ object: unknown; result: unknown }>,
343+
resolved?: Readonly<{ object: unknown; result: unknown; resultId: URL }>,
355344
): Promise<boolean> {
356-
if (accept.actorId == null || accept.resultId == null) return false;
345+
// Dereferencing result can replace the signed reference with a fetched ID.
346+
const resultId = resolved?.resultId ?? accept.resultId;
347+
if (accept.actorId == null || resultId == null) return false;
357348
let quoteRequestIri = accept.objectId?.href;
358349
if (quoteRequestIri != null) {
359350
quoteRequestIri = await resolveAcceptedQuoteRequestIri(
@@ -438,18 +429,26 @@ export async function onQuoteRequestAccepted(
438429
// Signed author approval permits authorization aliases on another origin.
439430
const validAuthorization =
440431
authorization instanceof QuoteAuthorization &&
441-
authorization.id?.href === accept.resultId.href &&
442-
authorization.interactingObjectId?.href === quote.iri &&
443-
authorization.interactionTargetId?.href === quotedPost.iri &&
444-
authorization.attributionId?.href === quotedPost.actor.iri;
432+
(
433+
await quoteInteraction.verifyAuthorization(fedCtx, {
434+
authorization,
435+
authorizationId: resultId,
436+
interactingObject: new URL(quote.iri),
437+
interactionTarget: new URL(quotedPost.iri),
438+
attributedTo: new URL(quotedPost.actor.iri),
439+
allowOffOrigin: true,
440+
// The signed Accept's actor was checked against the target's owner.
441+
verifyAuthenticity: () => true,
442+
})
443+
).verified;
445444
if (!validAuthorization) {
446445
logger.warn("Ignoring invalid quote authorization: {iri}", {
447-
iri: accept.resultId.href,
446+
iri: resultId.href,
448447
});
449448
return true;
450449
}
451450
const accepted = new Date();
452-
const resultIri = accept.resultId.href;
451+
const resultIri = resultId.href;
453452
const shouldSendUpdate =
454453
quote.quoteAuthorizationIri !== resultIri ||
455454
quote.quotedPostId !== quotedPost.id;

‎federation/interaction-controls.test.ts‎

Lines changed: 137 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,7 @@ test("quote handler verifies compatible instruments without fetching local targe
154154
"wrong target",
155155
"wrong author",
156156
"cross-origin instrument",
157+
"missing author",
157158
]) {
158159
const requester = {
159160
id: "requester",
@@ -231,9 +232,11 @@ test("quote handler verifies compatible instruments without fetching local targe
231232
: "https://remote.example/quote",
232233
),
233234
attributions:
234-
variant === "multiple authors"
235-
? [other, new URL(requester.iri)]
236-
: [variant === "wrong author" ? other : new URL(requester.iri)],
235+
variant === "missing author"
236+
? []
237+
: variant === "multiple authors"
238+
? [other, new URL(requester.iri)]
239+
: [variant === "wrong author" ? other : new URL(requester.iri)],
237240
quote:
238241
variant === "quoteUrl"
239242
? null
@@ -254,18 +257,40 @@ test("quote handler verifies compatible instruments without fetching local targe
254257
},
255258
});
256259
}
260+
const request = new QuoteRequest({
261+
id: new URL("https://remote.example/request"),
262+
actor: new URL(requester.iri),
263+
object: new URL(target.iri),
264+
instrument:
265+
variant === "redirected instrument"
266+
? new URL("https://remote.example/alias")
267+
: instrument,
268+
});
257269
await onQuoteRequested(
258270
raw as unknown as InboxContext<ContextData>,
259-
new QuoteRequest({
260-
id: new URL("https://remote.example/request"),
261-
actor: new URL(requester.iri),
262-
object: new URL(target.iri),
263-
instrument:
264-
variant === "redirected instrument"
265-
? new URL("https://remote.example/alias")
266-
: instrument,
267-
}),
271+
request,
272+
);
273+
assert.equal(request.id?.href, "https://remote.example/request");
274+
assert.equal(request.actorId?.href, requester.iri);
275+
assert.equal(request.objectId?.href, target.iri);
276+
assert.equal(
277+
instrument.quoteUrl?.href,
278+
variant === "wrong target" ? other.href : target.iri,
268279
);
280+
assert.equal(
281+
instrument.quoteId?.href,
282+
variant === "quoteUrl"
283+
? undefined
284+
: variant === "conflicting quote" || variant === "wrong target"
285+
? other.href
286+
: target.iri,
287+
);
288+
if (variant === "multiple authors") {
289+
assert.deepEqual(
290+
instrument.attributionIds.map((id) => id.href),
291+
[other.href, requester.iri],
292+
);
293+
}
269294
const allowed = [
270295
"quoteUrl",
271296
"conflicting quote",
@@ -284,3 +309,103 @@ test("quote handler verifies compatible instruments without fetching local targe
284309
}
285310
}
286311
});
312+
313+
test("stored authorization helper requires DB evidence and exact bindings for aliases", async () => {
314+
const ctx = createContext();
315+
const id = new URL("https://alias.example/authorization");
316+
const author = new URL("https://author.example/actor");
317+
const quote = new URL("https://quoter.example/quote");
318+
const target = new URL("https://author.example/post");
319+
const other = new URL("https://other.example/object");
320+
for (const variant of [
321+
"valid",
322+
"no evidence",
323+
"idless",
324+
"wrong ID",
325+
"wrong quote",
326+
"wrong target",
327+
"wrong author",
328+
]) {
329+
const authorization = new QuoteAuthorization({
330+
id: variant === "idless" ? null : variant === "wrong ID" ? other : id,
331+
attribution: variant === "wrong author" ? other : author,
332+
interactingObject: variant === "wrong quote" ? other : quote,
333+
interactionTarget: variant === "wrong target" ? other : target,
334+
});
335+
assert.equal(
336+
await federationServices.verifyStoredQuoteAuthorization(
337+
ctx,
338+
authorization,
339+
{
340+
authorizationId: id,
341+
interactingObject: quote,
342+
interactionTarget: target,
343+
attributedTo: author,
344+
authentic: variant !== "no evidence",
345+
},
346+
),
347+
variant === "valid",
348+
variant,
349+
);
350+
}
351+
});
352+
353+
test("authorization verification separates document/context loaders with a default context fallback", async () => {
354+
const ctx = createContext();
355+
const id = new URL("https://author.example/authorization");
356+
const author = new URL("https://author.example/actor");
357+
const quote = new URL("https://quoter.example/quote");
358+
const target = new URL("https://author.example/post");
359+
const authorization = new QuoteAuthorization({
360+
id,
361+
attribution: author,
362+
interactingObject: quote,
363+
interactionTarget: target,
364+
});
365+
const document = await authorization.toJsonLd();
366+
assert.ok(
367+
document != null && typeof document === "object" && "@context" in document,
368+
);
369+
for (const separate of [false, true]) {
370+
const contextUrl = "https://contexts.example/custom";
371+
const contexts: string[] = [];
372+
const documents: string[] = [];
373+
assert.equal(
374+
await federationServices.verifyQuoteAuthorization(
375+
ctx,
376+
id,
377+
quote,
378+
target,
379+
author,
380+
async (url) => {
381+
documents.push(url);
382+
assert.equal(url, id.href);
383+
return {
384+
document: separate
385+
? { ...document, "@context": contextUrl }
386+
: document,
387+
documentUrl: url,
388+
contextUrl: null,
389+
};
390+
},
391+
separate
392+
? async (url) => {
393+
contexts.push(url);
394+
if (url === contextUrl) {
395+
return {
396+
document: { "@context": document["@context"] },
397+
documentUrl: url,
398+
contextUrl: null,
399+
};
400+
}
401+
return ctx.contextLoader(url);
402+
}
403+
: undefined,
404+
),
405+
true,
406+
);
407+
assert.deepEqual(documents, [id.href]);
408+
if (separate) assert.ok(contexts.includes(contextUrl));
409+
else assert.deepEqual(contexts, []);
410+
}
411+
});

‎federation/services.ts‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,21 @@ export const federationServices: FederationServices<ApplicationContext> = {
4747
interactingObject,
4848
interactionTarget,
4949
attributedTo,
50-
documentLoader: (url) =>
51-
url === authorization.href
52-
? documentLoader(url)
53-
: (contextLoader ?? getDocumentLoader())(url),
50+
documentLoader,
51+
contextLoader: contextLoader ?? getDocumentLoader(),
52+
})
53+
).verified,
54+
verifyStoredQuoteAuthorization: async (context, authorization, options) =>
55+
(
56+
await quoteInteraction.verifyAuthorization(getFedifyContext(context), {
57+
authorization,
58+
authorizationId: options.authorizationId,
59+
interactingObject: options.interactingObject,
60+
interactionTarget: options.interactionTarget,
61+
attributedTo: options.attributedTo,
62+
// Stored, unrevoked grants authenticate aliases independently of origin.
63+
allowOffOrigin: true,
64+
verifyAuthenticity: () => options.authentic,
5465
})
5566
).verified,
5667
subscribeTagsPubHashtag: (context, tag) =>

0 commit comments

Comments
 (0)