mirror of
https://github.com/HeyPuter/puter.git
synced 2026-08-25 15:37:12 +00:00
Identify permission requesters by origin only
The request-permission action took `app_uid` straight from the query string and used it as the grant target whenever the origin was absent or unresolvable. Now that /auth/grant-user-app accepts `origin` and prefers `app_uid` when both arrive, the displayed identity and the grant target could diverge; with only `app_uid` in the URL the dialog rendered with an empty name, so a link could produce a bare "Allow" prompt for an unnamed requester. Resolve the uid from the origin alone, and let the server resolve it from that same origin when the client lookup fails. Also give the no-gesture consent popup a unique window name. UI.js does this on the direct path because window.open() reuses a window with a matching name, but the PuterDialog fallback opened under the default 'Puter' — the same name sign-in uses, so a consent click could navigate an in-progress sign-in popup away. And guard the IPC responder: an app that closes its own window while the dialog is up leaves target_iframe.contentWindow null.
This commit is contained in:
+4
-2
@@ -1314,8 +1314,10 @@ const ipc_listener = async (event, handled) => {
|
||||
else if ( event.data.msg === 'requestPermission' ) {
|
||||
// Always respond, even on validation/auth failure, so the SDK's
|
||||
// promise settles instead of hanging forever.
|
||||
// The app can close its own window while the dialog is up, which
|
||||
// tears down the iframe — posting into it must not throw.
|
||||
const respond = (granted) => {
|
||||
target_iframe.contentWindow.postMessage({
|
||||
target_iframe?.contentWindow?.postMessage({
|
||||
msg: 'permissionGranted',
|
||||
granted: granted,
|
||||
original_msg_id: msg_id,
|
||||
@@ -1351,7 +1353,7 @@ const ipc_listener = async (event, handled) => {
|
||||
|
||||
// report the user's decision to the requester window
|
||||
respond(granted === true);
|
||||
$(target_iframe).get(0).focus({ preventScroll: true });
|
||||
$(target_iframe).get(0)?.focus({ preventScroll: true });
|
||||
}
|
||||
//--------------------------------------------------------
|
||||
// showFontPicker
|
||||
|
||||
+10
-8
@@ -465,15 +465,17 @@ const postAuthActions = async (action) => {
|
||||
// promise pending until the user closes it by hand.
|
||||
let granted = false;
|
||||
try {
|
||||
// Identify the requesting app by its origin rather than trusting a
|
||||
// caller-supplied uid, falling back to the query param otherwise.
|
||||
let app_uid = window.url_query_params.get('app_uid') ?? undefined;
|
||||
// The requesting app is identified by its origin only. A uid from
|
||||
// the query string is never trusted: it is chosen by whoever
|
||||
// opened this page, so honouring it would let a link grant a
|
||||
// permission to an app the dialog never named. When the origin
|
||||
// can't be resolved here, the uid is left unset and the server
|
||||
// resolves it from the same origin the dialog displayed.
|
||||
let app_uid;
|
||||
if ( origin ) {
|
||||
try {
|
||||
app_uid = window.host_app_uid ?? await window.getAppUIDFromOrigin(origin);
|
||||
} catch (e) {
|
||||
// Keep the query-param fallback.
|
||||
}
|
||||
app_uid = window.host_app_uid
|
||||
?? await window.getAppUIDFromOrigin(origin)
|
||||
?? undefined;
|
||||
}
|
||||
|
||||
granted = await UIPermissionDialog({
|
||||
|
||||
@@ -16,6 +16,10 @@ class PuterDialog extends (globalThis.HTMLElement || Object) { // It will fall b
|
||||
* implicit-auth URL), skips the `puter.token` message handling and
|
||||
* `puterAuthState` bookkeeping (the caller owns the auth result), and
|
||||
* reports cancellation through `options.onCancel`.
|
||||
* @param {string} [options.popupName] - Window name for the popup. Give
|
||||
* each concurrent launcher a distinct name: `window.open()` reuses a
|
||||
* window that already carries the requested name, so sharing one name
|
||||
* navigates (and hijacks) a popup another pending flow is waiting on.
|
||||
* @param {Function} [options.onLaunch] - Called with the opened popup
|
||||
* window (or null if the browser blocked it) right after launch.
|
||||
* @param {Function} [options.onCancel] - Called when the user dismisses
|
||||
@@ -528,7 +532,7 @@ class PuterDialog extends (globalThis.HTMLElement || Object) { // It will fall b
|
||||
// Wire the "Continue" button to open the auth popup. Opening here is
|
||||
// safe from being popup-blocked because it happens inside a click.
|
||||
this.shadowRoot.querySelector('#launch-auth-popup')?.addEventListener('click', () => {
|
||||
const popup = openAuthPopup(this.#popupURL());
|
||||
const popup = this.#openPopup();
|
||||
// Pinned as the expected event.source in messageListener.
|
||||
this.authPopup = popup;
|
||||
|
||||
@@ -553,9 +557,20 @@ class PuterDialog extends (globalThis.HTMLElement || Object) { // It will fall b
|
||||
this.shadowRoot.querySelector('.close-btn')?.addEventListener('click', this.cancelListener);
|
||||
}
|
||||
|
||||
/**
|
||||
* Opens the popup under the caller's window name when one was given, so
|
||||
* concurrent launchers don't reuse (and steal) each other's window.
|
||||
* @returns {Window|null}
|
||||
*/
|
||||
#openPopup () {
|
||||
return this.options.popupName
|
||||
? openAuthPopup(this.#popupURL(), this.options.popupName)
|
||||
: openAuthPopup(this.#popupURL());
|
||||
}
|
||||
|
||||
open () {
|
||||
if ( hasUserActivation() ) {
|
||||
const popup = openAuthPopup(this.#popupURL());
|
||||
const popup = this.#openPopup();
|
||||
// Pinned as the expected event.source in messageListener.
|
||||
this.authPopup = popup;
|
||||
if ( this.options.popupURL && typeof this.options.onLaunch === 'function' ) {
|
||||
|
||||
@@ -1245,6 +1245,8 @@ class UI extends EventListener {
|
||||
// the gesture the browser requires.
|
||||
const dialog = new PuterDialog(() => {}, () => {}, {
|
||||
popupURL: url,
|
||||
// Same unique-name reasoning as the direct path above.
|
||||
popupName: `puter-permission-${msg_id}`,
|
||||
onLaunch: (popup) => watchPopup(popup),
|
||||
onCancel: () => settle(false),
|
||||
});
|
||||
|
||||
@@ -209,3 +209,20 @@ test.describe('puter.ui.requestPermission (env=web popup)', () => {
|
||||
await expect(page.locator('#log [data-entry="perm:driver:false"]')).toBeVisible();
|
||||
});
|
||||
});
|
||||
|
||||
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
|
||||
// requesting origin — never from the link. A link carrying only a uid
|
||||
// would otherwise prompt for an unnamed requester and grant to an app
|
||||
// the dialog never showed the user.
|
||||
await page.goto(
|
||||
'/action/request-permission?permission=driver%3Aputer-image-generation%3Agenerate' +
|
||||
'&app_uid=app-00000000-0000-4000-8000-000000000000',
|
||||
);
|
||||
await page.waitForFunction(() => !!window.puter?.authToken, null, { timeout: 60_000 });
|
||||
// Give the post-auth action a chance to run before asserting absence.
|
||||
await page.locator('.desktop').waitFor({ timeout: 60_000 });
|
||||
await expect(page.locator('dialog.perm-dialog')).toHaveCount(0);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user