From 8a6e3b2a34561fb575ceecbb0f598347d47786e3 Mon Sep 17 00:00:00 2001 From: Bill Prin Date: Fri, 16 Sep 2016 15:50:17 -0700 Subject: [PATCH] Fix django authorization redirect by correctly checking validity of credentials (#651) --- oauth2client/contrib/django_util/views.py | 13 ++++---- tests/contrib/django_util/test_decorators.py | 4 +-- tests/contrib/django_util/test_views.py | 33 ++++++++++++++++++-- 3 files changed, 39 insertions(+), 11 deletions(-) diff --git a/oauth2client/contrib/django_util/views.py b/oauth2client/contrib/django_util/views.py index 4858f206e..009b544ca 100644 --- a/oauth2client/contrib/django_util/views.py +++ b/oauth2client/contrib/django_util/views.py @@ -170,7 +170,10 @@ def oauth2_authorize(request): A redirect to Google OAuth2 Authorization. """ return_url = request.GET.get('return_url', None) + if not return_url: + return_url = request.META.get('HTTP_REFERER', '/') + scopes = request.GET.getlist('scopes', django_util.oauth2_settings.scopes) # Model storage (but not session storage) requires a logged in user if django_util.oauth2_settings.storage_model: if not request.user.is_authenticated(): @@ -178,13 +181,11 @@ def oauth2_authorize(request): settings.LOGIN_URL, parse.quote(request.get_full_path()))) # This checks for the case where we ended up here because of a logged # out user but we had credentials for it in the first place - elif get_storage(request).get() is not None: - return redirect(return_url) + else: + user_oauth = django_util.UserOAuth2(request, scopes, return_url) + if user_oauth.has_credentials(): + return redirect(return_url) - scopes = request.GET.getlist('scopes', django_util.oauth2_settings.scopes) - - if not return_url: - return_url = request.META.get('HTTP_REFERER', '/') flow = _make_flow(request=request, scopes=scopes, return_url=return_url) auth_url = flow.step1_get_authorize_url() return shortcuts.redirect(auth_url) diff --git a/tests/contrib/django_util/test_decorators.py b/tests/contrib/django_util/test_decorators.py index 836e4cc8f..f237f88ea 100644 --- a/tests/contrib/django_util/test_decorators.py +++ b/tests/contrib/django_util/test_decorators.py @@ -92,7 +92,7 @@ def test_specified_scopes(self, dictionary_storage_mock): credentials_mock = mock.Mock( scopes=set(django.conf.settings.GOOGLE_OAUTH2_SCOPES)) - credentials_mock.has_scopes = True + credentials_mock.has_scopes = mock.Mock(return_value=True) credentials_mock.is_valid = True dictionary_storage_mock.get.return_value = credentials_mock @@ -183,7 +183,7 @@ def test_specified_scopes(self, OAuth2Credentials): credentials_mock = mock.Mock( scopes=set(django.conf.settings.GOOGLE_OAUTH2_SCOPES)) - credentials_mock.has_scopes = False + credentials_mock.has_scopes = mock.Mock(return_value=False) OAuth2Credentials.from_json.return_value = credentials_mock @decorators.oauth_required(scopes=['additional-scope']) diff --git a/tests/contrib/django_util/test_views.py b/tests/contrib/django_util/test_views.py index b6b6c228d..dc0666123 100644 --- a/tests/contrib/django_util/test_views.py +++ b/tests/contrib/django_util/test_views.py @@ -26,7 +26,6 @@ from oauth2client import client import oauth2client.contrib.django_util -from oauth2client.contrib.django_util import models from oauth2client.contrib.django_util import views from tests.contrib import django_util as tests_django_util from tests.contrib.django_util import models as tests_models @@ -117,15 +116,31 @@ def test_authorize_works_explicit_return_url(self): response = views.oauth2_authorize(request) self.assertIsInstance(response, http.HttpResponseRedirect) - def test_authorized_user_not_logged_in_redirects(self): + def test_authorized_user_no_credentials_redirects(self): request = self.factory.get('oauth2/oauth2authorize', data={'return_url': '/return_endpoint'}) request.session = self.session authorized_user = django_models.User.objects.create_user( username='bill2', email='bill@example.com', password='hunter2') - credentials = models.CredentialsField() + tests_models.CredentialsModel.objects.create( + user_id=authorized_user, + credentials=None) + + request.user = authorized_user + response = views.oauth2_authorize(request) + self.assertIsInstance(response, http.HttpResponseRedirect) + + def test_already_authorized(self): + request = self.factory.get('oauth2/oauth2authorize', + data={'return_url': '/return_endpoint'}) + request.session = self.session + + authorized_user = django_models.User.objects.create_user( + username='bill2', email='bill@example.com', password='hunter2') + + credentials = _Credentials() tests_models.CredentialsModel.objects.create( user_id=authorized_user, credentials=credentials) @@ -133,6 +148,18 @@ def test_authorized_user_not_logged_in_redirects(self): request.user = authorized_user response = views.oauth2_authorize(request) self.assertIsInstance(response, http.HttpResponseRedirect) + self.assertEqual(response.url, '/return_endpoint') + + +class _Credentials(object): + # Can't use mock when testing Django models + # https://code.djangoproject.com/ticket/25493 + def __init__(self): + self.invalid = False + self.scopes = set() + + def has_scopes(self, _): + return True class Oauth2CallbackTest(tests_django_util.TestWithDjangoEnvironment):