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.

Sign up for GitHub

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

[QL] Parse App Switch Return URL #1237

Merged
merged 26 commits into from
Apr 3, 2024
+184 −28
Merged

[QL] Parse App Switch Return URL #1237

Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
12257fb
switch BTAppContextSwitcher.sharedInstance.universalLink from String …
jaxdesmarais Mar 21, 2024
f5dfb31
parse parameters from URL in BTPayPalAppSwitchReturnURL
jaxdesmarais Mar 25, 2024
8022cfa
implement handleReturnURL; add new error cases; update method name
jaxdesmarais Mar 25, 2024
f1613c6
add BTPayPalAppSwitchReturnURL_Tests and test cases
jaxdesmarais Mar 25, 2024
cc1c934
add additonal persion to returnURL tests
jaxdesmarais Mar 25, 2024
b9171d8
stub out new tests
jaxdesmarais Mar 25, 2024
15fb13d
Merge branch 'universalLink-to-url' into parse-return-url
jaxdesmarais Mar 25, 2024
bb1aca1
add handleOpen tests
jaxdesmarais Mar 25, 2024
cdab32e
move universalLink into init
jaxdesmarais Mar 26, 2024
e261810
Merge branch 'paypal-app-switch-feature' into parse-return-url
jaxdesmarais Mar 26, 2024
cf2d05e
update unit test and docstrings
jaxdesmarais Mar 26, 2024
41ce86f
Merge branch 'paypal-app-switch-feature' into universalLink-to-url
jaxdesmarais Mar 26, 2024
b328d11
Merge branch 'universalLink-to-url' into parse-return-url
jaxdesmarais Mar 26, 2024
7ee949c
Merge branch 'paypal-app-switch-feature' into parse-return-url
jaxdesmarais Mar 26, 2024
997fa30
consolodate errors
jaxdesmarais Mar 28, 2024
2790f94
update tests
jaxdesmarais Mar 28, 2024
13f3dfd
update spelling in test
jaxdesmarais Mar 28, 2024
2a1d7d7
PR feedback: rename unknownAppSwitchError to appSwitchReturnURLPathIn…
jaxdesmarais Mar 29, 2024
0842e11
Merge branch 'paypal-app-switch-feature' into parse-return-url
jaxdesmarais Apr 2, 2024
f33b05e
PR feedback: remove unused properties in BTPayPalAppSwitchReturnURL
jaxdesmarais Apr 3, 2024
aa297db
PR feedback: rename unknown to unknownPath
jaxdesmarais Apr 3, 2024
bcbe0eb
PR feedback: swap case default with unknownPath
jaxdesmarais Apr 3, 2024
74209fc
Update Sources/BraintreePayPal/BTPayPalError.swift
jaxdesmarais Apr 3, 2024
4521520
Update UnitTests/BraintreePayPalTests/BTPayPalAppSwitchReturnURL_Test…
jaxdesmarais Apr 3, 2024
81e3b0b
Update UnitTests/BraintreePayPalTests/BTPayPalAppSwitchReturnURL_Test…
jaxdesmarais Apr 3, 2024
018c55d
Update Sources/BraintreePayPal/BTPayPalAppSwitchReturnURL.swift
jaxdesmarais Apr 3, 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
4 changes: 4 additions & 0 deletions Braintree.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,7 @@
BE9FB82B2898324C00D6FE2F /* BTPaymentMethodNonce.swift in Sources */ = {isa = PBXBuildFile; fileRef = BE9FB82A2898324C00D6FE2F /* BTPaymentMethodNonce.swift */; };
BE9FB82D28984ADE00D6FE2F /* BTPaymentMethodNonceParser.swift in Sources */ = {isa = PBXBuildFile; fileRef = BE9FB82C28984ADE00D6FE2F /* BTPaymentMethodNonceParser.swift */; };
BEB9BF532A26872B00A3673E /* BTWebAuthenticationSessionClient.swift in Sources */ = {isa = PBXBuildFile; fileRef = BEB9BF522A26872B00A3673E /* BTWebAuthenticationSessionClient.swift */; };
BEBA590F2BB1B5B9005FA8A2 /* BTPayPalAppSwitchReturnURL_Tests.swift in Sources */ = {isa = PBXBuildFile; fileRef = BEBA590E2BB1B5B9005FA8A2 /* BTPayPalAppSwitchReturnURL_Tests.swift */; };
BEBC222728D25BB400D83186 /* Helpers.swift in Sources */ = {isa = PBXBuildFile; fileRef = 80DBE69423A931A600373230 /* Helpers.swift */; };
BEBC6E4B29258FD4004E25A0 /* BraintreeCore.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 570B93AC285397520041BAFE /* BraintreeCore.framework */; };
BEBC6E5E2927CF59004E25A0 /* Braintree.h in Headers */ = {isa = PBXBuildFile; fileRef = BEBC6E5D2927CF59004E25A0 /* Braintree.h */; settings = {ATTRIBUTES = (Public, ); }; };
Expand Down Expand Up @@ -901,6 +902,7 @@
BE9FB82A2898324C00D6FE2F /* BTPaymentMethodNonce.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BTPaymentMethodNonce.swift; sourceTree = "<group>"; };
BE9FB82C28984ADE00D6FE2F /* BTPaymentMethodNonceParser.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BTPaymentMethodNonceParser.swift; sourceTree = "<group>"; };
BEB9BF522A26872B00A3673E /* BTWebAuthenticationSessionClient.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BTWebAuthenticationSessionClient.swift; sourceTree = "<group>"; };
BEBA590E2BB1B5B9005FA8A2 /* BTPayPalAppSwitchReturnURL_Tests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BTPayPalAppSwitchReturnURL_Tests.swift; sourceTree = "<group>"; };
BEBC6E5D2927CF59004E25A0 /* Braintree.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = Braintree.h; sourceTree = "<group>"; };
BEBC6F252937A510004E25A0 /* BTClientMetadata_Tests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BTClientMetadata_Tests.swift; sourceTree = "<group>"; };
BEBC6F272937BD1F004E25A0 /* BTGraphQLHTTP_Tests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BTGraphQLHTTP_Tests.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1717,6 +1719,7 @@
42FC218A25CDE0290047C49A /* BTPayPalRequest_Tests.swift */,
427F328F25D1A7B900435294 /* BTPayPalVaultRequest_Tests.swift */,
A9E5C1E424FD665D00EE691F /* Info.plist */,
BEBA590E2BB1B5B9005FA8A2 /* BTPayPalAppSwitchReturnURL_Tests.swift */,
);
path = BraintreePayPalTests;
sourceTree = "<group>";
Expand Down Expand Up @@ -3115,6 +3118,7 @@
BECB10C62B5999EE008D398E /* BTPayPalLineItem_Tests.swift in Sources */,
3B7A261429C35BD00087059D /* BTPayPalAnalytics_Tests.swift in Sources */,
A95229C724FD949D006F7D25 /* BTConfiguration+PayPal_Tests.swift in Sources */,
BEBA590F2BB1B5B9005FA8A2 /* BTPayPalAppSwitchReturnURL_Tests.swift in Sources */,
);
runOnlyForDeploymentPostprocessing = 0;
};
Expand Down
8 changes: 4 additions & 4 deletions IntegrationTests/BraintreePayPal_IntegrationTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class BraintreePayPal_IntegrationTests: XCTestCase {
let tokenizationExpectation = expectation(description: "Tokenize one-time payment")
let returnURL = URL(string: oneTouchCoreAppSwitchSuccessURLFixture)

payPalClient.handleBrowserSwitchReturn(returnURL, paymentType: .checkout) { tokenizedPayPalAccount, error in
payPalClient.handleReturn(returnURL, paymentType: .checkout) { tokenizedPayPalAccount, error in
guard let nonce = tokenizedPayPalAccount?.nonce else {
XCTFail("Failed to tokenize account.")
return
Expand All @@ -42,7 +42,7 @@ class BraintreePayPal_IntegrationTests: XCTestCase {
let tokenizationExpectation = expectation(description: "Tokenize one-time payment")
let returnURL = URL(string: oneTouchCoreAppSwitchSuccessURLFixture)

payPalClient.handleBrowserSwitchReturn(returnURL,paymentType: .checkout) { tokenizedPayPalAccount, error in
payPalClient.handleReturn(returnURL,paymentType: .checkout) { tokenizedPayPalAccount, error in
guard let nonce = tokenizedPayPalAccount?.nonce else {
XCTFail("Failed to tokenize account.")
return
Expand All @@ -68,7 +68,7 @@ class BraintreePayPal_IntegrationTests: XCTestCase {
let tokenizationExpectation = expectation(description: "Tokenize billing agreement payment")
let returnURL = URL(string: oneTouchCoreAppSwitchSuccessURLFixture)

payPalClient.handleBrowserSwitchReturn(returnURL, paymentType: .vault) { tokenizedPayPalAccount, error in
payPalClient.handleReturn(returnURL, paymentType: .vault) { tokenizedPayPalAccount, error in
guard let nonce = tokenizedPayPalAccount?.nonce else {
XCTFail("Failed to tokenize account.")
return
Expand All @@ -92,7 +92,7 @@ class BraintreePayPal_IntegrationTests: XCTestCase {
let tokenizationExpectation = expectation(description: "Tokenize billing agreement payment")
let returnURL = URL(string: oneTouchCoreAppSwitchSuccessURLFixture)

payPalClient.handleBrowserSwitchReturn(returnURL, paymentType: .vault) { tokenizedPayPalAccount, error in
payPalClient.handleReturn(returnURL, paymentType: .vault) { tokenizedPayPalAccount, error in
guard let nonce = tokenizedPayPalAccount?.nonce else {
XCTFail("Failed to tokenize account.")
return
Expand Down
32 changes: 31 additions & 1 deletion Sources/BraintreePayPal/BTPayPalAppSwitchReturnURL.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import Foundation

#if canImport(BraintreeCore)
import BraintreeCore
#endif

enum BTPayPalAppSwitchReturnURLState {
case unknown
scannillo marked this conversation as resolved.
Show resolved Hide resolved
case succeeded
Expand All @@ -11,10 +15,36 @@ enum BTPayPalAppSwitchReturnURLState {
/// PayPal app switch authorization requests should result in success or user-initiated cancelation. These states are communicated in the url.
struct BTPayPalAppSwitchReturnURL {

/// The overall status of the app switch - success, failure or cancelation
jaxdesmarais marked this conversation as resolved.
Show resolved Hide resolved
var state: BTPayPalAppSwitchReturnURLState = .unknown

/// The EC Token from the return URL.
var ecToken: String?

/// The BA Token from the return URL.
var baToken: String?
scannillo marked this conversation as resolved.
Show resolved Hide resolved

/// The timestamp from the return URL.
var timestamp: String?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar comment here, we aren't using this.

This surfaces that we need to ask the analytics folks what they want us to do with this number, let's ask them! They may want us including it in an additional analytic event send


/// Initializes a new `BTPayPalAppSwitchReturnURL`
/// - Parameter url: an incoming app switch url
init?(url: URL) {
// TODO: implement init based on return URL
let parameters = BTURLUtils.queryParameters(for: url)

if url.path.contains("success") {
state = .succeeded
if let ecToken = parameters["token"] {
self.ecToken = ecToken
}
jaxdesmarais marked this conversation as resolved.
Show resolved Hide resolved

baToken = parameters["ba_token"]
timestamp = parameters["switch_initiated_time"]
} else if url.path.contains("cancel") {
state = .canceled
} else {
state = .unknown
}
}

// MARK: - Static Methods
Expand Down
22 changes: 18 additions & 4 deletions Sources/BraintreePayPal/BTPayPalClient.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ import BraintreeDataCollector
/// Exposed for testing, the ASWebAuthenticationSession instance used for the PayPal flow
var webAuthenticationSession: BTWebAuthenticationSession

/// Used internally as a holder for the completion in methods that do not pass a completion such as `handleOpen`.
/// This allows us to set and return a completion in our methods that otherwise cannot require a completion.
var appSwitchCompletion: (BTPayPalAccountNonce?, Error?) -> Void = { _, _ in }

// MARK: - Static Properties

/// This static instance of `BTPayPalClient` is used during the app switch process.
Expand Down Expand Up @@ -157,7 +161,7 @@ import BraintreeDataCollector

// MARK: - Internal Methods

func handleBrowserSwitchReturn(
func handleReturn(
jaxdesmarais marked this conversation as resolved.
Show resolved Hide resolved
_ url: URL?,
paymentType: BTPayPalPaymentType,
completion: @escaping (BTPayPalAccountNonce?, Error?) -> Void
Expand Down Expand Up @@ -236,7 +240,17 @@ import BraintreeDataCollector
// MARK: - App Switch Methods

func handleReturnURL(_ url: URL) {
// TODO: implement handling return URL in a follow up PR
guard let returnURL = BTPayPalAppSwitchReturnURL(url: url) else {
notifyFailure(with: BTPayPalError.invalidURL("App Switch return URL cannot be nil"), completion: appSwitchCompletion)
return
}

switch returnURL.state {
case .succeeded, .canceled:
handleReturn(url, paymentType: .vault, completion: appSwitchCompletion)
default:
scannillo marked this conversation as resolved.
Show resolved Hide resolved
notifyFailure(with: BTPayPalError.unknownAppSwitchError, completion: appSwitchCompletion)
}
}

// MARK: - Private Methods
Expand Down Expand Up @@ -333,7 +347,7 @@ import BraintreeDataCollector
return
}

handleBrowserSwitchReturn(url, paymentType: paymentType, completion: completion)
handleReturn(url, paymentType: paymentType, completion: completion)
} sessionDidAppear: { [self] didAppear in
if didAppear {
apiClient.sendAnalyticsEvent(BTPayPalAnalytics.browserPresentationSucceeded, payPalContextID: payPalContextID)
Expand Down Expand Up @@ -368,7 +382,7 @@ import BraintreeDataCollector
hostAndPath.append("/")
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now we have some return URL parsing happening in BTPayPalClient and some happening in BTPayPalAppSwitchReturnURL. Maybe for another PR, we can repurpose BTPayPalAppSwitchReturnURL to do URL parsing for both the web flow & app switch flows.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created DTBTSDK-3751 for this

if hostAndPath != BTPayPalRequest.callbackURLHostAndPath {
if hostAndPath != BTPayPalRequest.callbackURLHostAndPath && (payPalRequest as? BTPayPalVaultRequest)?.universalLink == nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

curious why we add the check for universalLink being nil here 👀

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question, we only care about (and want to return false here for isValidURLAction) if the hostAndPath is not the callbackURLHostAndPath and if the universalLink is nil. That combination would mean that we aren't using the UL flow and the hostAndPath is invalid we wouldn't have a valid URL action. If we are using the UL flow we can essentially skip this check since it's not relevant to that flow.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is a little confusing to read IMO. I think it will clear up in DTBTSDK-3751

return false
}

Expand Down
13 changes: 10 additions & 3 deletions Sources/BraintreePayPal/BTPayPalError.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public enum BTPayPalError: Error, CustomNSError, LocalizedError, Equatable {
/// 3. HTTP POST request returned an error
case httpPostRequestError([String: Any])

/// 4. The web approval URL, web redirect URL, or PayPal native app approval URL is invalid
/// 4. The web approval URL, web redirect URL, App Switch return URL, PayPal native app approval URL is invalid
jaxdesmarais marked this conversation as resolved.
Show resolved Hide resolved
case invalidURL(String)

/// 5. The ASWebAuthenticationSession URL is invalid
Expand All @@ -32,7 +32,10 @@ public enum BTPayPalError: Error, CustomNSError, LocalizedError, Equatable {

/// 9. Deallocated BTPayPalClient
case deallocated


/// 10. Unknown app switch error occurred.
case unknownAppSwitchError
jaxdesmarais marked this conversation as resolved.
Show resolved Hide resolved

public static var errorDomain: String {
"com.braintreepayments.BTPayPalErrorDomain"
}
Expand All @@ -59,6 +62,8 @@ public enum BTPayPalError: Error, CustomNSError, LocalizedError, Equatable {
return 8
case .deallocated:
return 9
case .unknownAppSwitchError:
return 10
}
}

Expand All @@ -73,7 +78,7 @@ public enum BTPayPalError: Error, CustomNSError, LocalizedError, Equatable {
case .httpPostRequestError(let error):
return "HTTP POST request failed with \(error)."
case .invalidURL(let error):
scannillo marked this conversation as resolved.
Show resolved Hide resolved
return "An error occured with retrieving a PayPal authentication URL: \(error)"
return "An error occurred with retrieving a PayPal URL: \(error)"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We had a typo here and also are using this error for URLs outside of authentication so updated this string.

case .asWebAuthenticationSessionURLInvalid(let scheme):
return "Attempted to open an invalid URL in ASWebAuthenticationSession: \(scheme)://. Try again or contact Braintree Support."
case .invalidURLAction:
Expand All @@ -84,6 +89,8 @@ public enum BTPayPalError: Error, CustomNSError, LocalizedError, Equatable {
return "ASWebAuthenticationSession failed with \(error.localizedDescription)"
case .deallocated:
return "BTPayPalClient has been deallocated."
case .unknownAppSwitchError:
return "An unknown error occurred during the App Switch flow."
}
}

Expand Down
31 changes: 31 additions & 0 deletions UnitTests/BraintreePayPalTests/BTPayPalAppSwitchReturnURL_Tests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import XCTest
@testable import BraintreePayPal

final class BTPayPalAppSwitchReturnURL_Tests: XCTestCase {

func testInitWithURL_whenSuccessReturnURL_createsValuesAndSetsSuccessState() {
let returnURL = BTPayPalAppSwitchReturnURL(url: URL(string: "https://www.merchant-app.com/merchant-path/success?token=A_FAKE_EC_TOKEN&ba_token=A_FAKE_BA_TOKEN&switch_initiated_time=1234567890")!)
XCTAssertEqual(returnURL?.baToken, "A_FAKE_BA_TOKEN")
XCTAssertEqual(returnURL?.ecToken, "A_FAKE_EC_TOKEN")
XCTAssertEqual(returnURL?.timestamp, "1234567890")
XCTAssertEqual(returnURL?.state, .succeeded)
}

func testInitWithURL_whenSuccessReturnURLWithoutToken_createsValuesAndSetsSuccessState() {
let returnURL = BTPayPalAppSwitchReturnURL(url: URL(string: "https://www.merchant-app.com/merchant-path/success?ba_token=A_FAKE_BA_TOKEN&switch_initiated_time=1234567890")!)
XCTAssertEqual(returnURL?.baToken, "A_FAKE_BA_TOKEN")
XCTAssertNil(returnURL?.ecToken)
XCTAssertEqual(returnURL?.timestamp, "1234567890")
XCTAssertEqual(returnURL?.state, .succeeded)
}

func testInitWithURL_whenCancelURL_setsCancelState() {
jaxdesmarais marked this conversation as resolved.
Show resolved Hide resolved
let returnURL = BTPayPalAppSwitchReturnURL(url: URL(string: "https://www.merchant-app.com/merchant-path/cancel?ba_token=A_FAKE_BA_TOKEN&switch_initiated_time=1234567890")!)
XCTAssertEqual(returnURL?.state, .canceled)
}

func testInitWithURL_whenUnknownURL_setsUnknownState() {
jaxdesmarais marked this conversation as resolved.
Show resolved Hide resolved
let returnURL = BTPayPalAppSwitchReturnURL(url: URL(string: "https://www.merchant-app.com/merchant-path/garbage-url")!)
XCTAssertEqual(returnURL?.state, .unknown)
}
}
Loading
Loading