From 493de28a5d7989c123b02d8fdf1104d8a6c9806d Mon Sep 17 00:00:00 2001 From: Ben Kurtovic Date: Mon, 19 Dec 2016 05:56:09 -0500 Subject: [PATCH] Add clean message flashing, error handling. --- app.py | 53 ++++++++++++++++++++++++++++++++----------------- calefaction/auth.py | 8 ++++++++ calefaction/messages.py | 17 ++++++++++++++++ calefaction/util.py | 49 ++++++++++++++++++++++++++++++++++++++------- static/main.css | 20 +++++++++++++++++++ templates/_base.mako | 10 ++++++++++ templates/_default.mako | 2 +- templates/logout.mako | 9 +++++++++ 8 files changed, 142 insertions(+), 26 deletions(-) create mode 100644 calefaction/messages.py create mode 100644 templates/logout.mako diff --git a/app.py b/app.py index a80349d..e0028bb 100755 --- a/app.py +++ b/app.py @@ -3,7 +3,7 @@ from pathlib import Path -from flask import Flask, g, redirect, request, url_for +from flask import Flask, flash, g, redirect, request, url_for from flask_mako import MakoTemplates, render_template import calefaction @@ -11,8 +11,10 @@ from calefaction.auth import AuthManager from calefaction.config import Config from calefaction.database import Database from calefaction.eve import EVE -from calefaction.exceptions import AccessDeniedError, EVEAPIError -from calefaction.util import catch_errors, set_up_asset_versioning +from calefaction.messages import Messages +from calefaction.util import ( + try_func, make_error_catcher, make_route_restricter, + set_up_asset_versioning) app = Flask(__name__) @@ -21,6 +23,9 @@ config = Config(basepath / "config") Database.path = str(basepath / "data" / "db.sqlite3") eve = EVE(config) auth = AuthManager(config, eve) +catch_exceptions = make_error_catcher(app, "error.mako") +route_restricted = make_route_restricter( + auth, lambda: redirect(url_for("index"), 303)) MakoTemplates(app) set_up_asset_versioning(app) @@ -37,32 +42,44 @@ app.before_request(Database.pre_hook) app.teardown_appcontext(Database.post_hook) @app.route("/") -@catch_errors(app) +@catch_exceptions def index(): - ... # handle flashed error messages in _base.mako - if auth.is_authenticated(): # ... need to check for exceptions + success, _ = try_func(auth.is_authenticated) + if success: return render_template("home.mako") return render_template("landing.mako") @app.route("/login", methods=["GET", "POST"]) -@catch_errors(app) +@catch_exceptions def login(): code = request.args.get("code") state = request.args.get("state") - try: - auth.handle_login(code, state) - except EVEAPIError: - ... # flash error message - except AccessDeniedError: - ... # flash error message - if getattr(g, "_session_expired"): - ... # flash error message + + 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) -# @app.route("/logout") ... +@app.route("/logout", methods=["GET", "POST"]) +@catch_exceptions +def logout(): + if request.method == "GET": + return render_template("logout.mako") + + auth.handle_logout() + flash(Messages.LOGGED_OUT, "success") + return redirect(url_for("index"), 303) -# @auth.route_restricted ... -# check for same exceptions as login() and use same flashes +@app.route("/test") +@catch_exceptions +@route_restricted +def test(): + ... + return "Success! You are authenticated!" if __name__ == "__main__": app.run(debug=True, port=8080) diff --git a/calefaction/auth.py b/calefaction/auth.py index c945793..b726f45 100644 --- a/calefaction/auth.py +++ b/calefaction/auth.py @@ -238,3 +238,11 @@ class AuthManager: g.db.attach_session(sid, char_id) g.db.touch_session(sid) return True + + def handle_logout(self): + """Log out the user if they are logged in. + + Invalidates their session and clears the session cookie. + """ + self._invalidate_session() + session.clear() diff --git a/calefaction/messages.py b/calefaction/messages.py new file mode 100644 index 0000000..ff26cde --- /dev/null +++ b/calefaction/messages.py @@ -0,0 +1,17 @@ +# -*- coding: utf-8 -*- + +__all__ = ["Messages"] + +class Messages: + """Namespace for user interface message strings.""" + # success + LOGGED_IN = "Logged in." + LOGGED_OUT = "Logged out." + + # error + LOG_IN_FIRST = "You need to log in to access that page." + ACCESS_DENIED = "Your character is not permitted to access this site." + SESSION_EXPIRED = "Session expired. You need to log in again." + LOGIN_FAILED = "Login failed." + EVE_API_ERROR = ("There was an error communicating with EVE's servers. " + "Please wait a while and try again.") diff --git a/calefaction/util.py b/calefaction/util.py index 85ea51b..770a7a1 100644 --- a/calefaction/util.py +++ b/calefaction/util.py @@ -5,12 +5,33 @@ from hashlib import md5 from os import path from traceback import format_exc -from flask import url_for +from flask import flash, url_for from flask_mako import render_template, TemplateError -__all__ = ["catch_errors", "set_up_asset_versioning"] +from .exceptions import AccessDeniedError, EVEAPIError +from .messages import Messages -def catch_errors(app): +__all__ = [ + "try_func", "make_error_catcher", "make_route_restricter", + "set_up_asset_versioning"] + +def try_func(inner): + """Evaluate inner(), catching subclasses of CalefactionError. + + If nothing was caught, return (inner(), False). Otherwise, flash an + appropriate error message and return (False, True). + """ + try: + result = inner() + return (result, False) + except EVEAPIError: + flash(Messages.EVE_API_ERROR, "error") + return (False, True) + except AccessDeniedError: + flash(Messages.ACCESS_DENIED, "error") + return (False, True) + +def make_error_catcher(app, error_template): """Wrap a route to display and log any uncaught exceptions.""" def callback(func): @wraps(func) @@ -19,10 +40,24 @@ def catch_errors(app): return func(*args, **kwargs) except TemplateError as exc: app.logger.error("Caught exception:\n{0}".format(exc.text)) - return render_template("error.mako", traceback=exc.text) + return render_template(error_template, traceback=exc.text) except Exception: app.logger.exception("Caught exception:") - return render_template("error.mako", traceback=format_exc()) + return render_template(error_template, traceback=format_exc()) + return inner + return callback + +def make_route_restricter(auth, on_failure): + """Wrap a route to ensure the user is authenticated.""" + def callback(func): + @wraps(func) + def inner(*args, **kwargs): + success, caught = try_func(auth.is_authenticated) + if success: + return func(*args, **kwargs) + if not caught: + flash(Messages.LOG_IN_FIRST, "error") + return on_failure() return inner return callback @@ -40,8 +75,8 @@ def set_up_asset_versioning(app): if cache and cache[0] == mtime: hashstr = cache[1] else: - with open(fpath, "rb") as f: - hashstr = md5(f.read()).hexdigest() + with open(fpath, "rb") as fp: + hashstr = md5(fp.read()).hexdigest() app._hash_cache[fpath] = (mtime, hashstr) return url_for("static", filename=filename, v=hashstr) raise error diff --git a/static/main.css b/static/main.css index f130e54..dbaff9b 100644 --- a/static/main.css +++ b/static/main.css @@ -149,3 +149,23 @@ footer ul li:not(:last-child):after { #error pre { white-space: pre-wrap; } + +#flashes { + margin-top: 0.5em; +} + +#flashes > div { + padding: 0.5em 0.75em; + border-left-width: 4px; + border-left-style: solid; +} + +#flashes > .success { + border-color: #33AA22; + background-color: rgba(60, 255, 30, 0.2); +} + +#flashes > .error { + border-color: #AA3322; + background-color: rgba(255, 60, 30, 0.2); +} diff --git a/templates/_base.mako b/templates/_base.mako index 281d3c2..29af941 100644 --- a/templates/_base.mako +++ b/templates/_base.mako @@ -38,6 +38,16 @@
+ <%block name="flashes"> + <% messages = get_flashed_messages(with_categories=True) %> + % if messages: +
+ % for category, message in messages: +
${message | h}
+ % endfor +
+ % endif + ${next.body()}
diff --git a/templates/_default.mako b/templates/_default.mako index f329127..d971cbd 100644 --- a/templates/_default.mako +++ b/templates/_default.mako @@ -6,6 +6,6 @@ <%block name="righthead"> - PLAYER_NAME... [logout] + PLAYER_NAME... [logout] ${next.body()} diff --git a/templates/logout.mako b/templates/logout.mako new file mode 100644 index 0000000..a1a0b06 --- /dev/null +++ b/templates/logout.mako @@ -0,0 +1,9 @@ +<%inherit file="_base.mako"/> +<%block name="title"> + Log out – ${g.config.get("corp.name") | h} + +

Log out

+

Use the button below to safely log out and clear your session.

+
+ +