From 1270df15003f81edb9704a96367939ad04e4d762 Mon Sep 17 00:00:00 2001 From: "J. Cliff Dyer" Date: Tue, 31 May 2016 12:13:29 -0400 Subject: [PATCH] Enable dispatch of Authorization endpoint. * Created URL route * Expanded test code to confirm expected behavior MA-2124 --- .../oauth_dispatch/tests/constants.py | 2 +- .../oauth_dispatch/tests/test_views.py | 113 +++++++++++++----- lms/djangoapps/oauth_dispatch/urls.py | 2 +- lms/djangoapps/oauth_dispatch/views.py | 5 +- 4 files changed, 89 insertions(+), 33 deletions(-) diff --git a/lms/djangoapps/oauth_dispatch/tests/constants.py b/lms/djangoapps/oauth_dispatch/tests/constants.py index b38868bcfb..b3f19be8df 100644 --- a/lms/djangoapps/oauth_dispatch/tests/constants.py +++ b/lms/djangoapps/oauth_dispatch/tests/constants.py @@ -2,4 +2,4 @@ Constants for testing purposes """ -DUMMY_REDIRECT_URL = u'https://example.edx/redirect' +DUMMY_REDIRECT_URL = u'https://example.com/edx/redirect' diff --git a/lms/djangoapps/oauth_dispatch/tests/test_views.py b/lms/djangoapps/oauth_dispatch/tests/test_views.py index c943fd5dce..06d84eaece 100644 --- a/lms/djangoapps/oauth_dispatch/tests/test_views.py +++ b/lms/djangoapps/oauth_dispatch/tests/test_views.py @@ -8,6 +8,7 @@ import ddt from django.test import RequestFactory, TestCase from django.core.urlresolvers import reverse import httpretty +from provider import constants from student.tests.factories import UserFactory from third_party_auth.tests.utils import ThirdPartyOAuthTestMixin, ThirdPartyOAuthTestMixinGoogle @@ -37,7 +38,7 @@ class _DispatchingViewTestCase(TestCase): redirect_uri=DUMMY_REDIRECT_URL, client_id='dot-app-client-id', ) - self.dop_client = self.dop_adapter.create_public_client( + self.dop_app = self.dop_adapter.create_public_client( name='test dop client', user=self.user, redirect_uri=DUMMY_REDIRECT_URL, @@ -83,7 +84,7 @@ class TestAccessTokenView(mixins.AccessTokenMixin, _DispatchingViewTestCase): return body - @ddt.data('dop_client', 'dot_app') + @ddt.data('dop_app', 'dot_app') def test_access_token_fields(self, client_attr): client = getattr(self, client_attr) response = self._post_request(self.user, client) @@ -94,7 +95,7 @@ class TestAccessTokenView(mixins.AccessTokenMixin, _DispatchingViewTestCase): self.assertIn('scope', data) self.assertIn('token_type', data) - @ddt.data('dop_client', 'dot_app') + @ddt.data('dop_app', 'dot_app') def test_jwt_access_token(self, client_attr): client = getattr(self, client_attr) response = self._post_request(self.user, client, token_type='jwt') @@ -111,7 +112,7 @@ class TestAccessTokenView(mixins.AccessTokenMixin, _DispatchingViewTestCase): self.assertIn('refresh_token', data) def test_dop_public_client_access_token(self): - response = self._post_request(self.user, self.dop_client) + response = self._post_request(self.user, self.dop_app) self.assertEqual(response.status_code, 200) data = json.loads(response.content) self.assertNotIn('refresh_token', data) @@ -133,7 +134,7 @@ class TestAccessTokenExchangeView(ThirdPartyOAuthTestMixinGoogle, ThirdPartyOAut 'access_token': self.access_token, } - @ddt.data('dop_client', 'dot_app') + @ddt.data('dop_app', 'dot_app') def test_access_token_exchange_calls_dispatched_view(self, client_attr): client = getattr(self, client_attr) self.oauth_client = client @@ -143,7 +144,7 @@ class TestAccessTokenExchangeView(ThirdPartyOAuthTestMixinGoogle, ThirdPartyOAut @ddt.ddt -class TestAuthorizationView(TestCase): +class TestAuthorizationView(_DispatchingViewTestCase): """ Test class for AuthorizationView """ @@ -153,43 +154,81 @@ class TestAuthorizationView(TestCase): def setUp(self): super(TestAuthorizationView, self).setUp() self.user = UserFactory() - self.dop_client = self._create_confidential_client(user=self.user, client_id='dop-app-client-id') - - def _create_confidential_client(self, user, client_id): - """ - Create a confidential client suitable for testing purposes. - """ - return self.dop_adapter.create_confidential_client( - name='test_app', - user=user, - client_id=client_id, - redirect_uri=DUMMY_REDIRECT_URL + self.dot_app = self.dot_adapter.create_confidential_client( + name='test dot application', + user=self.user, + redirect_uri=DUMMY_REDIRECT_URL, + client_id='confidential-dot-app-client-id', + ) + self.dop_app = self.dop_adapter.create_confidential_client( + name='test dop client', + user=self.user, + redirect_uri=DUMMY_REDIRECT_URL, + client_id='confidential-dop-app-client-id', ) - def test_authorization_view(self): + @ddt.data( + ('dop', 'authorize'), + ('dot', 'allow') + ) + @ddt.unpack + def test_post_authorization_view(self, client_type, allow_field): + oauth_application = getattr(self, '{}_app'.format(client_type)) self.client.login(username=self.user.username, password='test') response = self.client.post( '/oauth2/authorize/', { - 'client_id': self.dop_client.client_id, # TODO: DOT is not yet supported (MA-2124) + 'client_id': oauth_application.client_id, 'response_type': 'code', 'state': 'random_state_string', 'redirect_uri': DUMMY_REDIRECT_URL, + 'scope': 'profile email', + allow_field: True, }, follow=True, ) - self.assertEqual(response.status_code, 200) + check_response = getattr(self, '_check_{}_response'.format(client_type)) + check_response(response) - # check form is in context and form params are valid - context = response.context_data # pylint: disable=no-member - self.assertIn('form', context) - self.assertIsNone(context['form']['authorize'].value()) + def _check_dot_response(self, response): + """ + Check that django-oauth-toolkit gives an appropriate authorization response. + """ + # django-oauth-toolkit tries to redirect to the user's redirect URL + self.assertEqual(response.status_code, 404) # We used a non-existent redirect url. + expected_redirect_prefix = u'{}?'.format(DUMMY_REDIRECT_URL) + self._assert_startswith(self._redirect_destination(response), expected_redirect_prefix) + + def _check_dop_response(self, response): + """ + Check that django-oauth2-provider gives an appropriate authorization response. + """ + # django-oauth-provider redirects to a confirmation page + self.assertRedirects(response, u'http://testserver/oauth2/authorize/confirm', target_status_code=200) + + context = response.context_data + form = context['form'] + self.assertIsNone(form['authorize'].value()) - self.assertIn('oauth_data', context) oauth_data = context['oauth_data'] self.assertEqual(oauth_data['redirect_uri'], DUMMY_REDIRECT_URL) self.assertEqual(oauth_data['state'], 'random_state_string') + # TODO: figure out why it chooses this scope. + self.assertEqual(oauth_data['scope'], constants.READ_WRITE) + + def _assert_startswith(self, string, prefix): + """ + Assert that the string starts with the specified prefix. + """ + self.assertTrue(string.startswith(prefix), u'{} does not start with {}'.format(string, prefix)) + + @staticmethod + def _redirect_destination(response): + """ + Return the final destination of the redirect chain in the response object + """ + return response.redirect_chain[-1][0] class TestViewDispatch(TestCase): @@ -232,26 +271,40 @@ class TestViewDispatch(TestCase): self.assertTrue(args, msg_no_request) self.assertEqual(args[0], 'request') - def _get_request(self, client_id): + def _post_request(self, client_id): """ Return a request with the specified client_id in the body """ return RequestFactory().post('/', {'client_id': client_id}) - def test_dispatching_to_dot(self): + def _get_request(self, client_id): + """ + Return a request with the specified client_id in the get parameters + """ + return RequestFactory().get('/?client_id={}'.format(client_id)) + + def test_dispatching_post_to_dot(self): + request = self._post_request('dot-id') + self.assertEqual(self.view.select_backend(request), self.dot_adapter.backend) + + def test_dispatching_post_to_dop(self): + request = self._post_request('dop-id') + self.assertEqual(self.view.select_backend(request), self.dop_adapter.backend) + + def test_dispatching_get_to_dot(self): request = self._get_request('dot-id') self.assertEqual(self.view.select_backend(request), self.dot_adapter.backend) - def test_dispatching_to_dop(self): + def test_dispatching_get_to_dop(self): request = self._get_request('dop-id') self.assertEqual(self.view.select_backend(request), self.dop_adapter.backend) def test_dispatching_with_no_client(self): - request = self._get_request(None) + request = self._post_request(None) self.assertEqual(self.view.select_backend(request), self.dop_adapter.backend) def test_dispatching_with_invalid_client(self): - request = self._get_request('abcesdfljh') + request = self._post_request('abcesdfljh') self.assertEqual(self.view.select_backend(request), self.dop_adapter.backend) def test_get_view_for_dot(self): diff --git a/lms/djangoapps/oauth_dispatch/urls.py b/lms/djangoapps/oauth_dispatch/urls.py index 59e9068bdb..a42bed408b 100644 --- a/lms/djangoapps/oauth_dispatch/urls.py +++ b/lms/djangoapps/oauth_dispatch/urls.py @@ -11,7 +11,7 @@ from . import views urlpatterns = patterns( '', - # TODO: authorize/ URL not yet supported for DOT (MA-2124) + url(r'^authorize/?$', csrf_exempt(views.AuthorizationView.as_view()), name='authorize'), url(r'^access_token/?$', csrf_exempt(views.AccessTokenView.as_view()), name='access_token'), ) diff --git a/lms/djangoapps/oauth_dispatch/views.py b/lms/djangoapps/oauth_dispatch/views.py index eec1f12f08..b78fab924a 100644 --- a/lms/djangoapps/oauth_dispatch/views.py +++ b/lms/djangoapps/oauth_dispatch/views.py @@ -74,7 +74,10 @@ class _DispatchingView(View): """ Return the client_id from the provided request """ - return request.POST.get('client_id') + if request.method == u'GET': + return request.GET.get('client_id') + else: + return request.POST.get('client_id') class AccessTokenView(_DispatchingView):