Repository navigation
feat(ee): SAML 2.0 foundation - python3-saml deps, strict SP settings and SP metadata - #441
Open
grumpy-miner wants to merge 1 commit into
Open
grumpy-miner wants to merge 1 commit into
grumpy-miner wants to merge 1 commit into
Conversation
… and SP metadata Slice 1 of the "SAML 2.0 sign-in (ee/)" card (board item PVTI_lADOElDmdM4Bn50jzhHLaw0). Pins python3-saml on the xmlsec and lxml wheels in the ee/ package, guards the lxml/xmlsec libxml2 mismatch (also at hosted image build time), and builds the hardened SP settings and SP metadata the ACS slices will use. No route is mounted and no assertion is validated yet. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RPybyrwWRrPcN25xgn8RXW
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
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.
Board card: SAML 2.0 sign-in (ee/) (PVTI_lADOElDmdM4Bn50jzhHLaw0)
Draft card, so no
Closes. This is slice 1 of 5 from the card ("library + Docker deps"). It contains no assertion-validation code, so nothing here is the security-critical part the card asks the owner to review before merge.What this PR does
ee/backend/pyproject.toml: addspython3-saml>=1.16,<2,xmlsec>=1.3.18,<2,lxml>=6.1,<7to theganek-eepackage only. The core AGPL package and the public image get no SAML code or deps.ee/backend/ganek_ee/saml.py(new):check_native_bindings()refuses to run when lxml and xmlsec load different libxml2 builds.IdpConfig,sp_entity_id(),acs_url()(/api/v1/auth/saml/metadata,/api/v1/auth/saml/acs, same prefix rule asoidc_redirect_uri).build_settings()/build_settings_dict(): strict toolkit settings.wantAssertionsSigned,wantNameId,rejectDeprecatedAlgorithm(no SHA-1),rejectUnsolicitedResponsesWithInResponseTo(no IdP-initiated login), no repeated attribute names, RSA-SHA256/SHA-256,debugoff. It refuses a missing IdP cert and plain-http URLs (excepthttp://localhost).sp_metadata(): SP metadata XML, validated by the toolkit.backend/Dockerfilehostedstage: runscheck_native_bindings()at build time, so a mismatched wheel pair fails the hosted image build and not the first sign-in. No apt packages are needed: the cp314 manylinux wheels for xmlsec and lxml bundle libxmlsec1/libxml2 (amd64 and aarch64 wheels exist).ee/backend/tests/test_saml.py: 11 tests (native binding match + mismatch refusal, URL building, strict flags, toolkit accepts the settings, missing cert, http refusal, localhost allowed, metadata entityID / POST ACS / WantAssertionsSigned, no DOCTYPE).Research summary
xmlsecbinding, and has explicit strict flags for the attacks the card names. pysaml2 shells out to thexmlsec1binary and needs an apt package plus a much larger config surface. Its CVE history (CVE-2021-21238/21239) is in exactly that signature path.xmlsecsource builds (python-xmlsec issues feat(interviews): cancel and reminder mails to both sides, invite.ics download #324/docs: refresh demo seed counts, frontend route map, question coverage #352, Indico forum threads). Fix: xmlsec 1.3.18 and lxml 6.1.3 both ship cp314 manylinux wheels built on libxml2 2.14.6 (checked here). The floors pin that, and the guard plus the build-time check catch any future drift.Remaining slices (not in this PR)
Owner decision needed: the card's AC says "Owner reviewed the validation code before merge", but
auto/*PRs auto-merge on green without review (2026-11-07 decision). The agent therefore stops after this slice. Options: (a) review slice 2 as an agent PR that the agent opens as draft or on a non-auto/branch, or (b) waive the review AC. Card set to In review until you decide.Testing
uv sync,uv pip install ../ee/backend,ruff check+ruff format --checkon ee/backend clean,pytest ../ee/backend/tests: 74 passed (11 new).check_native_bindings()passes with the installed wheels (libxml2 2.14.6 on both sides).hostedtarget either, so the new RUN line is first exercised on the next hosted build.git diff --cached | md5sume694e184... on both sides.🤖 Generated with Claude Code
https://claude.ai/code/session_01RPybyrwWRrPcN25xgn8RXW
Status (run 2026-11-09 ~17:55 UTC) - blocked, needs the owner
CI (all 7 jobs incl. the ee/ job), E2E and the auto-merge workflow are green and
auto_mergeis enabled. GitHub has not merged: the ruleset requires thecla-assistantcheck, and theclaworkflow run 42666647964 ended in startup_failure with no jobs. This is the same blocker as #439 and #440: everyclarun since 16:15 UTC has failed to start, and the last success was at 15:16 UTC on #438. The PR was not merged by hand and no workflow was edited. To unblock, open the cla run page for the startup error or commentrecheck. The PR then merges itself. Separately, slice 2 needs your decision on the owner-review AC (see above). Card set to In review.