fix(router): pop bugs in the payments confirm flow (#157)

crates/router/src/compatibility/stripe/errors.rs

@@ -64,6 +64,9 @@ pub(crate) enum ErrorCode {
     #[error(error_type = StripeErrorType::InvalidRequestError, code = "resource_missing", message = "No such refund")]
     RefundNotFound,
 
+    #[error(error_type = StripeErrorType::InvalidRequestError, code = "client_secret_invalid", message = "Expected client secret to be included in the request")]
+    ClientSecretNotFound,
+
     #[error(error_type = StripeErrorType::InvalidRequestError, code = "resource_missing", message = "No such customer")]
     CustomerNotFound,
 
@@ -353,6 +356,7 @@ impl From<ApiErrorResponse> for ErrorCode {
             ApiErrorResponse::CustomerNotFound => ErrorCode::CustomerNotFound,
             ApiErrorResponse::PaymentNotFound => ErrorCode::PaymentNotFound,
             ApiErrorResponse::PaymentMethodNotFound => ErrorCode::PaymentMethodNotFound,
+            ApiErrorResponse::ClientSecretNotGiven => ErrorCode::ClientSecretNotFound,
             ApiErrorResponse::MerchantAccountNotFound => ErrorCode::MerchantAccountNotFound,
             ApiErrorResponse::ResourceIdNotFound => ErrorCode::ResourceIdNotFound,
             ApiErrorResponse::MerchantConnectorAccountNotFound => {
@@ -420,6 +424,7 @@ impl actix_web::ResponseError for ErrorCode {
             | ErrorCode::DuplicateRefundRequest
             | ErrorCode::RefundNotFound
             | ErrorCode::CustomerNotFound
+            | ErrorCode::ClientSecretNotFound
             | ErrorCode::PaymentNotFound
             | ErrorCode::PaymentMethodNotFound
             | ErrorCode::MerchantAccountNotFound

crates/router/src/core/errors/api_error_response.rs

@@ -48,6 +48,8 @@ pub enum ApiErrorResponse {
     /// a field fails.
     #[error(error_type = ErrorType::InvalidRequestError, code = "IR_07", message = "Invalid value provided: {field_name}.")]
     InvalidDataValue { field_name: &'static str },
+    #[error(error_type = ErrorType::InvalidRequestError, code = "IR_07", message = "Client secret was not provided")]
+    ClientSecretNotGiven,
     #[error(error_type = ErrorType::InvalidRequestError, code = "IR_07", message = "The client_secret provided does not match the client_secret associated with the Payment.")]
     ClientSecretInvalid,
     #[error(error_type = ErrorType::InvalidRequestError, code = "IR_07", message = "Customer has existing mandate/subsciption.")]
@@ -178,6 +180,7 @@ impl actix_web::ResponseError for ApiErrorResponse {
             | ApiErrorResponse::MerchantAccountNotFound
             | ApiErrorResponse::MerchantConnectorAccountNotFound
             | ApiErrorResponse::MandateNotFound
+            | ApiErrorResponse::ClientSecretNotGiven
             | ApiErrorResponse::ClientSecretInvalid
             | ApiErrorResponse::SuccessfulPaymentNotFound
             | ApiErrorResponse::ResourceIdNotFound

crates/router/src/core/payments/helpers.rs

@@ -20,6 +20,7 @@ use crate::{
     db::StorageInterface,
     pii::Secret,
     routes::AppState,
+    scheduler::{metrics, workflows::payment_sync},
     services,
     types::{
         self,
@@ -423,6 +424,45 @@ pub fn payment_intent_status_fsm(
     }
 }
 
+pub async fn add_domain_task_to_pt<Op>(
+    operation: &Op,
+    state: &AppState,
+    payment_attempt: &storage::PaymentAttempt,
+) -> CustomResult<(), errors::ApiErrorResponse>
+where
+    Op: std::fmt::Debug,
+{
+    if check_if_operation_confirm(operation) {
+        let connector_name = payment_attempt
+            .connector
+            .clone()
+            .ok_or(errors::ApiErrorResponse::InternalServerError)?;
+
+        let schedule_time = payment_sync::get_sync_process_schedule_time(
+            &*state.store,
+            &connector_name,
+            &payment_attempt.merchant_id,
+            0,
+        )
+        .await
+        .into_report()
+        .change_context(errors::ApiErrorResponse::InternalServerError)?;
+
+        match schedule_time {
+            Some(stime) => {
+                metrics::TASKS_ADDED_COUNT.add(&metrics::CONTEXT, 1, &[]); // Metrics
+                super::add_process_sync_task(&*state.store, payment_attempt, stime)
+                    .await
+                    .into_report()
+                    .change_context(errors::ApiErrorResponse::InternalServerError)
+            }
+            None => Ok(()),
+        }
+    } else {
+        Ok(())
+    }
+}
+
 pub fn response_operation<'a, F, R>() -> BoxedOperation<'a, F, R>
 where
     F: Send + Clone,
@@ -1202,6 +1242,18 @@ pub(crate) fn authenticate_client_secret(
     }
 }
 
+pub(crate) fn validate_pm_or_token_given(
+    token: &Option<String>,
+    pm_data: &Option<api::PaymentMethod>,
+) -> Result<(), errors::ApiErrorResponse> {
+    utils::when(
+        token.is_none() && pm_data.is_none(),
+        Err(errors::ApiErrorResponse::InvalidRequestData {
+            message: "A payment token or payment method data is required".to_string(),
+        }),
+    )
+}
+
 // A function to perform database lookup and then verify the client secret
 pub(crate) async fn verify_client_secret(
     db: &dyn StorageInterface,

crates/router/src/core/payments/operations.rs

@@ -9,7 +9,7 @@ mod payment_start;
 mod payment_status;
 mod payment_update;
 use async_trait::async_trait;
-use error_stack::{report, IntoReport, ResultExt};
+use error_stack::{report, ResultExt};
 pub use payment_cancel::PaymentCancel;
 pub use payment_capture::PaymentCapture;
 pub use payment_confirm::PaymentConfirm;
@@ -29,7 +29,6 @@ use crate::{
     db::StorageInterface,
     pii::Secret,
     routes::AppState,
-    scheduler::{metrics, workflows::payment_sync},
     types::{
         self, api,
         storage::{self, enums},
@@ -170,110 +169,6 @@ pub trait PostUpdateTracker<F, D, R>: Send {
         F: 'b + Send;
 }
 
-#[async_trait]
-impl<F: Clone + Send, Op: Send + Sync + Operation<F, api::PaymentsRequest>>
-    Domain<F, api::PaymentsRequest> for Op
-where
-    for<'a> &'a Op: Operation<F, api::PaymentsRequest> + std::fmt::Debug,
-{
-    #[instrument(skip_all)]
-    async fn get_or_create_customer_details<'a>(
-        &'a self,
-        db: &dyn StorageInterface,
-        payment_data: &mut PaymentData<F>,
-        request: Option<CustomerDetails>,
-        merchant_id: &str,
-    ) -> CustomResult<
-        (
-            BoxedOperation<'a, F, api::PaymentsRequest>,
-            Option<storage::Customer>,
-        ),
-        errors::StorageError,
-    > {
-        helpers::create_customer_if_not_exist(
-            Box::new(self),
-            db,
-            payment_data,
-            request,
-            merchant_id,
-        )
-        .await
-    }
-
-    #[instrument(skip_all)]
-    async fn make_pm_data<'a>(
-        &'a self,
-        state: &'a AppState,
-        payment_method: Option<enums::PaymentMethodType>,
-        txn_id: &str,
-        payment_attempt: &storage::PaymentAttempt,
-        request: &Option<api::PaymentMethod>,
-        token: &Option<String>,
-        card_cvc: Option<Secret<String>>,
-        _storage_scheme: enums::MerchantStorageScheme,
-    ) -> RouterResult<(
-        BoxedOperation<'a, F, api::PaymentsRequest>,
-        Option<api::PaymentMethod>,
-        Option<String>,
-    )> {
-        helpers::make_pm_data(
-            Box::new(self),
-            state,
-            payment_method,
-            txn_id,
-            payment_attempt,
-            request,
-            token,
-            card_cvc,
-        )
-        .await
-    }
-
-    #[instrument(skip_all)]
-    async fn add_task_to_process_tracker<'a>(
-        &'a self,
-        state: &'a AppState,
-        payment_attempt: &storage::PaymentAttempt,
-    ) -> CustomResult<(), errors::ApiErrorResponse> {
-        if helpers::check_if_operation_confirm(self) {
-            metrics::TASKS_ADDED_COUNT.add(&metrics::CONTEXT, 1, &[]); // Metrics
-
-            let connector_name = payment_attempt
-                .connector
-                .clone()
-                .ok_or(errors::ApiErrorResponse::InternalServerError)?;
-
-            let schedule_time = payment_sync::get_sync_process_schedule_time(
-                &*state.store,
-                &connector_name,
-                &payment_attempt.merchant_id,
-                0,
-            )
-            .await
-            .into_report()
-            .change_context(errors::ApiErrorResponse::InternalServerError)?;
-
-            match schedule_time {
-                Some(stime) => super::add_process_sync_task(&*state.store, payment_attempt, stime)
-                    .await
-                    .into_report()
-                    .change_context(errors::ApiErrorResponse::InternalServerError),
-                None => Ok(()),
-            }
-        } else {
-            Ok(())
-        }
-    }
-
-    async fn get_connector<'a>(
-        &'a self,
-        merchant_account: &storage::MerchantAccount,
-        state: &AppState,
-    ) -> CustomResult<api::ConnectorCallType, errors::ApiErrorResponse> {
-        helpers::get_connector_default(merchant_account, state).await
-    }
-}
-
 #[async_trait]
 impl<F: Clone + Send, Op: Send + Sync + Operation<F, api::PaymentsRetrieveRequest>>
     Domain<F, api::PaymentsRetrieveRequest> for Op

crates/router/src/core/payments/operations/payment_confirm.rs

@@ -2,13 +2,14 @@ use std::marker::PhantomData;
 
 use async_trait::async_trait;
 use error_stack::{report, ResultExt};
+use masking::Secret;
 use router_derive::PaymentOperation;
 use router_env::{instrument, tracing};
 
 use super::{BoxedOperation, Domain, GetTracker, Operation, UpdateTracker, ValidateRequest};
 use crate::{
     core::{
-        errors::{self, RouterResult, StorageErrorExt},
+        errors::{self, CustomResult, RouterResult, StorageErrorExt},
         payments::{helpers, operations, CustomerDetails, PaymentAddress, PaymentData},
         utils as core_utils,
     },
@@ -167,6 +168,86 @@ impl<F: Send + Clone> GetTracker<F, PaymentData<F>, api::PaymentsRequest> for Pa
     }
 }
 
+#[async_trait]
+impl<F: Clone + Send> Domain<F, api::PaymentsRequest> for PaymentConfirm {
+    #[instrument(skip_all)]
+    async fn get_or_create_customer_details<'a>(
+        &'a self,
+        db: &dyn StorageInterface,
+        payment_data: &mut PaymentData<F>,
+        request: Option<CustomerDetails>,
+        merchant_id: &str,
+    ) -> CustomResult<
+        (
+            BoxedOperation<'a, F, api::PaymentsRequest>,
+            Option<storage::Customer>,
+        ),
+        errors::StorageError,
+    > {
+        helpers::create_customer_if_not_exist(
+            Box::new(self),
+            db,
+            payment_data,
+            request,
+            merchant_id,
+        )
+        .await
+    }
+
+    #[instrument(skip_all)]
+    async fn make_pm_data<'a>(
+        &'a self,
+        state: &'a AppState,
+        payment_method: Option<enums::PaymentMethodType>,
+        txn_id: &str,
+        payment_attempt: &storage::PaymentAttempt,
+        request: &Option<api::PaymentMethod>,
+        token: &Option<String>,
+        card_cvc: Option<Secret<String>>,
+        _storage_scheme: enums::MerchantStorageScheme,
+    ) -> RouterResult<(
+        BoxedOperation<'a, F, api::PaymentsRequest>,
+        Option<api::PaymentMethod>,
+        Option<String>,
+    )> {
+        let (op, payment_method, payment_token) = helpers::make_pm_data(
+            Box::new(self),
+            state,
+            payment_method,
+            txn_id,
+            payment_attempt,
+            request,
+            token,
+            card_cvc,
+        )
+        .await?;
+
+        utils::when(
+            payment_method.is_none(),
+            Err(errors::ApiErrorResponse::PaymentMethodNotFound),
+        )?;
+
+        Ok((op, payment_method, payment_token))
+    }
+
+    #[instrument(skip_all)]
+    async fn add_task_to_process_tracker<'a>(
+        &'a self,
+        state: &'a AppState,
+        payment_attempt: &storage::PaymentAttempt,
+    ) -> CustomResult<(), errors::ApiErrorResponse> {
+        helpers::add_domain_task_to_pt(self, state, payment_attempt).await
+    }
+
+    async fn get_connector<'a>(
+        &'a self,
+        merchant_account: &storage::MerchantAccount,
+        state: &AppState,
+    ) -> CustomResult<api::ConnectorCallType, errors::ApiErrorResponse> {
+        helpers::get_connector_default(merchant_account, state).await
+    }
+}
+
 #[async_trait]
 impl<F: Clone> UpdateTracker<F, PaymentData<F>, api::PaymentsRequest> for PaymentConfirm {
     #[instrument(skip_all)]
@@ -265,6 +346,9 @@ impl<F: Send + Clone> ValidateRequest<F, api::PaymentsRequest> for PaymentConfir
                 field_name: "merchant_id".to_string(),
                 expected_format: "merchant_id from merchant account".to_string(),
             })?;
+
+        helpers::validate_pm_or_token_given(&request.payment_token, &request.payment_method_data)?;
+
         let mandate_type = helpers::validate_mandate(request)?;
 
         let payment_id = core_utils::get_or_generate_id("payment_id", &given_payment_id, "pay")?;

crates/router/src/core/payments/operations/payment_create.rs

@@ -2,6 +2,7 @@ use std::marker::PhantomData;
 
 use async_trait::async_trait;
 use error_stack::ResultExt;
+use masking::Secret;
 use router_derive::PaymentOperation;
 use router_env::{instrument, tracing};
 use uuid::Uuid;
@@ -10,7 +11,7 @@ use super::{BoxedOperation, Domain, GetTracker, Operation, UpdateTracker, Valida
 use crate::{
     consts,
     core::{
-        errors::{self, RouterResult, StorageErrorExt},
+        errors::{self, CustomResult, RouterResult, StorageErrorExt},
         payments::{self, helpers, operations, CustomerDetails, PaymentAddress, PaymentData},
         utils as core_utils,
     },
@@ -221,6 +222,79 @@ impl<F: Send + Clone> GetTracker<F, PaymentData<F>, api::PaymentsRequest> for Pa
     }
 }
 
+#[async_trait]
+impl<F: Clone + Send> Domain<F, api::PaymentsRequest> for PaymentCreate {
+    #[instrument(skip_all)]
+    async fn get_or_create_customer_details<'a>(
+        &'a self,
+        db: &dyn StorageInterface,
+        payment_data: &mut PaymentData<F>,
+        request: Option<CustomerDetails>,
+        merchant_id: &str,
+    ) -> CustomResult<
+        (
+            BoxedOperation<'a, F, api::PaymentsRequest>,
+            Option<storage::Customer>,
+        ),
+        errors::StorageError,
+    > {
+        helpers::create_customer_if_not_exist(
+            Box::new(self),
+            db,
+            payment_data,
+            request,
+            merchant_id,
+        )
+        .await
+    }
+
+    #[instrument(skip_all)]
+    async fn make_pm_data<'a>(
+        &'a self,
+        state: &'a AppState,
+        payment_method: Option<enums::PaymentMethodType>,
+        txn_id: &str,
+        payment_attempt: &storage::PaymentAttempt,
+        request: &Option<api::PaymentMethod>,
+        token: &Option<String>,
+        card_cvc: Option<Secret<String>>,
+        _storage_scheme: enums::MerchantStorageScheme,
+    ) -> RouterResult<(
+        BoxedOperation<'a, F, api::PaymentsRequest>,
+        Option<api::PaymentMethod>,
+        Option<String>,
+    )> {
+        helpers::make_pm_data(
+            Box::new(self),
+            state,
+            payment_method,
+            txn_id,
+            payment_attempt,
+            request,
+            token,
+            card_cvc,
+        )
+        .await
+    }
+
+    #[instrument(skip_all)]
+    async fn add_task_to_process_tracker<'a>(
+        &'a self,
+        state: &'a AppState,
+        payment_attempt: &storage::PaymentAttempt,
+    ) -> CustomResult<(), errors::ApiErrorResponse> {
+        helpers::add_domain_task_to_pt(self, state, payment_attempt).await
+    }
+
+    async fn get_connector<'a>(
+        &'a self,
+        merchant_account: &storage::MerchantAccount,
+        state: &AppState,
+    ) -> CustomResult<api::ConnectorCallType, errors::ApiErrorResponse> {
+        helpers::get_connector_default(merchant_account, state).await
+    }
+}
+
 #[async_trait]
 impl<F: Clone> UpdateTracker<F, PaymentData<F>, api::PaymentsRequest> for PaymentCreate {
     #[instrument(skip_all)]

crates/router/src/core/payments/operations/payment_status.rs

@@ -2,13 +2,14 @@ use std::marker::PhantomData;
 
 use async_trait::async_trait;
 use error_stack::ResultExt;
+use masking::Secret;
 use router_derive::PaymentOperation;
 use router_env::{instrument, tracing};
 
 use super::{BoxedOperation, Domain, GetTracker, Operation, UpdateTracker, ValidateRequest};
 use crate::{
     core::{
-        errors::{self, ApiErrorResponse, RouterResult, StorageErrorExt},
+        errors::{self, ApiErrorResponse, CustomResult, RouterResult, StorageErrorExt},
         payments::{helpers, operations, CustomerDetails, PaymentAddress, PaymentData},
     },
     db::StorageInterface,
@@ -48,6 +49,79 @@ impl<F: Send + Clone> Operation<F, api::PaymentsRequest> for &PaymentStatus {
     }
 }
 
+#[async_trait]
+impl<F: Clone + Send> Domain<F, api::PaymentsRequest> for PaymentStatus {
+    #[instrument(skip_all)]
+    async fn get_or_create_customer_details<'a>(
+        &'a self,
+        db: &dyn StorageInterface,
+        payment_data: &mut PaymentData<F>,
+        request: Option<CustomerDetails>,
+        merchant_id: &str,
+    ) -> CustomResult<
+        (
+            BoxedOperation<'a, F, api::PaymentsRequest>,
+            Option<storage::Customer>,
+        ),
+        errors::StorageError,
+    > {
+        helpers::create_customer_if_not_exist(
+            Box::new(self),
+            db,
+            payment_data,
+            request,
+            merchant_id,
+        )
+        .await
+    }
+
+    #[instrument(skip_all)]
+    async fn make_pm_data<'a>(
+        &'a self,
+        state: &'a AppState,
+        payment_method: Option<enums::PaymentMethodType>,
+        txn_id: &str,
+        payment_attempt: &storage::PaymentAttempt,
+        request: &Option<api::PaymentMethod>,
+        token: &Option<String>,
+        card_cvc: Option<Secret<String>>,
+        _storage_scheme: enums::MerchantStorageScheme,
+    ) -> RouterResult<(
+        BoxedOperation<'a, F, api::PaymentsRequest>,
+        Option<api::PaymentMethod>,
+        Option<String>,
+    )> {
+        helpers::make_pm_data(
+            Box::new(self),
+            state,
+            payment_method,
+            txn_id,
+            payment_attempt,
+            request,
+            token,
+            card_cvc,
+        )
+        .await
+    }
+
+    #[instrument(skip_all)]
+    async fn add_task_to_process_tracker<'a>(
+        &'a self,
+        state: &'a AppState,
+        payment_attempt: &storage::PaymentAttempt,
+    ) -> CustomResult<(), errors::ApiErrorResponse> {
+        helpers::add_domain_task_to_pt(self, state, payment_attempt).await
+    }
+
+    async fn get_connector<'a>(
+        &'a self,
+        merchant_account: &storage::MerchantAccount,
+        state: &AppState,
+    ) -> CustomResult<api::ConnectorCallType, errors::ApiErrorResponse> {
+        helpers::get_connector_default(merchant_account, state).await
+    }
+}
+
 #[async_trait]
 impl<F: Clone> UpdateTracker<F, PaymentData<F>, api::PaymentsRequest> for PaymentStatus {
     async fn update_trackers<'b>(

crates/router/src/core/payments/operations/payment_update.rs

@@ -2,13 +2,14 @@ use std::marker::PhantomData;
 
 use async_trait::async_trait;
 use error_stack::{report, ResultExt};
+use masking::Secret;
 use router_derive::PaymentOperation;
 use router_env::{instrument, tracing};
 
 use super::{BoxedOperation, Domain, GetTracker, Operation, UpdateTracker, ValidateRequest};
 use crate::{
     core::{
-        errors::{self, RouterResult, StorageErrorExt},
+        errors::{self, CustomResult, RouterResult, StorageErrorExt},
         payments::{self, helpers, operations, CustomerDetails, PaymentAddress, PaymentData},
         utils as core_utils,
     },
@@ -168,6 +169,79 @@ impl<F: Send + Clone> GetTracker<F, PaymentData<F>, api::PaymentsRequest> for Pa
     }
 }
 
+#[async_trait]
+impl<F: Clone + Send> Domain<F, api::PaymentsRequest> for PaymentUpdate {
+    #[instrument(skip_all)]
+    async fn get_or_create_customer_details<'a>(
+        &'a self,
+        db: &dyn StorageInterface,
+        payment_data: &mut PaymentData<F>,
+        request: Option<CustomerDetails>,
+        merchant_id: &str,
+    ) -> CustomResult<
+        (
+            BoxedOperation<'a, F, api::PaymentsRequest>,
+            Option<storage::Customer>,
+        ),
+        errors::StorageError,
+    > {
+        helpers::create_customer_if_not_exist(
+            Box::new(self),
+            db,
+            payment_data,
+            request,
+            merchant_id,
+        )
+        .await
+    }
+
+    #[instrument(skip_all)]
+    async fn make_pm_data<'a>(
+        &'a self,
+        state: &'a AppState,
+        payment_method: Option<enums::PaymentMethodType>,
+        txn_id: &str,
+        payment_attempt: &storage::PaymentAttempt,
+        request: &Option<api::PaymentMethod>,
+        token: &Option<String>,
+        card_cvc: Option<Secret<String>>,
+        _storage_scheme: enums::MerchantStorageScheme,
+    ) -> RouterResult<(
+        BoxedOperation<'a, F, api::PaymentsRequest>,
+        Option<api::PaymentMethod>,
+        Option<String>,
+    )> {
+        helpers::make_pm_data(
+            Box::new(self),
+            state,
+            payment_method,
+            txn_id,
+            payment_attempt,
+            request,
+            token,
+            card_cvc,
+        )
+        .await
+    }
+
+    #[instrument(skip_all)]
+    async fn add_task_to_process_tracker<'a>(
+        &'a self,
+        state: &'a AppState,
+        payment_attempt: &storage::PaymentAttempt,
+    ) -> CustomResult<(), errors::ApiErrorResponse> {
+        helpers::add_domain_task_to_pt(self, state, payment_attempt).await
+    }
+
+    async fn get_connector<'a>(
+        &'a self,
+        merchant_account: &storage::MerchantAccount,
+        state: &AppState,
+    ) -> CustomResult<api::ConnectorCallType, errors::ApiErrorResponse> {
+        helpers::get_connector_default(merchant_account, state).await
+    }
+}
+
 #[async_trait]
 impl<F: Clone> UpdateTracker<F, PaymentData<F>, api::PaymentsRequest> for PaymentUpdate {
     #[instrument(skip_all)]