-
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(payout_link): secure payout links using server side validations and client side headers #5219
Conversation
feat(generic_link): add allowed_domains in GenericLink response feat(payout_link): add allowed_domains in payout_link_config (fallback profile config + payout request config) feat(payout_link): consume meta headers for validating render request for payout links feat(payout_link): consume CSP and X-Frame headers in the link's response
feat: add validator for generic link configs
crates/hyperswitch_domain_models/src/errors/api_error_response.rs
Outdated
Show resolved
Hide resolved
crates/hyperswitch_domain_models/src/errors/api_error_response.rs
Outdated
Show resolved
Hide resolved
None => Err(report!(errors::ApiErrorResponse::AccessForbidden { | ||
resource: "payout_link".to_string(), | ||
})) | ||
.attach_printable_lazy(|| { | ||
format!( | ||
"Access to payout_link [{}] is forbidden when sec-fetch-dest is not present in request headers", |
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 a seperate forked UI for these failures doesn't look good throwing json error in user flow
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.
For later
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.
There seems to be a bug in the payout link configs, since we are focusing on json. if one of the field is not sent in payment create and is set in business config, we will make the not sent value as None. Which is not the expected behavior i suppose.
crates/hyperswitch_domain_models/src/errors/api_error_response.rs
Outdated
Show resolved
Hide resolved
crates/hyperswitch_domain_models/src/errors/api_error_response.rs
Outdated
Show resolved
Hide resolved
chore(links): add migration query for payout links
Type of Change
Description
This PR includes below changes -
allowed_domains
field in payout link configallowed_domains
are not passed in payout link's create requestSec-Fetch-Dest
[ref] for request source (must beiframe
)Origin / Referer
for the host of the incoming request (must match one inallowed_domains
)allowed_domains
is not setup (only in non-production env)frame-ancestors
for directing which hosts can embed the links in an iframe [ref]attach X-Frame header - similar to CSP, but for older browsers [ref]Removing this as it's outdatedfeat(generic_link): add allowed_domains in GenericLink response
feat(payout_link): add allowed_domains in payout_link_config (fallback profile config + payout request config)
feat(payout_link): consume meta headers for validating render request for payout links
feat(payout_link): consume CSP and X-Frame headers in the link's response
Additional Changes
Motivation and Context
How did you test it?
Tested locally using postman
payout_link_config
to includeallowed_domains
allowed_domains
and the iframe's host is same)Test cases
Checklist
cargo +nightly fmt --all
cargo clippy