Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug 1810047 - Implement UI for webextensions optional permissions. #3917

Merged
merged 2 commits into from Oct 4, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -345,8 +345,22 @@ class GeckoEngine(
GeckoWebExtension(current, runtime),
GeckoWebExtension(updated, runtime),
newPermissions.toList() + newOrigins.toList(),
) {
allow ->
) { allow ->
if (allow) result.complete(AllowOrDeny.ALLOW) else result.complete(AllowOrDeny.DENY)
}
return result
}

override fun onOptionalPrompt(
extension: org.mozilla.geckoview.WebExtension,
permissions: Array<out String>,
origins: Array<out String>,
): GeckoResult<AllowOrDeny>? {
val result = GeckoResult<AllowOrDeny>()
webExtensionDelegate.onOptionalPermissionsRequest(
GeckoWebExtension(extension, runtime),
permissions.toList() + origins.toList(),
) { allow ->
if (allow) result.complete(AllowOrDeny.ALLOW) else result.complete(AllowOrDeny.DENY)
Amejia481 marked this conversation as resolved.
Show resolved Hide resolved
}
return result
Expand Down
Expand Up @@ -1156,6 +1156,72 @@ class GeckoEngineTest {
assertEquals(GeckoResult.allow(), result)
}

@Test
fun `web extension delegate handles optional permissions prompt - allow`() {
val runtime: GeckoRuntime = mock()
val webExtensionController: WebExtensionController = mock()
whenever(runtime.webExtensionController).thenReturn(webExtensionController)

val extension = mockNativeWebExtension("test", "uri")
val permissions = arrayOf("p1", "p2")
val origins = arrayOf("p3", "p4")
val webExtensionsDelegate: WebExtensionDelegate = mock()
val engine = GeckoEngine(context, runtime = runtime)
engine.registerWebExtensionDelegate(webExtensionsDelegate)

val geckoDelegateCaptor = argumentCaptor<WebExtensionController.PromptDelegate>()
verify(webExtensionController).promptDelegate = geckoDelegateCaptor.capture()

val result = geckoDelegateCaptor.value.onOptionalPrompt(extension, permissions, origins)
assertNotNull(result)

val extensionCaptor = argumentCaptor<WebExtension>()
val onPermissionsGrantedCaptor = argumentCaptor<((Boolean) -> Unit)>()
verify(webExtensionsDelegate).onOptionalPermissionsRequest(
extensionCaptor.capture(),
eq(permissions.toList() + origins.toList()),
onPermissionsGrantedCaptor.capture(),
)
val current = extensionCaptor.value as mozilla.components.browser.engine.gecko.webextension.GeckoWebExtension
assertEquals(extension, current.nativeExtension)

onPermissionsGrantedCaptor.value.invoke(true)
assertEquals(GeckoResult.allow(), result)
}

@Test
fun `web extension delegate handles optional permissions prompt - deny`() {
val runtime: GeckoRuntime = mock()
val webExtensionController: WebExtensionController = mock()
whenever(runtime.webExtensionController).thenReturn(webExtensionController)

val extension = mockNativeWebExtension("test", "uri")
val permissions = arrayOf("p1", "p2")
val origins = emptyArray<String>()
val webExtensionsDelegate: WebExtensionDelegate = mock()
val engine = GeckoEngine(context, runtime = runtime)
engine.registerWebExtensionDelegate(webExtensionsDelegate)

val geckoDelegateCaptor = argumentCaptor<WebExtensionController.PromptDelegate>()
verify(webExtensionController).promptDelegate = geckoDelegateCaptor.capture()

val result = geckoDelegateCaptor.value.onOptionalPrompt(extension, permissions, origins)
assertNotNull(result)

val extensionCaptor = argumentCaptor<WebExtension>()
val onPermissionsGrantedCaptor = argumentCaptor<((Boolean) -> Unit)>()
verify(webExtensionsDelegate).onOptionalPermissionsRequest(
extensionCaptor.capture(),
eq(permissions.toList() + origins.toList()),
onPermissionsGrantedCaptor.capture(),
)
val current = extensionCaptor.value as mozilla.components.browser.engine.gecko.webextension.GeckoWebExtension
assertEquals(extension, current.nativeExtension)

onPermissionsGrantedCaptor.value.invoke(false)
assertEquals(GeckoResult.deny(), result)
}

@Test
fun `web extension delegate notified of browser actions from built-in extensions`() {
val runtime = mock<GeckoRuntime>()
Expand Down
Expand Up @@ -48,6 +48,18 @@ sealed class WebExtensionPromptRequest {
val onConfirm: (Boolean) -> Unit,
) : AfterInstallation(extension)

/**
* Value type that represents a request for a (optional) permission prompt.
* @property extension The [WebExtension] that requested the dialog to be shown.
* @property permissions The optional permissions to list in the dialog.
* @property onConfirm A callback indicating whether the permissions were granted or not.
*/
data class OptionalPermissions(
override val extension: WebExtension,
val permissions: List<String>,
val onConfirm: (Boolean) -> Unit,
) : AfterInstallation(extension)

/**
* Value type that represents a request for showing post-installation prompt.
* Normally used to give users an opportunity to enable the [extension] in private browsing mode.
Expand Down
Expand Up @@ -140,6 +140,20 @@ interface WebExtensionDelegate {
onPermissionsGranted: ((Boolean) -> Unit),
) = Unit

/**
* Invoked when a web extension requests optional permissions. This requires user interaction since the
* user needs to grant or revoke these optional permissions.
*
* @param extension The [WebExtension].
* @param permissions The list of all the optional permissions.
* @param onPermissionsGranted A callback to indicate if the optional permissions have been granted or not.
*/
fun onOptionalPermissionsRequest(
extension: WebExtension,
permissions: List<String>,
onPermissionsGranted: ((Boolean) -> Unit),
) = Unit

/**
* Invoked when the list of installed extensions has been updated in the engine
* (the web extension runtime). This happens as a result of debugging tools (e.g
Expand Down
Expand Up @@ -33,6 +33,7 @@ private const val KEY_DIALOG_WIDTH_MATCH_PARENT = "KEY_DIALOG_WIDTH_MATCH_PARENT
private const val KEY_POSITIVE_BUTTON_BACKGROUND_COLOR = "KEY_POSITIVE_BUTTON_BACKGROUND_COLOR"
private const val KEY_POSITIVE_BUTTON_TEXT_COLOR = "KEY_POSITIVE_BUTTON_TEXT_COLOR"
private const val KEY_POSITIVE_BUTTON_RADIUS = "KEY_POSITIVE_BUTTON_RADIUS"
private const val KEY_FOR_OPTIONAL_PERMISSIONS = "KEY_FOR_OPTIONAL_PERMISSIONS"
private const val DEFAULT_VALUE = Int.MAX_VALUE

/**
Expand Down Expand Up @@ -82,6 +83,14 @@ class PermissionsDialogFragment : AppCompatDialogFragment() {
DEFAULT_VALUE,
)

/**
* This flag is used to adjust the permissions prompt for optional permissions (instead of asking
* users to grant the required permissions at install time, which is the default).
*/
internal val forOptionalPermissions: Boolean
get() =
safeArguments.getBoolean(KEY_FOR_OPTIONAL_PERMISSIONS)

override fun onCreateDialog(savedInstanceState: Bundle?): Dialog {
val sheetDialog = Dialog(requireContext())
sheetDialog.requestWindowFeature(Window.FEATURE_NO_TITLE)
Expand Down Expand Up @@ -133,16 +142,24 @@ class PermissionsDialogFragment : AppCompatDialogFragment() {
false,
)

rootView.findViewById<TextView>(R.id.title).text =
requireContext().getString(
R.string.mozac_feature_addons_permissions_dialog_title,
addon.translateName(requireContext()),
)
rootView.findViewById<TextView>(R.id.title).text = requireContext().getString(
if (forOptionalPermissions) {
R.string.mozac_feature_addons_optional_permissions_dialog_title
} else {
R.string.mozac_feature_addons_permissions_dialog_title
},
addon.translateName(requireContext()),
)
rootView.findViewById<TextView>(R.id.permissions).text = buildPermissionsText()

val positiveButton = rootView.findViewById<Button>(R.id.allow_button)
val negativeButton = rootView.findViewById<Button>(R.id.deny_button)

if (forOptionalPermissions) {
positiveButton.text = requireContext().getString(R.string.mozac_feature_addons_permissions_dialog_allow)
negativeButton.text = requireContext().getString(R.string.mozac_feature_addons_permissions_dialog_deny)
}

positiveButton.setOnClickListener {
onPositiveButtonClicked?.invoke(addon)
dismiss()
Expand Down Expand Up @@ -186,7 +203,12 @@ class PermissionsDialogFragment : AppCompatDialogFragment() {
val permissions = addon.translatePermissions(requireContext())

if (permissions.isNotEmpty()) {
permissionsText += getString(R.string.mozac_feature_addons_permissions_dialog_subtitle) + "\n\n"
permissionsText += if (forOptionalPermissions) {
getString(R.string.mozac_feature_addons_optional_permissions_dialog_subtitle)
} else {
getString(R.string.mozac_feature_addons_permissions_dialog_subtitle)
}
permissionsText += "\n\n"
permissions.forEachIndexed { index, item ->
val brakeLine = if (index + 1 != permissions.size) "\n\n" else ""
permissionsText += "• $item $brakeLine"
Expand All @@ -207,6 +229,7 @@ class PermissionsDialogFragment : AppCompatDialogFragment() {
*/
fun newInstance(
addon: Addon,
forOptionalPermissions: Boolean = false,
promptsStyling: PromptsStyling? = PromptsStyling(
gravity = Gravity.BOTTOM,
shouldWidthMatchParent = true,
Expand All @@ -219,6 +242,7 @@ class PermissionsDialogFragment : AppCompatDialogFragment() {

arguments.apply {
putParcelable(KEY_ADDON, addon)
putBoolean(KEY_FOR_OPTIONAL_PERMISSIONS, forOptionalPermissions)

promptsStyling?.gravity?.apply {
putInt(KEY_DIALOG_GRAVITY, this)
Expand Down
Expand Up @@ -117,10 +117,18 @@
<string name="mozac_feature_addons_remove">Remove</string>
<!-- This is the title of a dialog that asks the users if they to install or not an add-on. %1$s is the add-on name. -->
<string name="mozac_feature_addons_permissions_dialog_title">Add %1$s?</string>
<!-- This is the title of a dialog that asks the users to grant optional permissions. %1$s is the add-on name. -->
<string name="mozac_feature_addons_optional_permissions_dialog_title">%1$s requests additional permissions.</string>
<!-- This is the subtitle of a dialog that asks the users if they want to install or not an add-on, the subtitle lists the permissions that this add-on requires, the permissions will dynamically be listed after the ":". -->
<string name="mozac_feature_addons_permissions_dialog_subtitle">It requires your permission to:</string>
<!-- This is the subtitle of a dialog that asks the users to grant optional permissions, the subtitle lists the permissions that this add-on requests, the permissions will dynamically be listed after the ":". -->
<string name="mozac_feature_addons_optional_permissions_dialog_subtitle">It wants to:</string>
<!-- This is a button to add (install) an add-on . -->
<string name="mozac_feature_addons_permissions_dialog_add">Add</string>
<!-- This is a button to allow the optional permissions requested by an add-on . -->
<string name="mozac_feature_addons_permissions_dialog_allow">Allow</string>
<!-- This is a button to deny the optional permissions requested by an add-on . -->
<string name="mozac_feature_addons_permissions_dialog_deny">Deny</string>
<!-- This is a button to cancel the add-on installation . -->
<string name="mozac_feature_addons_permissions_dialog_cancel">Cancel</string>
<!-- Accessibility content description to install add-on button. -->
Expand Down
Expand Up @@ -16,6 +16,7 @@ import mozilla.components.feature.addons.R
import mozilla.components.feature.addons.ui.PermissionsDialogFragment.PromptsStyling
import mozilla.components.support.test.mock
import mozilla.components.support.test.robolectric.testContext
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue
import org.junit.Test
Expand Down Expand Up @@ -150,12 +151,70 @@ class PermissionsDialogFragmentTest {
assertFalse(permissionTextView.text.contains(testContext.getString(R.string.mozac_feature_addons_permissions_dialog_subtitle)))
}

@Test
fun `build dialog for optional permissions`() {
val addon = Addon(
"id",
translatableName = mapOf(Addon.DEFAULT_LOCALE to "my_addon"),
permissions = listOf("privacy", "https://example.org/", "tabs"),
)
val fragment = createPermissionsDialogFragment(addon, forOptionalPermissions = true)

doReturn(testContext).`when`(fragment).requireContext()
val dialog = fragment.onCreateDialog(null)
dialog.show()

val addonName = addon.translateName(testContext)
val titleTextView = dialog.findViewById<TextView>(R.id.title)
val permissionTextView = dialog.findViewById<TextView>(R.id.permissions)
val allowButton = dialog.findViewById<Button>(R.id.allow_button)
val denyButton = dialog.findViewById<Button>(R.id.deny_button)
val permissionText = fragment.buildPermissionsText()

assertEquals(
titleTextView.text,
testContext.getString(R.string.mozac_feature_addons_optional_permissions_dialog_title, addonName),
)

assertTrue(permissionText.contains(testContext.getString(R.string.mozac_feature_addons_optional_permissions_dialog_subtitle)))
assertTrue(permissionText.contains(testContext.getString(R.string.mozac_feature_addons_permissions_privacy_description)))
assertTrue(
permissionText.contains(
testContext.getString(
R.string.mozac_feature_addons_permissions_one_site_description,
"example.org",
),
),
)
assertTrue(permissionText.contains(testContext.getString(R.string.mozac_feature_addons_permissions_tabs_description)))

assertTrue(permissionTextView.text.contains(testContext.getString(R.string.mozac_feature_addons_optional_permissions_dialog_subtitle)))
assertTrue(permissionTextView.text.contains(testContext.getString(R.string.mozac_feature_addons_permissions_privacy_description)))
assertTrue(
permissionTextView.text.contains(
testContext.getString(
R.string.mozac_feature_addons_permissions_one_site_description,
"example.org",
),
),
)
assertTrue(permissionTextView.text.contains(testContext.getString(R.string.mozac_feature_addons_permissions_tabs_description)))

assertEquals(allowButton.text, testContext.getString(R.string.mozac_feature_addons_permissions_dialog_allow))
assertEquals(denyButton.text, testContext.getString(R.string.mozac_feature_addons_permissions_dialog_deny))
}

private fun createPermissionsDialogFragment(
addon: Addon,
promptsStyling: PromptsStyling? = null,
forOptionalPermissions: Boolean = false,
): PermissionsDialogFragment {
return spy(
PermissionsDialogFragment.newInstance(addon, promptsStyling = promptsStyling),
PermissionsDialogFragment.newInstance(
addon = addon,
promptsStyling = promptsStyling,
forOptionalPermissions = forOptionalPermissions,
),
).apply {
doNothing().`when`(this).dismiss()
}
Expand Down
Expand Up @@ -307,6 +307,22 @@ object WebExtensionSupport {
)
}

override fun onOptionalPermissionsRequest(
extension: WebExtension,
permissions: List<String>,
onPermissionsGranted: ((Boolean) -> Unit),
) {
store.dispatch(
WebExtensionAction.UpdatePromptRequestWebExtensionAction(
WebExtensionPromptRequest.AfterInstallation.OptionalPermissions(
extension,
permissions,
onPermissionsGranted,
),
),
)
}

override fun onExtensionListUpdated() {
installedExtensions.clear()
store.dispatch(WebExtensionAction.UninstallAllWebExtensionsAction)
Expand Down
Expand Up @@ -985,4 +985,23 @@ class WebExtensionSupportTest {
verify(ext).registerActionHandler(eq(engineSession), actionHandlerCaptor.capture())
verify(ext).registerTabHandler(eq(engineSession), tabHandlerCaptor.capture())
}

@Test
fun `reacts to optional permissions request`() {
val store = spy(BrowserStore())
val engine: Engine = mock()
val ext: WebExtension = mock()
val permissions = listOf("perm1", "perm2")
val onPermissionsGranted: ((Boolean) -> Unit) = mock()
val delegateCaptor = argumentCaptor<WebExtensionDelegate>()
WebExtensionSupport.initialize(engine, store)
verify(engine).registerWebExtensionDelegate(delegateCaptor.capture())

delegateCaptor.value.onOptionalPermissionsRequest(ext, permissions, onPermissionsGranted)
verify(store).dispatch(
WebExtensionAction.UpdatePromptRequestWebExtensionAction(
WebExtensionPromptRequest.AfterInstallation.OptionalPermissions(ext, permissions, onPermissionsGranted),
),
)
}
}