From 65c751710d1d216e8b305d94015c19fdea99625e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9E=97=E7=8E=AE=20=28Jade=20Lin=29?= Date: Tue, 10 Nov 2026 17:56:12 +0800 Subject: [PATCH] refactor(api): share Account email-change transport handlers --- api/.importlinter | 13 ++ api/README.md | 3 + api/controllers/account_email.py | 119 ++++++++++ api/controllers/account_email_errors.py | 65 ++++++ api/controllers/common/account.py | 2 +- api/controllers/console/auth/error.py | 48 +--- api/controllers/console/error.py | 26 +-- api/controllers/console/workspace/account.py | 97 +++----- api/controllers/web/account_email.py | 110 +++------ .../controllers/test_account_email.py | 212 ++++++++++++++++++ .../controllers/test_account_email_errors.py | 45 ++++ 11 files changed, 536 insertions(+), 204 deletions(-) create mode 100644 api/controllers/account_email.py create mode 100644 api/controllers/account_email_errors.py create mode 100644 api/tests/unit_tests/controllers/test_account_email.py create mode 100644 api/tests/unit_tests/controllers/test_account_email_errors.py diff --git a/api/.importlinter b/api/.importlinter index 6415e879664..5117892170d 100644 --- a/api/.importlinter +++ b/api/.importlinter @@ -212,6 +212,8 @@ allow_indirect_imports = True name = Account consumer transport uses application services type = forbidden source_modules = + controllers.account_email + controllers.account_email_errors controllers.web.me controllers.web.account controllers.web.account_email @@ -222,6 +224,17 @@ forbidden_modules = sqlalchemy allow_indirect_imports = True +[importlinter:contract:account-email-surface-boundary] +name = Shared Account email transport is independent of Console and WebApp surfaces +type = forbidden +source_modules = + controllers.account_email + controllers.account_email_errors +forbidden_modules = + controllers.console + controllers.web +allow_indirect_imports = True + [importlinter:contract:machinery-framework-boundary] name = API machinery is framework neutral type = forbidden diff --git a/api/README.md b/api/README.md index 21492918a16..67504fceb24 100644 --- a/api/README.md +++ b/api/README.md @@ -103,6 +103,9 @@ Verify the current email before requesting a `new_email` challenge, verify that challenge, then submit its verified token to `/reset`. Tokens are Account-bound; the reset response contains only personal settings. +Console and WebApp share email-change transport handlers and HTTP errors; each +surface owns its admission, Account serialization, and missing-Account response. + ### EndUser execution authorization Service API requests that resolve an EndUser are forbidden when the app's current diff --git a/api/controllers/account_email.py b/api/controllers/account_email.py new file mode 100644 index 00000000000..493ee2770da --- /dev/null +++ b/api/controllers/account_email.py @@ -0,0 +1,119 @@ +"""Shared email-change transport behavior after surface-specific Account admission. + +Payload validation, service arguments, shared HTTP errors and verification responses +have one owner. Account-not-found errors remain unmodified so each surface can retain +its authentication contract; Account serialization also belongs to the surface. +""" + +from collections.abc import Generator +from contextlib import contextmanager + +from controllers.account_email_errors import ( + AccountInFreezeError, + EmailAlreadyInUseError, + EmailChangeLimitError, + EmailChangeRateLimitExceededError, + EmailCodeError, + EmailDomainSuspendedError, + EmailSendIpLimitError, + InvalidEmailError, + InvalidTokenError, +) +from controllers.common.account import ( + ChangeEmailResetPayload, + ChangeEmailSendPayload, + ChangeEmailValidityPayload, + CheckEmailUniquePayload, +) +from controllers.common.fields import SimpleResultDataResponse, SimpleResultResponse, VerificationTokenResponse +from machinery.context import AccountRequestContext, RequestContext +from services import account_errors +from services.account_change_email_service import AccountChangeEmailService +from services.entities.account_entities import AccountSnapshot + + +@contextmanager +def _change_email_errors() -> Generator[None]: + """Map shared application failures to HTTP errors; preserve unknown and missing-Account failures.""" + try: + yield + except account_errors.ChangeEmailSendIPLimitedError as error: + raise EmailSendIpLimitError() from error + except account_errors.ChangeEmailSendRateLimitError as error: + raise EmailChangeRateLimitExceededError(error.retry_after_minutes) from error + except account_errors.ChangeEmailVerificationLimitError as error: + raise EmailChangeLimitError() from error + except account_errors.InvalidChangeEmailTokenError as error: + raise InvalidTokenError() from error + except account_errors.InvalidChangeEmailAddressError as error: + raise InvalidEmailError() from error + except account_errors.InvalidChangeEmailCodeError as error: + raise EmailCodeError() from error + except account_errors.AccountEmailDomainSuspendedError as error: + raise EmailDomainSuspendedError() from error + except account_errors.AccountEmailFrozenError as error: + raise AccountInFreezeError() from error + except account_errors.AccountEmailAlreadyInUseError as error: + raise EmailAlreadyInUseError() from error + + +def send_change_email_code( + service: AccountChangeEmailService, + context: AccountRequestContext | RequestContext, + payload: object, + *, + ip_address: str, +) -> SimpleResultDataResponse: + """Validate an admitted caller's payload and send its challenge without changing the Account. + + Raw bodies and validated payloads share language normalization and error mapping. + Invalid input never reaches the service; missing Accounts remain surface-specific. + """ + args = ChangeEmailSendPayload.model_validate(payload) + with _change_email_errors(): + token = service.send_code( + context, + requested_email=args.email, + language="zh-Hans" if args.language == "zh-Hans" else "en-US", + phase=args.phase, + predecessor_token=args.token, + ip_address=ip_address, + ) + return SimpleResultDataResponse(result="success", data=token) + + +def verify_change_email_code( + service: AccountChangeEmailService, context: AccountRequestContext | RequestContext, payload: object +) -> VerificationTokenResponse: + """Validate an admitted caller's challenge and return its promoted token without changing identity. + + Invalid payloads never reach the service; verification failures retain the shared HTTP contract. + """ + args = ChangeEmailValidityPayload.model_validate(payload) + with _change_email_errors(): + verified = service.verify_code(context, email=args.email, code=args.code, token=args.token) + return VerificationTokenResponse(is_valid=True, email=verified.email, token=verified.token) + + +def reset_change_email( + service: AccountChangeEmailService, context: AccountRequestContext | RequestContext, payload: object +) -> AccountSnapshot: + """Validate an admitted caller's reset and return its updated Account for surface-specific serialization. + + Shared failures become HTTP errors; missing Accounts propagate unchanged. Only the + application service may update identity after successful verification and policy checks. + """ + args = ChangeEmailResetPayload.model_validate(payload) + with _change_email_errors(): + return service.reset(context, new_email=args.new_email, token=args.token) + + +def check_change_email_available(service: AccountChangeEmailService, payload: object) -> SimpleResultResponse: + """Validate an admitted caller's address and check availability without account or token mutations. + + Invalid input never reaches the service; policy failures use the shared HTTP contract. + """ + args = CheckEmailUniquePayload.model_validate(payload) + with _change_email_errors(): + service.ensure_available(args.email) + return SimpleResultResponse(result="success") diff --git a/api/controllers/account_email_errors.py b/api/controllers/account_email_errors.py new file mode 100644 index 00000000000..db6e2a3d36c --- /dev/null +++ b/api/controllers/account_email_errors.py @@ -0,0 +1,65 @@ +"""Shared Account email HTTP errors, independent of surface initialization and handlers.""" + +from libs.exception import BaseHTTPException + + +class InvalidEmailError(BaseHTTPException): + error_code = "invalid_email" + description = "The email address is not valid." + code = 400 + + +class InvalidTokenError(BaseHTTPException): + error_code = "invalid_or_expired_token" + description = "The token is invalid or has expired." + code = 400 + + +class EmailChangeRateLimitExceededError(BaseHTTPException): + error_code = "email_change_rate_limit_exceeded" + description = "Too many email change emails have been sent. Please try again in {minutes} minutes." + code = 429 + + def __init__(self, minutes: int = 1): + """Render the policy's retry interval while preserving the legacy one-minute default.""" + description = self.description.format(minutes=minutes) if self.description else None + super().__init__(description=description) + + +class EmailCodeError(BaseHTTPException): + error_code = "email_code_error" + description = "Email code is invalid or expired." + code = 400 + + +class EmailChangeLimitError(BaseHTTPException): + error_code = "email_change_limit" + description = "Too many failed email change attempts. Please try again in 24 hours." + code = 429 + + +class EmailAlreadyInUseError(BaseHTTPException): + error_code = "email_already_in_use" + description = "A user with this email already exists." + code = 400 + + +class EmailSendIpLimitError(BaseHTTPException): + error_code = "email_send_ip_limit" + description = "Too many emails have been sent from this IP address recently. Please try again later." + code = 429 + + +class AccountInFreezeError(BaseHTTPException): + error_code = "account_in_freeze" + code = 400 + description = ( + "This email account has been deleted within the past 30 days " + "and is temporarily unavailable for new account registration." + ) + + +class EmailDomainSuspendedError(BaseHTTPException): + error_code = "email_domain_suspended" + code = 400 + description = "This email domain has been suspended." diff --git a/api/controllers/common/account.py b/api/controllers/common/account.py index 2153b57d771..0cd2e3c16ab 100644 --- a/api/controllers/common/account.py +++ b/api/controllers/common/account.py @@ -1,4 +1,4 @@ -"""Shared transport validation for Account profile changes.""" +"""Shared transport validation for Account settings.""" from typing import Annotated, Literal diff --git a/api/controllers/console/auth/error.py b/api/controllers/console/auth/error.py index af34860d5ec..c71d1c372c8 100644 --- a/api/controllers/console/auth/error.py +++ b/api/controllers/console/auth/error.py @@ -1,5 +1,13 @@ +from controllers import account_email_errors from libs.exception import BaseHTTPException +EmailAlreadyInUseError = account_email_errors.EmailAlreadyInUseError +EmailChangeLimitError = account_email_errors.EmailChangeLimitError +EmailChangeRateLimitExceededError = account_email_errors.EmailChangeRateLimitExceededError +EmailCodeError = account_email_errors.EmailCodeError +InvalidEmailError = account_email_errors.InvalidEmailError +InvalidTokenError = account_email_errors.InvalidTokenError + class DataSourceApiKeyAuthProviderNotSupportedError(BaseHTTPException): error_code = "unsupported_data_source_api_key_auth_provider" @@ -25,12 +33,6 @@ class DataSourceApiKeyAuthProviderUnavailableRequestError(BaseHTTPException): code = 502 -class InvalidEmailError(BaseHTTPException): - error_code = "invalid_email" - description = "The email address is not valid." - code = 400 - - class InvitationAccountMismatchError(BaseHTTPException): error_code = "invitation_account_mismatch" description = "This invitation was sent to another account. Please sign in with the invited account." @@ -43,12 +45,6 @@ class PasswordMismatchError(BaseHTTPException): code = 400 -class InvalidTokenError(BaseHTTPException): - error_code = "invalid_or_expired_token" - description = "The token is invalid or has expired." - code = 400 - - class PasswordResetRateLimitExceededError(BaseHTTPException): error_code = "password_reset_rate_limit_exceeded" description = "Too many password reset emails have been sent. Please try again in {minutes} minutes." @@ -69,16 +65,6 @@ class EmailRegisterRateLimitExceededError(BaseHTTPException): super().__init__(description=description) -class EmailChangeRateLimitExceededError(BaseHTTPException): - error_code = "email_change_rate_limit_exceeded" - description = "Too many email change emails have been sent. Please try again in {minutes} minutes." - code = 429 - - def __init__(self, minutes: int = 1): - description = self.description.format(minutes=minutes) if self.description else None - super().__init__(description=description) - - class OwnerTransferRateLimitExceededError(BaseHTTPException): error_code = "owner_transfer_rate_limit_exceeded" description = "Too many owner transfer emails have been sent. Please try again in {minutes} minutes." @@ -89,12 +75,6 @@ class OwnerTransferRateLimitExceededError(BaseHTTPException): super().__init__(description=description) -class EmailCodeError(BaseHTTPException): - error_code = "email_code_error" - description = "Email code is invalid or expired." - code = 400 - - class EmailCodeLoginServiceUnavailableError(BaseHTTPException): error_code = "email_code_login_service_unavailable" description = "Email code verification is temporarily unavailable. Please try again later." @@ -163,18 +143,6 @@ class EmailRegisterLimitError(BaseHTTPException): code = 429 -class EmailChangeLimitError(BaseHTTPException): - error_code = "email_change_limit" - description = "Too many failed email change attempts. Please try again in 24 hours." - code = 429 - - -class EmailAlreadyInUseError(BaseHTTPException): - error_code = "email_already_in_use" - description = "A user with this email already exists." - code = 400 - - class NormalizedEmailAlreadyInUseError(EmailAlreadyInUseError): error_code = "normalized_email_already_in_use" description = "An account with an equivalent email address already exists." diff --git a/api/controllers/console/error.py b/api/controllers/console/error.py index 68beb0abc1a..fd21dd61446 100644 --- a/api/controllers/console/error.py +++ b/api/controllers/console/error.py @@ -1,5 +1,10 @@ +from controllers import account_email_errors from libs.exception import BaseHTTPException +AccountInFreezeError = account_email_errors.AccountInFreezeError +EmailDomainSuspendedError = account_email_errors.EmailDomainSuspendedError +EmailSendIpLimitError = account_email_errors.EmailSendIpLimitError + class PaidPlanRequiredError(BaseHTTPException): error_code = "paid_plan_required" @@ -88,33 +93,12 @@ class InvalidAccountPasswordRequestError(BaseHTTPException): code = 400 -class EmailSendIpLimitError(BaseHTTPException): - error_code = "email_send_ip_limit" - description = "Too many emails have been sent from this IP address recently. Please try again later." - code = 429 - - class UnauthorizedAndForceLogout(BaseHTTPException): error_code = "unauthorized_and_force_logout" description = "Unauthorized and force logout." code = 401 -class AccountInFreezeError(BaseHTTPException): - error_code = "account_in_freeze" - code = 400 - description = ( - "This email account has been deleted within the past 30 days " - "and is temporarily unavailable for new account registration." - ) - - -class EmailDomainSuspendedError(BaseHTTPException): - error_code = "email_domain_suspended" - code = 400 - description = "This email domain has been suspended." - - class EducationVerifyLimitError(BaseHTTPException): error_code = "education_verify_limit" description = "Rate limit exceeded" diff --git a/api/controllers/console/workspace/account.py b/api/controllers/console/workspace/account.py index e0dfd6c98c4..fd0bf64752f 100644 --- a/api/controllers/console/workspace/account.py +++ b/api/controllers/console/workspace/account.py @@ -10,6 +10,23 @@ from pydantic import BaseModel, Field, RootModel, field_validator, model_validat from werkzeug.exceptions import NotFound from constants.languages import supported_language +from controllers.account_email import ( + check_change_email_available, + reset_change_email, + send_change_email_code, + verify_change_email_code, +) +from controllers.account_email_errors import ( + AccountInFreezeError, + EmailAlreadyInUseError, + EmailChangeLimitError, + EmailChangeRateLimitExceededError, + EmailCodeError, + EmailDomainSuspendedError, + EmailSendIpLimitError, + InvalidEmailError, + InvalidTokenError, +) from controllers.common.account import ( AccountProfilePatchPayload, ChangeEmailResetPayload, @@ -25,15 +42,7 @@ from controllers.common.fields import ( ) from controllers.common.schema import query_params_from_model, register_response_schema_models, register_schema_models from controllers.console import console_ns -from controllers.console.auth.error import ( - EmailAlreadyInUseError, - EmailChangeLimitError, - EmailChangeRateLimitExceededError, - EmailCodeAccountDeletionRateLimitExceededError, - EmailCodeError, - InvalidEmailError, - InvalidTokenError, -) +from controllers.console.auth.error import EmailCodeAccountDeletionRateLimitExceededError from controllers.console.billing.error import ( BillingOperationFailedError, BillingRequestInProgressError, @@ -41,14 +50,11 @@ from controllers.console.billing.error import ( to_billing_request_error, ) from controllers.console.error import ( - AccountInFreezeError, AccountNotFound, EducationActivateFailedError, EducationActivateLimitError, EducationVerifyFailedError, EducationVerifyLimitError, - EmailDomainSuspendedError, - EmailSendIpLimitError, InvalidAccountPasswordRequestError, ) from controllers.console.flask_admission import console_account_admission, console_account_identity_admission @@ -513,28 +519,16 @@ class ChangeEmailSendEmailApi(Resource): @console_account_admission(require_change_email_enabled=True) @model_validate(ChangeEmailSendPayload) def post(self, args: ChangeEmailSendPayload, request_context: RequestContext): - ip_address = extract_remote_ip(request) - language = "zh-Hans" if args.language == "zh-Hans" else "en-US" try: - token = application_services().accounts.change_email.send_code( + result = send_change_email_code( + application_services().accounts.change_email, request_context, - requested_email=args.email, - language=language, - phase=args.phase, - predecessor_token=args.token, - ip_address=ip_address, + args, + ip_address=extract_remote_ip(request), ) - except account_errors.ChangeEmailSendIPLimitedError: - raise EmailSendIpLimitError() from None - except account_errors.ChangeEmailSendRateLimitError as error: - raise EmailChangeRateLimitExceededError(error.retry_after_minutes) from None - except account_errors.InvalidChangeEmailTokenError: - raise InvalidTokenError() from None - except account_errors.InvalidChangeEmailAddressError: - raise InvalidEmailError() from None except account_errors.AccountNotFoundError: raise AccountNotFound() from None - return SimpleResultDataResponse(result="success", data=token).model_dump(mode="json") + return result.model_dump(mode="json") @console_ns.route("/account/change-email/validity") @@ -545,24 +539,8 @@ class ChangeEmailCheckApi(Resource): @console_account_admission(require_change_email_enabled=True) @model_validate(ChangeEmailValidityPayload) def post(self, args: ChangeEmailValidityPayload, request_context: RequestContext): - try: - verification = application_services().accounts.change_email.verify_code( - request_context, - email=args.email, - code=args.code, - token=args.token, - ) - except account_errors.ChangeEmailVerificationLimitError: - raise EmailChangeLimitError() from None - except account_errors.InvalidChangeEmailTokenError: - raise InvalidTokenError() from None - except account_errors.InvalidChangeEmailAddressError: - raise InvalidEmailError() from None - except account_errors.InvalidChangeEmailCodeError: - raise EmailCodeError() from None - return VerificationTokenResponse(is_valid=True, email=verification.email, token=verification.token).model_dump( - mode="json" - ) + result = verify_change_email_code(application_services().accounts.change_email, request_context, args) + return result.model_dump(mode="json") @console_ns.route("/account/change-email/reset") @@ -576,19 +554,7 @@ class ChangeEmailResetApi(Resource): @model_validate(ChangeEmailResetPayload) def post(self, args: ChangeEmailResetPayload, request_context: RequestContext): try: - updated_account = application_services().accounts.change_email.reset( - request_context, - new_email=args.new_email, - token=args.token, - ) - except account_errors.AccountEmailDomainSuspendedError: - raise EmailDomainSuspendedError() from None - except account_errors.AccountEmailFrozenError: - raise AccountInFreezeError() from None - except account_errors.AccountEmailAlreadyInUseError: - raise EmailAlreadyInUseError() from None - except account_errors.InvalidChangeEmailTokenError: - raise InvalidTokenError() from None + updated_account = reset_change_email(application_services().accounts.change_email, request_context, args) except account_errors.AccountNotFoundError: raise AccountNotFound() from None @@ -603,12 +569,5 @@ class CheckEmailUnique(Resource): @console_account_admission() @model_validate(CheckEmailUniquePayload) def post(self, args: CheckEmailUniquePayload, request_context: RequestContext): - try: - application_services().accounts.change_email.ensure_available(args.email) - except account_errors.AccountEmailDomainSuspendedError: - raise EmailDomainSuspendedError() from None - except account_errors.AccountEmailFrozenError: - raise AccountInFreezeError() from None - except account_errors.AccountEmailAlreadyInUseError: - raise EmailAlreadyInUseError() from None - return SimpleResultResponse(result="success").model_dump(mode="json") + result = check_change_email_available(application_services().accounts.change_email, args) + return result.model_dump(mode="json") diff --git a/api/controllers/web/account_email.py b/api/controllers/web/account_email.py index 3f50770478b..0b29d47830f 100644 --- a/api/controllers/web/account_email.py +++ b/api/controllers/web/account_email.py @@ -2,10 +2,27 @@ from http import HTTPStatus -from flask import make_response, request +from flask import Response, make_response, request from flask_restx import Resource from werkzeug.exceptions import Forbidden, Unauthorized +from controllers.account_email import ( + check_change_email_available, + reset_change_email, + send_change_email_code, + verify_change_email_code, +) +from controllers.account_email_errors import ( + AccountInFreezeError, + EmailAlreadyInUseError, + EmailChangeLimitError, + EmailChangeRateLimitExceededError, + EmailCodeError, + EmailDomainSuspendedError, + EmailSendIpLimitError, + InvalidEmailError, + InvalidTokenError, +) from controllers.common.account import ( ChangeEmailResetPayload, ChangeEmailSendPayload, @@ -14,21 +31,13 @@ from controllers.common.account import ( ) from controllers.common.fields import SimpleResultDataResponse, SimpleResultResponse, VerificationTokenResponse from controllers.common.schema import register_response_schema_models, register_schema_models -from controllers.console.auth.error import ( - EmailAlreadyInUseError, - EmailChangeLimitError, - EmailChangeRateLimitExceededError, - EmailCodeError, - InvalidEmailError, - InvalidTokenError, -) -from controllers.console.error import AccountInFreezeError, EmailDomainSuspendedError, EmailSendIpLimitError from controllers.web import web_ns from controllers.web.account import WebAccountResponse from controllers.web.account_admission import web_account_context from extensions.ext_application_services import application_services +from fields.base import ResponseModel from libs.external_api import error_responses -from libs.helper import dump_response, extract_remote_ip +from libs.helper import extract_remote_ip from machinery.context import AccountRequestContext from services import account_errors from services.system_feature_service import SystemFeatureService @@ -47,6 +56,13 @@ def _change_email_context() -> AccountRequestContext: return context +def _private_response(payload: ResponseModel) -> Response: + """Serialize admitted Account data without shared caching; schemas stay surface-specific.""" + response = make_response(payload.model_dump(mode="json")) + response.headers["Cache-Control"] = "no-store" + return response + + @web_ns.route("/account/change-email") class WebAccountChangeEmailSendApi(Resource): @web_ns.expect(web_ns.models[ChangeEmailSendPayload.__name__]) @@ -55,29 +71,16 @@ class WebAccountChangeEmailSendApi(Resource): def post(self): """Send an old/new-email challenge for the cookie Account; no profile changes occur.""" context = _change_email_context() - payload = ChangeEmailSendPayload.model_validate(web_ns.payload or {}) try: - token = application_services().accounts.change_email.send_code( + result = send_change_email_code( + application_services().accounts.change_email, context, - requested_email=payload.email, - language="zh-Hans" if payload.language == "zh-Hans" else "en-US", - phase=payload.phase, - predecessor_token=payload.token, + web_ns.payload or {}, ip_address=extract_remote_ip(request), ) - except account_errors.ChangeEmailSendIPLimitedError as error: - raise EmailSendIpLimitError() from error - except account_errors.ChangeEmailSendRateLimitError as error: - raise EmailChangeRateLimitExceededError(error.retry_after_minutes) from error - except account_errors.InvalidChangeEmailTokenError as error: - raise InvalidTokenError() from error - except account_errors.InvalidChangeEmailAddressError as error: - raise InvalidEmailError() from error except account_errors.AccountNotFoundError as error: raise Unauthorized("Account login required.") from error - response = make_response(SimpleResultDataResponse(result="success", data=token).model_dump(mode="json")) - response.headers["Cache-Control"] = "no-store" - return response + return _private_response(result) @web_ns.route("/account/change-email/validity") @@ -88,24 +91,8 @@ class WebAccountChangeEmailValidityApi(Resource): def post(self): """Verify this Account's challenge and promote its token without changing identity.""" context = _change_email_context() - payload = ChangeEmailValidityPayload.model_validate(web_ns.payload or {}) - try: - verified = application_services().accounts.change_email.verify_code( - context, email=payload.email, code=payload.code, token=payload.token - ) - except account_errors.ChangeEmailVerificationLimitError as error: - raise EmailChangeLimitError() from error - except account_errors.InvalidChangeEmailTokenError as error: - raise InvalidTokenError() from error - except account_errors.InvalidChangeEmailAddressError as error: - raise InvalidEmailError() from error - except account_errors.InvalidChangeEmailCodeError as error: - raise EmailCodeError() from error - response = make_response( - VerificationTokenResponse(is_valid=True, email=verified.email, token=verified.token).model_dump(mode="json") - ) - response.headers["Cache-Control"] = "no-store" - return response + result = verify_change_email_code(application_services().accounts.change_email, context, web_ns.payload or {}) + return _private_response(result) @web_ns.route("/account/change-email/reset") @@ -116,24 +103,11 @@ class WebAccountChangeEmailResetApi(Resource): def post(self): """Change only the admitted Account's verified address; failures preserve identity and memberships.""" context = _change_email_context() - payload = ChangeEmailResetPayload.model_validate(web_ns.payload or {}) try: - updated = application_services().accounts.change_email.reset( - context, new_email=payload.new_email, token=payload.token - ) - except account_errors.AccountEmailDomainSuspendedError as error: - raise EmailDomainSuspendedError() from error - except account_errors.AccountEmailFrozenError as error: - raise AccountInFreezeError() from error - except account_errors.AccountEmailAlreadyInUseError as error: - raise EmailAlreadyInUseError() from error - except account_errors.InvalidChangeEmailTokenError as error: - raise InvalidTokenError() from error + updated = reset_change_email(application_services().accounts.change_email, context, web_ns.payload or {}) except account_errors.AccountNotFoundError as error: raise Unauthorized("Account login required.") from error - response = make_response(dump_response(WebAccountResponse, updated)) - response.headers["Cache-Control"] = "no-store" - return response + return _private_response(WebAccountResponse.model_validate(updated, from_attributes=True)) @web_ns.route("/account/change-email/check-email-unique") @@ -144,15 +118,5 @@ class WebAccountCheckEmailUniqueApi(Resource): def post(self): """Check email availability for an admitted Account; do not mutate accounts or tokens.""" _change_email_context() - payload = CheckEmailUniquePayload.model_validate(web_ns.payload or {}) - try: - application_services().accounts.change_email.ensure_available(payload.email) - except account_errors.AccountEmailDomainSuspendedError as error: - raise EmailDomainSuspendedError() from error - except account_errors.AccountEmailFrozenError as error: - raise AccountInFreezeError() from error - except account_errors.AccountEmailAlreadyInUseError as error: - raise EmailAlreadyInUseError() from error - response = make_response(SimpleResultResponse(result="success").model_dump(mode="json")) - response.headers["Cache-Control"] = "no-store" - return response + result = check_change_email_available(application_services().accounts.change_email, web_ns.payload or {}) + return _private_response(result) diff --git a/api/tests/unit_tests/controllers/test_account_email.py b/api/tests/unit_tests/controllers/test_account_email.py new file mode 100644 index 00000000000..22ab948a332 --- /dev/null +++ b/api/tests/unit_tests/controllers/test_account_email.py @@ -0,0 +1,212 @@ +"""Exercise shared email-change transport with real application behavior and persistence.""" + +import pytest +from pydantic import ValidationError +from sqlalchemy.orm import Session, sessionmaker + +from controllers.account_email import ( + check_change_email_available, + reset_change_email, + send_change_email_code, + verify_change_email_code, +) +from controllers.account_email_errors import ( + AccountInFreezeError, + EmailAlreadyInUseError, + EmailChangeLimitError, + EmailChangeRateLimitExceededError, + EmailCodeError, + EmailDomainSuspendedError, + EmailSendIpLimitError, + InvalidEmailError, + InvalidTokenError, +) +from controllers.common.account import ChangeEmailSendPayload +from machinery.context import AccountRequestContext, RequestContext +from models.account import Account +from repositories.account.repository import SQLAlchemyAccountRepository +from services.account.adapters import SecureChangeEmailCodeGenerator +from services.account_change_email_service import AccountChangeEmailService +from services.account_errors import AccountNotFoundError +from services.entities.account_entities import AccountChangeEmailNewEmailVerifiedToken, AccountChangeEmailOldEmailToken +from tests.helpers.unit.account_email import AccountEmailTransports +from tests.helpers.unit.model_factories import make_account + + +@pytest.fixture(params=["account", "workspace"]) +def context(request: pytest.FixtureRequest) -> AccountRequestContext | RequestContext: + if request.param == "account": + return AccountRequestContext(request_id="request", trace_id=None, account_id="account-1") + return RequestContext( + request_id="request", trace_id=None, account_id="account-1", active_workspace_id="workspace-1" + ) + + +@pytest.fixture +def transports() -> AccountEmailTransports: + return AccountEmailTransports() + + +@pytest.fixture +def service( + sqlite_session_factory: sessionmaker[Session], transports: AccountEmailTransports +) -> AccountChangeEmailService: + with sqlite_session_factory.begin() as session: + session.add(make_account(account_id="account-1")) + session.add(make_account(account_id="account-2", email="other@example.com")) + return AccountChangeEmailService( + accounts=SQLAlchemyAccountRepository(sqlite_session_factory), + tokens=transports, + codes=SecureChangeEmailCodeGenerator(), + notifications=transports, + send_limits=transports, + security=transports, + email_policy=transports, + ) + + +@pytest.mark.parametrize("language", ["zh-Hans", "unsupported"]) +def test_shared_flow_accepts_raw_and_validated_payloads( + service: AccountChangeEmailService, + context: AccountRequestContext | RequestContext, + transports: AccountEmailTransports, + language: str, + sqlite_session_factory: sessionmaker[Session], +) -> None: + challenge = send_change_email_code( + service, + context, + ChangeEmailSendPayload(email="test@example.com", language=language), + ip_address="127.0.0.1", + ) + assert transports.sent[-1][2] == ("zh-Hans" if language == "zh-Hans" else "en-US") + verified_old = verify_change_email_code( + service, context, {"email": "test@example.com", "code": transports.sent[-1][1], "token": challenge.data} + ) + assert check_change_email_available(service, {"email": "new@example.com"}).model_dump() == {"result": "success"} + challenge = send_change_email_code( + service, + context, + {"email": "new@example.com", "phase": "new_email", "token": verified_old.token}, + ip_address="127.0.0.1", + ) + verified_new = verify_change_email_code( + service, context, {"email": "new@example.com", "code": transports.sent[-1][1], "token": challenge.data} + ) + assert verified_new.is_valid is True + updated = reset_change_email(service, context, {"new_email": "new@example.com", "token": verified_new.token}) + assert updated.email == "new@example.com" + assert transports.completed == ["new@example.com"] + assert verified_new.token not in transports.tokens + with sqlite_session_factory() as session: + assert session.get_one(Account, "account-1").email == "new@example.com" + assert session.get_one(Account, "account-2").email == "other@example.com" + + +def test_shared_payload_validation_precedes_service_effects( + service: AccountChangeEmailService, + context: AccountRequestContext | RequestContext, + transports: AccountEmailTransports, +) -> None: + with pytest.raises(ValidationError): + send_change_email_code(service, context, {}, ip_address="127.0.0.1") + with pytest.raises(ValidationError): + verify_change_email_code(service, context, {}) + with pytest.raises(ValidationError): + reset_change_email(service, context, {}) + with pytest.raises(ValidationError): + check_change_email_available(service, {}) + assert transports.sent == [] + assert transports.tokens == {} + assert transports.completed == [] + + +def test_shared_send_error_mapping( + service: AccountChangeEmailService, + context: AccountRequestContext | RequestContext, + transports: AccountEmailTransports, +) -> None: + with pytest.raises(InvalidEmailError): + send_change_email_code(service, context, {"email": "other@example.com"}, ip_address="127.0.0.1") + with pytest.raises(InvalidTokenError): + send_change_email_code( + service, context, {"email": "new@example.com", "phase": "new_email"}, ip_address="127.0.0.1" + ) + transports.sends["test@example.com"] = 3 + with pytest.raises(EmailChangeRateLimitExceededError): + send_change_email_code(service, context, {"email": "test@example.com"}, ip_address="127.0.0.1") + transports.ip_limited = True + with pytest.raises(EmailSendIpLimitError): + send_change_email_code(service, context, {"email": "test@example.com"}, ip_address="127.0.0.1") + assert transports.sent == [] + + +def test_shared_verification_error_mapping( + service: AccountChangeEmailService, + context: AccountRequestContext | RequestContext, + transports: AccountEmailTransports, +) -> None: + token = transports.issue( + AccountChangeEmailOldEmailToken( + account_id=context.account_id, email="test@example.com", old_email="test@example.com", code="123456" + ) + ) + with pytest.raises(InvalidEmailError): + verify_change_email_code(service, context, {"email": "other@example.com", "code": "123456", "token": token}) + with pytest.raises(EmailCodeError): + verify_change_email_code(service, context, {"email": "test@example.com", "code": "wrong", "token": token}) + transports.failures["test@example.com"] = 5 + with pytest.raises(EmailChangeLimitError): + verify_change_email_code(service, context, {"email": "test@example.com", "code": "123456", "token": token}) + assert token in transports.tokens + + +@pytest.mark.parametrize( + ("freeze_type", "http_error"), + [ + ("frozen", AccountInFreezeError), + ("email_domain_suspended", EmailDomainSuspendedError), + (None, EmailAlreadyInUseError), + ], +) +def test_shared_reset_and_availability_policy_errors( + service: AccountChangeEmailService, + context: AccountRequestContext | RequestContext, + transports: AccountEmailTransports, + freeze_type: str | None, + http_error: type[Exception], +) -> None: + if freeze_type is not None: + transports.frozen["other@example.com"] = freeze_type + token = transports.issue( + AccountChangeEmailNewEmailVerifiedToken( + account_id=context.account_id, email="other@example.com", old_email="test@example.com", code="123456" + ) + ) + with pytest.raises(http_error): + check_change_email_available(service, {"email": "other@example.com"}) + with pytest.raises(http_error): + reset_change_email(service, context, {"new_email": "other@example.com", "token": token}) + assert token in transports.tokens + assert transports.completed == [] + + +def test_missing_account_errors_remain_surface_specific( + service: AccountChangeEmailService, + context: AccountRequestContext | RequestContext, + transports: AccountEmailTransports, + sqlite_session_factory: sessionmaker[Session], +) -> None: + with sqlite_session_factory.begin() as session: + session.delete(session.get_one(Account, context.account_id)) + with pytest.raises(AccountNotFoundError): + send_change_email_code(service, context, {"email": "test@example.com"}, ip_address="127.0.0.1") + token = transports.issue( + AccountChangeEmailNewEmailVerifiedToken( + account_id=context.account_id, email="new@example.com", old_email="test@example.com", code="123456" + ) + ) + with pytest.raises(AccountNotFoundError): + reset_change_email(service, context, {"new_email": "new@example.com", "token": token}) + assert transports.sent == [] + assert transports.completed == [] diff --git a/api/tests/unit_tests/controllers/test_account_email_errors.py b/api/tests/unit_tests/controllers/test_account_email_errors.py new file mode 100644 index 00000000000..2f0d7f2adaf --- /dev/null +++ b/api/tests/unit_tests/controllers/test_account_email_errors.py @@ -0,0 +1,45 @@ +"""Keep shared Account email errors compatible with existing Console imports and wire contracts.""" + +import pytest + +from controllers import account_email_errors as shared +from controllers.console import error as console +from controllers.console.auth import error as auth +from libs.exception import BaseHTTPException + + +@pytest.mark.parametrize( + ("shared_error", "legacy_error", "code", "status"), + [ + (shared.InvalidEmailError, auth.InvalidEmailError, "invalid_email", 400), + (shared.InvalidTokenError, auth.InvalidTokenError, "invalid_or_expired_token", 400), + (shared.EmailCodeError, auth.EmailCodeError, "email_code_error", 400), + (shared.EmailChangeLimitError, auth.EmailChangeLimitError, "email_change_limit", 429), + (shared.EmailAlreadyInUseError, auth.EmailAlreadyInUseError, "email_already_in_use", 400), + (shared.EmailSendIpLimitError, console.EmailSendIpLimitError, "email_send_ip_limit", 429), + (shared.AccountInFreezeError, console.AccountInFreezeError, "account_in_freeze", 400), + (shared.EmailDomainSuspendedError, console.EmailDomainSuspendedError, "email_domain_suspended", 400), + ( + shared.EmailChangeRateLimitExceededError, + auth.EmailChangeRateLimitExceededError, + "email_change_rate_limit_exceeded", + 429, + ), + ], +) +def test_legacy_exports_keep_the_same_error_type_and_wire_contract( + shared_error: type[BaseHTTPException], legacy_error: type[BaseHTTPException], code: str, status: int +) -> None: + assert legacy_error is shared_error + error = shared_error() + assert error.code == status + assert error.data == {"code": code, "status": status, "message": error.description} + + +def test_retry_interval_preserves_default_and_policy_values() -> None: + assert shared.EmailChangeRateLimitExceededError().description == ( + "Too many email change emails have been sent. Please try again in 1 minutes." + ) + assert shared.EmailChangeRateLimitExceededError(5).description == ( + "Too many email change emails have been sent. Please try again in 5 minutes." + )