mirror of
https://github.com/HeyPuter/puter.git
synced 2026-08-24 23:17:23 +00:00
Withhold the auth token on the popup's first-visit paths
Keeping the token inside the permission popup only covered the plain token exchange. Two other popup paths mint a user-app token and posted it to the opener unconditionally: first-visit temp-user creation, and the manual signup shown when temp users are refused. Both sit on the path a brand-new visitor takes — the audience the website popup flow exists for — so a site that asked about one permission and was denied still walked away holding a token, for a temp account or a real one. The SDK's global puter.token handler feeds whatever arrives straight into setAuthToken(), so posting it is the whole of it. Move the rule into util/popupAuth.js and consult it at every site that posts the token, so the next token path has one place to ask. The first-visit path also left the prompt itself unreachable: it waits on the spinner promise, which only resolves when the spinner was up for under 2s. End that wait for any action that keeps the popup open; sign-in still closes the window as before. The e2e test fails without the fix — the site holds a token after the user presses "Don't Allow".
This commit is contained in:
jelveh
+34
-31
@@ -55,6 +55,7 @@ import { LocaleService } from './services/LocaleService.js';
|
||||
import { ProcessService } from './services/ProcessService.js';
|
||||
import { ThemeService } from './services/ThemeService.js';
|
||||
import { privacy_aware_path } from './util/desktop.js';
|
||||
import { deliversTokenToOpener } from './util/popupAuth.js';
|
||||
|
||||
const postAuthActions = async (action) => {
|
||||
// -------------------------------------------------------------------------------------
|
||||
@@ -196,15 +197,7 @@ const postAuthActions = async (action) => {
|
||||
}
|
||||
return;
|
||||
} else {
|
||||
// A permission prompt is not a sign-in: the user is deciding about
|
||||
// one permission, so the opener must not walk away holding this
|
||||
// user's credentials. The exchange below still runs (it bootstraps
|
||||
// the app row the grant needs and caches `host_app_uid`), but the
|
||||
// token stays in this window. Handing it over would also let a
|
||||
// failed exchange sign the opener out, because the SDK feeds the
|
||||
// `token: null` of the failure message straight into
|
||||
// setAuthToken().
|
||||
const deliver_token_to_opener = action !== 'request-permission';
|
||||
const deliver_token_to_opener = deliversTokenToOpener(action);
|
||||
try {
|
||||
let data = await window.getUserAppToken(new URL(window.openerOrigin).origin);
|
||||
// This is an implicit app and the app_uid is sent back from the server
|
||||
@@ -1730,21 +1723,29 @@ window.initgui = async function (options) {
|
||||
// we cache it here so that we can use it later
|
||||
window.host_app_uid = data.app_uid;
|
||||
// send token to parent
|
||||
window.opener.postMessage(
|
||||
{
|
||||
msg: 'puter.token',
|
||||
success: true,
|
||||
msg_id: msg_id,
|
||||
token: data.token,
|
||||
username: window.user.username,
|
||||
app_uid: data.app_uid,
|
||||
},
|
||||
window.openerOrigin,
|
||||
);
|
||||
if (deliversTokenToOpener(action)) {
|
||||
window.opener?.postMessage(
|
||||
{
|
||||
msg: 'puter.token',
|
||||
success: true,
|
||||
msg_id: msg_id,
|
||||
token: data.token,
|
||||
username: window.user.username,
|
||||
app_uid: data.app_uid,
|
||||
},
|
||||
window.openerOrigin,
|
||||
);
|
||||
}
|
||||
// close popup
|
||||
if (!action || action === 'sign-in') {
|
||||
window.close();
|
||||
window.open('', '_self').close();
|
||||
} else {
|
||||
// Actions that keep the popup open still
|
||||
// have work to do after this wait; the
|
||||
// sleep below only ends it when the
|
||||
// spinner was up for under 2s.
|
||||
resolve();
|
||||
}
|
||||
})();
|
||||
if (spinner_duration < 2000) {
|
||||
@@ -1787,17 +1788,19 @@ window.initgui = async function (options) {
|
||||
// we cache it here so that we can use it later
|
||||
window.host_app_uid = data.app_uid;
|
||||
// send token to parent
|
||||
window.opener.postMessage(
|
||||
{
|
||||
msg: 'puter.token',
|
||||
success: true,
|
||||
msg_id: msg_id,
|
||||
token: data.token,
|
||||
username: window.user.username,
|
||||
app_uid: data.app_uid,
|
||||
},
|
||||
window.openerOrigin,
|
||||
);
|
||||
if (deliversTokenToOpener(action)) {
|
||||
window.opener?.postMessage(
|
||||
{
|
||||
msg: 'puter.token',
|
||||
success: true,
|
||||
msg_id: msg_id,
|
||||
token: data.token,
|
||||
username: window.user.username,
|
||||
app_uid: data.app_uid,
|
||||
},
|
||||
window.openerOrigin,
|
||||
);
|
||||
}
|
||||
// close popup
|
||||
if (!action || action === 'sign-in') {
|
||||
window.close();
|
||||
|
||||
@@ -0,0 +1,52 @@
|
||||
/**
|
||||
* 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 <https://www.gnu.org/licenses/>.
|
||||
*/
|
||||
|
||||
/**
|
||||
* Rules for what a GUI popup may hand back to the site that opened it.
|
||||
*
|
||||
* A popup boot (`?embedded_in_popup=true`) mints a user-app token on several
|
||||
* different paths — the plain token exchange, first-visit temp-user creation,
|
||||
* and manual signup when temp users are refused — and historically every one of
|
||||
* them posted `puter.token` to the opener. The SDK's global message listener
|
||||
* feeds that straight into `setAuthToken()`, so posting it signs the site in.
|
||||
*
|
||||
* That is only what the user asked for when the popup's whole purpose was
|
||||
* signing in. `request-permission` popups must not do it: the user is deciding
|
||||
* about a single permission, not handing over their account. Posting the
|
||||
* failure message is just as unsafe, since its `token: null` would clobber a
|
||||
* token the site already holds.
|
||||
*/
|
||||
|
||||
/** Popup actions that exist to answer a question, not to authenticate. */
|
||||
const NON_AUTH_POPUP_ACTIONS = new Set(['request-permission']);
|
||||
|
||||
/**
|
||||
* Whether a popup running `action` may post `puter.token` to its opener.
|
||||
*
|
||||
* The token exchange itself still runs for the excluded actions — it bootstraps
|
||||
* the app row a permission grant needs and caches `host_app_uid` — only the
|
||||
* hand-off to the opener is suppressed.
|
||||
*
|
||||
* @param {string|null|undefined} action - The popup's `action`, as parsed from
|
||||
* the URL (`/action/<name>` or `?action=<name>`); undefined for a plain
|
||||
* sign-in popup.
|
||||
* @returns {boolean} `true` if the token may be delivered to the opener.
|
||||
*/
|
||||
export const deliversTokenToOpener = (action) =>
|
||||
!NON_AUTH_POPUP_ACTIONS.has(action);
|
||||
@@ -0,0 +1,49 @@
|
||||
/*
|
||||
* 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 <https://www.gnu.org/licenses/>.
|
||||
*/
|
||||
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { deliversTokenToOpener } from './popupAuth.js';
|
||||
|
||||
describe('deliversTokenToOpener', () => {
|
||||
it('withholds the token from a permission prompt', () => {
|
||||
// Answering a permission prompt is not consent to hand the site this
|
||||
// user's credentials. Every popup path that mints a user-app token has
|
||||
// to honour this, not just the plain token exchange.
|
||||
expect(deliversTokenToOpener('request-permission')).toBe(false);
|
||||
});
|
||||
|
||||
it('delivers the token for the sign-in flows that exist to authenticate', () => {
|
||||
// `undefined` is a plain sign-in popup, which carries no action.
|
||||
for ( const action of [undefined, 'sign-in'] ) {
|
||||
expect(deliversTokenToOpener(action)).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
it('delivers the token for the other popup actions', () => {
|
||||
for ( const action of [
|
||||
'show-open-file-picker',
|
||||
'show-directory-picker',
|
||||
'show-save-file-picker',
|
||||
'login',
|
||||
'signup',
|
||||
] ) {
|
||||
expect(deliversTokenToOpener(action)).toBe(true);
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -418,6 +418,36 @@ test.describe('puter.ui.requestPermission (env=web popup)', () => {
|
||||
});
|
||||
});
|
||||
|
||||
test.describe('puter.ui.requestPermission (env=web popup, first visit)', () => {
|
||||
// No Puter session at all: the popup takes the first-visit branch, which
|
||||
// creates a temp user. That path mints its own user-app token, so it has to
|
||||
// withhold it from the opener for the same reason the plain token exchange
|
||||
// does — otherwise a site that only ever asked about one permission walks
|
||||
// away signed in as a brand-new account.
|
||||
test.use({ storageState: { cookies: [], origins: [] } });
|
||||
|
||||
test('creating a user to answer the prompt does not sign the site in', async ({ page }) => {
|
||||
await page.goto(PERMISSION_FIXTURE_URL);
|
||||
await page.locator('body.ready').waitFor({ timeout: 60_000 });
|
||||
expect(await page.evaluate(() => !!puter.authToken)).toBe(false);
|
||||
|
||||
const [popup] = await Promise.all([
|
||||
page.waitForEvent('popup'),
|
||||
page.locator('#req-driver-perm').click(),
|
||||
]);
|
||||
|
||||
// The prompt still has to arrive: the first-visit path waits on the
|
||||
// spinner before handing back control, and that wait has to end for
|
||||
// any action that keeps the popup open.
|
||||
await expect(popup.locator('dialog.perm-dialog')).toBeVisible({ timeout: 60_000 });
|
||||
await popup.locator('dialog.perm-dialog .perm-dialog-deny').click();
|
||||
await expect(page.locator('#log [data-entry="perm:driver:false"]')).toBeVisible();
|
||||
|
||||
expect(await page.evaluate(() => !!puter.authToken)).toBe(false);
|
||||
expect(await page.evaluate(() => localStorage.getItem('puter.auth.token.v2'))).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
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
|
||||
|
||||
Reference in New Issue
Block a user