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

Removed android.permission.GET_ACCOUNTS from required permissions. #223

Merged
merged 1 commit into from Dec 2, 2017

Conversation

Projects
None yet
4 participants
@noughts
Contributor

noughts commented Jul 28, 2016

From documentation (https://github.com/ParsePlatform/parse-server/wiki/Push-Configuring-Clients#configure-broadcast-receiver-and-permissions), GET_ACCOUNTS is only required for GCM on devices running Android lower than 4.0.4.
I think it should be optional.

Removed android.permission.GET_ACCOUNTS from required permissions.
From documentation (https://github.com/ParsePlatform/parse-server/wiki/Push-Configuring-Clients#configure-broadcast-receiver-and-permissions), GET_ACCOUNTS is only required for GCM on devices running Android lower than 4.0.4.
I think it should be optional.
@ghost

This comment has been minimized.

ghost commented Jul 28, 2016

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

@ghost ghost added the CLA Signed label Jul 28, 2016

@ghost

This comment has been minimized.

ghost commented Jul 28, 2016

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@richardjrossiii

This comment has been minimized.

Contributor

richardjrossiii commented Jul 28, 2016

What happens with your patch if an app targets less than android 4.0.4? Will GCM notifications break for those users? That would not be optimal.

@noughts

This comment has been minimized.

Contributor

noughts commented Aug 1, 2016

@richardjrossiii
For Android 6.0, GET_ACCOUNTS permission pops up wasted dialog on app launch.
So developer, who is developing for >=4.0.4 like me, should be able to avoid it by this patch.

Surely developer who targets <4.0.4 should use GET_ACCOUNTS permission.
Sorry for my poor English.
Thanks.

@ghost ghost added the CLA Signed label Aug 1, 2016

@miracle7

This comment has been minimized.

miracle7 commented Aug 22, 2016

@richardjrossiii Supposedly if you are using GoogleCloudMessaging.register instead of GCMRegistrar.register, you no longer need GET_ACCOUNTS for any version of android.

Source: http://stackoverflow.com/questions/18444227/get-accounts-permission-while-using-gcm-why-is-this-needed

I see a reference to GoogleCloudMessaging in Java:
ParsePushService.java#L56

But also a reference to GCMRegistrar in C#:
DeviceInfoController.cs#L47

So is it safe to remove GET_ACCOUNTS or not? I feel like it should at least be optional rather than mandatory now that Android 6 will ask users if it can access their contacts.

We are trying to go through cert on Android atm in order to be featured on Thursday and they are complaining that we are asking for too many permissions, so any update on this would be greatly appreciated :)

@ghost ghost added the CLA Signed label Aug 22, 2016

@montymxb

:octocat: approved. If there's a particular need for this to be a required permission we can consider re-adding at a later point.

@montymxb montymxb merged commit e8f333b into parse-community:master Dec 2, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment