fix: Narrow validation on relationships.list (#13147)

server/routes/api/relationships/relationships.test.ts

@@ -232,21 +232,33 @@ describe("#relationships.list", () => {
     expect(body.message).toEqual("Authentication required");
   });
 
-  it("should succeed with status 200 ok returning all relationships", async () => {
+  it("should fail with status 400 bad request when no anchor document is supplied", async () => {
     const res = await server.post("/api/relationships.list", admin);
     const body = await res.json();
-    expect(res.status).toEqual(200);
-    expect(body.data).toBeTruthy();
-    expect(body.data.relationships).toBeTruthy();
-    expect(body.data.relationships.length).toBeGreaterThanOrEqual(3);
-    expect(body.data.documents).toBeTruthy();
-    expect(body.pagination).toBeTruthy();
-    expect(body.policies).toBeTruthy();
+    expect(res.status).toEqual(400);
+    expect(body.message).toContain(
+      "One of documentId or reverseDocumentId is required"
+    );
+  });
+
+  it("should fail with status 400 bad request when both anchor documents are supplied", async () => {
+    const res = await server.post("/api/relationships.list", admin, {
+      body: {
+        documentId: documents[0].id,
+        reverseDocumentId: documents[1].id,
+      },
+    });
+    const body = await res.json();
+    expect(res.status).toEqual(400);
+    expect(body.message).toContain(
+      "One of documentId or reverseDocumentId is required"
+    );
   });
 
   it("should succeed with status 200 ok returning relationships filtered by type", async () => {
     const res = await server.post("/api/relationships.list", admin, {
       body: {
+        documentId: documents[0].id,
         type: RelationshipType.Backlink,
       },
     });
@@ -339,6 +351,7 @@ describe("#relationships.list", () => {
   it("should respect pagination", async () => {
     const res = await server.post("/api/relationships.list", admin, {
       body: {
+        documentId: documents[1].id,
         limit: 1,
         offset: 0,
       },
@@ -351,16 +364,31 @@ describe("#relationships.list", () => {
     expect(body.pagination.offset).toEqual(0);
   });
 
-  it("should return empty results when no relationships match filters", async () => {
+  it("should fail with status 404 not found when the anchor document does not exist", async () => {
     const res = await server.post("/api/relationships.list", admin, {
       body: {
         documentId: "550e8400-e29b-41d4-a716-446655440000",
       },
     });
-    const body = await res.json();
-    expect(res.status).toEqual(200);
-    expect(body.data.relationships).toHaveLength(0);
-    expect(body.data.documents).toHaveLength(0);
+    expect(res.status).toEqual(404);
+  });
+
+  it("should fail with status 403 forbidden when user cannot read the anchor document", async () => {
+    const cannotAccessCollection = await buildCollection({
+      teamId: user.teamId,
+      permission: null,
+    });
+    const cannotAccessDocument = await buildDocument({
+      collectionId: cannotAccessCollection.id,
+      teamId: user.teamId,
+    });
+
+    const res = await server.post("/api/relationships.list", user, {
+      body: {
+        documentId: cannotAccessDocument.id,
+      },
+    });
+    expect(res.status).toEqual(403);
   });
 
   it("should only return documents that the user can read", async () => {
@@ -400,4 +428,39 @@ describe("#relationships.list", () => {
     expect(body.data.relationships).toHaveLength(0);
     expect(body.data.documents).toHaveLength(0);
   });
+
+  it("should not disclose documents that the user cannot read when anchored on reverseDocumentId", async () => {
+    const cannotAccessCollection = await buildCollection({
+      teamId: user.teamId,
+      permission: null,
+    });
+    const collection = await buildCollection({
+      teamId: user.teamId,
+      permission: CollectionPermission.Read,
+    });
+    const userDocument = await buildDocument({
+      collectionId: collection.id,
+      teamId: user.teamId,
+    });
+    const cannotAccessDocument = await buildDocument({
+      collectionId: cannotAccessCollection.id,
+      teamId: user.teamId,
+    });
+
+    await buildRelationship({
+      userId: user.id,
+      documentId: cannotAccessDocument.id,
+      reverseDocumentId: userDocument.id,
+    });
+
+    const res = await server.post("/api/relationships.list", user, {
+      body: {
+        reverseDocumentId: userDocument.id,
+      },
+    });
+    const body = await res.json();
+    expect(res.status).toEqual(200);
+    expect(body.data.relationships).toHaveLength(0);
+    expect(body.data.documents).toHaveLength(0);
+  });
 });

server/routes/api/relationships/relationships.ts

@@ -1,4 +1,5 @@
 import Router from "koa-router";
+import { InvalidRequestError } from "@server/errors";
 import auth from "@server/middlewares/authentication";
 import validate from "@server/middlewares/validate";
 import { Document, Relationship } from "@server/models";
@@ -58,32 +59,48 @@ router.post(
   pagination(),
   validate(T.RelationshipsListSchema),
   async (ctx: APIContext<T.RelationshipsListReq>) => {
+    const { type, documentId, reverseDocumentId } = ctx.input.body;
     const { user } = ctx.state.auth;
-    const where = ctx.input.body || {};
+
+    // Results are anchored to a single document, which the user must be able to
+    // read. Documents on the opposite side of each relationship are filtered by
+    // access below.
+    const anchorId = documentId ?? reverseDocumentId;
+    if (!anchorId) {
+      throw InvalidRequestError(
+        "One of documentId or reverseDocumentId is required"
+      );
+    }
+
+    const anchorDocument = await Document.findByPk(anchorId, {
+      userId: user.id,
+      rejectOnEmpty: true,
+    });
+    authorize(user, "read", anchorDocument);
 
     const relationships = await Relationship.findAll({
-      where,
+      where: {
+        ...(type ? { type } : {}),
+        ...(documentId
+          ? { documentId: anchorDocument.id }
+          : { reverseDocumentId: anchorDocument.id }),
+      },
       order: [["createdAt", "DESC"]],
       offset: ctx.state.pagination.offset,
       limit: ctx.state.pagination.limit,
     });
 
+    const relatedDocumentId = (relationship: Relationship) =>
+      documentId ? relationship.reverseDocumentId : relationship.documentId;
+
     const documents = await Document.findByIds(
-      relationships.flatMap((relationship) =>
-        where.reverseDocumentId
-          ? relationship.documentId
-          : relationship.reverseDocumentId
-      ),
+      relationships.map(relatedDocumentId),
       { userId: user.id }
     );
 
     const documentIds = new Set(documents.map((d) => d.id));
     const filteredRelationships = relationships.filter((relationship) =>
-      documentIds.has(
-        where.reverseDocumentId
-          ? relationship.documentId
-          : relationship.reverseDocumentId
-      )
+      documentIds.has(relatedDocumentId(relationship))
     );
 
     ctx.body = {

server/routes/api/relationships/schema.ts

@@ -28,7 +28,10 @@ export const RelationshipsListSchema = BaseSchema.extend({
         })
         .optional(),
     })
-    .optional(),
+    .refine(
+      (body) => !!body.documentId !== !!body.reverseDocumentId,
+      "One of documentId or reverseDocumentId is required"
+    ),
 });
 
 export type RelationshipsListReq = z.infer<typeof RelationshipsListSchema>;