Skip to content

Revert "AUT-31 code changes & implementation for redirection from Legacy page to new Authn MFE" #118

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ferdis merged 1 commit into from
Feb 11, 2026
Merged

Revert "AUT-31 code changes & implementation for redirection from Legacy page to new Authn MFE" #118

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lms/envs/common.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,7 @@
# .. toggle_tickets: 'https://github.com/openedx/edx-platform/pull/24908'
# .. toggle_warning: Also set settings.AUTHN_MICROFRONTEND_URL for rollout. This temporary feature
# toggle does not have a target removal date.
ENABLE_AUTHN_MICROFRONTEND = os.getenv("EDXAPP_ENABLE_AUTHN_MFE", "false").lower() == "true"
ENABLE_AUTHN_MICROFRONTEND = os.environ.get("EDXAPP_ENABLE_AUTHN_MFE", False)
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

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

The use of os.environ.get("EDXAPP_ENABLE_AUTHN_MFE", False) is problematic because when the environment variable is set, it returns a string (e.g., "true", "false", "1", "0"), not a boolean. In Python, any non-empty string is truthy, meaning even setting EDXAPP_ENABLE_AUTHN_MFE="false" would evaluate to True.

This differs from the code being reverted which used .lower() == "true" to properly convert the string to a boolean. The current implementation will incorrectly enable the authn microfrontend whenever the environment variable is set to any non-empty value.

Consider using a proper boolean conversion such as:

  • os.getenv("EDXAPP_ENABLE_AUTHN_MFE", "false").lower() == "true"
  • Or a helper function that safely converts string values to booleans
Suggested change
ENABLE_AUTHN_MICROFRONTEND = os.environ.get("EDXAPP_ENABLE_AUTHN_MFE", False)
ENABLE_AUTHN_MICROFRONTEND = os.getenv("EDXAPP_ENABLE_AUTHN_MFE", "false").lower() == "true"

Copilot uses AI. Check for mistakes.

# .. toggle_name: settings.ENABLE_CATALOG_MICROFRONTEND
# .. toggle_implementation: DjangoSetting
Expand Down
4 changes: 0 additions & 4 deletions openedx/core/djangoapps/user_authn/toggles.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,6 @@ def should_redirect_to_authn_microfrontend():
return False
return configuration_helpers.get_value(
'ENABLE_AUTHN_MICROFRONTEND', settings.FEATURES.get('ENABLE_AUTHN_MICROFRONTEND')
) and not (
configuration_helpers.get_value('ENABLE_ENTERPRISE_CUSTOMER', False) and
configuration_helpers.get_value('ENABLE_TPA_HINT_PROVIDER', False) and
configuration_helpers.get_value('ENABLE_SAML_PROVIDER', False)
Comment on lines 24 to -29
Copy link

@robrap robrap Feb 20, 2026

Choose a reason for hiding this comment

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

Here is some example pseudo-code that would check a new additional toggle before entering the new logic.

# example new toggle to protect check of additional conditions
if AUTHN_MFE_ADDITIONAL_SKIP_CONDITIONS.is_enabled():
    If configuration_helpers checks say we should skip:
        return False
return configuration_helpers.get_value(
        'ENABLE_AUTHN_MICROFRONTEND', settings.FEATURES.get('ENABLE_AUTHN_MICROFRONTEND')
)

Note that there were no unit tests for this, which could help ensure you are getting what you wish before getting this to any environment for testing.

)


Expand Down
Loading