Skip to content

Commit 8ac4bad

Browse files
authored
fix: readUrl perm check (#3183)
1 parent f140d72 commit 8ac4bad

2 files changed

Lines changed: 154 additions & 0 deletions

File tree

src/backend/services/auth/AuthService.test.ts

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import { v4 as uuidv4 } from 'uuid';
2323
import type { Actor } from '../../core/actor.js';
2424
import { PuterServer } from '../../server.js';
2525
import { setupTestServer } from '../../testUtil.js';
26+
import { generateDefaultFsentries } from '../../util/userProvisioning.js';
2627
import { AuthService } from './AuthService.js';
2728

2829
function createAuthService(): AuthService {
@@ -1449,6 +1450,114 @@ describe('AuthService (integration)', () => {
14491450
),
14501451
).rejects.toMatchObject({ statusCode: 403 });
14511452
});
1453+
1454+
// Regression for the post-#1001 token-inval check: an
1455+
// app-under-user actor must be able to mint a token for a file
1456+
// inside its own AppData. Without the `app-owns-appdata`
1457+
// implicator the issuer-subset gate 403s these — even though
1458+
// ACLService already allows the equivalent fs.read via its own
1459+
// short-circuit. puter-js getReadURL is the canonical caller.
1460+
it('app-under-user actor can mint fs:<uuid>:read for a file inside its own AppData', async () => {
1461+
const user = await makeUser();
1462+
await generateDefaultFsentries(
1463+
server.clients.db,
1464+
server.stores.user,
1465+
user,
1466+
);
1467+
const appUid = `app-${uuidv4()}`;
1468+
await server.clients.db.write(
1469+
'INSERT INTO `apps` (`uid`, `name`, `title`, `index_url`, `owner_user_id`) VALUES (?, ?, ?, ?, ?)',
1470+
[
1471+
appUid,
1472+
`n-${appUid}`,
1473+
`t-${appUid}`,
1474+
`https://${appUid}.example/`,
1475+
user.id,
1476+
],
1477+
);
1478+
1479+
const appDataPath = `/${user.username}/AppData/${appUid}`;
1480+
await server.services.fs.mkdir(user.id, {
1481+
path: appDataPath,
1482+
createMissingParents: true,
1483+
} as never);
1484+
const body = Buffer.from('hello');
1485+
await server.services.fs.write(user.id, {
1486+
fileMetadata: {
1487+
path: `${appDataPath}/note.txt`,
1488+
size: body.byteLength,
1489+
contentType: 'text/plain',
1490+
},
1491+
fileContent: body,
1492+
} as never);
1493+
const fileEntry = await server.stores.fsEntry.getEntryByPath(
1494+
`${appDataPath}/note.txt`,
1495+
);
1496+
expect(fileEntry).not.toBeNull();
1497+
1498+
const appActor: Actor = {
1499+
user: { id: user.id, uuid: user.uuid, username: user.username },
1500+
app: { id: 0, uid: appUid },
1501+
} as Actor;
1502+
1503+
const jwt = await authService.createAccessToken(appActor, [
1504+
[`fs:${fileEntry!.uuid}:read`],
1505+
]);
1506+
expect(typeof jwt).toBe('string');
1507+
});
1508+
1509+
// Negative side of the implicator: a file the user owns but
1510+
// that lives *outside* the app's AppData must still be
1511+
// rejected — the issuer-subset gate is the only thing
1512+
// preventing an authorized app from minting a token over
1513+
// arbitrary user-owned uuids.
1514+
it('app-under-user actor cannot mint fs:<uuid>:read for a user-owned file outside its AppData', async () => {
1515+
const user = await makeUser();
1516+
await generateDefaultFsentries(
1517+
server.clients.db,
1518+
server.stores.user,
1519+
user,
1520+
);
1521+
const appUid = `app-${uuidv4()}`;
1522+
await server.clients.db.write(
1523+
'INSERT INTO `apps` (`uid`, `name`, `title`, `index_url`, `owner_user_id`) VALUES (?, ?, ?, ?, ?)',
1524+
[
1525+
appUid,
1526+
`n-${appUid}`,
1527+
`t-${appUid}`,
1528+
`https://${appUid}.example/`,
1529+
user.id,
1530+
],
1531+
);
1532+
1533+
const body = Buffer.from('secret');
1534+
await server.services.fs.write(user.id, {
1535+
fileMetadata: {
1536+
path: `/${user.username}/Documents/secret.txt`,
1537+
size: body.byteLength,
1538+
contentType: 'text/plain',
1539+
},
1540+
fileContent: body,
1541+
} as never);
1542+
const fileEntry = await server.stores.fsEntry.getEntryByPath(
1543+
`/${user.username}/Documents/secret.txt`,
1544+
);
1545+
expect(fileEntry).not.toBeNull();
1546+
1547+
const appActor: Actor = {
1548+
user: { id: user.id, uuid: user.uuid, username: user.username },
1549+
app: { id: 0, uid: appUid },
1550+
} as Actor;
1551+
1552+
await expect(
1553+
authService.createAccessToken(appActor, [
1554+
[`fs:${fileEntry!.uuid}:read`],
1555+
]),
1556+
).rejects.toMatchObject({
1557+
statusCode: 403,
1558+
legacyCode: 'forbidden',
1559+
});
1560+
});
14521561
});
14531562

14541563
describe('private-asset / public hosted-actor tokens', () => {

src/backend/services/fs/FSService.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,51 @@ export class FSService extends PuterService {
206206
},
207207
});
208208

209+
// -- app-owns-appdata -----------------------------------------
210+
// Mirror of the ACLService short-circuit at ACLService.check:
211+
// an app-under-user actor implicitly holds fs:<uuid>:* on any
212+
// entry inside its own /<username>/AppData/<appUid> subtree.
213+
// ACL paths (fs.read etc.) already accept these via that
214+
// short-circuit, but permissionService.checkMany — used by
215+
// createAccessToken's issuer-subset gate — bypasses ACL, so
216+
// without this implicator an app can fs.read its appdata but
217+
// can't mint a token for the same fs:<uuid>:read it just read.
218+
permissions.registerImplicator({
219+
id: 'app-owns-appdata',
220+
shortcut: true,
221+
matches: (permission: string): boolean => {
222+
return (
223+
permission.startsWith('fs:') ||
224+
permission.startsWith(`${MANAGE_PERM_PREFIX}:fs:`) ||
225+
permission.startsWith(
226+
`${MANAGE_PERM_PREFIX}:${MANAGE_PERM_PREFIX}:fs:`,
227+
)
228+
);
229+
},
230+
check: async ({ actor, permission }): Promise<unknown> => {
231+
if (!actor.app || actor.accessToken) return undefined;
232+
const username = actor.user?.username;
233+
const appUid = actor.app.uid;
234+
if (!username || !appUid) return undefined;
235+
236+
const stripped = permission.replaceAll(
237+
`${MANAGE_PERM_PREFIX}:`,
238+
'',
239+
);
240+
const uid = PermissionUtil.split(stripped)[1];
241+
if (!uid) return undefined;
242+
243+
const entry = await fsEntryStore.getEntryByUuid(uid);
244+
if (!entry) return undefined;
245+
246+
const root = `/${username}/AppData/${appUid}`;
247+
if (entry.path === root || entry.path.startsWith(`${root}/`)) {
248+
return {};
249+
}
250+
return undefined;
251+
},
252+
});
253+
209254
// -- fs-access-levels exploder ----------------------------------
210255
// `fs:UUID:see` implies `[list, read, write, manage:fs:UUID]`.
211256
// ACLService.check already expands the same-family chain

0 commit comments

Comments
 (0)