From 4f96d371f44d07473a4eb7d5575a5f73886520a5 Mon Sep 17 00:00:00 2001 From: Nariman Jelveh Date: Tue, 11 Aug 2026 18:13:13 -0700 Subject: [PATCH] fix: don't mint a user-app token as a side effect of the feedback popup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every embedded_in_popup boot ran the user-app token exchange, and the exchange is a write: /auth/get-user-app-token bootstraps an app row for the opener origin, grants flag:app-is-authenticated (what makes the site count as connected to the account), and creates its AppData dir. So merely opening — or immediately cancelling — a send-feedback popup recorded a user<->site relationship the read-only feedback flow never needs: the server resolves the feedback target from the attested origin without any of it. Gate the exchange behind runsUserAppTokenExchange(action) in all three popup paths that mint (main postAuthActions exchange, temp-user signup success, manual signup fallback). request-permission keeps the exchange since grants are written against the app row it bootstraps. --- src/gui/src/initgui.js | 118 +++++++++++++++-------------- src/gui/src/util/popupAuth.js | 31 +++++++- src/gui/src/util/popupAuth.test.js | 32 ++++++++ 3 files changed, 122 insertions(+), 59 deletions(-) diff --git a/src/gui/src/initgui.js b/src/gui/src/initgui.js index 69ce81e02..cc508acec 100644 --- a/src/gui/src/initgui.js +++ b/src/gui/src/initgui.js @@ -72,7 +72,7 @@ import { ThemeService } from './services/ThemeService.js'; // silently resolve to the factory — use `window.privacy_aware_path` instead. import { privacy_aware_path as privacy_aware_path_factory } from './util/desktop.js'; import { resolveAPIOrigin } from './util/apiOrigin.js'; -import { deliversTokenToOpener } from './util/popupAuth.js'; +import { deliversTokenToOpener, runsUserAppTokenExchange } from './util/popupAuth.js'; import { verifyOidcPopupReturn } from './util/popupOidcReturn.js'; const postAuthActions = async (action) => { @@ -367,7 +367,7 @@ const postAuthActions = async (action) => { }); } return; - } else { + } else if ( runsUserAppTokenExchange(action) ) { const deliver_token_to_opener = deliversTokenToOpener(action); try { let data = await window.getUserAppToken(new URL(window.openerOrigin).origin); @@ -2180,37 +2180,39 @@ window.initgui = async function (options) { (async () => { let closing = false; try { - let msg_id = - window.url_query_params.get('msg_id'); - let data = await window.getUserAppToken( - new URL(window.openerOrigin).origin, - ); - // A network failure here returns null and - // an HTTP failure returns the parsed error - // body, neither of which carries a token; - // the reads below would fault or hand the - // opener an `undefined` token. - if (!data?.token) { - throw new Error( - 'user-app token exchange returned no token', - ); - } - // 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; - // send token to parent - 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, + if (runsUserAppTokenExchange(action)) { + let msg_id = + window.url_query_params.get('msg_id'); + let data = await window.getUserAppToken( + new URL(window.openerOrigin).origin, ); + // A network failure here returns null and + // an HTTP failure returns the parsed error + // body, neither of which carries a token; + // the reads below would fault or hand the + // opener an `undefined` token. + if (!data?.token) { + throw new Error( + 'user-app token exchange returned no token', + ); + } + // 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; + // send token to parent + 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') { @@ -2273,32 +2275,34 @@ window.initgui = async function (options) { if (window.embedded_in_popup) (async () => { try { - let msg_id = - window.url_query_params.get('msg_id'); - let data = await window.getUserAppToken( - new URL(window.openerOrigin).origin, - ); - if (!data?.token) { - throw new Error( - 'user-app token exchange returned no token', - ); - } - // 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; - // send token to parent - 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, + if (runsUserAppTokenExchange(action)) { + let msg_id = + window.url_query_params.get('msg_id'); + let data = await window.getUserAppToken( + new URL(window.openerOrigin).origin, ); + if (!data?.token) { + throw new Error( + 'user-app token exchange returned no token', + ); + } + // 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; + // send token to parent + 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, + ); + } } } catch (err) { console.error( diff --git a/src/gui/src/util/popupAuth.js b/src/gui/src/util/popupAuth.js index f99acf83f..2a3fc3cc4 100644 --- a/src/gui/src/util/popupAuth.js +++ b/src/gui/src/util/popupAuth.js @@ -39,9 +39,10 @@ const NON_AUTH_POPUP_ACTIONS = new Set(['request-permission', 'send-feedback']); /** * Whether a popup running `action` may post `puter.token` to its opener. * - * The token exchange itself still runs for the excluded actions — it bootstraps + * For `request-permission` the token exchange itself still runs — it bootstraps * the app row a permission grant needs and caches `host_app_uid` — only the - * hand-off to the opener is suppressed. + * hand-off to the opener is suppressed. `send-feedback` skips the exchange + * entirely; see {@link runsUserAppTokenExchange}. * * @param {string|null|undefined} action - The popup's `action`, as parsed from * the URL (`/action/` or `?action=`); undefined for a plain @@ -51,6 +52,32 @@ const NON_AUTH_POPUP_ACTIONS = new Set(['request-permission', 'send-feedback']); export const deliversTokenToOpener = (action) => !NON_AUTH_POPUP_ACTIONS.has(action); +/** + * Popup actions that must not run the user-app token exchange at all. + * + * The exchange (`/auth/get-user-app-token`) is a write, not a read: it + * bootstraps an app row for the opener origin, grants + * `flag:app-is-authenticated` (what makes the site count as connected to the + * account), and creates the app's per-user AppData directory. Sign-in and + * file-picker popups need that, and `request-permission` needs the + * bootstrapped app row a grant is written against — but a send-feedback popup + * only ever *reads* app identity from its attested origin server-side, so + * merely opening (or cancelling) the feedback dialog must not record a + * user↔site relationship. + */ +const TOKEN_EXCHANGE_FREE_ACTIONS = new Set(['send-feedback']); + +/** + * Whether a popup running `action` runs the user-app token exchange. + * + * @param {string|null|undefined} action - The popup's `action`, as parsed from + * the URL (`/action/` or `?action=`); undefined for a plain + * sign-in popup. + * @returns {boolean} `true` if the exchange should run for this popup. + */ +export const runsUserAppTokenExchange = (action) => + !TOKEN_EXCHANGE_FREE_ACTIONS.has(action); + /* * On the `opener_origin` URL parameter, which this module used to gate. * diff --git a/src/gui/src/util/popupAuth.test.js b/src/gui/src/util/popupAuth.test.js index 3287b71a7..3aaa54bcf 100644 --- a/src/gui/src/util/popupAuth.test.js +++ b/src/gui/src/util/popupAuth.test.js @@ -22,6 +22,7 @@ import * as popupAuth from './popupAuth.js'; import { deliversTokenToOpener, offersFederatedSignInInPopup, + runsUserAppTokenExchange, } from './popupAuth.js'; describe('deliversTokenToOpener', () => { @@ -32,6 +33,11 @@ describe('deliversTokenToOpener', () => { expect(deliversTokenToOpener('request-permission')).toBe(false); }); + it('withholds the token from a feedback popup', () => { + // Sending feedback is not consent to sign the site in either. + expect(deliversTokenToOpener('send-feedback')).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'] ) { @@ -68,6 +74,32 @@ describe('offersFederatedSignInInPopup', () => { }); }); +describe('runsUserAppTokenExchange', () => { + it('skips the exchange entirely for a feedback popup', () => { + // The exchange is a write, not a read: it bootstraps an app row for + // the opener origin and records the user↔site relationship. The + // feedback flow resolves app identity read-only from the attested + // origin server-side, so opening (or cancelling) the dialog must not + // connect the site to the account. + expect(runsUserAppTokenExchange('send-feedback')).toBe(false); + }); + + it('runs it for sign-in, the pickers, and the permission prompt', () => { + // request-permission keeps the exchange: the app row it bootstraps is + // what a grant is written against. + for ( const action of [ + undefined, + 'sign-in', + 'show-open-file-picker', + 'show-directory-picker', + 'show-save-file-picker', + 'request-permission', + ] ) { + expect(runsUserAppTokenExchange(action)).toBe(true); + } + }); +}); + describe('the retired opener_origin gate', () => { it('is gone, because no action believes the raw parameter now', () => { // It used to allow `opener_origin` for every action but