Protect against login CSRF without cookies for anonymous users

Anonymous users must not get cookies or session store entries, so a
CSRF token for /login is not possible. Instead, every unsafe request is
checked statelessly via Sec-Fetch-Site, falling back to Origin. Requests
without both headers are not from a browser and are accepted. The
expected origin comes from the new MAX_ORIGIN setting, which is required
outside of debug mode.

Further CSRF changes:
- Do not delete the CSRF token on a failed check. It added no security,
  but invalidated forms in open tabs and could be triggered cross-site.
- Treat HEAD and OPTIONS as safe methods.

Keep the login page free of sessions:
- Show login messages via ?msg=login_required|logout (fixed keys only)
  and the login error via the template context instead of flash().
- Only send the session deletion cookie if the client sent a cookie.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Makefile

@@ -10,7 +10,7 @@ run-dev:
 
 run-dev-as-prod:
 	mkdir -p log
-	FLASK_APP=max.app LOG_DIR=log gunicorn -w 1 -b 0.0.0.0:8757 max.app:app
+	FLASK_APP=max.app LOG_DIR=log MAX_ORIGIN=http://localhost:8757 gunicorn -w 1 -b 0.0.0.0:8757 max.app:app
 
 run-cleanup:
 	scripts/dev/cleanup.sh

README.md

@@ -9,3 +9,8 @@ The proxy should add X-Real-IP to get a better logging.
 Requirements: Docker, Make
 
 Start with `make run-dev`
+
+## Configuration
+
+- `MAX_ORIGIN` (required in production): The public origin of MAX, e.g.
+  `https://max.finn.st`

docker-compose.yml

@@ -10,6 +10,7 @@ services:
             - PGSQL_USER=max
             - PGSQL_PASSWORD=max
             - PGSQL_NAME=max
+            - MAX_ORIGIN=https://max.finn.st
         networks:
             - postgresql
             - uplink

max/app.py

@@ -37,7 +37,13 @@ def main():
     app.jinja_env.trim_blocks = True
     app.jinja_env.lstrip_blocks = True
     app.url_rule_class = Rule
+    max_origin = os.environ.get("MAX_ORIGIN")
+    if not max_origin and not app.debug:
+        raise RuntimeError(
+            "MAX_ORIGIN must be set to the public origin, e.g. https://max.example.com"
+        )
     app.config.update(
+        MAX_ORIGIN=max_origin.rstrip("/") if max_origin else None,
         SECRET_KEY=(
             "123456" if app.debug else base64.b64encode(os.urandom(64)).decode("utf-8")
         ),

max/auth/middlewares.py

@@ -23,12 +23,12 @@ class AuthMiddleware(BaseMiddleware):
         if not getattr(self.handler, "no_auth", False) and not auth_user:
             logout()  # remove the session (if any)
             # No usage of flash here to not create a session...
-            return redirect(url_for("login", a=1))
+            return redirect(url_for("login", msg="login_required"))
 
         if auth_user:
             if not auth_user.enabled:
                 logout()
-                return redirect(url_for("login", a=1))
+                return redirect(url_for("login", msg="login_required"))
 
             kwargs["auth_user"] = auth_user
         return self.handler(*args, **kwargs)

max/auth/routes.py

@@ -8,6 +8,12 @@ from max.views import BaseTemplateGetView, BaseView
 from .auth import login, logout
 from .middlewares import no_authentication_required
 
+# Messages for the login page (text, category), selected by ?msg=<key>.
+LOGIN_MESSAGES = {
+    "login_required": ("Please login to visit the requested url", "error"),
+    "logout": ("Logout was sucessfull!", "success"),
+}
+
 
 def init_routes(app):
     app.add_url_rule(
@@ -26,7 +32,11 @@ class Login(AllowAnyAccess, BaseTemplateGetView):
 
     def get_context(self):
         context = super().get_context()
-        context["alert"] = bool(self.request.args.get("a", False))
+        if "message" not in context:
+            message = LOGIN_MESSAGES.get(self.request.args.get("msg"))
+            if message is not None:
+                context["message"] = t(message[0])
+                context["message_category"] = message[1]
         return context
 
     def get_webpagetitle(self):
@@ -41,7 +51,8 @@ class Login(AllowAnyAccess, BaseTemplateGetView):
             self.app.logger.info(
                 "Failed login attempt from %s", self.request.real_remote_addr
             )
-            flash(error, category="error")
+            self.context["message"] = error
+            self.context["message_category"] = "error"
             self.context["email"] = email
         else:
             flash(t("Login was sucessfull!"), category="success")
@@ -51,5 +62,4 @@ class Login(AllowAnyAccess, BaseTemplateGetView):
 class Logout(AllowAnyAccess, BaseView):
     def post(self):
         logout()
-        flash(t("Logout was sucessfull!"), category="success")
+        return self.redirect(url_for("login", msg="logout"))
-        return self.redirect(url_for("login"))

max/csrf.py

@@ -3,39 +3,70 @@ from functools import wraps
 from flask import request, session
 
 from max.crypto import constant_time_compare, get_random_string
-from max.middlewares import BaseMiddleware
+from max.middlewares import BaseMiddleware, get_real_remote_addr
 
 CSRF_TOKEN_KEY = "csrf_token"
 CSRF_TOKEN_LENGTH = 32
+SAFE_METHODS = ("GET", "HEAD", "OPTIONS")
+# "none": navigation started by the user (e.g. reload). "same-site" is rejected,
+# since other services on a subdomain must not trigger requests.
+ALLOWED_FETCH_SITES = ("same-origin", "none")
 
 
 class CsrfMiddleware(BaseMiddleware):
     def __call__(self, *args, **kwargs):
         error = self.process_request()
         if error is not None:
-            # clear csrf from session
+            return error
-            if CSRF_TOKEN_KEY in session:
-                del session[CSRF_TOKEN_KEY]
-            return error, 400
 
         return self.handler(*args, **kwargs)
 
     def process_request(self):
-        """If a string is returned, it mist be used as a 400er error"""
+        """If a (message, status) tuple is returned, it must be used as the response"""
-        if getattr(self.handler, "no_csrf", False):
+        no_csrf = getattr(self.handler, "no_csrf", False)
+
+        if request.method in SAFE_METHODS:
+            if not no_csrf and CSRF_TOKEN_KEY not in session:
+                session[CSRF_TOKEN_KEY] = get_random_string(CSRF_TOKEN_LENGTH)
             return
 
-        session_has_csrf_token = CSRF_TOKEN_KEY in session
+        # Stateless check, so it also protects routes without a session (login)
-        method_is_unsafe = request.method.lower() != "get"
+        if not self.is_origin_allowed():
+            self.app.logger.debug(
+                'Blocked cross-origin request %s %s origin="%s" sec_fetch_site="%s" from %s',
+                request.method,
+                request.path,
+                request.headers.get("Origin"),
+                request.headers.get("Sec-Fetch-Site"),
+                get_real_remote_addr(self.app),
+            )
+            return "Cross-origin request blocked", 403
 
-        if not session_has_csrf_token and method_is_unsafe:
+        if no_csrf:
-            return "CSRF token is missing"
+            return
 
-        elif not session_has_csrf_token and not method_is_unsafe:
+        if CSRF_TOKEN_KEY not in session:
-            session[CSRF_TOKEN_KEY] = get_random_string(CSRF_TOKEN_LENGTH)
+            return "CSRF token is missing", 400
 
-        elif session_has_csrf_token and method_is_unsafe:
+        error = self.check_token(session[CSRF_TOKEN_KEY], request.form)
-            return self.check_token(session[CSRF_TOKEN_KEY], request.form)
+        if error is not None:
+            return error, 400
+
+    def is_origin_allowed(self):
+        fetch_site = request.headers.get("Sec-Fetch-Site")
+        if fetch_site is not None:
+            return fetch_site in ALLOWED_FETCH_SITES
+
+        origin = request.headers.get("Origin")
+        if origin is None:
+            return True  # not a browser, so no CSRF possible
+        return origin == self.get_expected_origin()
+
+    def get_expected_origin(self):
+        origin = self.app.config["MAX_ORIGIN"]
+        if origin is None:
+            origin = request.host_url.rstrip("/")
+        return origin
 
     def check_token(self, session_token, formdata):
         if CSRF_TOKEN_KEY not in formdata:

max/middlewares.py

@@ -20,12 +20,16 @@ class BaseMiddleware:
         raise NotImplementedError()
 
 
+def get_real_remote_addr(app):
+    real_ip = request.headers.get("X-Real-IP", None)
+    if real_ip is None and app.debug:
+        real_ip = request.remote_addr
+    return real_ip
+
+
 class LoggingMiddleware(BaseMiddleware):
     def __call__(self, *args, **kwargs):
-        real_ip = request.headers.get("X-Real-IP", None)
+        setattr(request, "real_remote_addr", get_real_remote_addr(self.app))
-        if real_ip is None and self.app.debug:
-            real_ip = request.remote_addr
-        setattr(request, "real_remote_addr", real_ip)
 
         start = time.time()
 

max/sessions.py

@@ -1,3 +1,4 @@
+from flask import request
 from flask.sessions import SessionInterface as FlaskSessionInterface, SessionMixin
 from itsdangerous import BadSignature, Signer, want_bytes
 from werkzeug.datastructures import CallbackDict
@@ -113,7 +114,9 @@ class SessionInterface(FlaskSessionInterface):
         if session.is_empty():
             if session.modified:
                 del session_storage[session.id]
-            response.delete_cookie(cookie_name, domain=domain, path=path)
+            # Only send the deletion header if the client sent a cookie
+            if cookie_name in request.cookies:
+                response.delete_cookie(cookie_name, domain=domain, path=path)
         else:
             httponly = self.get_cookie_httponly(app)
             secure = self.get_cookie_secure(app)

max/templates/auth/login.html

@@ -2,11 +2,8 @@
 {% block content %}
     <h1>{{ t("Login") }}</h1>
    
-    {% for category, message in get_flashed_messages(with_categories=true) %}
+    {% if message %}
-    <div class="msg-{{ category }}">{{ t(message) }}</div>
+    <div class="msg-{{ message_category }}">{{ message }}</div>
-    {% endfor %}
-    {% if alert %}
-    <div class="msg-error">{{ t("Please login to visit the requested url") }}</div>
     {% endif %}
 
     <form class="pure-form" action="/login" method="post">