mirror of
https://github.com/HeyPuter/puter.git
synced 2026-09-30 09:06:37 +00:00
fix: settle a URL file-access prompt from a grant the app already holds
Opening `/app/<name>?file=<path>` asked for consent on every launch: the gate prompted unconditionally and never read the `fs:<uid>:write` grant its own Allow had written. `/auth/check-permissions` takes an optional `app_uid` so the account can ask what one of its apps holds, and the launch gate skips the dialog when the answer is yes. Sessions only — an app or a scoped token asking would be a window onto its neighbours' grants.
This commit is contained in:
@@ -5865,6 +5865,122 @@ describe('AuthController.handleCheckPermissions + handleListPermissions', () =>
|
||||
});
|
||||
});
|
||||
|
||||
// What lets a launch settle a consent prompt it holds no app token for.
|
||||
it('check-permissions: `app_uid` answers for that app, not for the asking user', async () => {
|
||||
const { user, actor } = await makeUserAndActor();
|
||||
const app = await server.stores.app.create(
|
||||
{
|
||||
name: `cpa-${uuidv4()}`,
|
||||
title: 'TestCheckPermsAppUid',
|
||||
index_url: 'https://check-perms-uid.example.test/index.html',
|
||||
},
|
||||
{ ownerUserId: user.id },
|
||||
);
|
||||
const permission = `user:${user.uuid}:email:read`;
|
||||
|
||||
// Held by the user, so asking as the user would answer `true`.
|
||||
const asUser = makeRes();
|
||||
await inCtx(actor, () =>
|
||||
controller.handleCheckPermissions(
|
||||
makeReq({ permissions: [permission] }, { actor }),
|
||||
asUser,
|
||||
),
|
||||
);
|
||||
expect(asUser.body).toEqual({ permissions: { [permission]: true } });
|
||||
|
||||
const before = makeRes();
|
||||
await inCtx(actor, () =>
|
||||
controller.handleCheckPermissions(
|
||||
makeReq(
|
||||
{ permissions: [permission], app_uid: app.uid },
|
||||
{ actor },
|
||||
),
|
||||
before,
|
||||
),
|
||||
);
|
||||
expect(before.body).toEqual({ permissions: { [permission]: false } });
|
||||
|
||||
await inCtx(actor, () =>
|
||||
controller.handleGrantUserApp(
|
||||
makeReq(
|
||||
{ app_uid: app.uid, permission, extra: {} },
|
||||
{ actor },
|
||||
),
|
||||
makeRes(),
|
||||
),
|
||||
);
|
||||
|
||||
const after = makeRes();
|
||||
await inCtx(actor, () =>
|
||||
controller.handleCheckPermissions(
|
||||
makeReq(
|
||||
{ permissions: [permission], app_uid: app.uid },
|
||||
{ actor },
|
||||
),
|
||||
after,
|
||||
),
|
||||
);
|
||||
expect(after.body).toEqual({ permissions: { [permission]: true } });
|
||||
});
|
||||
|
||||
it('check-permissions: an app cannot ask about another app, and an unknown app 404s', async () => {
|
||||
const { user, actor } = await makeUserAndActor();
|
||||
const app = await server.stores.app.create(
|
||||
{
|
||||
name: `cpx-${uuidv4()}`,
|
||||
title: 'TestCheckPermsCrossApp',
|
||||
index_url: 'https://check-perms-x.example.test/index.html',
|
||||
},
|
||||
{ ownerUserId: user.id },
|
||||
);
|
||||
const appActor = makeActor({
|
||||
user: actor.user,
|
||||
app: { id: app.id, uid: app.uid },
|
||||
});
|
||||
|
||||
await expect(
|
||||
inCtx(appActor, () =>
|
||||
controller.handleCheckPermissions(
|
||||
makeReq(
|
||||
{ permissions: ['service:foo:ii:read'], app_uid: app.uid },
|
||||
{ actor: appActor },
|
||||
),
|
||||
makeRes(),
|
||||
),
|
||||
),
|
||||
).rejects.toMatchObject({ statusCode: 403 });
|
||||
|
||||
await expect(
|
||||
inCtx(actor, () =>
|
||||
controller.handleCheckPermissions(
|
||||
makeReq(
|
||||
{
|
||||
permissions: ['service:foo:ii:read'],
|
||||
app_uid: `app-${uuidv4()}`,
|
||||
},
|
||||
{ actor },
|
||||
),
|
||||
makeRes(),
|
||||
),
|
||||
),
|
||||
).rejects.toMatchObject({ statusCode: 404 });
|
||||
|
||||
await expect(
|
||||
inCtx(actor, () =>
|
||||
controller.handleCheckPermissions(
|
||||
makeReq(
|
||||
{
|
||||
permissions: ['service:foo:ii:read'],
|
||||
app_uid: { not: 'a string' } as unknown as string,
|
||||
},
|
||||
{ actor },
|
||||
),
|
||||
makeRes(),
|
||||
),
|
||||
),
|
||||
).rejects.toMatchObject({ statusCode: 400 });
|
||||
});
|
||||
|
||||
it('list-permissions: returns the shape and includes a user→app grant with its app_uid', async () => {
|
||||
const { user, actor } = await makeUserAndActor();
|
||||
const app = await server.stores.app.create(
|
||||
|
||||
@@ -33,6 +33,7 @@ import {
|
||||
hasVerifiedPhone,
|
||||
} from '../../core/http/middleware/gates.js';
|
||||
import type { Actor } from '../../core/actor.js';
|
||||
import { isPlainUserActor, makeActor } from '../../core/actor.js';
|
||||
import { checkRateLimit } from '../../core/http/middleware/rateLimit.js';
|
||||
import {
|
||||
signStepUpToken,
|
||||
@@ -3535,27 +3536,56 @@ export class AuthController extends PuterController {
|
||||
|
||||
// -- Permission checks -------------------------------------------
|
||||
|
||||
/**
|
||||
* The caller's account acting as `appIdentifier`, resolved uid-or-name like
|
||||
* the grant handlers.
|
||||
*/
|
||||
async #appUnderUserActor(
|
||||
actor: Actor,
|
||||
appIdentifier: unknown,
|
||||
): Promise<Actor> {
|
||||
this.#validateAppPermissionParams({ app_uid: appIdentifier });
|
||||
// Sessions only: for an app, this would be a window onto its neighbours' grants.
|
||||
if (!isPlainUserActor(actor)) {
|
||||
throw new HttpError(403, 'actor must be a user', {
|
||||
legacyCode: 'forbidden',
|
||||
});
|
||||
}
|
||||
const app = await this.stores.app.resolveApp(appIdentifier as string);
|
||||
if (!app) {
|
||||
throw new HttpError(404, `App ${appIdentifier} does not exist`, {
|
||||
legacyCode: 'not_found',
|
||||
});
|
||||
}
|
||||
return makeActor({
|
||||
user: actor.user,
|
||||
app: { id: app.id, uid: app.uid },
|
||||
});
|
||||
}
|
||||
|
||||
@Post('/auth/check-permissions', {
|
||||
subdomain: 'api',
|
||||
requireAuth: true,
|
||||
rateLimit: AUTH_CHECK_LIMIT,
|
||||
})
|
||||
async handleCheckPermissions(req: Request, res: Response): Promise<void> {
|
||||
const { permissions } = req.body ?? {};
|
||||
const { permissions, app_uid } = req.body ?? {};
|
||||
if (!Array.isArray(permissions)) {
|
||||
throw new HttpError(400, 'Missing or invalid `permissions` array', {
|
||||
legacyCode: 'bad_request',
|
||||
});
|
||||
}
|
||||
|
||||
// `app_uid` asks what an app of mine holds, not what I hold.
|
||||
const actor = app_uid
|
||||
? await this.#appUnderUserActor(req.actor!, app_uid)
|
||||
: req.actor!;
|
||||
|
||||
const unique = [...new Set(permissions)] as string[];
|
||||
const result: Record<string, boolean> = {};
|
||||
let granted: Map<string, boolean>;
|
||||
try {
|
||||
granted = await this.services.permission.checkMany(
|
||||
req.actor!,
|
||||
unique,
|
||||
);
|
||||
granted = await this.services.permission.checkMany(actor, unique);
|
||||
} catch {
|
||||
granted = new Map<string, boolean>();
|
||||
}
|
||||
|
||||
@@ -18,6 +18,7 @@
|
||||
*/
|
||||
|
||||
import UIPermissionDialog from '../UI/UIPermissionDialog.js';
|
||||
import { appHoldsPermissions } from './holdsPermissions.js';
|
||||
import { isUuid } from './sharePaths.js';
|
||||
|
||||
/**
|
||||
@@ -52,14 +53,16 @@ export const urlFileLaunchOptions = (value) => {
|
||||
* @param {object} [deps] Injectable seams for tests.
|
||||
* @param {(target: { path?: string, uid?: string }) => Promise<{ uid?: string, path?: string, is_dir?: boolean }>} [deps.stat]
|
||||
* @param {(options: object) => Promise<boolean>} [deps.permissionDialog]
|
||||
* @param {(permissions: string[], appUid: string) => Promise<boolean>} [deps.holdsPermissions]
|
||||
* @returns {Promise<{ uid: string, path?: string } | null>} The file as
|
||||
* stat'd, only if the user allowed it; `null` otherwise.
|
||||
* stat'd, only if the user allowed it, or if they already had; `null` otherwise.
|
||||
*/
|
||||
export const confirmUrlFileAccess = async (
|
||||
{ path, uid, appUid, appName },
|
||||
{
|
||||
stat = (target) => puter.fs.stat({ ...target, consistency: 'eventual' }),
|
||||
permissionDialog = UIPermissionDialog,
|
||||
holdsPermissions = appHoldsPermissions,
|
||||
} = {},
|
||||
) => {
|
||||
if ( (! path && ! uid) || ! appUid ) return null;
|
||||
@@ -79,16 +82,23 @@ export const confirmUrlFileAccess = async (
|
||||
return null;
|
||||
}
|
||||
|
||||
// By uid: it is what the grant is stored against, and a recipient's path is masked.
|
||||
const permission = `fs:${fsentry.uid}:write`;
|
||||
const file = { uid: fsentry.uid, path: fsentry.path };
|
||||
|
||||
// Consent already given is not a question to ask again on every launch.
|
||||
if ( await holdsPermissions([permission], appUid) ) {
|
||||
return file;
|
||||
}
|
||||
|
||||
const granted = await permissionDialog({
|
||||
app_uid: appUid,
|
||||
app_name: appName,
|
||||
// By uid: it is what the grant is stored against, and a path a
|
||||
// recipient sees is a masked stand-in for the owner's.
|
||||
permission: `fs:${fsentry.uid}:write`,
|
||||
permission,
|
||||
// The entry was just stat'd; nothing here should bring one into being.
|
||||
create: false,
|
||||
});
|
||||
return granted === true ? { uid: fsentry.uid, path: fsentry.path } : null;
|
||||
return granted === true ? file : null;
|
||||
};
|
||||
|
||||
export default confirmUrlFileAccess;
|
||||
|
||||
@@ -31,10 +31,12 @@ const FILE = '/alice/Documents/notes.txt';
|
||||
const FILE_UID = '2b7d8c1e-4f3a-4b6c-9d1e-0a1b2c3d4e5f';
|
||||
|
||||
let permissionDialog;
|
||||
const deps = (stat) => ({ stat, permissionDialog });
|
||||
let holdsPermissions;
|
||||
const deps = (stat) => ({ stat, permissionDialog, holdsPermissions });
|
||||
|
||||
beforeEach(() => {
|
||||
permissionDialog = vi.fn(async () => true);
|
||||
holdsPermissions = vi.fn(async () => false);
|
||||
vi.spyOn(console, 'warn').mockImplementation(() => {});
|
||||
});
|
||||
|
||||
@@ -78,6 +80,30 @@ describe('confirmUrlFileAccess', () => {
|
||||
}));
|
||||
});
|
||||
|
||||
// The reason this gate stopped asking on every launch of the same link.
|
||||
it('hands the file over without prompting when the app already holds the grant', async () => {
|
||||
const stat = vi.fn(async () => ({ uid: FILE_UID, path: FILE, is_dir: false }));
|
||||
holdsPermissions = vi.fn(async () => true);
|
||||
|
||||
await expect(confirmUrlFileAccess(
|
||||
{ path: FILE, appUid: APP, appName: 'notepad' }, deps(stat),
|
||||
)).resolves.toEqual({ uid: FILE_UID, path: FILE });
|
||||
|
||||
expect(holdsPermissions).toHaveBeenCalledWith([`fs:${FILE_UID}:write`], APP);
|
||||
expect(permissionDialog).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// A check that couldn't be made is not consent.
|
||||
it('still prompts when the check for an existing grant fails', async () => {
|
||||
const stat = vi.fn(async () => ({ uid: FILE_UID, path: FILE, is_dir: false }));
|
||||
holdsPermissions = vi.fn(async () => false);
|
||||
|
||||
await expect(confirmUrlFileAccess(
|
||||
{ path: FILE, appUid: APP }, deps(stat),
|
||||
)).resolves.toEqual({ uid: FILE_UID, path: FILE });
|
||||
expect(permissionDialog).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('reports a refusal when the user denies', async () => {
|
||||
permissionDialog = vi.fn(async () => false);
|
||||
const stat = vi.fn(async () => ({ uid: FILE_UID, path: FILE, is_dir: false }));
|
||||
|
||||
@@ -20,6 +20,60 @@
|
||||
// The dialog waits behind this check, so a stalled read must not hold it up.
|
||||
const CHECK_TIMEOUT_MS = 5000;
|
||||
|
||||
/**
|
||||
* One `/auth/check-permissions` round trip: `token` says whose access is in question, unless `appUid` names an app under that user.
|
||||
*
|
||||
* @param {string[]} permissions
|
||||
* @param {object} options
|
||||
* @param {string} options.token
|
||||
* @param {string} [options.appUid]
|
||||
* @param {typeof fetch} [options.fetchImpl]
|
||||
* @param {string} [options.apiOrigin]
|
||||
* @param {number} [options.timeoutMs]
|
||||
* @returns {Promise<boolean>}
|
||||
*/
|
||||
const queryHeld = async (
|
||||
permissions,
|
||||
{
|
||||
token,
|
||||
appUid,
|
||||
fetchImpl = globalThis.fetch?.bind(globalThis),
|
||||
apiOrigin = window.api_origin,
|
||||
timeoutMs = CHECK_TIMEOUT_MS,
|
||||
},
|
||||
) => {
|
||||
if ( ! token || ! Array.isArray(permissions) || permissions.length === 0 ) {
|
||||
return false;
|
||||
}
|
||||
const controller = typeof AbortController !== 'undefined'
|
||||
? new AbortController()
|
||||
: null;
|
||||
const expiry = setTimeout(() => controller?.abort(), timeoutMs);
|
||||
try {
|
||||
const resp = await fetchImpl(`${apiOrigin}/auth/check-permissions`, {
|
||||
method: 'POST',
|
||||
headers: {
|
||||
'Content-Type': 'application/json',
|
||||
Authorization: `Bearer ${token}`,
|
||||
},
|
||||
body: JSON.stringify({
|
||||
permissions: [...new Set(permissions)],
|
||||
...(appUid ? { app_uid: appUid } : {}),
|
||||
}),
|
||||
...(controller ? { signal: controller.signal } : {}),
|
||||
});
|
||||
if ( ! resp.ok ) return false;
|
||||
const held = (await resp.json())?.permissions ?? {};
|
||||
// Every scope: one prompt is one decision, so partly-held is unheld.
|
||||
return permissions.every((p) => held[p] === true);
|
||||
} catch (e) {
|
||||
console.error('Failed to check held permissions', e);
|
||||
return false;
|
||||
} finally {
|
||||
clearTimeout(expiry);
|
||||
}
|
||||
};
|
||||
|
||||
/**
|
||||
* Whether every one of these permissions is already held by whoever `token`
|
||||
* identifies — an app-under-user token, so the answer is about that app's
|
||||
@@ -37,42 +91,27 @@ const CHECK_TIMEOUT_MS = 5000;
|
||||
* @param {number} [deps.timeoutMs]
|
||||
* @returns {Promise<boolean>}
|
||||
*/
|
||||
export const holdsPermissions = async (
|
||||
permissions,
|
||||
token,
|
||||
{
|
||||
fetchImpl = globalThis.fetch?.bind(globalThis),
|
||||
apiOrigin = window.api_origin,
|
||||
timeoutMs = CHECK_TIMEOUT_MS,
|
||||
} = {},
|
||||
) => {
|
||||
if ( ! token || ! Array.isArray(permissions) || permissions.length === 0 ) {
|
||||
return false;
|
||||
}
|
||||
const controller = typeof AbortController !== 'undefined'
|
||||
? new AbortController()
|
||||
: null;
|
||||
const expiry = setTimeout(() => controller?.abort(), timeoutMs);
|
||||
try {
|
||||
const resp = await fetchImpl(`${apiOrigin}/auth/check-permissions`, {
|
||||
method: 'POST',
|
||||
headers: {
|
||||
'Content-Type': 'application/json',
|
||||
Authorization: `Bearer ${token}`,
|
||||
},
|
||||
body: JSON.stringify({ permissions: [...new Set(permissions)] }),
|
||||
...(controller ? { signal: controller.signal } : {}),
|
||||
});
|
||||
if ( ! resp.ok ) return false;
|
||||
const held = (await resp.json())?.permissions ?? {};
|
||||
// Every scope: one prompt is one decision, so partly-held is unheld.
|
||||
return permissions.every((p) => held[p] === true);
|
||||
} catch (e) {
|
||||
console.error('Failed to check held permissions', e);
|
||||
return false;
|
||||
} finally {
|
||||
clearTimeout(expiry);
|
||||
}
|
||||
export const holdsPermissions = (permissions, token, deps = {}) =>
|
||||
queryHeld(permissions, { ...deps, token });
|
||||
|
||||
/**
|
||||
* The same question asked as the user, for flows holding no app token yet.
|
||||
*
|
||||
* A missing uid reports not held: asked as the user alone, their own file answers `true`.
|
||||
*
|
||||
* @param {string[]} permissions
|
||||
* @param {string} appUid
|
||||
* @param {object} [deps] Injectable seams for tests.
|
||||
* @param {string} [deps.authToken]
|
||||
* @param {typeof fetch} [deps.fetchImpl]
|
||||
* @param {string} [deps.apiOrigin]
|
||||
* @param {number} [deps.timeoutMs]
|
||||
* @returns {Promise<boolean>}
|
||||
*/
|
||||
export const appHoldsPermissions = (permissions, appUid, deps = {}) => {
|
||||
if ( ! appUid ) return Promise.resolve(false);
|
||||
const { authToken = window.auth_token, ...rest } = deps;
|
||||
return queryHeld(permissions, { ...rest, appUid, token: authToken });
|
||||
};
|
||||
|
||||
export default holdsPermissions;
|
||||
|
||||
@@ -18,7 +18,7 @@
|
||||
*/
|
||||
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { holdsPermissions } from './holdsPermissions.js';
|
||||
import { appHoldsPermissions, holdsPermissions } from './holdsPermissions.js';
|
||||
|
||||
const EMAIL = 'user:u-1:email:read';
|
||||
const APPS = 'apps-of-user:u-1:read';
|
||||
@@ -102,3 +102,36 @@ describe('holdsPermissions', () => {
|
||||
expect(calls).toHaveLength(0);
|
||||
});
|
||||
});
|
||||
|
||||
const FILE = 'fs:2b7d8c1e-4f3a-4b6c-9d1e-0a1b2c3d4e5f:write';
|
||||
const appDeps = (fetchImpl) => ({ ...deps(fetchImpl), authToken: 'user-token' });
|
||||
|
||||
describe('appHoldsPermissions', () => {
|
||||
it('asks as the user about the named app', async () => {
|
||||
const { fetchImpl, calls } = makeFetch({ held: { [FILE]: true } });
|
||||
|
||||
await expect(appHoldsPermissions([FILE], 'app-uid-1', appDeps(fetchImpl)))
|
||||
.resolves.toBe(true);
|
||||
|
||||
expect(calls[0].headers.Authorization).toBe('Bearer user-token');
|
||||
expect(calls[0].body).toEqual({ permissions: [FILE], app_uid: 'app-uid-1' });
|
||||
});
|
||||
|
||||
// Asked as the user with no app named, the user's own file answers `true`.
|
||||
it('reports not held, and asks nothing, without an app to name', async () => {
|
||||
const { fetchImpl, calls } = makeFetch({ held: { [FILE]: true } });
|
||||
|
||||
await expect(appHoldsPermissions([FILE], undefined, appDeps(fetchImpl)))
|
||||
.resolves.toBe(false);
|
||||
await expect(appHoldsPermissions([FILE], '', appDeps(fetchImpl)))
|
||||
.resolves.toBe(false);
|
||||
|
||||
expect(calls).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('reports not held when the app has not been granted it', async () => {
|
||||
const { fetchImpl } = makeFetch({ held: { [FILE]: false } });
|
||||
await expect(appHoldsPermissions([FILE], 'app-uid-1', appDeps(fetchImpl)))
|
||||
.resolves.toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user