Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Commit

Permalink
Closes #8323: Add-on permission dialog does not prompt
Browse files Browse the repository at this point in the history
 for host permissions
  • Loading branch information
Amejia481 committed Sep 11, 2020
1 parent cd01302 commit 9ebe59d
Show file tree
Hide file tree
Showing 8 changed files with 343 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@

package mozilla.components.feature.addons

import android.content.Context
import android.os.Parcelable
import androidx.core.net.toUri
import kotlinx.android.parcel.Parcelize

/**
Expand Down Expand Up @@ -107,12 +109,11 @@ data class Addon(
) : Parcelable

/**
* Returns a list of id resources per each item on the [permissions] list.
* Returns a list of localized Strings per each item on the [permissions] list.
*/
fun translatePermissions(): List<Int> {
return localizePermissions(permissions)
fun translatePermissions(context: Context): List<String> {
return localizePermissions(permissions, context)
}

/**
* Returns whether or not this [Addon] is currently installed.
*/
Expand Down Expand Up @@ -193,8 +194,118 @@ data class Addon(
* @param permissions The list of permissions to be localized. Valid permissions can be found in
* https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/permissions#API_permissions
*/
fun localizePermissions(permissions: List<String>): List<Int> {
return permissions.mapNotNull { permissionToTranslation[it] }
fun localizePermissions(permissions: List<String>, context: Context): List<String> {
var localizedUrlAccessPermissions = emptyList<String>()
val requireAllUrlsAccess = permissions.contains("<all_urls>")
val notFoundPermissions = mutableListOf<String>()

val localizedNormalPermissions = permissions.mapNotNull {
val id = permissionToTranslation[it]
if (id == null) notFoundPermissions.add(it)
id
}.map { context.getString(it) }

if (!requireAllUrlsAccess && notFoundPermissions.isNotEmpty()) {
localizedUrlAccessPermissions = localizedURLAccessPermissions(context, notFoundPermissions)
}

return localizedNormalPermissions + localizedUrlAccessPermissions
}

@Suppress("MaxLineLength")
internal fun localizedURLAccessPermissions(context: Context, accessPermissions: List<String>): List<String> {
val localizedSiteAccessPermissions = mutableListOf<String>()
val permissionsToTranslations = mutableMapOf<String, Int>()

accessPermissions.forEach { permission ->
val id = localizeURLAccessPermission(permission)
if (id != null) {
permissionsToTranslations[permission] = id
}
}

if (permissionsToTranslations.values.any { it.isAllURLsPermission() }) {
localizedSiteAccessPermissions.add(context.getString(R.string.mozac_feature_addons_permissions_all_urls_description))
} else {
formatURLAccessPermission(permissionsToTranslations, localizedSiteAccessPermissions, context)
}
return localizedSiteAccessPermissions
}

@Suppress("MagicNumber", "ComplexMethod")
private fun formatURLAccessPermission(
permissionsToTranslations: MutableMap<String, Int>,
localizedSiteAccessPermissions: MutableList<String>,
context: Context
) {
var domainCount = 0
var siteCount = 0

loop@ for ((permission, stringId) in permissionsToTranslations) {
var host = permission.toUri().host ?: ""
when {
stringId.isDomainAccessPermission() -> {
++domainCount
host = host.removePrefix("*.")

if (domainCount > 4) continue@loop
}

stringId.isSiteAccessPermission() -> {
++siteCount
if (siteCount > 4) continue@loop
}
}
localizedSiteAccessPermissions.add(context.getString(stringId, host))
}

// If we have 4 or fewer permissions, display them all, otherwise we
// display the first 4 followed by an item that says "...plus N others"
if (domainCount > 4) {
val collapsedPermissions = domainCount - 4
val string = if (collapsedPermissions > 1) {
R.string.mozac_feature_addons_permissions_to_many_domains_description_plural
} else {
R.string.mozac_feature_addons_permissions_to_many_domains_description
}
localizedSiteAccessPermissions.add(context.getString(string, collapsedPermissions))
}
if (siteCount > 4) {
val collapsedPermissions = siteCount - 4
val string = if (collapsedPermissions > 1) {
R.string.mozac_feature_addons_permissions_to_many_sites_description_plural
} else {
R.string.mozac_feature_addons_permissions_to_many_sites_description
}
localizedSiteAccessPermissions.add(context.getString(string, collapsedPermissions))
}
}

private fun Int.isSiteAccessPermission(): Boolean {
return this == R.string.mozac_feature_addons_permissions_one_site_description
}

private fun Int.isDomainAccessPermission(): Boolean {
return this == R.string.mozac_feature_addons_permissions_sites_in_domain_description
}

private fun Int.isAllURLsPermission(): Boolean {
return this == R.string.mozac_feature_addons_permissions_all_urls_description
}

internal fun localizeURLAccessPermission(urlAccess: String): Int? {
val uri = urlAccess.toUri()
val host = (uri.host ?: "").trim()
val path = (uri.path ?: "").trim()

return when {
host == "*" || urlAccess == "<all_urls>" -> {
R.string.mozac_feature_addons_permissions_all_urls_description
}
host.isEmpty() || path.isEmpty() -> null
host.startsWith(prefix = "*.") -> R.string.mozac_feature_addons_permissions_sites_in_domain_description
else -> R.string.mozac_feature_addons_permissions_one_site_description
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,12 +183,11 @@ class PermissionsDialogFragment : AppCompatDialogFragment() {
internal fun buildPermissionsText(): String {
var permissionsText =
getString(R.string.mozac_feature_addons_permissions_dialog_subtitle) + "\n\n"
val permissions = addon.translatePermissions()
val permissions = addon.translatePermissions(requireContext())

permissions.forEachIndexed { index, item ->
val brakeLine = if (index + 1 != permissions.size) "\n\n" else ""
val permissionText = requireContext().getString(item)
permissionsText += "$permissionText $brakeLine"
permissionsText += "$item $brakeLine"
}
return permissionsText
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ class DefaultAddonUpdater(
) {
logger.info("onUpdatePermissionRequest $current")

val shouldGrantWithoutPrompt = Addon.localizePermissions(newPermissions).isEmpty()
val shouldGrantWithoutPrompt = Addon.localizePermissions(newPermissions, applicationContext).isEmpty()
val shouldShowNotification =
updateStatusStorage.isPreviouslyAllowed(applicationContext, updated.id) || shouldGrantWithoutPrompt

Expand Down Expand Up @@ -306,7 +306,7 @@ class DefaultAddonUpdater(

@VisibleForTesting
internal fun createContentText(newPermissions: List<String>): String {
val validNewPermissions = Addon.localizePermissions(newPermissions)
val validNewPermissions = Addon.localizePermissions(newPermissions, applicationContext)

val string = if (validNewPermissions.size == 1) {
R.string.mozac_feature_addons_updater_notification_content_singular
Expand All @@ -317,7 +317,7 @@ class DefaultAddonUpdater(
var permissionIndex = 1
val permissionsText =
validNewPermissions.joinToString(separator = "\n") {
"${permissionIndex++}-${applicationContext.getString(it)}"
"${permissionIndex++}-$it"
}
return "$contentText:\n $permissionsText"
}
Expand Down
12 changes: 12 additions & 0 deletions components/feature/addons/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,18 @@
<string name="mozac_feature_addons_permissions_privacy_description">Read and modify privacy settings</string>
<!-- Description for all_urls add-on permission. -->
<string name="mozac_feature_addons_permissions_all_urls_description">Access your data for all websites</string>
<!-- Description for one site access add-on permission. %1$s will be replaced by the DNS host name for which a web extension is requesting access (e.g., www.mozilla.org). -->
<string name="mozac_feature_addons_permissions_one_site_description">Access your data for %1$s</string>
<!-- Description for accessing multiple sites in one domain add-on permission. %1$s will be replaced by the DNS domain for which a web extension is requesting access (e.g., mozilla.org). -->
<string name="mozac_feature_addons_permissions_sites_in_domain_description">Access your data for sites in the %1$s domain</string>
<!-- Description for accessing multiple sites permission. %1$d will be replaced by an integer indicating the number of additional hosts for which this webextension is requesting permission. -->
<string name="mozac_feature_addons_permissions_to_many_sites_description">Access your data on %1$d other site</string>
<!-- Description for accessing multiple sites permission. %1$d will be replaced by an integer indicating the number of additional hosts for which this webextension is requesting permission. -->
<string name="mozac_feature_addons_permissions_to_many_sites_description_plural">Access your data on %1$d other sites</string>
<!-- Description for accessing multiple domains permission. %1$s will be replaced by an integer indicating the number of additional hosts for which this webextension is requesting permission. -->
<string name="mozac_feature_addons_permissions_to_many_domains_description">Access your data on %1$d other domain</string>
<!-- Description for accessing multiple domains permission. %1$s will be replaced by an integer indicating the number of additional hosts for which this webextension is requesting permission. -->
<string name="mozac_feature_addons_permissions_to_many_domains_description_plural">Access your data on %1$d other domains</string>
<!-- Description for tabs add-on permission. -->
<string name="mozac_feature_addons_permissions_tabs_description">Access browser tabs</string>
<!-- Description for unlimited_storage permission. -->
Expand Down
Loading

3 comments on commit 9ebe59d

@firefoxci-taskcluster
Copy link

Choose a reason for hiding this comment

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

Uh oh! Looks like an error! Details

Failed to get your artifact.

@firefoxci-taskcluster
Copy link

Choose a reason for hiding this comment

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

Uh oh! Looks like an error! Details

Failed to get your artifact.

@firefoxci-taskcluster
Copy link

Choose a reason for hiding this comment

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

Uh oh! Looks like an error! Details

Failed to get your artifact.

Please sign in to comment.