Merge #6301

6301: Closes #6299: Auto reject any media site permission when a system permission is not allowed first r=rocketsroger a=Amejia481




Co-authored-by: Arturo Mejia <[email protected]>

components/feature/sitepermissions/src/main/java/mozilla/components/feature/sitepermissions/SitePermissionsFeature.kt

@@ -4,6 +4,7 @@
 
 package mozilla.components.feature.sitepermissions
 
+import android.Manifest.permission.CAMERA
 import android.Manifest.permission.RECORD_AUDIO
 import android.annotation.SuppressLint
 import android.content.Context
@@ -221,9 +222,11 @@ class SitePermissionsFeature(
         request: PermissionRequest
     ): SitePermissionsDialogFragment? {
 
-        // Preventing this behavior https://github.com/mozilla-mobile/android-components/issues/2668
-        if (request.isMicrophone && isMicrophoneAndroidPermissionNotGranted) {
+        // We want to warranty that all media permissions have the required system
+        // permissions are granted first, otherwise, we reject the request
+        if (request.isMedia && !request.areAllMediaPermissionsGranted) {
             request.reject()
+            session.contentPermissionRequest.consume { true }
             return null
         }
 
@@ -482,20 +485,29 @@ class SitePermissionsFeature(
     }
 
     private val PermissionRequest.host get() = uri?.toUri()?.host ?: ""
-    private val PermissionRequest.isMicrophone: Boolean
+    private val PermissionRequest.isMedia: Boolean
         get() {
-            if (containsVideoAndAudioSources()) {
-                return false
-            }
             return when (permissions.first()) {
+                is ContentVideoCamera, is ContentVideoCapture,
                 is ContentAudioCapture, is ContentAudioMicrophone -> true
                 else -> false
             }
         }
 
-    private val isMicrophoneAndroidPermissionNotGranted: Boolean
+    private val PermissionRequest.areAllMediaPermissionsGranted: Boolean
         get() {
-            return !context.isPermissionGranted(RECORD_AUDIO)
+            val systemPermissions = mutableListOf<String>()
+            permissions.forEach { permission ->
+                when (permission) {
+                    is ContentVideoCamera, is ContentVideoCapture -> {
+                        systemPermissions.add(CAMERA)
+                    }
+                    is ContentAudioCapture, is ContentAudioMicrophone -> {
+                        systemPermissions.add(RECORD_AUDIO)
+                    }
+                }
+            }
+            return systemPermissions.all { context.isPermissionGranted((it)) }
         }
 
     internal class SitePermissionsRequestObserver(

components/feature/sitepermissions/src/test/java/mozilla/components/feature/sitepermissions/SitePermissionsFeatureTest.kt

@@ -215,6 +215,7 @@ class SitePermissionsFeatureTest {
             mockStorage = mock()
             session.contentPermissionRequest = Consumable.from(permissionRequest)
 
+            grantPermission(Manifest.permission.CAMERA)
             grantPermission(Manifest.permission.RECORD_AUDIO)
             runBlocking {
                 val prompt = sitePermissionFeature.onContentPermissionRequested(session, permissionRequest)
@@ -722,7 +723,7 @@ class SitePermissionsFeatureTest {
             }
 
             session.contentPermissionRequest = Consumable.from(permissionRequest)
-
+            grantPermission(Manifest.permission.CAMERA)
             runBlocking {
                 val prompt = sitePermissionFeature.onContentPermissionRequested(session, permissionRequest)
 
@@ -760,6 +761,8 @@ class SitePermissionsFeatureTest {
         }
 
         session.contentPermissionRequest = Consumable.from(permissionRequest)
+        grantPermission(Manifest.permission.CAMERA)
+        grantPermission(Manifest.permission.RECORD_AUDIO)
 
         runBlocking {
             val prompt = sitePermissionFeature.onContentPermissionRequested(session, permissionRequest)
@@ -797,6 +800,8 @@ class SitePermissionsFeatureTest {
         }
 
         session.contentPermissionRequest = Consumable.from(permissionRequest)
+        grantPermission(Manifest.permission.CAMERA)
+        grantPermission(Manifest.permission.RECORD_AUDIO)
 
         runBlocking {
             val prompt = sitePermissionFeature.onContentPermissionRequested(session, permissionRequest)
@@ -944,6 +949,44 @@ class SitePermissionsFeatureTest {
         assertEquals(ALLOWED, sitePermissions.autoplayInaudible)
     }
 
+    @Test
+    fun `any media request must be rejected WHEN system permissions are not granted first`() {
+        val permissions = listOf(
+                ContentVideoCapture("", "back camera"),
+                ContentVideoCamera("", "front camera"),
+                ContentAudioCapture(),
+                ContentAudioMicrophone()
+        )
+
+        permissions.forEach { permission ->
+            val session = getSelectedSession()
+            var grantWasCalled = false
+
+            val permissionRequest: PermissionRequest = object : PermissionRequest {
+                override val uri: String?
+                    get() = "http://www.mozilla.org"
+                override val permissions: List<Permission>
+                    get() = listOf(permission)
+
+                override fun grant(permissions: List<Permission>) {
+                    grantWasCalled = true
+                }
+
+                override fun reject() = Unit
+            }
+
+            mockStorage = mock()
+            session.contentPermissionRequest = Consumable.from(permissionRequest)
+
+            runBlocking {
+                val prompt = sitePermissionFeature.onContentPermissionRequested(session, permissionRequest)
+                assertNull(prompt)
+                assertFalse(grantWasCalled)
+                assertTrue(session.contentPermissionRequest.isConsumed())
+            }
+        }
+    }
+
     private fun mockFragmentManager(): FragmentManager {
         val fragmentManager: FragmentManager = mock()
         val transaction: FragmentTransaction = mock()

docs/changelog.md

@@ -35,6 +35,9 @@ permalink: /changelog/
   *  ⚠️ **This is a breaking change**: `FxaWebChannelFeature` takes a `ServerConfig` as 4th parameter to ensure incoming WebChannel messages
      are sent by the expected FxA host.
 
+* **feature-sitepermissions**
+  * Fixed issue [#6299](https://github.com/mozilla-mobile/android-components/issues/6299), from now on, any media requests like a microphone or a camera permission will require the system permissions to be granted before a dialog can be shown.
+
 * **service-sync-logins**
   * ⚠️ **This is a breaking change**: `DefaultLoginValidationDelegate`, `GeckoLoginStorageDelegate` constructors changed to take `Lazy<LoginsStorage>` instead of `LoginsStorage`, to facilitate late initialization.