From 8b8d694abd3d0ab27692b5a32cf640d83dcb0a58 Mon Sep 17 00:00:00 2001 From: dandds Date: Fri, 28 Sep 2018 15:39:35 -0400 Subject: [PATCH 1/6] apply destination url path as next parameter to login redirect --- atst/domain/auth.py | 2 +- atst/routes/__init__.py | 1 + tests/test_auth.py | 11 +++++++++-- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/atst/domain/auth.py b/atst/domain/auth.py index 9af97dd7..999553dc 100644 --- a/atst/domain/auth.py +++ b/atst/domain/auth.py @@ -21,7 +21,7 @@ def apply_authentication(app): if user: g.current_user = user elif not _unprotected_route(request): - return redirect(url_for("atst.root")) + return redirect(url_for("atst.root", next=request.path)) def get_current_user(): diff --git a/atst/routes/__init__.py b/atst/routes/__init__.py index 4edbdb57..fd5b213a 100644 --- a/atst/routes/__init__.py +++ b/atst/routes/__init__.py @@ -1,4 +1,5 @@ from flask import Blueprint, render_template, g, redirect, session, url_for, request + from flask import current_app as app import pendulum diff --git a/tests/test_auth.py b/tests/test_auth.py index 47d7c6e1..85bc62e0 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -88,12 +88,19 @@ def test_protected_routes_redirect_to_login(client, app): if "GET" in rule.methods: resp = client.get(protected_route) assert resp.status_code == 302 - assert resp.headers["Location"] == "http://localhost/" + assert "http://localhost/" in resp.headers["Location"] if "POST" in rule.methods: resp = client.post(protected_route) assert resp.status_code == 302 - assert resp.headers["Location"] == "http://localhost/" + assert "http://localhost/" in resp.headers["Location"] + + +def test_get_protected_route_encodes_redirect(client): + workspace_index = url_for("workspaces.workspaces") + response = client.get(workspace_index) + redirect = url_for("atst.root", next=workspace_index) + assert redirect in response.headers["Location"] def test_unprotected_routes_set_user_if_logged_in(client, app, user_session): From b137a41a8f237c2d79ee0d0364b1328ce769e4e8 Mon Sep 17 00:00:00 2001 From: dandds Date: Fri, 28 Sep 2018 15:46:34 -0400 Subject: [PATCH 2/6] logout should redirect to root, not home --- atst/routes/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/atst/routes/__init__.py b/atst/routes/__init__.py index fd5b213a..c3536ae5 100644 --- a/atst/routes/__init__.py +++ b/atst/routes/__init__.py @@ -84,7 +84,7 @@ def login_redirect(): @bp.route("/logout") def logout(): _logout() - return redirect(url_for(".home")) + return redirect(url_for(".root")) @bp.route("/activity-history") From 9f4b284d6ced327fa4db399223aa15867e1cec5e Mon Sep 17 00:00:00 2001 From: dandds Date: Mon, 1 Oct 2018 13:53:09 -0400 Subject: [PATCH 3/6] add login redirect query param to login button --- atst/routes/__init__.py | 11 ++++++++++- templates/login.html | 11 +++++++++-- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/atst/routes/__init__.py b/atst/routes/__init__.py index c3536ae5..beaa0c8e 100644 --- a/atst/routes/__init__.py +++ b/atst/routes/__init__.py @@ -1,3 +1,4 @@ +import urllib.parse as url from flask import Blueprint, render_template, g, redirect, session, url_for, request from flask import current_app as app @@ -16,7 +17,15 @@ bp = Blueprint("atst", __name__) @bp.route("/") def root(): - return render_template("login.html") + redirect_url = app.config.get("CAC_URL") + if request.args.get("next"): + redirect_url = url.urljoin( + redirect_url, "?{}".format(url.urlencode(request.args)) + ) + + return render_template( + "login.html", redirect=bool(request.args.get("next")), redirect_url=redirect_url + ) @bp.route("/help") diff --git a/templates/login.html b/templates/login.html index 7beaee66..0ff48b72 100644 --- a/templates/login.html +++ b/templates/login.html @@ -15,13 +15,20 @@ - + {% if g.dev %} - DEV Login + DEV Login {% endif %} + {% if redirect %} + {{ Alert('Log in Required.', + message='After you log in, you will be redirected to your destination page.', + level='warning' + ) }} + {% endif %} + {{ Alert('Certificate Selection', message='When you are prompted to select a certificate, please select E-mail Certificate from the provided choices.', actions=[ From 8a89c519ebd26116435c518e7fe8c2550662b44f Mon Sep 17 00:00:00 2001 From: dandds Date: Mon, 1 Oct 2018 14:22:44 -0400 Subject: [PATCH 4/6] on user login, redirect based on next query parameter if available --- atst/routes/__init__.py | 9 ++++++++- atst/routes/dev.py | 3 ++- tests/test_auth.py | 17 +++++++++++++++-- 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/atst/routes/__init__.py b/atst/routes/__init__.py index beaa0c8e..18e719f4 100644 --- a/atst/routes/__init__.py +++ b/atst/routes/__init__.py @@ -80,6 +80,13 @@ def _make_authentication_context(): ) +def redirect_url(): + if request.args.get("next"): + return request.args.get("next") + else: + return url_for(".home") + + @bp.route("/login-redirect") def login_redirect(): auth_context = _make_authentication_context() @@ -87,7 +94,7 @@ def login_redirect(): user = auth_context.get_user() session["user_id"] = user.id - return redirect(url_for(".home")) + return redirect(redirect_url()) @bp.route("/logout") diff --git a/atst/routes/dev.py b/atst/routes/dev.py index 0b53daf0..1deff0ac 100644 --- a/atst/routes/dev.py +++ b/atst/routes/dev.py @@ -1,5 +1,6 @@ from flask import Blueprint, request, session, redirect, url_for +from . import redirect_url from atst.domain.users import Users bp = Blueprint("dev", __name__) @@ -63,4 +64,4 @@ def login_dev(): ) session["user_id"] = user.id - return redirect(url_for("atst.home")) + return redirect(redirect_url()) diff --git a/tests/test_auth.py b/tests/test_auth.py index 85bc62e0..87e90f77 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -17,9 +17,9 @@ def _fetch_user_info(c, t): return MOCK_USER -def _login(client, verify="SUCCESS", sdn=DOD_SDN, cert=""): +def _login(client, verify="SUCCESS", sdn=DOD_SDN, cert="", **url_query_args): return client.get( - url_for("atst.login_redirect"), + url_for("atst.login_redirect", **url_query_args), environ_base={ "HTTP_X_SSL_CLIENT_VERIFY": verify, "HTTP_X_SSL_CLIENT_S_DN": sdn, @@ -185,3 +185,16 @@ def test_logout(app, client, monkeypatch): assert resp_failure.status_code == 302 destination = urlparse(resp_failure.headers["Location"]).path assert destination == url_for("atst.root") + + +def test_redirected_on_login(client, monkeypatch): + monkeypatch.setattr( + "atst.domain.authnid.AuthenticationContext.authenticate", lambda *args: True + ) + monkeypatch.setattr( + "atst.domain.authnid.AuthenticationContext.get_user", + lambda *args: UserFactory.create(), + ) + target_route = url_for("requests.requests_form_new", screen=1) + response = _login(client, next=target_route) + assert target_route in response.headers.get("Location") From 4eaea6c0558d10b5d470b848bf895547df5ed3a2 Mon Sep 17 00:00:00 2001 From: dandds Date: Mon, 1 Oct 2018 14:37:37 -0400 Subject: [PATCH 5/6] fix unused import --- atst/routes/dev.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/atst/routes/dev.py b/atst/routes/dev.py index 1deff0ac..02d290cc 100644 --- a/atst/routes/dev.py +++ b/atst/routes/dev.py @@ -1,4 +1,4 @@ -from flask import Blueprint, request, session, redirect, url_for +from flask import Blueprint, request, session, redirect from . import redirect_url from atst.domain.users import Users From 509b4c55a2faa23f8ee49e05ec81b16b501e42f2 Mon Sep 17 00:00:00 2001 From: dandds Date: Mon, 1 Oct 2018 16:16:17 -0400 Subject: [PATCH 6/6] more explicit naming, query param handling for login redirects --- atst/routes/__init__.py | 9 +++++---- atst/routes/dev.py | 4 ++-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/atst/routes/__init__.py b/atst/routes/__init__.py index 18e719f4..d13bdb36 100644 --- a/atst/routes/__init__.py +++ b/atst/routes/__init__.py @@ -20,7 +20,8 @@ def root(): redirect_url = app.config.get("CAC_URL") if request.args.get("next"): redirect_url = url.urljoin( - redirect_url, "?{}".format(url.urlencode(request.args)) + redirect_url, + "?{}".format(url.urlencode({"next": request.args.get("next")})), ) return render_template( @@ -80,11 +81,11 @@ def _make_authentication_context(): ) -def redirect_url(): +def redirect_after_login_url(): if request.args.get("next"): return request.args.get("next") else: - return url_for(".home") + return url_for("atst.home") @bp.route("/login-redirect") @@ -94,7 +95,7 @@ def login_redirect(): user = auth_context.get_user() session["user_id"] = user.id - return redirect(redirect_url()) + return redirect(redirect_after_login_url()) @bp.route("/logout") diff --git a/atst/routes/dev.py b/atst/routes/dev.py index 02d290cc..e9b107e8 100644 --- a/atst/routes/dev.py +++ b/atst/routes/dev.py @@ -1,6 +1,6 @@ from flask import Blueprint, request, session, redirect -from . import redirect_url +from . import redirect_after_login_url from atst.domain.users import Users bp = Blueprint("dev", __name__) @@ -64,4 +64,4 @@ def login_dev(): ) session["user_id"] = user.id - return redirect(redirect_url()) + return redirect(redirect_after_login_url())