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.

Sign up for GitHub

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

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

Merged
merged 1 commit into from
Jan 19, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 0 additions & 36 deletions src/extension-runners/firefox-android.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
60 changes: 0 additions & 60 deletions src/util/adb.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
}
Expand Down
88 changes: 0 additions & 88 deletions tests/unit/test-extension-runners/test.firefox-android.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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,
};

Expand Down Expand Up @@ -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();

Expand Down
Loading