feat(sap): add encrypted private key and self-signed SAML assertion config (ENT-12301) - #206

Pulls in pbitla's force-pushed update to ENT-12301 (PR openedx#206): the
write-only decrypted_private_key admin form (never re-renders the
stored key; blank submit keeps it) and the encrypted_key/secret/
private_key dedup into a single _encrypted_property() factory.

Conflict resolution in models.py kept this branch's auth_type-aware
is_valid rewrite from ENT-12302 (2e12334) over the older version this
branch had originally ported from ENT-12301 -- it adds PEM validation
via is_valid_pem_private_key() and, importantly, no longer requires
the deprecated saml_assertion_api_path for modern auth, which
pbitla's version still does. Also restored this branch's decision to
keep decrypted_key required for both auth types (needed as the SAML
assertion Issuer) after the merge silently adopted pbitla's newer
change dropping that requirement -- flagged for confirmation against
SAP's actual token-endpoint contract, since neither branch's client
wiring exists yet to verify it. Also removed field-definition
duplication (auth_type, decrypted_private_key, saml_assertion_*,
oauth_token_api_path) that the merge introduced silently, since both
sides added them as new relative to their common ancestor. Restored
this branch's test_models.py wholesale (real generated RSA keys,
explicit legacy/modern is_valid coverage per field) over pbitla's
simpler version.

Co-Authored-By: Claude Sonnet 5 <[email protected]>

Add an is_ready_to_transmit hook to EnterpriseCustomerPluginConfiguration,
called by every transmission entry point. The base implementation passes
everything through, so no other channel's behaviour changes.

SAPSuccessFactorsEnterpriseCustomerConfiguration overrides it to abort
before any request to SAP when is_valid reports a problem other than
display_name, logging the missing and invalid fields and recording
blocked sync runs as errored attempts. unlink_inactive_learners is gated
the same way, without touching sync timestamps.

Both downstream transmission toggles, disable_learner_data_transmissions
and dry_run_mode_enabled, are honoured inside the transmitters, so this
gate runs ahead of them. Tests pin that ordering so it stays a decision
rather than a surprise.

Known gap: is_valid does not require the client secret for
self_signed_assertion configs (openedx#206), while the API client still obtains
every token through the SAP-signed flow, so such a config passes this
gate and then fails at SAP. ENT-12305 closes this once self-signed
assertions are wired into the client.

Co-Authored-By: Claude Sonnet 5 <[email protected]>

…v, add signxml dep

- Bump python-semantic-release action pin from v10.6.2 to v10.7.0
  (b700dbeb1f431a0fcc30a79f3f0869407fe07278), matching openedx/sample-plugin's
  current live pin.
- Fold {include-group = "doc"} into the dev dependency-group so `make
  requirements` (uv sync --group dev) provisions doc8/Sphinx, matching the
  pattern already correct in openedx/edx-enterprise's pyproject.toml.
- Add signxml (pulled in transitively via the SAML assertion generator added
  on main in #201/#206) to [project.dependencies], since this migration
  replaced requirements/base.in with pyproject.toml as the source of truth
  for runtime dependencies and base.in's new direct dependency needs an
  equivalent entry there. Re-lock uv.lock accordingly.

Co-Authored-By: Claude Sonnet 5 <[email protected]>

…12303)

Built directly on merged main (PR openedx#206), not on PR openedx#207 -- confirmed
our code only depends on fields/validation openedx#206 already added
(auth_type, decrypted_private_key, decrypted_private_key_passphrase,
saml_assertion_audience), nothing openedx#207 adds (is_ready_to_transmit,
COSMETIC_CONFIG_PROBLEMS). No need to wait on openedx#207 to merge first.

- serializers.py: auth_type, write-only private_key and
  private_key_passphrase, validated as a PEM-encoded RSA key before
  save, never returned in responses.
- admin/__init__.py: conditional field rendering based on auth_type;
  write-only widgets for the private key/passphrase fields so a
  stored value is never rendered and a blank submission preserves
  rather than clears it; form-level validation matching the
  serializer. Fixes the same plaintext-exposure class of bug flagged
  (and still open) on PR openedx#206's own review.
- utils.py: is_valid_pem_private_key requires the key to parse as RSA
  specifically, since that utility no longer exists upstream (removed
  in openedx#206's own rewrite) and needed reimplementing to match.

Verified: 198/198 tests passing, plus manual end-to-end testing
against a live devstack (Django Admin click-through + API via Bruno).

Co-Authored-By: Claude Sonnet 5 <[email protected]>

…12303)

Built on merged main (PR openedx#206) -- confirmed this only depends on
fields/validation openedx#206 already added (auth_type, decrypted_private_key,
decrypted_private_key_passphrase, saml_assertion_audience), nothing
from openedx#207.

Serializer (channel_integrations/api/v1/sap_success_factors/serializers.py):
- Expose auth_type, write-only private_key and private_key_passphrase.
- validate() checks the key against whichever passphrase applies once
  saved (submitted or already-stored) and vice versa, so a PATCH
  touching only one of the two is still validated against the other.

Admin (channel_integrations/sap_success_factors/admin/__init__.py):
- Every credential field stays visible on the add page and for
  SAP-signed configs; only decrypted_secret is hidden once a config
  is already self-signed. decrypted_key stays visible in both modes,
  since the model requires it either way (it's the assertion's
  Issuer). This guarantees clean() always validates a bound
  decrypted_private_key -- the previous design showed
  decrypted_private_key only for already-self-signed configs, so a
  self-signed submission on the add page, or one switching an
  existing SAP-signed config in the same save, skipped validation
  entirely and saved with no key. clean() also falls back to a
  non-field error instead of crashing if some other, non-standard
  form ever restricts the field out again.
- decrypted_private_key uses a write-only Textarea and
  decrypted_private_key_passphrase Django's own PasswordInput, so a
  stored value is never rendered and a rejected submission doesn't
  echo it back either. clear_decrypted_private_key and
  clear_decrypted_private_key_passphrase checkboxes let an admin
  explicitly clear either field, since a blank submission alone means
  "leave unchanged" (the field can't show what's currently stored).

utils.py: consolidates the RSA PEM-parsing check that the model, the
SAML assertion generator (saml.py), and the serializer/admin each
implemented separately into one shared load_rsa_private_key (with a
skip_rsa_validation option matching the model's existing perf
tradeoff); the model and saml.py now delegate to it instead of
repeating it.

Tests: unit tests for serializer masking/validation and admin
conditional fields/validation, including a regression test through
the real ModelAdmin.get_form() add page, a clear-checkbox test, and
API tests for the passphrase-only-update and new-key-against-stored-
passphrase cases. 203/203 passing. Also manually verified end-to-end
against a live devstack (Django Admin click-through + API via Bruno).

Co-Authored-By: Claude Sonnet 5 <[email protected]>

…o 0.1.74

The SAML bearer tests passed encrypted_private_key, which isn't a model field, and used an http base URL, which the assertion generator rejects for self-signed configs. Also drops the ENT-12305 TODO and the saml.py 'not yet called' note, now that the client uses the generator.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

…12303)

Built on merged main (PR openedx#206) -- confirmed this only depends on
fields/validation openedx#206 already added (auth_type, decrypted_private_key,
decrypted_private_key_passphrase, saml_assertion_audience), nothing
from openedx#207.

Serializer (channel_integrations/api/v1/sap_success_factors/serializers.py):
- Expose auth_type, write-only private_key and private_key_passphrase.
- validate() requires and validates a usable private key whenever the
  *resulting* auth type is self-signed (checked against whichever
  key/passphrase apply once saved, submitted or already-stored, not
  just whatever the request happens to touch), and validates a
  submitted private_key regardless of auth type. A request touching
  only auth_type, with neither credential field present, used to skip
  this check entirely and could save a self-signed configuration with
  no usable key; private_key_passphrase alone does not trigger the
  check for a non-self-signed configuration, since it's allow_blank
  and a client may harmlessly send it empty on every request.
- Malformed-key validation uses full RSA validation
  (skip_rsa_validation=False), not the fast path used for repeated
  is_valid reads, so a mathematically inconsistent key is caught
  immediately instead of failing later when the signing path reloads
  it with full validation.

Admin (channel_integrations/sap_success_factors/admin/__init__.py):
- Every credential field stays visible on the add page and for
  SAP-signed configs; only decrypted_secret is hidden once a config
  is already self-signed. decrypted_key stays visible in both modes,
  since the model requires it either way (it's the assertion's
  Issuer). This guarantees clean() always validates a bound
  decrypted_private_key -- the previous design showed
  decrypted_private_key only for already-self-signed configs, so a
  self-signed submission on the add page, or one switching an
  existing SAP-signed config in the same save, skipped validation
  entirely and saved with no key. clean() also falls back to a
  non-field error instead of crashing if some other, non-standard
  form ever restricts the field out again.
- decrypted_private_key uses a write-only Textarea and
  decrypted_private_key_passphrase Django's own PasswordInput, so a
  stored value is never rendered and a rejected submission doesn't
  echo it back either. clear_decrypted_private_key and
  clear_decrypted_private_key_passphrase checkboxes let an admin
  explicitly clear either field, since a blank submission alone means
  "leave unchanged" (the field can't show what's currently stored).

utils.py: consolidates the RSA PEM-parsing check that the model, the
SAML assertion generator (saml.py), and the serializer/admin each
implemented separately into one shared load_rsa_private_key (with a
skip_rsa_validation option matching the model's existing perf
tradeoff for repeated reads); the model and saml.py now delegate to
it instead of repeating it. is_valid_pem_private_key exposes the same
option so write-time callers can request full validation.

Tests: unit tests for serializer masking/validation and admin
conditional fields/validation, including a regression test through
the real ModelAdmin.get_form() add page, a clear-checkbox test, and
API tests for the auth-type-only-switch, passphrase-only-update, and
new-key-against-stored-passphrase cases. 204/204 passing. Also
manually verified end-to-end against a live devstack (Django Admin
click-through + API via Bruno).

Co-Authored-By: Claude Sonnet 5 <[email protected]>

…12303)

Surface SAP SuccessFactors modern authentication (auth_type, private
key, passphrase) through the REST API serializer, with validation
centralized in a single model method (get_credential_errors) so the
serializer and admin can't drift, a minimum RSA key-size floor, and
regression coverage for auth-type switching, key-clearing, and
credential-validation edge cases (schemeless URLs, passphrase
mismatches, weak/non-RSA/malformed keys). The admin form follows the
existing repo convention (plain ModelForm, no custom masking widgets)
per the precedent set in PR openedx#206.

Co-Authored-By: Claude Sonnet 5 <[email protected]>

…12303)

Surface SAP SuccessFactors modern authentication (auth_type, private
key, passphrase) through the REST API serializer, with validation
centralized in a single model method (get_credential_errors) so the
serializer and admin can't drift, a minimum RSA key-size floor, and
regression coverage for auth-type switching, key-clearing, and
credential-validation edge cases (schemeless URLs, passphrase
mismatches, weak/non-RSA/malformed keys). The admin form follows the
existing repo convention (plain ModelForm, no custom masking widgets)
per the precedent set in PR openedx#206, and no longer crashes on an
over-length sapsf_base_url. cryptography is now declared as a direct
dependency.

Co-Authored-By: Claude Sonnet 5 <[email protected]>

…e) (#180)

* feat: consolidate package metadata into pyproject.toml

Replace setup.py/setup.cfg with PEP 621 [project] metadata and
setuptools-scm for git-tag-based versioning.

Co-Authored-By: Claude Sonnet 5 <[email protected]>

* feat: switch dependency management from pip-compile to uv

Replace requirements/*.in + *.txt with PEP 735 dependency-groups in
pyproject.toml and a single uv.lock. Update tox.ini to use tox-uv's
uv-venv-lock-runner, update Makefile targets, and switch CI to install
uv and run tests via `uv run tox`. Point .readthedocs.yaml at uv too,
since it referenced the now-deleted requirements/doc.txt.

Co-Authored-By: Claude Sonnet 5 <[email protected]>

* feat: add semantic-release for automated PyPI publishing

Replace the manual GitHub-release-triggered publish workflow with
python-semantic-release: pushes to main with conventional commits
now automatically bump the version, tag it, and publish to PyPI.

Co-Authored-By: Claude Sonnet 5 <[email protected]>

* fix: correct release tag format and drop stale MANIFEST.in entries

semantic-release defaults to a "v{version}" tag format, but this repo's
existing release tags are bare version numbers -- without tag_format set,
semantic-release wouldn't recognize any prior release. Also removes
MANIFEST.in references to requirements/base.in and requirements/constraints.txt,
which no longer exist after the uv migration.

Co-Authored-By: Claude Sonnet 5 <[email protected]>

* fix: prevent uv sync from implicitly pulling in the dev group

Every uv sync/uv run invocation in this repo names an explicit --group,
but uv's implicit default group (named "dev") was still being synced
alongside it, silently pulling the entire dev/test/quality/ci superset
into every target. Verified with `uv sync --group ci`.

Co-Authored-By: Claude Sonnet 5 <[email protected]>

* fix: publish to PyPI via OIDC instead of token

Parent issue openedx/public-engineering#506 asks for OIDC trusted-publisher
PyPI auth, not a stored token. Grant id-token: write on publish_to_pypi and
drop the explicit __token__/PYPI_UPLOAD_TOKEN credentials -- pypa/gh-action-pypi-publish
uses OIDC automatically once the permission is present and no credentials are given.

Co-Authored-By: Claude Sonnet 5 <[email protected]>

* fix: prevent premature 1.0 bump and pin pypa/gh-action-pypi-publish

- major_on_zero defaults to True in python-semantic-release, which would
  auto-bump this still-0.x repo straight to 1.0.0 on the first breaking-change
  commit. 1.0.0 is supposed to be a deliberate "first stable release" signal,
  not an accidental side effect -- keep 0.x bumps within 0.x until that's an
  intentional decision (allow_zero_version stays true so 0.x remains valid).
- @release/v1 is a floating branch ref -- xblocks-core's release just failed
  with "docker: manifest unknown" because the Docker image tag it resolved to
  at checkout time wasn't published on ghcr.io yet. Pin to the exact commit
  backing the current v1.14.0 release instead.

Co-Authored-By: Claude Sonnet 5 <[email protected]>

* refactor: adopt src-layout, matching openedx/sample-plugin

Moves channel_integrations/ to src/channel_integrations/, in line with
the reference implementation for this modernization effort
(openedx/sample-plugin) and openedx/forum#281.

- pyproject.toml: add where = ["src"] to packages.find
- tox.ini: prefix src/ onto pylint/pycodestyle targets in the quality env
- Makefile: prefix src/ onto all 5 localization targets (extract,
  compile, detect_changed, pull, dummy); atlas pull's local destination
  path updated, remote Transifex identifier left unchanged
- test_settings.py: LOCALE_PATHS root() call updated
- docs/conf.py: sphinx-apidoc call updated to point at src/channel_integrations
- MANIFEST.in: recursive-include path updated

Verified: uv sync (no lock drift), full django42 test run (893 passed),
quality and pii_check envs pass, uv build --wheel + twine check pass.

The docs tox env fails both before and after this change (a pre-existing
sphinx/autodoc ValueError in object_description, unrelated to src-layout
and unrelated to this repo's file layout) -- confirmed by reproducing the
identical failure on the pre-migration commit via git stash.

* fix: pattern-gap audit (uv setup, package=true, coverage, changelog)

Same category of fixes farhan flagged on the batch's other reviewed PRs:

- ci.yml: add enable-cache/python-version to astral-sh/setup-uv and drop
  the now-redundant actions/setup-python step
- pyproject.toml: add [tool.uv] package = true
- Drop CHANGELOG.rst from the dynamic readme file list and delete it --
  python-semantic-release + GitHub Releases is the changelog of record.
  Also removes the now-stale MANIFEST.in include and the docs/changelog.rst
  page (and its toctree entry).
- Migrate .coveragerc into [tool.coverage.run] and delete the old file

Verified: uv sync (no lock drift), full django42 test run (893 passed),
quality env passes. docs env still fails on the same pre-existing
sphinx/autodoc ValueError already documented in this repo's src-layout
commit (confirmed via git stash at that time it predates and is
unrelated to any of our changes) -- docs isn't part of the CI matrix.

* fix: pattern-gap audit round 2 (uv run wrapping, dynamic readme, tox envlist, readthedocs os)

Cross-checked against review comments/fixes from openedx-ledger#242,
edx-enterprise-subsidy-client#222, enterprise-access#1015, and
enterprise-subsidy#441.

- Wrap remaining bare tool invocations in the Makefile (coverage erase,
  pytest --cov-report html, PYTHONPATH pytest, diff-cover, i18n_tool,
  manage.py compilemessages) with `uv run`. Transifex's tx binary is left
  bare since it's installed via curl, not uv/pip.
- Drop the redundant dynamic readme field; keep only version as dynamic.
- Match the py{311}-django{42,52} brace convention in the tox envlist.
- Bump Read the Docs build image to ubuntu-26.04.

* fix: correct Makefile recipe indentation (tabs, not spaces)

Make requires recipe lines to be indented with a literal tab
character. An earlier edit in this migration accidentally used
spaces for several targets and dropped the ifeq(OPENEDX_ATLAS_PULL)
conditional line, breaking 'make selfcheck' and CI.

* fix: restore CHANGELOG.rst and re-enable changelog generation

A previous migration pass deleted CHANGELOG.rst and disabled
python-semantic-release's changelog generation (changelog: "false" in
release.yml). Neither was required by the tooling-modernization ticket,
and deleting the file discarded real historical release notes.

- Restore CHANGELOG.rst from the commit before it was deleted
- Add the .. changelog-insertion-marker as the first line so PSR's
  "update" mode inserts new release notes above existing history
- Remove changelog: "false" from release.yml's PSR step
- Add [tool.semantic_release.changelog] config to pyproject.toml,
  pointing PSR at the restored CHANGELOG.rst in RST format
- tag_format was already correctly set to "{version}" matching this
  repo's bare version tags (e.g. 0.1.65), no change needed there

Co-Authored-By: Claude Sonnet 5 <[email protected]>

* docs: remove stale version-bump/changelog checklist items from PR template

python-semantic-release now handles versioning and changelog generation
automatically, so asking PR authors to manually bump the version or add
a changelog record is obsolete and was left over from before that
automation was wired up.

Co-Authored-By: Claude Sonnet 5 <[email protected]>

* fix: use stable pypa publish tag and drop tox from Makefile

- pypa/gh-action-pypi-publish: revert the hash-pinned SHA back to the
  stable @release/v1 tag. A hash-pinned version of this action broke
  PyPI publishing previously, which is why the org standardized on the
  stable tag for this specific action.
- Makefile: inline the actual doc8/sphinx/pylint/pycodestyle/
  code_annotations commands from tox.ini's docs/quality/pii_check envs
  instead of shelling out to `uv run tox -e ...`, matching the
  no-tox-in-Makefile convention already used elsewhere (e.g.
  enterprise-catalog). test-all now depends on quality/pii_check/test/
  docs directly. tox.ini itself is untouched; CI's own matrix testing
  still uses tox directly.

* fix: use int-parseable fallback_version instead of 0.0.0.dev0

"0.0.0.dev0" is the exact fallback_version value that crashed 17 tests
in a sibling repo (openedx-events) -- runtime code that parses
__version__ via tuple(map(int, __version__.split("."))) chokes on the
non-numeric "dev0" segment. No current consumer here does that (this
repo's own __version__ usage, if any, only interpolates it as a
string), but there's no reason to keep a fallback value from the exact
banned-pattern class when a plain int-parseable "0.0.0" is equally
valid and strictly safer.

* fix: disable python-semantic-release changelog generation

Set changelog: "false" on the PSR release step and remove the
[tool.semantic_release.changelog] config / insertion marker.

Checked against openedx/XBlock's actual production release.yml (the one
repo in this effort that has cut real automated releases) -- every run
passes changelog: false and invokes `semantic-release -v version
--no-changelog`, and the repo has zero github-actions[bot] commits ever.
The auto-changelog config this migration previously added was only ever
verified via a local dry-run prototype, never against a real release.

feanil flagged the same issue on openedx/DoneXBlock#388: "I thought we
were not going to add changelogs since they can't be updated by
python-semantic-release the way we have it setup."

* fix: create GitHub release with assets attached, not after publish

This repo has immutable releases enabled, which freezes a release's assets the moment it's published. The old flow (main PSR step publishes the release, a separate publish-action step attaches assets afterward) can never work under that constraint -- it would 422 on the first real release. Matches the fix already proven and merged on openedx/sample-plugin#57 and validated end-to-end on openedx/event-tracking#434: build without publishing (vcs_release: false), then create the release with dist/* attached in one gh release create call.

* fix: conform release.yml action pins to sample-plugin gold standard

release.yml's actions/checkout, python-semantic-release, upload-artifact,
and download-artifact pins were all stale relative to
openedx/sample-plugin's release.yml, this org's designated reference for
this file. Also re-pins pypa/gh-action-pypi-publish to a SHA
(dc37677b2e1c63e2034f94d8a5b11f265b73ba33 # v1.14.2), reversing an earlier
revert to @release/v1 that was made on this PR at salman2013's request
after a prior SHA-pin caused issues. That guidance has since been
superseded: salman2013 has since directed (on a sibling repo's PR) to
match sample-plugin exactly, which now itself SHA-pins
gh-action-pypi-publish at v1.14.2.

Every pin below was independently verified against the GitHub API before
use (commit resolves, and matches its claimed tag, dereferencing through
the tag object for annotated tags):
- actions/checkout -> 3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
- python-semantic-release/python-semantic-release ->
  9a026e9303981c866c3425723009becb2437c757 # v10.6.2
- actions/upload-artifact -> 043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
- actions/download-artifact -> 3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1
- pypa/gh-action-pypi-publish -> dc37677b2e1c63e2034f94d8a5b11f265b73ba33 # v1.14.2

Co-Authored-By: Claude Sonnet 5 <[email protected]>

* fix: reconcile CHANGELOG.rst with main after rebase

The CHANGELOG.rst restoration commit recreated the file from a fixed
snapshot frozen at 0.1.62. Rebasing onto current main (now at 0.1.69)
exposed that the restoration doesn't track main's ongoing history --
it's a one-time static blob, not a merge. Replaced with main's actual
current content, restoring the 7 missing entries (0.1.63-0.1.69)
between Unreleased and 0.1.62, including PR #197's entry that
collided with this rebase.

Co-Authored-By: Claude Sonnet 5 <[email protected]>

* fix: use v-prefixed tag_format for release tag consistency

* fix: use secrets.GITHUB_TOKEN instead of custom org secret for release job

Matches openedx/sample-plugin's current release.yml; the custom
OPENEDX_SEMANTIC_RELEASE_GITHUB_TOKEN secret isn't needed.

* fix: restore tox delegation for docs/quality/pii_check targets

Makefile targets were fully inlining commands that tox.ini already
defines as standalone envs (docs, quality, pii_check), restoring the
pre-migration one-liner tox -e <env> delegation.

Co-Authored-By: Claude Sonnet 5 <[email protected]>

* fix: drop uv run from Makefile per feanil's review feedback

Makefile now assumes an already-synced/activated local env; uv run
stays in CI workflow steps only. See openedx/ccx-keys#190 (review
comment r4097501942).

Co-Authored-By: Claude Sonnet 5 <[email protected]>

* fix: bump python-semantic-release pin to v10.7.0, add doc group to dev, add signxml dep

- Bump python-semantic-release action pin from v10.6.2 to v10.7.0
  (b700dbeb1f431a0fcc30a79f3f0869407fe07278), matching openedx/sample-plugin's
  current live pin.
- Fold {include-group = "doc"} into the dev dependency-group so `make
  requirements` (uv sync --group dev) provisions doc8/Sphinx, matching the
  pattern already correct in openedx/edx-enterprise's pyproject.toml.
- Add signxml (pulled in transitively via the SAML assertion generator added
  on main in #201/#206) to [project.dependencies], since this migration
  replaced requirements/base.in with pyproject.toml as the source of truth
  for runtime dependencies and base.in's new direct dependency needs an
  equivalent entry there. Re-lock uv.lock accordingly.

Co-Authored-By: Claude Sonnet 5 <[email protected]>

---------

Co-authored-by: Irfan Ahmad <[email protected]>
Co-authored-by: Claude Sonnet 5 <[email protected]>