From edbba9e2533752da2911f5decf6bc67d828a3b9c Mon Sep 17 00:00:00 2001 From: Daniel Salazar Date: Thu, 8 Oct 2026 20:23:45 -0700 Subject: [PATCH] fix: a dismissed account picker hands the opener no token --- src/gui/src/initgui.gates.test.js | 19 +++++++++++++++ src/gui/src/initgui.js | 16 +++++++++--- src/gui/src/util/popupAuth.js | 10 ++++++++ src/gui/src/util/popupAuth.test.js | 39 ++++++++++++++++++++++++++++++ 4 files changed, 81 insertions(+), 3 deletions(-) diff --git a/src/gui/src/initgui.gates.test.js b/src/gui/src/initgui.gates.test.js index f7dd67f7f..2cfc91a7d 100644 --- a/src/gui/src/initgui.gates.test.js +++ b/src/gui/src/initgui.gates.test.js @@ -68,3 +68,22 @@ describe('the popup token hand-off in postAuthActions', () => { } }); }); + +describe('the account pickers in a popup', () => { + const PICKER = 'picked_a_user_for_sdk_login = await UIWindowSessionList('; + + it('record that they were shown', () => { + // A dismissed picker has to outrank an existing relationship, so the + // relationship check needs to know one was shown. + const chunks = src.split(PICKER); + expect(chunks.length - 1).toBe(2); + for (const before of chunks.slice(0, -1)) { + expect(before.trimEnd()).toMatch(/showed_account_picker = true;$/); + } + }); + + it('only let the relationship stand in when no picker was shown', () => { + expect(src).toContain('showedAccountPicker: showed_account_picker'); + expect(src).not.toMatch(/if \(window\.userAppToken\) \{\s*window\.popup_signin_consent = true;/); + }); +}); diff --git a/src/gui/src/initgui.js b/src/gui/src/initgui.js index ec4eed494..901a1210e 100644 --- a/src/gui/src/initgui.js +++ b/src/gui/src/initgui.js @@ -85,7 +85,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 { deliversTokenAtBoot, runsUserAppTokenExchange } from './util/popupAuth.js'; +import { deliversTokenAtBoot, relationshipIsConsent, runsUserAppTokenExchange } from './util/popupAuth.js'; import { verifyOidcPopupReturn } from './util/popupOidcReturn.js'; /** @@ -1284,6 +1284,8 @@ window.initgui = async function (options) { installAppIconFallback(); let picked_a_user_for_sdk_login = false; + // Whether a popup put an account picker in front of the user. + let showed_account_picker = false; // update SDK if auth_token is different from the one in the SDK if (window.auth_token && puter.authToken !== window.auth_token) { @@ -1625,6 +1627,7 @@ window.initgui = async function (options) { await window.getUserAppToken(window.openerOrigin); } else { // Show session list so user can pick which account to use + showed_account_picker = true; picked_a_user_for_sdk_login = await UIWindowSessionList({ reload_on_success: false, draggable_body: false, @@ -1787,6 +1790,7 @@ window.initgui = async function (options) { (!window.userAppToken || window.url_query_params.get('request_auth')) ) { + showed_account_picker = true; picked_a_user_for_sdk_login = await UIWindowSessionList({ reload_on_success: false, draggable_body: false, @@ -1802,8 +1806,14 @@ window.initgui = async function (options) { // be re-approved on every visit — that grant is what // `checkUserSiteRelationship` reports. This is also what keeps the // file-picker and permission popups, which never show an account - // picker, from being blocked by the gate in `postAuthActions`. - if (window.userAppToken) { + // picker, from being blocked by the gate in `postAuthActions`. A + // picker the user dismissed here still means no. + if ( + relationshipIsConsent({ + hasRelationship: !!window.userAppToken, + showedAccountPicker: showed_account_picker, + }) + ) { window.popup_signin_consent = true; } } diff --git a/src/gui/src/util/popupAuth.js b/src/gui/src/util/popupAuth.js index 02c4b5e20..77da4d7ad 100644 --- a/src/gui/src/util/popupAuth.js +++ b/src/gui/src/util/popupAuth.js @@ -88,6 +88,16 @@ export const defersTokenToOpener = (action) => export const deliversTokenAtBoot = (action) => deliversTokenToOpener(action) && !defersTokenToOpener(action); +/** + * Whether the opener's existing relationship with the user stands in for + * consent. Only when no account picker was shown: a dismissed picker is a no. + * + * @param {{ hasRelationship: boolean, showedAccountPicker: boolean }} state + * @returns {boolean} + */ +export const relationshipIsConsent = ({ hasRelationship, showedAccountPicker }) => + hasRelationship && !showedAccountPicker; + /** * Whether a popup running `action` runs the user-app token exchange. * diff --git a/src/gui/src/util/popupAuth.test.js b/src/gui/src/util/popupAuth.test.js index 6b1667cf7..2b69b8c3a 100644 --- a/src/gui/src/util/popupAuth.test.js +++ b/src/gui/src/util/popupAuth.test.js @@ -24,6 +24,7 @@ import { deliversTokenAtBoot, deliversTokenToOpener, offersFederatedSignInInPopup, + relationshipIsConsent, runsUserAppTokenExchange, } from './popupAuth.js'; @@ -162,6 +163,44 @@ describe('any other popup action', () => { }); }); +describe('relationshipIsConsent', () => { + // How initgui decides whether a boot-time hand-off goes ahead. + const handsOverAtBoot = (action, { picked, hasRelationship, showedAccountPicker }) => + deliversTokenAtBoot(action) && + (picked || relationshipIsConsent({ hasRelationship, showedAccountPicker })); + + it('gives no token when the user dismissed the account picker', () => { + const state = { picked: false, hasRelationship: true, showedAccountPicker: true }; + expect(relationshipIsConsent(state)).toBe(false); + for ( const action of [undefined, 'sign-in'] ) { + expect(handsOverAtBoot(action, state)).toBe(false); + } + }); + + it('gives a token when the user picked an account', () => { + for ( const hasRelationship of [true, false] ) { + const state = { picked: true, hasRelationship, showedAccountPicker: true }; + expect(handsOverAtBoot('sign-in', state)).toBe(true); + } + }); + + it('counts the relationship as consent when no picker was shown', () => { + const state = { picked: false, hasRelationship: true, showedAccountPicker: false }; + expect(relationshipIsConsent(state)).toBe(true); + expect(handsOverAtBoot('login', state)).toBe(true); + expect(relationshipIsConsent({ ...state, hasRelationship: false })).toBe(false); + }); + + it('leaves the picker popups to answer with the pick', () => { + const state = { picked: false, hasRelationship: true, showedAccountPicker: false }; + for ( const action of PICKER_ACTIONS ) { + expect(handsOverAtBoot(action, state)).toBe(false); + expect(defersTokenToOpener(action)).toBe(true); + 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