anonfaded / FadCam

Seamless background video recorder for Android – ad-free and open-source, with customizable options.
GNU General Public License v3.0
376 stars 28 forks source link

Question on permissions #21

Closed IzzySoft closed 2 months ago

IzzySoft commented 2 months ago

Now that we moved to per-ABI builds and I triggered the first update manually to ensure all is working fine, the scanners send a warning:

! repo/com.fadcam_5.apk declares sensitive permission(s):
  android.permission.READ_MEDIA_VIDEO android.permission.READ_MEDIA_IMAGES
  android.permission.ACCESS_FINE_LOCATION android.permission.ACCESS_COARSE_LOCATION
  android.permission.REQUEST_INSTALL_PACKAGES

Can you please clarify what those permissions are needed for? Especially REQUEST_INSTALL_PACKAGES, which caused me to rollback the update before it goes live. What packages are you going to install there?

anonfaded commented 2 months ago

Ok so i have a feature in app's about section to check for updates and if there is new update on github then it downloads it. Screenshot_20240806-160816_FadCam.jpg

The main idea was to make it work like a normal appstore to download new update and update the existing app instead of deleting and then installing the new version. Tho its not working right now, like the app is downloaded but it won't update the app and shows error that 'app not installed', I'll try to find a workaround for it and if there isn't anyway then i will remove the feature or maybe mention that do it manually when new package is downloaded.

The other permissions related to photos and videos is bcz if user want to save video to gallery then Ai suggested to add those permissions as in latest android versions there are some different permissions while in older versions there was a 'write to external' permission.

The location permissions is only used for one feature, Watermarking. In settings i have an option to enable location and gps so if user want to add location data in watermark of video like a cctv camera then they can do it, by default app don't ask for location permission and this feature is disabled, it asks for location permission with a descriptive dialog too explaining the purpose of permission.

Screenshot_20240806-162437_FadCam.jpg

IzzySoft commented 2 months ago

The in-app updater is against the IzzyOnDroid App Inclusion Policy:

[the app] must not download additional executable binary files (e.g. addons, auto-updates, etc.) without explicit user consent. Consent means it needs to be opt-in (it must not be harder to decline than to accept or presented in a way users are likely to press accept without reading) and structured in a way that clearly explains to users that they’re choosing to bypass the checks performed in this repo if they activate it.

While the opt-in is given, the part of explaining the implications is not. Also, updates reach IzzyOnDroid within less than 24h (once they are enabled again for your app), so does it really need an integrated self-updater? PlayStore e.g. forbids such altogether, IIRC.

its not working right now

then maybe drop it right away?

then Ai suggested to add those permissions

:man_facepalming: Ah, the Ai again – I prefer the Ni (natural/native intelligence). You'd need those permissions if you wanted to access media from other applications, if I'm not mistaken – your own media belong to your own application's scope AFAIR. So I'd rather test things before believing an AI (or check in in the documentation instead). An AI might give you a faster answer, but then it often takes longer to verify whether it was the right one or even just a viable one… In this case: did you test if there are any issues if you do not request those permissions? Because then you'd know the Ai was wrong again, and you don't need them.

anonfaded commented 2 months ago

I'll try after removing those permisisons, and for the updater part; i will remove the feature of downloading apk instead just make it check if there is new version available, and if is then just show a message in dialog that user is on older version and needs to update app. Will this be fine?

Also i have to make new tag for this right? Tho it will look weird that in a single day i release 2 updates😅

Also these rules apply to F-Droid too?

IzzySoft commented 2 months ago

just make it check if there is new version available, and if is then just show a message in dialog that user is on older version and needs to update app.

What good is that? For app updates, the fdroid client is responsible. Why must the app check at Github? Whoever installs the app via their fdroid clients expects updates shipping the same way. So you pubish an update at 1 pm, someone gets the notification at 2 pm – but there's no new version in the client as the repo sync would be at 6 pm. Which means the notification just causes confusion. Further, you've just managed to get the APK back into the 30 MB window – so why adding libraries etc. for something the app itself does not need?

Also i have to make new tag for this right?

Yes, please! And once it's there and checked (so I see the REQUEST_INSTALL_PACKAGES is gone and what happened to the storage permissions) I'd re-enable updates again and we take care for the other permissions from above.

Also these rules apply to F-Droid too?

According to their inclusion criteria, yes.

anonfaded commented 2 months ago

Okay, but many people still don't use f droid and 3k people already downloaded just from github so I am planning to make the update-check logic to check if there is new release on GitHub, and if there is then show a dialog with a message like ' your app is not updated, update from f droid app store ' and a ok button which closes that dialog. Having this feature makes sense and is a good easy way to inform user about the versions.

And right now the libraries that i added are all only the essential ones, like app is using them, main big library is the ffmpeg-kit which was 40mb when i downloaded.

I'll do this in some hours and then let you know.

IzzySoft commented 2 months ago

It's useful if the app was installed from Github, true. But rather confusing otherwise. You could include a hint that you check at Github, and that APK would bypass the checks performed e.g. at IzzyOnDroid (where the update should show up shortly as well if it's not yet there). Then it would fit the "informed consent". If then the update check is opt-in (i.e. must be actively enabled – which you could ask for once at first start to announce its availability, and make possible to change later from the app's setting screen), it would fully fit.

And right now the libraries that i added are all only the essential ones

Apologies for being unclear there, I just meant the "update checker".

I'll do this in some hours and then let you know.

Thanks a lot, much appreciated!

anonfaded commented 2 months ago

Hmm for that update-checking im using no library, just a simple github api check for seeing if the release has a newer version tag than the one app currently is. But do you mean i should remove this completely? I think it won't be a problem to have it as the f droid, IoD and github all will be having same tags and versions then how is that a problem🤔

IzzySoft commented 2 months ago

for that update-checking im using no library,

Ah, OK. Then it's at least nothing noticeably contributing to the APK size.

But do you mean i should remove this completely?

From the point of the repositories: yes, that would definitely preferable (both, F-Droid and IzzyOnDroid have that policy on it – and not having such a checker is the cleanest way to keeping to it).

I think it won't be a problem to have it as the f droid, IoD and github all will be having same tags and versions then how is that a problem

That's right – especially if you achieve reproducible builds with F-Droid as well. The only difference then will be the time updates show up: at Github immediately when you add the APK, at IzzyOnDroid within less than 24h (sync here starts at around 7:25 pm local time – which depending on daylight saving corresponds to either 5:25 pm UTC in summer and 6:25 pm UTC in winter – so if you attach the APK slightly before that, it will be less than an hour). With F-Droid it usually takes 2..5 days.

If you really want that updater fully-fledged, you could e.g. have a separate build flavor producing the "fat build" with it, while keeping the per-ABI ones without. So IzzyOnDroid and F-Droid would pick per ABI – and those who want the updater could pick the fat build.

anonfaded commented 2 months ago

Okay I'll do this, the universal apk will have it, but the abi ones not.

Btw i just downloaded the abi one from my releases and i noticed that in the records fragment, 3 drawable icons have disappeared. Idk why thus happened i did nothing, and in universal its there.

Screenshot_20240807-030610_FadCam.jpg

The icons were to the left of these 3 drop down menu items.

anonfaded commented 2 months ago

I have fixed everything, now it shouldn't create any problem hopefully. Removed the whole functionality of installing package and removed its permission. And i tried removing those read video and read photos permissions, without that the app wasn't working so added them back. It took really long time to publish to F-Droid haha, now i have to inform them too to proceed with the process as now everything is fine.

https://github.com/anonfaded/FadCam/releases/tag/v1.1.3

IzzySoft commented 2 months ago

Apologies for the delay, I was AFK the whole day long (was needed at the customer's site), so the new release didn't make it with today's sync (as updates were still disabled) and thus will go out tomorrow. Updates are re-enabled now.

Okay I'll do this, the universal apk will have it, but the abi ones not.

Sounds good, thanks! Both arm64 and armeabi from today are RB btw (didn't run the fat one).

I have fixed everything, now it shouldn't create any problem hopefully.

Great!

Removed the whole functionality of installing package and removed its permission. And i tried removing those read video and read photos permissions, without that the app wasn't working so added them back.

Strange – but as you verified, I'll add them to the "green list" now. I mean, it's clear for the location permissions with the watermark feature, but the media storage access irritates me a little. But if it's needed, it's needed.

now i have to inform them too to proceed with the process as now everything is fine.

They'll build and include all 4 ABIs then (and leave the "fat build" alone as well), so all should be fine. You'll get fast releases for armeabi via IzzyOnDroid, and slower ones for all ABIs via F-Droid. Should you have an "emergency update", all F-Droid folks can quickly cross-update to armeabi via IzzyOnDroid, as F-Droid will establish your app as RB as well.

I guess this issue then is solved?

anonfaded commented 2 months ago

Yes it seems like it's solved.

And those permissions when removed, the recording don't start and when i manually checked from settings, there the permission was also not visible which is required to save videos to storage. And when i added the permission back and opened app then i got the permission request for 'allow access to media and files' which then worked. Altho the code is all open source so anyone can verify this themselves by editing the manifest file and building the apk from source. Thanks.

IzzySoft commented 2 months ago

Yeah, it's just strange it needs a READ permission in order to WRITE – but well, I'm no Android dev, and especially when it comes to SAF and scoped storage, things seem really weird (and really hard to grasp for the average Jane & Joe, if we already struggling here).

So I think all that could be done here is done, and thank you for all your efforts! Closing up then as all seems solved.

anonfaded commented 2 months ago

Yeah I'm also not android developer and just made it all with Ai, things are for real confusing 😆 Well thanks for assistance.

IzzySoft commented 2 months ago

Compressing that comment: "Yeah […] AI, […] for real confusing" :stuck_out_tongue_winking_eye: For that you held up really fine, congrats! And thanks for patiently staying with us!

licaon-kter commented 2 months ago

and slower ones for all ABIs via F-Droid.

not wanted by upstream, so 90Mb fat one it is

@anonfaded this needs to be fixed too? 0j86lKymQH-DI3CpFq72ow

anonfaded commented 2 months ago

and slower ones for all ABIs via F-Droid.

not wanted by upstream, so 90Mb fat one it is

@anonfaded this needs to be fixed too? 0j86lKymQH-DI3CpFq72ow

I'll do the per ABI different versionCodes later that you suggested on GitLab. I got busy in other project so wasn't able to do it...

Also the screenshot you shared, its fine on my side, how it's different on your side🤔😅

Screenshot_20240821-174904.jpg

licaon-kter commented 2 months ago

I can't scroll down :shrug: not a big issue :)

anonfaded commented 2 months ago

I'll try to find the problem, so many fixes needed rn lol. Thanks tho :)