diff --git a/openedx/core/djangoapps/oauth_dispatch/admin.py b/openedx/core/djangoapps/oauth_dispatch/admin.py index 0c503f2c42..5401a81a8e 100644 --- a/openedx/core/djangoapps/oauth_dispatch/admin.py +++ b/openedx/core/djangoapps/oauth_dispatch/admin.py @@ -6,7 +6,7 @@ Override admin configuration for django-oauth-toolkit from django.contrib.admin import ModelAdmin, site from oauth2_provider import models -from .models import ApplicationAccess, ApplicationOrganization, RestrictedApplication +from .models import ApplicationAccess, RestrictedApplication def reregister(model_class): @@ -83,13 +83,6 @@ class ApplicationAccessAdmin(ModelAdmin): list_display = ['application', 'scopes', 'filters'] -class ApplicationOrganizationAdmin(ModelAdmin): - """ - ModelAdmin for ApplicationOrganization - """ - list_display = [u'application', u'organization', u'relation_type'] - - class RestrictedApplicationAdmin(ModelAdmin): """ ModelAdmin for the Restricted Application @@ -98,5 +91,4 @@ class RestrictedApplicationAdmin(ModelAdmin): site.register(ApplicationAccess, ApplicationAccessAdmin) -site.register(ApplicationOrganization, ApplicationOrganizationAdmin) site.register(RestrictedApplication, RestrictedApplicationAdmin) diff --git a/openedx/core/djangoapps/oauth_dispatch/dot_overrides/views.py b/openedx/core/djangoapps/oauth_dispatch/dot_overrides/views.py index 7e717c9b3f..a03142fd52 100644 --- a/openedx/core/djangoapps/oauth_dispatch/dot_overrides/views.py +++ b/openedx/core/djangoapps/oauth_dispatch/dot_overrides/views.py @@ -10,21 +10,20 @@ from oauth2_provider.scopes import get_scopes_backend from oauth2_provider.settings import oauth2_settings from oauth2_provider.views import AuthorizationView -from openedx.core.djangoapps.oauth_dispatch.models import ApplicationOrganization +from openedx.core.djangoapps.oauth_dispatch.models import ApplicationAccess -# TODO (ARCH-83) remove once we have full support of OAuth Scopes class EdxOAuth2AuthorizationView(AuthorizationView): """ Override the AuthorizationView's GET method so the user isn't prompted to approve the application if they have already in the past, even if their access token is expired. - This is a temporary override of the base implementation - in order to accommodate our Restricted Applications support - until OAuth Scopes are fully supported. + This is override of the base implementation accommodates our + Restricted Applications support and custom filters. """ def get(self, request, *args, **kwargs): + # pylint: disable=line-too-long # Note: This code is copied from https://github.com/evonove/django-oauth-toolkit/blob/34f3b7b3511c15686039079026165feaadb1b87d/oauth2_provider/views/base.py#L111 # Places that we have changed are noted with ***. application = None @@ -46,10 +45,13 @@ class EdxOAuth2AuthorizationView(AuthorizationView): # at this point we know an Application instance with such client_id exists in the database application = get_application_model().objects.get(client_id=credentials['client_id']) - content_orgs = ApplicationOrganization.get_related_org_names( - application, - relation_type=ApplicationOrganization.RELATION_TYPE_CONTENT_ORG - ) + try: + content_orgs = list(ApplicationAccess.get_filter_values(application, ApplicationAccess.CONTENT_ORG_FILTER_NAME)) + except ApplicationAccess.DoesNotExist: + # No application access policy for this application exists. + # so we have no content orgs. + content_orgs = [] + kwargs['application'] = application kwargs['content_orgs'] = content_orgs kwargs['client_id'] = credentials['client_id'] diff --git a/openedx/core/djangoapps/oauth_dispatch/models.py b/openedx/core/djangoapps/oauth_dispatch/models.py index 5bbe14f8e7..e1ae9b2aeb 100644 --- a/openedx/core/djangoapps/oauth_dispatch/models.py +++ b/openedx/core/djangoapps/oauth_dispatch/models.py @@ -5,7 +5,6 @@ Specialized models for oauth_dispatch djangoapp from datetime import datetime -import six from django.db import models from django.utils.encoding import python_2_unicode_compatible from django.utils.translation import ugettext_lazy as _ @@ -71,6 +70,12 @@ class ApplicationAccess(models.Model): .. no_pii: """ + # Content org filters are of the form "content_org:" eg. "content_org:SchoolX" + # and indicate that for anything that cares about the content_org filter, that the response + # should be filtered based on the filter value. ie. We should only get responses pertain + # to objects that are relevant to the SchoolX organization. + CONTENT_ORG_FILTER_NAME = 'content_org' + application = models.OneToOneField(oauth2_settings.APPLICATION_MODEL, related_name='access', on_delete=models.CASCADE) scopes = ListCharField( @@ -100,6 +105,14 @@ class ApplicationAccess(models.Model): def get_filters(cls, application): return cls.objects.get(application=application).filters + @classmethod + def get_filter_values(cls, application, filter_name): + filters = cls.get_filters(application=application) + for filter_constraint in filters: + name, filter_value = filter_constraint.split(':', 1) + if name == filter_name: + yield filter_value + def __str__(self): """ Return a unicode representation of this object. @@ -114,12 +127,16 @@ class ApplicationAccess(models.Model): @python_2_unicode_compatible class ApplicationOrganization(models.Model): """ - Associates a DOT Application to an Organization. + DEPRECATED: Associates a DOT Application to an Organization. - See openedx/core/djangoapps/oauth_dispatch/docs/decisions/0007-include-organizations-in-tokens.rst - for the intended use of this model. + This model is no longer in use. - Deprecated: Use filters in ApplicationAccess instead. + TODO: BOM-1270: This model and table will be removed post-Juniper + so Open edX instances can migrate data if necessary. + + To migrate, use ApplicationAccess and add a ``filter`` of the form + ``content_org:`` (e.g. content_org:edx), for each record + in this model's table. .. no_pii: """ @@ -140,31 +157,3 @@ class ApplicationOrganization(models.Model): class Meta: app_label = 'oauth_dispatch' unique_together = ('application', 'relation_type', 'organization') - - @classmethod - def get_related_org_names(cls, application, relation_type=None): - """ - Return the names of the Organizations related to the given DOT Application. - - Filter by relation_type if provided. - """ - queryset = application.organizations.all() - if relation_type: - queryset = queryset.filter(relation_type=relation_type) - return [r.organization.name for r in queryset] - - def __str__(self): - """ - Return a unicode representation of this object. - """ - return u"{application_name}:{organization}:{relation_type}".format( - application_name=self.application.name, - organization=self.organization.short_name, - relation_type=self.relation_type, - ) - - def to_jwt_filter_claim(self): - """ - Serialize for use in JWT filter claim. - """ - return six.text_type(':'.join([self.relation_type, self.organization.short_name])) diff --git a/openedx/core/djangoapps/oauth_dispatch/tests/factories.py b/openedx/core/djangoapps/oauth_dispatch/tests/factories.py index 2953473a3d..fe1405cefd 100644 --- a/openedx/core/djangoapps/oauth_dispatch/tests/factories.py +++ b/openedx/core/djangoapps/oauth_dispatch/tests/factories.py @@ -8,9 +8,8 @@ import pytz from factory.django import DjangoModelFactory from factory.fuzzy import FuzzyText from oauth2_provider.models import AccessToken, Application, RefreshToken -from organizations.tests.factories import OrganizationFactory -from openedx.core.djangoapps.oauth_dispatch.models import ApplicationAccess, ApplicationOrganization +from openedx.core.djangoapps.oauth_dispatch.models import ApplicationAccess from student.tests.factories import UserFactory @@ -34,15 +33,6 @@ class ApplicationAccessFactory(DjangoModelFactory): scopes = ['grades:read'] -class ApplicationOrganizationFactory(DjangoModelFactory): - class Meta(object): - model = ApplicationOrganization - - application = factory.SubFactory(ApplicationFactory) - organization = factory.SubFactory(OrganizationFactory) - relation_type = ApplicationOrganization.RELATION_TYPE_CONTENT_ORG - - class AccessTokenFactory(DjangoModelFactory): class Meta(object): model = AccessToken diff --git a/openedx/core/djangoapps/oauth_dispatch/tests/test_models.py b/openedx/core/djangoapps/oauth_dispatch/tests/test_models.py deleted file mode 100644 index 8601587730..0000000000 --- a/openedx/core/djangoapps/oauth_dispatch/tests/test_models.py +++ /dev/null @@ -1,23 +0,0 @@ -""" -Tests for oauth_dispatch models. -""" - - -import six -from django.test import TestCase - -from openedx.core.djangoapps.oauth_dispatch.tests.factories import ApplicationOrganizationFactory -from openedx.core.djangolib.testing.utils import skip_unless_lms - - -@skip_unless_lms -class ApplicationOrganizationTestCase(TestCase): - """ - Tests for the ApplicationOrganization model. - """ - def test_to_jwt_filter_claim(self): - """ Verify to_jwt_filter_claim returns the expected serialization of the model. """ - org_relation = ApplicationOrganizationFactory() - organization = org_relation.organization - jwt_filter_claim = org_relation.to_jwt_filter_claim() - assert jwt_filter_claim == six.text_type(':'.join([org_relation.relation_type, organization.short_name])) diff --git a/openedx/core/djangoapps/oauth_dispatch/tests/test_views.py b/openedx/core/djangoapps/oauth_dispatch/tests/test_views.py index a27fad6be9..e53094a925 100644 --- a/openedx/core/djangoapps/oauth_dispatch/tests/test_views.py +++ b/openedx/core/djangoapps/oauth_dispatch/tests/test_views.py @@ -15,7 +15,6 @@ from django.urls import reverse from jwkest import jwk from mock import call, patch from oauth2_provider import models as dot_models -from organizations.tests.factories import OrganizationFactory from provider import constants from openedx.core.djangoapps.oauth_dispatch.toggles import ENFORCE_JWT_SCOPES @@ -107,10 +106,6 @@ class _DispatchingViewTestCase(TestCase): application=self.dot_app, scopes=['grades:read'], ) - self.dot_app_org = models.ApplicationOrganization.objects.create( - application=self.dot_app, - organization=OrganizationFactory() - ) # Create a "restricted" DOT Application which means any AccessToken/JWT # generated for this application will be immediately expired @@ -350,10 +345,6 @@ class TestAccessTokenView(AccessTokenLoginMixin, mixins.AccessTokenMixin, _Dispa scopes=['grades:read'], filters=['test:filter'], ) - models.ApplicationOrganization.objects.create( - application=dot_app, - organization=OrganizationFactory() - ) scopes = dot_app_access.scopes filters = self.dot_adapter.get_authorization_filters(dot_app) assert 'test:filter' in filters @@ -416,10 +407,10 @@ class TestAuthorizationView(_DispatchingViewTestCase): models.ApplicationAccess.objects.create( application=self.dot_app, scopes=['grades:read'], - ) - self.dot_app_org = models.ApplicationOrganization.objects.create( - application=self.dot_app, - organization=OrganizationFactory() + filters=[ + 'content_org:test content org', + 'other_filter:filter_val', + ] ) self.dop_app = self.dop_adapter.create_confidential_client( name='test dop client', @@ -497,7 +488,13 @@ class TestAuthorizationView(_DispatchingViewTestCase): # Are the content provider organizations listed on the page? self.assertContains( response, - '
  • {org}
    • '.format(org=self.dot_app_org.organization.name) + '
  • {org}
    • '.format(org='test content org') + ) + + # Make sure other filters don't show up as orgs. + self.assertNotContains( + response, + '
  • {org}
    • '.format(org='filter_val') ) def _check_dot_response(self, response):