fix: properly user Redis sentinel client (#668)

* fix: properly user Redis sentinel client

* squashme: minor fix

app/__init__.py

@@ -24,12 +24,12 @@
 import sys
 
 import jwt
+import redis
 import requests
 import sentry_sdk
 from flask import Flask, Response, current_app, jsonify, request
 from flask_cors import CORS
 from flask_kvsession import KVSessionExtension
-from redis.sentinel import Sentinel
 from sentry_sdk.integrations.flask import FlaskIntegration
 from simplekv.decorator import PrefixDecorator
 from simplekv.memory.redisstore import RedisStore
@@ -86,35 +86,36 @@
 )
 
 
-@app.before_request
+@app.before_first_request
 def setup_redis_client():
     """Set up a redis connection to the master by querying sentinel."""
 
     if "pytest" not in sys.modules:
+        _config = {
+            "db": current_app.config["REDIS_DB"],
+            "password": current_app.config["REDIS_PASSWORD"],
+            "retry_on_timeout": True,
+            "health_check_interval": 60,
+        }
         if current_app.config["REDIS_IS_SENTINEL"]:
-            sentinel = Sentinel(
+            _sentinel = redis.Sentinel(
                 [(current_app.config["REDIS_HOST"], current_app.config["REDIS_PORT"])],
                 sentinel_kwargs={"password": current_app.config["REDIS_PASSWORD"]},
             )
-            host, port = sentinel.discover_master(
-                current_app.config["REDIS_MASTER_SET"]
+            _client = _sentinel.master_for(
+                current_app.config["REDIS_MASTER_SET"], **_config
             )
-            current_app.logger.debug(f"Discovered redis master at {host}:{port}")
         else:
-            (host, port) = (
-                current_app.config["REDIS_HOST"],
-                current_app.config["REDIS_PORT"],
+            _client = redis.Redis(
+                host=current_app.config["REDIS_HOST"],
+                port=current_app.config["REDIS_PORT"],
+                **_config,
             )
 
         # Set up the redis store for tokens
-        current_app.store = OAuthRedis(
-            hex_key=current_app.config["SECRET_KEY"],
-            host=host,
-            password=current_app.config["REDIS_PASSWORD"],
-            db=current_app.config["REDIS_DB"],
-        )
+        current_app.store = OAuthRedis(_client, current_app.config["SECRET_KEY"])
         # We use the same store for sessions.
-        session_store = PrefixDecorator("sessions_", RedisStore(current_app.store))
+        session_store = PrefixDecorator("sessions_", RedisStore(_client))
         KVSessionExtension(session_store, app)
 
 

app/auth/oauth_redis.py

@@ -22,19 +22,15 @@
 """
 
 import base64
-import sys
+from typing import Any, Optional
 
 from cryptography.fernet import Fernet
 from flask import current_app
 from oauthlib.oauth2.rfc6749.errors import OAuth2Error
+from redis import Redis
 
 from .oauth_client import RenkuWebApplicationClient
 
-if "pytest" in sys.modules:
-    from fakeredis import FakeStrictRedis as StrictRedis
-else:
-    from redis import StrictRedis
-
 
 def create_fernet_key(hex_key):
     """Small helper to transform a standard 64 hex character secret
@@ -55,22 +51,22 @@ def create_fernet_key(hex_key):
     )
 
 
-class OAuthRedis(StrictRedis):
-    """Just a regular StrictRedis store with extra methods for
+class OAuthRedis:
+    """Just a thin wrapper around redis store with extra methods for
     setting and getting encrypted serializations of oauth client objects."""
 
-    def __init__(self, *args, hex_key=None, **kwargs):
-        super().__init__(*args, **kwargs)
-        self.fernet = Fernet(create_fernet_key(hex_key))
+    def __init__(self, redis_client: Redis, fernet_key: Optional[str] = None):
+        self._redis_client = redis_client
+        self._fernet = Fernet(create_fernet_key(fernet_key))
 
     def set_enc(self, name, value):
         """Set method with encryption."""
-        return super().set(name, self.fernet.encrypt(value))
+        return self._redis_client.set(name, self._fernet.encrypt(value))
 
     def get_enc(self, name):
         """Get method with decryption."""
-        value = super().get(name)
-        return None if value is None else self.fernet.decrypt(value)
+        value = self._redis_client.get(name)
+        return None if value is None else self._fernet.decrypt(value)
 
     def set_oauth_client(self, name, oauth_client):
         """Put a client object into the store."""
@@ -102,3 +98,10 @@ def get_oauth_client(self, name, no_refresh=False):
                 return None
 
         return oauth_client
+
+    def __repr__(self) -> str:
+        """Overriden to avoid leaking the encryption key or Redis password."""
+        return "OAuthRedis()"
+
+    def __getattr__(self, name: str) -> Any:
+        return self._redis_client.__getattribute__(name)

app/tests/test_proxy.py

@@ -22,6 +22,8 @@
 import requests
 import responses
 
+from fakeredis import FakeStrictRedis
+
 from .. import app
 from ..auth.oauth_client import RenkuWebApplicationClient
 from ..auth.oauth_provider_app import OAuthProviderApp
@@ -97,7 +99,7 @@ def set_dummy_oauth_client(token, key_suffix):
     access_token = jwt.encode(payload=TOKEN_PAYLOAD, key=PRIVATE_KEY, algorithm="RS256")
     headers = {"Authorization": "Bearer {}".format(access_token)}
 
-    app.store = OAuthRedis(hex_key=app.config["SECRET_KEY"])
+    app.store = OAuthRedis(FakeStrictRedis(), app.config["SECRET_KEY"])
 
     set_dummy_oauth_client(access_token, app.config["CLI_SUFFIX"])