Skip to content

Commit

Permalink
Closes mozilla-mobile#3944: AppLinksFeature: Do not open third-party …
Browse files Browse the repository at this point in the history
…apps for URLs with javascript scheme.
  • Loading branch information
pocmo authored and jonalmeida committed Jul 30, 2019
1 parent 2925e47 commit 688d1f2
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 {
Expand All @@ -75,7 +81,7 @@ class AppLinksFeature(
}

override fun stop() {
if (interceptLinkClicks || whitelistedSchemes.isNotEmpty()) {
if (interceptLinkClicks || alwaysAllowedSchemes.isNotEmpty()) {
observer.stop()
}
}
Expand Down Expand Up @@ -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
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -95,7 +99,7 @@ class AppLinksFeatureTest {
mockSessionManager,
useCases = mockUseCases,
interceptLinkClicks = false,
whitelistedSchemes = setOf()
alwaysAllowedSchemes = setOf()
)

subject.start()
Expand Down Expand Up @@ -128,7 +132,7 @@ class AppLinksFeatureTest {
mockContext,
mockSessionManager,
interceptLinkClicks = false,
whitelistedSchemes = setOf(whitelistedScheme),
alwaysAllowedSchemes = setOf(whitelistedScheme),
useCases = mockUseCases
)

Expand Down Expand Up @@ -178,7 +182,7 @@ class AppLinksFeatureTest {
sessionManager = mockSessionManager,
useCases = mockUseCases,
interceptLinkClicks = false,
whitelistedSchemes = setOf("whitelisted")
alwaysAllowedSchemes = setOf("whitelisted")
)

feature.start()
Expand Down Expand Up @@ -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)
}
}
3 changes: 3 additions & 0 deletions docs/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ permalink: /changelog/
* [Gecko](https://github.com/mozilla-mobile/android-components/blob/master/buildSrc/src/main/java/Gecko.kt)
* [Configuration](https://github.com/mozilla-mobile/android-components/blob/master/buildSrc/src/main/java/Config.kt)

* **feature-app-links**
* Fixed [#3944](https://github.com/mozilla-mobile/android-components/issues/3944) causing third-party apps being opened when links with a `javascript` scheme are clicked.

# 6.0.0

* [Commits](https://github.com/mozilla-mobile/android-components/compare/v5.0.0...v6.0.0)
Expand Down

0 comments on commit 688d1f2

Please sign in to comment.