Archives
/
iceraven-browser
mirror of https://github.com/fork-maintainers/iceraven-browser
2
0
Fork 0
Code Issues Releases Wiki Activity

Bug 1857539 - Remove AMO request interceptor

Browse Source
fenix/120.0
William Durand 8 months ago committed by mergify[bot]
parent 68a90a9c0f
commit 27acc1d22b
7 changed files with 4 additions and 300 deletions
Show Stats Download Patch File Download Diff File

41
app/src/androidTest/java/org/mozilla/fenix/AppRequestInterceptor.kt
Unescape Escape View File

@ -55,11 +55,6 @@ class AppRequestInterceptor(
)?.let { response ->
return response
}
interceptAmoRequest(uri, isSameDomain, hasUserGesture)?.let { response ->
return response
}
return context.components.services.appLinksInterceptor
.onLoadRequest(
engineSession,
@ -95,40 +90,6 @@ class AppRequestInterceptor(
return RequestInterceptor.ErrorResponse(errorPageUri)
}
/**
* Checks if the provided [uri] is a request to install an add-on from addons.mozilla.org and
* redirects to Add-ons Manager to trigger installation if needed.
*
* @return [RequestInterceptor.InterceptionResponse.Deny] when installation was triggered and
* the original request can be skipped, otherwise null to continue loading the page.
*/
private fun interceptAmoRequest(
uri: String,
isSameDomain: Boolean,
hasUserGesture: Boolean,
): RequestInterceptor.InterceptionResponse? {
// First we execute a quick check to see if this is a request we're interested in i.e. a
// request triggered by the user and coming from AMO.
if (hasUserGesture && isSameDomain && uri.startsWith(AMO_BASE_URL)) {
// Check if this is a request to install an add-on.
val matchResult = AMO_INSTALL_URL_REGEX.toRegex().matchEntire(uri)
if (matchResult != null) {
// Navigate and trigger add-on installation.
matchResult.groupValues.getOrNull(1)?.let { addonId ->
navController?.get()?.navigate(
NavGraphDirections.actionGlobalAddonsManagementFragment(addonId),
)
// We've redirected to the add-ons management fragment, skip original request.
return RequestInterceptor.InterceptionResponse.Deny
}
}
}
// In all other case we let the original request proceed.
return null
}
// This method is the only difference from the production code.
// Otherwise the code should be kept identical
@Suppress("LongParameterList")
@ -236,7 +197,5 @@ class AppRequestInterceptor(
companion object {
internal const val LOW_AND_MEDIUM_RISK_ERROR_PAGES = "low_and_medium_risk_error_pages.html"
internal const val HIGH_RISK_ERROR_PAGES = "high_risk_error_pages.html"
internal const val AMO_BASE_URL = BuildConfig.AMO_BASE_URL
internal const val AMO_INSTALL_URL_REGEX = "$AMO_BASE_URL/android/downloads/file/([^\\s]+)/([^\\s]+\\.xpi)"
}
}

40
app/src/main/java/org/mozilla/fenix/AppRequestInterceptor.kt
Unescape Escape View File

@ -37,10 +37,6 @@ class AppRequestInterceptor(
isDirectNavigation: Boolean,
isSubframeRequest: Boolean,
): RequestInterceptor.InterceptionResponse? {
interceptAmoRequest(uri, isSameDomain, hasUserGesture)?.let { response ->
return response
}
return context.components.services.appLinksInterceptor
.onLoadRequest(
engineSession,
@ -76,40 +72,6 @@ class AppRequestInterceptor(
return RequestInterceptor.ErrorResponse(errorPageUri)
}
/**
* Checks if the provided [uri] is a request to install an add-on from addons.mozilla.org and
* redirects to Add-ons Manager to trigger installation if needed.
*
* @return [RequestInterceptor.InterceptionResponse.Deny] when installation was triggered and
* the original request can be skipped, otherwise null to continue loading the page.
*/
private fun interceptAmoRequest(
uri: String,
isSameDomain: Boolean,
hasUserGesture: Boolean,
): RequestInterceptor.InterceptionResponse? {
// First we execute a quick check to see if this is a request we're interested in i.e. a
// request triggered by the user and coming from AMO.
if (hasUserGesture && isSameDomain && uri.startsWith(AMO_BASE_URL)) {
// Check if this is a request to install an add-on.
val matchResult = AMO_INSTALL_URL_REGEX.toRegex().find(uri)
if (matchResult != null) {
// Navigate and trigger add-on installation.
matchResult.groupValues.getOrNull(1)?.let { addonId ->
navController?.get()?.navigate(
NavGraphDirections.actionGlobalAddonsManagementFragment(addonId),
)
// We've redirected to the add-ons management fragment, skip original request.
return RequestInterceptor.InterceptionResponse.Deny
}
}
}
// In all other case we let the original request proceed.
return null
}
/**
* Where possible, this will make the error type more accurate by including information not
* available to AC.
@ -192,7 +154,5 @@ class AppRequestInterceptor(
companion object {
internal const val LOW_AND_MEDIUM_RISK_ERROR_PAGES = "low_and_medium_risk_error_pages.html"
internal const val HIGH_RISK_ERROR_PAGES = "high_risk_error_pages.html"
internal const val AMO_BASE_URL = BuildConfig.AMO_BASE_URL
internal const val AMO_INSTALL_URL_REGEX = "$AMO_BASE_URL/android/downloads/file/([^\\s]+)/([^\\s]+\\.xpi)"
}
}

39
app/src/main/java/org/mozilla/fenix/addons/AddonsManagementFragment.kt
Unescape Escape View File

@ -16,7 +16,6 @@ import androidx.core.view.isVisible
import androidx.fragment.app.Fragment
import androidx.lifecycle.lifecycleScope
import androidx.navigation.fragment.findNavController
import androidx.navigation.fragment.navArgs
import androidx.recyclerview.widget.LinearLayoutManager
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.Dispatchers.IO
@ -49,21 +48,11 @@ import org.mozilla.fenix.theme.ThemeManager
@Suppress("TooManyFunctions", "LargeClass")
class AddonsManagementFragment : Fragment(R.layout.fragment_add_ons_management) {
private val args by navArgs<AddonsManagementFragmentArgs>()
private var binding: FragmentAddOnsManagementBinding? = null
private val webExtensionPromptFeature = ViewBoundFeatureWrapper<WebExtensionPromptFeature>()
private var addons: List<Addon> = emptyList()
private var installExternalAddonComplete: Boolean
set(value) {
arguments?.putBoolean(BUNDLE_KEY_INSTALL_EXTERNAL_ADDON_COMPLETE, value)
}
get() {
return arguments?.getBoolean(BUNDLE_KEY_INSTALL_EXTERNAL_ADDON_COMPLETE, false) ?: false
}
private var adapter: AddonsManagerAdapter? = null
override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
@ -110,12 +99,9 @@ class AddonsManagementFragment : Fragment(R.layout.fragment_add_ons_management)
recyclerView?.layoutManager = LinearLayoutManager(requireContext())
val shouldRefresh = adapter != null
// If the fragment was launched to install an "external" add-on from AMO, we deactivate
// the cache to get the most up-to-date list of add-ons to match against.
val allowCache = args.installAddonId == null || installExternalAddonComplete
lifecycleScope.launch(IO) {
try {
addons = requireContext().components.addonManager.getAddons(allowCache)
addons = requireContext().components.addonManager.getAddons()
// Add-ons that should be excluded in Mozilla Online builds
val excludedAddonIDs = if (Config.channel.isMozillaOnline &&
!BuildConfig.MOZILLA_ONLINE_ADDON_EXCLUSIONS.isNullOrEmpty()
@ -142,12 +128,6 @@ class AddonsManagementFragment : Fragment(R.layout.fragment_add_ons_management)
if (shouldRefresh) {
adapter?.updateAddons(addons)
}
args.installAddonId?.let { addonIn ->
if (!installExternalAddonComplete) {
installExternalAddon(addons, addonIn)
}
}
}
}
} catch (e: AddonManagerException) {
@ -167,21 +147,6 @@ class AddonsManagementFragment : Fragment(R.layout.fragment_add_ons_management)
}
}
@VisibleForTesting
internal fun installExternalAddon(supportedAddons: List<Addon>, installAddonId: String) {
val addonToInstall = supportedAddons.find { it.downloadId == installAddonId }
if (addonToInstall == null) {
showErrorSnackBar(getString(R.string.addon_not_supported_error))
} else {
if (addonToInstall.isInstalled()) {
showErrorSnackBar(getString(R.string.addon_already_installed))
} else {
installAddon(addonToInstall)
}
}
installExternalAddonComplete = true
}
@VisibleForTesting
internal fun showErrorSnackBar(text: String, anchorView: View? = this.view) {
runIfFragmentIsAttached {
@ -286,8 +251,6 @@ class AddonsManagementFragment : Fragment(R.layout.fragment_add_ons_management)
}
companion object {
private const val BUNDLE_KEY_INSTALL_EXTERNAL_ADDON_COMPLETE = "INSTALL_EXTERNAL_ADDON_COMPLETE"
// This is locale-less on purpose so that the content negotiation happens on the AMO side because the current
// user language might not be supported by AMO and/or the language might not be exactly what AMO is expecting
// (e.g. `en` instead of `en-US`).

8
app/src/main/res/navigation/nav_graph.xml
Unescape Escape View File

@ -121,13 +121,7 @@
app:destination="@id/bookmarkEditFragment" />
<action
android:id="@+id/action_global_addonsManagementFragment"
app:destination="@id/addons_management_graph">
<argument
android:name="installAddonId"
android:defaultValue="@null"
app:argType="string"
app:nullable="true" />
</action>
app:destination="@id/addons_management_graph" />
<action
android:id="@+id/action_global_trackingProtectionFragment"
app:destination="@id/trackingProtectionFragment" />

4
app/src/main/res/values/strings.xml
Unescape Escape View File

@ -640,9 +640,9 @@
<!-- Add-on Installation from AMO-->
<!-- Error displayed when user attempts to install an add-on from AMO (addons.mozilla.org) that is not supported -->
<string name="addon_not_supported_error">Add-on is not supported</string>
<string name="addon_not_supported_error" moz:removedIn="120" tools:ignore="UnusedResources">Add-on is not supported</string>
<!-- Error displayed when user attempts to install an add-on from AMO (addons.mozilla.org) that is already installed -->
<string name="addon_already_installed">Add-on is already installed</string>
<string name="addon_already_installed" moz:removedIn="120" tools:ignore="UnusedResources">Add-on is already installed</string>
<!-- Add-on process crash dialog to user -->
<!-- Title of a dialog shown to the user when enough errors have occurred with addons and they need to be temporarily disabled -->

112
app/src/test/java/org/mozilla/fenix/AppRequestInterceptorTest.kt
Unescape Escape View File

@ -10,14 +10,12 @@ import androidx.navigation.NavController
import io.mockk.every
import io.mockk.mockk
import io.mockk.mockkStatic
import io.mockk.verify
import mozilla.components.browser.errorpages.ErrorPages
import mozilla.components.browser.errorpages.ErrorType
import mozilla.components.concept.engine.request.RequestInterceptor
import mozilla.components.support.test.robolectric.testContext
import mozilla.telemetry.glean.testing.GleanTestRule
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNull
import org.junit.Before
import org.junit.Rule
import org.junit.Test
@ -25,8 +23,6 @@ import org.junit.runner.RunWith
import org.mozilla.fenix.AppRequestInterceptor.Companion.HIGH_RISK_ERROR_PAGES
import org.mozilla.fenix.AppRequestInterceptor.Companion.LOW_AND_MEDIUM_RISK_ERROR_PAGES
import org.mozilla.fenix.GleanMetrics.ErrorPage
import org.mozilla.fenix.components.Services
import org.mozilla.fenix.ext.components
import org.mozilla.fenix.ext.isOnline
import org.mozilla.fenix.ext.settings
import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
@ -53,114 +49,6 @@ class AppRequestInterceptorTest {
}
}
@Test
fun `GIVEN request to install add-on WHEN on same domain and triggered by user THEN start add-on installation`() {
val addonId = "12345678"
val result = interceptor.onLoadRequest(
engineSession = mockk(),
uri = "https://addons.mozilla.org/android/downloads/file/$addonId/test.xpi",
lastUri = "https://addons.mozilla.org/en-US/firefox/",
hasUserGesture = true,
isSameDomain = true,
isDirectNavigation = false,
isRedirect = false,
isSubframeRequest = false,
)
verify { navigationController.navigate(NavGraphDirections.actionGlobalAddonsManagementFragment(addonId)) }
assertEquals(RequestInterceptor.InterceptionResponse.Deny, result)
}
@Test
fun `GIVEN valid request to install add-on WHEN url is provided with query parameters THEN start add-on installation`() {
val addonId = "12345678"
val result = interceptor.onLoadRequest(
engineSession = mockk(),
uri = "https://addons.mozilla.org/android/downloads/file/$addonId/test.xpi?queryParam=test",
lastUri = "https://addons.mozilla.org/en-US/firefox/",
hasUserGesture = true,
isSameDomain = true,
isDirectNavigation = false,
isRedirect = false,
isSubframeRequest = false,
)
verify { navigationController.navigate(NavGraphDirections.actionGlobalAddonsManagementFragment(addonId)) }
assertEquals(RequestInterceptor.InterceptionResponse.Deny, result)
}
@Test
fun `GIVEN request to install add-on WHEN on a different domain THEN no add-on installation is started`() {
every { testContext.components.services } returns Services(testContext, mockk(relaxed = true))
val result = interceptor.onLoadRequest(
engineSession = mockk(),
uri = "https://addons.mozilla.org/android/downloads/file/12345678/test.xpi",
lastUri = "https://getpocket.com",
hasUserGesture = true,
isSameDomain = false,
isDirectNavigation = false,
isRedirect = false,
isSubframeRequest = false,
)
verify(exactly = 0) { navigationController.navigate(NavGraphDirections.actionGlobalAddonsManagementFragment()) }
assertNull(result)
}
@Test
fun `GIVEN invalid request to install add-on WHEN on same domain and triggered by user THEN no add-on installation is started`() {
every { testContext.components.services } returns Services(testContext, mockk(relaxed = true))
val result = interceptor.onLoadRequest(
engineSession = mockk(),
uri = "https://addons.mozilla.org/android/downloads/file/12345678/test.invalid",
lastUri = "https://addons.mozilla.org/en-US/firefox/",
hasUserGesture = true,
isSameDomain = true,
isDirectNavigation = false,
isRedirect = false,
isSubframeRequest = false,
)
verify(exactly = 0) { navigationController.navigate(NavGraphDirections.actionGlobalAddonsManagementFragment()) }
assertNull(result)
}
@Test
fun `GIVEN request to install add-on WHEN not triggered by user THEN no add-on installation is started`() {
every { testContext.components.services } returns Services(testContext, mockk(relaxed = true))
val result = interceptor.onLoadRequest(
engineSession = mockk(),
uri = "https://addons.mozilla.org/android/downloads/file/12345678/test.xpi",
lastUri = "https://addons.mozilla.org/en-US/firefox/",
hasUserGesture = false,
isSameDomain = true,
isDirectNavigation = false,
isRedirect = false,
isSubframeRequest = false,
)
verify(exactly = 0) { navigationController.navigate(NavGraphDirections.actionGlobalAddonsManagementFragment()) }
assertNull(result)
}
@Test
fun `GIVEN any request WHEN on same domain and triggered by user THEN no add-on installation is started`() {
every { testContext.components.services } returns Services(testContext, mockk(relaxed = true))
val result = interceptor.onLoadRequest(
engineSession = mockk(),
uri = "https://blog.mozilla.org/blog/2020/10/20/mozilla-reaction-to-u-s-v-google/",
lastUri = "https://blog.mozilla.org",
hasUserGesture = true,
isSameDomain = true,
isDirectNavigation = false,
isRedirect = false,
isSubframeRequest = false,
)
verify(exactly = 0) { navigationController.navigate(NavGraphDirections.actionGlobalAddonsManagementFragment()) }
assertNull(result)
}
@Test
fun `onErrorRequest results in correct error page for low risk level error`() {
setOf(

60
app/src/test/java/org/mozilla/fenix/addons/AddonsManagementFragmentTest.kt
Unescape Escape View File

@ -1,60 +0,0 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
package org.mozilla.fenix.addons
import android.content.Context
import androidx.coordinatorlayout.widget.CoordinatorLayout
import io.mockk.every
import io.mockk.mockk
import io.mockk.spyk
import io.mockk.verify
import mozilla.components.feature.addons.Addon
import org.junit.Before
import org.junit.Test
import org.mozilla.fenix.R
class AddonsManagementFragmentTest {
private lateinit var context: Context
private lateinit var view: CoordinatorLayout
private lateinit var fragment: AddonsManagementFragment
private val addonNotSupportedErrorMessage = "not supported"
private val addonAlreadyInstalledErrorMessage = "already installed"
@Before
fun setup() {
context = mockk(relaxed = true)
view = mockk(relaxed = true)
fragment = spyk(AddonsManagementFragment())
every { fragment.context } returns context
every { fragment.view } returns view
every { fragment.showErrorSnackBar(any()) } returns Unit
every { fragment.getString(R.string.addon_not_supported_error) } returns addonNotSupportedErrorMessage
every { fragment.getString(R.string.addon_already_installed) } returns addonAlreadyInstalledErrorMessage
every { fragment.getString(R.string.mozac_feature_addons_blocklisted_1) } returns addonAlreadyInstalledErrorMessage
}
@Test
fun `GIVEN add-on is installed from external source WHEN add-on is not supported THEN error is shown`() {
val supportedAddons = listOf(
Addon("1", downloadId = "d1"),
Addon("2", downloadId = "d2"),
)
val installAddonId = "d3"
fragment.installExternalAddon(supportedAddons, installAddonId)
verify { fragment.showErrorSnackBar(addonNotSupportedErrorMessage) }
}
@Test
fun `GIVEN add-on is installed from external source WHEN add-on is already installed THEN error is shown`() {
val addon1 = Addon("1", downloadId = "d1", installedState = mockk())
val addon2 = Addon("2", downloadId = "d2")
val supportedAddons = listOf(addon1, addon2)
fragment.installExternalAddon(supportedAddons, "d1")
verify { fragment.showErrorSnackBar(addonAlreadyInstalledErrorMessage) }
}
}
Write Preview
Loading…
Cancel
Save