-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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(payment_link): add provision for secured payment links #5357
Conversation
refactor(payment_link): segregate open and secured payment links
4904cd7
to
4a51278
Compare
…ent links only when they're not opened at top
@@ -1271,6 +1271,10 @@ impl PaymentLink { | |||
web::resource("{merchant_id}/{payment_id}") | |||
.route(web::get().to(initiate_payment_link)), | |||
) | |||
.service( | |||
web::resource("s/{merchant_id}/{payment_id}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this supposed to be only /{merchant_id}/{payment_id}
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've decided to add /s/
for secure links
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should change it to secure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was added based on the discussions, primarily for ensuring the context of open / secure links can not be inferred from these links // @knutties
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than that, looks good to me!
"{}/payment_link/s/{}/{}", | ||
domain_name, | ||
merchant_id.clone(), | ||
payment_id.clone() | ||
) | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is s
in the path param, why not explicitly state secure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PaymentLinkConfig { | ||
theme: DEFAULT_BACKGROUND_COLOR.to_string(), | ||
logo: DEFAULT_MERCHANT_LOGO.to_string(), | ||
seller_name: merchant_name_from_merchant_account, | ||
sdk_layout: DEFAULT_SDK_LAYOUT.to_owned(), | ||
display_sdk_only: DEFAULT_DISPLAY_SDK_ONLY, | ||
enabled_saved_payment_method: DEFAULT_ENABLE_SAVED_PAYMENT_METHOD, | ||
allowed_domains: DEFAULT_ALLOWED_DOMAINS, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we make a new structs for PaymentLinkConfig
as PaymentLinkOpenConfig
and PaymentLinkSecureConfig
. As it doesn't make sense to have allowed_domains params inside open_link
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is stored in DB as well, we will need to split tables / columns if we want to store different structs
@@ -1271,6 +1271,10 @@ impl PaymentLink { | |||
web::resource("{merchant_id}/{payment_id}") | |||
.route(web::get().to(initiate_payment_link)), | |||
) | |||
.service( | |||
web::resource("s/{merchant_id}/{payment_id}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should change it to secure
c133f9e
to
22928cc
Compare
committed from GitHub web Co-authored-by: Sanchith Hegde <[email protected]>
Type of Change
Description
Described in #5372
Additional Changes
Motivation and Context
How did you test it?
Tested locally using postman collection. Expectations -
Open links
1. Create a payment link - (unfold to view cURL)
curl --location 'http://localhost:8080/payments' \ --header 'Content-Type: application/json' \ --header 'Accept: application/json' \ --header 'api-key: dev_eBTVu8OMN1HmE2xr0I0dsfypWOVwQBlietvkxf0sgYMdyFM5nRO40M8M4MLH42Jk' \ --data '{ "customer_id": "cus_izAOZiynP5URGEvEfmgH", "amount": 100, "currency": "USD", "payment_link": true, "connector": [ "stripe" ], "session_expiry": 1000000, "return_url": "http://127.0.0.1:5500/src/pl_iframe.html", "payment_link_config": { "theme": "#14356f", "logo": "https://logosandtypes.com/wp-content/uploads/2020/08/zurich.svg", "seller_name": "Zurich Inc." } }'link
in API response) and validate the functionalitySecure links
1. Update `allowed_domains` in business profile - (unfold to view cURL)
curl --location 'http://localhost:8080/account/merchant_1721984694/business_profile/pro_oXHnmgfZSnfe92PvodqP' \ --header 'Content-Type: application/json' \ --header 'api-key: test_admin' \ --data '{ "payment_link_config": { "allowed_domains": [ "*" ], "enabled_saved_payment_method": true } }'2. Create a payment link - (unfold to view cURL)
curl --location 'http://localhost:8080/payments' \ --header 'Content-Type: application/json' \ --header 'Accept: application/json' \ --header 'api-key: dev_eBTVu8OMN1HmE2xr0I0dsfypWOVwQBlietvkxf0sgYMdyFM5nRO40M8M4MLH42Jk' \ --data '{ "customer_id": "cus_izAOZiynP5URGEvEfmgH", "amount": 100, "currency": "USD", "payment_link": true, "connector": [ "stripe" ], "session_expiry": 1000000, "return_url": "http://127.0.0.1:5500/src/pl_iframe.html", "payment_link_config": { "theme": "#14356f", "logo": "https://logosandtypes.com/wp-content/uploads/2020/08/zurich.svg", "seller_name": "Zurich Inc." } }'Secure links demo
Screencast.from.26-07-24.02.34.52.PM.IST.webm
Checklist
cargo +nightly fmt --all
cargo clippy