mirror of
https://github.com/HeyPuter/puter.git
synced 2026-08-24 23:17:23 +00:00
fix: don't mint a user-app token as a side effect of the feedback popup
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.
This commit is contained in:
Nariman Jelveh
+61
-57
@@ -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(
|
||||
|
||||
@@ -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/<name>` or `?action=<name>`); 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/<name>` or `?action=<name>`); 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.
|
||||
*
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user