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

READ_PHONE_STATE permission added when using with expo (bare workflow) #3021

Closed
1 of 3 tasks
andrewjack-cat opened this issue Feb 21, 2022 · 3 comments · Fixed by #3113
Closed
1 of 3 tasks

READ_PHONE_STATE permission added when using with expo (bare workflow) #3021

andrewjack-cat opened this issue Feb 21, 2022 · 3 comments · Fixed by #3113
Labels
Needs review Issue is ready to be reviewed by a maintainer Platform: Android This issue is specific to Android Repro provided A reproduction with a snippet of code, snack or repo is provided

Comments

@andrewjack-cat
Copy link

andrewjack-cat commented Feb 21, 2022

Description

compileSdkVersion safeExtGet('compileSdkVersion', 30)

Due to no targetSdkVersion being specified in the expo gradle file. This causes the Android manifest to assume the targetSdkVersion is < 4. This causes the Android manifest merger to add the "dangerous" READ_PHONE_STATE permission.

Expected behavior

READ_PHONE_STATE should not be present in the final Android Manifest.

This can be fixed by adding this to expo/linking.gradle:

targetSdkVersion safeExtGet('targetSdkVersion', 30)

Actual behavior & steps to reproduce

READ_PHONE_STATE is present in the final Android Manifest. & can see reanimated mention in manifest-merger-logs.

uses-permission#android.permission.READ_PHONE_STATE
IMPLIED from /path/to/project/app/android/app/src/debug/AndroidManifest.xml:1:1-10:12 reason: com.swmansion.reanimated has a targetSdkVersion < 4

Snack or minimal code example

  1. create bare expo app
  2. install reanimated
  3. build debug or release app
  4. Check manifest & app/build/outputs/logs/manifest-merger-debug-report.txt for references to READ_PHONE_STATE

Package versions

name version
react-native 0.64.3
react-native-reanimated 2.4.1
NodeJS 14
Xcode 13
Java 1.8
Gradle 6.8
expo 44.0.6

Affected platforms

  • Android
  • iOS
  • Web
@andrewjack-cat andrewjack-cat added the Needs review Issue is ready to be reviewed by a maintainer label Feb 21, 2022
@github-actions github-actions bot added Missing repro This issue need minimum repro scenario Platform: Android This issue is specific to Android labels Feb 21, 2022
@andrewjack-cat
Copy link
Author

cc @Kudo

@Kudo
Copy link
Contributor

Kudo commented Mar 3, 2022

thanks @andrewjack-cat for the good catch. i am working on a better way for expo integration in reanimated and will address this issue together.

@SleeplessByte
Copy link

SleeplessByte commented Mar 13, 2022

For people running into this with eas and needing a workaround today:

In app.json add a plugin:

{
  "plugins": [
     [
        "./withAndroidRemovedPermissions",
        [
          "android.permission.READ_PHONE_STATE"
        ]
      ],
   ],
}

Then, add a new file in the root of your project, called withAndroidRemovedPermissions.js

const {
  AndroidConfig,
  createRunOncePlugin,
  withAndroidManifest,
} = require('@expo/config-plugins');

const withAndroidRemovedPermissions = (config, permissions) => {
  return withAndroidManifest(config, (config) => {
    AndroidConfig.Permissions.removePermissions(config.modResults, permissions);

    const usesPermissions = config.modResults.manifest['uses-permission'] || [];

    permissions.forEach((permission) => {
      usesPermissions.push({
        $: {
          'tools:node': 'remove',
          'android:name': permission,
        },
      });
    });

    config.modResults.manifest['uses-permission'] = usesPermissions;
    config.modResults.manifest.$['xmlns:tools'] =
      'http://schemas.android.com/tools';

    return config;
  });
};

module.exports = createRunOncePlugin(
  withAndroidRemovedPermissions,
  'withAndroidRemovedPermissions'
);

You can also use this too, for example, remove android.permission.RECORD_AUDIO, which is added by expo-av, if you do not intend to record, but only play:

{
  "plugins": [
     [
        "./withAndroidRemovedPermissions",
        [
          "android.permission.RECORD_AUDIO",
          "android.permission.READ_PHONE_STATE"
        ]
      ],
   ],
}

@github-actions github-actions bot added Repro provided A reproduction with a snippet of code, snack or repo is provided and removed Missing repro This issue need minimum repro scenario labels Mar 13, 2022
@Kudo Kudo mentioned this issue Mar 29, 2022
6 tasks
piaskowyk pushed a commit that referenced this issue Apr 1, 2022
## Description

with #3005, we don't need the expo adapter to install JSI bindings. by removing the adapter, this pr will also fix #3084 and fix #3021

## Changes

remove expo adapter

## Test code and steps to reproduce

1. `$ createNPMPackage.sh`
2. create an expo sdk44 project, `yarn add file:/path/to/react-native-reanimated-2.5.0.tgz` and test launching.
2. create an react-native 0.67 project, `yarn add file:/path/to/react-native-reanimated-2.5.0.tgz` and test launching.

## Checklist

- [ ] Included code example that can be used to test this change
- [ ] Updated TS types
- [ ] Added TS types tests
- [ ] Added unit / integration tests
- [ ] Updated documentation
- [ ] Ensured that CI passes
sponrad added a commit to sponrad/crypto-interest-checker that referenced this issue Jun 1, 2022
I ran expo upgrade

This will actually fix a bug where react-reanimated was asking for more
permissions on android than wwe are needing
software-mansion/react-native-reanimated#3021

So thats a win
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs review Issue is ready to be reviewed by a maintainer Platform: Android This issue is specific to Android Repro provided A reproduction with a snippet of code, snack or repo is provided
Projects
None yet
3 participants