From 897ffb6b35051f08e7a4bd0965d7fa338f62665e Mon Sep 17 00:00:00 2001 From: -LAN- Date: Fri, 27 Feb 2026 18:35:49 +0800 Subject: [PATCH] fix(auth): prevent phase spoofing in change-email flow --- api/controllers/console/workspace/account.py | 19 +++--- api/services/account_service.py | 2 + .../console/test_workspace_account.py | 58 +++++++++++++++++++ 3 files changed, 71 insertions(+), 8 deletions(-) diff --git a/api/controllers/console/workspace/account.py b/api/controllers/console/workspace/account.py index 5e82a81df1..d935afebaa 100644 --- a/api/controllers/console/workspace/account.py +++ b/api/controllers/console/workspace/account.py @@ -547,7 +547,9 @@ class ChangeEmailSendEmailApi(Resource): account = None user_email = None email_for_sending = args.email.lower() + send_phase = AccountService.CHANGE_EMAIL_PHASE_OLD if args.phase is not None and args.phase == AccountService.CHANGE_EMAIL_PHASE_NEW: + send_phase = AccountService.CHANGE_EMAIL_PHASE_NEW if args.token is None: raise InvalidTokenError() @@ -555,8 +557,8 @@ class ChangeEmailSendEmailApi(Resource): if reset_data is None: raise InvalidTokenError() - token_phase = reset_data.get(AccountService.CHANGE_EMAIL_TOKEN_PHASE_KEY) - if token_phase != AccountService.CHANGE_EMAIL_PHASE_OLD_VERIFIED: + reset_token_phase = reset_data.get(AccountService.CHANGE_EMAIL_TOKEN_PHASE_KEY) + if reset_token_phase != AccountService.CHANGE_EMAIL_PHASE_OLD_VERIFIED: raise InvalidTokenError() user_email = reset_data.get("email", "") @@ -577,7 +579,7 @@ class ChangeEmailSendEmailApi(Resource): email=email_for_sending, old_email=user_email, language=language, - phase=args.phase, + phase=send_phase, ) return {"result": "success", "data": token} @@ -612,12 +614,13 @@ class ChangeEmailCheckApi(Resource): AccountService.add_change_email_error_rate_limit(user_email) raise EmailCodeError() + phase_transitions = { + AccountService.CHANGE_EMAIL_PHASE_OLD: AccountService.CHANGE_EMAIL_PHASE_OLD_VERIFIED, + AccountService.CHANGE_EMAIL_PHASE_NEW: AccountService.CHANGE_EMAIL_PHASE_NEW_VERIFIED, + } token_phase = token_data.get(AccountService.CHANGE_EMAIL_TOKEN_PHASE_KEY) - if token_phase == AccountService.CHANGE_EMAIL_PHASE_OLD: - refreshed_phase = AccountService.CHANGE_EMAIL_PHASE_OLD_VERIFIED - elif token_phase == AccountService.CHANGE_EMAIL_PHASE_NEW: - refreshed_phase = AccountService.CHANGE_EMAIL_PHASE_NEW_VERIFIED - else: + refreshed_phase = phase_transitions.get(token_phase) + if refreshed_phase is None: raise InvalidTokenError() # Verified, revoke the first token diff --git a/api/services/account_service.py b/api/services/account_service.py index bf4dfa4b7f..b0f4978f37 100644 --- a/api/services/account_service.py +++ b/api/services/account_service.py @@ -547,6 +547,8 @@ class AccountService: raise ValueError("Email must be provided.") if not phase: raise ValueError("phase must be provided.") + if phase not in (cls.CHANGE_EMAIL_PHASE_OLD, cls.CHANGE_EMAIL_PHASE_NEW): + raise ValueError("phase must be one of old_email or new_email.") if cls.change_email_rate_limiter.is_rate_limited(account_email): from controllers.console.auth.error import EmailChangeRateLimitExceededError diff --git a/api/tests/unit_tests/controllers/console/test_workspace_account.py b/api/tests/unit_tests/controllers/console/test_workspace_account.py index 702b361dec..f4aa0404a9 100644 --- a/api/tests/unit_tests/controllers/console/test_workspace_account.py +++ b/api/tests/unit_tests/controllers/console/test_workspace_account.py @@ -95,6 +95,64 @@ class TestChangeEmailSend: mock_is_ip_limit.assert_called_once_with("127.0.0.1") mock_csrf.assert_called_once() + @patch("controllers.console.wraps.db") + @patch("controllers.console.workspace.account.current_account_with_tenant") + @patch("controllers.console.workspace.account.db") + @patch("controllers.console.workspace.account.Session") + @patch("controllers.console.workspace.account.AccountService.get_account_by_email_with_case_fallback") + @patch("controllers.console.workspace.account.AccountService.send_change_email_email") + @patch("controllers.console.workspace.account.AccountService.is_email_send_ip_limit", return_value=False) + @patch("controllers.console.workspace.account.extract_remote_ip", return_value="127.0.0.1") + @patch("libs.login.check_csrf_token", return_value=None) + @patch("controllers.console.wraps.FeatureService.get_system_features") + def test_should_force_old_email_phase_for_legacy_or_unexpected_phase_input( + self, + mock_features, + mock_csrf, + mock_extract_ip, + mock_is_ip_limit, + mock_send_email, + mock_get_account_by_email, + mock_session_cls, + mock_account_db, + mock_current_account, + mock_db, + app, + ): + _mock_wraps_db(mock_db) + mock_features.return_value = SimpleNamespace(enable_change_email=True) + mock_current_account.return_value = (_build_account("current@example.com", "current"), None) + existing_account = _build_account("old@example.com", "acc-old") + mock_get_account_by_email.return_value = existing_account + mock_send_email.return_value = "token-legacy" + mock_session = MagicMock() + mock_session_cm = MagicMock() + mock_session_cm.__enter__.return_value = mock_session + mock_session_cm.__exit__.return_value = None + mock_session_cls.return_value = mock_session_cm + mock_account_db.engine = MagicMock() + + with app.test_request_context( + "/account/change-email", + method="POST", + json={"email": "old@example.com", "language": "en-US", "phase": "old_email_verified"}, + ): + _set_logged_in_user(_build_account("tester@example.com", "tester")) + response = ChangeEmailSendEmailApi().post() + + assert response == {"result": "success", "data": "token-legacy"} + mock_get_account_by_email.assert_called_once_with("old@example.com", session=mock_session) + mock_send_email.assert_called_once_with( + account=existing_account, + email="old@example.com", + old_email="old@example.com", + language="en-US", + phase=AccountService.CHANGE_EMAIL_PHASE_OLD, + ) + mock_extract_ip.assert_called_once() + mock_is_ip_limit.assert_called_once_with("127.0.0.1") + mock_csrf.assert_called_once() + @patch("controllers.console.wraps.db") @patch("controllers.console.workspace.account.current_account_with_tenant") @patch("controllers.console.workspace.account.AccountService.get_change_email_data")