Skip to content

Why do Parse Push Notification still need "Get_Account" permission to work? #129

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

Closed
Kuoyhout opened this issue Sep 13, 2015 · 21 comments
Closed
Labels
type:feature New feature or improvement of existing feature
Milestone

Comments

@Kuoyhout
Copy link

GCM is no longer require "android.permission.GET_ACCOUNTS" to work, but why parse still need that permission.

@wangmengyan95 wangmengyan95 added this to the 1.10.3 milestone Sep 14, 2015
@wangmengyan95 wangmengyan95 self-assigned this Sep 14, 2015
@wangmengyan95
Copy link
Contributor

Hi @Kuoyhout , you are correct. GET_ACCOUNTS was required only for 4.0.3 and below. We will change the way we check the permission for push. Thanks for open this issue.

@grantland
Copy link
Contributor

To clarify, the method we use to register and utilize GCM is using Google's old documented method that doesn't require Google Play Services and requires GET_ACCOUNTS on 4.0.3 and below.

We require it in the manifest since there's no real way to programmatically check minSdkVersion and it would be confusing if developers supported 4.0.3 or below and only tested their apps on 4.0.4+, but never tested on on 4.0.3 and below.

There doesn't seem to be a good way to guarantee having GET_ACCOUNTS without strictly requiring it, so we'll be leaving it for now unless someone has a suggestion that prevents applications from missing it when required.

@Kuoyhout
Copy link
Author

@grantland So if my app only supported 4.0.4+, and I don't add this permission to my manifest, and in Android Marshmallow (6.0) I don't request user for this permission in runtime, will push notification still working?

@grantland
Copy link
Contributor

AFAIK that will not work right now since our SDK throws if you're missing GET_ACCOUNTS.

@wangmengyan95 wangmengyan95 removed their assignment Sep 16, 2015
@wangmengyan95 wangmengyan95 added type:question Support or code-level question and removed enhancement labels Sep 16, 2015
@hiBrianLee
Copy link

@grantland

AFAIK that will not work right now since our SDK throws if you're missing GET_ACCOUNTS.

With the new M permission model, GET_ACCOUNTS will result in a runtime checking of the permission since it's listed as a dangerous permission - https://developer.android.com/guide/topics/security/permissions.html#perm-groups

So are you saying that with the current SDK, push will not work on M devices (regardless of having it in the manifest)?

@grantland
Copy link
Contributor

@tigerpenguin We require it being requested, aka being in AndroidManifest.xml, but don't require it being granted. Additionally, GCM only requires it for < 4.0.4.

With these two things in mind, push will work on all devices as it's currently configured.

@grantland grantland removed this from the 1.10.3 milestone Oct 6, 2015
@jlmcdonnell
Copy link

@grantland Out of curiosity, why do you require it being requested, but not granted? I don't want this permission in my Manifest if It's not required, which it isn't by the looks of it.

@grantland
Copy link
Contributor

We require it in the manifest since there's no real way to programmatically check minSdkVersion and it would be confusing if developers supported 4.0.3 or below and only tested their apps on 4.0.4+, but never tested on on 4.0.3 and below.

@Kane-Shih
Copy link

If (GET_ACCOUNTS is not granted) {
if (runs on 4.0.4+) {
print log at warning level to notify developers to check minSdkVersion
} else {
throw exception
}
}

@jlmcdonnell
Copy link

I'm with Kane-Shih on this one, I've been forced to fork the project and change it myself, which seems really unnecessary.

@Kuoyhout
Copy link
Author

How about let developer specify the app's minSdkVersion in the initialize method?

From
Parse.initialize(context, appID, clientKey);

To
Parse.initialize(context, appID, clientKey, minSdkVersion);

@grantland
Copy link
Contributor

What @Kane-Shih posted is not possible, since there are no APIs to access minSdkVersion.

Adding an extra parameter to Parse.initialize isn't good API design since it's complicating the API for very little gain.

@grantland grantland added the state:wont-fix Won’t be fixed with a clearly stated reason label Oct 21, 2015
@Kane-Shih
Copy link

"runs on", I mean android.os.Build.VERSION.SDK_INT.
It's not necessary to throw the exception on Android 4.0.4+.
Because we cannot get minSdkVersion programatically, it's developer's responsibility to check document and logcat, and to test on 4.0.3 and below if they declare it's supported. What we should do is to help developers discover this issue earlier, by good document/change-log/annoucement/tutorial...
If the client app does not support 4.0.3 and below, then it should not bother developers.

@bherbst
Copy link

bherbst commented Oct 22, 2015

A reasonable solution would be to instead require the following permission in the manifest:

<uses-permission android:name="string"
    android:maxSdkVersion="19" />

This way the permission will not be included on API 19+ (and thus users won't see that permission in marshmallow), but it will still be available to users on lower versions. Note that 19 is the lowest we can go here because the maxSdkVersion attribute wasn't added until then.

As far as the runtime check for the permission- I don't follow the logic of why you would need to check the minSdkVersion. The relevant version here is the version that the user is running on, not the minimum version supported by the app. If the user is running on anything older than 4.0.4, GCM doesn't need GET_ACCOUNTS and a simple Build.VERSION.SDK_INT check should suffice.

@grantland
Copy link
Contributor

We originally designed this to be extra cautious to prevent developers from shipping a bad build due to insufficiently testing with a 4.0.4+ device.

Since the probability of developers targeting < 4.0.3 is dwindling and there seems to be enough push from the community, we'll remove this restriction.

@grantland grantland added enhancement and removed state:wont-fix Won’t be fixed with a clearly stated reason labels Oct 22, 2015
@grantland grantland self-assigned this Oct 22, 2015
@grantland grantland added this to the 1.10.3 milestone Oct 22, 2015
@chris-mitchell
Copy link

Thanks! Any thoughts on when this might make it into a stable release?

@grantland
Copy link
Contributor

Unfortunately we don't have an ETA on the next release.

@Kazs99
Copy link

Kazs99 commented Nov 28, 2015

The next release 1.6.2 is out but GET_ACCOUNTS is required... :/

@grantland
Copy link
Contributor

@allo86
Copy link

allo86 commented Jan 14, 2016

If GET_ACCOUNTS is no longer needed, the information here needs to be updated https://parse.com/apps/quickstart#parse_push/android/native/existing

@grantland
Copy link
Contributor

We intentionally left it since it's required for Android 4.0.3 and below and if a developer was inclined to remove it they'd see the specifics GCM requires.

@mtrezza mtrezza added type:feature New feature or improvement of existing feature and removed type:improvement type:question Support or code-level question labels Dec 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feature New feature or improvement of existing feature
Projects
None yet
Development

No branches or pull requests