fix(auth): 401 dal backend svuota la sessione e segnala session_expired (#25)
api_client._handle_response: su 401 pulisce la sessione Flask e marca g.session_expired. Un solo after_request (app.py) trasforma le pagine HTML rimaste in piedi con un errore generico in un redirect al login con flash; le route JSON/AJAX che già inoltrano il dict di api_client (measure.py, statistics.py) ricevono session_expired nel corpo insieme al 401. Backend (riuso della key, bcrypt a tempo costante) resta al lotto A2. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -5,7 +5,7 @@ import re
|
||||
from datetime import date
|
||||
from urllib.parse import urlparse
|
||||
|
||||
from flask import Flask, flash, jsonify, redirect, url_for, session, request
|
||||
from flask import Flask, flash, g, jsonify, redirect, url_for, session, request
|
||||
from flask_babel import Babel, gettext as _
|
||||
from flask_wtf.csrf import CSRFError, CSRFProtect
|
||||
from markupsafe import Markup, escape
|
||||
@@ -186,6 +186,23 @@ def create_app() -> Flask:
|
||||
response.headers.setdefault("Referrer-Policy", "strict-origin-when-cross-origin")
|
||||
return response
|
||||
|
||||
@app.after_request
|
||||
def handle_expired_session(response):
|
||||
"""Turn a 401 from the backend into a redirect, for page loads (#25).
|
||||
|
||||
api_client already emptied the session and set ``g.session_expired``
|
||||
on any 401. JSON/AJAX proxy routes already forward the api_client
|
||||
dict (which carries ``session_expired``) as their own body, so a
|
||||
fetch caller has what it needs. What is left is the page views that
|
||||
swallowed the error (flashed something generic and rendered anyway,
|
||||
e.g. an empty recipe list): those get replaced here, in the one
|
||||
place that sees every response, instead of one check per route.
|
||||
"""
|
||||
if getattr(g, "session_expired", False) and not response.is_json:
|
||||
flash(_("Sessione scaduta: effettua nuovamente l'accesso."), "error")
|
||||
return redirect(url_for("auth.login"))
|
||||
return response
|
||||
|
||||
@app.context_processor
|
||||
def inject_globals():
|
||||
"""Inject global variables into all templates."""
|
||||
|
||||
@@ -481,10 +481,13 @@ def save_measurement():
|
||||
|
||||
if resp.get("error"):
|
||||
status_code = resp.get("status_code", 500)
|
||||
return jsonify({
|
||||
error_body = {
|
||||
"error": True,
|
||||
"detail": resp.get("detail", _("Errore nel salvataggio")),
|
||||
}), status_code if status_code >= 400 else 500
|
||||
}
|
||||
if resp.get("session_expired"):
|
||||
error_body["session_expired"] = True
|
||||
return jsonify(error_body), status_code if status_code >= 400 else 500
|
||||
|
||||
return jsonify(resp), 201
|
||||
|
||||
|
||||
@@ -48,7 +48,10 @@ def _proxy_statistics(endpoint: str):
|
||||
resp = api_client.get(f"/api/statistics/{endpoint}", params=params)
|
||||
|
||||
if isinstance(resp, dict) and resp.get("error"):
|
||||
return jsonify({"error": True, "detail": resp.get("detail", "")}), resp.get("status_code", 500)
|
||||
error_body = {"error": True, "detail": resp.get("detail", "")}
|
||||
if resp.get("session_expired"):
|
||||
error_body["session_expired"] = True
|
||||
return jsonify(error_body), resp.get("status_code", 500)
|
||||
|
||||
return jsonify(resp)
|
||||
|
||||
@@ -101,7 +104,10 @@ def api_subtasks():
|
||||
resp = api_client.get("/api/statistics/subtasks", params=params)
|
||||
|
||||
if isinstance(resp, dict) and resp.get("error"):
|
||||
return jsonify({"error": True, "detail": resp.get("detail", "")}), resp.get("status_code", 500)
|
||||
error_body = {"error": True, "detail": resp.get("detail", "")}
|
||||
if resp.get("session_expired"):
|
||||
error_body["session_expired"] = True
|
||||
return jsonify(error_body), resp.get("status_code", 500)
|
||||
|
||||
# Response is a list
|
||||
if isinstance(resp, list):
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
from typing import Any
|
||||
|
||||
import requests
|
||||
from flask import has_request_context, request, session
|
||||
from flask import g, has_request_context, request, session
|
||||
|
||||
from config import Config
|
||||
|
||||
@@ -39,6 +39,21 @@ class APIClient:
|
||||
headers["X-Real-IP"] = client_ip
|
||||
return headers
|
||||
|
||||
@staticmethod
|
||||
def _handle_session_expired() -> None:
|
||||
"""The backend says our api_key is no longer valid (#25).
|
||||
|
||||
One point for the whole app: whichever route made the call, the
|
||||
Flask session is emptied here so nothing downstream keeps acting as
|
||||
that user. A flag is left on ``g`` for the single after_request
|
||||
hook in app.py to turn into a redirect-to-login on HTML pages; JSON
|
||||
callers already get ``session_expired`` in the body returned below.
|
||||
"""
|
||||
if not has_request_context():
|
||||
return
|
||||
session.clear()
|
||||
g.session_expired = True
|
||||
|
||||
def _handle_response(self, response: requests.Response) -> dict[str, Any]:
|
||||
"""Parse response and return normalized dict.
|
||||
|
||||
@@ -46,12 +61,16 @@ class APIClient:
|
||||
- If 2xx and not 204: response JSON
|
||||
- If 204 No Content: empty dict
|
||||
- If 4xx/5xx: {"error": True, "status_code": ..., "detail": "..."}
|
||||
(401 also carries "session_expired": True)
|
||||
"""
|
||||
if response.ok:
|
||||
if response.status_code == 204:
|
||||
return {}
|
||||
return response.json()
|
||||
|
||||
if response.status_code == 401:
|
||||
self._handle_session_expired()
|
||||
|
||||
# Non-OK response (4xx/5xx)
|
||||
try:
|
||||
error_body = response.json()
|
||||
@@ -74,11 +93,14 @@ class APIClient:
|
||||
except Exception:
|
||||
detail = response.text or f"HTTP {response.status_code}"
|
||||
|
||||
return {
|
||||
result = {
|
||||
"error": True,
|
||||
"status_code": response.status_code,
|
||||
"detail": detail
|
||||
}
|
||||
if response.status_code == 401:
|
||||
result["session_expired"] = True
|
||||
return result
|
||||
|
||||
def get(self, endpoint: str, params: dict | None = None) -> dict[str, Any]:
|
||||
"""GET request to API server."""
|
||||
|
||||
@@ -0,0 +1,100 @@
|
||||
"""Tests for the 401 -> session-expired handling (#25, frontend half).
|
||||
|
||||
Backend half (auth_service key reuse, constant-time auth) is lot A2 and
|
||||
lives in src/backend - not covered here.
|
||||
"""
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
from flask import g, session
|
||||
|
||||
|
||||
def _mock_401_response(detail="Invalid or expired API key"):
|
||||
resp = MagicMock()
|
||||
resp.ok = False
|
||||
resp.status_code = 401
|
||||
resp.json.return_value = {"detail": detail}
|
||||
resp.text = detail
|
||||
return resp
|
||||
|
||||
|
||||
class TestApiClientHandlesResponse:
|
||||
"""Unit-level: services/api_client.py clears the session on 401."""
|
||||
|
||||
def test_401_clears_session_and_flags_g(self, flask_app):
|
||||
from services.api_client import api_client
|
||||
|
||||
with flask_app.test_request_context("/"):
|
||||
session["api_key"] = "stale-key"
|
||||
session["user"] = {"id": 1, "username": "op1"}
|
||||
|
||||
result = api_client._handle_response(_mock_401_response())
|
||||
|
||||
assert result["error"] is True
|
||||
assert result["status_code"] == 401
|
||||
assert result["session_expired"] is True
|
||||
assert "api_key" not in session
|
||||
assert "user" not in session
|
||||
assert g.session_expired is True
|
||||
|
||||
def test_non_401_error_does_not_touch_session(self, flask_app):
|
||||
from services.api_client import api_client
|
||||
|
||||
with flask_app.test_request_context("/"):
|
||||
session["api_key"] = "still-good"
|
||||
|
||||
resp = MagicMock(ok=False, status_code=404)
|
||||
resp.json.return_value = {"detail": "not found"}
|
||||
result = api_client._handle_response(resp)
|
||||
|
||||
assert "session_expired" not in result
|
||||
assert session.get("api_key") == "still-good"
|
||||
assert not getattr(g, "session_expired", False)
|
||||
|
||||
|
||||
class TestHtmlPageRedirectsOnSessionExpired:
|
||||
"""Integration: a page view that got a 401 ends up redirected to login."""
|
||||
|
||||
def test_recipe_list_401_redirects_to_login(self, logged_in_client):
|
||||
with patch("services.api_client.requests.get", return_value=_mock_401_response()):
|
||||
resp = logged_in_client.get("/maker/recipes", follow_redirects=False)
|
||||
|
||||
assert resp.status_code == 302
|
||||
assert "/auth/login" in resp.headers["Location"]
|
||||
|
||||
def test_recipe_list_401_empties_the_session(self, logged_in_client):
|
||||
with patch("services.api_client.requests.get", return_value=_mock_401_response()):
|
||||
logged_in_client.get("/maker/recipes", follow_redirects=False)
|
||||
|
||||
with logged_in_client.session_transaction() as sess:
|
||||
assert "api_key" not in sess
|
||||
assert "user" not in sess
|
||||
|
||||
def test_recipe_list_401_flashes_session_expired_message(self, logged_in_client):
|
||||
with patch("services.api_client.requests.get", return_value=_mock_401_response()):
|
||||
resp = logged_in_client.get("/maker/recipes", follow_redirects=True)
|
||||
|
||||
assert "Sessione scaduta" in resp.get_data(as_text=True)
|
||||
|
||||
|
||||
class TestJsonProxyReturns401WithFlag:
|
||||
"""Integration: an AJAX proxy route surfaces session_expired in its JSON body."""
|
||||
|
||||
def test_save_measurement_401_returns_session_expired_flag(self, logged_in_client):
|
||||
with patch("services.api_client.requests.post", return_value=_mock_401_response()):
|
||||
resp = logged_in_client.post(
|
||||
"/measure/save-measurement",
|
||||
json={"subtask_id": 1, "version_id": 1, "value": 1.0},
|
||||
)
|
||||
|
||||
assert resp.status_code == 401
|
||||
body = resp.get_json()
|
||||
assert body["error"] is True
|
||||
assert body["session_expired"] is True
|
||||
|
||||
def test_statistics_summary_401_returns_session_expired_flag(self, logged_in_client):
|
||||
with patch("services.api_client.requests.get", return_value=_mock_401_response()):
|
||||
resp = logged_in_client.get("/statistics/api/summary")
|
||||
|
||||
assert resp.status_code == 401
|
||||
body = resp.get_json()
|
||||
assert body["session_expired"] is True
|
||||
Reference in New Issue
Block a user