feat: add SAML provider admin viewsets to enterprise API by pwnage101 · Pull Request #2550 · openedx/edx-enterprise

pwnage101 · 2026-03-05T00:16:29Z

Migrate two enterprise-specific viewsets from openedx-platform into edx-enterprise.

ENT-11567

Copilot

Pull request overview

Adds enterprise API endpoints to administer SAML provider configuration and provider data (including a “sync metadata” action), migrating supporting utilities from openedx-platform into edx-enterprise.

Changes:

Introduces SAMLProviderConfigViewSet and SAMLProviderDataViewSet under /enterprise/api/v1/auth/saml/v0/....
Adds shared SAML helper utilities (validate_uuid4_string, metadata fetch/parsing helpers).
Adds unit/API tests covering the new utilities and viewsets.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
`enterprise/api/v1/views/saml_utils.py`	New SAML helper functions (UUID validation, slug/provider_id conversion, metadata XML fetch/parse).
`enterprise/api/v1/views/saml_provider_config.py`	New admin CRUD viewset for SAML provider config + enterprise IDP association management.
`enterprise/api/v1/views/saml_provider_data.py`	New admin CRUD viewset for SAML provider data + metadata sync action.
`enterprise/api/v1/urls.py`	Registers new SAML admin routes via a router.
`tests/test_enterprise/api/test_saml_utils.py`	Unit tests for the migrated SAML utilities.
`tests/test_enterprise/api/test_saml_provider_config.py`	API tests for the SAML provider config viewset behaviors.
`tests/test_enterprise/api/test_saml_provider_data.py`	API tests for the SAML provider data viewset and sync action.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-31T01:52:05Z

+            raise Http404('No matching SAML provider found.')  # pylint: disable=raise-missing-from
+        provider_data_id = self.request.parser_context.get('kwargs', {}).get('pk')
+        if provider_data_id:
+            return SAMLProviderData.objects.filter(id=provider_data_id)


In the detail case (when pk is present), the queryset is filtered only by id, not by the enterprise’s SAML provider/entity_id. That means an enterprise admin could potentially access/update/delete a SAMLProviderData row belonging to a different enterprise by guessing its ID. Filter by both id and entity_id (derived from the enterprise’s SAML provider), or otherwise enforce enterprise scoping for the detail route as well.

Suggested change

return SAMLProviderData.objects.filter(id=provider_data_id)

return SAMLProviderData.objects.filter(

id=provider_data_id,

entity_id=saml_provider.entity_id,

)

this is probably worth doing as a general safeguard

Copilot · 2026-03-31T01:52:07Z

+    try:
+        log.info("Fetching %s", url)
+        if not url.lower().startswith('https'):
+            log.warning("This SAML metadata URL is not secure! It should use HTTPS. (%s)", url)
+        response = requests.get(url, verify=True)  # May raise HTTPError or SSLError or ConnectionError
+        response.raise_for_status()  # May raise an HTTPError
+
+        try:
+            parser = etree.XMLParser(remove_comments=True)
+            xml = etree.fromstring(response.content, parser)
+        except etree.XMLSyntaxError:  # lint-amnesty, pylint: disable=try-except-raise
+            raise
+        return xml
+    except (requests.exceptions.SSLError, requests.exceptions.HTTPError, requests.exceptions.RequestException) as error:
+        log.exception(str(error), exc_info=error)
+        raise error
+    except etree.XMLSyntaxError as error:
+        log.exception(str(error), exc_info=error)
+        raise error


In fetch_metadata_xml, using log.exception(..., exc_info=error) is incorrect (it should typically be exc_info=True inside an except block), and raise error will replace the original traceback context. Prefer log.exception("...") (or log.exception("...", exc_info=True)) and a bare raise to preserve the original traceback. Also consider adding a reasonable timeout to requests.get to avoid hanging requests.

codecov · 2026-04-08T00:25:05Z

Codecov Report

❌ Patch coverage is 97.64706% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.91%. Comparing base (b9cb840) to head (f079944).
⚠️ Report is 3 commits behind head on master.

Files with missing lines	Patch %	Lines
enterprise/api/v1/views/saml_provider_data.py	97.22%	1 Missing and 1 partial ⚠️
enterprise/api/v1/views/saml_utils.py	94.11%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2550      +/-   ##
==========================================
+ Coverage   85.79%   85.91%   +0.12%     
==========================================
  Files         247      250       +3     
  Lines       16434    16604     +170     
  Branches     1629     1639      +10     
==========================================
+ Hits        14100    14266     +166     
- Misses       1999     2001       +2     
- Partials      335      337       +2

Flag	Coverage Δ
unittests	`85.91% <97.64%> (+0.12%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot

Pull request overview

Copilot reviewed 17 out of 18 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-08T00:34:01Z

+log = logging.getLogger(__name__)
+


log = logging.getLogger(__name__) is currently unused in this module; if no logging is needed, remove it to avoid lint failures and reduce noise.

pwnage101 · 2026-04-08T05:53:34Z

 #enterprise/static/enterprise/bundles/

 # claude code personalization
-CLAUDE.md


@iloveagent57 how can we stop adding CLAUDE.md to .gitignore?

- Add SAMLProviderConfigViewSet and SAMLProviderDataViewSet to the enterprise API with permission checks and comprehensive tests. - Add get_saml_identity_provider and get_identity_providers utility views for enterprise SAML operations. - Migrate SAML utility functions (get_saml_provider_data_by_enterprise, get_saml_provider_configuration) from third_party_auth to enterprise, with comprehensive tests using django-mock-queries MockSet. ENT-11567 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

iloveagent57

approving to unblock, but I agree with these two copilot suggestions

iloveagent57 · 2026-04-09T18:00:35Z

+        saml_config_ids = [
+            config.id for config in SAMLProviderConfig.objects.current_set()
+            if config.provider_id in slug_list
+        ]
+        return SAMLProviderConfig.objects.filter(id__in=saml_config_ids)


I agree with this.

iloveagent57 · 2026-04-09T18:01:27Z

+            raise Http404('No matching SAML provider found.')  # pylint: disable=raise-missing-from
+        provider_data_id = self.request.parser_context.get('kwargs', {}).get('pk')
+        if provider_data_id:
+            return SAMLProviderData.objects.filter(id=provider_data_id)


this is probably worth doing as a general safeguard

- Push provider_id filtering into database via current_set().filter() instead of iterating all configs in Python. - Scope SAMLProviderData detail lookups by entity_id to prevent cross-enterprise data access by guessing IDs. ENT-11567 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

pwnage101 commented Mar 5, 2026

View reviewed changes

Comment thread enterprise/api/v1/views/saml_provider_config.py Outdated

pwnage101 mentioned this pull request Mar 5, 2026

feat: remove SAML provider admin views from openedx-platform openedx/openedx-platform#38104

Merged

pwnage101 force-pushed the pwnage101/ENT-11567 branch from a9d2a21 to 8cbe8bf Compare March 31, 2026 01:22

pwnage101 requested a review from Copilot March 31, 2026 01:46

Copilot started reviewing on behalf of pwnage101 March 31, 2026 01:47 View session

Copilot AI reviewed Mar 31, 2026

View reviewed changes

pwnage101 force-pushed the pwnage101/ENT-11567 branch 10 times, most recently from b5db25d to 96d8730 Compare April 8, 2026 00:10

pwnage101 requested a review from Copilot April 8, 2026 00:28

Copilot started reviewing on behalf of pwnage101 April 8, 2026 00:29 View session

Copilot AI reviewed Apr 8, 2026

View reviewed changes

pwnage101 force-pushed the pwnage101/ENT-11567 branch 3 times, most recently from 251d5ea to f7f3e47 Compare April 8, 2026 04:53

pwnage101 marked this pull request as ready for review April 8, 2026 04:53

pwnage101 force-pushed the pwnage101/ENT-11567 branch 3 times, most recently from 9c60030 to 4b2d4bf Compare April 8, 2026 05:50

pwnage101 commented Apr 8, 2026

View reviewed changes

pwnage101 force-pushed the pwnage101/ENT-11567 branch from 4b2d4bf to 2fd6aa8 Compare April 8, 2026 20:58

pwnage101 assigned pwnage101 and kiram15 and unassigned pwnage101 Apr 8, 2026

iloveagent57 approved these changes Apr 9, 2026

View reviewed changes

pwnage101 force-pushed the pwnage101/ENT-11567 branch from 3236df4 to f079944 Compare April 14, 2026 00:06

pwnage101 merged commit 35a295f into master Apr 14, 2026
11 checks passed

pwnage101 deleted the pwnage101/ENT-11567 branch April 14, 2026 00:25

		log = logging.getLogger(__name__)

Conversation

pwnage101 commented Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

iloveagent57 Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pwnage101 Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

iloveagent57 left a comment

Choose a reason for hiding this comment

Uh oh!

iloveagent57 Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

iloveagent57 Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

pwnage101 commented Mar 5, 2026 •

edited

Loading

codecov bot commented Apr 8, 2026 •

edited

Loading