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.
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
Remove OIDC values and DOP references. #5635
Conversation
SOCIAL_AUTH_EDX_OIDC_ID_TOKEN_DECRYPTION_KEY: '{{ edx_django_service_with_rendered_config_social_auth_edx_oidc_secret }}' | ||
SOCIAL_AUTH_EDX_OIDC_URL_ROOT: '{{ edx_django_service_with_rendered_config_oidc_url_root }}' | ||
SOCIAL_AUTH_EDX_OIDC_PUBLIC_URL_ROOT: '{{ edx_django_service_with_rendered_config_oidc_public_url_root }}' | ||
SOCIAL_AUTH_REDIRECT_IS_HTTPS: '{{ edx_django_service_with_rendered_config_social_auth_redirect_is_https }}' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this line removal intentional? It's in the middle of a block of OIDC settings (inexplicably) and I don't see it removed in other files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this line should be removed, since it belongs to the same social_auth library right? That means everywhere else this line was not removed, that should be updated to be removed as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's kind of messy, but this is a setting that seems to be used by the underlying social-django
library that we're still using and not the django-oauth2-provider
library that we're trying to remove.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So it sounds like this line should not be removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's re-added a few lines down.
b881761
to
f91dd21
Compare
f91dd21
to
b993978
Compare
Remove OIDC values and DOP references. (cherry picked from commit 34a8add)
https://openedx.atlassian.net/browse/BOM-1215
Internal PR: https://github.com/edx/edx-internal/pull/1497
Edge PR: https://github.com/edx/edge-internal/pull/201
Sandbox PR: https://github.com/edx/sandbox-internal/pull/75
Configuration Pull Request
Make sure that the following steps are done before merging: