✨ Allow dynamic labeler header through ozone #2442

foysalit · 2024-04-23T20:34:42Z

This PR aims to allow moderators of an ozone instance plug in an external ozone service/moderation authority and view labels they have added to a given subject.

The current implementation only adds this to getRecord lexicon but lays out the ground work to easily enable this for other lexicons.

bnewbold · 2024-04-23T23:43:47Z

Cool!

Any thoughts on adding labels on repoView, to be symmetric with the addition to recordView? Both of the "ViewDetail" variants already have labels.

…-labeler-in-ozone

foysalit · 2024-04-24T16:27:22Z

packages/api/src/agent.ts

@@ -263,14 +263,26 @@ export class AtpAgent {
        'atproto-proxy': this.proxyHeader,
      }
    }
+
+    const labelerHeaderName = 'atproto-accept-labelers'


@pfrazee wanted to run these changes by you

devinivy · 2024-05-01T15:04:03Z

packages/bsky/src/api/com/atproto/repo/getRecord.ts

+    const labelers = ctx.reqLabelers(req)
+    const labelMap = await ctx.hydrator.label.getLabelsForSubjects(
+      [uri],
+      labelers,
+    )
+    const labels = Array.from(labelMap.get(uri)?.labels?.values() || [])


Hydrating labels here in repo.getRecord is a little bit funky since it's quietly off-contract (and I don't think we'll want to add it to the contract). I believe we had plans for the appview to implement com.atproto.label.queryLabels to provide the labels on arbitrary subjects, which I think would speak to ozone's use case. Perhaps we could implement that now, I don't think it's going to be super big lift.

@bnewbold what do you think?

…-labeler-in-ozone

foysalit · 2024-05-02T21:17:37Z

packages/bsky/src/api/com/atproto/label/queryLabels.ts

+    const labelMap = await ctx.hydrator.label.getLabelsForSubjects(
+      uriPatterns,
+      // If sources are provided, use them. Otherwise, use the labelers from the request header
+      sources


Not sure if this creates some kind of confusion because someone might pass different labelers through header and then pass different labelers through the source param and get results only for the source param.

We can always ignore the header values here.

I guess my question is, should we? to me, it feels a bit nicer to be able to just send it via header and ignore source param entirely.

In my view it's not really the purpose of the header, which is about looking at the content of the response and applying labels to it. This endpoint doesn't really serve any contents that can be labeled, so it's a little bit outside the purview of the header. As you pointed out, it may also cause confusion because there's a parameter dedicated to querying specific labelers, and it's not well-defined how that parameter and the header should interact with each other. By relying only on the dedicated parameter here, I think it makes the behavior pretty clear/predictable. @bnewbold @dholms any thoughts to add?

alright we're no longer defaulting to labelers in header and throwing error if sources param is missing.

foysalit · 2024-05-02T21:22:25Z

packages/ozone/src/util.ts

@@ -45,3 +46,52 @@ export function retryableHttp(err: unknown) {
 const retryableHttpStatusCodes = new Set([
  408, 425, 429, 500, 502, 503, 504, 522, 524,
 ])
+
+export type ParsedLabelers = {


This is copied from bsky/src/utils

✅ Update pds snapshots ✅ Update ozone snapshots ✅ Update pds snapshots ✅ Update pds snapshots ✅ Update pds snapshots ✅ Update pds snapshots

devinivy · 2024-05-06T18:22:06Z

packages/ozone/tests/3p-labeler.test.ts

+    const ozoneKey = await Secp256k1Keypair.create({ exportable: true })
+    const ozoneDid = await createOzoneDid(network.plc.url, ozoneKey)
+    thirdPartyLabeler = await TestOzone.create({
+      port: ozone.port + Math.floor(Math.random() * 10) + 1,


What was the issue you were running into with ozone.port + 10 from before this change? My understanding is that it was trying to ensure the port was out of the range of ports assigned by dev-env. With this change, I'm not sure if the port assignment will be outside that range, you may end-up with overlap.

I think it was causing tests to fail randomly but it may have been a separate issue, let me try reverting this to see what happens.

devinivy · 2024-05-15T14:28:03Z

packages/bsky/src/api/com/atproto/admin/getAccountInfos.ts

@@ -6,7 +6,7 @@ import { INVALID_HANDLE } from '@atproto/syntax'
 export default function (server: Server, ctx: AppContext) {
  server.com.atproto.admin.getAccountInfos({
    auth: ctx.authVerifier.optionalStandardOrRole,
-    handler: async ({ params, auth }) => {
+    handler: async ({ params, auth, req }) => {


The lingering changes here can probably be removed.

devinivy · 2024-05-15T14:28:58Z

packages/bsky/src/api/com/atproto/label/queryLabels.ts

+        redact: new Set(),
+      },
+    )
+    const labels = uriPatterns.map((uri) => labelMap.getBySubject(uri)).flat()


Or even 👀

Suggested change

const labels = uriPatterns.map((uri) => labelMap.getBySubject(uri)).flat()

const labels = uriPatterns.flatMap((uri) => labelMap.getBySubject(uri))

devinivy · 2024-05-15T14:55:03Z

packages/ozone/src/mod-service/views.ts

+    labelers?: ParsedLabelers,
+  ): Promise<Label[]> {
+    if (!labelers?.dids.length && !labelers?.redact.size) return []
+    const auth = await this.appviewAuth(labelers)


I think we can omit auth from here now.

yep thanks 👍

devinivy · 2024-05-15T14:57:22Z

packages/pds/src/api/com/atproto/label/queryLabels.ts

+import { Server } from '../../../../lexicon'
+import AppContext from '../../../../context'
+import { pipethrough } from '../../../../pipethrough'
+
+export default function (server: Server, ctx: AppContext) {
+  server.com.atproto.label.queryLabels({
+    auth: ctx.authVerifier.accessCheckTakedown,
+    handler: async ({ req, auth }) => {
+      const requester = auth.credentials.did
+      return pipethrough(ctx, req, requester)
+    },
+  })
+}


Hey good news, we actually don't need to add every new endpoint to the pds anymore 😆 Daniel covered this recently in #2425, so now the PDS will proxy any lexrpc endpoint.

yep realized that when syncing main with mod-manager branch but forgot to adjust here. cleaned it up now!

…-labeler-in-ozone

devinivy

Solid! Before merge mind just adding a pnpm changeset for a patch version bump of the @atproto/api package?

…-labeler-in-ozone

✨ Allow dynamic labeler header through ozone

289a776

foysalit mentioned this pull request Apr 23, 2024

view labels from other labelers in Ozone bluesky-social/ozone#93

Merged

foysalit requested a review from devinivy April 24, 2024 14:05

foysalit added 2 commits April 24, 2024 18:11

Merge branch 'main' of github.com:bluesky-social/atproto into forward…

0cbc489

…-labeler-in-ozone

✅ Close third party ozone

543707f

foysalit commented Apr 24, 2024

View reviewed changes

foysalit added 5 commits April 24, 2024 20:13

✨ Add 3p labels in for repos

cfbdcb3

✅ Update snapshots

28f96ce

✅ Update pds snapshots

34ef6c6

✅ Update ozone snapshots

0bea2e6

✨ Fix typing for labels

2829643

devinivy reviewed May 1, 2024

View reviewed changes

foysalit added 4 commits May 2, 2024 14:29

Merge branch 'main' of github.com:bluesky-social/atproto into forward…

21b1e6d

…-labeler-in-ozone

✨ Add queryLabels endpoint in appview

f74b921

✨ Use queryLabels to hydrate labels

8e8d581

Merge branch 'main' of github.com:bluesky-social/atproto into forward…

d2e6311

…-labeler-in-ozone

foysalit commented May 2, 2024

View reviewed changes

foysalit added 3 commits May 2, 2024 23:22

🧹 Cleanup

b7122c6

✅ Update pds snapshots

27f434a

✅ Update pds snapshots ✅ Update ozone snapshots ✅ Update pds snapshots ✅ Update pds snapshots ✅ Update pds snapshots ✅ Update pds snapshots

✅ Update pds snapshots

cac18fe

foysalit force-pushed the forward-labeler-in-ozone branch from 3b70ca1 to cac18fe Compare May 2, 2024 22:44

devinivy reviewed May 6, 2024

View reviewed changes

foysalit added 3 commits May 7, 2024 19:26

✅ Checking if tests fail

5ed433a

✨ Don't default to labelers in header

89ee2f2

✅ Adjust tests

e1785d0

foysalit requested a review from devinivy May 8, 2024 21:35

devinivy reviewed May 15, 2024

View reviewed changes

foysalit added 3 commits May 15, 2024 18:35

Merge branch 'main' of github.com:bluesky-social/atproto into forward…

f098bef

…-labeler-in-ozone

🧹 Cleanup

8197137

🧹 Remove unnecessary auth header

62270b7

foysalit requested a review from devinivy May 15, 2024 17:08

devinivy approved these changes May 21, 2024

View reviewed changes

foysalit added 3 commits May 22, 2024 17:15

Merge branch 'main' of github.com:bluesky-social/atproto into forward…

dbdb065

…-labeler-in-ozone

📝 Add changeset

511726b

📝 Add @atproto/api to patch update list

327e2fa

foysalit merged commit 1f560f0 into main May 22, 2024
10 checks passed

foysalit deleted the forward-labeler-in-ozone branch May 22, 2024 15:55

github-actions bot mentioned this pull request May 22, 2024

Version packages #2508

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ Allow dynamic labeler header through ozone #2442

✨ Allow dynamic labeler header through ozone #2442

foysalit commented Apr 23, 2024

bnewbold commented Apr 23, 2024

foysalit Apr 24, 2024

devinivy May 1, 2024

foysalit May 2, 2024

devinivy May 2, 2024

foysalit May 2, 2024

devinivy May 7, 2024 •

edited

Loading

foysalit May 8, 2024

foysalit May 2, 2024

devinivy May 6, 2024

foysalit May 7, 2024

devinivy May 15, 2024

foysalit May 15, 2024

devinivy May 15, 2024

devinivy May 15, 2024

foysalit May 15, 2024

devinivy May 15, 2024 •

edited

Loading

foysalit May 15, 2024

devinivy left a comment

	const labels = uriPatterns.map((uri) => labelMap.getBySubject(uri)).flat()
	const labels = uriPatterns.flatMap((uri) => labelMap.getBySubject(uri))

✨ Allow dynamic labeler header through ozone #2442

✨ Allow dynamic labeler header through ozone #2442

Conversation

foysalit commented Apr 23, 2024

bnewbold commented Apr 23, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

devinivy May 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

devinivy May 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

devinivy left a comment

Choose a reason for hiding this comment

devinivy May 7, 2024 •

edited

Loading

devinivy May 15, 2024 •

edited

Loading