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 datetime import date
|
||||||
from urllib.parse import urlparse
|
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_babel import Babel, gettext as _
|
||||||
from flask_wtf.csrf import CSRFError, CSRFProtect
|
from flask_wtf.csrf import CSRFError, CSRFProtect
|
||||||
from markupsafe import Markup, escape
|
from markupsafe import Markup, escape
|
||||||
@@ -186,6 +186,23 @@ def create_app() -> Flask:
|
|||||||
response.headers.setdefault("Referrer-Policy", "strict-origin-when-cross-origin")
|
response.headers.setdefault("Referrer-Policy", "strict-origin-when-cross-origin")
|
||||||
return response
|
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
|
@app.context_processor
|
||||||
def inject_globals():
|
def inject_globals():
|
||||||
"""Inject global variables into all templates."""
|
"""Inject global variables into all templates."""
|
||||||
|
|||||||
@@ -481,10 +481,13 @@ def save_measurement():
|
|||||||
|
|
||||||
if resp.get("error"):
|
if resp.get("error"):
|
||||||
status_code = resp.get("status_code", 500)
|
status_code = resp.get("status_code", 500)
|
||||||
return jsonify({
|
error_body = {
|
||||||
"error": True,
|
"error": True,
|
||||||
"detail": resp.get("detail", _("Errore nel salvataggio")),
|
"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
|
return jsonify(resp), 201
|
||||||
|
|
||||||
|
|||||||
@@ -48,7 +48,10 @@ def _proxy_statistics(endpoint: str):
|
|||||||
resp = api_client.get(f"/api/statistics/{endpoint}", params=params)
|
resp = api_client.get(f"/api/statistics/{endpoint}", params=params)
|
||||||
|
|
||||||
if isinstance(resp, dict) and resp.get("error"):
|
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)
|
return jsonify(resp)
|
||||||
|
|
||||||
@@ -101,7 +104,10 @@ def api_subtasks():
|
|||||||
resp = api_client.get("/api/statistics/subtasks", params=params)
|
resp = api_client.get("/api/statistics/subtasks", params=params)
|
||||||
|
|
||||||
if isinstance(resp, dict) and resp.get("error"):
|
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
|
# Response is a list
|
||||||
if isinstance(resp, list):
|
if isinstance(resp, list):
|
||||||
|
|||||||
@@ -2,7 +2,7 @@
|
|||||||
from typing import Any
|
from typing import Any
|
||||||
|
|
||||||
import requests
|
import requests
|
||||||
from flask import has_request_context, request, session
|
from flask import g, has_request_context, request, session
|
||||||
|
|
||||||
from config import Config
|
from config import Config
|
||||||
|
|
||||||
@@ -39,6 +39,21 @@ class APIClient:
|
|||||||
headers["X-Real-IP"] = client_ip
|
headers["X-Real-IP"] = client_ip
|
||||||
return headers
|
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]:
|
def _handle_response(self, response: requests.Response) -> dict[str, Any]:
|
||||||
"""Parse response and return normalized dict.
|
"""Parse response and return normalized dict.
|
||||||
|
|
||||||
@@ -46,12 +61,16 @@ class APIClient:
|
|||||||
- If 2xx and not 204: response JSON
|
- If 2xx and not 204: response JSON
|
||||||
- If 204 No Content: empty dict
|
- If 204 No Content: empty dict
|
||||||
- If 4xx/5xx: {"error": True, "status_code": ..., "detail": "..."}
|
- If 4xx/5xx: {"error": True, "status_code": ..., "detail": "..."}
|
||||||
|
(401 also carries "session_expired": True)
|
||||||
"""
|
"""
|
||||||
if response.ok:
|
if response.ok:
|
||||||
if response.status_code == 204:
|
if response.status_code == 204:
|
||||||
return {}
|
return {}
|
||||||
return response.json()
|
return response.json()
|
||||||
|
|
||||||
|
if response.status_code == 401:
|
||||||
|
self._handle_session_expired()
|
||||||
|
|
||||||
# Non-OK response (4xx/5xx)
|
# Non-OK response (4xx/5xx)
|
||||||
try:
|
try:
|
||||||
error_body = response.json()
|
error_body = response.json()
|
||||||
@@ -74,11 +93,14 @@ class APIClient:
|
|||||||
except Exception:
|
except Exception:
|
||||||
detail = response.text or f"HTTP {response.status_code}"
|
detail = response.text or f"HTTP {response.status_code}"
|
||||||
|
|
||||||
return {
|
result = {
|
||||||
"error": True,
|
"error": True,
|
||||||
"status_code": response.status_code,
|
"status_code": response.status_code,
|
||||||
"detail": detail
|
"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]:
|
def get(self, endpoint: str, params: dict | None = None) -> dict[str, Any]:
|
||||||
"""GET request to API server."""
|
"""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