New version of the Cardboard SDK requires 'READ_PHONE_STATE'

[R1ck77]

and I might add "and it's not documented anywhere"...

I just found out that my newly updated application requires "READ_PHONE_STATE" permission, after a rightfully concerned user reported that to me.

After a brief investigation, it resulted that

• the new version of the Cardboard SDK apparently requires it
• no cardboard aar directly declares it
• ...but the sample app treasure hunt requires it too (so it's clearly your fault ;) )

Now, I think that this might be due to this side effect (which btw would also involve an issue with the manifest merger, as cardboard apps require min=android-19, but this is another story...):

Note: If both your minSdkVersion and targetSdkVersion values are set to 3 or lower, the system implicitly grants your app this permission. If you don't need this permission, be sure your targetSdkVersion is 4 or higher.

(from READ_PHONE_STATE in the Manifest.permission documentation ), as the manifests in the aar files don't declare any minimum target.

Regardless, this is alarming stuff from a user's point of view, especially when there is no explanation on why the permission is requested. Confront on this topic "What some of those scary application permissions mean" on Android Central, to get an idea of the "general vibe" around it [EDIT if there is any doubt about that, half an hour and I already got 2 very vocal 1-star reviews on the store...].

So please consider documenting the usage you make of the permission, if it's required, or if it's not, consider removing the source of the permission from the next release and (since 0.7.1 isn't going to come out tomorrow, I guess) updating the release notes to account for the bug, thus providing developers with a quick reference for any concerned users (this thread might be too technical for most).

I'm also aware that you are probably overbooked, but it comes without saying that any effort to deal with this issue promptly, at least documentation-wise, will be greatly appreciated.

Keep up the good work, anyway: your efforts are appreciated!
Riccardo

[jdduke]

I just found out that my newly updated application requires "READ_PHONE_STATE" permission, after a rightfully concerned user reported that to me.

After a brief investigation, it resulted that

the new version of the Cardboard SDK apparently requires it
no cardboard aar directly declares it

READ_PHONE_STATE is neither requested by the Cardboard SDK nor any of the sample apps. How did you come to this conclusion? We take privacy very seriously, and would never add such a permission without due diligence.

[R1ck77]

1. git clone https://github.com/googlesamples/cardboard-java.git
2. cd cardboard-java
3. ./gradlew assembleDebug
4. aapt d permissions samples/treasurehunt/build/outputs/apk/samples-treasurehunt-debug.apk

which will yield:

 package: com.google.vrtoolkit.cardboard.samples.treasurehunt
 uses-permission: name='android.permission.INTERNET'
 uses-permission: name='android.permission.NFC'
 uses-permission: name='android.permission.VIBRATE'
 uses-permission: name='android.permission.READ_EXTERNAL_STORAGE'
 uses-permission: name='android.permission.WRITE_EXTERNAL_STORAGE'
 uses-permission: name='android.permission.READ_PHONE_STATE'  <---------- !!!!

I'm quite certain that you take privacy seriously, which is why I'm not accusing you of having added that on purpose (as a matter of fact I'm a fan of your work), but may I suggest you to take bug reports seriously as well? The fact that your demo requires the READ_PHONE_STATE was already clearly stated in my first message.

That said, I think we started with the wrong foot: I'm more than willing to collaborate and provide further details on the matter, and I'll welcome your help, as soon as you'll look into the issue.

[jdduke]

I see. That permission has never been added explicitly to the Cardboard SDK (see https://github.com/googlesamples/cardboard-java/blob/master/samples/treasurehunt/src/main/AndroidManifest.xml#L7). It looks instead like this may be a Gradle-specific quirk (see, for example, https://code.google.com/p/android/issues/detail?id=80278, commonsguy/cwac-merge#16).

What version of Android Studio are you running?

[jdduke]

What version of Android Studio are you running?

OK, ignore that. Clearly I don't use the gradle-based build setup very often. We'll investigate this issue, but just to be clear, the READ_PHONE_STATE permission is not explicitly added by the Cardboard SDK.

[R1ck77]

This is also my conclusion.

Thank you for the update: this is already something I can use, to take some heat off on the store.

If it can be of help, I think the only "local" factor involved might be the java version I'm using (you can get all other details quicker from the demo):

$ java -version
java version "1.8.0_60"
Java(TM) SE Runtime Environment (build 1.8.0_60-b27)
Java HotSpot(TM) 64-Bit Server VM (build 25.60-b23, mixed mode)
$ cat /etc/issue
Ubuntu 14.04.4 LTS \n \l

but I would be quite surprised if it would matter: I didn't update neither java or my distro since the last release, so until now the SDK seems to be the only discriminant.

As I said in the first message, I would put my money on a missing min-target in the AARs manifests (but it's just a semi-informed guess).

Thank you for your help: I look forward to hear from you!

[jdduke]

Yes, it's quite possible we're lacking some metadata with the library manifest/build files. Thanks for reporting the issue, and for the detailed bug report. I'll make sure this issue is prioritized and passed along to the right folks.

[R1ck77]

Hi @jdduke, I have further investigated the issue in order to provide my users with a fixed version, and I found the solution to the problem.

The root cause was indeed the missing <uses-sdk> element in at least library-common and library-audio (I didn't check the other aar, as I don't use them).

Adding the line:

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

to the AndroidManifest.xml of library-common and library-audio ("16" has been picked for consistency with library-core which declares the tag, but any number above 3 would have done) fixes the issue for both the treasure hunt demo, and my app.

For those who may be experiencing the same issue, here are the steps to temporarily fix the problem

1. locate in your project directory the location where the Cardboard SDK aar are stored
2. unzip each aar in turn, in a separate directory
3. check whether the AndroidManifest.xml within each contains the <uses-sdk android:minSdkVersion="16"/> and, if not, add it
4. for each modified directory, repack the content with zip, making sure to maintain the original structure, and replace the aar in your project with it.

Hope this helps.

[jdduke]

Thanks @R1ck77 , I'll pass that along.

[anakin78z]

Confirming this issue (and also the issue with getting 1 star reviews as a result).
A bit surprised nobody is assigned to this yet.

[dav-cz]

This is fixed in v0.8. Thanks for the reports!