From e880d71468ae14a31f0481be3e48dd12ca4b560e Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Sat, 21 Sep 2024 10:32:44 +0200 Subject: [PATCH 1/2] =?UTF-8?q?=20=F0=9F=90=9B=20Improved=20Error=20Handli?= =?UTF-8?q?ng=20for=20Missing=20=20Billing=20Details=20(#6418)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/models_library/users.py | 2 +- .../payments/_onetime_api.py | 2 ++ .../simcore_service_webserver/users/_db.py | 8 ++++-- .../users/exceptions.py | 5 ++++ .../wallets/_constants.py | 5 ++++ .../wallets/_handlers.py | 26 +++++++++++++++++-- .../03/wallets/payments/test_payments.py | 26 +++++++++++++++++++ 7 files changed, 69 insertions(+), 5 deletions(-) diff --git a/packages/models-library/src/models_library/users.py b/packages/models-library/src/models_library/users.py index 31ca948a1b8..a28add967a6 100644 --- a/packages/models-library/src/models_library/users.py +++ b/packages/models-library/src/models_library/users.py @@ -22,7 +22,7 @@ class UserBillingDetails(BaseModel): address: str | None city: str | None state: str | None = Field(description="State, province, canton, ...") - country: str + country: str # Required for taxes postal_code: str | None phone: str | None diff --git a/services/web/server/src/simcore_service_webserver/payments/_onetime_api.py b/services/web/server/src/simcore_service_webserver/payments/_onetime_api.py index f1c9f9df733..f54f48403bb 100644 --- a/services/web/server/src/simcore_service_webserver/payments/_onetime_api.py +++ b/services/web/server/src/simcore_service_webserver/payments/_onetime_api.py @@ -279,6 +279,7 @@ async def init_creation_of_wallet_payment( Raises: UserNotFoundError WalletAccessForbiddenError + BillingDetailsNotFoundError """ # wallet: check permissions @@ -293,6 +294,7 @@ async def init_creation_of_wallet_payment( # user info user = await get_user_display_and_id_names(app, user_id=user_id) user_invoice_address = await get_user_invoice_address(app, user_id=user_id) + # stripe info product_stripe_info = await get_product_stripe_info(app, product_name=product_name) diff --git a/services/web/server/src/simcore_service_webserver/users/_db.py b/services/web/server/src/simcore_service_webserver/users/_db.py index 100575cd522..f7d8769f963 100644 --- a/services/web/server/src/simcore_service_webserver/users/_db.py +++ b/services/web/server/src/simcore_service_webserver/users/_db.py @@ -21,6 +21,7 @@ from ..db.models import user_to_groups from ..db.plugin import get_database_engine +from .exceptions import BillingDetailsNotFoundError from .schemas import Permission _ALL = None @@ -203,9 +204,12 @@ async def new_user_details( async def get_user_billing_details( engine: Engine, user_id: UserID ) -> UserBillingDetails: + """ + Raises: + BillingDetailsNotFoundError + """ async with engine.acquire() as conn: user_billing_details = await UsersRepo.get_billing_details(conn, user_id) if not user_billing_details: - msg = f"Missing biling details for user {user_id}" - raise ValueError(msg) + raise BillingDetailsNotFoundError(user_id=user_id) return UserBillingDetails.from_orm(user_billing_details) diff --git a/services/web/server/src/simcore_service_webserver/users/exceptions.py b/services/web/server/src/simcore_service_webserver/users/exceptions.py index 08e1432ece0..13b14ee0240 100644 --- a/services/web/server/src/simcore_service_webserver/users/exceptions.py +++ b/services/web/server/src/simcore_service_webserver/users/exceptions.py @@ -45,3 +45,8 @@ class AlreadyPreRegisteredError(UsersBaseError): msg_template = ( "Found {num_found} matches for '{email}'. Cannot pre-register existing user" ) + + +class BillingDetailsNotFoundError(UsersBaseError): + # NOTE: this is for internal log and should not be transmitted to the final user + msg_template = "Billing details are missing for user_id={user_id}. TIP: Check whether this user is pre-registered" diff --git a/services/web/server/src/simcore_service_webserver/wallets/_constants.py b/services/web/server/src/simcore_service_webserver/wallets/_constants.py index a8354070f93..eab6335e3df 100644 --- a/services/web/server/src/simcore_service_webserver/wallets/_constants.py +++ b/services/web/server/src/simcore_service_webserver/wallets/_constants.py @@ -3,3 +3,8 @@ MSG_PRICE_NOT_DEFINED_ERROR: Final[ str ] = "No payments are accepted until this product has a price" + +MSG_BILLING_DETAILS_NOT_DEFINED_ERROR: Final[str] = ( + "Payments cannot be processed: Required billing details (e.g. country for tax) are missing from your account." + "Please contact support to resolve this configuration issue." +) diff --git a/services/web/server/src/simcore_service_webserver/wallets/_handlers.py b/services/web/server/src/simcore_service_webserver/wallets/_handlers.py index 0b43bbea59a..e7c67919f10 100644 --- a/services/web/server/src/simcore_service_webserver/wallets/_handlers.py +++ b/services/web/server/src/simcore_service_webserver/wallets/_handlers.py @@ -18,6 +18,8 @@ parse_request_path_parameters_as, ) from servicelib.aiohttp.typing_extension import Handler +from servicelib.error_codes import create_error_code +from servicelib.logging_utils import LogExtra, get_log_record_extra from servicelib.request_keys import RQT_USERID_KEY from .._constants import RQ_PRODUCT_KEY @@ -36,10 +38,16 @@ ) from ..products.errors import BelowMinimumPaymentError, ProductPriceNotDefinedError from ..security.decorators import permission_required -from ..users.exceptions import UserDefaultWalletNotFoundError +from ..users.exceptions import ( + BillingDetailsNotFoundError, + UserDefaultWalletNotFoundError, +) from ..utils_aiohttp import envelope_json_response from . import _api -from ._constants import MSG_PRICE_NOT_DEFINED_ERROR +from ._constants import ( + MSG_BILLING_DETAILS_NOT_DEFINED_ERROR, + MSG_PRICE_NOT_DEFINED_ERROR, +) from .errors import WalletAccessForbiddenError, WalletNotFoundError _logger = logging.getLogger(__name__) @@ -80,6 +88,20 @@ async def wrapper(request: web.Request) -> web.StreamResponse: except ProductPriceNotDefinedError as exc: raise web.HTTPConflict(reason=MSG_PRICE_NOT_DEFINED_ERROR) from exc + except BillingDetailsNotFoundError as exc: + error_code = create_error_code(exc) + log_extra: LogExtra = {} + if user_id := getattr(exc, "user_id", None): + log_extra = get_log_record_extra(user_id=user_id) or {} + + log_msg = f"{exc} [{error_code}]" + _logger.exception( + log_msg, + extra={"error_code": error_code, **log_extra}, + ) + user_msg = f"{MSG_BILLING_DETAILS_NOT_DEFINED_ERROR} ({error_code})" + raise web.HTTPServiceUnavailable(reason=user_msg) from exc + return wrapper diff --git a/services/web/server/tests/unit/with_dbs/03/wallets/payments/test_payments.py b/services/web/server/tests/unit/with_dbs/03/wallets/payments/test_payments.py index ed8b2868481..f6519735ed1 100644 --- a/services/web/server/tests/unit/with_dbs/03/wallets/payments/test_payments.py +++ b/services/web/server/tests/unit/with_dbs/03/wallets/payments/test_payments.py @@ -33,6 +33,9 @@ PaymentsSettings, get_plugin_settings, ) +from simcore_service_webserver.wallets._constants import ( + MSG_BILLING_DETAILS_NOT_DEFINED_ERROR, +) OpenApiDict: TypeAlias = dict[str, Any] @@ -312,6 +315,29 @@ async def test_complete_payment_errors( send_message.assert_called_once() +async def test_billing_info_missing_error( + latest_osparc_price: Decimal, + client: TestClient, + logged_user_wallet: WalletGet, +): + # NOTE: setup_user_pre_registration_details_db is not setup to emulate missing pre-registration + + assert client.app + wallet = logged_user_wallet + + # Pay + response = await client.post( + f"/v0/wallets/{wallet.wallet_id}/payments", json={"priceDollars": 25} + ) + data, error = await assert_status(response, status.HTTP_503_SERVICE_UNAVAILABLE) + + assert not data + assert MSG_BILLING_DETAILS_NOT_DEFINED_ERROR in error["message"] + + assert response.reason + assert MSG_BILLING_DETAILS_NOT_DEFINED_ERROR in response.reason + + async def test_payment_not_found( latest_osparc_price: Decimal, client: TestClient, From 48eca2f8e55ad7a569de6417e725b8c5bef5183e Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Mon, 23 Sep 2024 08:46:01 +0200 Subject: [PATCH 2/2] =?UTF-8?q?=F0=9F=8E=A8=20Enhances=20Product=20parsing?= =?UTF-8?q?=20to=20strip=20whitespaces=20in=20host=5Fregex=20(#6419)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../products/_model.py | 16 +++++++++++++--- .../unit/isolated/test_products_model.py | 19 +++++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/products/_model.py b/services/web/server/src/simcore_service_webserver/products/_model.py index de3955652a3..cccd7707008 100644 --- a/services/web/server/src/simcore_service_webserver/products/_model.py +++ b/services/web/server/src/simcore_service_webserver/products/_model.py @@ -111,7 +111,7 @@ class Product(BaseModel): @validator("*", pre=True) @classmethod - def parse_empty_string_as_null(cls, v): + def _parse_empty_string_as_null(cls, v): """Safe measure: database entries are sometimes left blank instead of null""" if isinstance(v, str) and len(v.strip()) == 0: return None @@ -119,12 +119,21 @@ def parse_empty_string_as_null(cls, v): @validator("name", pre=True, always=True) @classmethod - def validate_name(cls, v): + def _validate_name(cls, v): if v not in FRONTEND_APPS_AVAILABLE: msg = f"{v} is not in available front-end apps {FRONTEND_APPS_AVAILABLE}" raise ValueError(msg) return v + @validator("host_regex", pre=True) + @classmethod + def _strip_whitespaces(cls, v): + if v and isinstance(v, str): + # Prevents unintended leading & trailing spaces when added + # manually in the database + return v.strip() + return v + @property def twilio_alpha_numeric_sender_id(self) -> str: return self.short_name or self.display_name.replace(string.punctuation, "")[:11] @@ -132,9 +141,10 @@ def twilio_alpha_numeric_sender_id(self) -> str: class Config: alias_generator = snake_to_camel # to export allow_population_by_field_name = True + anystr_strip_whitespace = True + extra = Extra.ignore frozen = True # read-only orm_mode = True - extra = Extra.ignore schema_extra: ClassVar[dict[str, Any]] = { "examples": [ { diff --git a/services/web/server/tests/unit/isolated/test_products_model.py b/services/web/server/tests/unit/isolated/test_products_model.py index b3ee823a37e..84fa67d94eb 100644 --- a/services/web/server/tests/unit/isolated/test_products_model.py +++ b/services/web/server/tests/unit/isolated/test_products_model.py @@ -76,3 +76,22 @@ def test_product_to_static(): ], "isPaymentEnabled": False, } + + +def test_product_host_regex_with_spaces(): + data = Product.Config.schema_extra["examples"][2] + + # with leading and trailing spaces and uppercase (tests anystr_strip_whitespace ) + data["support_email"] = " fOO@BaR.COM " + + # with leading trailing spaces (tests validator("host_regex", pre=True)) + expected = r"([\.-]{0,1}osparc[\.-])".strip() + data["host_regex"] = expected + " " + + # parsing should strip all whitespaces and normalize email + product = Product.parse_obj(data) + + assert product.host_regex.pattern == expected + assert product.host_regex.search("osparc.bar.com") + + assert product.support_email == "foo@bar.com"