Closes #3944: AppLinksFeature: Do not open third-party apps for URLs …

…with javascript scheme.
mozilla-mobile · Jul 31, 2019 · 57b6cff · 57b6cff
1 parent 865f310
commit 57b6cff
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 8 deletions.
diff --git a/...s/feature/app-links/src/main/java/mozilla/components/feature/app/links/AppLinksFeature.kt b/...s/feature/app-links/src/main/java/mozilla/components/feature/app/links/AppLinksFeature.kt
@@ -38,13 +38,19 @@ import mozilla.components.support.ktx.android.net.hostWithoutCommonPrefixes
  * A [Boolean] flag is provided at construction to allow the feature and use cases to be landed without
  * adjoining UI. The UI will be activated in https://github.com/mozilla-mobile/android-components/issues/2974
  * and https://github.com/mozilla-mobile/android-components/issues/2975.
+ *
+ * @param alwaysAllowedSchemes List of schemes that will always be allowed to be opened in a third-party
+ * app even if [interceptLinkClicks] is `false`.
+ * @param alwaysDeniedSchemes List of schemes that will never be opened in a third-party app even if
+ * [interceptLinkClicks] is `true`.
  */
 class AppLinksFeature(
     private val context: Context,
     private val sessionManager: SessionManager,
     private val sessionId: String? = null,
     private val interceptLinkClicks: Boolean = false,
-    private val whitelistedSchemes: Set<String> = setOf("mailto", "market", "sms", "tel"),
+    private val alwaysAllowedSchemes: Set<String> = setOf("mailto", "market", "sms", "tel"),
+    private val alwaysDeniedSchemes: Set<String> = setOf("javascript"),
     private val fragmentManager: FragmentManager? = null,
     private var dialog: RedirectDialogFragment = SimpleRedirectDialogFragment.newInstance(),
     private val useCases: AppLinksUseCases = AppLinksUseCases(context)
@@ -66,7 +72,7 @@ class AppLinksFeature(
      * Starts observing app links on the selected session.
      */
     override fun start() {
-        if (interceptLinkClicks || whitelistedSchemes.isNotEmpty()) {
+        if (interceptLinkClicks || alwaysAllowedSchemes.isNotEmpty()) {
             observer.observeIdOrSelected(sessionId)
         }
         findPreviousDialogFragment()?.let {
@@ -75,7 +81,7 @@ class AppLinksFeature(
     }
 
     override fun stop() {
-        if (interceptLinkClicks || whitelistedSchemes.isNotEmpty()) {
+        if (interceptLinkClicks || alwaysAllowedSchemes.isNotEmpty()) {
             observer.stop()
         }
     }
@@ -107,8 +113,9 @@ class AppLinksFeature(
             return
         }
 
-        redirect.appIntent?.data?.scheme?.let {
-            if (!interceptLinkClicks && !whitelistedSchemes.contains(it)) {
+        redirect.appIntent?.data?.scheme?.let { scheme ->
+            if ((!interceptLinkClicks && !alwaysAllowedSchemes.contains(scheme)) ||
+                alwaysDeniedSchemes.contains(scheme)) {
                 return
             }
         }

diff --git a/...ature/app-links/src/test/java/mozilla/components/feature/app/links/AppLinksFeatureTest.kt b/...ature/app-links/src/test/java/mozilla/components/feature/app/links/AppLinksFeatureTest.kt
@@ -8,18 +8,22 @@ package mozilla.components.feature.app.links
 
 import android.content.Context
 import android.content.Intent
+import android.net.Uri
 import androidx.fragment.app.FragmentManager
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import mozilla.components.browser.session.Session
 import mozilla.components.browser.session.SessionManager
 import mozilla.components.concept.engine.Engine
 import mozilla.components.support.test.any
 import mozilla.components.support.test.mock
+import mozilla.components.support.test.robolectric.testContext
+import mozilla.components.support.test.whenever
 import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.mockito.ArgumentMatchers
 import org.mockito.Mockito.`when`
+import org.mockito.Mockito.never
 import org.mockito.Mockito.spy
 import org.mockito.Mockito.verify
 import org.mockito.Mockito.verifyNoMoreInteractions
@@ -95,7 +99,7 @@ class AppLinksFeatureTest {
             mockSessionManager,
             useCases = mockUseCases,
             interceptLinkClicks = false,
-            whitelistedSchemes = setOf()
+            alwaysAllowedSchemes = setOf()
         )
 
         subject.start()
@@ -128,7 +132,7 @@ class AppLinksFeatureTest {
             mockContext,
             mockSessionManager,
             interceptLinkClicks = false,
-            whitelistedSchemes = setOf(whitelistedScheme),
+            alwaysAllowedSchemes = setOf(whitelistedScheme),
             useCases = mockUseCases
         )
 
@@ -178,7 +182,7 @@ class AppLinksFeatureTest {
             sessionManager = mockSessionManager,
             useCases = mockUseCases,
             interceptLinkClicks = false,
-            whitelistedSchemes = setOf("whitelisted")
+            alwaysAllowedSchemes = setOf("whitelisted")
         )
 
         feature.start()
@@ -298,4 +302,33 @@ class AppLinksFeatureTest {
         verifyNoMoreInteractions(mockDialog)
         verify(mockOpenRedirect).invoke(any())
     }
+
+    @Test
+    fun `an external app is not opened for URLs with javascript scheme`() {
+        val javascriptUri = "javascript:;"
+
+        val openAppUseCase: AppLinksUseCases.OpenAppLinkRedirect = mock()
+        val useCases: AppLinksUseCases = mock()
+        whenever(useCases.openAppLink).thenReturn(openAppUseCase)
+
+        val feature = AppLinksFeature(
+            testContext,
+            sessionManager = mock(),
+            interceptLinkClicks = true,
+            useCases = useCases
+        )
+
+        val intent = Intent().apply {
+            data = Uri.parse(javascriptUri)
+        }
+
+        val redirect = AppLinkRedirect(
+            intent,
+            javascriptUri,
+            false)
+
+        feature.handleRedirect(redirect, Session("https://www.amazon.ca"))
+
+        verify(openAppUseCase, never()).invoke(redirect)
+    }
 }