refactor: extract email validation and PII utils to common_utils crate (#72)

Cargo.lock

@@ -897,11 +897,16 @@ version = "0.1.0"
 dependencies = [
  "bytes",
  "error-stack",
+ "fake",
  "masking",
+ "once_cell",
+ "proptest",
+ "regex",
  "router_env",
  "serde",
  "serde_json",
  "serde_urlencoded",
+ "thiserror",
  "time",
 ]
 
@@ -2625,7 +2630,6 @@ dependencies = [
  "dyn-clone",
  "encoding_rs",
  "error-stack",
- "fake",
  "fred",
  "futures",
  "hex",
@@ -2637,7 +2641,6 @@ dependencies = [
  "mime",
  "nanoid",
  "once_cell",
- "proptest",
  "rand",
  "redis_interface",
  "regex",

crates/common_utils/Cargo.toml

@@ -7,11 +7,18 @@ edition = "2021"
 [dependencies]
 bytes = "1.2.1"
 error-stack = "0.2.1"
+once_cell = "1.16.0"
+regex = "1.7.0"
 serde = { version = "1.0.145", features = ["derive"] }
 serde_json = "1.0.85"
 serde_urlencoded = "0.7.1"
+thiserror = "1.0.37"
 time = { version = "0.3.17", features = ["serde", "serde-well-known", "std"] }
 
 # First party crates
 masking = { version = "0.1.0", path = "../masking" }
 router_env = { version = "0.1.0", path = "../router_env", features = ["log_extra_implicit_fields", "log_custom_entries_to_extra"] }
+
+[dev-dependencies]
+fake = "2.5.0"
+proptest = "1.0.0"

crates/common_utils/src/errors.rs

@@ -1,9 +1,8 @@
-//!
+//! Errors and error specific types for universal use
-//! errors and error specific types for universal use
 
 /// Custom Result
 /// A custom datatype that wraps the error variant <E> into a report, allowing
-/// error_stack::Report<E> specific extendability  
+/// error_stack::Report<E> specific extendability
 ///
 /// Effectively, equivalent to `Result<T, error_stack::Report<E>>`
 ///
@@ -38,3 +37,20 @@ macro_rules! impl_error_type {
 }
 
 impl_error_type!(ParsingError, "Parsing error");
+
+/// Validation errors.
+#[allow(missing_docs)] // Only to prevent warnings about struct fields not being documented
+#[derive(Debug, thiserror::Error)]
+pub enum ValidationError {
+    /// The provided input is missing a required field.
+    #[error("Missing required field: {field_name}")]
+    MissingRequiredField { field_name: String },
+
+    /// An incorrect value was provided for the field specified by `field_name`.
+    #[error("Incorrect value provided for field: {field_name}")]
+    IncorrectValueProvided { field_name: &'static str },
+
+    /// An invalid input was provided.
+    #[error("{message}")]
+    InvalidValue { message: String },
+}

crates/common_utils/src/lib.rs

@@ -16,6 +16,8 @@
 pub mod custom_serde;
 pub mod errors;
 pub mod ext_traits;
+pub mod pii;
+pub mod validation;
 
 /// Date-time utilities.
 pub mod date_time {

crates/common_utils/src/pii.rs

@@ -1,14 +1,13 @@
-//!
 //! Personal Identifiable Information protection.
-//!
 
 use std::{convert::AsRef, fmt};
 
-#[doc(inline)]
+use masking::{Strategy, WithType};
-pub use masking::*;
 
-use crate::utils::validate_email;
+use crate::validation::validate_email;
 
+/// Card number
+#[derive(Debug)]
 pub struct CardNumber;
 
 impl<T> Strategy<T> for CardNumber
@@ -22,32 +21,42 @@ where
             return WithType::fmt(val, f);
         }
 
-        write!(f, "{}{}", &val_str[..6], "*".repeat(val_str.len() - 6))
+        f.write_str(&format!(
+            "{}{}",
+            &val_str[..6],
+            "*".repeat(val_str.len() - 6)
+        ))
     }
 }
 
-//pub struct PhoneNumber;
+/*
+/// Phone number
+#[derive(Debug)]
+pub struct PhoneNumber;
 
-//impl<T> Strategy<T> for PhoneNumber
+impl<T> Strategy<T> for PhoneNumber
-//where
+where
-//T: AsRef<str>,
+    T: AsRef<str>,
-//{
+{
-//fn fmt(val: &T, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+    fn fmt(val: &T, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-//let val_str: &str = val.as_ref();
+        let val_str: &str = val.as_ref();
 
-//if val_str.len() < 10 || val_str.len() > 12 {
+        if val_str.len() < 10 || val_str.len() > 12 {
-//return WithType::fmt(val, f);
+            return WithType::fmt(val, f);
-//}
+        }
 
-//f.write_str(&format!(
+        f.write_str(&format!(
-//"{}{}{}",
+            "{}{}{}",
-//&val_str[..2],
+            &val_str[..2],
-//"*".repeat(val_str.len() - 5),
+            "*".repeat(val_str.len() - 5),
-//&val_str[(val_str.len() - 3)..]
+            &val_str[(val_str.len() - 3)..]
-//))
+        ))
-//}
+    }
-//}
+}
+*/
 
+/// Email address
+#[derive(Debug)]
 pub struct Email;
 
 impl<T> Strategy<T> for Email
@@ -62,14 +71,17 @@ where
             return WithType::fmt(val, f);
         }
 
-        if let Some((a, b)) = val_str.split_once('@') {
+        let parts: Vec<&str> = val_str.split('@').collect();
-            write!(f, "{}@{}", "*".repeat(a.len()), b)
+        if parts.len() != 2 {
-        } else {
+            return WithType::fmt(val, f);
-            WithType::fmt(val, f)
         }
+
+        f.write_str(&format!("{}@{}", "*".repeat(parts[0].len()), parts[1]))
     }
 }
 
+/// IP address
+#[derive(Debug)]
 pub struct IpAddress;
 
 impl<T> Strategy<T> for IpAddress
@@ -90,13 +102,15 @@ where
             }
         }
 
-        write!(f, "{}.**.**.**", segments[0])
+        f.write_str(&format!("{}.**.**.**", segments[0]))
     }
 }
 
 #[cfg(test)]
 mod pii_masking_strategy_tests {
-    use super::{CardNumber, Email, IpAddress, Secret};
+    use masking::Secret;
+
+    use super::{CardNumber, Email, IpAddress};
 
     #[test]
     fn test_valid_card_number_masking() {
@@ -110,7 +124,8 @@ mod pii_masking_strategy_tests {
         assert_eq!("123456****", &format!("{:?}", secret));
     }
 
-    /* #[test]
+    /*
+    #[test]
     fn test_valid_phone_number_masking() {
         let secret: Secret<String, PhoneNumber> = Secret::new("9922992299".to_string());
         assert_eq!("99*****299", &format!("{}", secret));
@@ -123,7 +138,8 @@ mod pii_masking_strategy_tests {
 
         let secret: Secret<String, PhoneNumber> = Secret::new("9922992299229922".to_string());
         assert_eq!("*** alloc::string::String ***", &format!("{}", secret));
-    } */
+    }
+    */
 
     #[test]
     fn test_valid_email_masking() {

crates/common_utils/src/validation.rs

@@ -0,0 +1,102 @@
+//! Custom validations for some shared types.
+
+use error_stack::report;
+use once_cell::sync::Lazy;
+use regex::Regex;
+use router_env::logger;
+
+use crate::errors::{CustomResult, ValidationError};
+
+/// Performs a simple validation against a provided email address.
+pub fn validate_email(email: &str) -> CustomResult<(), ValidationError> {
+    #[deny(clippy::invalid_regex)]
+    static EMAIL_REGEX: Lazy<Option<Regex>> = Lazy::new(|| {
+        match Regex::new(
+            r"^(?i)[a-z0-9.!#$%&'*+/=?^_`{|}~-]+@[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?(?:\.[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?)*$",
+        ) {
+            Ok(regex) => Some(regex),
+            Err(error) => {
+                logger::error!(?error);
+                None
+            }
+        }
+    });
+    let email_regex = match EMAIL_REGEX.as_ref() {
+        Some(regex) => Ok(regex),
+        None => Err(report!(ValidationError::InvalidValue {
+            message: "Invalid regex expression".into()
+        })),
+    }?;
+
+    const EMAIL_MAX_LENGTH: usize = 319;
+    if email.is_empty() || email.chars().count() > EMAIL_MAX_LENGTH {
+        return Err(report!(ValidationError::InvalidValue {
+            message: "Email address is either empty or exceeds maximum allowed length".into()
+        }));
+    }
+
+    if !email_regex.is_match(email) {
+        return Err(report!(ValidationError::InvalidValue {
+            message: "Invalid email address format".into()
+        }));
+    }
+
+    Ok(())
+}
+
+#[cfg(test)]
+mod tests {
+    use fake::{faker::internet::en::SafeEmail, Fake};
+    use proptest::{
+        prop_assert,
+        strategy::{Just, NewTree, Strategy},
+        test_runner::TestRunner,
+    };
+
+    use super::*;
+
+    #[derive(Debug)]
+    struct ValidEmail;
+
+    impl Strategy for ValidEmail {
+        type Tree = Just<String>;
+        type Value = String;
+
+        fn new_tree(&self, _runner: &mut TestRunner) -> NewTree<Self> {
+            Ok(Just(SafeEmail().fake()))
+        }
+    }
+
+    #[test]
+    fn test_validate_email() {
+        let result = validate_email("abc@example.com");
+        assert!(result.is_ok());
+
+        let result = validate_email("abc+123@example.com");
+        assert!(result.is_ok());
+
+        let result = validate_email("");
+        assert!(result.is_err());
+    }
+
+    proptest::proptest! {
+        /// Example of unit test
+        #[test]
+        fn proptest_valid_fake_email(email in ValidEmail) {
+            prop_assert!(validate_email(&email).is_ok());
+        }
+
+        /// Example of unit test
+        #[test]
+        fn proptest_invalid_data_email(email in "\\PC*") {
+            prop_assert!(validate_email(&email).is_err());
+        }
+
+        // TODO: make maybe unit test working
+        // minimal failing input: email = "+@a"
+        // #[test]
+        // fn proptest_invalid_email(email in "[.+]@(.+)") {
+        //     prop_assert!(validate_email(&email).is_err());
+        // }
+    }
+}

crates/router/Cargo.toml

@@ -75,15 +75,13 @@ router_env = { version = "0.1.0", path = "../router_env", features = ["log_extra
 router_env = { version = "0.1.0", path = "../router_env", default-features = false, features = ["vergen"] }
 
 [dev-dependencies]
+actix-http = "3.2.2"
 awc = { version = "3.0.1", features = ["rustls"] }
+derive_deref = "1.1.1"
+rand = "0.8.5"
 time = { version = "0.3.14", features = ["macros"] }
 tokio = "1.21.2"
 toml = "0.5.9"
-derive_deref = "1.1.1"
-actix-http = "3.2.2"
-proptest = "1.0"
-fake = "2.5"
-rand = "0.8"
 
 [[bin]]
 name = "router"

crates/router/src/connector/braintree/transformers.rs

@@ -40,7 +40,7 @@ pub struct Card {
 }
 
 impl TryFrom<&types::PaymentsAuthorizeRouterData> for BraintreePaymentsRequest {
-    type Error = error_stack::Report<errors::ValidateError>;
+    type Error = error_stack::Report<errors::ConnectorError>;
     fn try_from(item: &types::PaymentsAuthorizeRouterData) -> Result<Self, Self::Error> {
         match item.request.payment_method_data {
             api::PaymentMethod::Card(ref ccard) => {
@@ -62,7 +62,7 @@ impl TryFrom<&types::PaymentsAuthorizeRouterData> for BraintreePaymentsRequest {
                     transaction: braintree_payment_request,
                 })
             }
-            _ => Err(errors::ValidateError.into()),
+            _ => Err(errors::ConnectorError::RequestEncodingFailed.into()),
         }
     }
 }

crates/router/src/core/errors.rs

@@ -5,7 +5,7 @@ pub(crate) mod utils;
 use std::fmt::Display;
 
 use actix_web::{body::BoxBody, http::StatusCode, HttpResponse, ResponseError};
-pub use common_utils::errors::{CustomResult, ParsingError};
+pub use common_utils::errors::{CustomResult, ParsingError, ValidationError};
 use config::ConfigError;
 use error_stack;
 pub use redis_interface::errors::RedisError;
@@ -409,16 +409,6 @@ error_to_process_tracker_error!(
     ProcessTrackerError::EValidationError(error_stack::Report<ValidationError>)
 );
 
-#[derive(Debug, thiserror::Error)]
-pub enum ValidationError {
-    #[error("Missing required field: {field_name}")]
-    MissingRequiredField { field_name: String },
-    #[error("Incorrect value provided for field: {field_name}")]
-    IncorrectValueProvided { field_name: &'static str },
-    #[error("{message}")]
-    InvalidValue { message: String },
-}
-
 #[derive(Debug, thiserror::Error)]
 pub enum WebhooksFlowError {
     #[error("Merchant webhook config not found")]

crates/router/src/lib.rs

@@ -27,7 +27,6 @@ pub mod core;
 pub mod cors;
 pub mod db;
 pub mod env;
-pub mod pii;
 pub mod routes;
 pub mod scheduler;
 
@@ -61,6 +60,14 @@ pub mod headers {
     pub const X_API_VERSION: &str = "X-ApiVersion";
 }
 
+pub mod pii {
+    //! Personal Identifiable Information protection.
+
+    pub(crate) use common_utils::pii::{CardNumber, Email, IpAddress};
+    #[doc(inline)]
+    pub use masking::*;
+}
+
 pub fn mk_app(
     state: AppState,
     request_body_limit: usize,

crates/router/src/utils.rs

@@ -5,11 +5,14 @@ mod fp_utils;
 #[cfg(feature = "kv_store")]
 pub(crate) mod storage_partitioning;
 
-pub(crate) use common_utils::ext_traits::{ByteSliceExt, BytesExt, Encode, StringExt, ValueExt};
+pub(crate) use common_utils::{
+    ext_traits::{ByteSliceExt, BytesExt, Encode, StringExt, ValueExt},
+    validation::validate_email,
+};
 use nanoid::nanoid;
 
 pub(crate) use self::{
-    ext_traits::{validate_address, validate_email, OptionExt, ValidateCall},
+    ext_traits::{validate_address, OptionExt, ValidateCall},
     fp_utils::when,
 };
 use crate::consts;

crates/router/src/utils/ext_traits.rs

@@ -1,11 +1,8 @@
 use common_utils::ext_traits::ValueExt;
 use error_stack::{report, IntoReport, Report, ResultExt};
-use once_cell::sync::Lazy;
-use regex::Regex;
 
 use crate::{
     core::errors::{self, ApiErrorResponse, CustomResult, RouterResult},
-    logger,
     types::api::AddressDetails,
     utils::when,
 };
@@ -132,42 +129,6 @@ where
     }
 }
 
-pub fn validate_email(email: &str) -> CustomResult<(), errors::ValidationError> {
-    #[deny(clippy::invalid_regex)]
-    static EMAIL_REGEX: Lazy<Option<Regex>> = Lazy::new(|| {
-        match Regex::new(
-            r"^(?i)[a-z0-9.!#$%&'*+/=?^_`{|}~-]+@[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?(?:\.[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?)*$",
-        ) {
-            Ok(regex) => Some(regex),
-            Err(error) => {
-                logger::error!(?error);
-                None
-            }
-        }
-    });
-    let email_regex = match EMAIL_REGEX.as_ref() {
-        Some(regex) => Ok(regex),
-        None => Err(report!(errors::ValidationError::InvalidValue {
-            message: "Invalid regex expression".into()
-        })),
-    }?;
-
-    const EMAIL_MAX_LENGTH: usize = 319;
-    if email.is_empty() || email.chars().count() > EMAIL_MAX_LENGTH {
-        return Err(report!(errors::ValidationError::InvalidValue {
-            message: "Email address is either empty or exceeds maximum allowed length".into()
-        }));
-    }
-
-    if !email_regex.is_match(email) {
-        return Err(report!(errors::ValidationError::InvalidValue {
-            message: "Invalid email address format".into()
-        }));
-    }
-
-    Ok(())
-}
-
 pub fn validate_address(address: &serde_json::Value) -> CustomResult<(), errors::ValidationError> {
     if let Err(err) = serde_json::from_value::<AddressDetails>(address.clone()) {
         return Err(report!(errors::ValidationError::InvalidValue {
@@ -176,62 +137,3 @@ pub fn validate_address(address: &serde_json::Value) -> CustomResult<(), errors:
     }
     Ok(())
 }
-
-#[cfg(test)]
-mod tests {
-    use fake::{faker::internet::en::SafeEmail, Fake};
-    use proptest::{
-        prop_assert,
-        strategy::{Just, NewTree, Strategy},
-        test_runner::TestRunner,
-    };
-
-    use super::*;
-
-    #[derive(Debug)]
-    struct ValidEmail;
-
-    impl Strategy for ValidEmail {
-        type Tree = Just<String>;
-        type Value = String;
-
-        fn new_tree(&self, _runner: &mut TestRunner) -> NewTree<Self> {
-            Ok(Just(SafeEmail().fake()))
-        }
-    }
-
-    #[test]
-    fn test_validate_email() {
-        let result = validate_email("abc@example.com");
-        assert!(result.is_ok());
-
-        let result = validate_email("abc+123@example.com");
-        assert!(result.is_ok());
-
-        let result = validate_email("");
-        assert!(result.is_err());
-    }
-
-    proptest::proptest! {
-        /// Example of unit test
-        /// Kind of test: output-based testing
-        #[test]
-        fn proptest_valid_fake_email(email in ValidEmail) {
-            prop_assert!(validate_email(&email).is_ok());
-        }
-
-        /// Example of unit test
-        /// Kind of test: output-based testing
-        #[test]
-        fn proptest_invalid_data_email(email in "\\PC*") {
-            prop_assert!(validate_email(&email).is_err());
-        }
-
-        // TODO: make maybe unit test working
-        // minimal failing input: email = "+@a"
-        // #[test]
-        // fn proptest_invalid_email(email in "[.+]@(.+)") {
-        //     prop_assert!(validate_email(&email).is_err());
-        // }
-    }
-}