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

Question on permissions #227

Closed
IzzySoft opened this issue Mar 6, 2024 · 22 comments
Closed

Question on permissions #227

IzzySoft opened this issue Mar 6, 2024 · 22 comments

Comments

@IzzySoft
Copy link

IzzySoft commented Mar 6, 2024

My scanner recently got additional checks implemented, and on your latest release reported:

! repo/remix.myplayer_16201.apk declares sensitive permission(s):
  android.permission.MANAGE_EXTERNAL_STORAGE android.permission.REQUEST_INSTALL_PACKAGES
  android.permission.READ_EXTERNAL_STORAGE android.permission.SYSTEM_ALERT_WINDOW
  android.permission.READ_MEDIA_AUDIO android.permission.READ_MEDIA_IMAGES
! repo/remix.myplayer_16201.apk contains signature block blobs: 0x504b4453 (DEPENDENCY_INFO_BLOCK; GOOGLE)

Now I could easily align READ_EXTERNAL_STORAGE and READ_MEDIA_AUDIO being needed to access the audio files to play. But could you please clarify what the other permissions are needed for? Thanks in advance!

Btw, the DEPENDENCY_INFO_BLOCK can easily be avoided:

android {
    dependenciesInfo {
        // Disables dependency metadata when building APKs.
        includeInApk = false
        // Disables dependency metadata when building Android App Bundles.
        includeInBundle = false
    }
}

For some background: that BLOB is supposed to be just a binary representation of your app's dependency tree. But as it's encrypted with a public key belonging to Google, only Google can read it – and nobody else can even verify what it really contains.

@rRemix
Copy link
Owner

rRemix commented Mar 6, 2024

android.permission.MANAGE_EXTERNAL_STORAGE, such as reading lyric file, blacklist
android.permission.SYSTEM_ALERT_WINDOW, desktop lyric
android.permission.REQUEST_INSTALL_PACKAGES, in-app upgrade, but it's only available in non-google channel

@Henry-ZHR
Copy link
Contributor

MANAGE_EXTERNAL_STORAGE is initially for "Manual scan" function (users may want to scan files outside standard directories), and it may also solve some permission-related issues on some Chinese systems. It's not necessary for everyone.

@IzzySoft
Copy link
Author

IzzySoft commented Mar 6, 2024

in-app upgrade, but it's only available in non-google channel

It's been reported for the APK used in my repo. And self-updater are not in accordance with the repo's inclusion criteria – as those updates would bypass the checks performed in the repo. May I ask how it is configured? Enabled by default? Opt-in (so with clear consent)? Explaining the source of the update and the implications (such as bypassing repo-checks)?

@Henry-ZHR
Copy link
Contributor

DEPENDENCY_INFO_BLOCK excluded in b1f9901

@IzzySoft
Copy link
Author

Wonderful, thanks! So what's with REQUEST_INSTALL_PACKAGES now (see my last comment)?

@Henry-ZHR
Copy link
Contributor

I suggest adding a new flavor which disables the self-updater (and maybe the non-free dependencies?)

Anyway, @rRemix is the only owner of this repo :)

@IzzySoft
Copy link
Author

Thanks Henry! OK, so let's see what the owner says 😄

@rRemix
Copy link
Owner

rRemix commented Mar 29, 2024

i'll add it

@IzzySoft
Copy link
Author

Thanks, @rRemix! Please let me know when that flavor is available. I'd suggest to switch your app to that in my repo then. If it involves a new applicationId/packageName, is there a "migration path" (e.g. export/import settings)?

@rRemix
Copy link
Owner

rRemix commented Apr 3, 2024

@IzzySoft you can use flavor 'noUpdater'

@rRemix rRemix closed this as completed Apr 3, 2024
@Henry-ZHR
Copy link
Contributor

Henry-ZHR commented Apr 3, 2024

@rRemix 等下 我说开flavor是方便你那边直接release加一个 因为我以为他们是直接用你的apk 但是他们好像是自己build 那感觉不如开buildConfigField

@IzzySoft Are you building the app from source or using the binary from GitHub Release?

@rRemix
Copy link
Owner

rRemix commented Apr 3, 2024

先看他们怎么说,感觉是要自己build

@IzzySoft
Copy link
Author

IzzySoft commented Apr 5, 2024

Are you building the app from source or using the binary from GitHub Release?

I use the APKs provided by their authors – so yes, I'd need the corresponding APK at releases. Thanks!

@Henry-ZHR
Copy link
Contributor

@rRemix 那就还是放flavor吧

但是还有些小问题

  1. 大小写不能随便动 如果要动源码目录得跟着改

  2. 现在命名有点怪 感觉这仨flavor不是同级的 以及可以放另外的dimension?

@rRemix
Copy link
Owner

rRemix commented Apr 8, 2024

@rRemix 那就还是放flavor吧

但是还有些小问题

  1. 大小写不能随便动 如果要动源码目录得跟着改
  2. 现在命名有点怪 感觉这仨flavor不是同级的 以及可以放另外的dimension?

@Henry-ZHR
1.大小写的问题我已经改回去了
2.是有那么点怪,但是google渠道的又不存在是否有自动更新

@Henry-ZHR
Copy link
Contributor

Henry-ZHR commented Apr 8, 2024

@rRemix

  1. 其实我也觉得用大小写分隔单词更合理 只是提醒你记得改源码目录 Linux文件系统一般区分大小写(

  2. https://developer.android.com/build/build-variants#filter-variants 可以filter掉google+updater组合?

以及 忘说了 应该其他都算非google渠道 如果在同一个dimension应该不用依赖billingclient?(在新的dimension就没有这个问题

@IzzySoft
Copy link
Author

Today's release still has my scanner report

android.permission.REQUEST_INSTALL_PACKAGES android.permission.READ_MEDIA_IMAGES

So what did I miss concerning the latter? And will there be an APK for the NoUpdater flavor?

@Henry-ZHR
Copy link
Contributor

Sorry, I forgot android.permission.REQUEST_INSTALL_PACKAGES. It will be included whether self-updater is enabled or not until #248 is merged.

@IzzySoft
Copy link
Author

Thanks! So that means after that PR is merged, android.permission.REQUEST_INSTALL_PACKAGES will be gone? And what about android.permission.READ_MEDIA_IMAGES?

@Henry-ZHR
Copy link
Contributor

READ_MEDIA_IMAGES may be needed to read the cover image of music

We need @rRemix or someone else who is familiar with Android permission system to check whether it's really necessary

@IzzySoft
Copy link
Author

That would be one way to find out. If you have the chance, you could just compile an APK with that permission commented out and try that on-device (or EMU). AFAIK READ_MEDIA_IMAGES refers to photos (DCIM) and maybe screenshots, but not to images embedded in IDv3 or such – but I'm not 100% sure and thus would appreciate if someone could "tell definitely". Thanks for taking care!

@Henry-ZHR
Copy link
Contributor

APlayer does read the cover not only directly from ID3 tag but also from MediaStore. For example:

Uri.parse("content://media/external/audio/albumart/"),

It should require READ_MEDIA_AUDIO only, but I'm not sure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants