Skip to content

fix: gate plugin_settings on ENABLE_ENTERPRISE_INTEGRATION#2610

Merged
pwnage101 merged 1 commit intomasterfrom
pwnage101/ENT-11566-2
May 6, 2026
Merged

fix: gate plugin_settings on ENABLE_ENTERPRISE_INTEGRATION#2610
pwnage101 merged 1 commit intomasterfrom
pwnage101/ENT-11566-2

Conversation

@pwnage101
Copy link
Copy Markdown
Contributor

@pwnage101 pwnage101 commented May 6, 2026

The main behavior change:

Other changes rolled into this commit to simplify testing and prep for making enterprise a truly optional plugin:

  • Migrate enterprise FEATURES-dict reads to top-level settings, matching openedx-platform's ongoing FEATURES flattening/removal.
  • Reorganize enterprise/settings/test.py and invoke plugin_settings (via add_plugins()) to prevent future duplication of test settings once we migrate enterprise-specific settings into this repo.

ENT-11566


The failing test in openedx-platform which prompted this PR:

______________ DOTAccessTokenExchangeFormTestGoogle.test_minimal _______________

self = <openedx.core.djangoapps.auth_exchange.tests.test_forms.DOTAccessTokenExchangeFormTestGoogle testMethod=test_minimal>

    def test_minimal(self):
        self._setup_provider_response(success=True)
>       self._assert_success(self.data, expected_scopes=[])

openedx/core/djangoapps/auth_exchange/tests/utils.py:53: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
openedx/core/djangoapps/auth_exchange/tests/test_forms.py:46: in _assert_success
    assert form.is_valid()
/opt/hostedtoolcache/Python/3.11.15/x64/lib/python3.11/site-packages/django/forms/forms.py:201: in is_valid
    return self.is_bound and not self.errors
/opt/hostedtoolcache/Python/3.11.15/x64/lib/python3.11/site-packages/django/forms/forms.py:196: in errors
    self.full_clean()
/opt/hostedtoolcache/Python/3.11.15/x64/lib/python3.11/site-packages/django/forms/forms.py:434: in full_clean
    self._clean_form()
openedx/core/djangoapps/auth_exchange/forms.py:108: in _clean_form
    super()._clean_form()
/opt/hostedtoolcache/Python/3.11.15/x64/lib/python3.11/site-packages/django/forms/forms.py:455: in _clean_form
    cleaned_data = self.clean()
openedx/core/djangoapps/auth_exchange/forms.py:187: in clean
    user = backend.do_auth(access_token, allow_inactive_user=True)
/opt/hostedtoolcache/Python/3.11.15/x64/lib/python3.11/site-packages/social_core/utils.py:235: in wrapper
    return func(*args, **kwargs)
/opt/hostedtoolcache/Python/3.11.15/x64/lib/python3.11/site-packages/social_core/backends/oauth.py:464: in do_auth
    return self.strategy.authenticate(*args, **kwargs)
/opt/hostedtoolcache/Python/3.11.15/x64/lib/python3.11/site-packages/social_django/strategy.py:104: in authenticate
    return authenticate(*args, **kwargs)
/opt/hostedtoolcache/Python/3.11.15/x64/lib/python3.11/site-packages/django/views/decorators/debug.py:42: in sensitive_variables_wrapper
    return func(*func_args, **func_kwargs)
/opt/hostedtoolcache/Python/3.11.15/x64/lib/python3.11/site-packages/django/contrib/auth/__init__.py:77: in authenticate
    user = backend.authenticate(request, **credentials)
/opt/hostedtoolcache/Python/3.11.15/x64/lib/python3.11/site-packages/social_core/backends/base.py:89: in authenticate
    return self.pipeline(pipeline, *args, **kwargs)
/opt/hostedtoolcache/Python/3.11.15/x64/lib/python3.11/site-packages/social_core/backends/base.py:92: in pipeline
    out = self.run_pipeline(pipeline, pipeline_index, *args, **kwargs)
/opt/hostedtoolcache/Python/3.11.15/x64/lib/python3.11/site-packages/social_core/backends/base.py:124: in run_pipeline
    result = func(*args, **out) or {}
/opt/hostedtoolcache/Python/3.11.15/x64/lib/python3.11/site-packages/social_core/pipeline/partial.py:34: in wrapper
    func(
/opt/hostedtoolcache/Python/3.11.15/x64/lib/python3.11/site-packages/enterprise/tpa_pipeline.py:171: in handle_enterprise_logistration
    enterprise_customer = get_enterprise_customer_for_running_pipeline(request, pipeline)
/opt/hostedtoolcache/Python/3.11.15/x64/lib/python3.11/site-packages/enterprise/tpa_pipeline.py:124: in get_enterprise_customer_for_running_pipeline
    return get_enterprise_customer_for_sso(get_sso_provider(request, pipeline))
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

request = <WSGIRequest: POST '/dummy_url'>
pipeline = {'backend': 'google-oauth2', 'kwargs': {'allow_inactive_user': True, 'auth_entry': 'login_api', 'current_partial': <Pa... object (None)>, 'details': {'email': 'test_social_uid', 'first_name': '', 'fullname': '', 'last_name': '', ...}, ...}}

    def get_sso_provider(request, pipeline):
        """
        Helper method to retrieve the sso provider ID from either a user's SSO login request
        """
        sso_provider_id = request.GET.get('tpa_hint')
        if pipeline:
>           sso_provider_id = Registry.get_from_pipeline(pipeline).provider_id
E           AttributeError: 'NoneType' object has no attribute 'provider_id'

/opt/hostedtoolcache/Python/3.11.15/x64/lib/python3.11/site-packages/enterprise/tpa_pipeline.py:116: AttributeError

@codecov
Copy link
Copy Markdown

codecov Bot commented May 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.01%. Comparing base (6d917c5) to head (4c0c0b0).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2610   +/-   ##
=======================================
  Coverage   86.01%   86.01%           
=======================================
  Files         249      249           
  Lines       16674    16673    -1     
  Branches     1655     1655           
=======================================
  Hits        14342    14342           
+ Misses       1998     1997    -1     
  Partials      334      334           
Flag Coverage Δ
unittests 86.01% <100.00%> (+<0.01%) ⬆️

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.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR restores the prior behavior of only injecting enterprise social-auth pipeline steps when enterprise integration is enabled, avoiding regressions in contexts (notably Studio/CMS) that should have no enterprise pipeline steps. It also continues a broader move away from settings.FEATURES[...] reads toward top-level Django settings flags, and updates enterprise test settings to invoke plugin settings via the Open edX plugin mechanism.

Changes:

  • Gate enterprise.settings.common.plugin_settings() (and related runtime hooks) on ENABLE_ENTERPRISE_INTEGRATION to avoid injecting enterprise pipeline steps when integration is disabled.
  • Migrate several settings reads from settings.FEATURES[...] to top-level settings attributes and update affected tests accordingly.
  • Reorganize enterprise/settings/test.py to apply plugin settings via add_plugins() and reduce duplication as enterprise becomes a more optional plugin.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
enterprise/settings/common.py Adds an early gate on ENABLE_ENTERPRISE_INTEGRATION before injecting social-auth pipeline steps.
enterprise/tpa_pipeline.py Switches enterprise pipeline-step gating to top-level ENABLE_ENTERPRISE_INTEGRATION.
enterprise/signals.py Switches signal-handler gating to top-level ENABLE_ENTERPRISE_INTEGRATION.
enterprise/admin/__init__.py Switches a former FEATURES-flag read to a top-level setting for admin delete permission.
enterprise/settings/test.py Refactors test settings: defines a base (enterprise-free) pipeline, enables enterprise integration, and applies plugin settings via add_plugins().
tests/test_settings.py Updates plugin_settings tests to include ENABLE_ENTERPRISE_INTEGRATION and adds a no-op test when disabled.
tests/test_tpa_pipeline.py Updates override_settings usage to the top-level integration flag.
tests/test_enterprise/test_signals.py Updates override_settings usage to the top-level integration flag.
consent/settings/common.py Updates docstring example away from settings.FEATURES[...].
enterprise/__init__.py Bumps package version to 8.0.6.
CHANGELOG.rst Adds the 8.0.6 release note entry.

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

Comment thread enterprise/settings/common.py
Comment thread enterprise/settings/test.py Outdated
Comment thread enterprise/admin/__init__.py
Comment thread tests/test_settings.py
Comment thread enterprise/tpa_pipeline.py
Comment thread enterprise/signals.py
@pwnage101 pwnage101 force-pushed the pwnage101/ENT-11566-2 branch from db9448b to 0916f16 Compare May 6, 2026 01:36
The main behavior change:
- Skip social auth pipeline injection when enterprise integration
  is disabled.
  - This fixes a broken CMS test in the edxapp which expects NO
    enterprise steps in the social auth pipeline.

Other changes rolled into this commit to simplify testing and prep for
making enterprise a truly optional plugin:

- Migrate enterprise FEATURES-dict reads to top-level settings,
  matching openedx-platform's ongoing FEATURES flattening/removal.
- Reorganize enterprise/settings/test.py and invoke plugin_settings
  (via add_plugins()) to prevent future duplication of test settings
  once we migrate enterprise-specific settings into this repo.

ENT-11566

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@pwnage101 pwnage101 force-pushed the pwnage101/ENT-11566-2 branch from 0916f16 to 4c0c0b0 Compare May 6, 2026 01:40
@pwnage101 pwnage101 merged commit 585472a into master May 6, 2026
11 checks passed
@pwnage101 pwnage101 deleted the pwnage101/ENT-11566-2 branch May 6, 2026 02:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants