Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #14570 from edx/ahsan/ECOM-6463-login-should-not-point-asset-new

Browse Source 
login should not point asset new
This commit is contained in:
Ahsan Ulhaq
2017-03-08 18:26:07 +05:00
committed by GitHub
parent 078974d24a 7712bf3151
commit e51c692ef8
6 changed files with 106 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

76
common/djangoapps/student/helpers.py
View File

@@ -2,8 +2,9 @@
from datetime import datetime
import logging
import urllib
import mimetypes
from pytz import UTC
from django.conf import settings
from django.core.urlresolvers import reverse, NoReverseMatch
from django.utils import http
from oauth2_provider.models import (
@@ -14,10 +15,12 @@ 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
from course_modes.models import CourseMode
from openedx.core.djangoapps.theming.helpers import get_themes
# Enumeration of per-course verification statuses
@@ -243,17 +246,7 @@ 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 = 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
redirect_to = get_redirect_to(request)
if not redirect_to:
try:
redirect_to = reverse('dashboard')
@@ -270,6 +263,65 @@ 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:
mime_type, _ = mimetypes.guess_type(redirect_to, strict=False)
if not http.is_safe_url(redirect_to):
log.warning(
u'Unsafe redirect parameter detected after login page: %(redirect_to)r',
{"redirect_to": redirect_to}
)
redirect_to = None
elif 'text/html' not in header_accept:
log.warning(
u'Redirect to non html content %(content_type)r detected from %(user_agent)r'
u' after login page: %(redirect_to)r',
{
"redirect_to": redirect_to, "content_type": header_accept,
"user_agent": request.META.get('HTTP_USER_AGENT', '')
}
)
redirect_to = None
elif mime_type:
log.warning(
u'Redirect to url path with specified filed type %(mime_type)r not allowed: %(redirect_to)r',
{"redirect_to": redirect_to, "mime_type": mime_type}
)
redirect_to = None
elif settings.STATIC_URL in redirect_to:
log.warning(
u'Redirect to static content detected after login page: %(redirect_to)r',
{"redirect_to": redirect_to}
)
redirect_to = None
else:
themes = get_themes()
for theme in themes:
if theme.theme_dir_name in redirect_to:
log.warning(
u'Redirect to theme content detected after login page: %(redirect_to)r',
{"redirect_to": redirect_to}
)
redirect_to = None
break
return redirect_to
def destroy_oauth_tokens(user):
"""
Destroys ALL OAuth access and refresh tokens for the given user.

31
common/djangoapps/student/tests/test_helpers.py
View File

@@ -1,7 +1,9 @@
""" 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
@@ -13,24 +15,45 @@ from student.helpers import get_next_url_for_login_page
LOGGER_NAME = "student.helpers"
@ddt.ddt
class TestLoginHelper(TestCase):
"""Test login helper methods."""
static_url = settings.STATIC_URL
def setUp(self):
super(TestLoginHelper, self).setUp()
self.request = RequestFactory()
def test_unsafe_next(self):
@ddt.data(
("https://www.amazon.com", "text/html", None,
"Unsafe redirect parameter detected after login page: u'https://www.amazon.com'"),
("favicon.ico", "image/*", "test/agent",
"Redirect to non html content 'image/*' detected from 'test/agent' after login page: u'favicon.ico'"),
("https://www.test.com/test.jpg", "image/*", None,
"Unsafe redirect parameter detected after login page: u'https://www.test.com/test.jpg'"),
(static_url + "dummy.png", "image/*", "test/agent",
"Redirect to non html content 'image/*' detected from 'test/agent' after login page: u'" + static_url +
"dummy.png" + "'"),
("test.png", "text/html", None,
"Redirect to url path with specified filed type 'image/png' not allowed: u'test.png'"),
(static_url + "dummy.png", "text/html", None,
"Redirect to url path with specified filed type 'image/png' not allowed: u'" + static_url + "dummy.png" + "'"),
)
@ddt.unpack
def test_unsafe_next(self, unsafe_url, http_accept, user_agent, expected_log):
""" Test unsafe next parameter """
unsafe_url = "https://www.amazon.com"
with LogCapture(LOGGER_NAME, level=logging.ERROR) as logger:
with LogCapture(LOGGER_NAME, level=logging.WARNING) as logger:
req = self.request.get(reverse("login") + "?next={url}".format(url=unsafe_url))
req.META["HTTP_ACCEPT"] = http_accept # pylint: disable=no-member
req.META["HTTP_USER_AGENT"] = user_agent # pylint: disable=no-member
get_next_url_for_login_page(req)
logger.check(
(LOGGER_NAME, "ERROR", u"Unsafe redirect parameter detected: u'{url}'".format(url=unsafe_url))
(LOGGER_NAME, "WARNING", expected_log)
)
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')

5
common/djangoapps/student/tests/test_login.py
View File

@@ -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)
noshib_response = self.client.get(TARGET_URL, follow=True, HTTP_ACCEPT="text/html")
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,7 +509,8 @@ 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/'})
'Shib-Identity-Provider': 'https://idp.stanford.edu/',
'HTTP_ACCEPT': "text/html"})
# 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

10
common/djangoapps/student/tests/test_login_registration_forms.py
View File

@@ -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)
response = self.client.get(self.courseware_url, follow=True, HTTP_ACCEPT="text/html")
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)
response = self.client.get(self.url, params, HTTP_ACCEPT="text/html")
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)
response = self.client.get(self.url, params, HTTP_ACCEPT="text/html")
# 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)
response = self.client.get(self.url, params, HTTP_ACCEPT="text/html")
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)
response = self.client.get(self.url, params, HTTP_ACCEPT="text/html")
# Verify that the parameters are sent on to the next page correctly
post_login_handler = _finish_auth_url(params)

10
lms/djangoapps/student_account/test/test_views.py
View File

@@ -341,7 +341,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)
response = self.client.get(reverse(url_name), params, HTTP_ACCEPT="text/html")
expected_url = '/login?{}'.format(self._finish_auth_url_param(params + [('next', '/dashboard')]))
self.assertContains(response, expected_url)
@@ -357,7 +357,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)
response = self.client.get(reverse(url_name), params, HTTP_ACCEPT="text/html")
expected_url = '/login?{}'.format(self._finish_auth_url_param(params))
self.assertContains(response, expected_url)
@@ -392,11 +392,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)
response = self.client.get(reverse(url_name), params, HTTP_ACCEPT="text/html")
# Do NOT simulate a running pipeline
else:
response = self.client.get(reverse(url_name), params)
response = self.client.get(reverse(url_name), params, HTTP_ACCEPT="text/html")
# This relies on the THIRD_PARTY_AUTH configuration in the test settings
expected_providers = [
@@ -429,7 +429,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)
response = self.client.get(reverse('signin_user'), params, HTTP_ACCEPT="text/html")
self.assertContains(response, '"third_party_auth_hint": "oa2-google-oauth2"')
tpa_hint = self.hidden_enabled_provider.provider_id

3
openedx/core/djangoapps/external_auth/tests/test_shib.py
View File

@@ -569,7 +569,8 @@ class ShibSPTestModifiedCourseware(ModuleStoreTestCase):
'data': dict(params),
'follow': False,
'REMOTE_USER': 'testuser@stanford.edu',
'Shib-Identity-Provider': 'https://idp.stanford.edu/'}
'Shib-Identity-Provider': 'https://idp.stanford.edu/',
'HTTP_ACCEPT': "text/html"}
response = self.client.get(**request_kwargs)
# successful login is a redirect to the URL that handles auto-enrollment
self.assertEqual(response.status_code, 302)