From e10614e75a287efe67c9d2e563e5013a13c5d974 Mon Sep 17 00:00:00 2001 From: Mark Sadecki Date: Tue, 20 Jan 2015 17:42:25 -0500 Subject: [PATCH 1/4] adds support to auth_auth for redirecting --- common/djangoapps/student/views.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 033901154b..63c0d09511 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -1799,6 +1799,7 @@ def auto_auth(request): * `course_id`: Enroll the student in the course with `course_id` * `roles`: Comma-separated list of roles to grant the student in the course with `course_id` * `no_login`: Define this to create the user but not login + * `redirect`: Set to "true" will redirect to course if course_id is defined, otherwise it will redirect to dashboard If username, email, or password are not provided, use randomly generated credentials. @@ -1823,6 +1824,7 @@ def auto_auth(request): if course_id: course_key = CourseLocator.from_string(course_id) role_names = [v.strip() for v in request.GET.get('roles', '').split(',') if v.strip()] + redirect_when_done = request.GET.get('redirect', None) login_when_done = 'no_login' not in request.GET form = AccountCreationForm( @@ -1885,7 +1887,15 @@ def auto_auth(request): create_comments_service_user(user) # Provide the user with a valid CSRF token - # then return a 200 response + # then return a 200 response unless redirect is true + if redirect_when_done: + # Redirect to course info page if course_id is known + if course_id: + return redirect(reverse('info', kwargs={'course_id': unicode(course_id)})) + # Otherwise redirect to dashboard + else: + return redirect(reverse('dashboard')) + if request.META.get('HTTP_ACCEPT') == 'application/json': response = JsonResponse({ 'created_status': u"Logged in" if login_when_done else "Created", From 3b217df9172fa52f33bfa9f0d056eda151ab3588 Mon Sep 17 00:00:00 2001 From: Christine Lytwynec Date: Fri, 4 Dec 2015 13:47:41 -0500 Subject: [PATCH 2/4] add new tests for auto_auth redirecting --- .../student/tests/test_auto_auth.py | 39 ++++++++++++++++++- common/djangoapps/student/views.py | 26 ++++++++++--- 2 files changed, 58 insertions(+), 7 deletions(-) diff --git a/common/djangoapps/student/tests/test_auto_auth.py b/common/djangoapps/student/tests/test_auto_auth.py index 2605cc65f5..22a45982b3 100644 --- a/common/djangoapps/student/tests/test_auto_auth.py +++ b/common/djangoapps/student/tests/test_auto_auth.py @@ -175,7 +175,40 @@ class AutoAuthEnabledTestCase(UrlResetMixin, TestCase): response_data ) - def _auto_auth(self, params=None, **kwargs): + @ddt.data(*COURSE_IDS_DDT) + @ddt.unpack + def test_redirect_to_course(self, course_id, course_key): + + # Create a user and enroll in a course + response = self._auto_auth({ + 'username': 'test', + 'course_id': course_id, + 'redirect': True, + 'staff': 'true', + }, status_code=302) + + # Check that a course enrollment was created for the user + self.assertEqual(CourseEnrollment.objects.count(), 1) + enrollment = CourseEnrollment.objects.get(course_id=course_key) + self.assertEqual(enrollment.user.username, "test") + + # Check that the redirect was to the course info/outline page + urls = ('/info', 'course/{}'.format(course_key.to_deprecated_string())) + response.url.endswith(urls) # pylint: disable=no-member + + def test_redirect_to_main(self): + # Create user and redirect to 'home' (cms) or 'dashboard' (lms) + response = self._auto_auth({ + 'username': 'test', + 'redirect': True, + 'staff': 'true', + }, status_code=302) + + # Check that the redirect was to either /dashboard or /home + urls = ('/dashboard', '/home') + response.url.endswith(urls) # pylint: disable=no-member + + def _auto_auth(self, params=None, status_code=None, **kwargs): """ Make a request to the auto-auth end-point and check that the response is successful. @@ -189,7 +222,9 @@ class AutoAuthEnabledTestCase(UrlResetMixin, TestCase): """ params = params or {} response = self.client.get(self.url, params, **kwargs) - self.assertEqual(response.status_code, 200) + + expected_status_code = status_code if status_code else 200 + self.assertEqual(response.status_code, expected_status_code) # Check that session and CSRF are set in the response for cookie in ['csrftoken', 'sessionid']: diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 63c0d09511..4ccb99f418 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -21,7 +21,7 @@ from django.contrib.auth.views import password_reset_confirm from django.contrib import messages from django.core.context_processors import csrf from django.core import mail -from django.core.urlresolvers import reverse +from django.core.urlresolvers import reverse, NoReverseMatch from django.core.validators import validate_email, ValidationError from django.db import IntegrityError, transaction from django.http import (HttpResponse, HttpResponseBadRequest, HttpResponseForbidden, @@ -1891,12 +1891,28 @@ def auto_auth(request): if redirect_when_done: # Redirect to course info page if course_id is known if course_id: - return redirect(reverse('info', kwargs={'course_id': unicode(course_id)})) - # Otherwise redirect to dashboard + try: + # redirect to course info page in LMS + redirect_url = reverse( + 'info', + kwargs={'course_id': unicode(course_id)} + ) + except NoReverseMatch: + # redirect to course outline page in Studio + redirect_url = reverse( + 'course_handler', + kwargs={'course_key_string': unicode(course_key)} + ) else: - return redirect(reverse('dashboard')) + try: + # redirect to dashboard for LMS + redirect_url = reverse('dashboard') + except NoReverseMatch: + # redirect to home for Studio + redirect_url = reverse('home') - if request.META.get('HTTP_ACCEPT') == 'application/json': + return redirect(redirect_url) + elif request.META.get('HTTP_ACCEPT') == 'application/json': response = JsonResponse({ 'created_status': u"Logged in" if login_when_done else "Created", 'username': username, From 9f5273a7d1fc781db7557edd1a4c4af5bb8bedca Mon Sep 17 00:00:00 2001 From: Christine Lytwynec Date: Mon, 7 Dec 2015 11:50:41 -0500 Subject: [PATCH 3/4] pr fixes --- common/djangoapps/student/tests/test_auto_auth.py | 4 ++-- common/djangoapps/student/views.py | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/common/djangoapps/student/tests/test_auto_auth.py b/common/djangoapps/student/tests/test_auto_auth.py index 22a45982b3..be8f233f73 100644 --- a/common/djangoapps/student/tests/test_auto_auth.py +++ b/common/djangoapps/student/tests/test_auto_auth.py @@ -194,7 +194,7 @@ class AutoAuthEnabledTestCase(UrlResetMixin, TestCase): # Check that the redirect was to the course info/outline page urls = ('/info', 'course/{}'.format(course_key.to_deprecated_string())) - response.url.endswith(urls) # pylint: disable=no-member + self.assertTrue(response.url.endswith(urls)) # pylint: disable=no-member def test_redirect_to_main(self): # Create user and redirect to 'home' (cms) or 'dashboard' (lms) @@ -206,7 +206,7 @@ class AutoAuthEnabledTestCase(UrlResetMixin, TestCase): # Check that the redirect was to either /dashboard or /home urls = ('/dashboard', '/home') - response.url.endswith(urls) # pylint: disable=no-member + self.assertTrue(response.url.endswith(urls)) # pylint: disable=no-member def _auto_auth(self, params=None, status_code=None, **kwargs): """ diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 4ccb99f418..516bd20790 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -1824,7 +1824,7 @@ def auto_auth(request): if course_id: course_key = CourseLocator.from_string(course_id) role_names = [v.strip() for v in request.GET.get('roles', '').split(',') if v.strip()] - redirect_when_done = request.GET.get('redirect', None) + redirect_when_done = request.GET.get('redirect', '').lower() == 'true' login_when_done = 'no_login' not in request.GET form = AccountCreationForm( @@ -1895,13 +1895,13 @@ def auto_auth(request): # redirect to course info page in LMS redirect_url = reverse( 'info', - kwargs={'course_id': unicode(course_id)} + kwargs={'course_id': course_id} ) except NoReverseMatch: # redirect to course outline page in Studio redirect_url = reverse( 'course_handler', - kwargs={'course_key_string': unicode(course_key)} + kwargs={'course_key_string': course_id} ) else: try: From df638088ca7f56fbd868ac411194d9628dc62548 Mon Sep 17 00:00:00 2001 From: Christine Lytwynec Date: Mon, 7 Dec 2015 14:21:00 -0500 Subject: [PATCH 4/4] pr fixes --- .../djangoapps/student/tests/test_auto_auth.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/common/djangoapps/student/tests/test_auto_auth.py b/common/djangoapps/student/tests/test_auto_auth.py index be8f233f73..9906665bcb 100644 --- a/common/djangoapps/student/tests/test_auto_auth.py +++ b/common/djangoapps/student/tests/test_auto_auth.py @@ -1,6 +1,7 @@ from django.test import TestCase from django.test.client import Client from django.contrib.auth.models import User +from django.conf import settings from django_comment_common.models import ( Role, FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_STUDENT) from django_comment_common.utils import seed_permissions_roles @@ -178,7 +179,6 @@ class AutoAuthEnabledTestCase(UrlResetMixin, TestCase): @ddt.data(*COURSE_IDS_DDT) @ddt.unpack def test_redirect_to_course(self, course_id, course_key): - # Create a user and enroll in a course response = self._auto_auth({ 'username': 'test', @@ -193,8 +193,12 @@ class AutoAuthEnabledTestCase(UrlResetMixin, TestCase): self.assertEqual(enrollment.user.username, "test") # Check that the redirect was to the course info/outline page - urls = ('/info', 'course/{}'.format(course_key.to_deprecated_string())) - self.assertTrue(response.url.endswith(urls)) # pylint: disable=no-member + if settings.ROOT_URLCONF == 'lms.urls': + url_pattern = '/info' + else: + url_pattern = '/course/{}'.format(unicode(course_key)) + + self.assertTrue(response.url.endswith(url_pattern)) # pylint: disable=no-member def test_redirect_to_main(self): # Create user and redirect to 'home' (cms) or 'dashboard' (lms) @@ -205,8 +209,12 @@ class AutoAuthEnabledTestCase(UrlResetMixin, TestCase): }, status_code=302) # Check that the redirect was to either /dashboard or /home - urls = ('/dashboard', '/home') - self.assertTrue(response.url.endswith(urls)) # pylint: disable=no-member + if settings.ROOT_URLCONF == 'lms.urls': + url_pattern = '/dashboard' + else: + url_pattern = '/home' + + self.assertTrue(response.url.endswith(url_pattern)) # pylint: disable=no-member def _auto_auth(self, params=None, status_code=None, **kwargs): """