-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
feat(connector): Integrate PAZE Wallet #6030
Conversation
let (first_name, last_name) = paze_data | ||
.billing_address | ||
.name | ||
.unwrap_or( | ||
item.router_data | ||
.get_optional_billing() | ||
.and_then(|address| address.get_optional_full_name()) | ||
.ok_or(errors::ConnectorError::MissingRequiredField { | ||
field_name: "billing_address.name", | ||
})?, | ||
) | ||
.peek() | ||
.split_once(' ') |
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 logic may be incorrect though, splitting on first space to obtain first name and last name?
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 add an additional check where if first_name
or last_name
are empty strings then we can throw missing field error?
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.
I was thinking of situations where the beginning of the name contains initials, as in R. K. Narayan
for example. There would be multiple edge cases honestly.
Do we have provision to accept first name and last name as two separate fields itself? If that's possible, I'd prefer we opt for it. If that's not possible, we may have to consider other options.
Also looping in @Narayanbhat166.
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 do have the provision to collect first_name
and last_name
at hyperswitch which I've fixed.
In case of the billing name coming from paze we only receive full name which is handled by splitting it on the first whitespace for now. Once we start testing on sandbox we can take further decision on how to handle this. This extensive testing is not possible from backend for now because we only have 1 paze payload.
let paze_complete_response: Vec<&str> = paze_wallet_data | ||
.complete_response | ||
.peek() | ||
.split('.') | ||
.collect(); | ||
let encrypted_jwe_key = paze_complete_response | ||
.get(1) | ||
.ok_or(errors::PazeDecryptionError::DecryptionFailed)? | ||
.to_string(); |
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.
If this is handling JWT tokens, should we consider using a library like jsonwebtoken
or josekit
to decode the JWT token instead?
CC: @ShankarSinghC
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.
Hey @SanchithHegde
I am unable to find a function which returns the jwt payload. Can you help here?
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.
As discussed we are going ahead with parsing this data without a library because we were unable to find this functionality in any existing library.
let (first_name, last_name) = paze_data | ||
.billing_address | ||
.name | ||
.unwrap_or( | ||
item.router_data | ||
.get_optional_billing() | ||
.and_then(|address| address.get_optional_full_name()) | ||
.ok_or(errors::ConnectorError::MissingRequiredField { | ||
field_name: "billing_address.name", | ||
})?, | ||
) | ||
.peek() | ||
.split_once(' ') |
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.
I was thinking of situations where the beginning of the name contains initials, as in R. K. Narayan
for example. There would be multiple edge cases honestly.
Do we have provision to accept first name and last name as two separate fields itself? If that's possible, I'd prefer we opt for it. If that's not possible, we may have to consider other options.
Also looping in @Narayanbhat166.
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.
I see that we are implementing Paze decryption flow where the decryption credentials reside in application env. Are we going to implement any other flows for Paze (like connector tokenization or decrypting Paze token with merchant provided key).
If yes, then we would need to check if the Paze metadata will changes in these cases. So that while implementing new flow for Paze we don't have worry about migration of backwards compatibility of connector_wallet_details
.
let decoded_paze_private_key = BASE64_ENGINE | ||
.decode(paze_private_key.expose().as_bytes()) | ||
.change_context(errors::PazeDecryptionError::Base64DecodingFailed)?; | ||
let decrypted_private_key = openssl::rsa::Rsa::private_key_from_pem_passphrase( |
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.
Is it possible to use some other library ?
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.
I am unable to find a library which provides this functionality.
@SanchithHegde Can you please help here?
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.
As discussed we are going ahead with using the openssl library because we are unable to find this functionality in other libraries.
The wallet details needs to populated in the payment attempt and payment response
|
This is the only flow for Paze. In future we might implement connector tokenization flow but even in that connector wallet details won't change. |
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!
crates/router/src/core/payments.rs
Outdated
#[derive(Debug, Clone, serde::Serialize, serde::Deserialize, PartialEq, Eq, utoipa::ToSchema)] | ||
pub struct PazePaymentProcessingDetails { | ||
#[schema(value_type = String)] | ||
pub paze_private_key: Secret<String>, | ||
#[schema(value_type = String)] | ||
pub paze_private_key_passphrase: Secret<String>, | ||
} |
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.
Nit: I think we can remove the ToSchema
derive?
crates/router/src/core/payments.rs
Outdated
pub async fn add_decrypted_payment_method_token<F, Req, D>( | ||
tokenization_action: TokenizationAction, | ||
payment_data: &D, | ||
mut router_data: RouterData<F, Req, router_types::PaymentsResponseData>, |
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.
Is it possible to refactor the function so that instead of taking mut router_data
, it returns only the PaymentMethodToken
. Then, you can assign it to router_data.payment_method_token
as needed.
Something like
router_data.payment_method_token = add_decrypted_payment_method_token(tokenization_action.clone(), payment_data)
.await?;
@srujanchikke can you please confirm if this is fine ? |
@ShankarSinghC Currently we are getting network transaction id from Paze which we are not sure either to display it's last numbers. We can update the payments response after we get clarification from Paze. |
chore: generate openapi spec chore: generate openapi spec chore: generate openapi spec
Type of Change
Description
New wallet Paze is integrated in Hyperswitch with Cybserource as the supported payment processor.
Additional Changes
Motivation and Context
#6028
How did you test it?
Paze Payments need to be tested via Cybersource:
Request:
Response:
Request:
Response:
Request:
Response:
Checklist
cargo +nightly fmt --all
cargo clippy