From f20448cf98a4ef6202f468877d71d40c4935f9ec Mon Sep 17 00:00:00 2001 From: jelveh Date: Sun, 26 Jul 2026 12:02:24 -0700 Subject: [PATCH] Deliver the uncertain-grant withdrawal from a closing popup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The permission dialog reconciles a denial after an uncertain grant by firing a revoke in the background, but the popup flow posts the answer and closes the window right after settling — and a plain fetch is cancelled with its document, so the withdrawal never reached the server and the user was told "denied" while the grant stayed live. Send it with keepalive so the browser delivers it independently of the popup, and cover the popup flow with a regression test (the existing withdrawal tests only exercise the desktop flow, where the GUI outlives the dialog). Also make the popup boot's getAppUIDFromOrigin guard functional: the helper reports failure by resolving to a null/undefined uid, not by throwing, so the catch never engaged and a failed lookup clobbered window.host_app_uid with undefined despite the comment claiming the token exchange's value was kept. --- src/gui/src/UI/UIPermissionDialog.js | 4 ++ src/gui/src/initgui.js | 11 +-- .../tests/e2e/specs/requestPermission.spec.js | 70 +++++++++++++++++++ 3 files changed, 81 insertions(+), 4 deletions(-) diff --git a/src/gui/src/UI/UIPermissionDialog.js b/src/gui/src/UI/UIPermissionDialog.js index b4d6313d0..65ffca046 100644 --- a/src/gui/src/UI/UIPermissionDialog.js +++ b/src/gui/src/UI/UIPermissionDialog.js @@ -316,6 +316,10 @@ async function undo_uncertain_grant (options) { permission: options.permission, }), method: 'POST', + // In the popup flow the answer is posted and the window closed + // right after `settle`, and a plain fetch is cancelled with its + // document — taking this reconciliation down with it. + keepalive: true, }); } catch (e) { console.error('Failed to withdraw an uncertain permission grant', e); diff --git a/src/gui/src/initgui.js b/src/gui/src/initgui.js index 76840e9ed..37d1ec1dc 100644 --- a/src/gui/src/initgui.js +++ b/src/gui/src/initgui.js @@ -273,14 +273,17 @@ const postAuthActions = async (action) => { if ( window.openerOrigin ) { try { - app_uid = await window.getAppUIDFromOrigin(window.openerOrigin); - window.host_app_uid = app_uid; + // `getAppUIDFromOrigin` reports failure by resolving to a + // null/undefined uid, not by throwing — so check the value, + // and on either failure mode keep the host_app_uid set by + // the token exchange above, which resolved the same origin. + const resolved_app_uid = await window.getAppUIDFromOrigin(window.openerOrigin); + app_uid = resolved_app_uid ?? window.host_app_uid; } catch (e) { - // Keep the host_app_uid set by the token exchange above; a - // throw here would wedge the popup with no way to answer. console.error('getAppUIDFromOrigin failed', e); app_uid = window.host_app_uid; } + window.host_app_uid = app_uid; } if ( action === 'show-open-file-picker' ) { diff --git a/src/puter-js/tests/e2e/specs/requestPermission.spec.js b/src/puter-js/tests/e2e/specs/requestPermission.spec.js index 54300e0b1..f22bd45d2 100644 --- a/src/puter-js/tests/e2e/specs/requestPermission.spec.js +++ b/src/puter-js/tests/e2e/specs/requestPermission.spec.js @@ -708,6 +708,76 @@ test.describe('puter.ui.requestPermission (env=web, cross-origin-isolated)', () }); }); +test.describe('request-permission popup reconciliation', () => { + test('a denial after an uncertain grant is withdrawn even though the popup closes itself', async ({ page, context }) => { + // Same reconciliation the env=app suite covers, but in the popup flow, + // where answering is immediately followed by `window.close()`. The + // withdrawal request is fired from the closing document, so unless it + // is sent `keepalive` the browser cancels it with the popup — leaving + // the user told "denied" while the grant is live in their account. + const permission = 'driver:puter-image-generation:generate'; + const fixtureOrigin = new URL(PERMISSION_FIXTURE_URL).origin; + + // A GUI-origin page for server-side state checks: the fixture origin + // cannot fetch the API directly (Chrome blocks loopback-address + // requests from it), but the GUI origin can. + await page.goto('/'); + await page.waitForFunction(() => !!window.getUserAppToken && !!window.auth_token, + null, { timeout: 60_000 }); + const appUid = await page.evaluate( + async (origin) => (await window.getUserAppToken(origin))?.app_uid, + fixtureOrigin, + ); + expect(typeof appUid).toBe('string'); + const checker = await context.newPage(); + await checker.goto('/'); + await checker.waitForFunction(() => !!window.auth_token, null, { timeout: 60_000 }); + const isGranted = () => checker.evaluate(async ({ perm, uid }) => { + const res = await fetch(`${window.api_origin}/auth/list-permissions`, { + headers: { 'Authorization': `Bearer ${window.auth_token}` }, + }); + const body = await res.json(); + return body.myself_to_app.some( + r => r.permission === perm && r.app_uid === uid, + ); + }, { perm: permission, uid: appUid }); + + await page.goto(PERMISSION_FIXTURE_URL); + await page.locator('body.ready').waitFor({ timeout: 60_000 }); + + // Grant for real first, so there is a live row the withdrawal must + // remove. + let [popup] = await Promise.all([ + page.waitForEvent('popup'), + page.locator('#req-driver-perm').click(), + ]); + await expect(popup.locator('dialog.perm-dialog')).toBeVisible({ timeout: 60_000 }); + await popup.locator('dialog.perm-dialog .perm-dialog-allow').click(); + await expect(page.locator('#log [data-entry="perm:driver:true"]')).toBeVisible(); + expect(await isGranted()).toBe(true); + + // Now make the next grant's outcome unknowable (a 5xx says nothing + // about whether the row was written — and here one already is). + await context.route('**/auth/grant-user-app', route => + route.fulfill({ status: 502, body: '{}' })); + + [popup] = await Promise.all([ + page.waitForEvent('popup'), + page.locator('#req-driver-perm').click(), + ]); + const dialog = popup.locator('dialog.perm-dialog'); + await expect(dialog).toBeVisible({ timeout: 60_000 }); + await dialog.locator('.perm-dialog-allow').click(); + await expect(dialog.locator('.perm-dialog-error')).toBeVisible({ timeout: 30_000 }); + await dialog.locator('.perm-dialog-deny').click(); + + // The popup answers and closes itself; the withdrawal must survive it. + await expect(page.locator('#log [data-entry="perm:driver:false"]')).toBeVisible(); + await expect.poll(() => popup.isClosed(), { timeout: 15_000 }).toBe(true); + await expect.poll(isGranted, { timeout: 20_000 }).toBe(false); + }); +}); + test.describe('request-permission action hardening', () => { test('an app_uid in the URL never produces a prompt on its own', async ({ page }) => { // The uid identifies who receives the grant, so it must come from the