Skip to content

Commit

Permalink
Closes mozilla-mobile#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 edd37a8 commit d665a3d
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. -->
<!-- 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

0 comments on commit d665a3d

Please sign in to comment.