feat: add mastodon style relays #109

TheNoim · 2025-02-15T21:53:58Z

In this PR I try to add mastodon style relays conforming to FEP-ae0c.

Todos

src/api/v1/statuses.ts

ThisIsMissEm · 2025-02-15T22:29:17Z

src/entities/relay.ts

+ *  "id": "https://client.example/6ae15297",
+ *  "type": "Follow",
+ *  "actor": "https://client.example/actor",
+ *  "object": "https://www.w3.org/ns/activitystreams#Public"


He the relay should actually understand both, as they're semantically the same, I'd file a bug with the relay implementation

I am not sure about this.

The object of the Follow request MUST be the fully expanded URI of the Public pseudo-collection (https://www.w3.org/ns/activitystreams#Public).

As far as I understand, but you can correct me:

https://www.w3.org/ns/activitystreams#Public fully expanded

as:public not fully expanded

Imagine building a new relay software: I would look at the spec and see that the object property should be equal to https://www.w3.org/ns/activitystreams#Public. So I would just check for this. This is why I think using https://www.w3.org/ns/activitystreams#Public would make it more compatible with more relay softwares. But at this point I only tested one software.

dahlia

Thanks for your amazing work, @TheNoim! I'm really looking forward to shipping this feature!

I left several review comments. Could you address them?

src/api/v1/statuses.ts

src/federation/inbox.ts

src/api/v1/statuses.ts

ThisIsMissEm · 2025-02-16T01:01:32Z

Would probably also make sense to abstract out functions for determining "should this be relayed" and "forward this to a relay"

TheNoim · 2025-02-16T19:10:08Z

@dahlia can you look into the verify key issue? You can use https://relay2.bergsman.de/actor for testing.

dahlia · 2025-02-17T09:52:46Z

@dahlia can you look into the verify key issue? You can use https://relay2.bergsman.de/actor for testing.

It seems that Fedify fails to parse its public key is because it's encoded in PEM-PKCS#1 whereas Fedify expects PEM-SPKI, which is much more common in the fediverse (as far as I know). Hmm… should Fedify accept PEM-PKCS#1 as well? 🤔

TheNoim · 2025-02-17T10:05:58Z

@dahlia can you look into the verify key issue? You can use https://relay2.bergsman.de/actor for testing.

It seems that Fedify fails to parse its public key is because it's encoded in PEM-PKCS#1 whereas Fedify expects PEM-SPKI, which is much more common in the fediverse (as far as I know). Hmm… should Fedify accept PEM-PKCS#1 as well? 🤔

I think, yes 🙃 As far as I know, it is not against the spec. Even though I think this is not the only issue with the tested relay software. But this would bring it one step further. I naively tried to patch fedify yesterday, but what I did was 100% wrong 🥲

dahlia · 2025-02-18T05:36:56Z

Now Fedify accepts PEM-PKCS#1-encoded RSA public keys (besides PEM-SPKI-encoded ones), and it will be shipped in Fedify v1.5.0, the next minor release. Until then, you could give it a try Fedify v1.5.0-dev.654+b5166915, which is an unstable release.

TheNoim · 2025-02-19T19:41:58Z

The next issue I found. This is the http signature:

keyId="https://relay2.bergsman.de/actor",algorithm="rsa-sha256",headers="(request-target) host date digest content-type",signature="gavImwimAZcFMcLiCPhmg87I5n5fsAnBDSUNSaiPhnSvev5A0bgxaXnMlu2PL3kB1Ru5eWxAPiFEMPmoYicJZkN9sCgVd/CSEVCP3dnbeHH5HsT+G43MhHZcd3Ofg4xtO0yziXNQkwVoBhsN63iXSfxxqwipIKg5yp/3o3YDWUWKf6mGd/ykO95Dq5lpgQmd5WpRJ3udYggjITBtJ5bNA+kGWgWJxHq2NPixiEhQcwld1J3o7BW/5UjOS64lbP1/p2I3LAOCnLe8sBD4zj72NPdLaXLnSG4x3pfloLo4xAV5xJl4eScbfrzSNeWp+D2YiTr9tc5l3VQl4kZKP93aHg=="

The keyId is set to https://relay2.bergsman.de/actor. Fedify fetches the key from https://relay2.bergsman.de/actor, but will not find the key there. The key there has the id https://relay2.bergsman.de/actor#main-key which is obviously != https://relay2.bergsman.de/actor. See https://github.com/fedify-dev/fedify/blob/4eb092ee66e5b66a5760b8178b3e1272bb4c7abf/src/sig/key.ts#L330

I am not sure if this is also an incorrect behavior of fedify. https://swicg.github.io/activitypub-http-signature/#how-to-obtain-a-signature-s-public-key

If it has a fragment, discard it.

Maybe the spec is meant this way: Fetch the key from keyId. If keyId specifies a specific key, use it. Otherwise, use first.

Damn, activity pub is harder than I could have ever imagined 😁 Respect for implementing fedify.

dahlia · 2025-02-20T02:46:50Z

I am not sure if this is also an incorrect behavior of fedify. https://swicg.github.io/activitypub-http-signature/#how-to-obtain-a-signature-s-public-key

If it has a fragment, discard it.

Maybe the spec is meant this way: Fetch the key from keyId. If keyId specifies a specific key, use it. Otherwise, use first.

Whether it is Fedify's fault or not, Fedify should behave as liberal as possible in accordance with the robustness principle—be conservative in what you do, be liberal in what you accept from others. So I just filed an issue: fedify-dev/fedify#211.

dahlia · 2025-02-20T03:42:03Z

Fedify now does the best to find the appropriate public key. If there is no fragment in keyId and the actor has only one public key, it chooses that key. It will be shipped in Fedify 1.5.0, the next minor release, but you can try it with Fedify 1.5.0-dev.672+6a6ed659, an unstable release.

TheNoim · 2025-05-03T11:17:59Z

I am not sure how to go forward with this PR. The most important parts are done. I may consider change it state from draft to ready. Everything else can be changed/implemented later. Otherwise this PR might stay in a infinite draft limbo. Any option?

dahlia · 2025-05-03T11:52:41Z

If you change it to ready, I'll review it!

dahlia

I'd like to test this on my local setup. Could you let me know how to test it? Sorry, I'm not familiar with ActivityPub relays.

dahlia · 2025-05-03T11:57:34Z

src/entities/relay.ts

+export class RelayUndo extends Undo {
+  async toJsonLd(options?: {
+    format?: "compact" | "expand";
+    contextLoader?: DocumentLoader;
+    context?:
+      | string
+      | Record<string, string>
+      | (string | Record<string, string>)[];
+  }): Promise<unknown> {
+    await this.getObject();
+    const json = (await super.toJsonLd(options)) as { object: unknown };
+    json.object = await (await this.getObject())?.toJsonLd();
+    return json;
+  }
+}


Instead of having this subclass, how about hydrating the property before calling toJsonLd()?

dahlia · 2025-05-03T12:00:39Z

src/pages/accounts.tsx

@@ -67,6 +68,7 @@ accounts.use(loginRequired);

 accounts.get("/", async (c) => {
  const owners = await db.query.accountOwners.findMany({
+    where: ne(accountOwners.handle, HOLLO_RELAY_ACTOR_USERNAME),


Sometimes you've compare account_owners.id with HOLLO_RELAY_ACTOR_ID, and sometimes account_owners.handle with HOLLO_RELAY_ACTOR_USERNAME. Is there any difference between both ways?

I think this check should be using the ID here, not the username.

This may also affect the setup page, which checks the number of actors on the server.

Oh, it may affect the NodeInfo stats as well:

hollo/src/federation/nodeinfo.ts

Lines 18 to 28 in 65c2b11

const [{ total }] = await db.select({ total: count() }).from(accountOwners);

const [{ activeMonth }] = await db

.select({ activeMonth: countDistinct(accountOwners.id) })

.from(accountOwners)

.rightJoin(posts, eq(accountOwners.id, posts.accountId))

.where(gt(posts.updated, sql`CURRENT_TIMESTAMP - INTERVAL '1 month'`));

const [{ activeHalfyear }] = await db

.select({ activeHalfyear: countDistinct(accountOwners.id) })

.from(accountOwners)

.rightJoin(posts, eq(accountOwners.id, posts.accountId))

.where(gt(posts.updated, sql`CURRENT_TIMESTAMP - INTERVAL '6 months'`));

dahlia · 2025-05-03T12:02:02Z

src/pages/accounts.tsx

@@ -119,6 +121,28 @@ accounts.post("/", async (c) => {
  const bioResult = await formatText(db, bio ?? "", fedCtx);
  const nameEmojis = await extractCustomEmojis(db, name);
  const emojis = { ...nameEmojis, ...bioResult.emojis };
+  const handle = `@${username}@${fedCtx.host}`;
+  if (handle === `@${HOLLO_OFFICIAL_ACCOUNT}@${fedCtx.host}`) {


I guess it's a bug?

Suggested change

if (handle === `@${HOLLO_OFFICIAL_ACCOUNT}@${fedCtx.host}`) {

if (username === HOLLO_RELAY_ACTOR_USERNAME) {

TheNoim · 2025-05-03T12:22:33Z

I'd like to test this on my local setup. Could you let me know how to test it? Sorry, I'm not familiar with ActivityPub relays.

I started two test relay servers on my server.

https://relay.bergsman.de/inbox (AodeRelay)
https://relay2.bergsman.de/inbox (yukimochi relay)

And used a test mastodon instance. And sometimes I used some public relay that don't require approve.

ThisIsMissEm · 2025-05-25T19:42:17Z

src/oauth.tsx

@@ -55,6 +57,7 @@ app.get(
      return c.json({ error: "invalid_redirect_uri" }, 400);
    }
    const accountOwners = await db.query.accountOwners.findMany({
+      where: not(eq(accountOwnersTable.id, HOLLO_RELAY_ACTOR_ID)),


We'll definitely need a test case for this, but probably after #154 lands.

ThisIsMissEm · 2025-05-25T19:43:07Z

src/schema.ts

+  relayServerActorId: uuid("relay_server_actor_id")
+    .$type<Uuid>()
+    .references(() => accounts.id, { onDelete: "cascade" }),


Should this be not-null?

ThisIsMissEm · 2025-05-25T19:44:53Z

src/schema.ts

+    .$type<Uuid>()
+    .references(() => accounts.id, { onDelete: "cascade" }),
+  state: relayStateEnum("state").notNull().default("idle"),
+  followRequestId: text("follow_request_id").notNull().unique(),


Shouldn't this reference a sent follow request? Though maybe we're not storing in the database the follow requests we've sent? cc @dahlia

Looks like we could actually store the follow request, by doing an insert to follows for the relay actor, and having approved be null?

From: https://github.com/dahlia/hollo/blob/9d5aa116141e43ed824ea8b44ba621f5978143c8/src/api/v1/follow_requests.ts

const followers = await db.query.follows.findMany({ where: and(eq(follows.followingId, owner.id), isNull(follows.approved)), with: { follower: { with: { owner: true, successor: true } } }, });

Yeah, and as the primary key of the follows table is iri, the column should be renamed to followRequestIri:

Suggested change

followRequestId: text("follow_request_id").notNull().unique(),

followRequestIri: text("follow_request_iri")

.notNull()

.unique()

.references(() => follows.iri, { onDelete: "cascade" }),

ThisIsMissEm · 2025-05-25T19:46:18Z

src/pages/federation.tsx

+import { exportJwk, generateCryptoKeyPair, isActor } from "@fedify/fedify";
+import { Temporal } from "@js-temporal/polyfill";
+import { count, eq, sql } from "drizzle-orm";
+import { interval, jsonb, pgTable, timestamp, uuid } from "drizzle-orm/pg-core";


You've a few unused variables here.

ThisIsMissEm · 2025-05-25T19:47:06Z

src/pages/accounts.tsx

@@ -67,6 +68,7 @@ accounts.use(loginRequired);

 accounts.get("/", async (c) => {
  const owners = await db.query.accountOwners.findMany({
+    where: ne(accountOwners.handle, HOLLO_RELAY_ACTOR_USERNAME),


I think this check should be using the ID here, not the username.

ThisIsMissEm · 2025-05-25T19:47:32Z

src/pages/accounts.tsx

@@ -67,6 +68,7 @@ accounts.use(loginRequired);

 accounts.get("/", async (c) => {
  const owners = await db.query.accountOwners.findMany({
+    where: ne(accountOwners.handle, HOLLO_RELAY_ACTOR_USERNAME),


This may also affect the setup page, which checks the number of actors on the server.

ThisIsMissEm · 2025-05-25T19:48:37Z

src/pages/federation.tsx

@@ -160,4 +243,202 @@ data.post("/refresh", async (c) => {
  return c.redirect("/federation?error=refresh");
 });

+data.post("/relay", async (c) => {


I'd be inclined to move this to a dedicated relays page, as this file already is quite large.

ThisIsMissEm · 2025-05-25T19:48:53Z

src/pages/federation.tsx

+  const form = await c.req.formData();
+
+  const inboxUrl = form.get("inbox_url");


Missing form validation.

ThisIsMissEm · 2025-05-25T19:49:35Z

src/pages/federation.tsx

+    const existingRelay = await tx.query.relays.findFirst({
+      where: eq(relays.inboxUrl, inboxUrl),
+    });
+
+    if (existingRelay) {
+      throw tx.rollback();
+    }


You could do this first, and then not need to bail on the insert.

ThisIsMissEm · 2025-05-25T19:50:58Z

src/pages/federation.tsx

+    // Create follow request for this relay
+    const followRequestId = new URL(
+      `#relay-follows/${crypto.randomUUID()}`,
+      account[0].iri,
+    );


Should this actually be a real Object ID, and not using the hash fragment?

Perhaps we could use followAccount instead? Since that'll send the follow request and store it in the database.

TheNoim · 2025-05-27T09:20:18Z

Thanks for the review! It will take some time to address all, because time is rare at the moment for me.

ThisIsMissEm · 2025-08-10T16:12:09Z

I'm going to mark this as a draft due to pending changes and inactivity

ThisIsMissEm reviewed Feb 15, 2025

View reviewed changes

src/api/v1/statuses.ts Outdated Show resolved Hide resolved

ThisIsMissEm reviewed Feb 15, 2025

View reviewed changes

dahlia requested changes Feb 16, 2025

View reviewed changes

dahlia mentioned this pull request Feb 18, 2025

Support PEM-PKCS#1 besides PEM-SPKI for RSA public keys fedify-dev/fedify#209

Closed

dahlia mentioned this pull request Feb 20, 2025

Choose key without fragment if key with fragment is not found fedify-dev/fedify#211

Closed

TheNoim force-pushed the feat/relays branch from 52ed641 to 53f427a Compare March 4, 2025 17:53

TheNoim mentioned this pull request Mar 4, 2025

Activity allows string and string array for to and cc yukimochi/Activity-Relay#104

Open

TheNoim added 10 commits May 3, 2025 12:56

feat: add mastodon style relays

4c82660

refactor: some improvements

d763e0c

fix: change user again

8585a94

fix: don't show relay account on oauth screen

ead5a47

fix: more internal follow user fixes

a9d5f10

chore: upgrade fedify to latest dev version

fb529e0

fix: drizzle rebase

4ecb095

fix: rebase issue

dd79a0b

feat: exclude relay accounts from home page

6bddc7c

feat: delete outgoing activities for deleted relay

4582aa9

TheNoim force-pushed the feat/relays branch from 3ee6ad2 to 4582aa9 Compare May 3, 2025 11:00

dahlia requested changes May 3, 2025

View reviewed changes

TheNoim added 2 commits May 3, 2025 14:37

fix: reserved handle comparison

bbc7237

chore: remove unused import

9610adb

TheNoim marked this pull request as ready for review May 5, 2025 16:54

ThisIsMissEm reviewed May 25, 2025

View reviewed changes

ThisIsMissEm marked this pull request as draft August 10, 2025 16:12

	const [{ total }] = await db.select({ total: count() }).from(accountOwners);
	const [{ activeMonth }] = await db
	.select({ activeMonth: countDistinct(accountOwners.id) })
	.from(accountOwners)
	.rightJoin(posts, eq(accountOwners.id, posts.accountId))
	.where(gt(posts.updated, sql`CURRENT_TIMESTAMP - INTERVAL '1 month'`));
	const [{ activeHalfyear }] = await db
	.select({ activeHalfyear: countDistinct(accountOwners.id) })
	.from(accountOwners)
	.rightJoin(posts, eq(accountOwners.id, posts.accountId))
	.where(gt(posts.updated, sql`CURRENT_TIMESTAMP - INTERVAL '6 months'`));

	if (handle === `@${HOLLO_OFFICIAL_ACCOUNT}@${fedCtx.host}`) {
	if (username === HOLLO_RELAY_ACTOR_USERNAME) {

		const form = await c.req.formData();

		const inboxUrl = form.get("inbox_url");

Uh oh!

feat: add mastodon style relays #109

Are you sure you want to change the base?

feat: add mastodon style relays #109

Conversation

TheNoim commented Feb 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Todos

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dahlia left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ThisIsMissEm commented Feb 16, 2025

Uh oh!

TheNoim commented Feb 16, 2025

Uh oh!

dahlia commented Feb 17, 2025

Uh oh!

TheNoim commented Feb 17, 2025

Uh oh!

dahlia commented Feb 18, 2025

Uh oh!

TheNoim commented Feb 19, 2025

Uh oh!

dahlia commented Feb 20, 2025

Uh oh!

dahlia commented Feb 20, 2025

Uh oh!

TheNoim commented May 3, 2025

Uh oh!

dahlia commented May 3, 2025

Uh oh!

dahlia left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TheNoim commented May 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TheNoim commented Feb 15, 2025 •

edited

Loading