mirror of
https://github.com/HeyPuter/puter.git
synced 2026-08-24 23:17:23 +00:00
feat(permissions): let manage inherit down the filesystem tree
Access already reached descendants through the ancestor chain while authority did not, so someone trusted to manage a shared folder could re-share the folder but nothing inside it, and could not see who had access to a file within it. A manage-inherits-from-ancestor implicator resolves it in the permission layer, beside is-owner, so every caller agrees rather than just ShareService. It consults only the immediate parent — resolving that re-enters one level up, making a chain of depth d cost d checks rather than d². That makes two cascade gaps reachable, both fixed here. A revoke now walks the subtree, since a grant on a descendant can rest on authority held at the folder. And it stops at a delegate whose authority survives another issuer, because what they granted was never theirs to lose. Also pins that manage is not transitive: granting it needs manage:manage:fs:<uid>, which only the owner holds, so delegation is one level deep by construction.
This commit is contained in:
@@ -240,6 +240,49 @@ export class FSService extends PuterService {
|
||||
},
|
||||
});
|
||||
|
||||
// -- manage-inherits-from-ancestor -----------------------------
|
||||
// `manage` on a directory covers what is inside it, the way `fs:*`
|
||||
// access already reaches descendants through the ancestor chain.
|
||||
// Without this the two are asymmetric: someone trusted to manage a
|
||||
// shared folder can re-share the folder itself but nothing in it, and
|
||||
// cannot even see who has access to a file within it.
|
||||
//
|
||||
// Only the parent is consulted; resolving it re-enters one level up,
|
||||
// so a chain of depth d costs d checks rather than d².
|
||||
permissions.registerImplicator({
|
||||
id: 'manage-inherits-from-ancestor',
|
||||
shortcut: true,
|
||||
matches: (permission: string): boolean =>
|
||||
permission.startsWith(`${MANAGE_PERM_PREFIX}:fs:`),
|
||||
check: async ({ actor, permission }): Promise<unknown> => {
|
||||
// Apps are bounded by their user through a separate path;
|
||||
// widening them here would let one outrun that bound.
|
||||
if (actor.app || actor.accessToken) return undefined;
|
||||
if (!actor.user?.id) return undefined;
|
||||
|
||||
const stripped = permission.replaceAll(
|
||||
`${MANAGE_PERM_PREFIX}:`,
|
||||
'',
|
||||
);
|
||||
const uid = PermissionUtil.split(stripped)[1];
|
||||
if (!uid) return undefined;
|
||||
|
||||
const entry = await fsEntryStore.getEntryByUuid(uid);
|
||||
if (!entry) return undefined;
|
||||
|
||||
const [, parent] = await this.getAncestorChain(entry.path);
|
||||
if (!parent) return undefined;
|
||||
|
||||
// uuids carry no `:`, so swapping it in leaves the manage
|
||||
// prefixes and the mode suffix exactly as they were.
|
||||
const held = await permissions.check(
|
||||
actor,
|
||||
permission.replace(`fs:${uid}`, `fs:${parent.uid}`),
|
||||
);
|
||||
return held ? {} : undefined;
|
||||
},
|
||||
});
|
||||
|
||||
// -- app-owns-appdata -----------------------------------------
|
||||
// Mirror of the ACLService short-circuit at ACLService.check:
|
||||
// an app-under-user actor implicitly holds fs:<uuid>:* on any
|
||||
|
||||
@@ -402,6 +402,178 @@ describe('ShareService', () => {
|
||||
expect(await canRead(third.actor, file.path)).toBe(false);
|
||||
});
|
||||
|
||||
describe('manage inherits down the tree', () => {
|
||||
it('lets a folder delegate re-share and inspect a file inside it', async () => {
|
||||
const owner = await makeUser();
|
||||
const delegate = await makeUser();
|
||||
const third = await makeUser();
|
||||
const { dir, file } = await makeDirWithFile(owner.user);
|
||||
|
||||
await share(owner.actor, {
|
||||
uid: dir.uuid,
|
||||
recipient: { email: delegate.email },
|
||||
mode: 'manage',
|
||||
});
|
||||
|
||||
// Authority now reaches the child the way access already did.
|
||||
const rows = await server.services.share.listSharesOf(
|
||||
delegate.actor,
|
||||
{ uid: file.uuid },
|
||||
);
|
||||
expect(rows.map((r) => r.holder.username)).toContain(
|
||||
delegate.user.username,
|
||||
);
|
||||
|
||||
await share(delegate.actor, {
|
||||
uid: file.uuid,
|
||||
recipient: { email: third.email },
|
||||
mode: 'read',
|
||||
});
|
||||
expect(await canRead(third.actor, file.path)).toBe(true);
|
||||
});
|
||||
|
||||
it('revokes what a folder delegate re-shared from inside it', async () => {
|
||||
const owner = await makeUser();
|
||||
const delegate = await makeUser();
|
||||
const third = await makeUser();
|
||||
const { dir, file } = await makeDirWithFile(owner.user);
|
||||
|
||||
await share(owner.actor, {
|
||||
uid: dir.uuid,
|
||||
recipient: { email: delegate.email },
|
||||
mode: 'manage',
|
||||
});
|
||||
await share(delegate.actor, {
|
||||
uid: file.uuid,
|
||||
recipient: { email: third.email },
|
||||
mode: 'read',
|
||||
});
|
||||
expect(await canRead(third.actor, file.path)).toBe(true);
|
||||
|
||||
// The grant on the child came from authority held on the folder,
|
||||
// so withdrawing that authority has to reach down to it.
|
||||
await unshare(owner.actor, {
|
||||
uid: dir.uuid,
|
||||
recipient: { username: delegate.user.username },
|
||||
});
|
||||
|
||||
expect(await canRead(delegate.actor, file.path)).toBe(false);
|
||||
expect(await canRead(third.actor, file.path)).toBe(false);
|
||||
});
|
||||
|
||||
it('does not let plain access on a folder manage what is inside', async () => {
|
||||
const owner = await makeUser();
|
||||
const reader = await makeUser();
|
||||
const third = await makeUser();
|
||||
const { dir, file } = await makeDirWithFile(owner.user);
|
||||
|
||||
await share(owner.actor, {
|
||||
uid: dir.uuid,
|
||||
recipient: { email: reader.email },
|
||||
mode: 'write',
|
||||
});
|
||||
|
||||
// `write` reaches the child, but managing is a separate namespace.
|
||||
// 403 rather than 404 here: they can already see the file, so
|
||||
// hiding it would protect nothing.
|
||||
expect(await canRead(reader.actor, file.path)).toBe(true);
|
||||
await expect(
|
||||
share(reader.actor, {
|
||||
uid: file.uuid,
|
||||
recipient: { email: third.email },
|
||||
mode: 'read',
|
||||
}),
|
||||
).rejects.toMatchObject({ statusCode: 403 });
|
||||
});
|
||||
|
||||
it('does not let manage on a file leak up to its folder', async () => {
|
||||
const owner = await makeUser();
|
||||
const delegate = await makeUser();
|
||||
const third = await makeUser();
|
||||
const { dir, file } = await makeDirWithFile(owner.user);
|
||||
|
||||
await share(owner.actor, {
|
||||
uid: file.uuid,
|
||||
recipient: { email: delegate.email },
|
||||
mode: 'manage',
|
||||
});
|
||||
|
||||
// Inheritance runs one way; the parent is not implied by the child.
|
||||
await expect(
|
||||
share(delegate.actor, {
|
||||
uid: dir.uuid,
|
||||
recipient: { email: third.email },
|
||||
mode: 'read',
|
||||
}),
|
||||
).rejects.toMatchObject({ statusCode: 404 });
|
||||
});
|
||||
});
|
||||
|
||||
it('does not let a delegate pass on `manage` itself', async () => {
|
||||
const owner = await makeUser();
|
||||
const delegate = await makeUser();
|
||||
const third = await makeUser();
|
||||
const file = await makeFile(owner.user);
|
||||
|
||||
await share(owner.actor, {
|
||||
uid: file.uuid,
|
||||
recipient: { email: delegate.email },
|
||||
mode: 'manage',
|
||||
});
|
||||
|
||||
// Granting `manage` needs `manage:manage:fs:<uid>`, which only the
|
||||
// owner holds — so delegation is one level deep by construction.
|
||||
await expect(
|
||||
share(delegate.actor, {
|
||||
uid: file.uuid,
|
||||
recipient: { email: third.email },
|
||||
mode: 'manage',
|
||||
}),
|
||||
).rejects.toMatchObject({ statusCode: 403 });
|
||||
});
|
||||
|
||||
it('leaves a delegate alone when their authority survives another issuer', async () => {
|
||||
const owner = await makeUser();
|
||||
const delegate = await makeUser();
|
||||
const middle = await makeUser();
|
||||
const leaf = await makeUser();
|
||||
const file = await makeFile(owner.user);
|
||||
|
||||
// `middle` manages by the owner's grant, and separately holds a plain
|
||||
// read the delegate handed out.
|
||||
await share(owner.actor, {
|
||||
uid: file.uuid,
|
||||
recipient: { email: delegate.email },
|
||||
mode: 'manage',
|
||||
});
|
||||
await share(owner.actor, {
|
||||
uid: file.uuid,
|
||||
recipient: { email: middle.email },
|
||||
mode: 'manage',
|
||||
});
|
||||
await share(delegate.actor, {
|
||||
uid: file.uuid,
|
||||
recipient: { email: middle.email },
|
||||
mode: 'read',
|
||||
});
|
||||
await share(middle.actor, {
|
||||
uid: file.uuid,
|
||||
recipient: { email: leaf.email },
|
||||
mode: 'read',
|
||||
});
|
||||
|
||||
await unshare(owner.actor, {
|
||||
uid: file.uuid,
|
||||
recipient: { username: delegate.user.username },
|
||||
});
|
||||
|
||||
// Withdrawing the delegate costs `middle` nothing it was relying on,
|
||||
// so what `middle` granted must stand.
|
||||
expect(await canRead(delegate.actor, file.path)).toBe(false);
|
||||
expect(await canRead(middle.actor, file.path)).toBe(true);
|
||||
expect(await canRead(leaf.actor, file.path)).toBe(true);
|
||||
});
|
||||
|
||||
it('lets a delegate clear only what it issued', async () => {
|
||||
const owner = await makeUser();
|
||||
const delegate = await makeUser();
|
||||
|
||||
@@ -233,14 +233,7 @@ export class ShareService extends PuterService {
|
||||
);
|
||||
if (isNewShare) await this.#assertDailyQuota(issuerId);
|
||||
|
||||
const holderActor: Actor = {
|
||||
user: {
|
||||
id: holder.id,
|
||||
uuid: holder.uuid,
|
||||
username: holder.username,
|
||||
} as Actor['user'],
|
||||
effectiveApp: null,
|
||||
};
|
||||
const holderActor: Actor = this.#actorFor(holder);
|
||||
|
||||
await this.services.acl.setUserUser(
|
||||
actor,
|
||||
@@ -367,27 +360,30 @@ export class ShareService extends PuterService {
|
||||
if (seen.has(issuerId)) return 0;
|
||||
seen.add(issuerId);
|
||||
|
||||
const rows = (await this.stores.share.listByFsentry(entry.id)).filter(
|
||||
// The whole subtree, not just this node: `manage` inherits downwards,
|
||||
// so a grant on a descendant can rest on authority held here.
|
||||
const rows = (
|
||||
await this.stores.share.listByFsentrySubtree(entry.id, entry.path)
|
||||
).filter(
|
||||
(row: { issuer_user_id: number }) =>
|
||||
Number(row.issuer_user_id) === issuerId,
|
||||
);
|
||||
if (rows.length === 0) return 0;
|
||||
|
||||
const nodes = await this.stores.fsEntry.getEntriesByIds(
|
||||
rows.map((row: { fsentry_id: number }) => Number(row.fsentry_id)),
|
||||
);
|
||||
|
||||
let revoked = 0;
|
||||
for (const row of rows) {
|
||||
const holderId = Number(row.holder_user_id);
|
||||
const node = nodes.get(Number(row.fsentry_id));
|
||||
const downstream = await this.stores.user.getById(holderId);
|
||||
if (!downstream?.username) continue;
|
||||
|
||||
revoked += await this.#revokeDownstream(
|
||||
actor,
|
||||
entry,
|
||||
holderId,
|
||||
seen,
|
||||
);
|
||||
if (!node || !downstream?.username) continue;
|
||||
|
||||
const { revoked: didRevoke, authorized } = await this.#revokeFor(
|
||||
actor,
|
||||
entry,
|
||||
node,
|
||||
downstream.username,
|
||||
issuerId,
|
||||
);
|
||||
@@ -395,10 +391,27 @@ export class ShareService extends PuterService {
|
||||
if (authorized) {
|
||||
await this.stores.share.deleteActive({
|
||||
holderUserId: holderId,
|
||||
fsentryId: entry.id,
|
||||
fsentryId: node.id,
|
||||
issuerUserId: issuerId,
|
||||
});
|
||||
}
|
||||
|
||||
// Only carry on down if this actually cost them their authority.
|
||||
// A delegate granted `manage` by two people keeps it when one
|
||||
// withdraws, and what they granted is not theirs to lose.
|
||||
const stillHolds =
|
||||
await this.services.permission.canManagePermission(
|
||||
this.#actorFor(downstream),
|
||||
`fs:${node.uuid}:read`,
|
||||
);
|
||||
if (stillHolds) continue;
|
||||
|
||||
revoked += await this.#revokeDownstream(
|
||||
actor,
|
||||
entry,
|
||||
holderId,
|
||||
seen,
|
||||
);
|
||||
}
|
||||
return revoked;
|
||||
}
|
||||
@@ -602,6 +615,18 @@ export class ShareService extends PuterService {
|
||||
};
|
||||
}
|
||||
|
||||
/** A plain user actor, for asking the permission layer about someone else. */
|
||||
#actorFor(user: { id: number; uuid?: string; username?: string }): Actor {
|
||||
return {
|
||||
user: {
|
||||
id: user.id,
|
||||
uuid: user.uuid,
|
||||
username: user.username,
|
||||
} as Actor['user'],
|
||||
effectiveApp: null,
|
||||
};
|
||||
}
|
||||
|
||||
#requireUserId(actor: Actor): number {
|
||||
const id = actor?.user?.id;
|
||||
if (typeof id !== 'number') {
|
||||
|
||||
@@ -110,6 +110,28 @@ export class ShareStore extends PuterStore {
|
||||
return rows.map((r) => this.#normalizeRow(r));
|
||||
}
|
||||
|
||||
/**
|
||||
* Active shares on a directory and everything beneath it. `manage` inherits
|
||||
* downwards, so a revoke here has to see what rests on it.
|
||||
*
|
||||
* @param {number} fsentryId
|
||||
* @param {string} path Directory path, used to match descendants.
|
||||
*/
|
||||
async listByFsentrySubtree(fsentryId, path) {
|
||||
// `!` escapes the LIKE wildcards so a directory named with `%` or `_`
|
||||
// cannot widen the match into siblings.
|
||||
const prefix = `${String(path).replace(/([!%_])/g, '!$1')}/%`;
|
||||
const rows = await this.clients.db.read(
|
||||
'SELECT `share`.* FROM `share` ' +
|
||||
'JOIN `fsentries` ON `fsentries`.`id` = `share`.`fsentry_id` ' +
|
||||
'WHERE `share`.`holder_user_id` IS NOT NULL AND ' +
|
||||
"(`share`.`fsentry_id` = ? OR `fsentries`.`path` LIKE ? ESCAPE '!') " +
|
||||
'ORDER BY `share`.`id`',
|
||||
[fsentryId, prefix],
|
||||
);
|
||||
return rows.map((r) => this.#normalizeRow(r));
|
||||
}
|
||||
|
||||
async countByHolder(holderUserId) {
|
||||
const rows = await this.clients.db.read(
|
||||
'SELECT COUNT(*) AS `count` FROM `share` WHERE `holder_user_id` = ?',
|
||||
|
||||
Reference in New Issue
Block a user