Carry a share link through the OIDC round trip (#3642)

A share email lands on `/?shared=…`, and its recipient is signed out or
in a temporary session more often than not. Password login reloads the
same URL, so the link came back with it — but an OIDC login leaves the
origin and returns to a URL the backend builds from a whitelist of bare
paths. `return_to` was a pathname only, and `/` wasn't on the list, so
the recipient came back signed in on the Dashboard's Apps tab with
nothing to say what had just been shared with them.

`shared` now travels with the path. The GUI sends it along with the
return path; the backend accepts it as the one parameter a `return_to`
may carry, and only values shaped like the masked share path the mail was
built from — everything else rejects the whole value, the path is still
matched as a raw string so a protocol-relative one can't smuggle an
origin through, and the query is rebuilt rather than reflected. The items
ride onto the error page too, so a failed attempt retries on the link
instead of losing it.

The root joins the whitelist for this, but only when it names something:
on its own it is where the flow already lands.

src/backend/controllers/oidc/OIDCController.test.ts

@@ -401,6 +401,97 @@ describe('OIDCController GET /auth/oidc/:provider/start', () => {
         }
     });
 
+    // A share email lands on `/?shared=…` and its recipient often has to sign
+    // in first, so the link has to survive the round trip to the provider.
+    describe('share links in return_to', () => {
+        const SHARE_UUID = '11111111-2222-3333-4444-555555555555';
+        const sharedPath = (name: string) => `/alice/${SHARE_UUID}/${name}`;
+
+        const redirectUriFor = async (return_to: string) => {
+            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',
+            );
+            return String(oidc().verifyState(state!)?.redirect_uri);
+        };
+
+        const returnToFor = (paths: string[], path = '/') => {
+            const params = new URLSearchParams();
+            for (const p of paths) params.append('shared', p);
+            return `${path}?${params.toString()}`;
+        };
+
+        it('carries a share link back to the root', async () => {
+            const uri = await redirectUriFor(
+                returnToFor([sharedPath('Report.pdf')]),
+            );
+            const url = new URL(uri);
+            expect(url.origin + url.pathname).toBe(`${TEST_ORIGIN}/`);
+            expect(url.searchParams.getAll('shared')).toEqual([
+                sharedPath('Report.pdf'),
+            ]);
+        });
+
+        it('carries every item, deduplicated, on any whitelisted page', async () => {
+            const uri = await redirectUriFor(
+                returnToFor(
+                    [
+                        sharedPath('a.txt'),
+                        sharedPath('b.txt'),
+                        sharedPath('a.txt'),
+                    ],
+                    '/desktop',
+                ),
+            );
+            const url = new URL(uri);
+            expect(url.origin + url.pathname).toBe(`${TEST_ORIGIN}/desktop`);
+            expect(url.searchParams.getAll('shared')).toEqual([
+                sharedPath('a.txt'),
+                sharedPath('b.txt'),
+            ]);
+        });
+
+        it('carries no more items than a share link may name', async () => {
+            const paths = Array.from({ length: 25 }, (_, i) =>
+                sharedPath(`file-${i}.txt`),
+            );
+            const uri = await redirectUriFor(returnToFor(paths));
+            expect(new URL(uri).searchParams.getAll('shared')).toEqual(
+                paths.slice(0, 20),
+            );
+        });
+
+        it('refuses a query it does not fully recognize', async () => {
+            const bad_values = [
+                // not a masked share path: no uuid, no item after it, or a
+                // hand-edited absolute path
+                returnToFor(['/alice/Documents/Report.pdf']),
+                returnToFor([`/alice/${SHARE_UUID}`]),
+                returnToFor([`/alice/${SHARE_UUID}/`]),
+                returnToFor(['']),
+                // a parameter that isn't `shared`, alone or alongside one
+                '/?x=1',
+                `${returnToFor([sharedPath('a.txt')])}&x=1`,
+                // the root is only a destination when it names something
+                '/',
+                // still no origin smuggling, share link or not
+                `//evil.test${returnToFor([sharedPath('a.txt')])}`,
+            ];
+            for (const return_to of bad_values) {
+                expect(await redirectUriFor(return_to)).toBe(TEST_ORIGIN);
+            }
+        });
+    });
+
     it('signs revalidate-flow state with user_uuid + flow=revalidate', async () => {
         const userUuid = uuidv4();
         const { res, captured } = makeRes();
@@ -693,6 +784,77 @@ describe('OIDCController login callback', () => {
         expect(captured.cookies).toHaveLength(0);
     });
 
+    it('redirects back to a share link after sign-in', async () => {
+        const shared = '/alice/11111111-2222-3333-4444-555555555555/Report.pdf';
+        const state = oidc().signState({
+            provider: 'custom',
+            redirect_uri: `${TEST_ORIGIN}/?shared=${encodeURIComponent(shared)}`,
+        });
+        const sub = `sub-${Math.random().toString(36).slice(2, 8)}`;
+        const email = `share-${Math.random().toString(36).slice(2, 8)}@test.local`;
+        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.origin + url.pathname).toBe(`${TEST_ORIGIN}/`);
+        expect(url.searchParams.getAll('shared')).toEqual([shared]);
+    });
+
+    it('keeps a share link on the error page so a retry still lands on it', async () => {
+        const shared = '/alice/11111111-2222-3333-4444-555555555555/Report.pdf';
+        const sub = `sub-${Math.random().toString(36).slice(2, 8)}`;
+        const email = `sus-${Math.random().toString(36).slice(2, 8)}@test.local`;
+        const created = await runWithContext({ req: makeReq({}) }, () =>
+            oidc().createUserFromOIDC('custom', {
+                sub,
+                email,
+                email_verified: true,
+            }),
+        );
+        await server.stores.user.update(created.user!.id, { suspended: 1 });
+
+        const state = oidc().signState({
+            provider: 'custom',
+            redirect_uri: `${TEST_ORIGIN}/?shared=${encodeURIComponent(shared)}`,
+        });
+        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,
+        );
+        const url = new URL(captured.redirectUrl ?? '');
+        expect(url.searchParams.get('auth_error')).toBe('1');
+        expect(url.searchParams.get('action')).toBe('login');
+        expect(url.searchParams.getAll('shared')).toEqual([shared]);
+    });
+
     it('redirects back to an /app/<name> landing after sign-in', async () => {
         const state = oidc().signState({
             provider: 'custom',

src/backend/controllers/oidc/OIDCController.ts

@@ -23,6 +23,11 @@ import { HttpError } from '../../core/http/HttpError.js';
 import type { PuterRouter } from '../../core/http/PuterRouter.js';
 import { PuterController } from '../types.js';
 import { sessionCookieFlags } from '../../util/cookieFlags.js';
+import { parseMaskedSharePath } from '../../services/fs/sharePathMask.js';
+import {
+    SHARE_DEEP_LINK_ITEMS_LIMIT,
+    SHARE_DEEP_LINK_PARAM,
+} from '../../services/share/shareDeepLink.js';
 
 const REVALIDATION_COOKIE_NAME = 'puter_revalidation';
 const REVALIDATION_EXPIRY_SEC = 300;
@@ -64,18 +69,84 @@ function resolutionErrorCode(code: string | undefined): string {
         : 'signup_blocked';
 }
 
-// GUI pages an OIDC flow may return to: /desktop, /dashboard, and direct app
-// landings (/app/<name> and its desktop-booted twin /desktop/app/<name>,
-// mirroring APP_NAME_REGEX in AppDriver). Strict whitelist — never a
-// client-supplied URL (no open redirect).
+// GUI pages an OIDC flow may return to: the root (where a share email lands),
+// /desktop, /dashboard, and direct app landings (/app/<name> and its
+// desktop-booted twin /desktop/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 === '/' ||
         path === '/desktop' ||
         path === '/dashboard' ||
         /^(\/desktop)?\/app\/[a-zA-Z0-9_-]{1,100}$/.test(path)
     );
 }
 
+/**
+ * The share items a return target's query names, or null when the query is
+ * anything else at all.
+ *
+ * A share email lands on `/?shared=…`, and its recipient usually has to sign in
+ * before they can see what was shared. An OIDC flow leaves the origin and comes
+ * back to a URL this server builds, so the parameter travels through the flow
+ * or the recipient returns to a bare Home with nothing to say what they were
+ * sent.
+ *
+ * `shared` is the only parameter that makes the trip, and only values shaped
+ * like the masked path the mail was built from — the value is user-visible
+ * text, so a hand-edited one is refused rather than reflected back into the
+ * browser.
+ */
+function sharedPathsFromReturnQuery(query: string): string[] | null {
+    const paths: string[] = [];
+    for (const [key, value] of new URLSearchParams(query)) {
+        if (key !== SHARE_DEEP_LINK_PARAM) return null;
+        const parsed = parseMaskedSharePath(value);
+        // The segment after the uuid is the shared item itself. A mask without
+        // one addresses the owner's parent directory, which is not the
+        // recipient's to open.
+        if (!parsed || !parsed.tail) return null;
+        // Same rule the link builder follows: the first items are the ones
+        // that travel, so what gets highlighted reads as the top of the list.
+        if (
+            paths.length < SHARE_DEEP_LINK_ITEMS_LIMIT &&
+            !paths.includes(value)
+        ) {
+            paths.push(value);
+        }
+    }
+    return paths;
+}
+
+/**
+ * A client-supplied `return_to` reduced to what will actually be redirected to,
+ * or null when it isn't a page an OIDC flow returns to.
+ *
+ * The path is matched as given, never parsed as a URL: a protocol-relative
+ * value (`//evil.test/desktop`) has to fail the whitelist rather than smuggle
+ * an origin through as a `pathname`. The query is rebuilt from the values that
+ * survived, so nothing reaches the redirect verbatim.
+ */
+function sanitizeReturnTo(raw: string): string | null {
+    const separator = raw.indexOf('?');
+    const path = separator === -1 ? raw : raw.slice(0, separator);
+    if (!isWhitelistedReturnPath(path)) return null;
+
+    const shared =
+        separator === -1
+            ? []
+            : sharedPathsFromReturnQuery(raw.slice(separator + 1));
+    if (shared === null) return null;
+    // The root is only a destination when it names something: on its own it is
+    // where the flow already lands.
+    if (shared.length === 0) return path === '/' ? null : path;
+
+    const params = new URLSearchParams();
+    for (const value of shared) params.append(SHARE_DEEP_LINK_PARAM, value);
+    return `${path}?${params.toString()}`;
+}
+
 function buildErrorRedirectUrl(
     origin: string,
     sourceFlow: string,
@@ -100,11 +171,17 @@ function buildErrorRedirectUrl(
     // /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.
+    // A share link's items come back too: the retry happens on that page, and
+    // its success reloads it, so they have to be on it to survive.
     let pagePath = '/';
+    let sharedPaths: string[] = [];
     if (typeof stateDecoded?.redirect_uri === 'string') {
         try {
-            const statePath = new URL(stateDecoded.redirect_uri).pathname;
-            if (isWhitelistedReturnPath(statePath)) pagePath = statePath;
+            const stateUrl = new URL(stateDecoded.redirect_uri);
+            if (isWhitelistedReturnPath(stateUrl.pathname)) {
+                pagePath = stateUrl.pathname;
+                sharedPaths = sharedPathsFromReturnQuery(stateUrl.search) ?? [];
+            }
         } catch {
             // unparsable redirect_uri: fall back to the root page
         }
@@ -145,6 +222,9 @@ function buildErrorRedirectUrl(
     if (requestCode) {
         params.set('request_code', requestCode);
     }
+    for (const path of sharedPaths) {
+        params.append(SHARE_DEEP_LINK_PARAM, path);
+    }
     return `${base}${pagePath}?${params.toString()}`;
 }
 
@@ -289,16 +369,17 @@ export class OIDCController extends PuterController {
                 let appRedirectUri = flowRedirects[flow] ?? (origin || '/');
 
                 // Optional GUI return path so login started from /desktop,
-                // /dashboard, or an /app/<name> landing lands back there.
+                // /dashboard, an /app/<name> landing, or a share link 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') &&
-                    typeof rawReturnTo === 'string' &&
-                    isWhitelistedReturnPath(rawReturnTo)
-                ) {
-                    appRedirectUri = `${origin}${rawReturnTo}`;
+                const returnTo =
+                    typeof rawReturnTo === 'string'
+                        ? sanitizeReturnTo(rawReturnTo)
+                        : null;
+                if ((flow === 'login' || flow === 'signup') && returnTo) {
+                    appRedirectUri = `${origin}${returnTo}`;
                 }
 
                 // Popup support

src/gui/src/helpers/auth_redirect.js

@@ -17,6 +17,8 @@
  * along with this program.  If not, see <https://www.gnu.org/licenses/>.
  */
 
+import parse_shared_path, { SHARED_PATH_PARAM } from './parse_shared_path.js';
+
 /**
  * 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
@@ -49,18 +51,15 @@ export const get_auth_redirect_url = () => {
 };
 
 /**
- * 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/<name>`, plus the desktop-booted
- * `/desktop/app/<name>`), so OIDC login started from an app landing comes back
- * to the app — and to the same interface it was opened in.
+ * The pathname part of an OIDC `return_to`, or null when the current page isn't
+ * one the backend will return to. The root is in here only for the share links
+ * below — on its own it is where the flow already lands.
  *
- * @returns {string|null} whitelistable pathname, or null
+ * @returns {string|null}
  */
-export const get_oidc_return_to = () => {
+const oidc_return_path = () => {
     const pathname = window.location.pathname;
-    if ( pathname === '/desktop' || pathname === '/dashboard' ) {
+    if ( pathname === '/' || pathname === '/desktop' || pathname === '/dashboard' ) {
         return pathname;
     }
     // app landing: normalize away a trailing slash to match the backend whitelist
@@ -69,3 +68,36 @@ export const get_oidc_return_to = () => {
     }
     return null;
 };
+
+/**
+ * The `return_to` 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/<name>`, plus the desktop-booted
+ * `/desktop/app/<name>`), so OIDC login started from an app landing comes back
+ * to the app — and to the same interface it was opened in.
+ *
+ * A share link (`?shared=`, from an email) is carried along with the path: the
+ * recipient usually has to sign in before they can see what was shared, and an
+ * OIDC round trip leaves the origin, so the parameter has to travel through the
+ * flow or they come back to a bare Home. Only well-formed values go — the
+ * backend refuses the rest, and a hand-edited link is no one's destination.
+ *
+ * @returns {string|null} whitelistable path, with its share items, or null
+ */
+export const get_oidc_return_to = () => {
+    const path = oidc_return_path();
+    if ( path === null ) return null;
+
+    const shared = new URLSearchParams(window.location.search ?? '')
+        .getAll(SHARED_PATH_PARAM)
+        .filter(value => parse_shared_path(value) !== null);
+    if ( shared.length === 0 ) {
+        // The root is only a destination when it names something.
+        return path === '/' ? null : path;
+    }
+
+    const params = new URLSearchParams();
+    for ( const value of shared ) params.append(SHARED_PATH_PARAM, value);
+    return `${path}?${params.toString()}`;
+};

src/gui/src/helpers/auth_redirect.test.js

@@ -20,11 +20,21 @@
 import { describe, it, expect, afterEach } from 'vitest';
 import { get_oidc_return_to } from './auth_redirect.js';
 
-const at = (pathname) => {
-    globalThis.window = { location: { pathname } };
+const at = (pathname, search = '') => {
+    globalThis.window = { location: { pathname, search } };
     return get_oidc_return_to();
 };
 
+const SHARE_UUID = '11111111-2222-3333-4444-555555555555';
+const shared_path = (name) => `/alice/${SHARE_UUID}/${name}`;
+
+/** `window.location.search` for a page opened by a share link. */
+const share_search = (...paths) => {
+    const params = new URLSearchParams();
+    for ( const path of paths ) params.append('shared', path);
+    return `?${params.toString()}`;
+};
+
 afterEach(() => {
     delete globalThis.window;
 });
@@ -45,6 +55,29 @@ describe('get_oidc_return_to', () => {
         expect(at('/desktop/app/editor/')).toBe('/desktop/app/editor');
     });
 
+    it('carries a share link so the item survives the round trip', () => {
+        expect(at('/', share_search(shared_path('Report.pdf')))).toBe(
+            `/${share_search(shared_path('Report.pdf'))}`,
+        );
+        expect(at('/desktop', share_search(shared_path('Report.pdf')))).toBe(
+            `/desktop${share_search(shared_path('Report.pdf'))}`,
+        );
+        expect(
+            at('/', share_search(shared_path('a.txt'), shared_path('b.txt'))),
+        ).toBe(`/${share_search(shared_path('a.txt'), shared_path('b.txt'))}`);
+    });
+
+    it('leaves behind everything that is not a share link', () => {
+        // a hand-edited value the backend would refuse anyway
+        expect(at('/', share_search('/alice/Documents/Report.pdf'))).toBe(null);
+        expect(at('/', '?shared=')).toBe(null);
+        // other parameters are not ours to carry
+        expect(at('/desktop', '?app=editor')).toBe('/desktop');
+        expect(
+            at('/desktop', `${share_search(shared_path('a.txt'))}&app=editor`),
+        ).toBe(`/desktop${share_search(shared_path('a.txt'))}`);
+    });
+
     it('returns null for anything the backend would reject', () => {
         expect(at('/')).toBe(null);
         expect(at('/settings')).toBe(null);