diff --git a/src/backend/controllers/oidc/OIDCController.test.ts b/src/backend/controllers/oidc/OIDCController.test.ts index 1d6af9860..9dd6d0831 100644 --- a/src/backend/controllers/oidc/OIDCController.test.ts +++ b/src/backend/controllers/oidc/OIDCController.test.ts @@ -329,6 +329,54 @@ describe('OIDCController GET /auth/oidc/:provider/start', () => { expect(decoded).toMatchObject({ referrer: 'http://ref.test' }); }); + it('bakes a whitelisted return_to (/app/) into the state redirect_uri', async () => { + const { res, captured } = makeRes(); + await callRoute( + 'get', + '/auth/oidc/:provider/start', + makeReq({ + params: { provider: 'custom' }, + query: { return_to: '/app/my-App_2' }, + }), + res, + ); + const state = new URL(captured.redirectUrl ?? '').searchParams.get( + 'state', + ); + const decoded = oidc().verifyState(state!); + expect(String(decoded?.redirect_uri)).toBe( + `${TEST_ORIGIN}/app/my-App_2`, + ); + }); + + it('ignores a non-whitelisted return_to', async () => { + const bad_values = [ + '/app/evil/extra', + '/app/', + '/app/name?x=1', + '//evil.test', + '/settings', + `/app/${'a'.repeat(101)}`, + ]; + for (const return_to of bad_values) { + const { res, captured } = makeRes(); + await callRoute( + 'get', + '/auth/oidc/:provider/start', + makeReq({ + params: { provider: 'custom' }, + query: { return_to }, + }), + res, + ); + const state = new URL( + captured.redirectUrl ?? '', + ).searchParams.get('state'); + const decoded = oidc().verifyState(state!); + expect(String(decoded?.redirect_uri)).toBe(TEST_ORIGIN); + } + }); + it('signs revalidate-flow state with user_uuid + flow=revalidate', async () => { const userUuid = uuidv4(); const { res, captured } = makeRes(); @@ -621,6 +669,76 @@ describe('OIDCController login callback', () => { expect(captured.cookies).toHaveLength(0); }); + it('redirects back to an /app/ landing after sign-in', async () => { + const state = oidc().signState({ + provider: 'custom', + redirect_uri: `${TEST_ORIGIN}/app/some-app`, + }); + vi.spyOn(oidc(), 'exchangeCodeForTokens').mockResolvedValue({ + access_token: 'access', + id_token: 'id', + } as never); + vi.spyOn(oidc(), 'getUserInfo').mockResolvedValue({ + sub: `sub-${Math.random().toString(36).slice(2, 8)}`, + email: `oidc-${Math.random().toString(36).slice(2, 8)}@test.local`, + email_verified: true, + } as never); + + const { res, captured } = makeRes(); + await callRoute( + 'get', + '/auth/oidc/callback/login', + makeReq({ query: { code: 'c', state } }), + res, + ); + expect(captured.cookies).toHaveLength(1); + expect(captured.redirectUrl).toBe(`${TEST_ORIGIN}/app/some-app`); + }); + + it('keeps an /app/ landing in the error redirect (suspended user)', async () => { + const sub = `sub-${Math.random().toString(36).slice(2, 8)}`; + const email = `sus-app-${Math.random().toString(36).slice(2, 8)}@test.local`; + const created = await runWithContext( + { req: makeReq({}) }, + () => + oidc().createUserFromOIDC('custom', { + sub, + email, + email_verified: true, + }), + ); + expect(created.success).toBe(true); + await server.stores.user.update(created.user!.id, { suspended: 1 }); + + const state = oidc().signState({ + provider: 'custom', + redirect_uri: `${TEST_ORIGIN}/app/some-app`, + }); + vi.spyOn(oidc(), 'exchangeCodeForTokens').mockResolvedValue({ + access_token: 'access', + id_token: 'id', + } as never); + vi.spyOn(oidc(), 'getUserInfo').mockResolvedValue({ + sub, + email, + email_verified: true, + } as never); + + const { res, captured } = makeRes(); + await callRoute( + 'get', + '/auth/oidc/callback/login', + makeReq({ query: { code: 'c', state } }), + res, + ); + expect(captured.redirectStatus).toBe(302); + const url = new URL(captured.redirectUrl ?? ''); + expect(url.pathname).toBe('/app/some-app'); + expect(url.searchParams.get('auth_error')).toBe('1'); + expect(url.searchParams.get('action')).toBe('login'); + expect(captured.cookies).toHaveLength(0); + }); + it('appends oidc_login=true and uses popup-style URL when state is from a popup flow', async () => { const sub = `sub-${Math.random().toString(36).slice(2, 8)}`; const email = `oidc-${Math.random().toString(36).slice(2, 8)}@test.local`; diff --git a/src/backend/controllers/oidc/OIDCController.ts b/src/backend/controllers/oidc/OIDCController.ts index f519e2558..784360e37 100644 --- a/src/backend/controllers/oidc/OIDCController.ts +++ b/src/backend/controllers/oidc/OIDCController.ts @@ -3,18 +3,19 @@ * * 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. + * 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. + * 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 . + * along with this program. If not, see + * [https://www.gnu.org/licenses/](https://www.gnu.org/licenses/). */ import crypto from 'node:crypto'; @@ -47,6 +48,17 @@ const ALLOWED_ERRORS = [ 'signup_blocked', ] as const; +// GUI pages an OIDC flow may return to: /desktop, /dashboard, and direct app +// landings (/app/, mirroring APP_NAME_REGEX in AppDriver). Strict +// whitelist — never a client-supplied URL (no open redirect). +function isWhitelistedReturnPath(path: string): boolean { + return ( + path === '/desktop' || + path === '/dashboard' || + /^\/app\/[a-zA-Z0-9_-]{1,100}$/.test(path) + ); +} + function buildErrorRedirectUrl( origin: string, sourceFlow: string, @@ -62,6 +74,20 @@ function buildErrorRedirectUrl( ? message : 'unauthorized'; + // Land back on the whitelisted page the flow started from (e.g. an + // /app/ landing) so the retry — and the eventual success — keeps + // the user's destination. redirect_uri comes from the signed state and + // was built server-side, but re-check the path against the whitelist. + let pagePath = '/'; + if (typeof stateDecoded?.redirect_uri === 'string') { + try { + const statePath = new URL(stateDecoded.redirect_uri).pathname; + if (isWhitelistedReturnPath(statePath)) pagePath = statePath; + } catch { + // unparsable redirect_uri: fall back to the root page + } + } + let params: URLSearchParams; if (stateDecoded?.embedded_in_popup && stateDecoded?.msg_id != null) { params = new URLSearchParams({ @@ -84,7 +110,7 @@ function buildErrorRedirectUrl( if (requestCode) { params.set('request_code', requestCode); } - return `${base}/?${params.toString()}`; + return `${base}${pagePath}?${params.toString()}`; } function appendQueryParam(url: string, key: string, value: string): string { @@ -101,8 +127,8 @@ function constantTimeEqual(a: string, b: string): boolean { } /** - * True iff `target` parses as a URL whose origin equals `origin`. Used to - * clamp OIDC redirect targets — `startsWith` would accept + * True iff `target` parses as a URL whose origin equals `origin`. Used to clamp + * OIDC redirect targets — `startsWith` would accept * `https://puter.com.evil.com` against `https://puter.com`. */ function isSameOrigin(target: string, origin: string): boolean { @@ -166,15 +192,15 @@ export class OIDCController extends PuterController { let appRedirectUri = flowRedirects[flow] ?? (origin || '/'); - // Optional GUI return path so login started from /desktop or - // /dashboard lands back there. Strict whitelist — never a - // client-supplied URL (no open redirect). + // Optional GUI return path so login started from /desktop, + // /dashboard, or an /app/ landing lands back there. const rawReturnTo = Array.isArray(req.query.return_to) ? req.query.return_to[0] : req.query.return_to; if ( (flow === 'login' || flow === 'signup') && - (rawReturnTo === '/desktop' || rawReturnTo === '/dashboard') + typeof rawReturnTo === 'string' && + isWhitelistedReturnPath(rawReturnTo) ) { appRedirectUri = `${origin}${rawReturnTo}`; } @@ -497,13 +523,14 @@ if (window.opener) { /** * Resolve an OIDC callback to a Puter user. In order: - * 1. Existing link on (provider, sub) → that user. - * 2. Email matches an existing account whose email is CONFIRMED → - * link (provider, sub) to that user. - * 3. Email matches an account whose email is UNCONFIRMED → refuse. - * We don't know who owns an unconfirmed address, so linking would - * let whoever controls the OIDC identity hijack a pending signup. - * 4. Otherwise create a new user and link. + * + * 1. Existing link on (provider, sub) → that user. + * 2. Email matches an existing account whose email is CONFIRMED → link + * (provider, sub) to that user. + * 3. Email matches an account whose email is UNCONFIRMED → refuse. We don't + * know who owns an unconfirmed address, so linking would let whoever + * controls the OIDC identity hijack a pending signup. + * 4. Otherwise create a new user and link. * * Step 2 also requires `email_verified !== false` on the OIDC side, * otherwise a malicious IdP could claim someone else's email. diff --git a/src/gui/src/UI/UIWindowLogin.js b/src/gui/src/UI/UIWindowLogin.js index 43668edd5..ec2d85fcd 100644 --- a/src/gui/src/UI/UIWindowLogin.js +++ b/src/gui/src/UI/UIWindowLogin.js @@ -23,6 +23,7 @@ import UIWindowRecoverPassword from './UIWindowRecoverPassword.js'; import UIWindowSignup from './UIWindowSignup.js'; import { KNOWN_OIDC_PROVIDERS, OIDC_GENERIC_PROVIDER_ICON, humanizeOidcProviderId } from '../util/openid.js'; import { offersFederatedSignInInPopup } from '../util/popupAuth.js'; +import { get_auth_redirect_url, get_oidc_return_to } from '../helpers/auth_redirect.js'; // ── 2FA Login CSS (injected once) ─────────────────────────────────────────── const LOGIN_2FA_CSS = ` @@ -260,14 +261,9 @@ async function UIWindowLogin (options) { if ( options.redirect_url === undefined ) { - if ( window.location?.href?.toLowerCase().endsWith('/action/login') ) - { - options.redirect_url = '/'; - } - else - { - options.redirect_url = window.location.href; - } + // stay on the page the login started from (e.g. an /app/ + // landing), with standalone auth pages going to the root dashboard + options.redirect_url = get_auth_redirect_url(); } return new Promise(async (resolve) => { @@ -420,8 +416,8 @@ async function UIWindowLogin (options) { let url = `${window.gui_origin}/auth/oidc/${provider}/start?flow=login`; // return to the interface the login started from (backend whitelists the path) - const return_to = window.location.pathname; - if ( return_to === '/desktop' || return_to === '/dashboard' ) { + const return_to = get_oidc_return_to(); + if ( return_to ) { url += `&return_to=${encodeURIComponent(return_to)}`; } diff --git a/src/gui/src/UI/UIWindowSignup.js b/src/gui/src/UI/UIWindowSignup.js index 189ea0308..22d8b79fc 100644 --- a/src/gui/src/UI/UIWindowSignup.js +++ b/src/gui/src/UI/UIWindowSignup.js @@ -25,6 +25,7 @@ import UIWindowCardVerificationRequired from './UIWindowCardVerificationRequired import UIWindowLogin from './UIWindowLogin.js'; import { KNOWN_OIDC_PROVIDERS, OIDC_GENERIC_PROVIDER_ICON, humanizeOidcProviderId } from '../util/openid.js'; import { offersFederatedSignInInPopup } from '../util/popupAuth.js'; +import { get_auth_redirect_url, get_oidc_return_to } from '../helpers/auth_redirect.js'; function UIWindowSignup(options) { options = options ?? {}; @@ -32,6 +33,11 @@ function UIWindowSignup(options) { options.has_head = options.has_head ?? true; options.send_confirmation_code = options.send_confirmation_code ?? false; options.show_close_button = options.show_close_button ?? true; + if (options.redirect_url === undefined) { + // stay on the page the signup started from (e.g. an /app/ + // landing), with standalone auth pages going to the root dashboard + options.redirect_url = get_auth_redirect_url(); + } return new Promise(async (resolve) => { const internal_id = window.uuidv4(); @@ -242,11 +248,8 @@ function UIWindowSignup(options) { .on('click', function () { let url = `${window.gui_origin}/auth/oidc/${provider}/start?flow=signup`; // return to the interface the signup started from (backend whitelists the path) - const return_to = window.location.pathname; - if ( - return_to === '/desktop' || - return_to === '/dashboard' - ) { + const return_to = get_oidc_return_to(); + if (return_to) { url += `&return_to=${encodeURIComponent(return_to)}`; } const referrer = @@ -571,7 +574,6 @@ function UIWindowSignup(options) { if (options.reload_on_success) { window.onbeforeunload = null; - // either options.redirect_url or the current page const redirectUrl = options.redirect_url || '/'; window.location.replace(redirectUrl); } else { diff --git a/src/gui/src/helpers/auth_redirect.js b/src/gui/src/helpers/auth_redirect.js new file mode 100644 index 000000000..5a94f0442 --- /dev/null +++ b/src/gui/src/helpers/auth_redirect.js @@ -0,0 +1,70 @@ +/** + * 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 . + */ + +/** + * Where to send the user after a successful login/signup started from the + * current page. Keeps the user on the page they authenticated from — most + * importantly direct app landings (`/app/`), so they end up in the app + * they came for instead of the dashboard. + * + * Standalone auth pages (`/action/login`, `/action/signup`, ...) have no page + * to return to, so they go to the root dashboard. Auth-flow query params + * (`action`, `auth_error`) are stripped so the reload doesn't re-open the + * auth window it just completed. + * + * @returns {string} URL to redirect to after successful authentication + */ +export const get_auth_redirect_url = () => { + const url = new URL(window.location.href); + const path_parts = url.pathname.split('/').filter(part => part); + if ( path_parts[0]?.toLowerCase() === 'action' ) { + return '/'; + } + if ( url.searchParams.has('auth_error') ) { + // OIDC error companions (see buildErrorRedirectUrl on the backend); + // `message` is only stripped alongside auth_error since on its own it + // may be a legitimate app param + url.searchParams.delete('message'); + url.searchParams.delete('request_code'); + } + url.searchParams.delete('action'); + url.searchParams.delete('auth_error'); + return url.toString(); +}; + +/** + * The `return_to` path to send along when starting an OIDC flow, or null if + * the current page isn't one the backend will return to. The backend strictly + * whitelists these (never a client-supplied URL): `/desktop`, `/dashboard`, + * and direct app landings (`/app/`), so OIDC login started from an app + * landing comes back to the app. + * + * @returns {string|null} whitelistable pathname, or null + */ +export const get_oidc_return_to = () => { + const pathname = window.location.pathname; + if ( pathname === '/desktop' || pathname === '/dashboard' ) { + return pathname; + } + // app landing: normalize away a trailing slash to match the backend whitelist + if ( /^\/app\/[^/]+\/?$/.test(pathname) ) { + return pathname.replace(/\/$/, ''); + } + return null; +};