-
Notifications
You must be signed in to change notification settings - Fork 604
[DTINAPPXO-3265][DTINAPPXO-3391] Resume Flow fix: Extract query-style params embedded in the hash fragment #2599
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,13 +16,67 @@ export type AppSwitchResumeParams = {| | |
| checkoutState: "onApprove" | "onCancel" | "onError", | ||
| |}; | ||
|
|
||
| // When the merchant's return_url contains a hash fragment (e.g. /checkout/#payment), | ||
| // PayPal params (token, PayerID) end up inside the hash as /checkout/#payment?token=...&PayerID=... | ||
| // This helper splits the hash into its name and query-string parts, | ||
| // checking for ? delimiter first, then falling back to &. | ||
| function parseHashFragment(): {| hash: string, queryString: string |} { | ||
| const hashString = | ||
| window.location.hash && String(window.location.hash).slice(1); | ||
| if (!hashString) { | ||
| return { hash: "", queryString: "" }; | ||
| } | ||
|
|
||
| // Check for ? delimiter first (e.g. #payment?token=...) | ||
| const questionMarkIndex = hashString.indexOf("?"); | ||
| if (questionMarkIndex !== -1) { | ||
| return { | ||
| hash: hashString.slice(0, questionMarkIndex), | ||
| queryString: hashString.slice(questionMarkIndex + 1), | ||
| }; | ||
| } | ||
|
|
||
| // Fallback to & delimiter (e.g. #payment&token=...) | ||
| const ampersandIndex = hashString.indexOf("&"); | ||
| if (ampersandIndex !== -1) { | ||
| return { | ||
| hash: hashString.slice(0, ampersandIndex), | ||
| queryString: hashString.slice(ampersandIndex + 1), | ||
| }; | ||
| } | ||
|
|
||
| return { hash: hashString, queryString: "" }; | ||
| } | ||
|
|
||
| function getParamsFromHashFragment(): { [string]: string } { | ||
| const { queryString } = parseHashFragment(); | ||
| if (!queryString) { | ||
| return {}; | ||
| } | ||
| // eslint-disable-next-line compat/compat | ||
| return Object.fromEntries(new URLSearchParams(queryString)); | ||
| } | ||
|
|
||
| // The Web fallback flow uses different set of query params then appswitch flow. | ||
| function getAppSwitchParamsWebFallback(): AppSwitchResumeParams | null { | ||
| try { | ||
| const params = Object.fromEntries( | ||
| const searchParams = Object.fromEntries( | ||
| // eslint-disable-next-line compat/compat | ||
| new URLSearchParams(window.location.search) | ||
| ); | ||
|
|
||
| // If no PayPal params found in query string, check if they are embedded | ||
| // inside the hash fragment. This happens when the merchant's return_url | ||
| // contains a hash (e.g. /checkout/#payment) and PayPal params were appended | ||
| // after the fragment: /checkout/#payment?token=...&PayerID=... | ||
| const params = | ||
| searchParams.token || | ||
| searchParams.vaultSetupToken || | ||
| searchParams.approval_token_id || | ||
| searchParams.approval_session_id | ||
| ? searchParams | ||
| : { ...getParamsFromHashFragment(), ...searchParams }; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Potential false positive for merchant hash params This new fallback to getParamsFromHashFragment() introduces a risk: if a merchant's SPA uses token in their own hash fragment (e.g. https://merchant.com/shop#products?token=my-cart-token), this will now be detected as a resume flow and trigger onCancel. On main this was safe — the web fallback only read window.location.search, so merchant hash params were ignored. With this change, any page with token= or a vault param in the hash will match.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @nikrom17 This isn't a new risk introduced by this change - the same issue already exists with query params today. If a merchant uses a query param named
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I created a ticket to handle this issue DTINAPPXO-3434 |
||
|
|
||
| const { | ||
| button_session_id: buttonSessionID, | ||
| fundingSource, | ||
|
|
@@ -57,31 +111,11 @@ function getAppSwitchParamsWebFallback(): AppSwitchResumeParams | null { | |
| } | ||
|
|
||
| export function getAppSwitchResumeParams(): AppSwitchResumeParams | null { | ||
| const hashString = | ||
| window.location.hash && String(window.location.hash).slice(1); | ||
| if (!hashString) { | ||
| const { hash, queryString } = parseHashFragment(); | ||
| if (!hash) { | ||
| return getAppSwitchParamsWebFallback(); | ||
| } | ||
|
|
||
| let hash = ""; | ||
| let queryString = ""; | ||
|
|
||
| // first check for ? as the hash/query separator | ||
| const questionMarkIndex = hashString.indexOf("?"); | ||
|
|
||
| if (questionMarkIndex !== -1) { | ||
| [hash, queryString] = hashString.split("?"); | ||
| } else { | ||
| const ampersandIndex = hashString.indexOf("&"); | ||
|
|
||
| if (ampersandIndex !== -1) { | ||
| hash = hashString.slice(0, ampersandIndex); | ||
| queryString = hashString.slice(ampersandIndex + 1); | ||
| } else { | ||
| hash = hashString; | ||
| } | ||
| } | ||
|
|
||
| const isPostApprovalAction = [ | ||
| APP_SWITCH_RETURN_HASH.ONAPPROVE, | ||
| APP_SWITCH_RETURN_HASH.ONCANCEL, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.