From 02db5438484b79f58690d384b9d278c77dd74b5b Mon Sep 17 00:00:00 2001 From: Yunfan Zhang Date: Mon, 6 Aug 2018 16:15:02 -0400 Subject: [PATCH] Do not create refresh tokens for apps with implicit grant type. Signed-off-by: Yunfan Zhang --- awx/api/serializers.py | 4 +- awx/main/tests/functional/api/test_oauth.py | 46 +++++++++++++++++++++ awx/main/tests/functional/test_named_url.py | 10 ++++- 3 files changed, 56 insertions(+), 4 deletions(-) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index d0ab6610db..9d1067c780 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -1093,7 +1093,7 @@ class UserAuthorizedTokenSerializer(BaseOAuth2TokenSerializer): ) obj = super(UserAuthorizedTokenSerializer, self).create(validated_data) obj.save() - if obj.application is not None: + if obj.application and obj.application.authorization_grant_type != 'implicit': RefreshToken.objects.create( user=current_user, token=generate_token(), @@ -1116,7 +1116,7 @@ class OAuth2TokenSerializer(BaseOAuth2TokenSerializer): if obj.application and obj.application.user: obj.user = obj.application.user obj.save() - if obj.application is not None: + if obj.application and obj.application.authorization_grant_type != 'implicit': RefreshToken.objects.create( user=current_user, token=generate_token(), diff --git a/awx/main/tests/functional/api/test_oauth.py b/awx/main/tests/functional/api/test_oauth.py index 38213164ed..31b1393a9c 100644 --- a/awx/main/tests/functional/api/test_oauth.py +++ b/awx/main/tests/functional/api/test_oauth.py @@ -4,6 +4,7 @@ import json from django.db import connection from django.test.utils import override_settings +from django.test import Client from awx.main.utils.encryption import decrypt_value, get_encryption_key from awx.api.versioning import reverse, drf_reverse @@ -174,6 +175,27 @@ def test_oauth_token_create(oauth_application, get, post, admin): assert response.data['summary_fields']['tokens']['results'][0] == { 'id': token.pk, 'scope': token.scope, 'token': '************' } + # If the application is implicit grant type, no new refresb tokens should be created. + # The following tests check for that. + oauth_application.authorization_grant_type = 'implicit' + oauth_application.save() + token_count = RefreshToken.objects.count() + response = post( + reverse('api:o_auth2_token_list'), + {'scope': 'read', 'application': oauth_application.pk}, admin, expect=201 + ) + assert response.data['refresh_token'] is None + response = post( + reverse('api:user_authorized_token_list', kwargs={'pk': admin.pk}), + {'scope': 'read', 'application': oauth_application.pk}, admin, expect=201 + ) + assert response.data['refresh_token'] is None + response = post( + reverse('api:application_o_auth2_token_list', kwargs={'pk': oauth_application.pk}), + {'scope': 'read'}, admin, expect=201 + ) + assert response.data['refresh_token'] is None + assert token_count == RefreshToken.objects.count() @pytest.mark.django_db @@ -268,3 +290,27 @@ def test_refresh_accesstoken(oauth_application, post, get, delete, admin): assert RefreshToken.objects.get(token=new_refresh_token) != 0 refresh_token = RefreshToken.objects.get(token=refresh_token) assert refresh_token.revoked + + +@pytest.mark.django_db +def test_implicit_authorization(oauth_application, admin): + oauth_application.client_type = 'confidential' + oauth_application.authorization_grant_type = 'implicit' + oauth_application.redirect_uris = 'http://test.com' + oauth_application.save() + data = { + 'response_type': 'token', + 'client_id': oauth_application.client_id, + 'client_secret': oauth_application.client_secret, + 'scope': 'read', + 'redirect_uri': 'http://test.com', + 'allow': True + } + + request_client = Client() + request_client.force_login(admin, 'django.contrib.auth.backends.ModelBackend') + refresh_token_count = RefreshToken.objects.count() + response = request_client.post(drf_reverse('api:authorize'), data) + assert 'http://test.com' in response.url and 'access_token' in response.url + # Make sure no refresh token is created for app with implicit grant type. + assert refresh_token_count == RefreshToken.objects.count() diff --git a/awx/main/tests/functional/test_named_url.py b/awx/main/tests/functional/test_named_url.py index 593822c806..df315e583d 100644 --- a/awx/main/tests/functional/test_named_url.py +++ b/awx/main/tests/functional/test_named_url.py @@ -5,10 +5,10 @@ from django.core.exceptions import ImproperlyConfigured from awx.api.versioning import reverse from awx.main.middleware import URLModificationMiddleware from awx.main.models import * # noqa +from awx.conf import settings_registry -@pytest.fixture(scope='function', autouse=True) -def init_url_modification_middleware(): +def setup_module(module): # In real-world scenario, named url graph structure is populated by __init__ # of URLModificationMiddleware. The way Django bootstraps ensures the initialization # will happen *once and only once*, while the number of initialization is uncontrollable @@ -20,6 +20,12 @@ def init_url_modification_middleware(): pass +def teardown_module(module): + # settings_registry will be persistent states unless we explicitly clean them up. + settings_registry.unregister('NAMED_URL_FORMATS') + settings_registry.unregister('NAMED_URL_GRAPH_NODES') + + @pytest.mark.django_db def test_user(get, admin_user): test_user = User.objects.create(username='test_user', password='test_user', is_superuser=False)