From d08cd9ce04b219ad4d8fc9603b6856ad3771a1c4 Mon Sep 17 00:00:00 2001 From: Manjinder Singh <49171515+jinder1s@users.noreply.github.com> Date: Mon, 2 Mar 2020 08:56:54 -0500 Subject: [PATCH] Removing provider imports from edx-platform (#23229) * Removing from provider imports from openedx * removed all uses of retire_dop_oauth2_models * Removing provider library from lms, common, and cms Created/copied function short_token(from django-oauth-provider) and create_hash256 to help with conversion --- common/djangoapps/third_party_auth/models.py | 4 +- lms/djangoapps/lti_provider/models.py | 2 +- .../accounts/tests/test_retirement_views.py | 4 +- .../djangoapps/user_api/accounts/views.py | 3 +- .../djangoapps/user_authn/views/logout.py | 6 +-- openedx/core/djangoapps/video_pipeline/api.py | 4 +- .../djangoapps/video_pipeline/tests/mixins.py | 13 +++---- .../core/djangolib/oauth2_retirement_utils.py | 10 ----- .../tests/test_oauth2_retirement_utils.py | 37 ------------------- openedx/core/lib/api/authentication.py | 27 -------------- .../core/lib/api/tests/test_authentication.py | 11 ------ openedx/core/lib/hash_utils.py | 34 +++++++++++++++++ 12 files changed, 50 insertions(+), 105 deletions(-) create mode 100644 openedx/core/lib/hash_utils.py diff --git a/common/djangoapps/third_party_auth/models.py b/common/djangoapps/third_party_auth/models.py index d8bc94dfb8..48a29ffa61 100644 --- a/common/djangoapps/third_party_auth/models.py +++ b/common/djangoapps/third_party_auth/models.py @@ -17,7 +17,6 @@ from django.db import models from django.utils import timezone from django.utils.translation import ugettext_lazy as _ from organizations.models import Organization -from django.contrib.auth.tokens import default_token_generator from social_core.backends.base import BaseAuth from social_core.backends.oauth import OAuthAuth from social_core.backends.saml import SAMLAuth @@ -26,6 +25,7 @@ from social_core.utils import module_member from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.theming.helpers import get_current_request +from openedx.core.lib.hash_utils import create_hash256 from .lti import LTI_PARAMS_KEY, LTIAuthBackend from .saml import STANDARD_SAML_PROVIDER_KEY, get_saml_idp_choices, get_saml_idp_class @@ -825,7 +825,7 @@ class LTIProviderConfig(ProviderConfig): ) lti_consumer_secret = models.CharField( - default=default_token_generator, + default=create_hash256, max_length=255, help_text=( u'The shared secret that the LTI Tool Consumer will use to ' diff --git a/lms/djangoapps/lti_provider/models.py b/lms/djangoapps/lti_provider/models.py index a0ecb06a5c..0d1f0a6c9b 100644 --- a/lms/djangoapps/lti_provider/models.py +++ b/lms/djangoapps/lti_provider/models.py @@ -15,9 +15,9 @@ import logging from django.contrib.auth.models import User from django.db import models from opaque_keys.edx.django.models import CourseKeyField, UsageKeyField -from provider.utils import short_token from openedx.core.djangolib.fields import CharNullField +from openedx.core.lib.hash_utils import short_token log = logging.getLogger("edx.lti_provider") diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_retirement_views.py b/openedx/core/djangoapps/user_api/accounts/tests/test_retirement_views.py index 64da8f4b80..99b6a3b58b 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_retirement_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_retirement_views.py @@ -201,8 +201,7 @@ class TestDeactivateLogout(RetirementTestCase): return {'password': password} @mock.patch('openedx.core.djangoapps.user_api.accounts.views.retire_dot_oauth2_models') - @mock.patch('openedx.core.djangoapps.user_api.accounts.views.retire_dop_oauth2_models') - def test_user_can_deactivate_self(self, mock_retire_dop, mock_retire_dot): + def test_user_can_deactivate_self(self, mock_retire_dot): """ Verify a user calling the deactivation endpoint logs out the user, deletes all their SSO tokens, and creates a user retirement row. @@ -219,7 +218,6 @@ class TestDeactivateLogout(RetirementTestCase): self.assertEqual(list(Registration.objects.filter(user=self.test_user)), []) self.assertEqual(len(UserRetirementStatus.objects.filter(user_id=self.test_user.id)), 1) # these retirement utils are tested elsewhere; just make sure we called them - mock_retire_dop.assert_called_with(self.test_user) mock_retire_dot.assert_called_with(self.test_user) # make sure the user cannot log in self.assertFalse(self.client.login(username=self.test_user.username, password=self.test_password)) diff --git a/openedx/core/djangoapps/user_api/accounts/views.py b/openedx/core/djangoapps/user_api/accounts/views.py index 643c3ae22c..c9ce1f3492 100644 --- a/openedx/core/djangoapps/user_api/accounts/views.py +++ b/openedx/core/djangoapps/user_api/accounts/views.py @@ -48,7 +48,7 @@ from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY from openedx.core.djangoapps.profile_images.images import remove_profile_images from openedx.core.djangoapps.user_api.accounts.image_helpers import get_profile_image_names, set_has_profile_image from openedx.core.djangoapps.user_authn.exceptions import AuthFailedError -from openedx.core.djangolib.oauth2_retirement_utils import retire_dop_oauth2_models, retire_dot_oauth2_models +from openedx.core.djangolib.oauth2_retirement_utils import retire_dot_oauth2_models from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser from openedx.core.lib.api.parsers import MergePatchParser from student.models import ( @@ -421,7 +421,6 @@ class DeactivateLogoutView(APIView): Registration.objects.filter(user=request.user).delete() # Delete OAuth tokens associated with the user. - retire_dop_oauth2_models(request.user) retire_dot_oauth2_models(request.user) AccountRecovery.retire_recovery_email(request.user.id) diff --git a/openedx/core/djangoapps/user_authn/views/logout.py b/openedx/core/djangoapps/user_authn/views/logout.py index 96e6f39a44..3cd8ffe697 100644 --- a/openedx/core/djangoapps/user_authn/views/logout.py +++ b/openedx/core/djangoapps/user_authn/views/logout.py @@ -8,7 +8,7 @@ from django.conf import settings from django.contrib.auth import logout from django.utils.http import urlencode from django.views.generic import TemplateView -from provider.oauth2.models import Client +from oauth2_provider.models import Application from six.moves.urllib.parse import parse_qs, urlsplit, urlunsplit # pylint: disable=import-error from openedx.core.djangoapps.user_authn.cookies import delete_logged_in_cookies @@ -130,8 +130,8 @@ class LogoutView(TemplateView): # Add the logout URIs for IDAs that the user was logged into (according to the session). This line is specific # to DOP. - uris += Client.objects.filter(client_id__in=self.oauth_client_ids, - logout_uri__isnull=False).values_list('logout_uri', flat=True) + uris += Application.objects.filter(client_id__in=self.oauth_client_ids, + redirect_uris__isnull=False).values_list('redirect_uris', flat=True) # Add the extra logout URIs from settings. This is added as a stop-gap solution for sessions that were # established via DOT. diff --git a/openedx/core/djangoapps/video_pipeline/api.py b/openedx/core/djangoapps/video_pipeline/api.py index 81083bbee6..74a4c13bc6 100644 --- a/openedx/core/djangoapps/video_pipeline/api.py +++ b/openedx/core/djangoapps/video_pipeline/api.py @@ -6,7 +6,7 @@ import json import logging from django.core.exceptions import ObjectDoesNotExist -from provider.oauth2.models import Client +from oauth2_provider.models import Application from slumber.exceptions import HttpClientError from openedx.core.djangoapps.video_pipeline.models import VideoPipelineIntegration @@ -32,7 +32,7 @@ def update_3rd_party_transcription_service_credentials(**credentials_payload): if pipeline_integration.enabled: try: video_pipeline_user = pipeline_integration.get_service_user() - oauth_client = Client.objects.get(name=pipeline_integration.client_name) + oauth_client = Application.objects.get(name=pipeline_integration.client_name) except ObjectDoesNotExist: return error_response, is_updated diff --git a/openedx/core/djangoapps/video_pipeline/tests/mixins.py b/openedx/core/djangoapps/video_pipeline/tests/mixins.py index 46cb01ec99..f0ebe0d33e 100644 --- a/openedx/core/djangoapps/video_pipeline/tests/mixins.py +++ b/openedx/core/djangoapps/video_pipeline/tests/mixins.py @@ -2,8 +2,7 @@ Mixins to test video pipeline integration. """ -from provider.constants import CONFIDENTIAL -from provider.oauth2.models import Client +from oauth2_provider.models import Application from openedx.core.djangoapps.video_pipeline.models import VideoPipelineIntegration @@ -19,14 +18,14 @@ class VideoPipelineIntegrationMixin(object): 'client_name': 'video_pipeline' } + request_uris = 'https://video-pipeline.example.com/api/v1/logout' + request_uris += ' https://video-pipeline.example.com/api/v1/redirect' video_pipelien_oauth_client_defaults = { 'name': 'video_pipeline', - 'url': 'https://video-pipeline.example.com/api/v1/', - 'redirect_uri': 'https://video-pipeline.example.com/api/v1/redirect', - 'logout_uri': 'https://video-pipeline.example.com/api/v1/logout', + 'redirect_uris': request_uris, 'client_id': 'video_pipeline_client_id', 'client_secret': 'video_pipeline_client_secret', - 'client_type': CONFIDENTIAL + 'client_type': Application.CLIENT_CONFIDENTIAL } def create_video_pipeline_integration(self, **kwargs): @@ -44,4 +43,4 @@ class VideoPipelineIntegrationMixin(object): """ fields = dict(self.video_pipelien_oauth_client_defaults, **kwargs) fields['user'] = user - return Client.objects.create(**fields) + return Application.objects.create(**fields) diff --git a/openedx/core/djangolib/oauth2_retirement_utils.py b/openedx/core/djangolib/oauth2_retirement_utils.py index f1577695f6..7b0c541e25 100644 --- a/openedx/core/djangolib/oauth2_retirement_utils.py +++ b/openedx/core/djangolib/oauth2_retirement_utils.py @@ -9,11 +9,6 @@ from oauth2_provider.models import ( Grant as DOTGrant, RefreshToken as DOTRefreshToken, ) -from provider.oauth2.models import ( - AccessToken as DOPAccessToken, - RefreshToken as DOPRefreshToken, - Grant as DOPGrant, -) class ModelRetirer(object): @@ -46,8 +41,3 @@ class ModelRetirer(object): def retire_dot_oauth2_models(user): dot_models = [DOTAccessToken, DOTApplication, DOTGrant, DOTRefreshToken] ModelRetirer(dot_models).retire_user_by_id(user.id) - - -def retire_dop_oauth2_models(user): - dop_models = [DOPAccessToken, DOPGrant, DOPRefreshToken] - ModelRetirer(dop_models).retire_user_by_id(user.id) diff --git a/openedx/core/djangolib/tests/test_oauth2_retirement_utils.py b/openedx/core/djangolib/tests/test_oauth2_retirement_utils.py index 8316bc98e4..01fd8f922e 100644 --- a/openedx/core/djangolib/tests/test_oauth2_retirement_utils.py +++ b/openedx/core/djangolib/tests/test_oauth2_retirement_utils.py @@ -14,15 +14,8 @@ from oauth2_provider.models import ( RefreshToken as DOTRefreshToken, Grant as DOTGrant, ) -from provider.oauth2.models import ( - AccessToken as DOPAccessToken, - RefreshToken as DOPRefreshToken, - Grant as DOPGrant, - Client as DOPClient, -) from ..oauth2_retirement_utils import ( - retire_dop_oauth2_models, retire_dot_oauth2_models, ) @@ -58,33 +51,3 @@ class RetireDOTModelsTest(TestCase): for query_set in query_sets: self.assertFalse(query_set.exists()) - - -class RetireDOPModelsTest(TestCase): - - def test_delete_dop_models(self): - user = UserFactory.create() - client = DOPClient.objects.create( - user=user, - client_type=1, - ) - access_token = DOPAccessToken.objects.create( - user=user, - client=client, - ) - DOPRefreshToken.objects.create( - user=user, - client=client, - access_token=access_token, - ) - - retire_dop_oauth2_models(user) - - access_tokens = DOPAccessToken.objects.filter(user=user) - refresh_tokens = DOPRefreshToken.objects.filter(user=user) - grants = DOPGrant.objects.filter(user_id=user.id) - - query_sets = [access_tokens, refresh_tokens, grants] - - for query_set in query_sets: - self.assertFalse(query_set.exists()) diff --git a/openedx/core/lib/api/authentication.py b/openedx/core/lib/api/authentication.py index 6abd24c62e..7f71fd8994 100644 --- a/openedx/core/lib/api/authentication.py +++ b/openedx/core/lib/api/authentication.py @@ -4,7 +4,6 @@ import logging import django.utils.timezone from oauth2_provider import models as dot_models -from provider.oauth2 import models as dop_models from rest_framework.exceptions import AuthenticationFailed from rest_framework.authentication import BaseAuthentication, get_authorization_header from edx_django_utils.monitoring import set_custom_metric @@ -105,32 +104,6 @@ class BearerAuthentication(BaseAuthentication): return user, token def get_access_token(self, access_token): - """ - Return a valid access token that exists in one of our OAuth2 libraries, - or None if no matching token is found. - """ - dot_token_return = self._get_dot_token(access_token) - if dot_token_return is not None: - set_custom_metric('BearerAuthentication_token_type', 'dot') - return dot_token_return - - dop_token_return = self._get_dop_token(access_token) - if dop_token_return is not None: - set_custom_metric('BearerAuthentication_token_type', 'dop') - return dop_token_return - - set_custom_metric('BearerAuthentication_token_type', 'None') - return None - - def _get_dop_token(self, access_token): - """ - Return a valid access token stored by django-oauth2-provider (DOP), or - None if no matching token is found. - """ - token_query = dop_models.AccessToken.objects.select_related('user') - return token_query.filter(token=access_token).first() - - def _get_dot_token(self, access_token): """ Return a valid access token stored by django-oauth-toolkit (DOT), or None if no matching token is found. diff --git a/openedx/core/lib/api/tests/test_authentication.py b/openedx/core/lib/api/tests/test_authentication.py index b0cc01dd1a..0c973e94c3 100644 --- a/openedx/core/lib/api/tests/test_authentication.py +++ b/openedx/core/lib/api/tests/test_authentication.py @@ -24,11 +24,9 @@ from rest_framework import status from rest_framework.permissions import IsAuthenticated from rest_framework.test import APIClient, APIRequestFactory from rest_framework.views import APIView -import provider as oauth2_provider from openedx.core.djangoapps.oauth_dispatch import adapters from openedx.core.lib.api import authentication -from provider import constants, scope factory = APIRequestFactory() # pylint: disable=invalid-name @@ -98,14 +96,6 @@ class OAuth2AllowInActiveUsersTests(TestCase): self.user.is_active = False self.user.save() - # This is the a change we've made from the django-rest-framework-oauth version - # of these tests. - # Override the SCOPE_NAME_DICT setting for tests for oauth2-with-scope-test. This is - # needed to support READ and WRITE scopes as they currently aren't supported by the - # edx-auth2-provider, and their scope values collide with other scopes defined in the - # edx-auth2-provider. - scope.SCOPE_NAME_DICT = {'read': constants.READ, 'write': constants.WRITE} - def _create_authorization_header(self, token=None): if token is None: token = self.dot_access_token.token @@ -138,7 +128,6 @@ class OAuth2AllowInActiveUsersTests(TestCase): self.assertEqual(response_dict['error_code'], error_code) @ddt.data(None, {}) - @unittest.skipUnless(oauth2_provider, 'django-oauth2-provider not installed') def test_get_form_with_wrong_authorization_header_token_type_failing(self, params): """Ensure that a wrong token type lead to the correct HTTP error status code""" response = self.csrf_client.get( diff --git a/openedx/core/lib/hash_utils.py b/openedx/core/lib/hash_utils.py new file mode 100644 index 0000000000..703e2a2824 --- /dev/null +++ b/openedx/core/lib/hash_utils.py @@ -0,0 +1,34 @@ +""" +Utilities related to hashing + +This duplicates functionality in django-oauth-provider, +specifically long_token and short token functions which was used to create +random tokens +""" +import hashlib +import shortuuid +from django.utils.encoding import force_bytes +from django.conf import settings + + +def create_hash256(max_length=None): + """ + Generate a hash that can be used as an application secret + Warning: this is not sufficiently secure for tasks like encription + Currently, this is just meant to create sufficiently random tokens + """ + hash_object = hashlib.sha256(force_bytes(shortuuid.uuid())) + hash_object.update(force_bytes(settings.SECRET_KEY)) + output_hash = hash_object.hexdigest() + if max_length is not None and len(output_hash) > max_length: + return output_hash[:max_length] + return output_hash + + +def short_token(): + """ + Generates a hash of length 32 + Warning: this is not sufficiently secure for tasks like encription + Currently, this is just meant to create sufficiently random tokens + """ + return create_hash256(max_length=32)