fix: Remove READ_EXTERNAL_STORAGE permission requirement (backport #3018) (#3023)

Co-authored-by: Rob Wu <rob@robwu.nl>

Author: willdurand

src/extension-runners/firefox-android.js

@@ -130,7 +130,6 @@ export class FirefoxAndroidExtensionRunner {
 
     await this.adbDevicesDiscoveryAndSelect();
     await this.apkPackagesDiscoveryAndSelect();
-    await this.adbCheckRuntimePermissions();
     await this.adbForceStopSelectedPackage();
 
     // Create profile prefs (with enabled remote RDP server), prepare the
@@ -381,41 +380,6 @@ export class FirefoxAndroidExtensionRunner {
     await adbUtils.amForceStopAPK(selectedAdbDevice, selectedFirefoxApk);
   }
 
-  async adbCheckRuntimePermissions() {
-    const { adbUtils, selectedAdbDevice, selectedFirefoxApk } = this;
-
-    log.debug(`Discovering Android version for ${selectedAdbDevice}...`);
-
-    const androidVersion = await adbUtils.getAndroidVersionNumber(
-      selectedAdbDevice
-    );
-
-    if (typeof androidVersion !== 'number' || Number.isNaN(androidVersion)) {
-      throw new WebExtError(`Invalid Android version: ${androidVersion}`);
-    }
-
-    log.debug(`Detected Android version ${androidVersion}`);
-
-    if (androidVersion < 23) {
-      return;
-    }
-
-    log.debug(
-      'Checking read/write permissions needed for web-ext' +
-        `on ${selectedFirefoxApk}...`
-    );
-
-    // Runtime permissions needed to Firefox to be able to access the
-    // xpi file uploaded to the android device or emulator.
-    const requiredPermissions = ['android.permission.READ_EXTERNAL_STORAGE'];
-
-    await adbUtils.ensureRequiredAPKRuntimePermissions(
-      selectedAdbDevice,
-      selectedFirefoxApk,
-      requiredPermissions
-    );
-  }
-
   async adbPrepareProfileDir() {
     const {
       adbUtils,

src/util/adb.js

@@ -135,66 +135,6 @@ export default class ADBUtils {
       });
   }
 
-  async getAndroidVersionNumber(deviceId: string): Promise<number> {
-    const androidVersion = (
-      await this.runShellCommand(deviceId, ['getprop', 'ro.build.version.sdk'])
-    ).trim();
-
-    const androidVersionNumber = parseInt(androidVersion);
-
-    // No need to check the granted runtime permissions on Android versions < Lollypop.
-    if (isNaN(androidVersionNumber)) {
-      throw new WebExtError(
-        'Unable to discovery android version on ' +
-          `${deviceId}: ${androidVersion}`
-      );
-    }
-
-    return androidVersionNumber;
-  }
-
-  // Raise an UsageError when the given APK does not have the required runtime permissions.
-  async ensureRequiredAPKRuntimePermissions(
-    deviceId: string,
-    apk: string,
-    permissions: Array<string>
-  ): Promise<void> {
-    const permissionsMap = {};
-
-    // Initialize every permission to false in the permissions map.
-    for (const perm of permissions) {
-      permissionsMap[perm] = false;
-    }
-
-    // Retrieve the permissions information for the given apk.
-    const pmDumpLogs = (
-      await this.runShellCommand(deviceId, ['pm', 'dump', apk])
-    ).split('\n');
-
-    // Set to true the required permissions that have been granted.
-    for (const line of pmDumpLogs) {
-      for (const perm of permissions) {
-        if (
-          line.includes(`${perm}: granted=true`) ||
-          line.includes(`${perm}, granted=true`)
-        ) {
-          permissionsMap[perm] = true;
-        }
-      }
-    }
-
-    for (const perm of permissions) {
-      if (!permissionsMap[perm]) {
-        throw new UsageError(
-          `Required ${perm} has not be granted for ${apk}. ` +
-            'Please grant them using the Android Settings ' +
-            'or using the following adb command:\n' +
-            `\t adb shell pm grant ${apk} ${perm}\n`
-        );
-      }
-    }
-  }
-
   async amForceStopAPK(deviceId: string, apk: string): Promise<void> {
     await this.runShellCommand(deviceId, ['am', 'force-stop', apk]);
   }

tests/unit/test-extension-runners/test.firefox-android.js

@@ -117,7 +117,6 @@ function prepareSelectedDeviceAndAPKParams(
         'org.mozilla.reference.browser',
       ])
     ),
-    getAndroidVersionNumber: sinon.spy(() => Promise.resolve(20)),
     amForceStopAPK: sinon.spy(() => Promise.resolve()),
     discoverRDPUnixSocket: sinon.spy(() =>
       Promise.resolve(fakeRDPUnixSocketFile)
@@ -132,7 +131,6 @@ function prepareSelectedDeviceAndAPKParams(
     clearArtifactsDir: sinon.spy(() => Promise.resolve()),
     detectOrRemoveOldArtifacts: sinon.spy(() => Promise.resolve(true)),
     setUserAbortDiscovery: sinon.spy(() => {}),
-    ensureRequiredAPKRuntimePermissions: sinon.spy(() => Promise.resolve()),
     ...adbOverrides,
   };
 
@@ -854,92 +852,6 @@ describe('util/extension-runners/firefox-android', () => {
       sinon.assert.calledOnce(anotherCallback);
     });
 
-    it('raises an error when unable to find an android version number', async () => {
-      async function expectInvalidVersionError(version: any) {
-        const { params, fakeADBUtils } = prepareSelectedDeviceAndAPKParams();
-
-        fakeADBUtils.getAndroidVersionNumber = sinon.spy(() => {
-          return version;
-        });
-
-        const runnerInstance = new FirefoxAndroidExtensionRunner(params);
-        const promise = runnerInstance.run();
-
-        const expectedMsg = `Invalid Android version: ${version}`;
-        await assert.isRejected(promise, WebExtError);
-        await assert.isRejected(promise, expectedMsg);
-      }
-
-      await expectInvalidVersionError(undefined);
-      await expectInvalidVersionError(NaN);
-    });
-
-    it('does not check granted android permissions on Android <= 21', async () => {
-      async function expectNoGrantedPermissionDiscovery(version) {
-        const { params, fakeADBUtils } = prepareSelectedDeviceAndAPKParams();
-
-        fakeADBUtils.getAndroidVersionNumber = sinon.spy(() => {
-          return Promise.resolve(version);
-        });
-
-        const runnerInstance = new FirefoxAndroidExtensionRunner(params);
-
-        await runnerInstance.run();
-
-        sinon.assert.calledWithMatch(
-          fakeADBUtils.getAndroidVersionNumber,
-          'emulator-1'
-        );
-
-        sinon.assert.notCalled(
-          fakeADBUtils.ensureRequiredAPKRuntimePermissions
-        );
-      }
-
-      // KitKat (Android 4.4).
-      await expectNoGrantedPermissionDiscovery(19);
-      await expectNoGrantedPermissionDiscovery(21);
-      // Lollipop versions (Android 5.0 and 5.1).
-      await expectNoGrantedPermissionDiscovery(22);
-    });
-
-    it('checks the granted android permissions on Android >= 23', async () => {
-      async function testGrantedPermissionDiscovery(version) {
-        const { params, fakeADBUtils } = prepareSelectedDeviceAndAPKParams();
-
-        fakeADBUtils.getAndroidVersionNumber = sinon.spy(() => {
-          return Promise.resolve(version);
-        });
-
-        const runnerInstance = new FirefoxAndroidExtensionRunner(params);
-
-        await runnerInstance.run();
-
-        sinon.assert.calledWithMatch(
-          fakeADBUtils.getAndroidVersionNumber,
-          'emulator-1'
-        );
-
-        sinon.assert.calledWithMatch(
-          fakeADBUtils.ensureRequiredAPKRuntimePermissions,
-          'emulator-1',
-          'org.mozilla.firefox',
-          ['android.permission.READ_EXTERNAL_STORAGE']
-        );
-
-        sinon.assert.callOrder(
-          fakeADBUtils.getAndroidVersionNumber,
-          fakeADBUtils.ensureRequiredAPKRuntimePermissions
-        );
-      }
-
-      // Marshmallow (Android 6.0)
-      await testGrantedPermissionDiscovery(23);
-      // Nougat versions (Android 7.0 and 7.1.1)
-      await testGrantedPermissionDiscovery(24);
-      await testGrantedPermissionDiscovery(25);
-    });
-
     it('logs warnings on the unsupported CLI options', async () => {
       const params = prepareSelectedDeviceAndAPKParams();