From 0d856dde6e3c0605e6a66dc0fcabec0438b98c3e Mon Sep 17 00:00:00 2001 From: Thomas Espach Date: Wed, 29 Nov 2023 16:20:05 +0000 Subject: [PATCH 1/6] Check external navigation actions for redirects first. If the external application URL was the result of a cross-origin redirect, then we incorrectly display the wrong origin in the external application request. https://app.asana.com/0/1176047645786601/1204521390227183/f --- DuckDuckGo/Tab/Navigation/ExternalAppSchemeHandler.swift | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/DuckDuckGo/Tab/Navigation/ExternalAppSchemeHandler.swift b/DuckDuckGo/Tab/Navigation/ExternalAppSchemeHandler.swift index 833710be55..74c2f8efe9 100644 --- a/DuckDuckGo/Tab/Navigation/ExternalAppSchemeHandler.swift +++ b/DuckDuckGo/Tab/Navigation/ExternalAppSchemeHandler.swift @@ -86,8 +86,12 @@ extension ExternalAppSchemeHandler: NavigationResponder { } let permissionType = PermissionType.externalScheme(scheme: scheme) - // use domain from the url for user-entered app schemes, otherwise use current website domain - let domain = navigationAction.isUserEnteredUrl ? navigationAction.url.host ?? "" : navigationAction.sourceFrame.securityOrigin.host + // Use domain from the url for user-entered app schemes, otherwise, check for redirects, then use current website domain + var previousDomain: String? + for nav in navigationAction.redirectHistory!.reversed() { + previousDomain = nav.url.host + } + let domain = previousDomain ?? (navigationAction.isUserEnteredUrl ? navigationAction.url.host ?? "" : navigationAction.sourceFrame.securityOrigin.host) permissionModel.permissions([permissionType], requestedForDomain: domain, url: externalUrl) { [workspace] isGranted in if isGranted { workspace.open(externalUrl) From abbbe09f9a98df9c5132f429748abef19163b8c6 Mon Sep 17 00:00:00 2001 From: Thomas Espach Date: Wed, 29 Nov 2023 16:23:33 +0000 Subject: [PATCH 2/6] Make check more robust. --- DuckDuckGo/Tab/Navigation/ExternalAppSchemeHandler.swift | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/DuckDuckGo/Tab/Navigation/ExternalAppSchemeHandler.swift b/DuckDuckGo/Tab/Navigation/ExternalAppSchemeHandler.swift index 74c2f8efe9..17a5d506d9 100644 --- a/DuckDuckGo/Tab/Navigation/ExternalAppSchemeHandler.swift +++ b/DuckDuckGo/Tab/Navigation/ExternalAppSchemeHandler.swift @@ -87,11 +87,14 @@ extension ExternalAppSchemeHandler: NavigationResponder { let permissionType = PermissionType.externalScheme(scheme: scheme) // Use domain from the url for user-entered app schemes, otherwise, check for redirects, then use current website domain - var previousDomain: String? + var redirectDomain: String? for nav in navigationAction.redirectHistory!.reversed() { - previousDomain = nav.url.host + if (nav.url.host != navigationAction.url.host) { + redirectDomain = nav.url.host + break + } } - let domain = previousDomain ?? (navigationAction.isUserEnteredUrl ? navigationAction.url.host ?? "" : navigationAction.sourceFrame.securityOrigin.host) + let domain = redirectDomain ?? (navigationAction.isUserEnteredUrl ? navigationAction.url.host ?? "" : navigationAction.sourceFrame.securityOrigin.host) permissionModel.permissions([permissionType], requestedForDomain: domain, url: externalUrl) { [workspace] isGranted in if isGranted { workspace.open(externalUrl) From 7b39091371b0dd3d4c90d808a7d791568d04368e Mon Sep 17 00:00:00 2001 From: Thomas Espach Date: Wed, 29 Nov 2023 16:30:48 +0000 Subject: [PATCH 3/6] Cleanup. --- DuckDuckGo/Tab/Navigation/ExternalAppSchemeHandler.swift | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/DuckDuckGo/Tab/Navigation/ExternalAppSchemeHandler.swift b/DuckDuckGo/Tab/Navigation/ExternalAppSchemeHandler.swift index 17a5d506d9..8a1527b14f 100644 --- a/DuckDuckGo/Tab/Navigation/ExternalAppSchemeHandler.swift +++ b/DuckDuckGo/Tab/Navigation/ExternalAppSchemeHandler.swift @@ -87,13 +87,7 @@ extension ExternalAppSchemeHandler: NavigationResponder { let permissionType = PermissionType.externalScheme(scheme: scheme) // Use domain from the url for user-entered app schemes, otherwise, check for redirects, then use current website domain - var redirectDomain: String? - for nav in navigationAction.redirectHistory!.reversed() { - if (nav.url.host != navigationAction.url.host) { - redirectDomain = nav.url.host - break - } - } + let redirectDomain = navigationAction.redirectHistory?.reversed().first(where: { $0.url.host != navigationAction.url.host })?.url.host let domain = redirectDomain ?? (navigationAction.isUserEnteredUrl ? navigationAction.url.host ?? "" : navigationAction.sourceFrame.securityOrigin.host) permissionModel.permissions([permissionType], requestedForDomain: domain, url: externalUrl) { [workspace] isGranted in if isGranted { From a4096907a38e0331fbceedf164c5c348b0622a20 Mon Sep 17 00:00:00 2001 From: Thom Espach Date: Thu, 30 Nov 2023 13:11:42 +0000 Subject: [PATCH 4/6] Update comment. --- DuckDuckGo/Tab/Navigation/ExternalAppSchemeHandler.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DuckDuckGo/Tab/Navigation/ExternalAppSchemeHandler.swift b/DuckDuckGo/Tab/Navigation/ExternalAppSchemeHandler.swift index 8a1527b14f..fa70f5755c 100644 --- a/DuckDuckGo/Tab/Navigation/ExternalAppSchemeHandler.swift +++ b/DuckDuckGo/Tab/Navigation/ExternalAppSchemeHandler.swift @@ -86,7 +86,7 @@ extension ExternalAppSchemeHandler: NavigationResponder { } let permissionType = PermissionType.externalScheme(scheme: scheme) - // Use domain from the url for user-entered app schemes, otherwise, check for redirects, then use current website domain + // Check for cross-origin redirects first, then use domain from the url for user-entered app schemes, then use current website domain let redirectDomain = navigationAction.redirectHistory?.reversed().first(where: { $0.url.host != navigationAction.url.host })?.url.host let domain = redirectDomain ?? (navigationAction.isUserEnteredUrl ? navigationAction.url.host ?? "" : navigationAction.sourceFrame.securityOrigin.host) permissionModel.permissions([permissionType], requestedForDomain: domain, url: externalUrl) { [workspace] isGranted in From c13a53e95966c05704138187b8a6e04a602d9712 Mon Sep 17 00:00:00 2001 From: Thomas Espach Date: Fri, 15 Dec 2023 14:23:58 +0000 Subject: [PATCH 5/6] Fix lint issues from merge. --- DuckDuckGo/Tab/Navigation/ExternalAppSchemeHandler.swift | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/DuckDuckGo/Tab/Navigation/ExternalAppSchemeHandler.swift b/DuckDuckGo/Tab/Navigation/ExternalAppSchemeHandler.swift index ed4a2de51b..e2d2fc40a8 100644 --- a/DuckDuckGo/Tab/Navigation/ExternalAppSchemeHandler.swift +++ b/DuckDuckGo/Tab/Navigation/ExternalAppSchemeHandler.swift @@ -120,14 +120,12 @@ extension ExternalAppSchemeHandler: NavigationResponder { permissionModel.permissions([permissionType], requestedForDomain: domain, url: externalUrl) { [workspace] isGranted in if isGranted { workspace.open(externalUrl) - // if "Always allow" is set and this is the only navigation in the tab: close the tab - if self?.shouldCloseTabOnExternalAppOpen == true, let webView = navigationAction.targetFrame?.webView { + if self.shouldCloseTabOnExternalAppOpen == true, let webView = navigationAction.targetFrame?.webView { webView.close() } } } - return .cancel } From 8105eecbbd28c4a51af11db75d7031d5a95ab326 Mon Sep 17 00:00:00 2001 From: Thomas Espach Date: Wed, 20 Dec 2023 10:49:05 +0000 Subject: [PATCH 6/6] Disable function_body_length lint rule on decidePolicy function. --- DuckDuckGo/Tab/Navigation/ExternalAppSchemeHandler.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/DuckDuckGo/Tab/Navigation/ExternalAppSchemeHandler.swift b/DuckDuckGo/Tab/Navigation/ExternalAppSchemeHandler.swift index e2d2fc40a8..75fdbbea30 100644 --- a/DuckDuckGo/Tab/Navigation/ExternalAppSchemeHandler.swift +++ b/DuckDuckGo/Tab/Navigation/ExternalAppSchemeHandler.swift @@ -53,8 +53,8 @@ final class ExternalAppSchemeHandler { extension ExternalAppSchemeHandler: NavigationResponder { - @MainActor - func decidePolicy(for navigationAction: NavigationAction, preferences: inout NavigationPreferences) async -> NavigationActionPolicy? { + // swiftlint:disable:next function_body_length + @MainActor func decidePolicy(for navigationAction: NavigationAction, preferences: inout NavigationPreferences) async -> NavigationActionPolicy? { let externalUrl = navigationAction.url // only proceed with non-external-scheme navigations guard externalUrl.isExternalSchemeLink, let scheme = externalUrl.scheme else {