Skip to content

Auth: land back on the /app/<name> the user came for after login/signup #3435

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jelveh wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Auth: land back on the /app/<name> the user came for after login/signup #3435

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 118 additions & 0 deletions src/backend/controllers/oidc/OIDCController.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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/<name>) 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();
Expand Down Expand Up @@ -621,6 +669,76 @@ describe('OIDCController login callback', () => {
expect(captured.cookies).toHaveLength(0);
});

it('redirects back to an /app/<name> 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/<name> 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`;
Expand Down
73 changes: 50 additions & 23 deletions src/backend/controllers/oidc/OIDCController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 <https://www.gnu.org/licenses/>.
* along with this program. If not, see
* [https://www.gnu.org/licenses/](https://www.gnu.org/licenses/).
*/

import crypto from 'node:crypto';
Expand Down Expand Up @@ -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/<name>, 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,
Expand All @@ -62,6 +74,20 @@ function buildErrorRedirectUrl(
? message
: 'unauthorized';

// Land back on the whitelisted page the flow started from (e.g. an
// /app/<name> 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({
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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/<name> 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}`;
}
Expand Down Expand Up @@ -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.
Expand Down
16 changes: 6 additions & 10 deletions src/gui/src/UI/UIWindowLogin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import UIWindow from './UIWindow.js';
import UIWindowRecoverPassword from './UIWindowRecoverPassword.js';
import UIWindowSignup from './UIWindowSignup.js';
import { KNOWN_OIDC_PROVIDERS, OIDC_GENERIC_PROVIDER_ICON, humanizeOidcProviderId } from '../util/openid.js';
import { get_auth_redirect_url, get_oidc_return_to } from '../helpers/auth_redirect.js';

// ── 2FA Login CSS (injected once) ───────────────────────────────────────────
const LOGIN_2FA_CSS = `
Expand Down Expand Up @@ -259,14 +260,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/<name>
// landing), with standalone auth pages going to the root dashboard
options.redirect_url = get_auth_redirect_url();
}

return new Promise(async (resolve) => {
Expand Down Expand Up @@ -411,8 +407,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)}`;
}

Expand Down
14 changes: 8 additions & 6 deletions src/gui/src/UI/UIWindowSignup.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,19 @@ import UIWindowPhoneVerificationRequired from './UIWindowPhoneVerificationRequir
import UIWindowCardVerificationRequired from './UIWindowCardVerificationRequired.js';
import UIWindowLogin from './UIWindowLogin.js';
import { KNOWN_OIDC_PROVIDERS, OIDC_GENERIC_PROVIDER_ICON, humanizeOidcProviderId } from '../util/openid.js';
import { get_auth_redirect_url, get_oidc_return_to } from '../helpers/auth_redirect.js';

function UIWindowSignup(options) {
options = options ?? {};
options.reload_on_success = options.reload_on_success ?? true;
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/<name>
// 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();
Expand Down Expand Up @@ -230,11 +236,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 =
Expand Down Expand Up @@ -559,7 +562,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 {
Expand Down
Loading
Loading