fix(api): build SAML entityID and SSO endpoints from our own host, not Slack's - #161
Merged
Conversation
themightychris
added a commit
that referenced
this pull request
Sep 9, 2026
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RdRwHvDupRLV8GuJpYKzEr
The live IdP metadata advertises entityID and SingleSignOnService Locations on the Slack team host because the route derived both from SLACK_TEAM_HOST. Spec now separates the three values by source: the entity ID (metadata entityID + assertion Issuer) is a stable logical identifier from a new optional SAML_ENTITY_ID env var, defaulting to https://codeforphilly.org/api/saml/slack/metadata and deliberately independent of CFP_SITE_HOST so the pre-/post-cutover host flip doesn't invalidate the issuer Slack stored at setup; endpoint Locations follow CFP_SITE_HOST; SLACK_TEAM_HOST keeps only its Slack-side roles. Env tables in architecture.md, deploy.md, secrets.md and .env.example gain the new var. Adds plans/saml-self-host.md (in-progress) to carry the code change, closing the follow-up left open by plans/saml-idp.md. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RdRwHvDupRLV8GuJpYKzEr
getSamlContext derived the IdP entity ID and both SingleSignOnService Locations from SLACK_TEAM_HOST, so the live metadata advertised https://codeforphilly.slack.com/api/saml/slack/... — Slack's host, not ours. Per specs/api/saml.md#idp-identity-and-hosts: - entityId (metadata entityID + assertion Issuer, one value flowing through SlackSamlEntities.entityId → issuerEntityId) now comes from the new SAML_ENTITY_ID env var, defaulting to https://codeforphilly.org/api/saml/slack/metadata. It is a stable logical identifier and intentionally does not follow CFP_SITE_HOST, so the next.codeforphilly.org → codeforphilly.org flip at cutover leaves the issuer Slack stored at setup untouched. - ssoLoginPostUrl / ssoLoginRedirectUrl are built on CFP_SITE_HOST so the metadata points Slack at the host actually serving the API. - SLACK_TEAM_HOST keeps only its Slack-side roles (ACS URL, NameID NameQualifier, launch redirect). Tests assert the default entityID, the CFP_SITE_HOST-driven Locations, Issuer == entityID on both Response and Assertion, that CFP_SITE_HOST=next.example.org moves the Locations without moving the entityID, and that an explicit SAML_ENTITY_ID flows to both. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RdRwHvDupRLV8GuJpYKzEr
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RdRwHvDupRLV8GuJpYKzEr
themightychris
force-pushed
the
fix/saml-self-host
branch
from
September 9, 2026 00:14
24e276d to
b8a3fe1
Compare
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The SAML IdP metadata at
https://next.codeforphilly.org/api/saml/slack/metadataadvertisedbecause
getSamlContextbuilt both fromSLACK_TEAM_HOST— Slack's host, not ours.plans/saml-idp.mdrecorded this as a known follow-up;specs/api/saml.mdalready said the entityID is our own URL.Change
Spec first (
specs/api/saml.mdgains an "IdP identity and hosts" section), then code:SAML_ENTITY_ID(new, optional; defaulthttps://codeforphilly.org/api/saml/slack/metadata)entityID;<saml:Issuer>on the Response and the Assertionhttps://<CFP_SITE_HOST>/api/saml/slack/ssoSingleSignOnService/@Location(POST + Redirect)SLACK_TEAM_HOST(unchanged)Destination/Recipient, NameIDNameQualifier,/launch+/chatredirectsSAML_ENTITY_IDis deliberately independent ofCFP_SITE_HOST: Slack stores the issuer at setup time, so the pre-cutovernext.codeforphilly.orgdeploy and the post-cutovercodeforphilly.orgdeploy must present the same identifier. Leave it unset everywhere Slack should trust the production IdP identity; only set it when registering a separate IdP (e.g. a sandbox against a test workspace). The endpointLocations do followCFP_SITE_HOST, so at cutover Slack's IdP config needs a metadata refresh for the URLs — but the trust relationship is untouched.The entity ID and the assertion Issuer flow from the same
SamlIdpSettings.entityId(buildSlackSamlEntities→ samlifyIdentityProvider({ entityID })for metadata;SlackSamlEntities.entityId→issuerEntityId→{Issuer}in the response template). Verified rather than assumed;config.tsonly gained doc comments.Files
specs/api/saml.md,specs/architecture.md— the rules + env table rowsdocs/operations/deploy.md,docs/operations/secrets.md,.env.example— operator-facing description ofSAML_ENTITY_IDandSLACK_TEAM_HOSTapps/api/src/env.ts—SAML_ENTITY_ID(zod + JSON schema), doc comments onSLACK_TEAM_HOST/CFP_SITE_HOSTapps/api/src/routes/saml.ts—getSamlContextbuilds fromSAML_ENTITY_ID+CFP_SITE_HOSTapps/api/src/saml/config.ts— doc comments onlyapps/api/tests/saml.test.ts— updated entityID expectation; new assertions for Locations, Issuer; new describe coveringCFP_SITE_HOST=next.example.org(Locations move, entityID stays) and an explicitSAML_ENTITY_IDoverride (metadata + Issuer both follow)plans/saml-self-host.md— plan, closed out in the last commitValidation
npm run type-check+npm run lintcleannpm test: api 34 files / 429 tests pass (saml.test.ts 11/11). One unrelated web test (ProjectEdit.test.tsx) hit its 5s timeout under full-suite load and passes in isolation.Operator note
If the
next.deployment's metadata was already uploaded to Slack, Slack has the wrong issuer on file and will reject assertions after this ships until its SAML config is re-synced from the metadata URL. Coordinate with the workspace admin before deploying.🤖 Generated with Claude Code
https://claude.ai/code/session_01RdRwHvDupRLV8GuJpYKzEr