Backport addition of THIRD_PARTY_AUTH_BACKENDS variable - #136
Conversation
shimulch
left a comment
There was a problem hiding this comment.
LGTM 👍
- I have compared with upstream PR#5958. It's just a cherry-pick commit.
- I read through the code
- [N/A] I checked for accessibility issues
- [N/A] Includes documentation
- [N/A ] I made sure any change in configuration variables is reflected in the corresponding client's
configuration-securerepository.
| - social_core.backends.linkedin.LinkedinOAuth2 | ||
| - social_core.backends.facebook.FacebookOAuth2 | ||
| - social_core.backends.azuread.AzureADOAuth2 | ||
| - third_party_auth.appleid.AppleIdAuth |
There was a problem hiding this comment.
Hi @kaizoku -- this breaks juniper.3 deployments because third_party_auth.appleid.AppleIdAuth doesn't exist in opencraft-release/juniper.3 (nor in upstream open-release/juniper.3, it's only in master).
The default settings should suffice if there's nothing set for EDXAPP_THIRD_PARTY_AUTH_BACKENDS though, so not sure why they're duplicated here?
There was a problem hiding this comment.
Ach bummer, I tried using EDXAPP_THIRD_PARTY_AUTH_BACKENDS: !!null to see if it would fallback to the defaults in the code, but it doesn't work:
File "/edx/app/edxapp/edx-platform/lms/envs/production.py", line 630, in <module>
AUTHENTICATION_BACKENDS = list(tmp_backends) + list(AUTHENTICATION_BACKENDS)
TypeError: 'NoneType' object is not iterable
We could have achieved the same effect by adding THIRD_PARTY_AUTH_BACKENDS to EDXAPP_LMS_ENV_EXTRA and/or EDXAPP_CMS_ENV_EXTRA instead of adding a new variable here. But since it's already upstreamed and we're stuck with it, could you submit an internal PR to remove this third_party_auth.appleid.AppleIdAuth line from opencraft-release/juniper.3?
This is blocking redeployments of juniper.3 instances.
There was a problem hiding this comment.
Thanks for investigating this @pomegranited. #140 has the fix.
It was pointed out to me that we could use EDXAPP_LMS_ENV_EXTRA unfortunately after we had opened the upstream PR, and they merged this upstream fairly quickly. Also as you found we had to duplicate the default variables since setting anything else would override the defaults.
Configuration Pull Request
This backports changes from https://github.com/edx/configuration/pull/5958/ to add the configuration variable
THIRD_PARTY_AUTH_BACKENDSto the edxapp role.