Merge pull request #14549 from edx/release-mergeback-to-master
Merge release back to master
This commit is contained in:
@@ -3,7 +3,7 @@ from datetime import datetime
|
||||
import logging
|
||||
import urllib
|
||||
|
||||
from django.conf import settings
|
||||
from pytz import UTC
|
||||
from django.core.urlresolvers import reverse, NoReverseMatch
|
||||
from django.utils import http
|
||||
from oauth2_provider.models import (
|
||||
@@ -14,7 +14,6 @@ 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
|
||||
@@ -244,7 +243,17 @@ def get_next_url_for_login_page(request):
|
||||
Otherwise, we go to the ?next= query param or to the dashboard if nothing else is
|
||||
specified.
|
||||
"""
|
||||
redirect_to = get_redirect_to(request)
|
||||
redirect_to = request.GET.get('next', None)
|
||||
|
||||
# if we get a redirect parameter, make sure it's safe. If it's not, drop the
|
||||
# parameter.
|
||||
if redirect_to and not http.is_safe_url(redirect_to):
|
||||
log.error(
|
||||
u'Unsafe redirect parameter detected: %(redirect_to)r',
|
||||
{"redirect_to": redirect_to}
|
||||
)
|
||||
redirect_to = None
|
||||
|
||||
if not redirect_to:
|
||||
try:
|
||||
redirect_to = reverse('dashboard')
|
||||
@@ -261,33 +270,6 @@ def get_next_url_for_login_page(request):
|
||||
return redirect_to
|
||||
|
||||
|
||||
def get_redirect_to(request):
|
||||
"""
|
||||
Determine the redirect url and return if safe
|
||||
:argument
|
||||
request: request object
|
||||
|
||||
:returns: redirect url if safe else None
|
||||
"""
|
||||
redirect_to = request.GET.get('next')
|
||||
header_accept = request.META.get('HTTP_ACCEPT', '')
|
||||
|
||||
# If we get a redirect parameter, make sure it's safe i.e. not redirecting outside our domain.
|
||||
# Also make sure that it is not redirecting to a static asset and redirected page is web page
|
||||
# not a static file. As allowing assets to be pointed to by "next" allows 3rd party sites to
|
||||
# get information about a user on edx.org. In any such case drop the parameter.
|
||||
if redirect_to and (not http.is_safe_url(redirect_to)
|
||||
or settings.STATIC_URL in redirect_to
|
||||
or 'text/html' not in header_accept):
|
||||
log.warning(
|
||||
u'Unsafe redirect parameter detected after login page: %(redirect_to)r',
|
||||
{"redirect_to": redirect_to}
|
||||
)
|
||||
redirect_to = None
|
||||
|
||||
return redirect_to
|
||||
|
||||
|
||||
def destroy_oauth_tokens(user):
|
||||
"""
|
||||
Destroys ALL OAuth access and refresh tokens for the given user.
|
||||
|
||||
@@ -1,9 +1,7 @@
|
||||
""" Test Student helpers """
|
||||
|
||||
import logging
|
||||
import ddt
|
||||
|
||||
from django.conf import settings
|
||||
from django.core.urlresolvers import reverse
|
||||
from django.test import TestCase
|
||||
from django.test.client import RequestFactory
|
||||
@@ -15,34 +13,24 @@ from student.helpers import get_next_url_for_login_page
|
||||
LOGGER_NAME = "student.helpers"
|
||||
|
||||
|
||||
@ddt.ddt
|
||||
class TestLoginHelper(TestCase):
|
||||
"""Test login helper methods."""
|
||||
def setUp(self):
|
||||
super(TestLoginHelper, self).setUp()
|
||||
self.request = RequestFactory()
|
||||
|
||||
@ddt.data(
|
||||
("https://www.amazon.com", "text/html"),
|
||||
("favicon.ico", "image/*"),
|
||||
("https://www.test.com/test.jpg", "image/*"),
|
||||
(settings.STATIC_URL + "dummy.png", "image/*"),
|
||||
)
|
||||
@ddt.unpack
|
||||
def test_unsafe_next(self, unsafe_url, http_accept):
|
||||
def test_unsafe_next(self):
|
||||
""" Test unsafe next parameter """
|
||||
with LogCapture(LOGGER_NAME, level=logging.WARNING) as logger:
|
||||
unsafe_url = "https://www.amazon.com"
|
||||
with LogCapture(LOGGER_NAME, level=logging.ERROR) as logger:
|
||||
req = self.request.get(reverse("login") + "?next={url}".format(url=unsafe_url))
|
||||
req.META["HTTP_ACCEPT"] = http_accept # pylint: disable=no-member
|
||||
get_next_url_for_login_page(req)
|
||||
logger.check(
|
||||
(LOGGER_NAME, "WARNING",
|
||||
u"Unsafe redirect parameter detected after login page: u'{url}'".format(url=unsafe_url))
|
||||
(LOGGER_NAME, "ERROR", u"Unsafe redirect parameter detected: u'{url}'".format(url=unsafe_url))
|
||||
)
|
||||
|
||||
def test_safe_next(self):
|
||||
""" Test safe next parameter """
|
||||
req = self.request.get(reverse("login") + "?next={url}".format(url="/dashboard"))
|
||||
req.META["HTTP_ACCEPT"] = "text/html" # pylint: disable=no-member
|
||||
next_page = get_next_url_for_login_page(req)
|
||||
self.assertEqual(next_page, u'/dashboard')
|
||||
|
||||
@@ -498,7 +498,7 @@ class ExternalAuthShibTest(ModuleStoreTestCase):
|
||||
Should vary by course depending on its enrollment_domain
|
||||
"""
|
||||
TARGET_URL = reverse('courseware', args=[self.course.id.to_deprecated_string()]) # pylint: disable=invalid-name
|
||||
noshib_response = self.client.get(TARGET_URL, follow=True, HTTP_ACCEPT="text/html")
|
||||
noshib_response = self.client.get(TARGET_URL, follow=True)
|
||||
self.assertEqual(noshib_response.redirect_chain[-1],
|
||||
('http://testserver/login?next={url}'.format(url=TARGET_URL), 302))
|
||||
self.assertContains(noshib_response, (u"Sign in or Register | {platform_name}"
|
||||
@@ -509,8 +509,7 @@ class ExternalAuthShibTest(ModuleStoreTestCase):
|
||||
shib_response = self.client.get(**{'path': TARGET_URL_SHIB,
|
||||
'follow': True,
|
||||
'REMOTE_USER': self.extauth.external_id,
|
||||
'Shib-Identity-Provider': 'https://idp.stanford.edu/',
|
||||
'HTTP_ACCEPT': "text/html"})
|
||||
'Shib-Identity-Provider': 'https://idp.stanford.edu/'})
|
||||
# Test that the shib-login redirect page with ?next= and the desired page are part of the redirect chain
|
||||
# The 'courseware' page actually causes a redirect itself, so it's not the end of the chain and we
|
||||
# won't test its contents
|
||||
|
||||
@@ -90,7 +90,7 @@ class LoginFormTest(ThirdPartyAuthTestMixin, UrlResetMixin, SharedModuleStoreTes
|
||||
def test_courseware_redirect(self, backend_name):
|
||||
# Try to access courseware while logged out, expecting to be
|
||||
# redirected to the login page.
|
||||
response = self.client.get(self.courseware_url, follow=True, HTTP_ACCEPT="text/html")
|
||||
response = self.client.get(self.courseware_url, follow=True)
|
||||
self.assertRedirects(
|
||||
response,
|
||||
u"{url}?next={redirect_url}".format(
|
||||
@@ -118,7 +118,7 @@ class LoginFormTest(ThirdPartyAuthTestMixin, UrlResetMixin, SharedModuleStoreTes
|
||||
('email_opt_in', 'true'),
|
||||
('next', '/custom/final/destination'),
|
||||
]
|
||||
response = self.client.get(self.url, params, HTTP_ACCEPT="text/html")
|
||||
response = self.client.get(self.url, params)
|
||||
expected_url = _third_party_login_url(
|
||||
backend_name,
|
||||
"login",
|
||||
@@ -137,7 +137,7 @@ class LoginFormTest(ThirdPartyAuthTestMixin, UrlResetMixin, SharedModuleStoreTes
|
||||
]
|
||||
|
||||
# Get the login page
|
||||
response = self.client.get(self.url, params, HTTP_ACCEPT="text/html")
|
||||
response = self.client.get(self.url, params)
|
||||
|
||||
# Verify that the parameters are sent on to the next page correctly
|
||||
post_login_handler = _finish_auth_url(params)
|
||||
@@ -194,7 +194,7 @@ class RegisterFormTest(ThirdPartyAuthTestMixin, UrlResetMixin, SharedModuleStore
|
||||
('email_opt_in', 'true'),
|
||||
('next', '/custom/final/destination'),
|
||||
]
|
||||
response = self.client.get(self.url, params, HTTP_ACCEPT="text/html")
|
||||
response = self.client.get(self.url, params)
|
||||
expected_url = _third_party_login_url(
|
||||
backend_name,
|
||||
"register",
|
||||
@@ -213,7 +213,7 @@ class RegisterFormTest(ThirdPartyAuthTestMixin, UrlResetMixin, SharedModuleStore
|
||||
]
|
||||
|
||||
# Get the login page
|
||||
response = self.client.get(self.url, params, HTTP_ACCEPT="text/html")
|
||||
response = self.client.get(self.url, params)
|
||||
|
||||
# Verify that the parameters are sent on to the next page correctly
|
||||
post_login_handler = _finish_auth_url(params)
|
||||
|
||||
@@ -336,7 +336,7 @@ class StudentAccountLoginAndRegistrationTest(ThirdPartyAuthTestMixin, UrlResetMi
|
||||
# The response should have a "Sign In" button with the URL
|
||||
# that preserves the querystring params
|
||||
with with_comprehensive_theme_context(theme):
|
||||
response = self.client.get(reverse(url_name), params, HTTP_ACCEPT="text/html")
|
||||
response = self.client.get(reverse(url_name), params)
|
||||
|
||||
expected_url = '/login?{}'.format(self._finish_auth_url_param(params + [('next', '/dashboard')]))
|
||||
self.assertContains(response, expected_url)
|
||||
@@ -352,7 +352,7 @@ class StudentAccountLoginAndRegistrationTest(ThirdPartyAuthTestMixin, UrlResetMi
|
||||
|
||||
# Verify that this parameter is also preserved
|
||||
with with_comprehensive_theme_context(theme):
|
||||
response = self.client.get(reverse(url_name), params, HTTP_ACCEPT="text/html")
|
||||
response = self.client.get(reverse(url_name), params)
|
||||
|
||||
expected_url = '/login?{}'.format(self._finish_auth_url_param(params))
|
||||
self.assertContains(response, expected_url)
|
||||
@@ -387,11 +387,11 @@ class StudentAccountLoginAndRegistrationTest(ThirdPartyAuthTestMixin, UrlResetMi
|
||||
if current_backend is not None:
|
||||
pipeline_target = "student_account.views.third_party_auth.pipeline"
|
||||
with simulate_running_pipeline(pipeline_target, current_backend):
|
||||
response = self.client.get(reverse(url_name), params, HTTP_ACCEPT="text/html")
|
||||
response = self.client.get(reverse(url_name), params)
|
||||
|
||||
# Do NOT simulate a running pipeline
|
||||
else:
|
||||
response = self.client.get(reverse(url_name), params, HTTP_ACCEPT="text/html")
|
||||
response = self.client.get(reverse(url_name), params)
|
||||
|
||||
# This relies on the THIRD_PARTY_AUTH configuration in the test settings
|
||||
expected_providers = [
|
||||
@@ -424,7 +424,7 @@ class StudentAccountLoginAndRegistrationTest(ThirdPartyAuthTestMixin, UrlResetMi
|
||||
|
||||
def test_hinted_login(self):
|
||||
params = [("next", "/courses/something/?tpa_hint=oa2-google-oauth2")]
|
||||
response = self.client.get(reverse('signin_user'), params, HTTP_ACCEPT="text/html")
|
||||
response = self.client.get(reverse('signin_user'), params)
|
||||
self.assertContains(response, '"third_party_auth_hint": "oa2-google-oauth2"')
|
||||
|
||||
@override_settings(SITE_NAME=settings.MICROSITE_TEST_HOSTNAME)
|
||||
|
||||
@@ -569,8 +569,7 @@ class ShibSPTestModifiedCourseware(ModuleStoreTestCase):
|
||||
'data': dict(params),
|
||||
'follow': False,
|
||||
'REMOTE_USER': 'testuser@stanford.edu',
|
||||
'Shib-Identity-Provider': 'https://idp.stanford.edu/',
|
||||
'HTTP_ACCEPT': "text/html"}
|
||||
'Shib-Identity-Provider': 'https://idp.stanford.edu/'}
|
||||
response = self.client.get(**request_kwargs)
|
||||
# successful login is a redirect to the URL that handles auto-enrollment
|
||||
self.assertEqual(response.status_code, 302)
|
||||
|
||||
Reference in New Issue
Block a user