feat: SAML 2.0 en el gateway para tenants legacy (S-3)#21
Open
dherrero wants to merge 10 commits into
Open
Conversation
…g types (T-33)
- Add strict SsoProtocol union ('oidc' | 'saml') and optional protocol
field to SsoProviderPublicDTO (backward-compatible, defaults to oidc)
- Document that resolveFederatedUserSchema covers both OIDC sub and
SAML persistent NameID (255-char column-bound ceiling, fail-closed)
- Add secret-bearing SamlProviderConfig type in the gateway (never
exported via @dto): multi-cert rotation, literal-true
wantAssertionsSigned, sha1 excluded by type design
Co-Authored-By: Claude Fable 5 <[email protected]>
…stry (T-34) - saml-provider-registry.ts: SAML_<NAME>_* env parsing mirroring the OIDC pattern — fail-fast validation, x509 cert checks (expired -> throw, <2048-bit RSA -> throw, <30-day expiry warn), multi-cert rotation via ';', sha256/sha512 only, deterministic SAML<->OIDC id collision check - federated-registry.ts: unified protocol-tagged public provider list (stable id order) and discriminated getFederatedProvider(id) lookup - Reuse (not duplicate) the OIDC SSRF guard and permission-map parser; harden the guard into two tiers: link-local/cloud-metadata and 0.0.0.0 are blocked unconditionally (even with SSO_ALLOW_INSECURE_ISSUERS), loopback/RFC1918 only allowed behind the explicit dev escape hatch - validateSamlConfig() wired into gateway boot next to validateSsoConfig() - 25 new Vitest cases incl. SSRF tiers, cert fixtures and registry mixing Co-Authored-By: Claude Fable 5 <[email protected]>
- saml-client.ts: hardened @node-saml/[email protected] factory — both Response and Assertion signatures required, validateInResponseTo always (IdP-initiated rejected by design), audience pinned to the SP entityID, idpIssuer pinned, 30s clock skew, 10min max assertion age, CSPRNG AuthnRequest ids injected via generateUniqueId, per-provider bounded request-id cache (10k FIFO + TTL, memory-DoS safe) - saml-transaction.service.ts: signed single-use transaction cookie {provider, requestId, returnTo}; SameSite=None+Secure in production (the ACS is a cross-site POST), typ-separated from the OIDC cookie - saml.controller.ts: SP-initiated login (empty RelayState — returnTo only ever travels in the signed cookie) + public SP metadata endpoint (no private material, application/samlmetadata+xml) - sso.controller.ts login now dispatches by protocol via the federated registry; OIDC flow untouched - Registry: optional SAML_<NAME>_FORCE_AUTHN and _DISABLE_REQUESTED_AUTHN_CONTEXT (legacy IdP compat, default omit) - 26 new tests incl. a real deflate+base64 AuthnRequest round-trip Co-Authored-By: Claude Fable 5 <[email protected]>
…suance (T-36)
- POST /api/v1/auth/sso/:provider/callback with route-scoped urlencoded
parser (256kb cap); CSRF protection via the signed single-use
transaction cookie + InResponseTo binding (the route is necessarily
exempt from cookie CSRF)
- Fail-closed chain: transaction cookie read+cleared unconditionally and
bound to the initiating provider (IdP-initiated rejected by design);
node-saml verifies Response+Assertion signatures against pinned certs,
Status, Conditions/skew, AudienceRestriction and idpIssuer; the
controller re-checks InResponseTo against the cookie's request id
- Stable-identifier policy: NameID allowlist persistent/emailAddress
(transient and unspecified rejected); plausible-email gate with
hostile-char rejection; per-provider email domain allowlist
(cross-tenant containment); emailVerified:true documented policy
- Same session shape as local/OIDC logins via resolveFederatedUser +
respondWithTokens; signed SLO hint {provider,nameId,sessionIndex}
cookie (saml-logout.service.ts) set only after full success
- Errors: fixed /login?sso_error=1 redirect, sanitised internal log
(control chars stripped, truncated), never reflects IdP data
- 12 new ACS tests covering every rejection path
Co-Authored-By: Claude Fable 5 <[email protected]>
…e refresh family (T-37) - /api/v1/auth/sso/logout now dispatches by the protocol of the signed logout hint: the refresh family is ALWAYS revoked and cookies cleared before any IdP involvement — a down IdP can never keep a session alive - sloRedirectUrl builds the HTTP-Redirect LogoutRequest from the signed saml_logout hint (NameID + SessionIndex); RelayState carries only a safeReturnTo-vetted local path - GET|POST /:provider/logout/callback validates the LogoutResponse when possible and ignores failures silently — no state change is reachable from that endpoint; landing path re-vetted with safeReturnTo - Stale dual-hint edge: OIDC end_session takes precedence, both hint cookies always cleared (tested) - Documented decisions for T-40: IdP-initiated SLO out of scope (residual risk mirror of OIDC back-channel logout); LogoutRequest unsigned (no SP private-key management in the starter) Co-Authored-By: Claude Fable 5 <[email protected]>
- Optional accessible badge (visually-hidden, i18n en/es/ca) for SAML providers without an iconKey; auto-hides if an icon is configured - protocol? consumed via Angular interpolation only — XSS test proves a hostile displayName renders as escaped text - sso.service/login specs extended for mixed OIDC+SAML lists; spec setup migrated to TranslocoTestingModule so template-rendering tests work Co-Authored-By: Claude Fable 5 <[email protected]>
…dening (T-39) e2e (apps/gateway/src/sso/saml-flow.e2e.spec.ts): full SP handshake against a throwaway self-signed test IdP that signs real Responses/Assertions with xml-crypto — happy path + 14 attacks, every one ending in rejection + /login?sso_error=1 with no session. Beyond the 11 in the spec: signed-response/ unsigned-assertion, rogue-key signature, XSW, NameID comment injection, replay, IdP-initiated (no tx cookie), InResponseTo mismatch, audience for another SP, expired assertion, mix-up issuer, domain outside allowlist, RelayState external URL never followed, transient NameID rejected. ApiClient mocked; no network. Security gate finding (HIGH, account takeover) closed by construction: the ACS stamps emailVerified:true (SAML carries no per-assertion verified signal) and the API auto-links a federated identity to any pre-existing local user with a matching email — so the email-domain allowlist is the sole cross-tenant takeover boundary. It was optional; now it is mandatory: - saml-types.ts: allowedDomains is required (string[]) - saml-provider-registry.ts: fail-fast at boot when SAML_<NAME>_ALLOWED_DOMAINS is absent or empty - saml.controller.ts: the domain check runs unconditionally before the emailVerified:true stamp Also pins the response Issuer to the configured idpIssuer in the ACS (node-saml only enforces idpIssuer on logout messages, not the login Response). test:gateway 139/139, test:e2e 18/18, build + lint green. Co-Authored-By: Claude Fable 5 <[email protected]>
…NTS, roadmap (T-40) - docs/SECURITY.md: new "Federación SAML 2.0 (tenants legacy)" section mirroring the OIDC one — SP-initiated flow diagram, full threat model with per-vector decisions (only SP-initiated/InResponseTo, signature on Response+Assertion, anti-XSW/XXE, persistent NameID only, mandatory _ALLOWED_DOMAINS as the sole cross-tenant takeover boundary, RelayState never used for the final redirect, SSRF guard, cert hygiene, best-effort SLO with refresh-family revoked first, IdP-initiated SLO out of scope with residual risk), and a legacy-IdP onboarding guide (SP metadata, exact ACS, cert rotation via multi-cert). - .env.example: commented SAML_<NAME>_* block; _ALLOWED_DOMAINS flagged REQUIRED; _IDP_CERT noted as the IdP's public cert; _DECRYPTION_PVK as a gateway-only secret. - apps/gateway/AGENTS.md: SAML SP responsibility, new routes (ACS POST, metadata, SLO callback), env table row, the /internal/federated/resolve reuse rule, the "do not relax wantAssertionsSigned/validateInResponseTo" and mandatory-allowlist invariants, and the e2e attack-suite description. - README.md + docs/README_eng.md: roadmap item "SAML for legacy tenants" marked [x] linking the new SECURITY.md section. Co-Authored-By: Claude Fable 5 <[email protected]>
npm ci failed in CI with EUSAGE (Missing: @rspack/[email protected]) because the lockfile regenerated while adding @node-saml/node-saml had silently dropped the peer-installed @rspack/[email protected] subtree (npm 11.11.0 prunes peer:true entries on regeneration, yet its own npm ci requires them). Rebuilt the lockfile as main's version plus only the node-saml entries, added a verify:lock script (npm ci --dry-run) to validate lock/manifest sync like CI does, and documented the lockfile convention in AGENTS.md. Co-Authored-By: Claude Fable 5 <[email protected]>
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.
S-3 — SAML 2.0 en el gateway para tenants legacy
El gateway pasa a actuar como Service Provider SAML 2.0 (solo SP-initiated) para IdPs que no hablan OIDC (ADFS, Shibboleth, Okta-SAML, Azure AD SAML). La identidad federada termina en la misma sesión local de siempre (access JWT + cookie refresh con familia/rotación); la API sigue confiando solo en el JWT interno EdDSA y nunca habla con el IdP. Sin proveedores
SAML_*configurados, SAML está desactivado y nada cambia (regresión cero).Núcleo criptográfico:
@node-saml/[email protected](con[email protected]/@xmldom/[email protected]), endurecido y sin opciones relajables por env.Tareas (8 commits, una por ticket)
protocol: 'oidc' | 'saml'enSsoProviderPublicDTO+ tipos internos de config SAMLSAML_<NAME>_*+ registry federado multi-protocolo (fail-fast, certs, SSRF, colisión de ids).env.example, AGENTS gateway, roadmapSeguridad
Modelo de amenazas con veredicto por vector en
docs/SECURITY.md → Federación SAML 2.0. Cubiertos y testeados: XSW, XXE, firma parcial (Response vs Assertion), comment injection en NameID (clase CVE-2017-11427), replay, IdP-initiated bloqueado, AudienceRestriction, mix-up multi-IdP, NameID transient rechazado, trust bypass por cert embebido, escalada por atributo de grupos, open redirect (returnTo/RelayState), SSRF en entryPoint/logoutUrl, fuga de certs/clave, sesión huérfana tras logout, DoS por payload XML.Hallazgo HIGH detectado por
/security-reviewy cerrado por construcción: el ACS sellaemailVerified: true(SAML no tiene señal por-aserción) y la API auto-vincula una identidad federada a una cuenta local con email coincidente → account takeover. El cerco_ALLOWED_DOMAINSera opcional; ahora es obligatorio (fail-fast al arranque + chequeo de dominio incondicional antes del sello). Verificado por el ataque e2e #12.Riesgo residual aceptado y documentado: SLO IdP-initiated fuera de alcance (espejo del back-channel logout OIDC);
LogoutRequestSP→IdP sin firmar (el starter no gestiona clave privada del SP).Verificación
npm run build(front + gateway + api) ✅npm run test:gateway139/139 ·npm run test:e2e18/18 · suite OIDC y login local intactosnpm auditen vivo no pudo correr en el entorno (sin red) — pendiente de ejecutar en CI.🤖 Generated with Claude Code