From 79b88d4d08ee0f2742296bff457d74ffc31d916a Mon Sep 17 00:00:00 2001 From: Ben Kurtovic Date: Wed, 21 Dec 2016 17:06:24 -0500 Subject: [PATCH] Better session expiry handling, storing creation timestamp in cookie. --- app.py | 2 -- calefaction/auth.py | 64 ++++++++++++++++++++++++++++++++++--------------- calefaction/database.py | 29 ++++++++++++++-------- calefaction/util.py | 8 +++++-- 4 files changed, 70 insertions(+), 33 deletions(-) diff --git a/app.py b/app.py index 3fdb4db..644baf8 100755 --- a/app.py +++ b/app.py @@ -60,8 +60,6 @@ def login(): success, caught = try_func(lambda: auth.handle_login(code, state)) if success: flash(Messages.LOGGED_IN, "success") - elif getattr(g, "_session_expired", False): - flash(Messages.SESSION_EXPIRED, "error") elif not caught: flash(Messages.LOGIN_FAILED, "error") return redirect(url_for("index"), 303) diff --git a/calefaction/auth.py b/calefaction/auth.py index 1521229..b16ed1c 100644 --- a/calefaction/auth.py +++ b/calefaction/auth.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- -from datetime import datetime, timedelta +from datetime import datetime, timedelta, timezone from flask import g, session, url_for from itsdangerous import BadSignature, URLSafeSerializer @@ -23,13 +23,19 @@ class AuthManager: self._logger = baseLogger.getChild("auth") self._debug = self._logger.debug + def _allocate_new_session(self): + """Create a new session for the current user.""" + sid, created = g.db.new_session() + session["id"] = sid + session["date"] = created.replace(tzinfo=timezone.utc).timestamp() + self._debug("Allocated session id=%d", sid) + g._session_check = True + g._session_expired = False + def _get_session_id(self): """Return the current session ID, allocating a new one if necessary.""" if "id" not in session: - session["id"] = g.db.new_session() - self._debug("Allocated session id=%d", session["id"]) - g._session_checked = True - g._session_expired = False + self._allocate_new_session() return session["id"] def _invalidate_session(self): @@ -41,9 +47,16 @@ class AuthManager: sid = session["id"] g.db.drop_session(sid) self._debug("Dropped session id=%d", sid) - del session["id"] + session.clear() + + def _expire_session(self, always_notify=False): + """Mark the current session as expired, then invalidate it.""" + if always_notify or session.get("expire-notify"): + g._session_expired = True + self._debug("Session expired id=%d", session["id"]) + self._invalidate_session() - def _check_session(self): + def _check_session(self, always_notify_expired=False): """Return whether the user has a valid, non-expired session. This checks for the session existing in the database, but does not @@ -52,15 +65,28 @@ class AuthManager: if "id" not in session: return False - if hasattr(g, "_session_checked"): - return g._session_checked + if hasattr(g, "_session_check"): + return g._session_check - g._session_checked = check = g.db.has_session(session["id"]) - if not check: - g._session_expired = True - self._debug("Session expired id=%d", session["id"]) - self._invalidate_session() - return check + if "date" not in session: + self._debug("Clearing dateless session id=%d", session["id"]) + session.clear() + return False + + created = g.db.has_session(session["id"]) + if not created: + self._expire_session(always_notify=always_notify_expired) + g._session_check = False + return False + + cstamp = created.replace(tzinfo=timezone.utc).timestamp() + if session["date"] != cstamp: + self._debug("Clearing bad-date session id=%d", session["id"]) + session.clear() + return False + + g._session_check = True + return True def _get_state_hash(self): """Return a hash of the user's session ID suitable for OAuth2 state. @@ -109,7 +135,8 @@ class AuthManager: return None token, expiry, refresh = result - expires = datetime.utcnow() + timedelta(seconds=expiry) + expires = (datetime.utcnow().replace(microsecond=0) + + timedelta(seconds=expiry)) result = self._eve.sso.get_character_info(token) if not result: @@ -265,7 +292,7 @@ class AuthManager: if "id" in session: self._debug("Logging in session id=%d", session["id"]) - if not self._check_session(): + if not self._check_session(always_notify_expired=True): return False if not self._verify_state_hash(state): return False @@ -285,6 +312,7 @@ class AuthManager: g.db.set_auth(char_id, token, expires, refresh) g.db.attach_session(sid, char_id) g.db.touch_session(sid) + session["expire-notify"] = True return True def handle_logout(self): @@ -294,6 +322,4 @@ class AuthManager: """ if "id" in session: self._debug("Logging out session id=%d", session["id"]) - self._invalidate_session() - session.clear() diff --git a/calefaction/database.py b/calefaction/database.py index 6ba7349..58d4cb4 100644 --- a/calefaction/database.py +++ b/calefaction/database.py @@ -72,28 +72,37 @@ class Database: conn.execute(query.format(create_thresh, touch_thresh)) def new_session(self): - """Allocate a new session in the database and return its ID.""" + """Allocate a new session in the database. + + Return its ID as an integer and creation timestamp as a naive UTC + datetime. + """ + created = datetime.utcnow().replace(microsecond=0) + query = "INSERT INTO session (session_created) VALUES (?)" with self._conn as conn: - cur = conn.execute("INSERT INTO session DEFAULT VALUES") - return cur.lastrowid + cur = conn.execute(query, (created,)) + return cur.lastrowid, created def has_session(self, sid): - """Return whether the given session ID exists in the database. + """Return the creation timestamp for the given session ID, or None. - Will only be True for non-expired sessions. This function randomly does - database maintenance; very old expired sessions may be cleared. + Will only return a timestamp for non-expired sessions. This function + randomly does database maintenance; very old expired sessions may be + cleared. """ if random.random() <= 0.2: self._clear_old_sessions() - query = """SELECT 1 FROM session + query = """SELECT session_created FROM session WHERE session_id = ? AND strftime("%s", "now") - strftime("%s", session_created) < {} AND strftime("%s", "now") - strftime("%s", session_touched) < {}""" query = query.format(self.MAX_SESSION_AGE, self.MAX_SESSION_STALENESS) - cur = self._conn.execute(query, (sid,)) - return bool(cur.fetchall()) + res = self._conn.execute(query, (sid,)).fetchall() + if not res: + return None + return datetime.strptime(res[0][0], "%Y-%m-%d %H:%M:%S") def read_session(self, sid): """Return the character associated with the given session, or None.""" @@ -187,7 +196,7 @@ class Database: return None token, expiry, refresh = res[0] - expires = datetime.strptime(expiry, "%Y-%m-%d %H:%M:%S.%f") + expires = datetime.strptime(expiry, "%Y-%m-%d %H:%M:%S") return token, expires, refresh def drop_auth(self, cid): diff --git a/calefaction/util.py b/calefaction/util.py index 4ca2045..c6aec9d 100644 --- a/calefaction/util.py +++ b/calefaction/util.py @@ -5,7 +5,7 @@ from hashlib import md5 from os import path from traceback import format_exc -from flask import flash, request, url_for +from flask import flash, g, request, url_for from flask_mako import render_template, TemplateError from werkzeug.exceptions import HTTPException @@ -24,7 +24,6 @@ def try_func(inner): """ try: result = inner() - return (result, False) except EVEAPIError: flash(Messages.EVE_API_ERROR, "error") return (False, True) @@ -32,6 +31,11 @@ def try_func(inner): flash(Messages.ACCESS_DENIED, "error") return (False, True) + if getattr(g, "_session_expired", False): + flash(Messages.SESSION_EXPIRED, "error") + return (result, True) + return (result, False) + def make_error_catcher(app, error_template): """Wrap a route to display and log any uncaught exceptions.""" def callback(func):