refactor: replace localStorage with HTTP-only cookies for auth tokens (#24365)
Signed-off-by: NeatGuyCoding <15627489+NeatGuyCoding@users.noreply.github.com> Signed-off-by: lyzno1 <yuanyouhuilyz@gmail.com> Signed-off-by: kenwoodjw <blackxin55+@gmail.com> Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Yunlu Wen <wylswz@163.com> Co-authored-by: Joel <iamjoel007@gmail.com> Co-authored-by: GareArc <chen4851@purdue.edu> Co-authored-by: NFish <douxc512@gmail.com> Co-authored-by: Davide Delbianco <davide.delbianco@outlook.com> Co-authored-by: minglu7 <1347866672@qq.com> Co-authored-by: Ponder <ruan.lj@foxmail.com> Co-authored-by: crazywoola <100913391+crazywoola@users.noreply.github.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: heyszt <270985384@qq.com> Co-authored-by: Asuka Minato <i@asukaminato.eu.org> Co-authored-by: Guangdong Liu <liugddx@gmail.com> Co-authored-by: Eric Guo <eric.guocz@gmail.com> Co-authored-by: NeatGuyCoding <15627489+NeatGuyCoding@users.noreply.github.com> Co-authored-by: XlKsyt <caixuesen@outlook.com> Co-authored-by: Dhruv Gorasiya <80987415+DhruvGorasiya@users.noreply.github.com> Co-authored-by: crazywoola <427733928@qq.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: lyzno1 <92089059+lyzno1@users.noreply.github.com> Co-authored-by: hj24 <mambahj24@gmail.com> Co-authored-by: GuanMu <ballmanjq@gmail.com> Co-authored-by: 非法操作 <hjlarry@163.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Tonlo <123lzs123@gmail.com> Co-authored-by: Yusuke Yamada <yamachu.dev@gmail.com> Co-authored-by: Novice <novice12185727@gmail.com> Co-authored-by: kenwoodjw <blackxin55+@gmail.com> Co-authored-by: Ademílson Tonato <ademilsonft@outlook.com> Co-authored-by: znn <jubinkumarsoni@gmail.com> Co-authored-by: yangzheli <43645580+yangzheli@users.noreply.github.com>
This commit is contained in:
@@ -2,7 +2,9 @@ from flask import Blueprint, Flask
|
||||
from flask_restx import Resource
|
||||
from werkzeug.exceptions import BadRequest, Unauthorized
|
||||
|
||||
from constants import COOKIE_NAME_ACCESS_TOKEN, COOKIE_NAME_CSRF_TOKEN, COOKIE_NAME_REFRESH_TOKEN
|
||||
from core.errors.error import AppInvokeQuotaExceededError
|
||||
from libs.exception import BaseHTTPException
|
||||
from libs.external_api import ExternalApi
|
||||
|
||||
|
||||
@@ -120,3 +122,66 @@ def test_external_api_param_mapping_and_quota_and_exc_info_none():
|
||||
assert res.status_code in (400, 429)
|
||||
finally:
|
||||
ext.sys.exc_info = orig_exc_info # type: ignore[assignment]
|
||||
|
||||
|
||||
def test_unauthorized_and_force_logout_clears_cookies():
|
||||
"""Test that UnauthorizedAndForceLogout error clears auth cookies"""
|
||||
|
||||
class UnauthorizedAndForceLogout(BaseHTTPException):
|
||||
error_code = "unauthorized_and_force_logout"
|
||||
description = "Unauthorized and force logout."
|
||||
code = 401
|
||||
|
||||
app = Flask(__name__)
|
||||
bp = Blueprint("test", __name__)
|
||||
api = ExternalApi(bp)
|
||||
|
||||
@api.route("/force-logout")
|
||||
class ForceLogout(Resource): # type: ignore
|
||||
def get(self): # type: ignore
|
||||
raise UnauthorizedAndForceLogout()
|
||||
|
||||
app.register_blueprint(bp, url_prefix="/api")
|
||||
client = app.test_client()
|
||||
|
||||
# Set cookies first
|
||||
client.set_cookie(COOKIE_NAME_ACCESS_TOKEN, "test_access_token")
|
||||
client.set_cookie(COOKIE_NAME_CSRF_TOKEN, "test_csrf_token")
|
||||
client.set_cookie(COOKIE_NAME_REFRESH_TOKEN, "test_refresh_token")
|
||||
|
||||
# Make request that should trigger cookie clearing
|
||||
res = client.get("/api/force-logout")
|
||||
|
||||
# Verify response
|
||||
assert res.status_code == 401
|
||||
data = res.get_json()
|
||||
assert data["code"] == "unauthorized_and_force_logout"
|
||||
assert data["status"] == 401
|
||||
assert "WWW-Authenticate" in res.headers
|
||||
|
||||
# Verify Set-Cookie headers are present to clear cookies
|
||||
set_cookie_headers = res.headers.getlist("Set-Cookie")
|
||||
assert len(set_cookie_headers) == 3, f"Expected 3 Set-Cookie headers, got {len(set_cookie_headers)}"
|
||||
|
||||
# Verify each cookie is being cleared (empty value and expired)
|
||||
cookie_names_found = set()
|
||||
for cookie_header in set_cookie_headers:
|
||||
# Check for cookie names
|
||||
if COOKIE_NAME_ACCESS_TOKEN in cookie_header:
|
||||
cookie_names_found.add(COOKIE_NAME_ACCESS_TOKEN)
|
||||
assert '""' in cookie_header or "=" in cookie_header # Empty value
|
||||
assert "Expires=Thu, 01 Jan 1970" in cookie_header # Expired
|
||||
elif COOKIE_NAME_CSRF_TOKEN in cookie_header:
|
||||
cookie_names_found.add(COOKIE_NAME_CSRF_TOKEN)
|
||||
assert '""' in cookie_header or "=" in cookie_header
|
||||
assert "Expires=Thu, 01 Jan 1970" in cookie_header
|
||||
elif COOKIE_NAME_REFRESH_TOKEN in cookie_header:
|
||||
cookie_names_found.add(COOKIE_NAME_REFRESH_TOKEN)
|
||||
assert '""' in cookie_header or "=" in cookie_header
|
||||
assert "Expires=Thu, 01 Jan 1970" in cookie_header
|
||||
|
||||
# Verify all three cookies are present
|
||||
assert len(cookie_names_found) == 3
|
||||
assert COOKIE_NAME_ACCESS_TOKEN in cookie_names_found
|
||||
assert COOKIE_NAME_CSRF_TOKEN in cookie_names_found
|
||||
assert COOKIE_NAME_REFRESH_TOKEN in cookie_names_found
|
||||
|
||||
@@ -19,10 +19,15 @@ class MockUser(UserMixin):
|
||||
return self._is_authenticated
|
||||
|
||||
|
||||
def mock_csrf_check(*args, **kwargs):
|
||||
return
|
||||
|
||||
|
||||
class TestLoginRequired:
|
||||
"""Test cases for login_required decorator."""
|
||||
|
||||
@pytest.fixture
|
||||
@patch("libs.login.check_csrf_token", mock_csrf_check)
|
||||
def setup_app(self, app: Flask):
|
||||
"""Set up Flask app with login manager."""
|
||||
# Initialize login manager
|
||||
@@ -39,6 +44,7 @@ class TestLoginRequired:
|
||||
|
||||
return app
|
||||
|
||||
@patch("libs.login.check_csrf_token", mock_csrf_check)
|
||||
def test_authenticated_user_can_access_protected_view(self, setup_app: Flask):
|
||||
"""Test that authenticated users can access protected views."""
|
||||
|
||||
@@ -53,6 +59,7 @@ class TestLoginRequired:
|
||||
result = protected_view()
|
||||
assert result == "Protected content"
|
||||
|
||||
@patch("libs.login.check_csrf_token", mock_csrf_check)
|
||||
def test_unauthenticated_user_cannot_access_protected_view(self, setup_app: Flask):
|
||||
"""Test that unauthenticated users are redirected."""
|
||||
|
||||
@@ -68,6 +75,7 @@ class TestLoginRequired:
|
||||
assert result == "Unauthorized"
|
||||
setup_app.login_manager.unauthorized.assert_called_once()
|
||||
|
||||
@patch("libs.login.check_csrf_token", mock_csrf_check)
|
||||
def test_login_disabled_allows_unauthenticated_access(self, setup_app: Flask):
|
||||
"""Test that LOGIN_DISABLED config bypasses authentication."""
|
||||
|
||||
@@ -87,6 +95,7 @@ class TestLoginRequired:
|
||||
# Ensure unauthorized was not called
|
||||
setup_app.login_manager.unauthorized.assert_not_called()
|
||||
|
||||
@patch("libs.login.check_csrf_token", mock_csrf_check)
|
||||
def test_options_request_bypasses_authentication(self, setup_app: Flask):
|
||||
"""Test that OPTIONS requests are exempt from authentication."""
|
||||
|
||||
@@ -103,6 +112,7 @@ class TestLoginRequired:
|
||||
# Ensure unauthorized was not called
|
||||
setup_app.login_manager.unauthorized.assert_not_called()
|
||||
|
||||
@patch("libs.login.check_csrf_token", mock_csrf_check)
|
||||
def test_flask_2_compatibility(self, setup_app: Flask):
|
||||
"""Test Flask 2.x compatibility with ensure_sync."""
|
||||
|
||||
@@ -120,6 +130,7 @@ class TestLoginRequired:
|
||||
assert result == "Synced content"
|
||||
setup_app.ensure_sync.assert_called_once()
|
||||
|
||||
@patch("libs.login.check_csrf_token", mock_csrf_check)
|
||||
def test_flask_1_compatibility(self, setup_app: Flask):
|
||||
"""Test Flask 1.x compatibility without ensure_sync."""
|
||||
|
||||
|
||||
23
api/tests/unit_tests/libs/test_token.py
Normal file
23
api/tests/unit_tests/libs/test_token.py
Normal file
@@ -0,0 +1,23 @@
|
||||
from constants import COOKIE_NAME_ACCESS_TOKEN
|
||||
from libs.token import extract_access_token
|
||||
|
||||
|
||||
class MockRequest:
|
||||
def __init__(self, headers: dict[str, str], cookies: dict[str, str], args: dict[str, str]):
|
||||
self.headers: dict[str, str] = headers
|
||||
self.cookies: dict[str, str] = cookies
|
||||
self.args: dict[str, str] = args
|
||||
|
||||
|
||||
def test_extract_access_token():
|
||||
def _mock_request(headers: dict[str, str], cookies: dict[str, str], args: dict[str, str]):
|
||||
return MockRequest(headers, cookies, args)
|
||||
|
||||
test_cases = [
|
||||
(_mock_request({"Authorization": "Bearer 123"}, {}, {}), "123"),
|
||||
(_mock_request({}, {COOKIE_NAME_ACCESS_TOKEN: "123"}, {}), "123"),
|
||||
(_mock_request({}, {}, {}), None),
|
||||
(_mock_request({"Authorization": "Bearer_aaa 123"}, {}, {}), None),
|
||||
]
|
||||
for request, expected in test_cases:
|
||||
assert extract_access_token(request) == expected # pyright: ignore[reportArgumentType]
|
||||
Reference in New Issue
Block a user