From 0d1494ea115d1a81a32605eeb37d82b8b122d640 Mon Sep 17 00:00:00 2001 From: dandds Date: Mon, 17 Sep 2018 10:18:14 -0400 Subject: [PATCH 1/2] create logout endpoint that clears user data from session --- atst/routes/__init__.py | 13 ++++++------- templates/navigation/topbar.html | 2 +- tests/test_auth.py | 31 ++++++++++++++++++++++++++++++- 3 files changed, 37 insertions(+), 9 deletions(-) diff --git a/atst/routes/__init__.py b/atst/routes/__init__.py index fb53802a..cee24c23 100644 --- a/atst/routes/__init__.py +++ b/atst/routes/__init__.py @@ -49,10 +49,9 @@ def login_redirect(): return redirect(url_for(".home")) -def _is_valid_certificate(request): - cert = request.environ.get("HTTP_X_SSL_CLIENT_CERT") - if cert: - result = app.crl_validator.validate(cert.encode()) - return result - else: - return False +@bp.route("/logout") +def logout(): + if session.get("user_id"): + del (session["user_id"]) + + return redirect(url_for(".home")) diff --git a/templates/navigation/topbar.html b/templates/navigation/topbar.html index 4db599f3..35b87473 100644 --- a/templates/navigation/topbar.html +++ b/templates/navigation/topbar.html @@ -17,7 +17,7 @@ {{ Icon('avatar', classes='topbar__link-icon') }} - + {{ Icon('logout', classes='topbar__link-icon') }} diff --git a/tests/test_auth.py b/tests/test_auth.py index 2bf24b69..fc551317 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -1,3 +1,5 @@ +from urllib.parse import urlparse + import pytest from flask import session, url_for from .mocks import DOD_SDN_INFO, DOD_SDN, FIXTURE_EMAIL_ADDRESS @@ -148,7 +150,6 @@ def test_creates_new_user_on_login(monkeypatch, client): def test_creates_new_user_without_email_on_login(monkeypatch, client): - monkeypatch.setattr("atst.routes._is_valid_certificate", lambda *args: True) cert_file = open("ssl/client-certs/atat.mil.crt").read() # ensure user does not exist @@ -168,3 +169,31 @@ def test_creates_new_user_without_email_on_login(monkeypatch, client): assert user.first_name == DOD_SDN_INFO["first_name"] assert user.last_name == DOD_SDN_INFO["last_name"] assert user.email == None + + +def test_logout(app, client, monkeypatch): + monkeypatch.setattr( + "atst.domain.authnid.AuthenticationContext.authenticate", lambda s: True + ) + monkeypatch.setattr( + "atst.domain.authnid.AuthenticationContext.get_user", + lambda s: UserFactory.create(), + ) + # create a real session + resp = client.get( + url_for("atst.login_redirect"), + environ_base={ + "HTTP_X_SSL_CLIENT_VERIFY": "SUCCESS", + "HTTP_X_SSL_CLIENT_S_DN": DOD_SDN, + "HTTP_X_SSL_CLIENT_CERT": "", + }, + ) + resp_success = client.get(url_for("requests.requests_index")) + # verify session is valid + assert resp_success.status_code == 200 + client.get(url_for("atst.logout")) + resp_failure = client.get(url_for("requests.requests_index")) + # verify that logging out has cleared the session + assert resp_failure.status_code == 302 + destination = urlparse(resp_failure.headers["Location"]).path + assert destination == url_for("atst.root") From 0e8d5f139016f536d17e5ad8fe1157f8a4a15ac1 Mon Sep 17 00:00:00 2001 From: dandds Date: Mon, 17 Sep 2018 10:27:49 -0400 Subject: [PATCH 2/2] some cleanup to auth tests --- tests/test_auth.py | 76 ++++++++++++---------------------------------- 1 file changed, 19 insertions(+), 57 deletions(-) diff --git a/tests/test_auth.py b/tests/test_auth.py index fc551317..c6af8448 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -16,6 +16,17 @@ def _fetch_user_info(c, t): return MOCK_USER +def _login(client, verify="SUCCESS", sdn=DOD_SDN, cert=""): + return client.get( + url_for("atst.login_redirect"), + environ_base={ + "HTTP_X_SSL_CLIENT_VERIFY": verify, + "HTTP_X_SSL_CLIENT_S_DN": sdn, + "HTTP_X_SSL_CLIENT_CERT": cert, + }, + ) + + def test_successful_login_redirect_non_ccpo(client, monkeypatch): monkeypatch.setattr( "atst.domain.authnid.AuthenticationContext.authenticate", lambda *args: True @@ -25,14 +36,7 @@ def test_successful_login_redirect_non_ccpo(client, monkeypatch): lambda *args: UserFactory.create(), ) - resp = client.get( - "/login-redirect", - environ_base={ - "HTTP_X_SSL_CLIENT_VERIFY": "SUCCESS", - "HTTP_X_SSL_CLIENT_S_DN": "", - "HTTP_X_SSL_CLIENT_CERT": "", - }, - ) + resp = _login(client) assert resp.status_code == 302 assert "home" in resp.headers["Location"] @@ -49,14 +53,7 @@ def test_successful_login_redirect_ccpo(client, monkeypatch): lambda *args: UserFactory.create(atat_role=role), ) - resp = client.get( - "/login-redirect", - environ_base={ - "HTTP_X_SSL_CLIENT_VERIFY": "SUCCESS", - "HTTP_X_SSL_CLIENT_S_DN": "", - "HTTP_X_SSL_CLIENT_CERT": "", - }, - ) + resp = _login(client) assert resp.status_code == 302 assert "home" in resp.headers["Location"] @@ -64,7 +61,7 @@ def test_successful_login_redirect_ccpo(client, monkeypatch): def test_unsuccessful_login_redirect(client, monkeypatch): - resp = client.get("/login-redirect") + resp = client.get(url_for("atst.login_redirect")) assert resp.status_code == 401 assert "user_id" not in session @@ -101,26 +98,12 @@ def test_crl_validation_on_login(client): bad_cert = open("ssl/client-certs/bad-atat.mil.crt").read() # bad cert is on the test CRL - resp = client.get( - "/login-redirect", - environ_base={ - "HTTP_X_SSL_CLIENT_VERIFY": "SUCCESS", - "HTTP_X_SSL_CLIENT_S_DN": DOD_SDN, - "HTTP_X_SSL_CLIENT_CERT": bad_cert, - }, - ) + resp = _login(client, cert=bad_cert) assert resp.status_code == 401 assert "user_id" not in session # good cert is not on the test CRL, passes - resp = client.get( - "/login-redirect", - environ_base={ - "HTTP_X_SSL_CLIENT_VERIFY": "SUCCESS", - "HTTP_X_SSL_CLIENT_S_DN": DOD_SDN, - "HTTP_X_SSL_CLIENT_CERT": good_cert, - }, - ) + resp = _login(client, cert=good_cert) assert session["user_id"] @@ -134,14 +117,7 @@ def test_creates_new_user_on_login(monkeypatch, client): with pytest.raises(NotFoundError): Users.get_by_dod_id(DOD_SDN_INFO["dod_id"]) - resp = client.get( - "/login-redirect", - environ_base={ - "HTTP_X_SSL_CLIENT_VERIFY": "SUCCESS", - "HTTP_X_SSL_CLIENT_S_DN": DOD_SDN, - "HTTP_X_SSL_CLIENT_CERT": cert_file, - }, - ) + resp = _login(client, cert=cert_file) user = Users.get_by_dod_id(DOD_SDN_INFO["dod_id"]) assert user.first_name == DOD_SDN_INFO["first_name"] @@ -156,14 +132,7 @@ def test_creates_new_user_without_email_on_login(monkeypatch, client): with pytest.raises(NotFoundError): Users.get_by_dod_id(DOD_SDN_INFO["dod_id"]) - resp = client.get( - "/login-redirect", - environ_base={ - "HTTP_X_SSL_CLIENT_VERIFY": "SUCCESS", - "HTTP_X_SSL_CLIENT_S_DN": DOD_SDN, - "HTTP_X_SSL_CLIENT_CERT": cert_file, - }, - ) + resp = _login(client, cert=cert_file) user = Users.get_by_dod_id(DOD_SDN_INFO["dod_id"]) assert user.first_name == DOD_SDN_INFO["first_name"] @@ -180,14 +149,7 @@ def test_logout(app, client, monkeypatch): lambda s: UserFactory.create(), ) # create a real session - resp = client.get( - url_for("atst.login_redirect"), - environ_base={ - "HTTP_X_SSL_CLIENT_VERIFY": "SUCCESS", - "HTTP_X_SSL_CLIENT_S_DN": DOD_SDN, - "HTTP_X_SSL_CLIENT_CERT": "", - }, - ) + resp = _login(client) resp_success = client.get(url_for("requests.requests_index")) # verify session is valid assert resp_success.status_code == 200