diff --git a/src/backend/controllers/auth/AuthController.test.ts b/src/backend/controllers/auth/AuthController.test.ts index 7cc824661..6d99313a5 100644 --- a/src/backend/controllers/auth/AuthController.test.ts +++ b/src/backend/controllers/auth/AuthController.test.ts @@ -5024,6 +5024,64 @@ describe('AuthController.handleCheckPermissions + handleListPermissions', () => ]); }); + // What lets a permission request settle without a prompt. The second string + // is a consent scope nothing implies, so the first's `true` is the grant. + it('check-permissions: an app-under-user actor sees a grant made to that app', async () => { + const { user, actor } = await makeUserAndActor(); + const app = await server.stores.app.create( + { + name: `cp-${uuidv4()}`, + title: 'TestCheckPermsApp', + index_url: 'https://check-perms.example.test/index.html', + }, + { ownerUserId: user.id }, + ); + const granted = `user:${user.uuid}:email:read`; + const ungranted = `apps-of-user:${user.uuid}:read`; + + const appActor = { + user: actor.user, + app: { id: app.id, uid: app.uid }, + } as unknown as Actor; + const before = makeRes(); + await inCtx(appActor, () => + controller.handleCheckPermissions( + makeReq( + { permissions: [granted, ungranted] }, + { actor: appActor }, + ), + before, + ), + ); + expect(before.body).toEqual({ + permissions: { [granted]: false, [ungranted]: false }, + }); + + await inCtx(actor, () => + controller.handleGrantUserApp( + makeReq( + { app_uid: app.uid, permission: granted, extra: {} }, + { actor }, + ), + makeRes(), + ), + ); + + const after = makeRes(); + await inCtx(appActor, () => + controller.handleCheckPermissions( + makeReq( + { permissions: [granted, ungranted] }, + { actor: appActor }, + ), + after, + ), + ); + expect(after.body).toEqual({ + permissions: { [granted]: true, [ungranted]: false }, + }); + }); + 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( diff --git a/src/gui/src/helpers/holdsPermissions.js b/src/gui/src/helpers/holdsPermissions.js new file mode 100644 index 000000000..da7139b81 --- /dev/null +++ b/src/gui/src/helpers/holdsPermissions.js @@ -0,0 +1,78 @@ +/* + * Copyright (C) 2024-present Puter Technologies Inc. + * + * This file is part of Puter. + * + * Puter is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +// The dialog waits behind this check, so a stalled read must not hold it up. +const CHECK_TIMEOUT_MS = 5000; + +/** + * 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 + * access and not the user's own. + * + * Used to skip a permission prompt that has nothing to ask about. A check that + * couldn't be made is not an answer: no token, an empty list, a failed read, or + * one that times out all report `false`, leaving the prompt to run. + * + * @param {string[]} permissions + * @param {string} token - App-under-user token to check as. + * @param {object} [deps] Injectable seams for tests. + * @param {typeof fetch} [deps.fetchImpl] + * @param {string} [deps.apiOrigin] + * @param {number} [deps.timeoutMs] + * @returns {Promise} + */ +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 default holdsPermissions; diff --git a/src/gui/src/helpers/holdsPermissions.test.js b/src/gui/src/helpers/holdsPermissions.test.js new file mode 100644 index 000000000..984effc66 --- /dev/null +++ b/src/gui/src/helpers/holdsPermissions.test.js @@ -0,0 +1,104 @@ +/* + * Copyright (C) 2024-present Puter Technologies Inc. + * + * This file is part of Puter. + * + * Puter is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { describe, it, expect } from 'vitest'; +import { holdsPermissions } from './holdsPermissions.js'; + +const EMAIL = 'user:u-1:email:read'; +const APPS = 'apps-of-user:u-1:read'; + +// Minimal fetch double: records calls and answers with what is held. +const makeFetch = ({ held = {}, ok = true, throws = false } = {}) => { + const calls = []; + const fetchImpl = async (url, opts = {}) => { + calls.push({ url, method: opts.method, headers: opts.headers, body: JSON.parse(opts.body) }); + if ( throws ) throw new Error('network down'); + return { + ok, + status: ok ? 200 : 500, + json: async () => ({ permissions: held }), + }; + }; + return { fetchImpl, calls }; +}; + +const deps = (fetchImpl) => ({ fetchImpl, apiOrigin: 'https://api.test' }); + +describe('holdsPermissions', () => { + it('reports held only when every permission is held', async () => { + const both = makeFetch({ held: { [EMAIL]: true, [APPS]: true } }); + await expect(holdsPermissions([EMAIL, APPS], 'app-token', deps(both.fetchImpl))) + .resolves.toBe(true); + + const partial = makeFetch({ held: { [EMAIL]: true, [APPS]: false } }); + await expect(holdsPermissions([EMAIL, APPS], 'app-token', deps(partial.fetchImpl))) + .resolves.toBe(false); + }); + + it('asks as the app whose access is in question, deduped', async () => { + const { fetchImpl, calls } = makeFetch({ held: { [EMAIL]: true } }); + + await holdsPermissions([EMAIL, EMAIL], 'app-token', deps(fetchImpl)); + + expect(calls).toHaveLength(1); + expect(calls[0].url).toBe('https://api.test/auth/check-permissions'); + expect(calls[0].headers.Authorization).toBe('Bearer app-token'); + expect(calls[0].body).toEqual({ permissions: [EMAIL] }); + }); + + // A check that couldn't be made is not an answer: the caller prompts. + it('reports not held when the read fails', async () => { + const failed = makeFetch({ ok: false }); + await expect(holdsPermissions([EMAIL], 'app-token', deps(failed.fetchImpl))) + .resolves.toBe(false); + + const broken = makeFetch({ throws: true }); + await expect(holdsPermissions([EMAIL], 'app-token', deps(broken.fetchImpl))) + .resolves.toBe(false); + }); + + // The prompt waits behind this check. + it('gives up on a read that outlasts its timeout', async () => { + const calls = []; + const fetchImpl = (url, opts = {}) => { + calls.push(opts.signal); + return new Promise((_resolve, reject) => { + opts.signal?.addEventListener('abort', () => reject(new Error('aborted'))); + }); + }; + + await expect(holdsPermissions([EMAIL], 'app-token', { + fetchImpl, + apiOrigin: 'https://api.test', + timeoutMs: 5, + })).resolves.toBe(false); + // Aborted rather than left running behind the answer. + expect(calls[0]?.aborted).toBe(true); + }); + + it('does not ask at all without a token or a permission', async () => { + const { fetchImpl, calls } = makeFetch({ held: { [EMAIL]: true } }); + + await expect(holdsPermissions([EMAIL], '', deps(fetchImpl))).resolves.toBe(false); + await expect(holdsPermissions([], 'app-token', deps(fetchImpl))).resolves.toBe(false); + await expect(holdsPermissions(null, 'app-token', deps(fetchImpl))).resolves.toBe(false); + + expect(calls).toHaveLength(0); + }); +}); diff --git a/src/gui/src/initgui.js b/src/gui/src/initgui.js index 96dc9cac1..889dc1460 100644 --- a/src/gui/src/initgui.js +++ b/src/gui/src/initgui.js @@ -51,6 +51,7 @@ import { wantsFullToken, } from './util/authmeGrant.js'; import init_device_signals from './helpers/deviceSignals.js'; +import { holdsPermissions } from './helpers/holdsPermissions.js'; import item_icon from './helpers/itemIcon.js'; import launch_app from './helpers/launchApp.js'; import { parse_url_paths } from './helpers/urlPaths.js'; @@ -81,6 +82,9 @@ const postAuthActions = async (action) => { // bootstraps the app row a permission grant is written against, so an // action that depends on it has to report failure rather than prompt. let token_exchange_failed = false; + // The token the exchange minted for the opener's app, kept only to ask what + // that app holds — a question this window's own token cannot answer. + let user_app_token = null; // ------------------------------------------------------------------------------------- // Action: AuthMe — redirect to a third-party URL with the user's auth token // ------------------------------------------------------------------------------------- @@ -385,6 +389,7 @@ const postAuthActions = async (action) => { // This is an implicit app and the app_uid is sent back from the server // we cache it here so that we can use it later window.host_app_uid = data.app_uid; + user_app_token = data.token; // send token to parent. The opener is unreachable when it is // cross-origin isolated (COOP severs the relationship); those // flows learn the outcome server-side instead. @@ -688,6 +693,9 @@ const postAuthActions = async (action) => { if ( token_exchange_failed ) { throw new Error('token exchange failed; not prompting'); } + // A signed-out opener holds no token to settle this for itself, so + // ask here, as the app, with the token the exchange just minted. + const already_held = await holdsPermissions(permissions, user_app_token); // The requesting app is identified by its origin, and only the // server turns that origin into a grant target. No uid is sent // from here: a uid from the query string is chosen by whoever @@ -700,7 +708,7 @@ const postAuthActions = async (action) => { // name. Passing the origin instead makes the server resolve the // same origin the dialog displayed, and reject it outright // unless it names an app that really exists. - granted = await UIPermissionDialog({ + granted = already_held || await UIPermissionDialog({ // See IPC.js: both forms, so a single scope still works with a // dialog that only understands the scalar. permissions, diff --git a/src/puter-js/src/modules/UI.js b/src/puter-js/src/modules/UI.js index 653232f37..7a23f8a07 100644 --- a/src/puter-js/src/modules/UI.js +++ b/src/puter-js/src/modules/UI.js @@ -2,6 +2,7 @@ import EventListener from '../lib/EventListener.js'; import { hasUserActivation, openAuthPopup } from '../lib/auth-popup.js'; import FSItem from './FSItem.js'; import PuterDialog from './PuterDialog.js'; +import { checkPermissions } from './perms/lib/holds.js'; /** @@ -278,6 +279,9 @@ const pipError = (name, message) => { const MAX_REQUESTED_PERMISSIONS = 16; +// Short on purpose: a request usually spends a user gesture while it waits. +const PERMISSION_CHECK_TIMEOUT_MS = 2000; + /** * An interface for interacting with another app. Returned by the UI methods * that launch or connect to one; it cannot be constructed directly. @@ -1707,6 +1711,47 @@ export class UIModule extends EventListener { document.body.appendChild(el); }; + /** + * Whether every permission in a request is already held, read without + * prompting and without changing anything. + * + * A check that couldn't be made is not an answer: no token, an unreadable + * request, a failed read, or one that outlasts its timeout all fall through + * to the prompt. + * + * @param {{ permission?: string, permissions?: string[] }} options + * @returns {Promise} + */ + async #alreadyHeld (options) { + if ( ! this.authToken ) return false; + const requested = Array.isArray(options?.permissions) + ? options.permissions + : [options?.permission]; + // The prompt paths answer false for a shape they can't read themselves. + if ( requested.length === 0 + || requested.length > MAX_REQUESTED_PERMISSIONS + || requested.some(p => typeof p !== 'string' || p === '') ) { + return false; + } + let expiry; + try { + const held = await Promise.race([ + checkPermissions(this.puter, [...new Set(requested)]), + // Waiting out a stalled read would cost the popup its gesture. + new Promise((resolve) => { + expiry = setTimeout(() => resolve(null), PERMISSION_CHECK_TIMEOUT_MS); + }), + ]); + if ( held === null ) return false; + // Every scope: one prompt is one decision, so partly-held is unheld. + return requested.every(p => held[p] === true); + } catch (e) { + return false; + } finally { + clearTimeout(expiry); + } + } + /** * Asks the user to grant a permission to this app. Inside the Puter GUI * the request is relayed to the desktop; on the web the permission @@ -1715,10 +1760,19 @@ export class UIModule extends EventListener { * One prompt may cover several scopes: pass `permissions` instead of * `permission` and the user answers for the whole list at once. * + * Access already granted resolves `true` without prompting. + * * @param {{ permission?: string, permissions?: string[] }} options * @returns {Promise} `true` only if the permission was granted. */ async requestPermission (options) { + // Only where a prompt would be raised. Elsewhere this answers false + // without asking anyone, and a check must not turn that into a grant. + if ( ( this.env === 'app' || this.env === 'web' ) + && await this.#alreadyHeld(options) ) { + return true; + } + if ( this.env === 'app' ) { const result = await this.#postMessageAsync('requestPermission', { options }); return result.granted === true; @@ -1954,12 +2008,13 @@ export class UIModule extends EventListener { 'Content-Type': 'application/json', 'Authorization': `Bearer ${puter.authToken}`, }, - body: JSON.stringify({ permissions: [permission] }), + body: JSON.stringify({ permissions: requested }), ...(controller ? { signal: controller.signal } : {}), }); if ( ! resp.ok ) continue; const data = await resp.json(); - if ( data?.permissions?.[permission] === true ) { + // The whole list, since one prompt is one decision. + if ( requested.every(p => data?.permissions?.[p] === true) ) { settle(true); } } catch (e) { diff --git a/src/puter-js/src/modules/UI.requestPermission.test.js b/src/puter-js/src/modules/UI.requestPermission.test.js new file mode 100644 index 000000000..7121068c4 --- /dev/null +++ b/src/puter-js/src/modules/UI.requestPermission.test.js @@ -0,0 +1,150 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +// The permission check goes through the shared request helper, so mocking it +// needs no server. +const mockReq = vi.fn(); +vi.mock('./perms/lib/req.js', () => ({ req: (...args) => mockReq(...args) })); + +const { UIModule } = await import('./UI.js'); + +const CHECK_ROUTE = '/auth/check-permissions'; + +/** Answer the mocked helper with what is held, and nothing else. */ +const held = (permissions = {}) => { + mockReq.mockImplementation(async (_puter, route) => { + if ( route === CHECK_ROUTE ) return { permissions }; + throw new Error(`unexpected route: ${route}`); + }); +}; + +const postMessage = vi.fn(); + +/** A UI module in one environment, with the GUI it posts to stubbed. */ +const makeUI = ({ env = 'app', authToken = 'token-1' } = {}) => + new UIModule({ + env, + authToken, + APIOrigin: 'https://api.test', + appID: 'app-1', + util: {}, + }, { appInstanceID: 'instance-1' }); + +/** The messages sent to the GUI, ignoring the constructor's READY. */ +const promptCalls = () => + postMessage.mock.calls.filter(([msg]) => msg?.msg === 'requestPermission'); + +beforeEach(() => { + globalThis.window = { parent: { postMessage } }; + mockReq.mockReset(); + postMessage.mockReset(); + held(); +}); + +afterEach(() => { + delete globalThis.window; +}); + +describe('ui.requestPermission on access that is already held', () => { + it('resolves true without prompting the user', async () => { + held({ 'fs:/alice/Documents:read': true }); + const ui = makeUI(); + + await expect(ui.requestPermission({ + permission: 'fs:/alice/Documents:read', + })).resolves.toBe(true); + + expect(promptCalls()).toHaveLength(0); + expect(mockReq).toHaveBeenCalledWith(ui.puter, CHECK_ROUTE, { + permissions: ['fs:/alice/Documents:read'], + }); + }); + + it('resolves true for a whole multi-scope request', async () => { + held({ 'apps-of-user:u-1:read': true, 'user:u-1:email:read': true }); + const ui = makeUI(); + + await expect(ui.requestPermission({ + permissions: ['apps-of-user:u-1:read', 'user:u-1:email:read'], + })).resolves.toBe(true); + + expect(promptCalls()).toHaveLength(0); + }); + + it('answers a third-party site the same way, with no popup', async () => { + held({ 'driver:puter-image-generation:generate': true }); + const ui = makeUI({ env: 'web' }); + + await expect(ui.requestPermission({ + permission: 'driver:puter-image-generation:generate', + })).resolves.toBe(true); + }); +}); + +describe('ui.requestPermission when the check does not settle it', () => { + // One prompt is one decision, so partly-held still has something to ask. + it('prompts for a partly-held set', async () => { + held({ 'apps-of-user:u-1:read': true }); + const ui = makeUI(); + + ui.requestPermission({ + permissions: ['apps-of-user:u-1:read', 'user:u-1:email:read'], + }); + + await vi.waitFor(() => expect(promptCalls()).toHaveLength(1)); + }); + + it('prompts when nothing is held', async () => { + const ui = makeUI(); + + ui.requestPermission({ permission: 'user:u-1:email:read' }); + + await vi.waitFor(() => expect(promptCalls()).toHaveLength(1)); + }); + + // Not a grant, and not a refusal either: the prompt is where this went before. + it('prompts when the check fails', async () => { + mockReq.mockImplementation(async () => ({ + error: true, + code: 'internal_error', + })); + const ui = makeUI(); + + ui.requestPermission({ permission: 'user:u-1:email:read' }); + + await vi.waitFor(() => expect(promptCalls()).toHaveLength(1)); + }); + + // The request is spending a user gesture while it waits. + it('prompts rather than waiting out a stalled check', async () => { + mockReq.mockImplementation(() => new Promise(() => {})); + const ui = makeUI(); + + ui.requestPermission({ permission: 'user:u-1:email:read' }); + + await vi.waitFor(() => expect(promptCalls()).toHaveLength(1), { timeout: 10_000 }); + }, 15_000); + + it('does not ask the server when there is no token to ask with', async () => { + const ui = makeUI({ authToken: null }); + + ui.requestPermission({ permission: 'user:u-1:email:read' }); + + await vi.waitFor(() => expect(promptCalls()).toHaveLength(1)); + expect(mockReq).not.toHaveBeenCalled(); + }); +}); + +describe('ui.requestPermission where no prompt can be raised', () => { + // This environment has always answered false, and a check run as the user — + // who holds far more than the app would — must not turn that into a grant. + it('keeps answering false in the GUI, without a check', async () => { + held({ 'user:u-1:email:read': true }); + const ui = makeUI({ env: 'gui' }); + + await expect(ui.requestPermission({ + permission: 'user:u-1:email:read', + })).resolves.toBe(false); + + expect(mockReq).not.toHaveBeenCalled(); + }); +}); diff --git a/src/puter-js/tests/api/suites/perms.suite.ts b/src/puter-js/tests/api/suites/perms.suite.ts index 3a62a08bb..51ac83ee4 100644 --- a/src/puter-js/tests/api/suites/perms.suite.ts +++ b/src/puter-js/tests/api/suites/perms.suite.ts @@ -289,12 +289,13 @@ export default suite('perms', { } }, - // The one-method-per-task names still ship for apps written against them, - // and still prompt without consulting what is already held — which is why - // the folder and apps assertions here are the opposite of what `request` - // now answers for the same access. `requestPermission` is the exception: - // it forwards to `request`, so it picks up the new behaviour (asserted - // separately below). + // The one-method-per-task names still ship, and go straight to the prompt + // without pooling a read of their own — which is why the folder and apps + // assertions here are the opposite of what `request` answers. Node and + // workerd raise no prompt, so nothing is consulted; in an app or on a + // website `ui.requestPermission` checks first and these would answer for + // access already held. `requestPermission` forwards to `request`, so it + // settles from the pooled read everywhere (asserted separately below). 'the deprecated request aliases keep delegating': { platforms: ['node', 'workerd'], fn: async (t) => { diff --git a/src/puter-js/tests/e2e/fixtures/request-permission.html b/src/puter-js/tests/e2e/fixtures/request-permission.html index 77f97eeb0..873832876 100644 --- a/src/puter-js/tests/e2e/fixtures/request-permission.html +++ b/src/puter-js/tests/e2e/fixtures/request-permission.html @@ -29,6 +29,14 @@