feat(auth): SAML 2.0 SSO service provider (enterprise, XSW-hardened) - #76
Merged
Conversation
Implements SAML 2.0 SP (Service Provider) for enterprise SSO alongside OIDC, completing the SSO story with hardened assertion validation via pysaml2, JIT provisioning, and license-gated admin controls. CRITICAL SECURITY FIX: Uses pysaml2's AuthnResponse for signature verification instead of hand-rolled XML parsing. pysaml2's built-in validation prevents XML Signature Wrapping (XSW) attacks by correctly validating DigestValue against assertion content and binding Reference/@uri to the assertion ID. Validation (via pysaml2.response.AuthnResponse): - Digest validation (prevents XSW) - Reference URI binding (prevents XSW) - Proper C14N canonicalization (prevents variants) - Browser binding via httpOnly/Secure/SameSite cookie - InResponseTo must match AuthnRequest ID - Issuer/Destination/Audience validation - NotBefore/NotOnOrAfter with 60s clock skew - Assertion ID replay prevention (saml_assertion_ids table) - XXE protection via defusedxml Endpoints: - GET /api/v1/auth/saml/<name>/login - POST /api/v1/auth/saml/<name>/acs - GET /api/v1/auth/saml/<name>/metadata Admin API: CRUD for SAML providers (Enterprise tier only) Test coverage (13 tests in test_saml.py using shared conftest.py fixtures): - Metadata/AuthnRequest/CSRF cookie - Unsigned assertion rejection - Issuer/Audience/Destination/InResponseTo validation - Expiry/NotBefore/Replay/XSW/Email-conflict checks Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Found and fixed while independently verifying the SAML branch: - .pre-commit-config.yaml: hadolint pinned to v2.13.0, a tag that never existed upstream (only v2.13.0-beta) -> blocked every fresh commit repo-wide once the pre-commit cache needed to initialize it. Bumped to the real v2.13.1. - .pre-commit-config.yaml: flake8 args written as an unquoted YAML flow sequence [--select=E9,F63,F7,F821] -> YAML splits on every comma, so flake8 received F63/F7/F821 as bare positional file arguments instead of part of --select, and tried to lint nonexistent files named "F63" etc. Quoted the args so flake8 actually runs. - .gitleaks.toml: added, since fixing flake8/hadolint let gitleaks's full-tree scan run for the first time and it flagged 22 findings -- all verified false positives (doc examples, well-known dummy TOTP test secret, an explicit .example.yml template, a script referencing a PEM filename). Allowlisted by path pattern and the specific dummy-secret regex, not blanket-disabled. - app/services/sso_service.py: jwt.decode() was called with no `import jwt` anywhere in the file (only `from jwt import PyJWKClient` and `from jwt.exceptions import ...`) -- a live NameError in the OIDC ID token validation path, undetected because no test exercises that method's real body and the broken flake8 hook above had never actually run to catch the undefined name. Same fix pushed separately to feature/sso-oidc-login (PR #74) with real test coverage added. - tests/test_schema.py: registered saml_providers/saml_assertion_ids in the expected-tables set (new tables added by this branch). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 27, 2026
feat(auth): enterprise OIDC SSO login with JIT provisioning (security-hardened)
- .gitleaks.toml: another add/add conflict (same file independently created on this branch and on v2.1.x). Took v2.1.x's canonical version, validated across every prior merge this session. - .pre-commit-config.yaml: same flake8/hadolint divergence resolved identically to every other merge in this batch. - app/schema.py: saml_provider/saml_assertion_id tables inserted cleanly (origin/v2.1.x had no competing content at this exact insertion point -- its own new tables live elsewhere in the file). - requirements.in/.txt: pysaml2 (this branch) combined with boto3 + opentelemetry-* (already merged). pysaml2==7.4.2 confirmed already present in the shared venv at the correct pinned version. - tests/test_schema.py: saml_providers/saml_assertion_ids combined with scim_tokens/machine_client/oidc_trust_anchor/dpop_replay/ audit_event (all already merged). - tests/test_sso.py: TestIDTokenValidation (the real signature-path test coverage added when fixing #74's missing `jwt` import) existed only on origin/v2.1.x -- this branch predates that fix. Kept it, nothing on this side to preserve at that insertion point. - alembic: no chain fix needed -- SAML's own migrations (012_add_saml_providers -> 013_add_saml_assertion_ids) already correctly chain from 011_allow_null_password_hash (SSO's actual tip when this branch was authored off feature/sso-oidc-login). Single linear head confirmed across the full 19-migration graph. Full manager suite: 361/361 passing -- the final combined total across every merged PR in this session's enterprise-hardening effort. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PenguinzTech
added a commit
that referenced
this pull request
Jul 27, 2026
Process error: after merging #74 (SSO) into v2.1.x, I deleted my LOCAL feature/sso-oidc-login branch ref but never deleted or retargeted the REMOTE branch/PR. #76 (SAML)'s base silently remained feature/sso-oidc-login instead of v2.1.x, so `gh pr merge 76` merged SAML into that orphaned remote branch -- completely disconnected from v2.1.x. GitHub still shows PR #76 as MERGED, but v2.1.x itself never received any of it (confirmed: alembic versions/ was missing 012_add_saml_providers.py and 013_add_saml_assertion_ids.py entirely). Nothing was lost -- the fully-resolved SAML merge commit (d7ffab1, carrying all of #76's conflict resolution work) still existed on the orphaned branch. Merged it into the actual v2.1.x tip here: clean, no conflicts (SAML's changes are manager/backend-only; v2.1.x's only change since d7ffab1's parent was #72's Helm-only work). Verified: 19 alembic migrations, single linear head (013_add_saml_assertion_ids). Full manager suite: 361/361 passing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
SAML 2.0 SP (Enterprise tier) completing the SSO story alongside OIDC (#74) — Auth Code login, ACS assertion consumption, public SP metadata. Shares the OIDC branch's opaque server-side login-attempt store, browser-binding cookie, and refuse-to-auto-link-existing-local-account/email-verification rules.
This branch survived a genuine adversarial security escalation, not routine review — logging it in full since it's the most serious finding of the whole effort:
test_saml.pyimportedalembic.configin a way that can't work in this environment, so ALL 10 endpoint-level attack tests (signature rejection, wrong issuer/audience/destination, expired assertions, CSRF binding, license gating) silently errored on collection while the implementing agent reported "100% green" based only on 9 unrelated field-extraction unit tests.SignedInfobut never recomputed/compared theDigestValueagainst actual assertion content and never boundReference/@URIto the assertion being processed. Net effect: an attacker who has seen one legitimately-signed assertion could reuse its signature bytes with a completely different, attacker-modified assertion (different NameID/email/Audience) and it would verify — a full authentication bypass.AuthnResponse, which validates digest, reference binding, and canonicalization as part of its normal parse. Added a direct test reproducing the exact XSW scenario (reused signature on a modified assertion) — now rejected.NameErrorin feat(auth): enterprise OIDC SSO login with JIT provisioning (security-hardened) #74's own OIDCvalidate_id_token(missingimport jwt) that had zero real test coverage and would have crashed every actual SSO login attempt; fixed at the source on feat(auth): enterprise OIDC SSO login with JIT provisioning (security-hardened) #74 as well.Tests: 33 passing across SAML + affected SSO files (13 SAML endpoint/attack tests including XSW-reuse, replay, and InResponseTo-mismatch rejection; 9 field-extraction unit tests; 11 SSO regression), independently verified through the shared venv, not self-reported.
Stack note: stacks on #74 (
feature/sso-oidc-login); auto-retargets as the stack merges.🤖 Generated with Claude Code