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

[Android] Removing unused permissions added at build time #5886

Closed
lucidrains opened this issue Feb 12, 2016 · 73 comments
Closed

[Android] Removing unused permissions added at build time #5886

lucidrains opened this issue Feb 12, 2016 · 73 comments
Labels
Platform: Android Android applications. Ran Commands One of our bots successfully processed a command. Resolution: Locked This issue was locked by the bot. Type: Discussion Long running discussion.

Comments

@lucidrains
Copy link

lucidrains commented Feb 12, 2016

edit: see https://medium.com/@applification/fixing-react-native-android-permissions-9e78996e9865 for a workaround -@hramos

It seems like Read/Write external storage and Read phone state permissions are automatically added to the manifest on building the android apk. Are these necessary for all React Native android apps? Is there any way to remove these permissions?

android:uses-permission#android.permission.READ_EXTERNAL_STORAGE
android:uses-permission#android.permission.WRITE_EXTERNAL_STORAGE

It feels weird that I'll see these permissions being requested if I install the app from the Play store if I'm not using them.

@facebook-github-bot
Copy link
Contributor

Hey lucidrains, thanks for reporting this issue!

React Native, as you've probably heard, is getting really popular and truth is we're getting a bit overwhelmed by the activity surrounding it. There are just too many issues for us to manage properly.

  • If you don't know how to do something or something is not working as you expect but not sure it's a bug, please ask on StackOverflow with the tag react-native or for more real time interactions, ask on Discord in the #react-native channel.
  • If this is a feature request or a bug that you would like to be fixed, please report it on Product Pains. It has a ranking feature that lets us focus on the most important issues the community is experiencing.
  • We welcome clear issues and PRs that are ready for in-depth discussion. Please provide screenshots where appropriate and always mention the version of React Native you're using. Thank you for your contributions!

@mkonicek
Copy link
Contributor

Just remove them from your AndroidManifest.xml if you don't need them. I believe these are for AsyncStorage.

@mkonicek
Copy link
Contributor

There's an awesome place to post code / ask question first before filing an issue: StackOverflow. It's the best system for Q&A. Many people from the community hang out there and will be able to see and answer your question. This lets us use the github bug tracker for bugs.

I'm posting this here because github issues haven't been working very well for us and because StackOverflow is so much better. Thanks for reading! :)

@satya164
Copy link
Contributor

@mkonicek aren't they added at build time? In that case it's not easy to remove them

@kevinejohn
Copy link
Contributor

+1

Even when you don't have the READ and WRITE permissions in your AndroidManifest they still show up on build. Do you just have to remove AsyncStorage completely?

@vshy108
Copy link

vshy108 commented Feb 28, 2016

I have similar problem when I generated release apk.

Before install, it asked for permissions:

  • Photos/Media/Files

  • Device ID & call information

    but I confirmed I removed AsyncStorage completely and only have

<uses-permission android:name="android.permission.INTERNET" />

in my AndroidManifest

@Bhullnatik
Copy link
Contributor

@mkonicek @satya164 @bestander Could this issue be re-opened please? I digged around a bit, I couldn't find where those permissions were added. So far there are :

<android:uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE"/>
<android:uses-permission android:name="android.permission.READ_PHONE_STATE"/>
<android:uses-permission android:name="android.permission.READ_EXTERNAL_STORAGE"/>

Which I'm not sure what they do, but I don't think there are actually used because it wouldn't work on Android 6.+ (run-time permissions). Also it seems strange to force permissions to use a module since in NetInfoModule on Android, React-native requires YOU to add the needed permission to your manifest (See NetInfoModule.java#L38).
Anyway it's not for AsyncStorage as it uses a SQLite DB under the hood, which doesn't require any permission. Even if it used another method of storage like SharedPreferences or the internal storage to store files, it shouldn't require any permission (See Storage Options).

I'm sure this is coming from React-native, I tried on a fresh project with no other permission, and there were here once the APK generated. I also looked at every Android app in the showcase and every app has those permissions, except one which doesn't have READ_PHONE_STATE at least : Facebook Ads manager (See Permissions: View details at the bottom).

Also not completely related, but this is a permission too:

<uses-permission android:name="android.permission.SYSTEM_ALERT_WINDOW"/>

This permission (Draw over other apps) is needed in debug mode to show the error redbox, but is still present
in release mode, which is quite scary for the user (even though it is not used). I could probably submit a PR for that (have 2 different manifests for debug and release, only uses the permission in the debug one).

Sorry for the long post, but my company is in the process of releasing our new Android app, and we would really like to ship it without un-needed permissions since that's not some the Android community really likes.

Thanks for your attention.

@bestander
Copy link
Contributor

Maybe gradle does that?
I can't find any string with READ_PHONE_STATE in the code.
I don't mind reopening this but could you help us investigating this?

@bestander
Copy link
Contributor

@facebook-github-bot reopen

@facebook-github-bot
Copy link
Contributor

Okay, reopening this issue.

@facebook-github-bot facebook-github-bot added the Ran Commands One of our bots successfully processed a command. label Mar 24, 2016
@Bhullnatik
Copy link
Contributor

@bestander Yeah I think it's added automatically by Android/gradle since I couldn't find any mention of them in the code, but I'm really not sure why. I'll try to investigate quickly and get back to you/submit a PR as soon as I find something.

@Bhullnatik
Copy link
Contributor

I forgot to check the manifest merger log... Turns out the problem is quite obvious:

IMPLIED from /Users/mayouk/Dev/tmp/AwesomeProject/android/app/src/main/AndroidManifest.xml:1:1-23:12 reason: org.webkit.android_jsc has a targetSdkVersion < 4
android:uses-permission#android.permission.READ_PHONE_STATE
IMPLIED from /Users/mayouk/Dev/tmp/AwesomeProject/android/app/src/main/AndroidManifest.xml:1:1-23:12 reason: org.webkit.android_jsc has a targetSdkVersion < 4
android:uses-permission#android.permission.READ_EXTERNAL_STORAGE
IMPLIED from /Users/mayouk/Dev/tmp/AwesomeProject/android/app/src/main/AndroidManifest.xml:1:1-23:12 reason: org.webkit.android_jsc requested WRITE_EXTERNAL_STORAGE

Because android-jsc doesn't provide a targetSdkVersion, the manifest merger added the permissions for various reasons (See Implicit permissions). Apparently @astuetz already fixed this in facebookarchive/android-jsc#12 but either the new version of android-jsc wasn't published or it isn't used in React-native currently. If you can contact the maintainers of android-jsc to solve this, that would be fantastic!

@bestander
Copy link
Contributor

@brentvatne ping, is it possible to publish new android-jsc?

@ghost
Copy link

ghost commented Mar 24, 2016

@Bhullnatik what we're doing until a new version of android-jsc will be released is just stripping away the unwanted permissions in our apps' manifest like this:

<uses-permission
  android:name="android.permission.READ_PHONE_STATE"
  tools:node="remove"/>

@Bhullnatik
Copy link
Contributor

@astuetz I was looking at the manifest merger on how to fix this until it gets solved, thanks for the heads-up 😄

@brentvatne
Copy link
Collaborator

@bestander - sure I can do that, we might also want to update to r189384 as well @kmagiera?

@mouneyrac
Copy link

@astuetz thanks, it helped me, justto add to this info I had to declare the tools attribut in

 <manifest xmlns:android="http://schemas.android.com/apk/res/android"
    xmlns:tools="http://schemas.android.com/tools"
    package="com.bepaw.esportninja">. 

By default there is only xmlns:android, no xmlns:tools

@Bhullnatik
Copy link
Contributor

@bestander @brentvatne Hey! Any update on this?

@danleveille
Copy link

👍 +1 for this.

@marcshilling
Copy link

I feel like the removal of SYSTEM_ALERT_WINDOW from release builds is far more critical than the other permissions being discussed.

@Bhullnatik
Copy link
Contributor

@marcshilling It's all equally important to remove them in my opinion, but this one is a little more tricky. The easiest way to remove it is to remove it in the release AndroidManifest.xml, but the configuration is a little bit heavy to include in every React Native project.
The other way would be to remove the usage, thus not needing the permission anymore. For that I'm not sure where/for what it is used, but I didn't see any particular need that couldn't be fulfilled with something else. I could be wrong though.

@marcshilling
Copy link

marcshilling commented Jun 7, 2016

For those who want to exclude SYSTEM_ALERT_WINDOW from release builds without separate Manifest files:

In your manifest:

<uses-permission android:name="android.permission.SYSTEM_ALERT_WINDOW" tools:remove="${excludeSystemAlertWindowPermission}"/>

In app/build.gradle:

    buildTypes {
        debug {
            manifestPlaceholders = [excludeSystemAlertWindowPermission: "false"]
        }
        release {
            manifestPlaceholders = [excludeSystemAlertWindowPermission: "true"]
        }
    }

Seems to work well.

@danleveille
Copy link

@marcshilling Perfect! Thank you! It's been kind of embarrassing to have to explain that to users because I couldn't figure out how to remove it. 😕

@jbrodriguez
Copy link

jbrodriguez commented Jul 5, 2016

Hi, I've been alerted about READ_PHONE_STATE by a user of one of my apps.

I'm getting the same reason in the build log

reason: org.webkit.android_jsc has a targetSdkVersion < 4

Hopefully it gets solved in a future release.

For the time being, I'm using the permission removal workarounds mentioned here
astuetz - READ_PHONE_STATE - #5886 (comment)
marcshilling - SYSTEM_ALERT_WINDOW - #5886 (comment)

As well as adding the namespace as explained by mouneyrac (#5886 (comment))

@jbrodriguez
Copy link

So, I wasn't being able to remove SYSTEM_ALERT_WINDOW with the solution above.

Had to resort to a hack (after much prodding and poking).

in AndroidManifest.xml

    <uses-permission android:name="${permissionName}" tools:node="remove"/>

in app/build.gradle

    buildTypes {
        debug {
            manifestPlaceholders = [permissionName: "android.permission.ACCESS_FINE_LOCATION"]
            ...
        }    
        release {
            manifestPlaceholders = [permissionName: "android.permission.SYSTEM_ALERT_WINDOW"]
            ...
        }
    }

In debug mode, I remove ACCESS_FINE_LOCATION, since I don't use (neither in dev nor in prod). Any other non-essential permission could be used (BIND_TV_INPUT?).

In release mode, I remove SYSTEM_ALERT_WINDOW. If you don't forcibly remove it, it will get merged in.

Hacky, but effective 😂

@Jeiwan
Copy link

Jeiwan commented Jul 12, 2016

If you use this solution #5886 (comment), don't forget to add xmlns:tools as described here: #5886 (comment)

@satya164
Copy link
Contributor

@marcshilling I think this is something we can have in the generator template by default.

@ulfgebhardt
Copy link

ulfgebhardt commented Mar 23, 2018

The Solution for me was to modify the
./android/app/src/main/AndroidManifest.xml

I had to add the following lines

<manifest xmlns:android="http://schemas.android.com/apk/res/android"
// Added this
    xmlns:tools="http://schemas.android.com/tools"
    package="de.democracydeutschland.app"
    android:versionCode="1"
    android:versionName="1.0">

    <uses-permission android:name="android.permission.INTERNET" />
    <uses-permission android:name="android.permission.SYSTEM_ALERT_WINDOW"/>
// Added tools:node="remove"
    <uses-permission android:name="android.permission.READ_PHONE_STATE" tools:node="remove"/>
// Added READ_EXTERNAL_STORAGE and WRITE_EXTERNAL_STORAGE both with tools:node="remove"
    <uses-permission android:name="android.permission.READ_EXTERNAL_STORAGE" tools:node="remove" />
    <uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" tools:node="remove" />

    <uses-sdk
        android:minSdkVersion="16"
        android:targetSdkVersion="22" />

    ...

For Debug Purposes: ./android/app/build/outputs/logs/manifest-merger-***-report.txt

Rel: https://github.com/demokratie-live/democracy-client/blob/master/android/app/src/main/AndroidManifest.xml

This removes the App-Permission Dialog when installing via PlayStore

The one thing I dont get:

Why do I have to add rights to remove them while the app itself does not even require permission?

Grüße Ulf

<3

@ishigamii
Copy link

ishigamii commented Mar 29, 2018

The "correction" will be added in any next version of React-native ?

@sladek-jan
Copy link

sladek-jan commented Apr 17, 2018

Our app has multiple build types (4 as of now), and we wanted to remove SYSTEM_ALERT_WINDOW permissions from all of them EXCEPT debug.
I didn't want to hack up permission names through manifest placeholders, nor create 3 extra AndroidManifests with equal content.
So, in the end I remove the permission in main/AndroidManifest.xml using:

<manifest xmlns:tools="http://schemas.android.com/tools" ...>
<uses-permission tools:node="remove" android:name="android.permission.SYSTEM_ALERT_WINDOW" />
...

and then in a separate debug/AndroidManifest.xml, I add it back:

<manifest xmlns:tools="http://schemas.android.com/tools" ...>
<uses-permission tools:node="merge" android:name="android.permission.SYSTEM_ALERT_WINDOW" />
...

Because the debug AndroidManifest has a higher priority than the main one (see Merge Priorities), it will override the removal and add the permission.

Sounds strange, but seems to work well (judging from the output of aapt d permissions <apk file>).

@nkov
Copy link

nkov commented Apr 19, 2018

Congress should have asked Zuck what's up with these permissions.

@anhtuank7c
Copy link

anhtuank7c commented May 3, 2018

I think the solution is create another android/app/src/release/AndroidManifest.xml like this:

<manifest xmlns:android="http://schemas.android.com/apk/res/android"
    xmlns:tools="http://schemas.android.com/tools"
    package="com.facebook.react.demo">

    <uses-permission
        android:name="android.permission.SYSTEM_ALERT_WINDOW"
        tools:node="remove" />

    <uses-permission
        android:name="android.permission.READ_PHONE_STATE"
        tools:node="remove" />

    <application>

    </application>

</manifest>

And edit android/app/build.gradle like this:

android {
    ...
    sourceSets.release {
        manifest.srcFile 'release/AndroidManifest.xml'
    }
}

https://code.videolan.org/videolan/vlc-android/blob/923bbada6687ecab31addc1dfd3f90619976ccdf/vlc-android/build.gradle#L169

@dulmandakh
Copy link
Contributor

Closing this issue, because unnecessary permissions are removed. Please see https://github.com/facebook/react-native/blob/master/local-cli/templates/HelloWorld/android/app/src/main/AndroidManifest.xml

@Bhullnatik
Copy link
Contributor

Bhullnatik commented Aug 24, 2018

@dulmandakh This is not relevant to the issue at all. The permissions are added to the Manifest when building an APK, they are not present in the default template (as mentioned in the original issue and in #5886 (comment)). Can you please re-open the issue?

@danilobuerger
Copy link
Contributor

@dulmandakh this shouldn't have been closed. The permissions are added at build time, unrelated to your linked file

@techrah
Copy link

techrah commented Jan 15, 2019

This seems to be the official guide on removing default permissions:
http://facebook.github.io/react-native/docs/removing-default-permissions

@hophoppoppop
Copy link

i don't know if this related to this forum or not.

so because recent change of google play console permissions rules. Google will check our manifest to see android permissions we used.

although i have use 'tools:node="remove"' in every unused permissions and in release apps the permission not shown. Google still doesn't let me to publish my apps in google play console.

anyone have some problem like this? i got this problem 27-01-2019.

@ulfgebhardt
Copy link

As a followup on this our currently used Manifest:

<manifest xmlns:android="http://schemas.android.com/apk/res/android"
    xmlns:tools="http://schemas.android.com/tools"
    package="de.democracydeutschland.app">

    <uses-permission android:name="android.permission.INTERNET" />

    <!-- Default permissions which are added silently and not needed by us - hence we remove them -->
    <uses-permission android:name="android.permission.READ_PHONE_STATE" tools:node="remove" />
    <uses-permission android:name="android.permission.READ_EXTERNAL_STORAGE" tools:node="remove" />
    <uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" tools:node="remove" />

    <!-- This needs to be added in dev mode but removed in all others
         since this config is used for production aswell we need to remove it here -->
    <uses-permission android:name="android.permission.SYSTEM_ALERT_WINDOW" tools:node="remove" />

    <!-- included from rn-fetch-blob - implications of removal are unknown -->
    <uses-permission android:name="android.permission.ACCESS_WIFI_STATE" tools:node="remove" />

    ...
</manifest>

Main Manifest

<manifest xmlns:android="http://schemas.android.com/apk/res/android"
    xmlns:tools="http://schemas.android.com/tools"
    package="de.democracydeutschland.app">

    <!-- This needs to be added in dev mode but removed in all others -->
    <uses-permission android:name="android.permission.SYSTEM_ALERT_WINDOW" tools:node="replace" />

</manifest>

Dev Flavor Manifest

@dulmandakh
Copy link
Contributor

dulmandakh commented Feb 1, 2019

Above mentioned permissions are implicitly added by Android SDK because old JSC or JavaScriptCore was targeting SDK 4. In the master we have new JSC and will be used default in 0.59. So there is nothing we can do about it. Please see https://developer.android.com/studio/build/manifest-merge

@hammadzz
Copy link

Heads up the only way to remove SYSTEM_ALERT_WINDOW is to follow the official react instructions on creating a new manifest file for release.

Perhaps it used to work by using manifestPlaceholders but there must have been a change or build tool versions that is breaking it.

@dulmandakh
Copy link
Contributor

@hammadzz thank you for reminding us about the unused permission, and I created a PR to fix this #23504. I hope that it'll land soon in the master and cherry picked for 0.59.

facebook-github-bot pushed a commit that referenced this issue Feb 18, 2019
Summary:
SYSTEM_ALERT_WINDOW permission is used only for debug builds to show draw overlay views or show warnings/errors. This permissions is unused in release builds, and there is an issue regarding removing unused permissions on Android build #5886 (comment). This PR removes SYSTEM_ALERT_WINDOW from all builds bug debug.

[Android] [Changed] - SYSTEM_ALERT_WINDOW permissions available only in debug builds
Pull Request resolved: #23504

Differential Revision: D14123045

Pulled By: cpojer

fbshipit-source-id: 68829a774ff23c7cb2721076a9d3870405f48fea
ericlewis pushed a commit to ericlewis/react-native that referenced this issue Feb 20, 2019
Summary:
SYSTEM_ALERT_WINDOW permission is used only for debug builds to show draw overlay views or show warnings/errors. This permissions is unused in release builds, and there is an issue regarding removing unused permissions on Android build facebook#5886 (comment). This PR removes SYSTEM_ALERT_WINDOW from all builds bug debug.

[Android] [Changed] - SYSTEM_ALERT_WINDOW permissions available only in debug builds
Pull Request resolved: facebook#23504

Differential Revision: D14123045

Pulled By: cpojer

fbshipit-source-id: 68829a774ff23c7cb2721076a9d3870405f48fea
grabbou pushed a commit that referenced this issue Feb 27, 2019
Summary:
SYSTEM_ALERT_WINDOW permission is used only for debug builds to show draw overlay views or show warnings/errors. This permissions is unused in release builds, and there is an issue regarding removing unused permissions on Android build #5886 (comment). This PR removes SYSTEM_ALERT_WINDOW from all builds bug debug.

[Android] [Changed] - SYSTEM_ALERT_WINDOW permissions available only in debug builds
Pull Request resolved: #23504

Differential Revision: D14123045

Pulled By: cpojer

fbshipit-source-id: 68829a774ff23c7cb2721076a9d3870405f48fea
@facebook facebook locked as resolved and limited conversation to collaborators Feb 1, 2020
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Feb 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Platform: Android Android applications. Ran Commands One of our bots successfully processed a command. Resolution: Locked This issue was locked by the bot. Type: Discussion Long running discussion.
Projects
None yet
Development

No branches or pull requests