systers / FirstAide-Android

FirstAide is a application to help the Peace Corps Volunteers who are victims of sexual harassment.
23 stars 104 forks source link

Requesting runtime permissions for Android M #160

Closed rohands closed 8 years ago

rohands commented 8 years ago

Quoting developers.android.com,

On all versions of Android, your app needs to declare both the normal and the dangerous permissions it needs in its app's manifest.
- If the device is running Android 5.1 or lower, or your app's target SDK is 22 or lower: If you list a dangerous permission in your manifest, the user has to grant the permission when they install the app; if they do not grant the permission, the system does not install the app at all.

- If the device is running Android 6.0 or higher, and your app's target SDK is 23 or higher: The app has to list the permissions in the manifest, and it must request each dangerous permission it needs while the app is running. The user can grant or deny each permission, and the app can continue to run with limited capabilities even if the user denies a permission request.

Looks like Android Manifest file needs some changes. Before sending message in CircleOfTrust, I think we should mention using sms feature as a dangerous permission in the manifest.

yogeshbalan commented 8 years ago

I claim this issue. I will add add the popup dialog requesting permission to send sms before sending sms in Android M devices. Thanks

sandarumk commented 8 years ago

Hi yogeshbalan, The issue usually belongs to the one who reports the issue unless it is not from a community member. Please go through the README first. You have to find your own issues, report them, once they are approved, you can work on them. There is a wiki too. Before you report, go through the closed issues as well to avoid duplicates

yogeshbalan commented 8 years ago

Okay sandarumk. Thanks for helping. I will keep that in mind.

rohands commented 8 years ago

@sandarumk Any updates? Can I go ahead with the implementation?

rohands commented 8 years ago

@sandarumk Can I proceed?

sandarumk commented 8 years ago

The circle of trust should send the message using a single click because it is being used in extreme circumstances. Therefore adding another button click should be stopped. So what should be done is taking the permission when the user installs it. What is the current behavior of Android M in this case? It should be similar to the older versions. If not you can fix that

rohands commented 8 years ago

SEND_SMS is classified as a dangerous permission. If listed in the manifest file, the system will NOT grant this permission directly during installation. Hence, the user can grant/decline this permission. So the point is that the system doesn't grant the permission during installation. But again, all this would matter only if the target SDK is set to 23 or higher. The current target SDK is at 22. So, it does not matter. All the permissions mentioned in the Android Manifest file are granted during the installation of the app itself. This could be a serious concern during the later stages of development of the app (setting target as 23) For now, nope no issues.

yogeshbalan commented 8 years ago

The best practice, IMO is to lower the targetSdkVersion to 21, and android will do the rest. OR take permission while showing app-intro.

rohands commented 8 years ago

@yogeshbalan the problem is with targetSdkVersion 23 or higher. The current implementation (22) will not pose any problems.

sandarumk commented 8 years ago

http://inthecheesefactory.com/blog/things-you-need-to-know-about-android-m-permission-developer-edition/en This will solve all the problems you are having. So since our target version is 22 we do not have any problem as rohan said. So there is no need of worrying this for now . So I think there won't be any changes necessary in the manifest file as you mentioned in your issue right @rohands .

rohands commented 8 years ago

@sandarumk yes, no problem in the current implementation.

sandarumk commented 8 years ago

So we will keep this as it is. If we make our target version higher, we will revisit this. But for the time being this is not necessary.