Deliver the uncertain-grant withdrawal from a closing popup

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.
This commit is contained in:
jelveh
2026-07-26 12:02:24 -07:00
parent 391510a49e
commit f20448cf98
3 changed files with 81 additions and 4 deletions
+4
View File
@@ -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);
+7 -4
View File
@@ -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' ) {
@@ -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