| status | done | ||
|---|---|---|---|
| depends |
|
||
| specs |
|
||
| issues | |||
| pr | 161 |
Fix the SAML IdP so that the metadata entityID, the assertion Issuer, and
the SingleSignOnService endpoint URLs are built from our own settings rather
than from SLACK_TEAM_HOST. Today the live metadata at
https://next.codeforphilly.org/api/saml/slack/metadata advertises
entityID="https://codeforphilly.slack.com/api/saml/slack/metadata" and
Location="https://codeforphilly.slack.com/api/saml/slack/sso" — both on
Slack's host. This closes the "entityID host source" follow-up left open by
saml-idp.
In scope:
- A new optional
SAML_ENTITY_IDenv var (defaulthttps://codeforphilly.org/api/saml/slack/metadata) that is the single source for both the metadataentityIDand every assertion<Issuer>. - SSO endpoint
Locations built fromCFP_SITE_HOST. SLACK_TEAM_HOSTretains exactly its Slack-side roles: ACS URL, NameIDNameQualifier,/launchredirect target.- Spec + operator docs +
.env.exampledescribing the three-way split. - Tests covering the default entity ID, the
CFP_SITE_HOST-driven endpoint URL, and the entity ID staying put whenCFP_SITE_HOSTchanges.
Out of scope: re-registering the IdP with Slack (operator action, not code); any change to NameID or the attribute set.
- api/saml.md — the new "IdP identity and hosts"
section:
entityID=SAML_ENTITY_ID;SingleSignOnService/@Location=https://<CFP_SITE_HOST>/api/saml/slack/sso;Issuer=entityID. - architecture.md — env table rows for
SAML_ENTITY_ID,SLACK_TEAM_HOST,CFP_SITE_HOST.
- Spec first. Add the "IdP identity and hosts" section to
specs/api/saml.mdstating the three sources and the stability rule for the entity ID; addSAML_ENTITY_IDto the env tables in the spec,specs/architecture.md,docs/operations/deploy.md,docs/operations/secrets.md, and.env.example. - Env. Add
SAML_ENTITY_IDtoEnvSchema(zod) and the mirrored JSON schema inapps/api/src/env.ts, defaulting tohttps://codeforphilly.org/api/saml/slack/metadata. - Route. In
getSamlContext(apps/api/src/routes/saml.ts) build theSamlIdpSettingsas:entityId: cfg.SAML_ENTITY_IDssoLoginPostUrl/ssoLoginRedirectUrl:https://${cfg.CFP_SITE_HOST}/api/saml/slack/ssoslackTeamHost: cfg.SLACK_TEAM_HOST(unchanged) Drop theissuerHostderivation and its misleading comment.
- Config.
apps/api/src/saml/config.tsalready threadssettings.entityIdinto both the samlifyIdentityProvider({ entityID })(→ metadata) andSlackSamlEntities.entityId, which the route passes asissuerEntityIdintobuildResponseSubstitutions(→{Issuer}on both the Response and the Assertion). No change needed there beyond doc comments; verify rather than assume. - Tests. Update
apps/api/tests/saml.test.ts: metadataentityIDequals the defaultSAML_ENTITY_ID; SSOLocations useCFP_SITE_HOST; assertionIssuer(both Response and Assertion) equals the entity ID; a second app booted withCFP_SITE_HOST=next.example.orggetsLocationon that host whileentityIDstays the default; an explicitSAML_ENTITY_IDoverride flows through to both metadata andIssuer.
-
GET /api/saml/slack/metadataentityIDishttps://codeforphilly.org/api/saml/slack/metadatawith noSAML_ENTITY_IDset, regardless ofSLACK_TEAM_HOST/CFP_SITE_HOST. - Both
SingleSignOnService/@Locationvalues arehttps://<CFP_SITE_HOST>/api/saml/slack/sso; withCFP_SITE_HOST=next.example.orgthey use that host. -
<saml:Issuer>on the Response and the Assertion equal the metadataentityID, including whenSAML_ENTITY_IDis overridden. - ACS URL, form action,
NameQualifier, and/launchredirect still useSLACK_TEAM_HOST(existing tests keep passing). -
SLACK_TEAM_HOSTno longer appears in any IdP-side URL (grep the route). -
npm run type-check && npm run lint && npm testclean.
- Slack already has the wrong entity ID on file. If the
next.deployment's metadata was uploaded to Slack, Slack storedhttps://codeforphilly.slack.com/api/saml/slack/metadataas the IdP issuer. After this ships, assertions carry the correct issuer and Slack will reject them until its SAML config is re-synced from the metadata URL. Operator coordination, noted in Follow-ups. - Cutover host flip.
Locationchanges fromnext.codeforphilly.orgtocodeforphilly.orgat cutover; Slack's IdP config needs a metadata refresh then too (endpoint URLs only — the entity ID is untouched, so the trust relationship survives).
config.tsneeded no logic change.SamlIdpSettings.entityIdalready fed both samlify'sIdentityProvider({ entityID })(→ metadata) andSlackSamlEntities.entityId, which the route passes asissuerEntityIdintobuildResponseSubstitutions(→{Issuer}on Response + Assertion). The bug was entirely ingetSamlContext's choice of host. Only doc comments changed there.- Entity ID is a URI by convention, not a fetchable URL. Nothing (Slack
included) dereferences it, which is why it can stay on
codeforphilly.orgwhile the deployment answers onnext.codeforphilly.org.SAML_ENTITY_IDis validated as a URL (z.url()) only to catch typos. SLACK_TEAM_HOSTwas never in the deploy env table — it rode on its default. Added a row so operators see it next toSAML_ENTITY_IDand don't confuse the two hosts again.- Web-suite timeout flake.
apps/web/tests/ProjectEdit.test.tsxhit its 5s limit (5252ms) once under full-suite load and passed in isolation. Unrelated to this change; noted in case it recurs.
- Tracked as: operator action before deploying to
next.codeforphilly.org— if Slack's SAML config was seeded from the old metadata it holds the wrong issuer (https://codeforphilly.slack.com/...) and will reject assertions until re-synced from the metadata URL. Same metadata refresh (endpoint URLs only) is needed again at cutover whenCFP_SITE_HOSTflips; seedocs/operations/cutover.mdowners. - Tracked as: boot-time
slackSamlNameIdinvariant scan — carried forward unchanged fromsaml-idp; not touched here.