Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(core): implement NameType for name validation #6734

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

Sakilmostak
Copy link
Contributor

@Sakilmostak Sakilmostak commented Dec 3, 2024

Type of Change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring
  • Dependency updates
  • Documentation
  • CI/CD

Description

for card holder name in cards, validation is added before creation of request to flag invalid data in case it is present.
Reference is taken from Visa Documetation
image

Additional Changes

  • This PR modifies the API contract
  • This PR modifies the database schema
  • This PR modifies application configuration/environment variables

Motivation and Context

How did you test it?

Tested through Postman:

  • Create a MCA (Cybersource):

Case 1: Create a Payment with valid name:

{
    "amount": 499,
    "currency": "EUR",
    "confirm": true,
    "capture_method": "automatic",
    "customer_id": "test_rec7",
    "email": "[email protected]",
    "customer_acceptance": {
        "acceptance_type": "online"
    },
    "payment_method": "card",
    "payment_method_type": "credit",
    "payment_method_data": {
        "card": {
            "card_number": "4242424242424242",
            "card_exp_month": "03",
            "card_exp_year": "2030",
            "card_holder_name": "Sakil Mostak",
            "card_cvc": "737",
            "card_network": "Visa"
        }
    },
    "billing": {
        "address": {
            "city": "test",
            "country": "US",
            "line1": "here is some \n there is some \n none is some? \n ",
            "line2": "there",
            "line3": "anywhere",
            "zip": "560095",
            "state": "Washington",
            "first_name": "Sakil",
            "last_name": "Mostak"
        },
        "phone": {
            "number": "1234567890",
            "country_code": "+1"
        },
        "email": "[email protected]"
    },
    "authentication_type": "no_three_ds"
}
  • The payment should succeed, below is an example:
{
    "payment_id": "pay_7YANCrkBMUVJngRBg36l",
    "merchant_id": "merchant_1737373987",
    "status": "succeeded",
    "amount": 499,
    "net_amount": 499,
    "shipping_cost": null,
    "amount_capturable": 0,
    "amount_received": 499,
    "connector": "cybersource",
    "client_secret": "pay_7YANCrkBMUVJngRBg36l_secret_jp4t6pcoJTnMFrQsGd1R",
    "created": "2025-01-20T17:52:24.651Z",
    "currency": "EUR",
    "customer_id": "test_rec7",
    "customer": {
        "id": "test_rec7",
        "name": null,
        "email": "[email protected]",
        "phone": null,
        "phone_country_code": null
    },
    "description": null,
    "refunds": null,
    "disputes": null,
    "mandate_id": null,
    "mandate_data": null,
    "setup_future_usage": null,
    "off_session": null,
    "capture_on": null,
    "capture_method": "automatic",
    "payment_method": "card",
    "payment_method_data": {
        "card": {
            "last4": "4242",
            "card_type": null,
            "card_network": null,
            "card_issuer": null,
            "card_issuing_country": null,
            "card_isin": "424242",
            "card_extended_bin": null,
            "card_exp_month": "03",
            "card_exp_year": "2030",
            "card_holder_name": "Sakil Mostak",
            "payment_checks": {
                "avs_response": {
                    "code": "X",
                    "codeRaw": "I1"
                },
                "card_verification": null
            },
            "authentication_data": null
        },
        "billing": null
    },
    "payment_token": "token_7usZJrOsD3Qoy6kLGDnQ",
    "shipping": null,
    "billing": {
        "address": {
            "city": "test",
            "country": "US",
            "line1": "here is some \n there is some \n none is some? \n ",
            "line2": "there",
            "line3": "anywhere",
            "zip": "560095",
            "state": "Washington",
            "first_name": "Sakil",
            "last_name": "Mostak"
        },
        "phone": {
            "number": "1234567890",
            "country_code": "+1"
        },
        "email": "[email protected]"
    },
    "order_details": null,
    "email": "[email protected]",
    "name": null,
    "phone": null,
    "return_url": null,
    "authentication_type": "no_three_ds",
    "statement_descriptor_name": null,
    "statement_descriptor_suffix": null,
    "next_action": null,
    "cancellation_reason": null,
    "error_code": null,
    "error_message": null,
    "unified_code": null,
    "unified_message": null,
    "payment_experience": null,
    "payment_method_type": "credit",
    "connector_label": null,
    "business_country": null,
    "business_label": "default",
    "business_sub_label": null,
    "allowed_payment_method_types": null,
    "ephemeral_key": {
        "customer_id": "test_rec7",
        "created_at": 1737395544,
        "expires": 1737399144,
        "secret": "epk_b4c0b9ee60c844dd88e6cbc9daa9e7cb"
    },
    "manual_retry_allowed": false,
    "connector_transaction_id": "7373955459786405604807",
    "frm_message": null,
    "metadata": null,
    "connector_metadata": null,
    "feature_metadata": null,
    "reference_id": "pay_7YANCrkBMUVJngRBg36l_1",
    "payment_link": null,
    "profile_id": "pro_VcDn0jWTcCmofCpz2CAH",
    "surcharge_details": null,
    "attempt_count": 1,
    "merchant_decision": null,
    "merchant_connector_id": "mca_KXO79qkYuOXsSZCYByGz",
    "incremental_authorization_allowed": false,
    "authorization_count": null,
    "incremental_authorizations": null,
    "external_authentication_details": null,
    "external_3ds_authentication_attempted": false,
    "expires_on": "2025-01-20T18:07:24.651Z",
    "fingerprint": null,
    "browser_info": null,
    "payment_method_id": null,
    "payment_method_status": null,
    "updated": "2025-01-20T17:52:26.511Z",
    "charges": null,
    "frm_metadata": null,
    "merchant_order_reference_id": null,
    "order_tax_amount": null,
    "connector_mandate_id": null
}

Case 2: Create a Payment with invalid name:

{
    "amount": 499,
    "currency": "EUR",
    "confirm": true,
    "capture_method": "automatic",
    "customer_id": "test_rec7",
    "email": "[email protected]",
    "customer_acceptance": {
        "acceptance_type": "online"
    },
    "payment_method": "card",
    "payment_method_type": "credit",
    "payment_method_data": {
        "card": {
            "card_number": "4242424242424242",
            "card_exp_month": "03",
            "card_exp_year": "2030",
            "card_holder_name": "S@k*l M0st@k",
            "card_cvc": "737",
            "card_network": "Visa"
        }
    },
    "billing": {
        "address": {
            "city": "test",
            "country": "US",
            "line1": "here is some \n there is some \n none is some? \n ",
            "line2": "there",
            "line3": "anywhere",
            "zip": "560095",
            "state": "Washington",
            "first_name": "Sakil",
            "last_name": "Mostak"
        },
        "phone": {
            "number": "1234567890",
            "country_code": "+1"
        },
        "email": "[email protected]"
    },
    "authentication_type": "no_three_ds"
}

Response should contain 400 error with below response:

{
    "error": {
        "error_type": "invalid_request",
        "message": "Json deserialize error: invalid character found in card holder name: @ at line 23 column 5",
        "code": "IR_06"
    }
}

Checklist

  • I formatted the code cargo +nightly fmt --all
  • I addressed lints thrown by cargo clippy
  • I reviewed the submitted code
  • I added unit tests for my changes where possible

@Sakilmostak Sakilmostak added A-core Area: Core flows C-feature Category: Feature request or enhancement labels Dec 3, 2024
@Sakilmostak Sakilmostak added this to the December 2024 Release milestone Dec 3, 2024
@Sakilmostak Sakilmostak self-assigned this Dec 3, 2024
@Sakilmostak Sakilmostak requested review from a team as code owners December 3, 2024 14:27
Copy link

semanticdiff-com bot commented Dec 3, 2024

Review changes with  SemanticDiff

Changed Files
File Status
  crates/router/src/connector/nuvei/transformers.rs  96% smaller
  crates/router/src/core/payments/helpers.rs  80% smaller
  crates/router/src/connector/stripe/transformers.rs  79% smaller
  crates/router/tests/connectors/adyen.rs  77% smaller
  crates/router/tests/connectors/rapyd.rs  75% smaller
  crates/router/tests/connectors/worldline.rs  74% smaller
  crates/router/tests/connectors/utils.rs  71% smaller
  crates/router/tests/connectors/fiserv.rs  70% smaller
  crates/router/src/connector/netcetera/netcetera_types.rs  70% smaller
  crates/router/tests/connectors/aci.rs  68% smaller
  crates/router/src/compatibility/stripe/setup_intents/types.rs  64% smaller
  crates/router/src/core/payment_methods/cards.rs  64% smaller
  crates/router/tests/connectors/airwallex.rs  63% smaller
  crates/router/tests/payments.rs  59% smaller
  crates/router/tests/payments2.rs  59% smaller
  crates/router/tests/connectors/bitpay.rs  43% smaller
  crates/router/tests/connectors/coinbase.rs  43% smaller
  crates/router/tests/connectors/cryptopay.rs  43% smaller
  crates/router/tests/connectors/cybersource.rs  43% smaller
  crates/router/tests/connectors/forte.rs  43% smaller
  crates/router/tests/connectors/iatapay.rs  43% smaller
  crates/router/tests/connectors/opennode.rs  43% smaller
  crates/router/tests/connectors/trustpay.rs  43% smaller
  crates/router/tests/connectors/bluesnap.rs  43% smaller
  crates/router/tests/connectors/multisafepay.rs  42% smaller
  crates/router/tests/connectors/payme.rs  42% smaller
  crates/api_models/src/payments.rs  40% smaller
  crates/router/src/connector/utils.rs  38% smaller
  crates/hyperswitch_connectors/src/utils.rs  37% smaller
  crates/router/src/types/api/payments.rs  35% smaller
  crates/api_models/src/payment_methods.rs  29% smaller
  crates/router/src/compatibility/stripe/payment_intents/types.rs  28% smaller
  crates/router/src/core/payment_methods.rs  27% smaller
  crates/router/src/connector/cybersource/transformers.rs  11% smaller
  crates/router/src/core/payment_methods/vault.rs  4% smaller
  crates/hyperswitch_domain_models/src/address.rs  2% smaller
  crates/common_utils/src/types.rs  1% smaller
  crates/api_models/src/payments/additional_info.rs  1% smaller
  Cargo.lock Unsupported file format
  crates/api_models/Cargo.toml Unsupported file format
  crates/api_models/src/customers.rs  0% smaller
  crates/api_models/src/mandates.rs  0% smaller
  crates/api_models/src/payouts.rs  0% smaller
  crates/cards/tests/basic.rs  0% smaller
  crates/hyperswitch_connectors/src/connectors/bluesnap/transformers.rs  0% smaller
  crates/hyperswitch_connectors/src/connectors/worldline/transformers.rs  0% smaller
  crates/hyperswitch_domain_models/src/payment_method_data.rs  0% smaller
  crates/hyperswitch_domain_models/src/router_data.rs  0% smaller
  crates/hyperswitch_domain_models/src/router_request_types/fraud_check.rs  0% smaller
  crates/router/src/compatibility/stripe/customers/types.rs  0% smaller
  crates/router/src/connector/adyen/transformers.rs  0% smaller
  crates/router/src/connector/authorizedotnet/transformers.rs  0% smaller
  crates/router/src/connector/bankofamerica/transformers.rs  0% smaller
  crates/router/src/connector/payone/transformers.rs  0% smaller
  crates/router/src/connector/paypal/transformers.rs  0% smaller
  crates/router/src/connector/riskified/transformers/api.rs  0% smaller
  crates/router/src/connector/signifyd/transformers/api.rs  0% smaller
  crates/router/src/connector/stripe/transformers/connect.rs  0% smaller
  crates/router/src/connector/trustpay/transformers.rs  0% smaller
  crates/router/src/connector/wellsfargo/transformers.rs  0% smaller
  crates/router/src/core/customers.rs  0% smaller
  crates/router/src/core/locker_migration.rs  0% smaller
  crates/router/src/core/payment_methods/transformers.rs  0% smaller
  crates/router/src/core/payouts/helpers.rs  0% smaller
  crates/router/src/core/utils.rs  0% smaller
  crates/router/src/types/transformers.rs  0% smaller
  crates/router/src/utils.rs  0% smaller

@@ -710,6 +712,17 @@ impl<F: Send + Clone> GetTracker<F, PaymentData<F>, api::PaymentsRequest> for Pa
payment_method_data_billing.get_billing_address()
});

// validate billing name for card holder name
helpers::validate_billing_name(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

create a domain type which will have these validations, use the domain type wherever validations are required

@Sakilmostak Sakilmostak requested review from a team as code owners December 20, 2024 13:28
@Sakilmostak Sakilmostak changed the title feat(router): add card_holder_name pre-validator feat(router): implement NameType for name validation Dec 23, 2024
@Sakilmostak Sakilmostak changed the title feat(router): implement NameType for name validation feat(core): implement NameType for name validation Dec 30, 2024
// pub struct NameTypeValidationErr(&'static str);

#[derive(Clone, Default, Debug, Eq, PartialEq, Serialize)]
pub struct NameType(Secret<LengthString<256, 1>>);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we move this to common_utils or common_types, since this is not specific to cards as you have used this in all the other payment methods as well

@@ -1785,13 +1785,13 @@ impl GetAddressFromPaymentMethodData for Card {
let first_name = card_holder_name_iter
.next()
.map(ToOwned::to_owned)
.map(Secret::new);
.and_then(|name| NameType::try_from(name).ok());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do not use .ok(), log the error

Copy link
Member

@Narayanbhat166 Narayanbhat166 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replace all instances of .ok() with logging the error, and if possible qualify the imports

Copy link
Member

@Narayanbhat166 Narayanbhat166 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wherever possible try to use the nametype, and if the domain is expanding too much convert the type, and raise errors when the conversion might fail

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-core Area: Core flows C-feature Category: Feature request or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants