Rebase: drop usage of flask.g.user globally as per #6428

Due to multiple reasons, now that there is a custom session wrapper, keeping the user state also in a flask.g object causes issues iof synchronization and cleaning. This commit completely removes its usage and replaces it with a a few additional session functions.

securedrop/journalist_app/__init__.py

@@ -4,8 +4,8 @@
 from datetime import datetime, timedelta, timezone
 from pathlib import Path
 from datetime import datetime
-from flask import (Flask, session, redirect, url_for, flash, g, request,
-                   render_template, json, abort)
+from flask import (Flask, redirect, url_for, flash, g, request,
+                   render_template, json, abort, session)
 from flask_assets import Environment
 from flask_babel import gettext
 from flask_wtf.csrf import CSRFProtect, CSRFError
@@ -27,8 +27,8 @@
     logged_in,
 )
 from journalist_app import account, admin, api, main, col
-from journalist_app.sessions import Session, ServerSideSession
-from journalist_app.utils import get_source, logged_in
+from journalist_app.sessions import Session, session
+from journalist_app.utils import get_source
 from models import InstanceConfig, Journalist
 from werkzeug.exceptions import default_exceptions
 
@@ -97,8 +97,7 @@ def default(self, obj: "Any") -> "Any":
     @app.errorhandler(CSRFError)  # type: ignore
     def handle_csrf_error(e: CSRFError) -> "Response":
         app.logger.error("The CSRF token is invalid.")
-        session2 = typing.cast(ServerSideSession, session)
-        session2.destroy()
+        session.destroy()
         msg = gettext('You have been logged out due to inactivity.')
         flash(msg, 'error')
         return redirect(url_for('main.login'))
@@ -138,15 +137,6 @@ def update_instance_config() -> None:
     def setup_g() -> "Optional[Response]":
         """Store commonly used values in Flask's special g object"""
 
-        if request.path.split('/')[1] != 'static':
-            uid = session.get('uid', None)
-            if uid:
-                g.uid = uid  # pylint: disable=assigning-non-slot
-                g.user = Journalist.query.get(uid)  # pylint: disable=assigning-non-slot
-            else:
-                g.uid = None  # pylint: disable=assigning-non-slot
-                g.user = None  # pylint: disable=assigning-non-slot
-
         i18n.set_locale(config)
 
         if InstanceConfig.get_default().organization_name:
@@ -162,11 +152,11 @@ def setup_g() -> "Optional[Response]":
             app.logger.error("Site logo not found.")
 
         if request.path.split('/')[1] == 'api':
-            if request.endpoint not in _insecure_api_views and not logged_in():
+            if request.endpoint not in _insecure_api_views and not session.logged_in():
                 abort(403)
         else:
-            if request.endpoint not in _insecure_views and not logged_in():
-                return redirect(url_for("main.login"))
+            if request.endpoint not in _insecure_views and not session.logged_in():
+                return redirect(url_for('main.login'))
 
         if request.method == "POST":
             filesystem_id = request.form.get("filesystem_id")

securedrop/journalist_app/account.py

@@ -1,9 +1,13 @@
 # -*- coding: utf-8 -*-
-from typing import Union, cast
+from typing import Union
 
 import werkzeug
+from flask import (Blueprint, render_template, request, g, redirect, url_for,
+                   flash)
+from flask_babel import gettext
+
 from db import db
-from journalist_app import sessions
+from journalist_app.sessions import session
 from journalist_app.utils import (set_diceware_password, set_name, validate_user,
                                   validate_hotp_secret)
 from passphrases import PassphraseGenerator
@@ -22,58 +26,55 @@ def edit() -> str:
 
     @view.route("/change-name", methods=("POST",))
     def change_name() -> werkzeug.Response:
-        first_name = request.form.get("first_name")
-        last_name = request.form.get("last_name")
-        set_name(g.user, first_name, last_name)
-        return redirect(url_for("account.edit"))
+        first_name = request.form.get('first_name')
+        last_name = request.form.get('last_name')
+        set_name(session.get_user(), first_name, last_name)
+        return redirect(url_for('account.edit'))
 
     @view.route("/new-password", methods=("POST",))
     def new_password() -> werkzeug.Response:
-        user = g.user
-        current_password = request.form.get("current_password")
-        token = request.form.get("token")
-        error_message = gettext("Incorrect password or two-factor code.")
+        user = session.get_user()
+        current_password = request.form.get('current_password')
+        token = request.form.get('token')
+        error_message = gettext('Incorrect password or two-factor code.')
         # If the user is validated, change their password
         if validate_user(user.username, current_password, token, error_message):
             password = request.form.get("password")
             set_diceware_password(user, password)
-            session2 = cast(sessions.ServerSideSession, session)
-            session2.destroy()
+            session.destroy()
             return redirect(url_for('main.login'))
         return redirect(url_for('account.edit'))
 
     @view.route("/2fa", methods=("GET", "POST"))
     def new_two_factor() -> Union[str, werkzeug.Response]:
-        if request.method == "POST":
-            token = request.form["token"]
-            if g.user.verify_token(token):
-                flash(
-                    gettext("Your two-factor credentials have been reset successfully."),
-                    "notification",
-                )
-                return redirect(url_for("account.edit"))
+        if request.method == 'POST':
+            token = request.form['token']
+            if session.get_user().verify_token(token):
+                flash(gettext("Your two-factor credentials have been reset successfully."),
+                      "notification")
+                return redirect(url_for('account.edit'))
             else:
                 flash(
                     gettext("There was a problem verifying the two-factor code. Please try again."),
                     "error",
                 )
 
-        return render_template("account_new_two_factor.html", user=g.user)
+        return render_template('account_new_two_factor.html', user=session.get_user())
 
     @view.route("/reset-2fa-totp", methods=["POST"])
     def reset_two_factor_totp() -> werkzeug.Response:
-        g.user.is_totp = True
-        g.user.regenerate_totp_shared_secret()
+        session.get_user().is_totp = True
+        session.get_user().regenerate_totp_shared_secret()
         db.session.commit()
         return redirect(url_for("account.new_two_factor"))
 
     @view.route("/reset-2fa-hotp", methods=["POST"])
     def reset_two_factor_hotp() -> Union[str, werkzeug.Response]:
         otp_secret = request.form.get("otp_secret", None)
         if otp_secret:
-            if not validate_hotp_secret(g.user, otp_secret):
-                return render_template("account_edit_hotp_secret.html")
-            g.user.set_hotp_secret(otp_secret)
+            if not validate_hotp_secret(session.get_user(), otp_secret):
+                return render_template('account_edit_hotp_secret.html')
+            session.get_user().set_hotp_secret(otp_secret)
             db.session.commit()
             return redirect(url_for("account.new_two_factor"))
         else:

securedrop/journalist_app/admin.py

@@ -6,6 +6,12 @@
 from typing import Optional, Union
 
 import werkzeug
+from flask import (Blueprint, render_template, request, url_for, redirect, g,
+                   current_app, flash, abort, session)
+from flask_babel import gettext
+from sqlalchemy.exc import IntegrityError
+from sqlalchemy.orm.exc import NoResultFound
+
 from db import db
 from flask import (
     Blueprint,
@@ -320,27 +326,26 @@ def edit_user(user_id: int) -> Union[str, werkzeug.Response]:
     @admin_required
     def delete_user(user_id: int) -> werkzeug.Response:
         user = Journalist.query.get(user_id)
-        if user_id == g.user.id:
+        if user_id == session.get_uid():
             # Do not flash because the interface already has safe guards.
             # It can only happen by manually crafting a POST request
-            current_app.logger.error("Admin {} tried to delete itself".format(g.user.username))
+            current_app.logger.error(
+                "Admin {} tried to delete itself".format(session.get_user().username))
             abort(403)
         elif not user:
             current_app.logger.error(
                 "Admin {} tried to delete nonexistent user with pk={}".format(
-                    g.user.username, user_id
-                )
-            )
+                    session.get_user().username, user_id))
             abort(404)
         elif user.is_deleted_user():
             # Do not flash because the interface does not expose this.
             # It can only happen by manually crafting a POST request
             current_app.logger.error(
-                'Admin {} tried to delete "deleted" user'.format(g.user.username)
-            )
+                "Admin {} tried to delete \"deleted\" user".format(session.get_user().username))
             abort(403)
         else:
             user.delete()
+            logout_user(user.id)
             db.session.commit()
             flash(gettext("Deleted user '{user}'.").format(user=user.username), "notification")
 
@@ -356,7 +361,7 @@ def new_password(user_id: int) -> werkzeug.Response:
 
         password = request.form.get('password')
         if set_diceware_password(user, password) is not False:
-            logout_user(user_id)
+            logout_user(user.id)
             db.session.commit()
         return redirect(url_for("admin.edit_user", user_id=user_id))
 

securedrop/journalist_app/api.py

@@ -1,11 +1,11 @@
 import collections.abc
 import json
 from datetime import datetime, timezone
-from typing import Tuple, Set, Union, cast
+from typing import Tuple, Set, Union
 
 import flask
 import werkzeug
-from flask import abort, Blueprint, jsonify, request, session, g
+from flask import abort, Blueprint, jsonify, request, g
 
 from sqlalchemy import Column
 from sqlalchemy.exc import IntegrityError
@@ -16,7 +16,8 @@
 import flask
 import werkzeug
 from db import db
-from journalist_app import utils, sessions
+from journalist_app import utils
+from journalist_app.sessions import session
 from models import (Journalist, Reply, SeenReply, Source, Submission,
                     LoginThrottledException, InvalidUsernameException,
                     BadTokenException, InvalidOTPSecretException,
@@ -91,10 +92,10 @@ def get_token() -> Tuple[flask.Response, int]:
 
         try:
             journalist = Journalist.login(username, passphrase, one_time_code)
-            session2 = cast(sessions.ServerSideSession, session)
+
             response = jsonify({
-                'token': session2.get_token(),
-                'expiration': session2.get_lifetime(),
+                'token': session.get_token(),
+                'expiration': session.get_lifetime(),
                 'journalist_uuid': journalist.uuid,
                 'journalist_first_name': journalist.first_name,
                 'journalist_last_name': journalist.last_name,
@@ -105,7 +106,7 @@ def get_token() -> Tuple[flask.Response, int]:
             db.session.add(journalist)
             db.session.commit()
 
-            session2['uid'] = journalist.id
+            session['uid'] = journalist.id
 
             return response, 200
         except (LoginThrottledException, InvalidUsernameException,
@@ -206,8 +207,6 @@ def all_source_replies(source_uuid: str) -> Tuple[flask.Response, int]:
             if "reply" not in request.json:
                 abort(400, "reply not found in request body")
 
-            user = Journalist.query.get(g.uid)
-
             data = request.json
             if not data["reply"]:
                 abort(400, "reply should not be empty")
@@ -226,7 +225,7 @@ def all_source_replies(source_uuid: str) -> Tuple[flask.Response, int]:
             # issue #3918
             filename = path.basename(filename)
 
-            reply = Reply(user, source, filename, Storage.get_default())
+            reply = Reply(session.get_user(), source, filename, Storage.get_default())
 
             reply_uuid = data.get("uuid", None)
             if reply_uuid is not None:
@@ -239,7 +238,7 @@ def all_source_replies(source_uuid: str) -> Tuple[flask.Response, int]:
 
             try:
                 db.session.add(reply)
-                seen_reply = SeenReply(reply=reply, journalist=user)
+                seen_reply = SeenReply(reply=reply, journalist=session.get_user())
                 db.session.add(seen_reply)
                 db.session.add(source)
                 db.session.commit()
@@ -292,7 +291,6 @@ def seen() -> Tuple[flask.Response, int]:
         """
         Lists or marks the source conversation items that the journalist has seen.
         """
-        user = Journalist.query.get(g.uid)
 
         if request.method == "POST":
             if request.json is None or not isinstance(request.json, collections.abc.Mapping):
@@ -323,16 +321,15 @@ def seen() -> Tuple[flask.Response, int]:
                 targets.add(r)
 
             # now mark everything seen.
-            utils.mark_seen(list(targets), user)
+            utils.mark_seen(list(targets), session.get_user())
 
             return jsonify({"message": "resources marked seen"}), 200
 
         abort(405)
 
     @api.route('/user', methods=['GET'])
     def get_current_user() -> Tuple[flask.Response, int]:
-        user = Journalist.query.get(g.uid)
-        return jsonify(user.to_json()), 200
+        return jsonify(session.get_user().to_json()), 200
 
     @api.route('/users', methods=['GET'])
     def get_all_users() -> Tuple[flask.Response, int]:
@@ -341,8 +338,7 @@ def get_all_users() -> Tuple[flask.Response, int]:
 
     @api.route('/logout', methods=['POST'])
     def logout() -> Tuple[flask.Response, int]:
-        session2 = cast(sessions.ServerSideSession, session)
-        session2.destroy()
+        session.destroy()
         return jsonify({'message': 'Your token has been revoked.'}), 200
 
     def _handle_api_http_exception(

securedrop/journalist_app/decorators.py

@@ -1,16 +1,18 @@
 # -*- coding: utf-8 -*-
+from typing import Any
+
+from flask import redirect, url_for, flash, g, session
+from flask_babel import gettext
 from functools import wraps
 from typing import Any, Callable
 
-from flask import flash, g, redirect, url_for
-from flask_babel import gettext
-from journalist_app.utils import logged_in
+from typing import Callable
 
 
 def admin_required(func: Callable) -> Callable:
     @wraps(func)
     def wrapper(*args: Any, **kwargs: Any) -> Any:
-        if logged_in() and g.user.is_admin:
+        if session.logged_in() and session.get_user().is_admin:
             return func(*args, **kwargs)
         flash(gettext("Only admins can access this page."), "notification")
         return redirect(url_for("main.index"))