Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Remove ThirdPartyAuthProviderApiPermission (#23195)

Browse Source 
* 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
This commit is contained in:
Manjinder Singh
2020-02-27 15:42:09 -05:00
committed by GitHub
parent 5cce110a91
commit 953b0e728b
8 changed files with 36 additions and 173 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
common/djangoapps/third_party_auth/admin.py
View File

@@ -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)

30
common/djangoapps/third_party_auth/api/permissions.py
View File

@@ -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) &

44
common/djangoapps/third_party_auth/api/tests/test_permissions.py
View File

@@ -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):

52
common/djangoapps/third_party_auth/api/tests/test_views.py
View File

@@ -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),

27
common/djangoapps/third_party_auth/models.py
View File

@@ -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'

16
common/djangoapps/third_party_auth/tests/testutil.py
View File

@@ -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)

7
common/djangoapps/third_party_auth/tests/utils.py
View File

@@ -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=''):

5
common/djangoapps/util/tests/test_db.py
View File

@@ -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):
"""