mirror of
https://github.com/langgenius/dify.git
synced 2026-09-08 02:43:49 +08:00
refactor(api): Stop masking refresh-token service errors as 401 (#38463)
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
This commit is contained in:
parent
d0ea5a5e0d
commit
fc01d112a0
@ -56,7 +56,7 @@ from models.account import Account
|
|||||||
from services.account_service import AccountService, InvitationDetailDict, RegisterService, TenantService
|
from services.account_service import AccountService, InvitationDetailDict, RegisterService, TenantService
|
||||||
from services.billing_service import BillingService
|
from services.billing_service import BillingService
|
||||||
from services.entities.auth_entities import LoginFailureReason, LoginPayloadBase
|
from services.entities.auth_entities import LoginFailureReason, LoginPayloadBase
|
||||||
from services.errors.account import AccountRegisterError
|
from services.errors.account import AccountRegisterError, RefreshTokenAccountNotFoundError, RefreshTokenNotFoundError
|
||||||
from services.errors.workspace import WorkSpaceNotAllowedCreateError, WorkspacesLimitExceededError
|
from services.errors.workspace import WorkSpaceNotAllowedCreateError, WorkspacesLimitExceededError
|
||||||
from services.feature_service import FeatureService
|
from services.feature_service import FeatureService
|
||||||
|
|
||||||
@ -359,18 +359,22 @@ class RefreshTokenApi(Resource):
|
|||||||
|
|
||||||
try:
|
try:
|
||||||
new_token_pair = AccountService.refresh_token(refresh_token, session=db.session)
|
new_token_pair = AccountService.refresh_token(refresh_token, session=db.session)
|
||||||
|
except Unauthorized as exc:
|
||||||
|
return SimpleResultMessageResponse(result="fail", message=exc.description or "Unauthorized.").model_dump(
|
||||||
|
mode="json"
|
||||||
|
), 401
|
||||||
|
except (RefreshTokenNotFoundError, RefreshTokenAccountNotFoundError) as exc:
|
||||||
|
return SimpleResultMessageResponse(result="fail", message=str(exc)).model_dump(mode="json"), 401
|
||||||
|
|
||||||
# Create response with new cookies
|
# Create response with new cookies
|
||||||
# response-contract:ignore cookie-bearing Flask response
|
# response-contract:ignore cookie-bearing Flask response
|
||||||
response = make_response(SimpleResultResponse(result="success").model_dump(mode="json"))
|
response = make_response(SimpleResultResponse(result="success").model_dump(mode="json"))
|
||||||
|
|
||||||
# Update cookies with new tokens
|
# Update cookies with new tokens
|
||||||
set_csrf_token_to_cookie(request, response, new_token_pair.csrf_token)
|
set_csrf_token_to_cookie(request, response, new_token_pair.csrf_token)
|
||||||
set_access_token_to_cookie(request, response, new_token_pair.access_token)
|
set_access_token_to_cookie(request, response, new_token_pair.access_token)
|
||||||
set_refresh_token_to_cookie(request, response, new_token_pair.refresh_token)
|
set_refresh_token_to_cookie(request, response, new_token_pair.refresh_token)
|
||||||
return response
|
return response
|
||||||
except Exception as e:
|
|
||||||
return SimpleResultMessageResponse(result="fail", message=str(e)).model_dump(mode="json"), 401
|
|
||||||
|
|
||||||
|
|
||||||
def _get_account_with_case_fallback(email: str):
|
def _get_account_with_case_fallback(email: str):
|
||||||
|
|||||||
@ -65,6 +65,8 @@ from services.errors.account import (
|
|||||||
LinkAccountIntegrateError,
|
LinkAccountIntegrateError,
|
||||||
MemberNotInTenantError,
|
MemberNotInTenantError,
|
||||||
NoPermissionError,
|
NoPermissionError,
|
||||||
|
RefreshTokenAccountNotFoundError,
|
||||||
|
RefreshTokenNotFoundError,
|
||||||
RoleAlreadyAssignedError,
|
RoleAlreadyAssignedError,
|
||||||
TenantNotFoundError,
|
TenantNotFoundError,
|
||||||
)
|
)
|
||||||
@ -654,11 +656,11 @@ class AccountService:
|
|||||||
# Verify the refresh token
|
# Verify the refresh token
|
||||||
account_id = redis_client.get(AccountService._get_refresh_token_key(refresh_token))
|
account_id = redis_client.get(AccountService._get_refresh_token_key(refresh_token))
|
||||||
if not account_id:
|
if not account_id:
|
||||||
raise ValueError("Invalid refresh token")
|
raise RefreshTokenNotFoundError("Invalid refresh token")
|
||||||
|
|
||||||
account = AccountService.load_user(account_id.decode("utf-8"), session)
|
account = AccountService.load_user(account_id.decode("utf-8"), session)
|
||||||
if not account:
|
if not account:
|
||||||
raise ValueError("Invalid account")
|
raise RefreshTokenAccountNotFoundError("Invalid account")
|
||||||
|
|
||||||
# Generate new access token and refresh token
|
# Generate new access token and refresh token
|
||||||
new_access_token = AccountService.get_account_jwt_token(account)
|
new_access_token = AccountService.get_account_jwt_token(account)
|
||||||
|
|||||||
@ -17,6 +17,14 @@ class AccountPasswordError(BaseServiceError):
|
|||||||
pass
|
pass
|
||||||
|
|
||||||
|
|
||||||
|
class RefreshTokenNotFoundError(BaseServiceError):
|
||||||
|
pass
|
||||||
|
|
||||||
|
|
||||||
|
class RefreshTokenAccountNotFoundError(BaseServiceError):
|
||||||
|
pass
|
||||||
|
|
||||||
|
|
||||||
class AccountNotLinkTenantError(BaseServiceError):
|
class AccountNotLinkTenantError(BaseServiceError):
|
||||||
pass
|
pass
|
||||||
|
|
||||||
|
|||||||
@ -13,8 +13,10 @@ from unittest.mock import ANY, MagicMock, patch
|
|||||||
import pytest
|
import pytest
|
||||||
from flask import Flask
|
from flask import Flask
|
||||||
from flask_restx import Api
|
from flask_restx import Api
|
||||||
|
from werkzeug.exceptions import Unauthorized
|
||||||
|
|
||||||
from controllers.console.auth.login import RefreshTokenApi
|
from controllers.console.auth.login import RefreshTokenApi
|
||||||
|
from services.errors.account import RefreshTokenAccountNotFoundError, RefreshTokenNotFoundError
|
||||||
|
|
||||||
|
|
||||||
class TestRefreshTokenApi:
|
class TestRefreshTokenApi:
|
||||||
@ -98,18 +100,19 @@ class TestRefreshTokenApi:
|
|||||||
|
|
||||||
@patch("controllers.console.auth.login.extract_refresh_token", autospec=True)
|
@patch("controllers.console.auth.login.extract_refresh_token", autospec=True)
|
||||||
@patch("controllers.console.auth.login.AccountService.refresh_token", autospec=True)
|
@patch("controllers.console.auth.login.AccountService.refresh_token", autospec=True)
|
||||||
def test_refresh_fails_with_invalid_token(self, mock_refresh_token, mock_extract_token, app: Flask):
|
def test_refresh_returns_unauthorized_for_invalid_refresh_token(
|
||||||
|
self, mock_refresh_token, mock_extract_token, app: Flask
|
||||||
|
):
|
||||||
"""
|
"""
|
||||||
Test token refresh failure with invalid refresh token.
|
Test token refresh maps invalid refresh tokens to unauthorized responses.
|
||||||
|
|
||||||
Verifies that:
|
Verifies that:
|
||||||
- Exception is caught when token is invalid
|
- Invalid refresh token validation failures return 401
|
||||||
- 401 status code is returned
|
- The failure response preserves the validation message
|
||||||
- Error message is included in response
|
|
||||||
"""
|
"""
|
||||||
# Arrange
|
# Arrange
|
||||||
mock_extract_token.return_value = "invalid_refresh_token"
|
mock_extract_token.return_value = "invalid_refresh_token"
|
||||||
mock_refresh_token.side_effect = Exception("Invalid refresh token")
|
mock_refresh_token.side_effect = RefreshTokenNotFoundError("Invalid refresh token")
|
||||||
|
|
||||||
# Act
|
# Act
|
||||||
with app.test_request_context("/refresh-token", method="POST"):
|
with app.test_request_context("/refresh-token", method="POST"):
|
||||||
@ -119,22 +122,21 @@ class TestRefreshTokenApi:
|
|||||||
# Assert
|
# Assert
|
||||||
assert status_code == 401
|
assert status_code == 401
|
||||||
assert response["result"] == "fail"
|
assert response["result"] == "fail"
|
||||||
assert "Invalid refresh token" in response["message"]
|
assert response["message"] == "Invalid refresh token"
|
||||||
|
|
||||||
@patch("controllers.console.auth.login.extract_refresh_token", autospec=True)
|
@patch("controllers.console.auth.login.extract_refresh_token", autospec=True)
|
||||||
@patch("controllers.console.auth.login.AccountService.refresh_token", autospec=True)
|
@patch("controllers.console.auth.login.AccountService.refresh_token", autospec=True)
|
||||||
def test_refresh_fails_with_expired_token(self, mock_refresh_token, mock_extract_token, app: Flask):
|
def test_refresh_returns_unauthorized_for_invalid_account(self, mock_refresh_token, mock_extract_token, app: Flask):
|
||||||
"""
|
"""
|
||||||
Test token refresh failure with expired refresh token.
|
Test token refresh maps missing accounts to unauthorized responses.
|
||||||
|
|
||||||
Verifies that:
|
Verifies that:
|
||||||
- Expired tokens are rejected
|
- Invalid account validation failures return 401
|
||||||
- 401 status code is returned
|
- The failure response preserves the validation message
|
||||||
- Appropriate error handling
|
|
||||||
"""
|
"""
|
||||||
# Arrange
|
# Arrange
|
||||||
mock_extract_token.return_value = "expired_refresh_token"
|
mock_extract_token.return_value = "refresh_token_for_missing_account"
|
||||||
mock_refresh_token.side_effect = Exception("Refresh token expired")
|
mock_refresh_token.side_effect = RefreshTokenAccountNotFoundError("Invalid account")
|
||||||
|
|
||||||
# Act
|
# Act
|
||||||
with app.test_request_context("/refresh-token", method="POST"):
|
with app.test_request_context("/refresh-token", method="POST"):
|
||||||
@ -144,7 +146,71 @@ class TestRefreshTokenApi:
|
|||||||
# Assert
|
# Assert
|
||||||
assert status_code == 401
|
assert status_code == 401
|
||||||
assert response["result"] == "fail"
|
assert response["result"] == "fail"
|
||||||
assert "expired" in response["message"].lower()
|
assert response["message"] == "Invalid account"
|
||||||
|
|
||||||
|
@patch("controllers.console.auth.login.extract_refresh_token", autospec=True)
|
||||||
|
@patch("controllers.console.auth.login.AccountService.refresh_token", autospec=True)
|
||||||
|
def test_refresh_returns_unauthorized_for_banned_account(self, mock_refresh_token, mock_extract_token, app: Flask):
|
||||||
|
"""
|
||||||
|
Test token refresh maps banned accounts to unauthorized responses.
|
||||||
|
|
||||||
|
Verifies that:
|
||||||
|
- Authorization failures raised during account loading return 401
|
||||||
|
- The failure response preserves the authorization message
|
||||||
|
"""
|
||||||
|
# Arrange
|
||||||
|
mock_extract_token.return_value = "refresh_token_for_banned_account"
|
||||||
|
mock_refresh_token.side_effect = Unauthorized("Account is banned.")
|
||||||
|
|
||||||
|
# Act
|
||||||
|
with app.test_request_context("/refresh-token", method="POST"):
|
||||||
|
refresh_api = RefreshTokenApi()
|
||||||
|
response, status_code = refresh_api.post()
|
||||||
|
|
||||||
|
# Assert
|
||||||
|
assert status_code == 401
|
||||||
|
assert response["result"] == "fail"
|
||||||
|
assert response["message"] == "Account is banned."
|
||||||
|
|
||||||
|
@patch("controllers.console.auth.login.extract_refresh_token", autospec=True)
|
||||||
|
@patch("controllers.console.auth.login.AccountService.refresh_token", autospec=True)
|
||||||
|
def test_refresh_propagates_non_whitelisted_value_error(self, mock_refresh_token, mock_extract_token, app: Flask):
|
||||||
|
"""
|
||||||
|
Test token refresh preserves non-whitelisted ValueError failures.
|
||||||
|
|
||||||
|
Verifies that:
|
||||||
|
- Only known refresh-token validation errors are mapped to 401
|
||||||
|
- Unexpected ValueError instances continue to propagate
|
||||||
|
"""
|
||||||
|
# Arrange
|
||||||
|
mock_extract_token.return_value = "valid_refresh_token"
|
||||||
|
mock_refresh_token.side_effect = ValueError("unexpected parse failure")
|
||||||
|
|
||||||
|
# Act & Assert
|
||||||
|
with app.test_request_context("/refresh-token", method="POST"):
|
||||||
|
refresh_api = RefreshTokenApi()
|
||||||
|
with pytest.raises(ValueError, match="unexpected parse failure"):
|
||||||
|
refresh_api.post()
|
||||||
|
|
||||||
|
@patch("controllers.console.auth.login.extract_refresh_token", autospec=True)
|
||||||
|
@patch("controllers.console.auth.login.AccountService.refresh_token", autospec=True)
|
||||||
|
def test_refresh_propagates_unexpected_service_errors(self, mock_refresh_token, mock_extract_token, app: Flask):
|
||||||
|
"""
|
||||||
|
Test token refresh preserves unexpected service failures.
|
||||||
|
|
||||||
|
Verifies that:
|
||||||
|
- Operational errors are not misreported as authentication failures
|
||||||
|
- The original exception is preserved for higher-level error handling
|
||||||
|
"""
|
||||||
|
# Arrange
|
||||||
|
mock_extract_token.return_value = "valid_refresh_token"
|
||||||
|
mock_refresh_token.side_effect = RuntimeError("redis unavailable")
|
||||||
|
|
||||||
|
# Act & Assert
|
||||||
|
with app.test_request_context("/refresh-token", method="POST"):
|
||||||
|
refresh_api = RefreshTokenApi()
|
||||||
|
with pytest.raises(RuntimeError, match="redis unavailable"):
|
||||||
|
refresh_api.post()
|
||||||
|
|
||||||
@patch("controllers.console.auth.login.extract_refresh_token", autospec=True)
|
@patch("controllers.console.auth.login.extract_refresh_token", autospec=True)
|
||||||
@patch("controllers.console.auth.login.AccountService.refresh_token", autospec=True)
|
@patch("controllers.console.auth.login.AccountService.refresh_token", autospec=True)
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user