-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
fix(dialog): DRAFT granting file-access for dialog.open
on Android devices
#9311
base: dev
Are you sure you want to change the base?
Conversation
I don't think we should grant these permissions by default; it should be something the developer adds themselves. |
Can't the dialog just grant the permission to modify only that directory? I remember some apps did that before |
Is it possible to link the generated
I've found a <path-permission android:path="string"
android:pathPrefix="string"
android:pathPattern="string"
android:permission="string"
android:readPermission="string"
android:writePermission="string" /> and would be on the same indentation level as the If we can:
then it should be possible to generate the appropriate There also might be the case where the developer wants to let the user browse and select files freely without restrictions, so having a separate capability that maps to these flags would also be developer-friendly: <uses-permission android:name="android.permission.READ_EXTERNAL_STORAGE"/>
<uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE"
android:maxSdkVersion="29" /> |
There is no automatic mapping between capability and AndroidManifest.xml permissions (might not be possible at all with the current capability design)
In this case, this is just a documentation issue and wouldn't require any changes in the default template. |
Wait sorry sorry I'm trying to follow this conversation, but if I understand, this AndroidManifest.xml overrides the generated one for permissions (like here mebbe)? So that's why when I modified the generated AndroidManifest.xml in my app, it won't solve this issue? |
In response to @amrbashir
Then would it be possible to link it to the
Oh wow, is it already possible to have these values auto-generated into the In response to @bpevs
Yeah, I tried updating it manually too and ran |
That could be done, will need changes in the cli but probably not needed
I believe you users can modify |
I attempted to reproduce a bug from this, but upon looking at the link about the manifest @bpevs sent, it seems that there are two or more # Can be modified by the user
path = "/src-tauri/gen/android/app/src/main/AndroidManifest.xml"
# Auto-generated (cannot be modified by the user)
path = "/src-tauri/gen/android/app/build/intermediates/packaged_manifests/x86_64Debug/AndroidManifest.xml"
# NOTE: In the second path the `x86_64Debug` directory may be different based on your system architecture Thus modifying the proper file resolves the Would it make sense to have this documented somewhere? Runtime PermissionsAnother thing I found out after doing some digging is that declaring the correct permissions in the They did provide a control-flow diagram for this, but honestly for tauri, it only boils down to steps 1, 4, 6, 7 - every other step is more-aligned to guide the Android developer rather than the framework. I'll outline each relevant point and what needs to be discussed:
Already discussed to potentially include a flag in the
Does tauri have a way to check whether runtime permissions have been granted on an Android device?
Would it make sense to implement in a separate |
Whatever the decision that Tauri Working Group makes, I'm happy to try and send in a PR to resolve this issue |
Yeah that should be documented on the fs and dialog plugins README and on the documentation website
This is what I hope the dialog plugin would do
Not directly in tauri but plugins like barcodescanner implemented a similar thing, see https://github.com/tauri-apps/plugins-workspace/blob/a32e3200de9cce2c9b9e6e628ff0e3ff63293417/plugins/barcode-scanner/guest-js/index.ts#L55-L68 and https://github.com/tauri-apps/plugins-workspace/blob/a32e3200de9cce2c9b9e6e628ff0e3ff63293417/plugins/barcode-scanner/android/src/main/java/BarcodeScannerPlugin.kt#L399-L425
preferably in the dialog plugin |
Okay then I'll send in a pull-request for updating the README for |
please check https://github.com/getActivity/XXPermissions , integrated with massive permissions on Android while could consider sdk compabilities. |
Once implemented, closes #9083
Does not require a new version as this is a draft that suggests changes (merging not recommended unless it is confirmed to resolve the issue).
Reference for discord discussions:
Tauri 2.0.0 - Convert content URI to filepath
open/readfile outside app dir?
Proposition
According to this reference, this happens is because the
AndroidManifest.xml
file generated (for granting permissions and other kinds of access) doesn't declare theREAD_EXTERNAL_STORAGE
flag (and optionally theWRITE_EXTERNAL_STORAGE
flag, for users that want to write to the physical device's local storage)Declaring this would grant access to storage outside the app, thus preventing
Permission denied (os error 13)
from occurring. I demonstrated what this may look like in theAdded hypothetical extra lines for granting file-access
commit.Why this is a draft
AndroidManifest.xml
file that is generated every timepnpm tauri android dev
and/orpnpm tauri android build
is called. Another place I saw a similar implementation is in PluginHandle.kt, which if I understand correctly, validates plugins and passes the appropriate permission flags to theAndroidManifest.xml
fileAndroidManifest.xml
file) and I'm unsure which way the Tauri Working Group would prefer. If i understand correctly, it can be solved by either:AndroidManifest.xml
file like shown in the commit to implicitly let all users access external storage (making it work consistently like Windows and Unix), orPluginHandle.kt
and creating an appropriate capabilities permission for thesrc-tauri/capabilities
folder (meaning the documentation fordialog.open
may need to be updated to reflect this, but adheres better to Tauri's "security" focus)I'm unsure if what I found helps, but if it does, I'm happy to solve this in whatever way the TWG deems appropriate 😀