From 953b0e728bda73a6a8aa6bcf470e50a3c4344d3e Mon Sep 17 00:00:00 2001
From: Manjinder Singh <49171515+jinder1s@users.noreply.github.com>
Date: Thu, 27 Feb 2020 15:42:09 -0500
Subject: [PATCH] Remove ThirdPartyAuthProviderApiPermission (#23195)
* Remove ThirdPartyAuthProviderApiPermission
Also removed ProviderApiPermissions and ApiPermissionsAdminForm and removal of DOP for third_party_auth
* Removing model
* Replaced long_token with default_token_generator
* Adding skip to test_migrations_are_in_sync
---
common/djangoapps/third_party_auth/admin.py | 28 +---------
.../third_party_auth/api/permissions.py | 30 +----------
.../api/tests/test_permissions.py | 44 +---------------
.../third_party_auth/api/tests/test_views.py | 52 +++++++------------
common/djangoapps/third_party_auth/models.py | 27 +---------
.../third_party_auth/tests/testutil.py | 16 ++----
.../third_party_auth/tests/utils.py | 7 ++-
common/djangoapps/util/tests/test_db.py | 5 +-
8 files changed, 36 insertions(+), 173 deletions(-)
diff --git a/common/djangoapps/third_party_auth/admin.py b/common/djangoapps/third_party_auth/admin.py
index 6ec3ce73b04..bbb4c5ea16d 100644
--- a/common/djangoapps/third_party_auth/admin.py
+++ b/common/djangoapps/third_party_auth/admin.py
@@ -7,19 +7,17 @@ Admin site configuration for third party authentication
from config_models.admin import KeyedConfigurationModelAdmin
from django import forms
from django.contrib import admin
-from django.db import DatabaseError, transaction
+from django.db import transaction
from django.urls import reverse
from django.utils.translation import ugettext_lazy as _
from openedx.core.djangolib.markup import HTML
-from third_party_auth.provider import Registry
from .models import (
_PSA_OAUTH2_BACKENDS,
_PSA_SAML_BACKENDS,
LTIProviderConfig,
OAuth2ProviderConfig,
- ProviderApiPermissions,
SAMLConfiguration,
SAMLProviderConfig,
SAMLProviderData
@@ -199,27 +197,3 @@ class LTIProviderConfigAdmin(KeyedConfigurationModelAdmin):
)
admin.site.register(LTIProviderConfig, LTIProviderConfigAdmin)
-
-
-class ApiPermissionsAdminForm(forms.ModelForm):
- """ Django admin form for ApiPermissions model """
- class Meta(object):
- model = ProviderApiPermissions
- fields = ['client', 'provider_id']
-
- provider_id = forms.ChoiceField(choices=[], required=True)
-
- def __init__(self, *args, **kwargs):
- super(ApiPermissionsAdminForm, self).__init__(*args, **kwargs)
- self.fields['provider_id'].choices = (
- (provider.provider_id, u"{} ({})".format(provider.name, provider.provider_id))
- for provider in Registry.enabled()
- )
-
-
-class ApiPermissionsAdmin(admin.ModelAdmin):
- """ Django Admin class for ApiPermissions """
- list_display = ('client', 'provider_id')
- form = ApiPermissionsAdminForm
-
-admin.site.register(ProviderApiPermissions, ApiPermissionsAdmin)
diff --git a/common/djangoapps/third_party_auth/api/permissions.py b/common/djangoapps/third_party_auth/api/permissions.py
index 0178fa0a550..91f5e9fa710 100644
--- a/common/djangoapps/third_party_auth/api/permissions.py
+++ b/common/djangoapps/third_party_auth/api/permissions.py
@@ -4,7 +4,6 @@ Third party auth API related permissions
import logging
-from edx_django_utils.monitoring import set_custom_metric
from edx_rest_framework_extensions.auth.jwt.decoder import decode_jwt_filters
from edx_rest_framework_extensions.permissions import (
IsSuperuser,
@@ -14,39 +13,12 @@ from edx_rest_framework_extensions.permissions import (
)
from rest_condition import C
from rest_framework.permissions import BasePermission
-from third_party_auth.models import ProviderApiPermissions
from openedx.core.lib.api.permissions import ApiKeyHeaderPermission
log = logging.getLogger(__name__)
-class ThirdPartyAuthProviderApiPermission(BasePermission):
- """
- Allow someone to access the view if they have valid OAuth client credential.
-
- Deprecated: Only works for DOP oauth applications. To be removed as part of DOPrecation.
-
- """
- def has_permission(self, request, view):
- """
- Check if the OAuth client associated with auth token in current request has permission to access
- the information for provider
- """
- provider_id = view.kwargs.get('provider_id')
- if not request.auth or not provider_id:
- # doesn't have access token or no provider_id specified
- return False
-
- try:
- ProviderApiPermissions.objects.get(client__pk=request.auth.client_id, provider_id=provider_id)
- except ProviderApiPermissions.DoesNotExist:
- return False
-
- set_custom_metric('deprecated_ThirdPartyAuthProviderApiPermission', True)
- return True
-
-
class JwtHasTpaProviderFilterForRequestedProvider(BasePermission):
"""
Ensures the JWT used to authenticate contains the appropriate tpa_provider
@@ -79,7 +51,7 @@ class JwtHasTpaProviderFilterForRequestedProvider(BasePermission):
# TODO: Remove ApiKeyHeaderPermission. Check deprecated_api_key_header custom metric for active usage.
_NOT_JWT_RESTRICTED_TPA_PERMISSIONS = (
C(NotJwtRestrictedApplication) &
- (C(IsSuperuser) | ApiKeyHeaderPermission | ThirdPartyAuthProviderApiPermission)
+ (C(IsSuperuser) | ApiKeyHeaderPermission)
)
_JWT_RESTRICTED_TPA_PERMISSIONS = (
C(JwtRestrictedApplication) &
diff --git a/common/djangoapps/third_party_auth/api/tests/test_permissions.py b/common/djangoapps/third_party_auth/api/tests/test_permissions.py
index 8011589c8a9..8dccb48aed4 100644
--- a/common/djangoapps/third_party_auth/api/tests/test_permissions.py
+++ b/common/djangoapps/third_party_auth/api/tests/test_permissions.py
@@ -10,58 +10,18 @@ from django.conf import settings
from django.test import RequestFactory, TestCase
from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication
from edx_rest_framework_extensions.auth.jwt.tests.utils import generate_jwt
-from mock import Mock, patch
+from mock import patch
from rest_framework.authentication import SessionAuthentication
from rest_framework.response import Response
-from rest_framework.test import APITestCase
from rest_framework.views import APIView
from student.tests.factories import UserFactory
-from third_party_auth.api.permissions import ThirdPartyAuthProviderApiPermission, TPA_PERMISSIONS
-from third_party_auth.tests.testutil import ThirdPartyAuthTestMixin
+from third_party_auth.api.permissions import TPA_PERMISSIONS
IDP_SLUG_TESTSHIB = 'testshib'
PROVIDER_ID_TESTSHIB = 'saml-' + IDP_SLUG_TESTSHIB
-@ddt.ddt
-@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
-class ThirdPartyAuthApiPermissionTest(ThirdPartyAuthTestMixin, APITestCase):
- """ Tests for third party auth API permission """
-
- @ddt.data(
- (1, PROVIDER_ID_TESTSHIB, True),
- (1, 'invalid-provider-id', False),
- (999, PROVIDER_ID_TESTSHIB, False),
- (999, 'invalid-provider-id', False),
- (1, None, False),
- )
- @ddt.unpack
- def test_api_permission(self, client_pk, provider_id, expect):
- dop_client = self.configure_oauth_dop_client()
- self.configure_api_permission(dop_client, PROVIDER_ID_TESTSHIB)
-
- request = Mock()
- request.auth = Mock()
- request.auth.client_id = client_pk
- view = Mock(kwargs={'provider_id': provider_id})
-
- result = ThirdPartyAuthProviderApiPermission().has_permission(request, view)
- self.assertEqual(result, expect)
-
- def test_api_permission_unauthorized_client(self):
- dop_client = self.configure_oauth_dop_client()
- self.configure_api_permission(dop_client, 'saml-anotherprovider')
-
- request = Mock()
- request.auth = Mock()
- request.auth.client_id = dop_client.pk
- view = Mock(kwargs={'provider_id': PROVIDER_ID_TESTSHIB})
-
- result = ThirdPartyAuthProviderApiPermission().has_permission(request, view)
- self.assertEqual(result, False)
-
-
@ddt.ddt
@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
class ThirdPartyAuthPermissionTest(TestCase):
diff --git a/common/djangoapps/third_party_auth/api/tests/test_views.py b/common/djangoapps/third_party_auth/api/tests/test_views.py
index d79b7eac6e2..820946c75be 100644
--- a/common/djangoapps/third_party_auth/api/tests/test_views.py
+++ b/common/djangoapps/third_party_auth/api/tests/test_views.py
@@ -12,20 +12,16 @@ from django.http import QueryDict
from django.test.utils import override_settings
from django.urls import reverse
from mock import patch
-from provider.constants import CONFIDENTIAL
-from provider.oauth2.models import AccessToken, Client
from rest_framework.test import APITestCase
from six.moves import range
from social_django.models import UserSocialAuth
-from openedx.core.lib.api.permissions import ApiKeyHeaderPermission
from student.tests.factories import UserFactory
-from third_party_auth.api.permissions import ThirdPartyAuthProviderApiPermission
-from third_party_auth.models import ProviderApiPermissions
from third_party_auth.tests.testutil import ThirdPartyAuthTestMixin
from third_party_auth.api.permissions import (JwtRestrictedApplication,
JwtHasScope,
JwtHasTpaProviderFilterForRequestedProvider)
+from edx_rest_framework_extensions.auth.jwt.tests.utils import generate_jwt
VALID_API_KEY = "i am a key"
IDP_SLUG_TESTSHIB = 'testshib'
@@ -247,25 +243,29 @@ class UserMappingViewAPITests(TpaAPITestCase):
response = self.client.get(url, HTTP_X_EDX_API_KEY=api_key)
self._verify_response(response, expect_code, expect_data)
+ def _create_jwt_header(self, user, is_restricted=False, scopes=None, filters=None):
+ token = generate_jwt(user, is_restricted=is_restricted, scopes=scopes, filters=filters)
+ return "JWT {}".format(token)
+
@ddt.data(
- (PROVIDER_ID_TESTSHIB, 'valid-token', 200, get_mapping_data_by_usernames(LINKED_USERS)),
- ('non-existing-id', 'valid-token', 404, []),
- (PROVIDER_ID_TESTSHIB, 'invalid-token', 401, []),
+ (True, 200, get_mapping_data_by_usernames(LINKED_USERS)),
+ (False, 401, []),
)
@ddt.unpack
- def test_list_all_user_mappings_oauth2(self, provider_id, access_token, expect_code, expect_data):
- url = reverse('third_party_auth_user_mapping_api', kwargs={'provider_id': provider_id})
+ def test_list_all_user_mappings_oauth2(self, valid_call, expect_code, expect_data):
+ url = reverse('third_party_auth_user_mapping_api', kwargs={'provider_id': PROVIDER_ID_TESTSHIB})
+ provider_filter = 'tpa_provider:' + PROVIDER_ID_TESTSHIB
+ filters = [provider_filter, 'tpa_provider:another_tpa_provider']
# create oauth2 auth data
user = UserFactory.create(username='api_user')
- client = Client.objects.create(name='oauth2_client', client_type=CONFIDENTIAL)
- token = AccessToken.objects.create(user=user, client=client)
- ProviderApiPermissions.objects.create(client=client, provider_id=provider_id)
-
- if access_token == 'valid-token':
- access_token = token.token
-
- response = self.client.get(url, HTTP_AUTHORIZATION=u'Bearer {}'.format(access_token))
- self._verify_response(response, expect_code, expect_data)
+ if valid_call:
+ auth_header = self._create_jwt_header(user, is_restricted=True, scopes=['tpa:read'], filters=filters)
+ else:
+ auth_header = ''
+ with patch('edx_rest_framework_extensions.permissions.waffle.switch_is_active') as mock_toggle:
+ mock_toggle.return_value = True
+ response = self.client.get(url, HTTP_AUTHORIZATION=auth_header)
+ self._verify_response(response, expect_code, expect_data)
@ddt.data(
({'username': [ALICE_USERNAME, STAFF_USERNAME]}, 200,
@@ -335,20 +335,6 @@ class UserMappingViewAPITests(TpaAPITestCase):
self.assertEqual(response.status_code, 200)
self._verify_response(response, 200, get_mapping_data_by_usernames(LINKED_USERS))
- @ddt.data(
- (True, True, 200),
- (False, True, 200),
- (True, False, 200),
- (False, False, 401)
- )
- @ddt.unpack
- def test_user_mapping_permission_logic(self, api_key_permission, token_permission, expect):
- url = reverse('third_party_auth_user_mapping_api', kwargs={'provider_id': PROVIDER_ID_TESTSHIB})
- with patch.object(ApiKeyHeaderPermission, 'has_permission', return_value=api_key_permission):
- with patch.object(ThirdPartyAuthProviderApiPermission, 'has_permission', return_value=token_permission):
- response = self.client.get(url)
- self.assertEqual(response.status_code, expect)
-
@ddt.data(
(True, 200),
(False, 401),
diff --git a/common/djangoapps/third_party_auth/models.py b/common/djangoapps/third_party_auth/models.py
index 08fb3ec3b62..d8bc94dfb85 100644
--- a/common/djangoapps/third_party_auth/models.py
+++ b/common/djangoapps/third_party_auth/models.py
@@ -17,8 +17,7 @@ from django.db import models
from django.utils import timezone
from django.utils.translation import ugettext_lazy as _
from organizations.models import Organization
-from provider.oauth2.models import Client
-from provider.utils import long_token
+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
@@ -826,7 +825,7 @@ class LTIProviderConfig(ProviderConfig):
)
lti_consumer_secret = models.CharField(
- default=long_token,
+ default=default_token_generator,
max_length=255,
help_text=(
u'The shared secret that the LTI Tool Consumer will use to '
@@ -878,25 +877,3 @@ class LTIProviderConfig(ProviderConfig):
app_label = "third_party_auth"
verbose_name = u"Provider Configuration (LTI)"
verbose_name_plural = verbose_name
-
-
-class ProviderApiPermissions(models.Model):
- """
- This model links OAuth2 client with provider Id.
-
- It gives permission for a OAuth2 client to access the information under certain IdPs.
-
- .. no_pii:
- """
- client = models.ForeignKey(Client, on_delete=models.CASCADE)
- provider_id = models.CharField(
- max_length=255,
- help_text=(
- u'Uniquely identify a provider. This is different from backend_name.'
- )
- )
-
- class Meta(object):
- app_label = "third_party_auth"
- verbose_name = u"Provider API Permission"
- verbose_name_plural = verbose_name + 's'
diff --git a/common/djangoapps/third_party_auth/tests/testutil.py b/common/djangoapps/third_party_auth/tests/testutil.py
index 763194b6f6e..dbe8082cc56 100644
--- a/common/djangoapps/third_party_auth/tests/testutil.py
+++ b/common/djangoapps/third_party_auth/tests/testutil.py
@@ -15,15 +15,13 @@ from django.conf import settings
from django.contrib.auth.models import User
from django.contrib.sites.models import Site
from mako.template import Template
-from provider import constants
-from provider.oauth2.models import Client as OAuth2Client
+from oauth2_provider.models import Application
from openedx.core.djangolib.testing.utils import CacheIsolationMixin
from openedx.core.storage import OverwriteStorage
from third_party_auth.models import (
LTIProviderConfig,
OAuth2ProviderConfig,
- ProviderApiPermissions,
SAMLConfiguration,
SAMLProviderConfig
)
@@ -173,14 +171,9 @@ class ThirdPartyAuthTestMixin(object):
user.save()
@staticmethod
- def configure_oauth_dop_client():
+ def configure_oauth_dot_client():
""" Configure an oauth DOP client for testing """
- return OAuth2Client.objects.create(client_type=constants.CONFIDENTIAL)
-
- @staticmethod
- def configure_api_permission(client, provider_id):
- """ Configure the client and provider_id pair. This will give the access to a client for that provider. """
- return ProviderApiPermissions.objects.create(client=client, provider_id=provider_id)
+ return Application.objects.create(client_type=Application.CLIENT_CONFIDENTIAL)
@staticmethod
def read_data_file(filename):
@@ -191,7 +184,8 @@ class ThirdPartyAuthTestMixin(object):
class TestCase(ThirdPartyAuthTestMixin, CacheIsolationMixin, django.test.TestCase):
"""Base class for auth test cases."""
- def setUp(self):
+
+ def setUp(self): # pylint: disable=arguments-differ
super(TestCase, self).setUp()
# Explicitly set a server name that is compatible with all our providers:
# (The SAML lib we use doesn't like the default 'testserver' as a domain)
diff --git a/common/djangoapps/third_party_auth/tests/utils.py b/common/djangoapps/third_party_auth/tests/utils.py
index 0fe7f779456..2360074f1ae 100644
--- a/common/djangoapps/third_party_auth/tests/utils.py
+++ b/common/djangoapps/third_party_auth/tests/utils.py
@@ -6,8 +6,7 @@ from base64 import b64encode
import httpretty
from onelogin.saml2.utils import OneLogin_Saml2_Utils
-from provider.constants import PUBLIC
-from provider.oauth2.models import Client
+from oauth2_provider.models import Application
from social_core.backends.facebook import API_VERSION as FACEBOOK_API_VERSION
from social_core.backends.facebook import FacebookOAuth2
from social_django.models import Partial, UserSocialAuth
@@ -52,9 +51,9 @@ class ThirdPartyOAuthTestMixin(ThirdPartyAuthTestMixin):
"""
Create an OAuth2 client application
"""
- return Client.objects.create(
+ return Application.objects.create(
client_id=self.client_id,
- client_type=PUBLIC,
+ client_type=Application.CLIENT_PUBLIC,
)
def _setup_provider_response(self, success=False, email=''):
diff --git a/common/djangoapps/util/tests/test_db.py b/common/djangoapps/util/tests/test_db.py
index 8fdcf40b9f0..a20896fa618 100644
--- a/common/djangoapps/util/tests/test_db.py
+++ b/common/djangoapps/util/tests/test_db.py
@@ -221,8 +221,9 @@ class MigrationTests(TestCase):
"""
Tests for migrations.
"""
-
- @unittest.skip("Need to skip as part of renaming a field in schedules app. This will be unskipped in DE-1825")
+ @unittest.skip(
+ "Need to skip as part of renaming a field in schedules app. This will be unskipped in DE-1825. Also need to skip as part of removal of ProviderApiPermissions.This will be unskipped in BOM-1350"
+ )
@override_settings(MIGRATION_MODULES={})
def test_migrations_are_in_sync(self):
"""
--
GitLab