From 223cef8a84b1f46c902d44b4d450fdaff8cf568d Mon Sep 17 00:00:00 2001 From: Chris Alfano Date: Fri, 18 Sep 2026 02:16:56 -0400 Subject: [PATCH 1/4] docs(specs): /chat launches Slack via SSO and accepts /chat/ Match laddr: the redirect targets Slack's SP-initiated SSO start URL with redir=/messages//, so the member is signed in through our IdP and lands in the channel. Years of codeforphilly.org/chat/ links depend on the path form; default channel is general. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01LFyA5poHwrhAktrnsKrUiQ --- specs/screens/chat.md | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/specs/screens/chat.md b/specs/screens/chat.md index a205ca5..cde91e3 100644 --- a/specs/screens/chat.md +++ b/specs/screens/chat.md @@ -1,38 +1,44 @@ -# Screen: Chat redirect +# Screen: Chat launch ## Route -`/chat` — public. Server-side redirect to the Code for Philly Slack workspace. +`/chat` and `/chat/` — public. Server-side redirect that signs the member into the Code for Philly Slack workspace and lands them in a channel. -Optional query parameter `?channel=` redirects to a specific channel. +`?channel=` is accepted as an alternative to the path form (the project "Chat Channel" button uses it). ## Behavior Not a rendered screen — a redirect endpoint handled at the API layer (and aliased on the web layer for nice URLs that work without JS). +The redirect targets Slack's **SP-initiated SSO start** URL, not the workspace directly. Slack then sends a SAML AuthnRequest to our IdP (`GET | POST /api/saml/slack/sso` — see [api/saml.md](../api/saml.md)), which signs the member in on our side if needed and posts the assertion back; Slack honours `redir` and opens the channel. This is the legacy laddr behaviour (`Emergence\Slack\Connector::handleLaunchRequest`), and the `codeforphilly.org/chat/` links distributed over the years depend on it. + ### Redirect rules | Request | Redirect target | HTTP | | ------- | --------------- | :--: | -| `/chat` | `https://codeforphilly.slack.com/` | 302 | -| `/chat?channel=foo` | `https://codeforphilly.slack.com/channels/foo` | 302 | +| `/chat` | `https://codeforphilly.slack.com/sso/saml/start?redir=%2Fmessages%2Fgeneral%2F` | 302 | +| `/chat/foo` | `https://codeforphilly.slack.com/sso/saml/start?redir=%2Fmessages%2Ffoo%2F` | 302 | +| `/chat?channel=foo` | Same as `/chat/foo` | 302 | +| `/chat/foo/` (trailing slash) | Same as `/chat/foo` | 302 | | `/chat?channel=` (empty) | Same as `/chat` | 302 | -| `/chat?channel=` | Same as `/chat`, with a query log warning | 302 | +| `/chat/` or `?channel=` | Same as `/chat`, with a log warning | 302 | + +The default channel is `general`, as in laddr. -`channel` is validated against the same regex as `Project.chatChannel` (`^[a-z0-9][a-z0-9_-]{0,40}$`) before interpolation, to prevent open-redirect / URL-injection on the Slack workspace URL. +`channel` is validated against the same regex as `Project.chatChannel` (`^[a-z0-9][a-z0-9_-]{0,40}$`) before interpolation, to prevent open-redirect / URL-injection on the Slack workspace URL. The host is always `SLACK_TEAM_HOST`. Use **302** (temporary) rather than 301 so we can change the destination later without browser-cached redirects sticking. ### Why this exists -- Marketing materials and old links say "join us at codeforphilly.org/chat" — historical, do-not-break. +- Marketing materials and old links say "join us at codeforphilly.org/chat" and deep-link `codeforphilly.org/chat/` — historical, do-not-break. - Project pages use `/chat?channel=` for the "Chat Channel" button so the link looks like part of the site rather than an external Slack URL. - If we move off Slack later, every link gets re-pointed by changing this one redirect rather than chasing references through the codebase. ## Open redirect protection -`channel` only feeds the path segment after `/channels/`; the host is hard-coded. No user input touches the host. +`channel` only feeds the `redir` value's path segment after `/messages/`; the host is hard-coded. No user input touches the host. ## Authorization -Public. +Public. The SSO round-trip that follows requires a Code for Philly sign-in, and Slack's own workspace membership rules still apply. From ebc163eea987a06435cd889b1f2bd4616b6551fb Mon Sep 17 00:00:00 2001 From: Chris Alfano Date: Fri, 18 Sep 2026 02:16:56 -0400 Subject: [PATCH 2/4] feat(api): /chat and /chat/ sign into Slack via SSO The rewrite's /chat pointed at https://.slack.com/channels/, which assumes an existing Slack session, and /chat/ fell through to the SPA's 404. laddr's Emergence\Slack\Connector redirected to Slack's SSO start with redir=/messages//; Slack then drives the SP-initiated flow against our IdP and opens the channel. Restore that, accept the path form (with and without a trailing slash), and default to general. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01LFyA5poHwrhAktrnsKrUiQ --- apps/api/src/routes/chat.ts | 113 ++++++++++++++++----------- apps/api/tests/chat-redirect.test.ts | 91 +++++++++++++-------- 2 files changed, 123 insertions(+), 81 deletions(-) diff --git a/apps/api/src/routes/chat.ts b/apps/api/src/routes/chat.ts index 3e2fcfe..731378a 100644 --- a/apps/api/src/routes/chat.ts +++ b/apps/api/src/routes/chat.ts @@ -1,10 +1,16 @@ /** - * Chat redirect. + * Chat launch. * - * GET /chat → 302 to https:/// - * GET /chat?channel= → 302 to https:///channels/ - * GET /chat?channel= → fall back to workspace root - * GET /chat?channel= → fall back to root + warn log + * GET /chat → 302 to Slack SSO start, redir=/messages/general/ + * GET /chat/ → 302 to Slack SSO start, redir=/messages// + * GET /chat?channel= → same as the path form + * GET /chat?channel= or invalid → fall back to general (+ warn log for invalid) + * + * The target is Slack's SP-initiated SSO start URL, not the workspace: Slack + * sends an AuthnRequest to our IdP (/api/saml/slack/sso), the member signs in + * here if needed, and Slack honours `redir` to open the channel. This mirrors + * laddr's Emergence\Slack\Connector::handleLaunchRequest, which the + * codeforphilly.org/chat/ links in the wild were built for. * * 302 (temporary) so the destination can flip later without browser caches * sticking. Channel format matches Project.chatChannel @@ -13,58 +19,71 @@ * * Per specs/screens/chat.md. */ -import type { FastifyInstance } from 'fastify'; +import type { FastifyInstance, FastifyReply } from 'fastify'; const CHANNEL_REGEX = /^[a-z0-9][a-z0-9_-]{0,40}$/; +const DEFAULT_CHANNEL = 'general'; + +export function slackSsoStartUrl(slackHost: string, channel: string): string { + return `https://${slackHost}/sso/saml/start?redir=${encodeURIComponent(`/messages/${channel}/`)}`; +} export async function chatRoutes(fastify: FastifyInstance): Promise { + const launch = (reply: FastifyReply, requested: string | null | undefined): FastifyReply => { + const slackHost = fastify.config.SLACK_TEAM_HOST; + // Empty string is spec'd to behave like no channel — fall back to general. + let channel = requested && requested.length > 0 ? requested : DEFAULT_CHANNEL; + if (!CHANNEL_REGEX.test(channel)) { + fastify.log.warn( + // The encoded value keeps log-injection-style payloads benign. + { channel: encodeURIComponent(channel) }, + 'chat launch: invalid channel format; falling back to general', + ); + channel = DEFAULT_CHANNEL; + } + return reply + .code(302) + .header('Location', slackSsoStartUrl(slackHost, channel)) + .header('Cache-Control', 'no-cache') + .send(); + }; + + const querySchema = { + type: 'object', + properties: { channel: { type: 'string' } }, + additionalProperties: false, + }; + fastify.get( '/chat', { schema: { tags: ['chat'], - summary: 'Redirect to the Code for Philly Slack workspace', - querystring: { - type: 'object', - properties: { channel: { type: 'string' } }, - additionalProperties: false, - }, + summary: 'Sign into the Code for Philly Slack workspace via SSO', + querystring: querySchema, }, }, - async (request, reply) => { - const slackHost = fastify.config.SLACK_TEAM_HOST; - const root = `https://${slackHost}/`; - - const raw = (request.query as { channel?: string }).channel; - // Empty string is spec'd to behave like no channel — fall back to root. - const channel = raw && raw.length > 0 ? raw : null; - - if (channel === null) { - return reply - .code(302) - .header('Location', root) - .header('Cache-Control', 'no-cache') - .send(); - } - - if (!CHANNEL_REGEX.test(channel)) { - fastify.log.warn( - // The encoded value keeps log-injection-style payloads benign. - { channel: encodeURIComponent(channel) }, - 'chat redirect: invalid channel format; falling back to root', - ); - return reply - .code(302) - .header('Location', root) - .header('Cache-Control', 'no-cache') - .send(); - } - - return reply - .code(302) - .header('Location', `https://${slackHost}/channels/${channel}`) - .header('Cache-Control', 'no-cache') - .send(); - }, + async (request, reply) => launch(reply, (request.query as { channel?: string }).channel), ); + + // The app does not ignore trailing slashes globally, and links in the wild + // come in both shapes, so register both. + for (const path of ['/chat/:channel', '/chat/:channel/']) { + fastify.get( + path, + { + schema: { + tags: ['chat'], + summary: 'Sign into the Code for Philly Slack workspace via SSO and open a channel', + params: { + type: 'object', + properties: { channel: { type: 'string' } }, + required: ['channel'], + }, + querystring: querySchema, + }, + }, + async (request, reply) => launch(reply, (request.params as { channel: string }).channel), + ); + } } diff --git a/apps/api/tests/chat-redirect.test.ts b/apps/api/tests/chat-redirect.test.ts index 9701d62..d2564ff 100644 --- a/apps/api/tests/chat-redirect.test.ts +++ b/apps/api/tests/chat-redirect.test.ts @@ -1,15 +1,21 @@ /** - * Tests for GET /chat — Slack-workspace redirect per specs/screens/chat.md. + * Tests for GET /chat and /chat/ — Slack SSO launch per + * specs/screens/chat.md. Every response is a 302 to Slack's SP-initiated + * SSO start URL carrying `redir=/messages//`. */ import { afterAll, beforeAll, describe, expect, it } from 'vitest'; import type { FastifyInstance } from 'fastify'; import { buildApp } from '../src/app.js'; +import { slackSsoStartUrl } from '../src/routes/chat.js'; import { createFullDataRepo, createPrivateStorageDir } from './helpers/test-full-repo.js'; let dataRepo: { path: string; cleanup: () => Promise }; let privateStore: { path: string; cleanup: () => Promise }; let app: FastifyInstance; +const HOST = 'codeforphilly.slack.com'; +const start = (channel: string): string => slackSsoStartUrl(HOST, channel); + beforeAll(async () => { dataRepo = await createFullDataRepo(); privateStore = await createPrivateStorageDir(); @@ -20,6 +26,7 @@ beforeAll(async () => { STORAGE_BACKEND: 'filesystem', CFP_PRIVATE_STORAGE_PATH: privateStore.path, CFP_JWT_SIGNING_KEY: 'test-jwt-signing-key-at-least-32-chars!!', + SLACK_TEAM_HOST: HOST, NODE_ENV: 'test', }, }); @@ -31,56 +38,53 @@ afterAll(async () => { await privateStore.cleanup(); }); +async function launch(url: string): Promise { + const res = await app.inject({ method: 'GET', url }); + expect(res.statusCode).toBe(302); + expect(res.headers['cache-control']).toBe('no-cache'); + return String(res.headers.location); +} + +describe('slackSsoStartUrl', () => { + it('targets Slack SSO start with an encoded /messages// redir', () => { + expect(start('general')).toBe( + 'https://codeforphilly.slack.com/sso/saml/start?redir=%2Fmessages%2Fgeneral%2F', + ); + }); +}); + describe('GET /chat', () => { - it('redirects to the Slack workspace root when no channel is given', async () => { - const res = await app.inject({ method: 'GET', url: '/chat' }); - expect(res.statusCode).toBe(302); - expect(res.headers.location).toBe('https://codeforphilly.slack.com/'); - expect(res.headers['cache-control']).toContain('no-cache'); + it('launches into #general when no channel is given', async () => { + expect(await launch('/chat')).toBe(start('general')); }); - it('deep-links to a valid channel', async () => { - const res = await app.inject({ method: 'GET', url: '/chat?channel=general' }); - expect(res.statusCode).toBe(302); - expect(res.headers.location).toBe('https://codeforphilly.slack.com/channels/general'); + it('deep-links a valid ?channel=', async () => { + expect(await launch('/chat?channel=phlask')).toBe(start('phlask')); }); it('accepts hyphens and underscores in the channel name', async () => { - const res = await app.inject({ method: 'GET', url: '/chat?channel=philly_civic-tech' }); - expect(res.statusCode).toBe(302); - expect(res.headers.location).toBe('https://codeforphilly.slack.com/channels/philly_civic-tech'); + expect(await launch('/chat?channel=philly_civic-tech')).toBe(start('philly_civic-tech')); }); - it('falls back to root for an empty channel', async () => { - const res = await app.inject({ method: 'GET', url: '/chat?channel=' }); - expect(res.statusCode).toBe(302); - expect(res.headers.location).toBe('https://codeforphilly.slack.com/'); + it('falls back to #general for an empty channel', async () => { + expect(await launch('/chat?channel=')).toBe(start('general')); }); - it('falls back to root for uppercase characters (invalid format)', async () => { - const res = await app.inject({ method: 'GET', url: '/chat?channel=General' }); - expect(res.statusCode).toBe(302); - expect(res.headers.location).toBe('https://codeforphilly.slack.com/'); + it('falls back to #general for uppercase characters (invalid format)', async () => { + expect(await launch('/chat?channel=General')).toBe(start('general')); }); - it('falls back to root for slashes (path-injection attempt)', async () => { - const res = await app.inject({ method: 'GET', url: '/chat?channel=foo%2Fbar' }); - expect(res.statusCode).toBe(302); - expect(res.headers.location).toBe('https://codeforphilly.slack.com/'); + it('falls back to #general for slashes (path-injection attempt)', async () => { + expect(await launch('/chat?channel=foo%2Fbar')).toBe(start('general')); }); - it('falls back to root for an over-long channel name', async () => { - // 42 chars total (> 41 max per the regex) + it('falls back to #general for an over-long channel name', async () => { const channel = 'a'.repeat(42); - const res = await app.inject({ method: 'GET', url: `/chat?channel=${channel}` }); - expect(res.statusCode).toBe(302); - expect(res.headers.location).toBe('https://codeforphilly.slack.com/'); + expect(await launch(`/chat?channel=${channel}`)).toBe(start('general')); }); - it('falls back to root for leading hyphen (invalid first char)', async () => { - const res = await app.inject({ method: 'GET', url: '/chat?channel=-leading-hyphen' }); - expect(res.statusCode).toBe(302); - expect(res.headers.location).toBe('https://codeforphilly.slack.com/'); + it('falls back to #general for a leading hyphen (invalid first char)', async () => { + expect(await launch('/chat?channel=-leading-hyphen')).toBe(start('general')); }); it('does not register on /api/chat (only /chat)', async () => { @@ -88,3 +92,22 @@ describe('GET /chat', () => { expect(res.statusCode).toBe(404); }); }); + +describe('GET /chat/', () => { + it('deep-links the path form — the shape of the links in the wild', async () => { + expect(await launch('/chat/phlask')).toBe(start('phlask')); + }); + + it('tolerates a trailing slash', async () => { + expect(await launch('/chat/phlask/')).toBe(start('phlask')); + }); + + it('falls back to #general for an invalid path segment', async () => { + expect(await launch('/chat/Not%20A%20Channel')).toBe(start('general')); + }); + + it('never lets the channel reach the host', async () => { + const location = await launch('/chat/evil.example'); + expect(new URL(location).host).toBe(HOST); + }); +}); From 98c05b558d9e2f98afcdc50b8408f1e9a738d104 Mon Sep 17 00:00:00 2001 From: Chris Alfano Date: Fri, 18 Sep 2026 02:16:56 -0400 Subject: [PATCH 3/4] chore(plans): add chat-sso-launch Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01LFyA5poHwrhAktrnsKrUiQ --- plans/chat-sso-launch.md | 51 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 plans/chat-sso-launch.md diff --git a/plans/chat-sso-launch.md b/plans/chat-sso-launch.md new file mode 100644 index 0000000..fc2ff4a --- /dev/null +++ b/plans/chat-sso-launch.md @@ -0,0 +1,51 @@ +--- +status: in-progress +depends: [samlify-esm-interop] +specs: + - specs/screens/chat.md +issues: [] +pr: null +--- + +# Plan: `/chat/` signs into Slack and opens the channel + +## Scope + +Years of distributed links point at `codeforphilly.org/chat/`. On +laddr that URL signed the member into Slack via SAML and landed them in the +channel. The rewrite's `/chat` only accepted `?channel=` and redirected to +`https://.slack.com/channels/` — which assumes an existing Slack +session — and `/chat/` fell through to the SPA's 404. + +In: spec the path form and the SSO-start target; implement; tests. Out: +IdP-initiated launch (`/api/saml/slack/launch` stays as-is); any change to +the project "Chat Channel" button, which already uses `?channel=`. + +## Implements + +- [screens/chat.md](../specs/screens/chat.md) — redirect rules: `/chat`, + `/chat/`, `?channel=`; target is Slack's SP-initiated SSO start + with `redir=/messages//`; default channel `general`. + +## Approach + +- Match laddr exactly: `Emergence\Slack\Connector::handleLaunchRequest` + redirected to `https://.slack.com/sso/saml/start?redir=/messages//`. + Slack then drives the SP-initiated flow against `/api/saml/slack/sso`, + verified live on 2026-09-18. +- `apps/api/src/routes/chat.ts`: one `launch()` helper behind `/chat` and + `/chat/:channel`; invalid or empty channel → `general` (warn log on + invalid). Fastify's default `ignoreTrailingSlash` handling covers + `/chat/foo/`. +- Tests: rewrite `chat-redirect.test.ts` expectations to the SSO-start URL + and add the path form + trailing slash + default channel. + +## Validation + +- `npm run type-check && npm run lint`; chat suite green. +- Live: `/chat/general` from a signed-out browser → CfP login → Slack opens + in #general. + +## Follow-ups + +None. From 27e30062eac73490e397f4a541f85099720dae5c Mon Sep 17 00:00:00 2001 From: Chris Alfano Date: Fri, 18 Sep 2026 02:17:00 -0400 Subject: [PATCH 4/4] chore(plans): mark chat-sso-launch done (PR #185) Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01LFyA5poHwrhAktrnsKrUiQ --- plans/chat-sso-launch.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plans/chat-sso-launch.md b/plans/chat-sso-launch.md index fc2ff4a..2faa16a 100644 --- a/plans/chat-sso-launch.md +++ b/plans/chat-sso-launch.md @@ -1,10 +1,10 @@ --- -status: in-progress +status: done depends: [samlify-esm-interop] specs: - specs/screens/chat.md issues: [] -pr: null +pr: 185 --- # Plan: `/chat/` signs into Slack and opens the channel