Release: v0.2.2

apps/api/src/routes/saml.ts

@@ -32,6 +32,8 @@ import type { Person, PrivateProfile } from '@cfp/shared/schemas';
 const CHAT_CHANNEL_REGEX = /^[a-z0-9][a-z0-9_-]{0,40}$/;
 const RESUME_COOKIE = 'cfp_saml_resume';
 const RESUME_COOKIE_TTL_SECONDS = 10 * 60;
+/** Site-relative — it is also the `?return=` handed to /login (see safeReturnPath). */
+const RESUME_PATH = '/api/saml/slack/sso/resume';
 
 /**
  * SAML ID values must start with an XML NCName character — `_` followed by
@@ -55,26 +57,6 @@ function safeReturnPath(input: string | undefined | null): string {
   return input;
 }
 
-function selfUrl(request: FastifyRequest): string {
-  const protocol = request.headers['x-forwarded-proto']
-    ? String(request.headers['x-forwarded-proto']).split(',')[0]?.trim() ?? 'http'
-    : request.protocol;
-  const host = request.headers['x-forwarded-host']
-    ? String(request.headers['x-forwarded-host']).split(',')[0]?.trim() ?? request.hostname
-    : request.hostname;
-  return `${protocol}://${host}${request.url}`;
-}
-
-function originBase(request: FastifyRequest): string {
-  const protocol = request.headers['x-forwarded-proto']
-    ? String(request.headers['x-forwarded-proto']).split(',')[0]?.trim() ?? 'http'
-    : request.protocol;
-  const host = request.headers['x-forwarded-host']
-    ? String(request.headers['x-forwarded-host']).split(',')[0]?.trim() ?? request.hostname
-    : request.hostname;
-  return `${protocol}://${host}`;
-}
-
 function htmlEscape(value: string): string {
   return value
     .replace(/&/g, '&')
@@ -361,8 +343,10 @@ async function handleSpInitiatedSso(
       path: '/api/saml',
       maxAge: RESUME_COOKIE_TTL_SECONDS,
     });
-    const resumeReturn = `${originBase(request)}/api/saml/slack/sso/resume`;
-    return reply.redirect(`/login?return=${encodeURIComponent(resumeReturn)}`);
+    // Site-relative on purpose: `/login` and the OAuth start both run the
+    // return through the same leading-slash guard as `safeReturnPath`, so an
+    // absolute URL here is silently dropped and the user lands on `/`.
+    return reply.redirect(`/login?return=${encodeURIComponent(RESUME_PATH)}`);
   }
 
   // Signed in — build the assertion immediately.
@@ -462,10 +446,11 @@ export async function samlRoutes(fastify: FastifyInstance): Promise<void> {
     async (request, reply) => {
       const cfg = fastify.config;
 
-      // Anonymous → bounce through /login preserving the current URL.
+      // Anonymous → bounce through /login preserving the current path + query
+      // (`channel` / `redir` ride along). Site-relative: see the SP-initiated
+      // handler for why an absolute URL would be dropped.
       if (!request.session.personId) {
-        const here = selfUrl(request);
-        return reply.redirect(`/login?return=${encodeURIComponent(here)}`);
+        return reply.redirect(`/login?return=${encodeURIComponent(safeReturnPath(request.url))}`);
       }
 
       const query = request.query as { channel?: string; redir?: string };
@@ -589,7 +574,7 @@ export async function samlRoutes(fastify: FastifyInstance): Promise<void> {
   // -------------------------------------------------------------------------
 
   fastify.get(
-    '/api/saml/slack/sso/resume',
+    RESUME_PATH,
     {
       schema: {
         tags: ['saml'],

apps/api/tests/saml.test.ts

@@ -73,6 +73,16 @@ function slackAuthnRequestXml(id: string, acsHost: string = SLACK_TEAM_HOST): st
 <samlp:AuthnRequest xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" ID="${id}" Version="2.0" IssueInstant="2026-05-01T00:00:00Z" AssertionConsumerServiceURL="https://${acsHost}/sso/saml" ProtocolBinding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST"><saml:Issuer>https://slack.com</saml:Issuer></samlp:AuthnRequest>`;
 }
 
+/**
+ * The decoded `?return=` a /login redirect carries. Must be site-relative:
+ * both /login and the OAuth start drop anything not beginning with one `/`.
+ */
+function loginReturnOf(location: string | string[] | undefined): string {
+  const url = new URL(String(location), 'http://placeholder.test');
+  expect(url.pathname).toBe('/login');
+  return url.searchParams.get('return') ?? '';
+}
+
 /**
  * HTTP-Redirect binding encoding (saml-bindings §3.4.4.1): raw DEFLATE →
  * base64 → URL-encode. Returns the ready-to-append query string.
@@ -232,10 +242,21 @@ describe('SAML IdP — Slack', () => {
     expect(formats).toContain('urn:oasis:names:tc:SAML:2.0:nameid-format:persistent');
   });
 
-  it('GET /api/saml/slack/launch (anonymous) redirects to /login', async () => {
+  it('GET /api/saml/slack/launch (anonymous) redirects to /login with a site-relative return', async () => {
     const res = await app.inject({ method: 'GET', url: '/api/saml/slack/launch' });
     expect(res.statusCode).toBe(302);
-    expect(res.headers.location).toMatch(/^\/login\?return=/);
+    // An absolute URL here is silently dropped by /login's leading-slash guard
+    // and the user lands on `/` instead of coming back to finish SSO.
+    expect(loginReturnOf(res.headers.location)).toBe('/api/saml/slack/launch');
+  });
+
+  it('GET /api/saml/slack/launch (anonymous) keeps channel/redir in the return', async () => {
+    const res = await app.inject({
+      method: 'GET',
+      url: '/api/saml/slack/launch?channel=general',
+    });
+    expect(res.statusCode).toBe(302);
+    expect(loginReturnOf(res.headers.location)).toBe('/api/saml/slack/launch?channel=general');
   });
 
   it('GET /api/saml/slack/launch (signed-in) returns auto-submit form with signed SAML response', async () => {
@@ -400,7 +421,7 @@ describe('SAML IdP — Slack', () => {
       url: `/api/saml/slack/sso?${redirectBindingQuery(slackAuthnRequestXml('id-redirect-1'), 'opaque-redirect-state')}`,
     });
     expect(res.statusCode).toBe(302);
-    expect(res.headers.location).toMatch(/^\/login\?return=/);
+    expect(loginReturnOf(res.headers.location)).toBe('/api/saml/slack/sso/resume');
     expect(resumeCookieValue(res)).not.toBe('');
   });
 
@@ -513,7 +534,7 @@ describe('SAML IdP — Slack', () => {
       headers: { 'content-type': 'application/x-www-form-urlencoded' },
     });
     expect(res.statusCode).toBe(302);
-    expect(res.headers.location).toMatch(/^\/login\?return=/);
+    expect(loginReturnOf(res.headers.location)).toBe('/api/saml/slack/sso/resume');
     // Resume cookie set
     const cookies = res.headers['set-cookie'];
     const cookieStr = Array.isArray(cookies) ? cookies.join('\n') : String(cookies ?? '');

apps/web/src/lib/return-path.ts

@@ -0,0 +1,26 @@
+import type { NavigateFunction } from 'react-router';
+
+/**
+ * Where to land after sign-in / account claim. Mirrors the API's
+ * `safeReturnPath`: site-relative only (one leading slash), so a crafted
+ * `?return=` can't bounce the browser off-site. Anything else → `/`.
+ */
+export function safeReturn(input: string | null): string {
+  if (!input) return '/';
+  if (!input.startsWith('/') || input.startsWith('//')) return '/';
+  return input;
+}
+
+/**
+ * `/api/**` is served by Fastify, not the SPA — the SAML resume endpoint hands
+ * the browser on to Slack from there. Client-side routing to one of those
+ * paths never reaches the server and renders the SPA's `*` NotFound instead,
+ * so they need a real navigation. Everything else stays in-app.
+ */
+export function goToReturn(navigate: NavigateFunction, path: string): void {
+  if (path.startsWith('/api/')) {
+    window.location.assign(path);
+    return;
+  }
+  void navigate(path, { replace: true });
+}

apps/web/src/pages/AccountClaim.tsx

@@ -11,6 +11,7 @@ import {
 import { Button } from '@/components/ui/button';
 import { Separator } from '@/components/ui/separator';
 import { useAuth } from '@/hooks/useAuth';
+import { goToReturn, safeReturn } from '@/lib/return-path';
 import {
   api,
   ApiError,
@@ -19,12 +20,6 @@ import {
 } from '@/lib/api';
 import { formatAbsoluteDate, formatRelativeTime } from '@/lib/time';
 
-function safeReturn(input: string | null): string {
-  if (!input) return '/';
-  if (!input.startsWith('/') || input.startsWith('//')) return '/';
-  return input;
-}
-
 export function AccountClaim() {
   const [searchParams] = useSearchParams();
   const navigate = useNavigate();
@@ -82,7 +77,7 @@ export function AccountClaim() {
       await api.accountClaim.confirm(candidate.personId);
       toast.success(`Welcome back, ${candidate.fullName}`);
       await reload();
-      void navigate(returnPath, { replace: true });
+      goToReturn(navigate, returnPath);
     } catch (err) {
       const msg = err instanceof ApiError ? err.message : 'Failed to confirm';
       toast.error(msg);
@@ -96,7 +91,7 @@ export function AccountClaim() {
       await api.accountClaim.decline();
       toast.success("Got it — we'll set up a fresh profile");
       await reload();
-      void navigate(returnPath, { replace: true });
+      goToReturn(navigate, returnPath);
     } catch (err) {
       toast.error(err instanceof ApiError ? err.message : 'Failed to start fresh');
       setDeclining(false);

apps/web/src/pages/AccountClaimByPassword.tsx

@@ -13,12 +13,7 @@ import { Input } from '@/components/ui/input';
 import { Label } from '@/components/ui/label';
 import { api, ApiError } from '@/lib/api';
 import { useAuth } from '@/hooks/useAuth';
-
-function safeReturn(input: string | null): string {
-  if (!input) return '/';
-  if (!input.startsWith('/') || input.startsWith('//')) return '/';
-  return input;
-}
+import { goToReturn, safeReturn } from '@/lib/return-path';
 
 export function AccountClaimByPassword() {
   const [searchParams] = useSearchParams();
@@ -39,7 +34,7 @@ export function AccountClaimByPassword() {
       await api.accountClaim.byPassword(slug.trim(), password);
       toast.success('Welcome back');
       await reload();
-      void navigate(returnPath, { replace: true });
+      goToReturn(navigate, returnPath);
     } catch (err) {
       if (err instanceof ApiError) {
         if (err.status === 401 && err.code === 'claim_token_invalid') {

apps/web/src/pages/AccountClaimRequestStaffReview.tsx

@@ -14,12 +14,7 @@ import { Label } from '@/components/ui/label';
 import { Textarea } from '@/components/ui/textarea';
 import { api, ApiError } from '@/lib/api';
 import { useAuth } from '@/hooks/useAuth';
-
-function safeReturn(input: string | null): string {
-  if (!input) return '/';
-  if (!input.startsWith('/') || input.startsWith('//')) return '/';
-  return input;
-}
+import { goToReturn, safeReturn } from '@/lib/return-path';
 
 export function AccountClaimRequestStaffReview() {
   const [searchParams] = useSearchParams();
@@ -55,7 +50,7 @@ export function AccountClaimRequestStaffReview() {
     try {
       await api.accountClaim.decline();
       await reload();
-      void navigate(returnPath, { replace: true });
+      goToReturn(navigate, returnPath);
     } catch (err) {
       toast.error(err instanceof ApiError ? err.message : 'Failed to continue');
       setContinuing(false);

apps/web/src/pages/LoginPlaceholder.tsx

@@ -13,6 +13,7 @@ import { Label } from '@/components/ui/label';
 import { GitHubIcon } from '@/components/icons/GitHubIcon';
 import { useAuth } from '@/hooks/useAuth';
 import { api, ApiError } from '@/lib/api';
+import { goToReturn, safeReturn } from '@/lib/return-path';
 
 type ErrorCode =
   | 'access_denied'
@@ -93,15 +94,14 @@ export function LoginPlaceholder() {
   const navigate = useNavigate();
   const [searchParams] = useSearchParams();
 
-  const returnPath = searchParams.get('return');
+  const rawReturn = searchParams.get('return');
+  const returnPath = safeReturn(rawReturn);
   const errorCode = searchParams.get('error') as ErrorCode | null;
 
   // Redirect already-authenticated users
   useEffect(() => {
     if (!loading && person) {
-      const target =
-        returnPath && returnPath.startsWith('/') ? returnPath : '/';
-      void navigate(target, { replace: true });
+      goToReturn(navigate, returnPath);
     }
   }, [loading, person, navigate, returnPath]);
 
@@ -117,7 +117,7 @@ export function LoginPlaceholder() {
     );
   }
 
-  const startUrl = returnPath
+  const startUrl = rawReturn
     ? `/api/auth/github/start?return=${encodeURIComponent(returnPath)}`
     : '/api/auth/github/start';
 
@@ -126,9 +126,7 @@ export function LoginPlaceholder() {
     // picks up the new session before we navigate. Without this, the
     // navbar stays in its anonymous state until the next hard refresh.
     await reload();
-    const target =
-      returnPath && returnPath.startsWith('/') ? returnPath : '/';
-    void navigate(target, { replace: true });
+    goToReturn(navigate, returnPath);
   };
 
   return (

plans/saml-login-return-path.md

@@ -0,0 +1,63 @@
+---
+status: done
+depends: [saml-authn-statement]
+specs:
+  - specs/api/saml.md
+issues: []
+pr: 175
+---
+
+# Plan: SAML sign-in return path
+
+## Scope
+
+Slack's "Test configuration" against production got as far as our `/login`,
+then dropped the user on `/` instead of finishing the SSO round-trip. Two
+defects, found by tracing the first real end-to-end attempt:
+
+1. Both anonymous SAML entry points hand `/login?return=` an **absolute** URL
+   (`https://<host>/api/saml/slack/sso/resume`, and `selfUrl()` for
+   `/launch`). Every consumer of `?return=` — the login page, the OAuth start,
+   the account-claim pages — accepts only a site-relative path (one leading
+   `/`, per the laddr/Emergence convention) and falls back to `/` otherwise.
+2. Even with a correct path, the login and account-claim pages hand the
+   return to React Router. `/api/**` is served by Fastify, not the SPA, so a
+   client-side `navigate()` there renders the `*` NotFound page and never
+   reaches the resume endpoint. Only the GitHub OAuth path escaped, because
+   the API redirects server-side.
+
+In: spec the return contract, fix both entry points, one shared web helper
+that hard-navigates for API routes, tests that pin the exact return. Out:
+a general "remember where you were" for arbitrary protected pages (already
+handled per-page via `?return=`); any change to the assertion itself.
+
+## Implements
+
+- [api/saml.md](../specs/api/saml.md) — `## GET /api/saml/slack/launch` step 1
+  and `## GET | POST /api/saml/slack/sso` step 2: the return is a
+  site-relative path (never absolute), and the login page performs a full
+  navigation when it is an API route.
+
+## Approach
+
+- `apps/api/src/routes/saml.ts`: `RESUME_PATH` constant used by both the park
+  redirect and the route registration; `/launch` returns
+  `safeReturnPath(request.url)` so `channel`/`redir` ride along. Drop the two
+  absolute-URL helpers that no longer have callers.
+- `apps/web/src/lib/return-path.ts`: `safeReturn` (was copy-pasted into three
+  account-claim pages; the login page had a weaker inline guard without the
+  `//` check) and `goToReturn`, which uses `window.location.assign` for
+  `/api/` targets and `navigate()` otherwise. All four pages use it.
+- Tests: the existing anonymous-redirect assertions only matched
+  `^/login?return=` and passed with the absolute URL. Decode the `return` and
+  assert the exact path, plus a `/launch?channel=` case.
+
+## Validation
+
+- `npm run type-check && npm run lint && npm test` clean.
+- Slack "Test configuration" against the deployed build completes the
+  round-trip after a fresh sign-in (the live check this plan exists for).
+
+## Follow-ups
+
+None.

specs/api/saml.md

@@ -114,7 +114,7 @@ Rules:
 
 ### Behavior
 
-1. Require a signed-in session. If not signed in → redirect to `/login?return=<encoded current URL>`.
+1. Require a signed-in session. If not signed in → redirect to `/login?return=<encoded site-relative path + query of this request>`. The return is a path, never an absolute URL: `/login` and the OAuth start both drop anything that does not begin with a single `/`.
 2. Validate the member is permitted (default: any `user` accountLevel; configurable via the same `userIsPermitted` hook the legacy code provided).
 3. Build a SAML Response containing the NameID + attributes for the current Person.
 4. Sign the Response with the IdP private key.
@@ -159,7 +159,7 @@ Query string (saml-bindings §3.4.4.1, `DEFLATE` encoding):
 ### Behavior
 
 1. Decode + parse the AuthnRequest (inflate first for the Redirect binding). Validate signature if Slack signs requests (configurable; usually no for Slack).
-2. Require a signed-in session. If not → store the AuthnRequest in a short-lived signed cookie, redirect to `/login?return=/api/saml/slack/sso?resume=1`. After login the user comes back here and the AuthnRequest replays from the cookie.
+2. Require a signed-in session. If not → store the AuthnRequest in a short-lived signed cookie, redirect to `/login?return=/api/saml/slack/sso/resume` (site-relative, as for `/launch`). After login the user comes back to `/sso/resume` and the AuthnRequest replays from the cookie. Because that return is an API route rather than an SPA route, the login page performs a full navigation to it instead of client-side routing.
 3. Resolve the AuthnRequest's `AssertionConsumerServiceURL` against Slack's documented ACS endpoint(s) — only Slack's ACS is accepted.
 4. Build + sign a SAML Response as in `/launch`.
 5. POST back to Slack's ACS via the auto-submitting form, including `RelayState`. The Response always goes back over HTTP-POST regardless of which binding carried the request — Slack's ACS only accepts POST.