From c5d975577ada00ff221e10d7239a9e3de30c3569 Mon Sep 17 00:00:00 2001 From: Ahsan Date: Thu, 22 Sep 2016 16:32:08 +0500 Subject: [PATCH] Invalidate access token ECOM-4641 --- common/djangoapps/student/helpers.py | 20 ++++++- .../student/tests/test_reset_password.py | 14 +++++ common/djangoapps/student/views.py | 2 + .../oauth_dispatch/tests/factories.py | 40 ++++++++++++++ .../oauth_dispatch/tests/test_factories.py | 45 ++++++++++++++++ .../student_account/test/test_views.py | 54 +++++++++++++++++++ lms/djangoapps/student_account/views.py | 6 ++- requirements/edx/base.txt | 2 +- 8 files changed, 180 insertions(+), 3 deletions(-) create mode 100644 lms/djangoapps/oauth_dispatch/tests/factories.py create mode 100644 lms/djangoapps/oauth_dispatch/tests/test_factories.py diff --git a/common/djangoapps/student/helpers.py b/common/djangoapps/student/helpers.py index c7229a0199..d5d8a91592 100644 --- a/common/djangoapps/student/helpers.py +++ b/common/djangoapps/student/helpers.py @@ -2,8 +2,16 @@ from datetime import datetime import urllib -from pytz import UTC from django.core.urlresolvers import reverse, NoReverseMatch +from oauth2_provider.models import ( + AccessToken as dot_access_token, + RefreshToken as dot_refresh_token +) +from provider.oauth2.models import ( + AccessToken as dop_access_token, + RefreshToken as dop_refresh_token +) +from pytz import UTC import third_party_auth from lms.djangoapps.verify_student.models import VerificationDeadline, SoftwareSecurePhotoVerification @@ -230,3 +238,13 @@ def get_next_url_for_login_page(request): # be saved in the session as part of the pipeline state. That URL will take priority # over this one. return redirect_to + + +def destroy_oauth_tokens(user): + """ + Destroys ALL OAuth access and refresh tokens for the given user. + """ + dop_access_token.objects.filter(user=user).delete() + dop_refresh_token.objects.filter(user=user).delete() + dot_access_token.objects.filter(user=user).delete() + dot_refresh_token.objects.filter(user=user).delete() diff --git a/common/djangoapps/student/tests/test_reset_password.py b/common/djangoapps/student/tests/test_reset_password.py index 13d8243d07..8c3535014d 100644 --- a/common/djangoapps/student/tests/test_reset_password.py +++ b/common/djangoapps/student/tests/test_reset_password.py @@ -12,12 +12,16 @@ from django.test.client import RequestFactory from django.contrib.auth.models import User from django.contrib.auth.hashers import UNUSABLE_PASSWORD_PREFIX from django.contrib.auth.tokens import default_token_generator +from edx_oauth2_provider.tests.factories import ClientFactory, AccessTokenFactory, RefreshTokenFactory +from oauth2_provider import models as dot_models +from provider.oauth2 import models as dop_models from django.utils.http import int_to_base36 from mock import Mock, patch import ddt +from lms.djangoapps.oauth_dispatch.tests import factories as dot_factories from openedx.core.djangolib.testing.utils import CacheIsolationTestCase from student.views import password_reset, password_reset_confirm_wrapper, SETTING_CHANGE_INITIATED from student.tests.factories import UserFactory @@ -113,8 +117,18 @@ class ResetPasswordTests(EventTestMixin, CacheIsolationTestCase): good_req = self.request_factory.post('/password_reset/', {'email': self.user.email}) good_req.user = self.user + dop_client = ClientFactory() + dop_access_token = AccessTokenFactory(user=self.user, client=dop_client) + RefreshTokenFactory(user=self.user, client=dop_client, access_token=dop_access_token) + dot_application = dot_factories.ApplicationFactory(user=self.user) + dot_access_token = dot_factories.AccessTokenFactory(user=self.user, application=dot_application) + dot_factories.RefreshTokenFactory(user=self.user, application=dot_application, access_token=dot_access_token) good_resp = password_reset(good_req) self.assertEquals(good_resp.status_code, 200) + self.assertFalse(dop_models.AccessToken.objects.filter(user=self.user).exists()) + self.assertFalse(dop_models.RefreshToken.objects.filter(user=self.user).exists()) + self.assertFalse(dot_models.AccessToken.objects.filter(user=self.user).exists()) + self.assertFalse(dot_models.RefreshToken.objects.filter(user=self.user).exists()) obj = json.loads(good_resp.content) self.assertEquals(obj, { 'success': True, diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 45c8b75541..e63b527517 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -105,6 +105,7 @@ from student.helpers import ( check_verify_status_by_course, auth_pipeline_urls, get_next_url_for_login_page, DISABLE_UNENROLL_CERT_STATES, + destroy_oauth_tokens ) from student.cookies import set_logged_in_cookies, delete_logged_in_cookies from student.models import anonymous_id_for_user, UserAttribute, EnrollStatusChange @@ -2116,6 +2117,7 @@ def password_reset(request): "user_id": request.user.id, } ) + destroy_oauth_tokens(request.user) else: # bad user? tick the rate limiter counter AUDIT_LOG.info("Bad password_reset user passed in.") diff --git a/lms/djangoapps/oauth_dispatch/tests/factories.py b/lms/djangoapps/oauth_dispatch/tests/factories.py new file mode 100644 index 0000000000..100c166c27 --- /dev/null +++ b/lms/djangoapps/oauth_dispatch/tests/factories.py @@ -0,0 +1,40 @@ +# pylint: disable=missing-docstring + +from datetime import datetime, timedelta + +import factory +from factory.django import DjangoModelFactory +from factory.fuzzy import FuzzyText +import pytz + +from oauth2_provider.models import Application, AccessToken, RefreshToken + +from student.tests.factories import UserFactory + + +class ApplicationFactory(DjangoModelFactory): + class Meta(object): + model = Application + + user = factory.SubFactory(UserFactory) + client_id = factory.Sequence(u'client_{0}'.format) + client_secret = 'some_secret' + client_type = 'confidential' + authorization_grant_type = 'Client credentials' + + +class AccessTokenFactory(DjangoModelFactory): + class Meta(object): + model = AccessToken + django_get_or_create = ('user', 'application') + + token = FuzzyText(length=32) + expires = datetime.now(pytz.UTC) + timedelta(days=1) + + +class RefreshTokenFactory(DjangoModelFactory): + class Meta(object): + model = RefreshToken + django_get_or_create = ('user', 'application') + + token = FuzzyText(length=32) diff --git a/lms/djangoapps/oauth_dispatch/tests/test_factories.py b/lms/djangoapps/oauth_dispatch/tests/test_factories.py new file mode 100644 index 0000000000..a48c88dc83 --- /dev/null +++ b/lms/djangoapps/oauth_dispatch/tests/test_factories.py @@ -0,0 +1,45 @@ +# pylint: disable=missing-docstring + +from django.test import TestCase +from oauth2_provider.models import Application, AccessToken, RefreshToken + +from lms.djangoapps.oauth_dispatch.tests import factories +from student.tests.factories import UserFactory + + +class TestClientFactory(TestCase): + def setUp(self): + super(TestClientFactory, self).setUp() + self.user = UserFactory.create() + + def test_client_factory(self): + actual_application = factories.ApplicationFactory(user=self.user) + expected_application = Application.objects.get(user=self.user) + self.assertEqual(actual_application, expected_application) + + +class TestAccessTokenFactory(TestCase): + def setUp(self): + super(TestAccessTokenFactory, self).setUp() + self.user = UserFactory.create() + + def test_access_token_client_factory(self): + application = factories.ApplicationFactory(user=self.user) + actual_access_token = factories.AccessTokenFactory(user=self.user, application=application) + expected_access_token = AccessToken.objects.get(user=self.user) + self.assertEqual(actual_access_token, expected_access_token) + + +class TestRefreshTokenFactory(TestCase): + def setUp(self): + super(TestRefreshTokenFactory, self).setUp() + self.user = UserFactory.create() + + def test_refresh_token_factory(self): + application = factories.ApplicationFactory(user=self.user) + access_token = factories.AccessTokenFactory(user=self.user, application=application) + actual_refresh_token = factories.RefreshTokenFactory( + user=self.user, application=application, access_token=access_token + ) + expected_refresh_token = RefreshToken.objects.get(user=self.user, access_token=access_token) + self.assertEqual(actual_refresh_token, expected_refresh_token) diff --git a/lms/djangoapps/student_account/test/test_views.py b/lms/djangoapps/student_account/test/test_views.py index 1f75f64023..e572269670 100644 --- a/lms/djangoapps/student_account/test/test_views.py +++ b/lms/djangoapps/student_account/test/test_views.py @@ -13,18 +13,29 @@ from django.core import mail from django.core.files.uploadedfile import SimpleUploadedFile from django.core.urlresolvers import reverse from django.contrib import messages +from django.contrib.auth import get_user_model from django.contrib.messages.middleware import MessageMiddleware from django.test import TestCase from django.test.utils import override_settings from django.http import HttpRequest +from edx_oauth2_provider.tests.factories import ClientFactory, AccessTokenFactory, RefreshTokenFactory from edx_rest_api_client import exceptions from nose.plugins.attrib import attr +from oauth2_provider.models import ( + AccessToken as dot_access_token, + RefreshToken as dot_refresh_token +) +from provider.oauth2.models import ( + AccessToken as dop_access_token, + RefreshToken as dop_refresh_token +) from testfixtures import LogCapture from commerce.models import CommerceConfiguration from commerce.tests import TEST_API_URL, TEST_API_SIGNING_KEY, factories from commerce.tests.mocks import mock_get_orders from course_modes.models import CourseMode +from lms.djangoapps.oauth_dispatch.tests import factories as dot_factories from openedx.core.djangoapps.programs.tests.mixins import ProgramsApiConfigMixin from openedx.core.djangoapps.user_api.accounts.api import activate_account, create_account from openedx.core.djangoapps.user_api.accounts import EMAIL_MAX_LENGTH @@ -39,6 +50,7 @@ from openedx.core.djangoapps.theming.tests.test_util import with_comprehensive_t LOGGER_NAME = 'audit' +User = get_user_model() # pylint:disable=invalid-name @ddt.ddt @@ -158,6 +170,23 @@ class StudentAccountUpdateTest(CacheIsolationTestCase, UrlResetMixin): response = self._change_password() self.assertEqual(response.status_code, 400) + def test_access_token_invalidation_logged_out(self): + self.client.logout() + user = User.objects.get(email=self.OLD_EMAIL) + self._create_dop_tokens(user) + self._create_dot_tokens(user) + response = self._change_password(email=self.OLD_EMAIL) + self.assertEqual(response.status_code, 200) + self.assert_access_token_destroyed(user) + + def test_access_token_invalidation_logged_in(self): + user = User.objects.get(email=self.OLD_EMAIL) + self._create_dop_tokens(user) + self._create_dot_tokens(user) + response = self._change_password() + self.assertEqual(response.status_code, 200) + self.assert_access_token_destroyed(user) + def test_password_change_inactive_user(self): # Log out the user created during test setup self.client.logout() @@ -217,6 +246,31 @@ class StudentAccountUpdateTest(CacheIsolationTestCase, UrlResetMixin): return self.client.post(path=reverse('password_change_request'), data=data) + def _create_dop_tokens(self, user=None): + """Create dop access token for given user if user provided else for default user.""" + if not user: + user = User.objects.get(email=self.OLD_EMAIL) + + client = ClientFactory() + access_token = AccessTokenFactory(user=user, client=client) + RefreshTokenFactory(user=user, client=client, access_token=access_token) + + def _create_dot_tokens(self, user=None): + """Create dop access token for given user if user provided else for default user.""" + if not user: + user = User.objects.get(email=self.OLD_EMAIL) + + application = dot_factories.ApplicationFactory(user=user) + access_token = dot_factories.AccessTokenFactory(user=user, application=application) + dot_factories.RefreshTokenFactory(user=user, application=application, access_token=access_token) + + def assert_access_token_destroyed(self, user): + """Assert all access tokens are destroyed.""" + self.assertFalse(dot_access_token.objects.filter(user=user).exists()) + self.assertFalse(dot_refresh_token.objects.filter(user=user).exists()) + self.assertFalse(dop_access_token.objects.filter(user=user).exists()) + self.assertFalse(dop_refresh_token.objects.filter(user=user).exists()) + @attr(shard=3) @ddt.ddt diff --git a/lms/djangoapps/student_account/views.py b/lms/djangoapps/student_account/views.py index d0c52341c2..645f573b5f 100644 --- a/lms/djangoapps/student_account/views.py +++ b/lms/djangoapps/student_account/views.py @@ -8,6 +8,7 @@ from datetime import datetime from django.conf import settings from django.contrib import messages from django.contrib.auth.decorators import login_required +from django.contrib.auth import get_user_model from django.core.urlresolvers import reverse, resolve from django.http import ( HttpResponse, HttpResponseBadRequest, HttpResponseForbidden, HttpRequest @@ -39,7 +40,7 @@ from student.views import ( signin_user as old_login_view, register_user as old_register_view ) -from student.helpers import get_next_url_for_login_page +from student.helpers import get_next_url_for_login_page, destroy_oauth_tokens import third_party_auth from third_party_auth import pipeline from third_party_auth.decorators import xframe_allow_whitelisted @@ -48,6 +49,7 @@ from util.date_utils import strftime_localized AUDIT_LOG = logging.getLogger("audit") log = logging.getLogger(__name__) +User = get_user_model() # pylint:disable=invalid-name @require_http_methods(['GET']) @@ -171,6 +173,8 @@ def password_change_request_handler(request): if email: try: request_password_change(email, request.get_host(), request.is_secure()) + user = user if user.is_authenticated() else User.objects.get(email=email) + destroy_oauth_tokens(user) except UserNotFound: AUDIT_LOG.info("Invalid password reset attempt") # Increment the rate limit counter diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 4131a7480b..fb32a8fdca 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -43,7 +43,7 @@ edx-drf-extensions==0.5.1 edx-lint==0.4.3 edx-django-oauth2-provider==1.1.1 edx-django-sites-extensions==2.1.1 -edx-oauth2-provider==1.1.3 +edx-oauth2-provider==1.2.0 edx-opaque-keys==0.3.4 edx-organizations==0.4.1 edx-rest-api-client==1.2.1