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

Is READ_EXTERNAL_STORAGE still needed? #105

Open
IzzySoft opened this issue Jan 22, 2024 · 11 comments
Open

Is READ_EXTERNAL_STORAGE still needed? #105

IzzySoft opened this issue Jan 22, 2024 · 11 comments

Comments

@IzzySoft
Copy link
Contributor

My freshly enhanced scanner reported on today's update:

! repo/com.openathena_31.apk declares risky permission(s): android.permission.READ_EXTERNAL_STORAGE

Your app needs at least Android 9 to run, so is this still needed – or was it rather forgotten with the changes from #4?

Thanks in advance for clarification!

@mkrupczak3
Copy link
Member

mkrupczak3 commented Jan 24, 2024

@IzzySoft I think it'd be best to keep it.

A common use case of the app is to load photos directly from the DJI Fly/DJI Go 4 App's gallery that are downloaded during flight

For this, the OpenAthena app would need READ_EXTERNAL_STORAGE read permissions. Maybe not the ideal solution, but the best one we have right now

@mkrupczak3
Copy link
Member

I'm not entirely sure if this permission is still needed actually. Maybe I should do some testing to see? Will report back within next week or so

@IzzySoft
Copy link
Contributor Author

If you need to access any of the media directories on the device itself, at least up to (and including) Android 9 you'd indeed still need that permission indeed. For this case it was made obsolete only later (this permission is such a mess that it's hard to remember; some things with it were changed in Android 10, others again with Android 13). For all else, SAF should do.

@IzzySoft
Copy link
Contributor Author

IzzySoft commented Feb 4, 2024

As today's release triggered this again: do you have any news on what the status is here?

@IzzySoft
Copy link
Contributor Author

IzzySoft commented Feb 4, 2024

PS: And what happened to the APK that its size almost doubled – plus to your signing key? Your update got rejected here as the signature does not match.

Previous release:

Signer #1 certificate DN: CN=Matthew Krupczak, O=Krupczak Org LLC, L=Atlanta, ST=GA
Signer #1 certificate SHA-256 digest: 4faa6dab09f9d15ac204ea8b8c056ccaf8c52993007cd643001a3d3497c7944b
Signer #1 certificate SHA-1 digest: dbbec2d41538b71e6759880015806029a3681049
Signer #1 certificate MD5 digest: be61a08cdb56a76f0c6f06fc83cbe989
Signer #1 key algorithm: RSA
Signer #1 key size (bits): 2048

Today's release:

Signer #1 certificate DN: C=US, O=Android, CN=Android Debug
Signer #1 certificate SHA-256 digest: f2b216fd5aaad930867888177000ea266d8b3c304fa136626617162117b7e621
Signer #1 certificate SHA-1 digest: 66b366737f871e24488cf9e32a76d0b5ee46f276
Signer #1 certificate MD5 digest: 290dc502aecc565ccbe2fb7959430f0c
Signer #1 key algorithm: RSA
Signer #1 key size (bits): 2048

Accidentally signed using a debug key?

@mkrupczak3
Copy link
Member

I'm not sure what caused the app size to jump so dramatically, I noticed that too. Only thing I changed was I replaced one of the drawable graphics with a higher resolution version. I checked the file size and it's not large enough to explain that difference, so I'm not sure what's going on there.

@IzzySoft looks like you're right, I built v0.19.8 with the debug key instead of the release key by accident. I'm going to replace the binary for v0.19.8 with the correct one I just built

@mkrupczak3
Copy link
Member

As today's release triggered this again: do you have any news on what the status is here?

To answer your original question, It appears the app should still keep the permission for backwards compatibility. Want to support as many versions of Android as possible. This software can and will run even on a potato

@mkrupczak3
Copy link
Member

As for the app's size increase, I accidentally used a 4k by 4k image instead of a 512x512 sized one.

The offending commit: 9051692 😅

Fixed this in 7ff1bce, next version will be the normal size

@IzzySoft
Copy link
Contributor Author

IzzySoft commented Feb 5, 2024

To answer your original question, It appears the app should still keep the permission for backwards compatibility.

Ah, right: Android 9 still has issues without that. OK, will add it accordingly.

And nice the culprit for the increased APK size was found as well – and eliminated 🥳 As the signing issue is solved, too, I'll re-enable daily updates then (had them temporarily enabled to avoid "daily alerts" about the signature mismatch).

Thanks a lot!

@mkrupczak3
Copy link
Member

np. Very glad you caught the signing key issue, that would have been frustrating for our users to deal with

@mkrupczak3
Copy link
Member

App appears to still function even if user denies this permission (tested on Android 14)

Need to test again with minimum supported version API level 28 (Android 9 'Pie') to see if this permission is really still necessary

@mkrupczak3 mkrupczak3 reopened this Jul 10, 2024
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

2 participants