mirror of
https://github.com/outline/outline.git
synced 2026-06-13 11:25:03 +03:00
Fix authorization gaps for restricted documents
- Tighten Document.findByIds so isPrivate filtering fails closed when the attribute is not loaded, and include isPrivate in the projection used by Relationship.findSourceDocumentIdsForUser so backlinks from restricted docs are no longer leaked to collection-only members. - Add !isPrivate gate to the unpublish policy so collection writers without direct membership cannot unpublish restricted documents. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -900,19 +900,17 @@ class Document extends ArchivableModel<
|
||||
return true;
|
||||
}
|
||||
|
||||
// Check isPrivate safely (may not be loaded when attributes are restricted)
|
||||
let isPrivate = false;
|
||||
try {
|
||||
isPrivate = doc.isPrivate;
|
||||
} catch {
|
||||
// attribute not loaded, treat as unrestricted
|
||||
// Fail closed if isPrivate cannot be determined — callers that restrict
|
||||
// attributes must explicitly include isPrivate to opt into collection
|
||||
// inheritance.
|
||||
if (doc.isPrivate !== false) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return (
|
||||
!isPrivate &&
|
||||
((!doc.collection?.isPrivate && !user?.isGuest) ||
|
||||
(doc.collection?.memberships.length || 0) > 0 ||
|
||||
(doc.collection?.groupMemberships.length || 0) > 0)
|
||||
(!doc.collection?.isPrivate && !user?.isGuest) ||
|
||||
(doc.collection?.memberships.length || 0) > 0 ||
|
||||
(doc.collection?.groupMemberships.length || 0) > 0
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
@@ -72,7 +72,7 @@ class Relationship extends IdModel<
|
||||
const documents = await Document.findByIds(
|
||||
relationships.map((relationship) => relationship.reverseDocumentId),
|
||||
{
|
||||
attributes: ["id"],
|
||||
attributes: ["id", "isPrivate", "collectionId"],
|
||||
userId: user.id,
|
||||
includeState: false,
|
||||
includeViews: false,
|
||||
|
||||
@@ -589,4 +589,77 @@ describe("restricted document", () => {
|
||||
const abilities = serialize(user, document);
|
||||
expect(abilities.manageUsers).toEqual(false);
|
||||
});
|
||||
|
||||
it("should not allow unpublish for collection writer without direct membership", async () => {
|
||||
const team = await buildTeam();
|
||||
const user = await buildUser({ teamId: team.id });
|
||||
const collection = await buildCollection({
|
||||
teamId: team.id,
|
||||
permission: CollectionPermission.ReadWrite,
|
||||
});
|
||||
const doc = await buildDocument({
|
||||
teamId: team.id,
|
||||
collectionId: collection.id,
|
||||
isPrivate: true,
|
||||
});
|
||||
const document = await Document.findByPk(doc.id, { userId: user.id });
|
||||
const abilities = serialize(user, document);
|
||||
expect(abilities.unpublish).toEqual(false);
|
||||
});
|
||||
|
||||
it("should allow unpublish for direct ReadWrite member", async () => {
|
||||
const team = await buildTeam();
|
||||
const user = await buildUser({ teamId: team.id });
|
||||
const collection = await buildCollection({
|
||||
teamId: team.id,
|
||||
permission: CollectionPermission.ReadWrite,
|
||||
});
|
||||
const doc = await buildDocument({
|
||||
teamId: team.id,
|
||||
collectionId: collection.id,
|
||||
isPrivate: true,
|
||||
});
|
||||
await UserMembership.create({
|
||||
documentId: doc.id,
|
||||
userId: user.id,
|
||||
permission: DocumentPermission.ReadWrite,
|
||||
createdById: user.id,
|
||||
});
|
||||
const document = await Document.findByPk(doc.id, { userId: user.id });
|
||||
const abilities = serialize(user, document);
|
||||
expect(abilities.unpublish).toBeTruthy();
|
||||
});
|
||||
|
||||
it("should allow unpublish for team admin", async () => {
|
||||
const team = await buildTeam();
|
||||
const admin = await buildAdmin({ teamId: team.id });
|
||||
const collection = await buildCollection({
|
||||
teamId: team.id,
|
||||
permission: CollectionPermission.ReadWrite,
|
||||
});
|
||||
const doc = await buildDocument({
|
||||
teamId: team.id,
|
||||
collectionId: collection.id,
|
||||
isPrivate: true,
|
||||
});
|
||||
const document = await Document.findByPk(doc.id, { userId: admin.id });
|
||||
const abilities = serialize(admin, document);
|
||||
expect(abilities.unpublish).toBeTruthy();
|
||||
});
|
||||
|
||||
it("should allow unpublish for collection writer on non-private document", async () => {
|
||||
const team = await buildTeam();
|
||||
const user = await buildUser({ teamId: team.id });
|
||||
const collection = await buildCollection({
|
||||
teamId: team.id,
|
||||
permission: CollectionPermission.ReadWrite,
|
||||
});
|
||||
const doc = await buildDocument({
|
||||
teamId: team.id,
|
||||
collectionId: collection.id,
|
||||
});
|
||||
const document = await Document.findByPk(doc.id, { userId: user.id });
|
||||
const abilities = serialize(user, document);
|
||||
expect(abilities.unpublish).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
+22
-21
@@ -1,7 +1,7 @@
|
||||
import invariant from "invariant";
|
||||
import { DocumentPermission, TeamPreference } from "@shared/types";
|
||||
import { Document, Revision, User, Team } from "@server/models";
|
||||
import { allow, cannot, can } from "./cancan";
|
||||
import { allow, can } from "./cancan";
|
||||
import { and, isTeamAdmin, isTeamModel, isTeamMutable, or } from "./utils";
|
||||
|
||||
allow(User, "createDocument", Team, (actor, document) =>
|
||||
@@ -270,26 +270,27 @@ allow(
|
||||
(document, revision) => document.id === revision?.documentId
|
||||
);
|
||||
|
||||
allow(User, "unpublish", Document, (user, document) => {
|
||||
if (
|
||||
!document ||
|
||||
user.isGuest ||
|
||||
user.isViewer ||
|
||||
!document.isActive ||
|
||||
document.isDraft
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
|
||||
invariant(
|
||||
document.collection,
|
||||
"collection is missing, did you forget to include in the query scope?"
|
||||
);
|
||||
if (cannot(user, "updateDocument", document.collection)) {
|
||||
return false;
|
||||
}
|
||||
return user.teamId === document.teamId;
|
||||
});
|
||||
allow(User, "unpublish", Document, (user, document) =>
|
||||
and(
|
||||
!!document,
|
||||
!user.isGuest,
|
||||
!user.isViewer,
|
||||
!!document?.isActive,
|
||||
!document?.isDraft,
|
||||
user.teamId === document?.teamId,
|
||||
or(
|
||||
includesMembership(document, [
|
||||
DocumentPermission.ReadWrite,
|
||||
DocumentPermission.Admin,
|
||||
]),
|
||||
and(isTeamAdmin(user, document), can(user, "read", document)),
|
||||
and(
|
||||
!document?.isPrivate,
|
||||
can(user, "updateDocument", document?.collection)
|
||||
)
|
||||
)
|
||||
)
|
||||
);
|
||||
|
||||
function includesMembership(
|
||||
document: Document | null,
|
||||
|
||||
@@ -1079,6 +1079,89 @@ describe("#documents.list", () => {
|
||||
expect(body.data[0].id).toEqual(anotherDoc.id);
|
||||
});
|
||||
|
||||
it("should not return restricted backlinks for non-members", async () => {
|
||||
const team = await buildTeam();
|
||||
const owner = await buildUser({ teamId: team.id });
|
||||
const user = await buildUser({ teamId: team.id });
|
||||
const collection = await buildCollection({
|
||||
teamId: team.id,
|
||||
permission: CollectionPermission.ReadWrite,
|
||||
});
|
||||
const document = await buildDocument({
|
||||
userId: owner.id,
|
||||
teamId: team.id,
|
||||
collectionId: collection.id,
|
||||
});
|
||||
const restrictedDoc = await buildDocument({
|
||||
title: "restricted backlink",
|
||||
text: "secret",
|
||||
userId: owner.id,
|
||||
teamId: team.id,
|
||||
collectionId: collection.id,
|
||||
isPrivate: true,
|
||||
});
|
||||
await Relationship.create({
|
||||
reverseDocumentId: restrictedDoc.id,
|
||||
type: RelationshipType.Backlink,
|
||||
documentId: document.id,
|
||||
userId: owner.id,
|
||||
});
|
||||
const res = await server.post("/api/documents.list", {
|
||||
body: {
|
||||
token: user.getJwtToken(),
|
||||
backlinkDocumentId: document.id,
|
||||
},
|
||||
});
|
||||
const body = await res.json();
|
||||
expect(res.status).toEqual(200);
|
||||
expect(body.data.length).toEqual(0);
|
||||
});
|
||||
|
||||
it("should return restricted backlinks for direct members", async () => {
|
||||
const team = await buildTeam();
|
||||
const owner = await buildUser({ teamId: team.id });
|
||||
const user = await buildUser({ teamId: team.id });
|
||||
const collection = await buildCollection({
|
||||
teamId: team.id,
|
||||
permission: CollectionPermission.ReadWrite,
|
||||
});
|
||||
const document = await buildDocument({
|
||||
userId: owner.id,
|
||||
teamId: team.id,
|
||||
collectionId: collection.id,
|
||||
});
|
||||
const restrictedDoc = await buildDocument({
|
||||
title: "restricted backlink",
|
||||
text: "secret",
|
||||
userId: owner.id,
|
||||
teamId: team.id,
|
||||
collectionId: collection.id,
|
||||
isPrivate: true,
|
||||
});
|
||||
await UserMembership.create({
|
||||
documentId: restrictedDoc.id,
|
||||
userId: user.id,
|
||||
permission: DocumentPermission.Read,
|
||||
createdById: owner.id,
|
||||
});
|
||||
await Relationship.create({
|
||||
reverseDocumentId: restrictedDoc.id,
|
||||
type: RelationshipType.Backlink,
|
||||
documentId: document.id,
|
||||
userId: owner.id,
|
||||
});
|
||||
const res = await server.post("/api/documents.list", {
|
||||
body: {
|
||||
token: user.getJwtToken(),
|
||||
backlinkDocumentId: document.id,
|
||||
},
|
||||
});
|
||||
const body = await res.json();
|
||||
expect(res.status).toEqual(200);
|
||||
expect(body.data.length).toEqual(1);
|
||||
expect(body.data[0].id).toEqual(restrictedDoc.id);
|
||||
});
|
||||
|
||||
it("should require authentication", async () => {
|
||||
const res = await server.post("/api/documents.list");
|
||||
const body = await res.json();
|
||||
|
||||
Reference in New Issue
Block a user