Demizo / Daily_You

Every day is worth remembering...
GNU General Public License v3.0
127 stars 6 forks source link

Permissions only for audio and photos (not all files) #55

Closed serrq closed 10 months ago

serrq commented 1 year ago

I don't want to give the "all files" permission to Daily You.

This is dangerous permission.

Android 13 allows the media files permission. Ask for this one.

Demizo commented 1 year ago

I agree! There are a few things blocking this change for now. One is compatibility with older android versions, but my understanding is if you offer both it will only request the media permission on modern android versions so that may be a non-issue. It is also worth noting that the app deals with more than just media files and allows for storage anywhere in the file system. This is flexibility I don't want to lose quite yet. I believe android has new APIs to offer these features without the risky blanket permission, but my hands are somewhat tied until Flutter packages update to the new standard (this isn't a native android app).

With that said I will look into this issue and potential ways to phase out this permission. I will clarify that this permission doesn't grant access to "important" folders like "Downloads" or other internal app folders.

IzzySoft commented 10 months ago

My security scanner also just brought this up:

! repo/com.demizo.daily_you_180.apk declares risky permissions: android.permission.MANAGE_EXTERNAL_STORAGE

One is compatibility with older android versions

Those do not even know of this permission. They instead use READ_EXTERNAL_STORAGE and WRITE_EXTERNAL_STORAGE. You declare MANAGE_EXTERNAL_STORAGE, which is mostly reserved for file managers. Since Android 5, there's the Storage Access Framework (SAF) available, which offers those using your app to choose the location(s) your app shall have access to. You should rather use that. MANAGE_EXTERNAL_STORAGE was only introduced with Android 10, so "older Android versions" don't even know of it.

if you offer both it will only request the media permission on modern android versions

No. "Modern Android versions" have their own permissions for media storage (READ_MEDIA_STORAGE and WRITE_MEDIA_STORAGE). If you grant them MANAGE_EXTERNAL_STORAGE that means full access to all accessible storage (only excluding areas where file system permissions block it, like /data/data/*). See the documentation of this permission for details.

the app deals with more than just media files and allows for storage anywhere in the file system. This is flexibility I don't want to lose quite yet.

That's exactly what SAF is for, in your case.

I believe android has new APIs to offer these features without the risky blanket permission

Since Android 5, yes. That's for almost 10 years now. And I very much doubt that Flutter cannot deal with that. I'm not a developer, especially not a Flutter one, but a quick search for "android storage access framework flutter" has as it's first result the package you most likely need for that: shared_storage – and there's a second one which might be more fitting: saf. Maybe you could take a look at those? :wink: If you need a dive into the handling, here's an article on the topic which can be helpful.

Please see you can get MANAGE_EXTERNAL_STORAGEremoved. It raises red flags with those who might stumble upon it and consider your app "risky" just because of this. Thanks in advance!

Demizo commented 10 months ago

@IzzySoft I will look into it again. I did in the past and had the ability to backup files to using SAF but ran into problems; however, at the time I was focused mainly on external storage providers #28. I want whatever search engine you have because I didn't find any of those last time I looked into this, thanks!

IzzySoft commented 10 months ago

I want whatever search engine you have

That was Startpage. And I sometimes revert to Gruble. Using a bunch of others, depending on the search to perform, but these two are currently my main ones.

I did in the past and had the ability to backup files to using SAF but ran into problems

Even TitaniumBackup (the famous root app) uses SAF by default (for its backup location). But as pointed out, I'm no Android dev, so I can only report 2nd or 3rd hand here.

I will look into it again.

Much appreciated, thanks!

Demizo commented 10 months ago

I'm no Android dev, so I can only report 2nd or 3rd hand here.

Neither am I really :) Regardless, I have a proof of concept working using shared_storage. It will take awhile to make the next version of the app backwards compatible, but it looks like the permission can be removed. Thanks for resources!

IzzySoft commented 10 months ago

Thanks for your efforts! And yes, of course some testing before the POC is "put to production" :wink:

IzzySoft commented 10 months ago

And even signed with a proper release key now, congrats! Pinned the new one on your app in my repo now.