From 78a257fbc8733aac9c5c79e1e441565b5506ab2b Mon Sep 17 00:00:00 2001
From: Bheesham Persaud <171007+bheesham@users.noreply.github.com>
Date: Tue, 29 Oct 2024 14:31:41 -0400
Subject: [PATCH] Swap Flask-KVSession for Flask-Session (#531)

* Tests for the user should be under tests/models

Jira: [IAM-1468](https://mozilla-hub.atlassian.net/browse/IAM-1468)

* Flask-Session is more maintained

While Flask-KVSession has worked well for us, Flask-Session is seeing more
maintainence.

Jira: [IAM-1468](https://mozilla-hub.atlassian.net/browse/IAM-1468)
---
 dashboard/app.py                | 39 +++++++++++++++++++++++++--------
 requirements.txt                |  5 ++---
 tests/{ => models}/test_user.py |  3 ++-
 tox.ini                         |  1 -
 4 files changed, 34 insertions(+), 14 deletions(-)
 rename tests/{ => models}/test_user.py (91%)

diff --git a/dashboard/app.py b/dashboard/app.py
index 7a8e2963..c2d65e06 100644
--- a/dashboard/app.py
+++ b/dashboard/app.py
@@ -16,15 +16,13 @@
 from flask import request
 from flask import send_from_directory
 from flask import session
+from flask.sessions import SessionInterface
 
 from flask_assets import Bundle  # type: ignore
 from flask_assets import Environment  # type: ignore
-from flask_kvsession import KVSessionExtension  # type: ignore
+from flask_session.redis import RedisSessionInterface  # type: ignore
 from flask_talisman import Talisman  # type: ignore
 
-from simplekv.memory.redisstore import RedisStore  # type: ignore
-from simplekv.decorator import PrefixDecorator  # type: ignore
-
 from dashboard import oidc_auth
 from dashboard import config
 from dashboard import get_config
@@ -54,11 +52,34 @@
 
 app_list = CDNTransfer(config.Config(app).settings)
 
-# Activate server-side redis sesssion KV
-redis_host, redis_port = app.config["REDIS_CONNECTOR"].split(":")
-store = RedisStore(redis.StrictRedis(host=redis_host, port=redis_port))
-prefixed_store = PrefixDecorator(app.config["SERVER_NAME"] + "_", store)
-KVSessionExtension(store, app)
+
+def session_configure(app: Flask) -> SessionInterface:
+    """
+    We should try doing what our dependencies prefer, falling back to what we
+    want to do only as a last resort. That is to say, try using a connection
+    string _first_, then do our logic.
+
+    This function will either return a _verified_ connection or raise an
+    exception (failing fast).
+
+    Considerations for the future:
+    * Auth
+    """
+    try:
+        client = redis.Redis.from_url(app.config["REDIS_CONNECTOR"])
+    except ValueError:
+        host, _, port = app.config["REDIS_CONNECTOR"].partition(":")
+        client = redis.Redis(host=host, port=int(port))
+    # [redis.Redis.ping] will raise an exception if it can't connect anyways,
+    # but at least this way we make use of it's return value. Feels weird to
+    # not?
+    #
+    # redis.Redis.ping: https://github.com/redis/redis-py/blob/00f5be420b397adfa1b9aa9c2761f7d8a27c0a9a/redis/commands/core.py#L1206
+    assert client.ping(), "Could not ping Redis"
+    return RedisSessionInterface(app, client=client)
+
+
+app.session_interface = session_configure(app)
 
 assets = Environment(app)
 js = Bundle("js/base.js", filters="jsmin", output="js/gen/packed.js")
diff --git a/requirements.txt b/requirements.txt
index f5d6d105..051fdd88 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -19,7 +19,7 @@ Faker==26.1.0
 filelock==3.15.4
 Flask==3.0.3
 Flask-Assets==2.1.0
-Flask-KVSession @ git+https://github.com/zacqed/flask-kvsession.git@83322aa18ed784bbe0cb309322e8d0d48bb762da
+Flask-Session==0.8.0
 Flask-pyoidc==3.14.3
 flask-talisman==1.1.0
 future==1.0.0
@@ -54,11 +54,10 @@ python-dateutil==2.9.0.post0
 python-dotenv==1.0.1
 python-jose==3.3.0
 PyYAML==6.0.1
-redis==5.0.8
+redis==5.2.0
 requests==2.32.3
 rsa==4.9
 setuptools==72.1.0
-simplekv==0.14.1
 six==1.16.0
 tox==4.16.0
 types-PyYAML==6.0.12.20240724
diff --git a/tests/test_user.py b/tests/models/test_user.py
similarity index 91%
rename from tests/test_user.py
rename to tests/models/test_user.py
index ca5d9fb4..0e532b3c 100644
--- a/tests/test_user.py
+++ b/tests/models/test_user.py
@@ -1,4 +1,5 @@
 import json
+from pathlib import Path
 import os
 
 import dashboard.models.user as user
@@ -6,8 +7,8 @@
 
 class TestUser:
     def setup_method(self):
+        self.fixture_file = Path(__file__).parent.parent / "data" / "userinfo.json"
         try:
-            self.fixture_file = os.path.join(os.path.abspath(os.path.dirname(__file__)), "data/userinfo.json")
             with open(self.fixture_file) as f:
                 self.session_fixture = json.load(f)
 
diff --git a/tox.ini b/tox.ini
index 16b05323..5b2d22db 100644
--- a/tox.ini
+++ b/tox.ini
@@ -41,7 +41,6 @@ deps =
     types-colorama>=0.4
     types-pyasn1>=0.6
     types-python-jose>=3.3
-    types-redis>=4.6
     types-requests>=2.32
     types-six>=1.16
 commands = mypy {tty:--color-output:--no-color-output} {posargs: ./tests ./dashboard}