From 5b5cf7d3b029f59167e8f7ee00e8aa2c57da22db Mon Sep 17 00:00:00 2001 From: Adam Palay Date: Tue, 17 Nov 2015 10:47:50 -0500 Subject: [PATCH 1/3] Fix login redirect when user is logged out while answering a question (TNL-3789) --- .../acceptance/tests/lms/test_lms_problems.py | 94 ++++++++++++++++++- lms/static/js/ajax-error.js | 4 +- 2 files changed, 94 insertions(+), 4 deletions(-) diff --git a/common/test/acceptance/tests/lms/test_lms_problems.py b/common/test/acceptance/tests/lms/test_lms_problems.py index 789d0e63df..5cb41bb081 100644 --- a/common/test/acceptance/tests/lms/test_lms_problems.py +++ b/common/test/acceptance/tests/lms/test_lms_problems.py @@ -10,6 +10,7 @@ from ..helpers import UniqueCourseTest from ...pages.studio.auto_auth import AutoAuthPage from ...pages.lms.courseware import CoursewarePage from ...pages.lms.problem import ProblemPage +from ...pages.lms.login_and_register import CombinedLoginAndRegisterPage from ...fixtures.course import CourseFixture, XBlockFixtureDesc from ..helpers import EventsTestMixin @@ -20,6 +21,7 @@ class ProblemsTest(UniqueCourseTest): """ USERNAME = "joe_student" EMAIL = "joe@example.com" + PASSWORD = "keep it secret; keep it safe." def setUp(self): super(ProblemsTest, self).setUp() @@ -42,8 +44,14 @@ class ProblemsTest(UniqueCourseTest): ).install() # Auto-auth register for the course. - AutoAuthPage(self.browser, username=self.USERNAME, email=self.EMAIL, - course_id=self.course_id, staff=False).visit() + AutoAuthPage( + self.browser, + username=self.USERNAME, + email=self.EMAIL, + password=self.PASSWORD, + course_id=self.course_id, + staff=False + ).visit() def get_problem(self): """ Subclasses should override this to complete the fixture """ @@ -319,3 +327,85 @@ class ProblemPartialCredit(ProblemsTest): problem_page.fill_answer_numerical('-1') problem_page.click_check() self.assertTrue(problem_page.simpleprob_is_partially_correct()) + + +class LogoutDuringAnswering(ProblemsTest): + """ + Tests for the scenario where a user is logged out (their session expires + or is revoked) just before they click "check" on a problem. + """ + def get_problem(self): + """ + Create a problem. + """ + xml = dedent(""" + +

The answer is 1

+ + + + + + """) + return XBlockFixtureDesc('problem', 'TEST PROBLEM', data=xml) + + def log_user_out(self): + """ + Log the user out by deleting their session cookie. + """ + self.browser.delete_cookie('sessionid') + + def test_logout_after_click_redirect(self): + """ + 1) User goes to a problem page. + 2) User fills out an answer to the problem. + 3) User is logged out because their session id is invalidated or removed. + 4) User clicks "check", and sees a confirmation modal asking them to + re-authenticate, since they've just been logged out. + 5) User clicks "ok". + 6) User is redirected to the login page. + 7) User logs in. + 8) User is redirected back to the problem page they started out on. + 9) User is able to submit an answer + """ + self.courseware_page.visit() + problem_page = ProblemPage(self.browser) + self.assertEqual(problem_page.problem_name, 'TEST PROBLEM') + problem_page.fill_answer_numerical('1') + + self.log_user_out() + with problem_page.handle_alert(confirm=True): + problem_page.click_check() + + login_page = CombinedLoginAndRegisterPage(self.browser) + login_page.wait_for_page() + + login_page.login(self.EMAIL, self.PASSWORD) + + problem_page.wait_for_page() + self.assertEqual(problem_page.problem_name, 'TEST PROBLEM') + + problem_page.fill_answer_numerical('1') + problem_page.click_check() + self.assertTrue(problem_page.simpleprob_is_correct()) + + def test_logout_cancel_no_redirect(self): + """ + 1) User goes to a problem page. + 2) User fills out an answer to the problem. + 3) User is logged out because their session id is invalidated or removed. + 4) User clicks "check", and sees a confirmation modal asking them to + re-authenticate, since they've just been logged out. + 5) User clicks "cancel". + 6) User is not redirected to the login page. + """ + self.courseware_page.visit() + problem_page = ProblemPage(self.browser) + self.assertEqual(problem_page.problem_name, 'TEST PROBLEM') + problem_page.fill_answer_numerical('1') + self.log_user_out() + with problem_page.handle_alert(confirm=False): + problem_page.click_check() + + self.assertTrue(problem_page.is_browser_on_page()) + self.assertEqual(problem_page.problem_name, 'TEST PROBLEM') diff --git a/lms/static/js/ajax-error.js b/lms/static/js/ajax-error.js index 460d2511fe..6b29d4c24b 100644 --- a/lms/static/js/ajax-error.js +++ b/lms/static/js/ajax-error.js @@ -8,8 +8,8 @@ $(document).ajaxError(function (event, jXHR) { ); if (window.confirm(message)) { - var currentLocation = window.location.href; - window.location.href = '/login?next=' + currentLocation; + var currentLocation = window.location.pathname; + window.location.href = '/login?next=' + encodeURIComponent(currentLocation); }; } }); From 57a4df73a5d05315d85dbccc2a9101bf0e15dc61 Mon Sep 17 00:00:00 2001 From: Kevin Falcone Date: Tue, 17 Nov 2015 12:47:01 -0500 Subject: [PATCH 2/3] Management commands should no longer access args[] https://docs.djangoproject.com/en/1.8/howto/custom-management-commands/ Django intercepts the 'pull' argument and says "unrecognized arguments pull" There were no tests for this command, so it wasn't noticed during the upgrade testing, only once our integration monitoring noticed. --- .../third_party_auth/management/commands/saml.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/common/djangoapps/third_party_auth/management/commands/saml.py b/common/djangoapps/third_party_auth/management/commands/saml.py index 01918157ae..dd2c6c1049 100644 --- a/common/djangoapps/third_party_auth/management/commands/saml.py +++ b/common/djangoapps/third_party_auth/management/commands/saml.py @@ -12,16 +12,14 @@ class Command(BaseCommand): """ manage.py commands to manage SAML/Shibboleth SSO """ help = '''Configure/maintain/update SAML-based SSO''' - def handle(self, *args, **options): - if len(args) != 1: - raise CommandError("saml requires one argument: pull") + def add_arguments(self, parser): + parser.add_argument('--pull', action='store_true', help="Pull updated metadata from external IDPs") + def handle(self, *args, **options): if not SAMLConfiguration.is_enabled(): raise CommandError("SAML support is disabled via SAMLConfiguration.") - subcommand = args[0] - - if subcommand == "pull": + if options['pull']: log_handler = logging.StreamHandler(self.stdout) log_handler.setLevel(logging.DEBUG) log = logging.getLogger('third_party_auth.tasks') From 84328c13f6545313025c181e2945fcded713a921 Mon Sep 17 00:00:00 2001 From: Kevin Falcone Date: Thu, 19 Nov 2015 17:00:18 -0500 Subject: [PATCH 3/3] Pin to what we have in production This was installing a newer version, with a migration, which we do not want for a hotfix. --- requirements/edx/github.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index 8a3eef81ec..dcacfd1004 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -45,7 +45,7 @@ git+https://github.com/edx/nose.git@99c2aff0ff51bf228bfa5482e97e612c97a23245#egg -e git+https://github.com/edx/django-splash.git@ammar/upgrade-to-django1.8#egg=django-splash==0.2 -e git+https://github.com/edx/acid-block.git@e46f9cda8a03e121a00c7e347084d142d22ebfb7#egg=acid-xblock -e git+https://github.com/edx/edx-ora2.git@mzfr/django18-from-release#egg=ora2==0.2.2 --e git+https://github.com/edx/edx-submissions.git@django-upgrade/1.8#egg=edx-submissions==0.1.2 +-e git+https://github.com/edx/edx-submissions.git@d6fc357831fb85ee6fdd169d0e36bfd29b490e31#egg=edx-submissions==0.1.2 -e git+https://github.com/edx/opaque-keys.git@27dc382ea587483b1e3889a3d19cbd90b9023a06#egg=opaque-keys git+https://github.com/edx/ease.git@release-2015-07-14#egg=ease==0.1.3 git+https://github.com/edx/i18n-tools.git@v0.1.3#egg=i18n-tools==v0.1.3