diff --git a/.secrets.baseline b/.secrets.baseline index ccfc932e..258ea89e 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -3,7 +3,7 @@ "files": "^.secrets.baseline$|^.*pgsslrootcert.yml$", "lines": null }, - "generated_at": "2020-01-16T16:10:27Z", + "generated_at": "2020-01-19T20:21:20Z", "plugins_used": [ { "base64_limit": 4.5, @@ -32,13 +32,6 @@ "is_verified": false, "line_number": 156, "type": "Secret Keyword" - }, - { - "hashed_secret": "81b127e2222d9bfc4609053faec85300f7525463", - "is_secret": false, - "is_verified": false, - "line_number": 290, - "type": "Secret Keyword" } ], "alembic.ini": [ @@ -89,7 +82,7 @@ "hashed_secret": "afc848c316af1a89d49826c5ae9d00ed769415f3", "is_secret": false, "is_verified": false, - "line_number": 30, + "line_number": 31, "type": "Secret Keyword" } ], diff --git a/README.md b/README.md index 1dfc1ad1..2681346e 100644 --- a/README.md +++ b/README.md @@ -255,6 +255,7 @@ To generate coverage reports for the Javascript tests: - `SERVER_NAME`: Hostname for ATAT. Only needs to be specified in contexts where the hostname cannot be inferred from the request, such as Celery workers. https://flask.palletsprojects.com/en/1.1.x/config/#SERVER_NAME - `SESSION_COOKIE_NAME`: String value specifying the name to use for the session cookie. https://flask.palletsprojects.com/en/1.1.x/config/#SESSION_COOKIE_NAME - `SESSION_COOKIE_DOMAIN`: String value specifying the name to use for the session cookie. This should be set to the root domain so that it is valid for both the main site and the authentication subdomain. https://flask.palletsprojects.com/en/1.1.x/config/#SESSION_COOKIE_DOMAIN +- `SESSION_KEY_PREFIX`: A prefix that is added before all session keys: https://pythonhosted.org/Flask-Session/#configuration - `SESSION_TYPE`: String value specifying the cookie storage backend. https://pythonhosted.org/Flask-Session/ - `SESSION_USE_SIGNER`: Boolean value specifying if the cookie sid should be signed. - `SQLALCHEMY_ECHO`: Boolean value specifying if SQLAlchemy should log queries to stdout. diff --git a/atst/domain/auth.py b/atst/domain/auth.py index 4dca060f..77c22438 100644 --- a/atst/domain/auth.py +++ b/atst/domain/auth.py @@ -1,4 +1,13 @@ -from flask import g, redirect, url_for, session, request, current_app as app +from flask import ( + g, + redirect, + url_for, + session, + request, + current_app as app, + _request_ctx_stack as request_ctx_stack, +) +from werkzeug.datastructures import ImmutableTypeConversionDict from atst.domain.users import Users @@ -56,12 +65,26 @@ def get_last_login(): return session.get("user_id") and session.get("last_login") +def _nullify_session(session): + session_key = f"{app.config.get('SESSION_KEY_PREFIX')}{session.sid}" + app.redis.delete(session_key) + request.cookies = ImmutableTypeConversionDict() + request_ctx_stack.top.session = app.session_interface.open_session(app, request) + + +def _current_dod_id(): + return g.current_user.dod_id if session.get("user_id") else None + + def logout(): - if session.get("user_id"): # pragma: no branch - dod_id = g.current_user.dod_id - del session["user_id"] - del session["last_login"] + dod_id = _current_dod_id() + + _nullify_session(session) + + if dod_id: app.logger.info(f"user with EDIPI {dod_id} has logged out") + else: + app.logger.info("unauthenticated user has logged out") def _unprotected_route(request): diff --git a/atst/utils/session_limiter.py b/atst/utils/session_limiter.py index cae43c5d..e0c02e0d 100644 --- a/atst/utils/session_limiter.py +++ b/atst/utils/session_limiter.py @@ -4,6 +4,7 @@ from atst.domain.users import Users class SessionLimiter(object): def __init__(self, config, session, redis): self.limit_logins = config["LIMIT_CONCURRENT_SESSIONS"] + self.session_prefix = config.get("SESSION_KEY_PREFIX", "session:") self.session = session self.redis = redis @@ -16,4 +17,4 @@ class SessionLimiter(object): Users.update_last_session_id(user, session_id) def _delete_session(self, session_id): - self.redis.delete("session:{}".format(session_id)) + self.redis.delete(f"{self.session_prefix}{session_id}") diff --git a/config/base.ini b/config/base.ini index 1df7f47a..6fbcce73 100644 --- a/config/base.ini +++ b/config/base.ini @@ -42,6 +42,7 @@ SECRET_KEY = change_me_into_something_secret SERVER_NAME SESSION_COOKIE_NAME=atat SESSION_COOKIE_DOMAIN +SESSION_KEY_PREFIX=session: SESSION_TYPE = redis SESSION_USE_SIGNER = True SQLALCHEMY_ECHO = False diff --git a/deploy/azure/atst-envvars-configmap.yml b/deploy/azure/atst-envvars-configmap.yml index 6f412a3d..edd049a7 100644 --- a/deploy/azure/atst-envvars-configmap.yml +++ b/deploy/azure/atst-envvars-configmap.yml @@ -15,6 +15,7 @@ data: CSP: azure DEBUG: "0" FLASK_ENV: master + LIMIT_CONCURRENT_SESSIONS: "true" LOG_JSON: "true" MAIL_PORT: "587" MAIL_SENDER: postmaster@atat.code.mil diff --git a/tests/domain/test_auth.py b/tests/domain/test_auth.py new file mode 100644 index 00000000..a4002d5c --- /dev/null +++ b/tests/domain/test_auth.py @@ -0,0 +1,29 @@ +from flask import make_response, session + +from atst.domain.auth import logout + + +def _write_session(app): + response = make_response("") + app.session_interface.save_session(app, session, response) + return session + + +def test_logout_destroys_session(app): + session = _write_session(app) + key = app.config.get("SESSION_KEY_PREFIX") + session.sid + assert app.redis.get(key) + logout() + assert app.redis.get(key) is None + + +def test_logout_logs_dod_id_for_current_user(monkeypatch, mock_logger): + dod_id = "3434343434" + monkeypatch.setattr("atst.domain.auth._current_dod_id", lambda: dod_id) + logout() + assert dod_id in mock_logger.messages[-1] + + +def test_logout_logs_message_for_unathenticated_user(mock_logger): + logout() + assert "unauthenticated" in mock_logger.messages[-1]