Skip to content
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(payout_link): secure payout links using server side validations and client side headers #5219

Merged
merged 23 commits into from
Jul 17, 2024
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
11a4916
feat(api): update html response to include headers if needed
kashif-m Jul 5, 2024
b316f47
Merge remote-tracking branch 'origin/main'
kashif-m Jul 5, 2024
68651b8
docs(openapi): re-generate OpenAPI specification
hyperswitch-bot[bot] Jul 5, 2024
d805a34
refactor: clippy fixes
kashif-m Jul 5, 2024
c2454b2
chore: run formatter
hyperswitch-bot[bot] Jul 5, 2024
b943498
Merge remote-tracking branch 'origin/main' into link_sec
kashif-m Jul 8, 2024
cc2ddc3
refactor: clippy fixes
kashif-m Jul 8, 2024
54e0b2c
Merge remote-tracking branch 'origin/main' into link_sec
kashif-m Jul 8, 2024
0c395f0
refactor: hacks fixes
kashif-m Jul 8, 2024
118dfc4
refactor: hack fixes
kashif-m Jul 9, 2024
cf7026c
Merge remote-tracking branch 'origin/main' into link_sec
kashif-m Jul 9, 2024
560371c
feat: attach client side JS to restrict loading the payout SDK
kashif-m Jul 11, 2024
f2d2c42
refactor(payout_link): enforce allowed_domains for payout links
kashif-m Jul 12, 2024
db20cb5
Merge remote-tracking branch 'origin/main' into link_sec
kashif-m Jul 12, 2024
247ae45
docs(openapi): re-generate OpenAPI specification
hyperswitch-bot[bot] Jul 12, 2024
3a444cb
refactor(links): remove x-frame-options as it's obsolete
kashif-m Jul 12, 2024
7f6377e
refactor(payout_link): enhance code by resolving comments
kashif-m Jul 16, 2024
fc0440c
refactor(docs): regenerate openAPI specs
kashif-m Jul 16, 2024
8ef79fe
chore(links): add unit test cases for domain regexes
kashif-m Jul 17, 2024
691d3e4
Merge remote-tracking branch 'origin/main' into link_sec
kashif-m Jul 17, 2024
a08ba9c
chore(links): update migration down query
kashif-m Jul 17, 2024
05aa948
refactor(payout_link): update domain validation for payout link's ren…
kashif-m Jul 17, 2024
74689eb
refactor(payout_link): reorder LinkConfigurationError
kashif-m Jul 17, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 11 additions & 0 deletions api-reference/openapi_spec.json
Original file line number Diff line number Diff line change
Expand Up @@ -6809,11 +6809,22 @@
},
{
"type": "object",
"required": [
"allowed_domains"
],
"properties": {
"domain_name": {
"type": "string",
"description": "Custom domain name to be used for hosting the link",
"nullable": true
},
"allowed_domains": {
"type": "array",
"items": {
"type": "string"
},
"description": "A list of allowed domains (glob patterns) where this link can be embedded / opened from",
"uniqueItems": true
}
}
}
Expand Down
30 changes: 29 additions & 1 deletion crates/api_models/src/admin.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::collections::HashMap;
use std::collections::{HashMap, HashSet};

#[cfg(feature = "v2")]
use common_utils::new_type;
Expand Down Expand Up @@ -1332,11 +1332,39 @@ pub struct BusinessGenericLinkConfig {
/// Custom domain name to be used for hosting the link
pub domain_name: Option<String>,

/// A list of allowed domains (glob patterns) where this link can be embedded / opened from
pub allowed_domains: HashSet<String>,

#[serde(flatten)]
#[schema(value_type = GenericLinkUiConfig)]
pub ui_config: link_utils::GenericLinkUiConfig,
}

impl BusinessGenericLinkConfig {
pub fn validate(&self) -> Result<(), &str> {
// Validate host domain name
let host_domain_valid = self
.domain_name
.clone()
.map(|host_domain| link_utils::validate_strict_domain(&host_domain))
.unwrap_or(true);
if !host_domain_valid {
return Err("Invalid host domain name received");
}

let are_allowed_domains_valid = self
.allowed_domains
.clone()
.iter()
.all(|allowed_domain| link_utils::validate_wildcard_domain(allowed_domain));
if !are_allowed_domains_valid {
return Err("Invalid allowed domain names received");
}

Ok(())
}
}

#[derive(Clone, Debug, serde::Deserialize, serde::Serialize, PartialEq, ToSchema)]
pub struct BusinessPaymentLinkConfig {
/// Custom domain name to be used for hosting the link in your own domain
Expand Down
16 changes: 16 additions & 0 deletions crates/common_utils/src/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,5 +99,21 @@ pub const MAX_ALLOWED_MERCHANT_REFERENCE_ID_LENGTH: u8 = 64;
/// Minimum allowed length for MerchantReferenceId
pub const MIN_REQUIRED_MERCHANT_REFERENCE_ID_LENGTH: u8 = 1;

/// Regex for matching a domain
/// Eg -
/// http://www.example.com
/// https://www.example.com
/// www.example.com
/// example.io
pub const STRICT_DOMAIN_REGEX: &str = r"^((http)s?://)?([A-Za-z0-9]{1,63}\.?)+([A-Za-z0-9-]{1,63}\.?)+([A-Za-z0-9]{1,63}\.?)+(\.[A-Za-z]{2,6}|:[0-9]{1,4})?$";

/// Regex for matching a wildcard domain
/// Eg -
/// *.example.com
/// *.subdomain.domain.com
/// *://example.com
/// *example.com
pub const WILDCARD_DOMAIN_REGEX: &str = r"^((((http)s|\*)?://)?([A-Za-z0-9]{1,63}\.?)+[A-Za-z0-9-*]{1,63}\.?)+([A-Za-z0-9]{1,63}\.?)+(\.[A-Za-z]{2,6}|:[0-9*]{1,4})?$";

/// Maximum allowed length for MerchantName
pub const MAX_ALLOWED_MERCHANT_NAME_LENGTH: usize = 64;
35 changes: 33 additions & 2 deletions crates/common_utils/src/link_utils.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//! Common
//! This module has common utilities for links in HyperSwitch

use std::{collections::HashSet, primitive::i64};

Expand All @@ -13,10 +13,13 @@ use diesel::{
};
use error_stack::{report, ResultExt};
use masking::Secret;
use regex::Regex;
#[cfg(feature = "logs")]
use router_env::logger;
use serde::Serialize;
use utoipa::ToSchema;

use crate::{errors::ParsingError, id_type, types::MinorUnit};
use crate::{consts, errors::ParsingError, id_type, types::MinorUnit};

#[derive(
Serialize, serde::Deserialize, Debug, Clone, Eq, PartialEq, FromSqlRow, AsExpression, ToSchema,
Expand Down Expand Up @@ -162,6 +165,8 @@ pub struct PayoutLinkData {
pub amount: MinorUnit,
/// Payout currency
pub currency: enums::Currency,
/// A list of allowed domains (glob patterns) where this link can be embedded / opened from
pub allowed_domains: HashSet<String>,
}

crate::impl_to_sql_from_sql_json!(PayoutLinkData);
Expand Down Expand Up @@ -209,3 +214,29 @@ pub struct EnabledPaymentMethod {
#[schema(value_type = HashSet<PaymentMethodType>)]
pub payment_method_types: HashSet<enums::PaymentMethodType>,
}

/// Util function for validating a domain without any wildcard characters.
pub fn validate_strict_domain(domain: &str) -> bool {
Regex::new(consts::STRICT_DOMAIN_REGEX)
kashif-m marked this conversation as resolved.
Show resolved Hide resolved
.map(|regex| regex.is_match(domain))
.map_err(|err| {
let err_msg = format!("Invalid strict domain regex: {err:?}");
#[cfg(feature = "logs")]
logger::error!(err_msg);
err_msg
})
.unwrap_or(false)
}

/// Util function for validating a domain with "*" wildcard characters.
pub fn validate_wildcard_domain(domain: &str) -> bool {
Regex::new(consts::WILDCARD_DOMAIN_REGEX)
.map(|regex| regex.is_match(domain))
.map_err(|err| {
let err_msg = format!("Invalid strict domain regex: {err:?}");
#[cfg(feature = "logs")]
logger::error!(err_msg);
err_msg
})
.unwrap_or(false)
}
14 changes: 10 additions & 4 deletions crates/hyperswitch_domain_models/src/api.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::fmt::Display;
use std::{collections::HashSet, fmt::Display};

use common_utils::{
events::{ApiEventMetric, ApiEventsType},
Expand Down Expand Up @@ -59,20 +59,26 @@ pub struct PaymentLinkStatusData {
}

#[derive(Debug, Eq, PartialEq)]
pub enum GenericLinks {
pub struct GenericLinks {
pub allowed_domains: HashSet<String>,
pub data: GenericLinksData,
}

#[derive(Debug, Eq, PartialEq)]
pub enum GenericLinksData {
ExpiredLink(GenericExpiredLinkData),
PaymentMethodCollect(GenericLinkFormData),
PayoutLink(GenericLinkFormData),
PayoutLinkStatus(GenericLinkStatusData),
PaymentMethodCollectStatus(GenericLinkStatusData),
}

impl Display for GenericLinks {
impl Display for GenericLinksData {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(
f,
"{}",
match self {
match *self {
Self::ExpiredLink(_) => "ExpiredLink",
Self::PaymentMethodCollect(_) => "PaymentMethodCollect",
Self::PayoutLink(_) => "PayoutLink",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,8 @@ pub enum ApiErrorResponse {
MissingFilePurpose,
#[error(error_type = ErrorType::InvalidRequestError, code = "HE_04", message = "File content type not found / valid")]
MissingFileContentType,
#[error(error_type = ErrorType::InvalidRequestError, code = "IR_28", message = "{message}")]
GenericConfigurationError { message: String },
kashif-m marked this conversation as resolved.
Show resolved Hide resolved
#[error(error_type = ErrorType::InvalidRequestError, code = "HE_05", message = "{message}")]
GenericNotFoundError { message: String },
#[error(error_type = ErrorType::InvalidRequestError, code = "HE_01", message = "{message}")]
Expand Down Expand Up @@ -523,6 +525,9 @@ impl ErrorSwitch<api_models::errors::types::ApiErrorResponse> for ApiErrorRespon
Self::AddressNotFound => {
AER::NotFound(ApiError::new("HE", 4, "Address does not exist in our records", None))
},
Self::GenericConfigurationError { message } => {
kashif-m marked this conversation as resolved.
Show resolved Hide resolved
AER::BadRequest(ApiError::new("IR", 28, message, None))
},
Self::GenericNotFoundError { message } => {
AER::NotFound(ApiError::new("HE", 5, message, None))
},
Expand Down
1 change: 1 addition & 0 deletions crates/router/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ dyn-clone = "1.0.17"
encoding_rs = "0.8.33"
error-stack = "0.4.1"
futures = "0.3.30"
globset = "0.4.14"
hex = "0.4.3"
http = "0.2.12"
hyper = "0.14.28"
Expand Down
10 changes: 9 additions & 1 deletion crates/router/src/compatibility/stripe/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,9 @@ pub enum StripeErrorCode {
#[error(error_type = StripeErrorType::InvalidRequestError, code = "resource_missing", message = "No such payment method")]
PaymentMethodNotFound,

#[error(error_type = StripeErrorType::InvalidRequestError, code = "not_configured", message = "{message}")]
GenericConfigurationError { message: String },
kashif-m marked this conversation as resolved.
Show resolved Hide resolved

#[error(error_type = StripeErrorType::InvalidRequestError, code = "resource_missing", message = "{message}")]
GenericNotFoundError { message: String },

Expand Down Expand Up @@ -461,6 +464,10 @@ impl From<errors::ApiErrorResponse> for StripeErrorCode {
param: field_names.clone().join(", "),
}
}

errors::ApiErrorResponse::GenericConfigurationError { message } => {
Self::GenericConfigurationError { message }
}
errors::ApiErrorResponse::GenericNotFoundError { message } => {
Self::GenericNotFoundError { message }
}
Expand Down Expand Up @@ -742,7 +749,8 @@ impl actix_web::ResponseError for StripeErrorCode {
| Self::InvalidConnectorConfiguration { .. }
| Self::CurrencyConversionFailed
| Self::PaymentMethodDeleteFailed
| Self::ExtendedCardInfoNotFound => StatusCode::BAD_REQUEST,
| Self::ExtendedCardInfoNotFound
| Self::GenericConfigurationError { .. } => StatusCode::BAD_REQUEST,
Self::RefundFailed
| Self::PayoutFailed
| Self::PaymentLinkNotFound
Expand Down
13 changes: 7 additions & 6 deletions crates/router/src/compatibility/wrap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,10 +141,11 @@ where
}

Ok(api::ApplicationResponse::GenericLinkForm(boxed_generic_link_data)) => {
let link_type = (boxed_generic_link_data).to_string();
match services::generic_link_response::build_generic_link_html(*boxed_generic_link_data)
{
Ok(rendered_html) => api::http_response_html_data(rendered_html),
let link_type = (boxed_generic_link_data).data.to_string();
match services::generic_link_response::build_generic_link_html(
boxed_generic_link_data.data,
) {
Ok(rendered_html) => api::http_response_html_data(rendered_html, None),
Err(_) => {
api::http_response_err(format!("Error while rendering {} HTML page", link_type))
}
Expand All @@ -155,7 +156,7 @@ where
match *boxed_payment_link_data {
api::PaymentLinkAction::PaymentLinkFormData(payment_link_data) => {
match api::build_payment_link_html(payment_link_data) {
Ok(rendered_html) => api::http_response_html_data(rendered_html),
Ok(rendered_html) => api::http_response_html_data(rendered_html, None),
Err(_) => api::http_response_err(
r#"{
"error": {
Expand All @@ -167,7 +168,7 @@ where
}
api::PaymentLinkAction::PaymentLinkStatus(payment_link_data) => {
match api::get_payment_link_status(payment_link_data) {
Ok(rendered_html) => api::http_response_html_data(rendered_html),
Ok(rendered_html) => api::http_response_html_data(rendered_html, None),
Err(_) => api::http_response_err(
r#"{
"error": {
Expand Down
24 changes: 16 additions & 8 deletions crates/router/src/core/admin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1979,6 +1979,21 @@ pub async fn update_business_profile(
.transpose()?
.map(Secret::new);

let payout_link_config = request
.payout_link_config
.as_ref()
.map(|payout_conf| match payout_conf.config.validate() {
Ok(_) => payout_conf.encode_to_value().change_context(
errors::ApiErrorResponse::InvalidDataValue {
field_name: "payout_link_config",
},
),
Err(e) => Err(report!(errors::ApiErrorResponse::InvalidRequestData {
message: e.to_string()
})),
})
.transpose()?;

let business_profile_update = storage::business_profile::BusinessProfileUpdate::Update {
profile_name: request.profile_name,
modified_at: Some(date_time::now()),
Expand Down Expand Up @@ -2007,14 +2022,7 @@ pub async fn update_business_profile(
.change_context(errors::ApiErrorResponse::InvalidDataValue {
field_name: "authentication_connector_details",
})?,
payout_link_config: request
.payout_link_config
.as_ref()
.map(Encode::encode_to_value)
.transpose()
.change_context(errors::ApiErrorResponse::InvalidDataValue {
field_name: "payout_link_config",
})?,
payout_link_config,
extended_card_info_config,
use_billing_as_payment_method_billing: request.use_billing_as_payment_method_billing,
collect_shipping_details_from_wallet_connector: request
Expand Down
Loading
Loading