Fix role auth for access-or-role verifier, getBlob check on actor tak…

…edowns (#1869) fix role auth for access-or-role verifier, fix getBlob actor takedown check
bluesky-social · Nov 20, 2023 · ef6586a · ef6586a
1 parent 98cd23b
commit ef6586a
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 5 deletions.
diff --git a/packages/pds/src/api/com/atproto/sync/getBlob.ts b/packages/pds/src/api/com/atproto/sync/getBlob.ts
@@ -8,7 +8,7 @@ export default function (server: Server, ctx: AppContext) {
   server.com.atproto.sync.getBlob({
     auth: ctx.authVerifier.optionalAccessOrRole,
     handler: async ({ params, res, auth }) => {
-      if (ctx.authVerifier.isUserOrAdmin(auth, params.did)) {
+      if (!ctx.authVerifier.isUserOrAdmin(auth, params.did)) {
         const available = await ctx.accountManager.isRepoAvailable(params.did)
         if (!available) {
           throw new InvalidRequestError('Blob not found')

diff --git a/packages/pds/src/auth-verifier.ts b/packages/pds/src/auth-verifier.ts
@@ -195,15 +195,15 @@ export class AuthVerifier {
       return await this.access(ctx)
     } else {
       const creds = this.parseRoleCreds(ctx.req)
-      if (creds.status === RoleStatus.Missing) {
-        return { credentials: null }
-      } else if (creds.admin) {
+      if (creds.status === RoleStatus.Valid) {
         return {
           credentials: {
             ...creds,
             type: 'role',
           },
         }
+      } else if (creds.status === RoleStatus.Missing) {
+        return { credentials: null }
       } else {
         throw new AuthRequiredError()
       }

diff --git a/packages/pds/tests/moderation.test.ts b/packages/pds/tests/moderation.test.ts
@@ -229,7 +229,7 @@ describe('moderation', () => {
       await expect(referenceBlob).rejects.toThrow('Could not find blob:')
     })
 
-    it('prevents image blob from being served, even when cached.', async () => {
+    it('prevents image blob from being served.', async () => {
       const attempt = agent.api.com.atproto.sync.getBlob({
         did: sc.dids.carol,
         cid: blobRef.image.ref.toString(),
@@ -261,6 +261,63 @@ describe('moderation', () => {
 
       expect(res.data.byteLength).toBeGreaterThan(9000)
     })
+
+    it('prevents blobs of takendown accounts from being served.', async () => {
+      await agent.api.com.atproto.admin.updateSubjectStatus(
+        {
+          subject: {
+            $type: 'com.atproto.admin.defs#repoRef',
+            did: sc.dids.carol,
+          },
+          takedown: { applied: true },
+        },
+        {
+          encoding: 'application/json',
+          headers: network.pds.adminAuthHeaders(),
+        },
+      )
+      const blobParams = {
+        did: sc.dids.carol,
+        cid: blobRef.image.ref.toString(),
+      }
+      // public, disallow
+      const attempt1 = agent.api.com.atproto.sync.getBlob(blobParams)
+      await expect(attempt1).rejects.toThrow('Blob not found')
+      // logged-in, disallow
+      const attempt2 = agent.api.com.atproto.sync.getBlob(blobParams, {
+        headers: sc.getHeaders(sc.dids.bob),
+      })
+      await expect(attempt2).rejects.toThrow('Blob not found')
+      // non-admin role, disallow
+      const attempt3 = agent.api.com.atproto.sync.getBlob(blobParams, {
+        headers: network.pds.adminAuthHeaders('moderator'),
+      })
+      await expect(attempt3).rejects.toThrow('Blob not found')
+      // logged-in as account, allow
+      const res1 = await agent.api.com.atproto.sync.getBlob(blobParams, {
+        headers: sc.getHeaders(sc.dids.carol),
+      })
+      expect(res1.data.byteLength).toBeGreaterThan(9000)
+      // admin role, allow
+      const res2 = await agent.api.com.atproto.sync.getBlob(blobParams, {
+        headers: network.pds.adminAuthHeaders('admin'),
+      })
+      expect(res2.data.byteLength).toBeGreaterThan(9000)
+      // revert takedown
+      await agent.api.com.atproto.admin.updateSubjectStatus(
+        {
+          subject: {
+            $type: 'com.atproto.admin.defs#repoRef',
+            did: sc.dids.carol,
+          },
+          takedown: { applied: false },
+        },
+        {
+          encoding: 'application/json',
+          headers: network.pds.adminAuthHeaders(),
+        },
+      )
+    })
   })
 
   describe('auth', () => {