From 4df0efa250a070c584b8a93353cb106a63a0f9e2 Mon Sep 17 00:00:00 2001 From: Anurag Ramdasan Date: Wed, 11 Jun 2014 19:04:16 +0000 Subject: [PATCH 1/9] decode uri component before redirect for safe redirect --- lms/templates/login.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/templates/login.html b/lms/templates/login.html index 5d79076227..db46d5e9c3 100644 --- a/lms/templates/login.html +++ b/lms/templates/login.html @@ -51,7 +51,7 @@ $('#login-form').on('ajax:success', function(event, json, xhr) { if(json.success) { var u=decodeURI(window.location.search); - next=u.split("next=")[1]; + next=decodeURIComponent(u.split("next=")[1]); if (next && !isExternal(next)) { location.href=next; } else if(json.redirect_url){ From c4fa475c7ac98c70b5f4bd295b4b79396737a0a0 Mon Sep 17 00:00:00 2001 From: Anurag Ramdasan Date: Fri, 13 Jun 2014 22:42:45 +0000 Subject: [PATCH 2/9] add var declaration for next --- lms/templates/login.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/templates/login.html b/lms/templates/login.html index db46d5e9c3..85d0eb4ae1 100644 --- a/lms/templates/login.html +++ b/lms/templates/login.html @@ -51,7 +51,7 @@ $('#login-form').on('ajax:success', function(event, json, xhr) { if(json.success) { var u=decodeURI(window.location.search); - next=decodeURIComponent(u.split("next=")[1]); + var next=decodeURIComponent(u.split("next=")[1]); if (next && !isExternal(next)) { location.href=next; } else if(json.redirect_url){ From f77c06ff1f2bfd6eede2d8bcd0077cdae6a15cbd Mon Sep 17 00:00:00 2001 From: Anurag Ramdasan Date: Tue, 22 Jul 2014 03:34:08 +0000 Subject: [PATCH 3/9] check if next is not defined --- lms/templates/login.html | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lms/templates/login.html b/lms/templates/login.html index 85d0eb4ae1..6670738af9 100644 --- a/lms/templates/login.html +++ b/lms/templates/login.html @@ -51,7 +51,11 @@ $('#login-form').on('ajax:success', function(event, json, xhr) { if(json.success) { var u=decodeURI(window.location.search); - var next=decodeURIComponent(u.split("next=")[1]); + var next = u.split("next=")[1]; + if (next != undefined) { + // if next is undefined, decodeURI returns "undefined" causing a bad redirect. + next = decodeURI(next); + } if (next && !isExternal(next)) { location.href=next; } else if(json.redirect_url){ From d9bb2868e8f71d909df6cb62d5e645c68dfb2d66 Mon Sep 17 00:00:00 2001 From: Ned Batchelder Date: Tue, 22 Jul 2014 11:24:53 -0400 Subject: [PATCH 4/9] Add a view that can show us what URL params we got --- lms/djangoapps/debug/views.py | 13 +++++++++++++ lms/urls.py | 4 ++++ 2 files changed, 17 insertions(+) diff --git a/lms/djangoapps/debug/views.py b/lms/djangoapps/debug/views.py index 5a5927e350..53c8d3ceff 100644 --- a/lms/djangoapps/debug/views.py +++ b/lms/djangoapps/debug/views.py @@ -10,6 +10,9 @@ from edxmako.shortcuts import render_to_response from codejail.safe_exec import safe_exec +from util.json_request import JsonResponse + + @login_required @ensure_csrf_cookie def run_python(request): @@ -29,3 +32,13 @@ def run_python(request): else: c['results'] = pprint.pformat(g) return render_to_response("debug/run_python_form.html", c) + + +@login_required +def show_parameters(request): + """A page that shows what GET parameters were on the URL.""" + params = { + 'get': dict(request.GET), + 'post': dict(request.POST), + } + return JsonResponse(params) diff --git a/lms/urls.py b/lms/urls.py index 4946586316..a1b5084ac3 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -481,6 +481,10 @@ if settings.FEATURES.get('ENABLE_DEBUG_RUN_PYTHON'): url(r'^debug/run_python', 'debug.views.run_python'), ) +urlpatterns += ( + url(r'^debug/show_parameters', 'debug.views.show_parameters'), +) + # Crowdsourced hinting instructor manager. if settings.FEATURES.get('ENABLE_HINTER_INSTRUCTOR_VIEW'): urlpatterns += ( From 412e6d34998fabb908d395425d9822ad280bcc3a Mon Sep 17 00:00:00 2001 From: Ned Batchelder Date: Wed, 23 Jul 2014 10:56:43 -0400 Subject: [PATCH 5/9] Add a more informative assert to a lettuce helper --- common/djangoapps/terrain/steps.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/common/djangoapps/terrain/steps.py b/common/djangoapps/terrain/steps.py index 32dc4db432..5e20b0a1d0 100644 --- a/common/djangoapps/terrain/steps.py +++ b/common/djangoapps/terrain/steps.py @@ -81,7 +81,9 @@ def click_the_link_with_the_text_group1(step, linktext): @step('I should see that the path is "([^"]*)"$') def i_should_see_that_the_path_is(step, path): - assert world.url_equals(path) + assert world.url_equals(path), ( + "path should be {!r} but is {!r}".format(path, world.browser.url) + ) @step(u'the page title should be "([^"]*)"$') From 533e072677f429815af71c10b5f26e1a9f7258f4 Mon Sep 17 00:00:00 2001 From: Ned Batchelder Date: Wed, 23 Jul 2014 10:58:02 -0400 Subject: [PATCH 6/9] A debug view for seeing the URL params --- lms/djangoapps/debug/views.py | 19 ++++++++++--------- lms/urls.py | 4 ++-- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/lms/djangoapps/debug/views.py b/lms/djangoapps/debug/views.py index 53c8d3ceff..6794c30348 100644 --- a/lms/djangoapps/debug/views.py +++ b/lms/djangoapps/debug/views.py @@ -3,15 +3,15 @@ import pprint import traceback -from django.http import Http404 +from django.http import Http404, HttpResponse from django.contrib.auth.decorators import login_required +from django.utils.html import escape + from django_future.csrf import ensure_csrf_cookie from edxmako.shortcuts import render_to_response from codejail.safe_exec import safe_exec -from util.json_request import JsonResponse - @login_required @ensure_csrf_cookie @@ -36,9 +36,10 @@ def run_python(request): @login_required def show_parameters(request): - """A page that shows what GET parameters were on the URL.""" - params = { - 'get': dict(request.GET), - 'post': dict(request.POST), - } - return JsonResponse(params) + """A page that shows what parameters were on the URL and post.""" + html = [] + for name, value in sorted(request.GET.items()): + html.append(escape("GET {}: {!r}".format(name, value))) + for name, value in sorted(request.POST.items()): + html.append(escape("POST {}: {!r}".format(name, value))) + return HttpResponse("\n".join("

{}

".format(h) for h in html)) diff --git a/lms/urls.py b/lms/urls.py index a1b5084ac3..f304e2c13c 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -478,11 +478,11 @@ urlpatterns += ( if settings.FEATURES.get('ENABLE_DEBUG_RUN_PYTHON'): urlpatterns += ( - url(r'^debug/run_python', 'debug.views.run_python'), + url(r'^debug/run_python$', 'debug.views.run_python'), ) urlpatterns += ( - url(r'^debug/show_parameters', 'debug.views.show_parameters'), + url(r'^debug/show_parameters$', 'debug.views.show_parameters'), ) # Crowdsourced hinting instructor manager. From dea5c7d3dbcc5363c80bbe66c64f04c81075a2ea Mon Sep 17 00:00:00 2001 From: Ned Batchelder Date: Wed, 23 Jul 2014 10:58:27 -0400 Subject: [PATCH 7/9] Add a test of login with params on the next URL --- lms/djangoapps/courseware/features/login.feature | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lms/djangoapps/courseware/features/login.feature b/lms/djangoapps/courseware/features/login.feature index 7af151ed26..3c4d006fb9 100644 --- a/lms/djangoapps/courseware/features/login.feature +++ b/lms/djangoapps/courseware/features/login.feature @@ -46,3 +46,13 @@ Feature: LMS.Login in as a registered user And I visit the url "/login?next=http://www.google.com/" When I submit my credentials on the login form Then I should be on the dashboard page + + Scenario: Login with a redirect with parameters + Given I am an edX user + And I am not logged in + And I visit the url "/debug/show_parameters?foo=hello&bar=world" + And I should see that the path is "/accounts/login?next=/debug/show_parameters%3Ffoo%3Dhello%26bar%3Dworld" + When I submit my credentials on the login form + And I wait for "2" seconds + Then I should see "foo: u'hello'" somewhere on the page + And I should see "bar: u'world'" somewhere on the page From fec283366fd5928cc81319b0c78cb9c39522c1ca Mon Sep 17 00:00:00 2001 From: Ned Batchelder Date: Wed, 23 Jul 2014 10:58:39 -0400 Subject: [PATCH 8/9] Fix the decoding of ?next= --- lms/templates/login.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lms/templates/login.html b/lms/templates/login.html index 6670738af9..a99a017a75 100644 --- a/lms/templates/login.html +++ b/lms/templates/login.html @@ -54,8 +54,8 @@ var next = u.split("next=")[1]; if (next != undefined) { // if next is undefined, decodeURI returns "undefined" causing a bad redirect. - next = decodeURI(next); - } + next = decodeURIComponent(next); + } if (next && !isExternal(next)) { location.href=next; } else if(json.redirect_url){ From 01cf702a61fcc4db29a30943e113d7011773804e Mon Sep 17 00:00:00 2001 From: Anurag Ramdasan Date: Wed, 23 Jul 2014 10:46:04 -0700 Subject: [PATCH 9/9] Merge pull request #1 from edx/ned/add-test-to-4080 Ned/add test to 4080