-
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(router): add external authentication webhooks flow #4339
feat(router): add external authentication webhooks flow #4339
Conversation
crates/router/src/core/webhooks.rs
Outdated
authentication_id, | ||
) | ||
.await | ||
.to_not_found_response(errors::ApiErrorResponse::InternalServerError) |
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.
shouldn't this be a 4xx?
crates/router/src/core/webhooks.rs
Outdated
connector_authentication_id, | ||
) | ||
.await | ||
.to_not_found_response(errors::ApiErrorResponse::InternalServerError) |
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 also should be a 4xx
"received a non-external-authentication id for retrieving authentication", | ||
) | ||
}?; | ||
let updated_authentication = state |
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 move this configuration to merchant connector account?
@@ -40,6 +40,8 @@ pub struct Authentication { | |||
pub acs_trans_id: Option<String>, | |||
pub three_ds_server_trans_id: Option<String>, | |||
pub acs_signed_content: Option<String>, | |||
pub profile_id: String, | |||
pub payment_id: Option<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.
we should also have merchant_connector_id
crates/router/src/core/webhooks.rs
Outdated
business_profile, | ||
)) | ||
.await | ||
.attach_printable("Incoming webhook flow for mandates failed")? |
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.
.attach_printable("Incoming webhook flow for mandates failed")? | |
.attach_printable("Incoming webhook flow for external authentication failed")? |
|
||
#[derive(Clone, Debug, Deserialize)] | ||
pub struct ExternalThreeDSConnectorMetadata { | ||
pub pull_mechanism_for_external_3ds_enabled: Option<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.
can we have this as an enum or bool instead of string?
Type of Change
Description
add external authentication webhooks flow
When external 3ds authentication is completed, the result of the challenge along with cavv, eci values are sent as a notification from 3ds connectors. We should consume the webhook (only if 3ds connector psync is not enabled), and then do the payments confirm to perform the authorization.
Additional Changes
Motivation and Context
How did you test it?
Sanity testing of payments, refunds and dispute webhooks (both using mca_id and connector name in url path)
Authentication webhooks can't be tested (will be implementing for netcetera then we'll be able to test)
Checklist
cargo +nightly fmt --all
cargo clippy