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

fix(dialog): DRAFT granting file-access for dialog.open on Android devices #9311

Draft
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

otamam818
Copy link

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).

Context for those not following the discord discussion: The solution for issue #9083 has progressed and now the fileResponse.path string is a file path (resolved here in this PR). The latest release of plugins-workspace/dialog v2 contains this fix, but now there is another issue - whilst the file path is provided, attempting to access it via Rust produces a Permission denied (os error 13) result.

Reference for discord discussions:

  • Part 1: Tauri 2.0.0 - Convert content URI to filepath
  • Part 2: 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 the READ_EXTERNAL_STORAGE flag (and optionally the WRITE_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 the Added hypothetical extra lines for granting file-access commit.

Why this is a draft

  • I'm not sure whether the file in the commit is the right AndroidManifest.xml file that is generated every time pnpm tauri android dev and/or pnpm 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 the AndroidManifest.xml file
  • this seems like it can be solved in multiple ways (as long as the correct permissions are put in the right AndroidManifest.xml file) and I'm unsure which way the Tauri Working Group would prefer. If i understand correctly, it can be solved by either:
    1. hard-coding the permissions on the template AndroidManifest.xml file like shown in the commit to implicitly let all users access external storage (making it work consistently like Windows and Unix), or
    2. parsing it in PluginHandle.kt and creating an appropriate capabilities permission for the src-tauri/capabilities folder (meaning the documentation for dialog.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 😀

@otamam818 otamam818 requested a review from a team as a code owner March 30, 2024 13:56
@otamam818 otamam818 marked this pull request as draft March 30, 2024 13:57
@amrbashir
Copy link
Member

I don't think we should grant these permissions by default; it should be something the developer adds themselves.

@amrbashir
Copy link
Member

Can't the dialog just grant the permission to modify only that directory? I remember some apps did that before

@otamam818
Copy link
Author

otamam818 commented Apr 2, 2024

I don't think we should grant these permissions by default; it should be something the developer adds themselves.

Is it possible to link the generated AndroidManifest.xml file to a capability? If so then the developer can add it themselves without having to know any extra Android/iOS knowledge.

Can't the dialog just grant the permission to modify only that directory? I remember some apps did that before

I've found a <path-permission> tag for the AndroidManifest.xml files that has a structure like this:

<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 <uses-permission> tag.

If we can:

  • have a $SOME_DIR directory variable that resolves to the respective directory (like what was done here), and
  • Link the configuration files (whether it's src-tauri/tauri.conf.json or src-tauri/capabilities/some_capability.json) to the AndroidManifest.xml file to auto-generate according to the given configuration

then it should be possible to generate the appropriate <path-permission> values to the user's configuration.

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" />

@amrbashir
Copy link
Member

If we can:

  • have a $SOME_DIR directory variable that resolves to the respective directory (like what was done here), and
  • Link the configuration files (whether it's src-tauri/tauri.conf.json or src-tauri/capabilities/some_capability.json) to the AndroidManifest.xml file to auto-generate according to the given configuration

There is no automatic mapping between capability and AndroidManifest.xml permissions (might not be possible at all with the current capability design)

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" />

In this case, this is just a documentation issue and wouldn't require any changes in the default template.

@bpevs
Copy link

bpevs commented Apr 2, 2024

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?

@otamam818
Copy link
Author

In response to @amrbashir

There is no automatic mapping between capability and AndroidManifest.xml permissions (might not be possible at all with the current capability design)

Then would it be possible to link it to the src-tauri/tauri.conf.json file? I remember seeing an android section in tauri.conf.json with a minSdk option, similar to the one found in the AndroidManifest.xml file

In this case, this is just a documentation issue and wouldn't require any changes in the default template.

Oh wow, is it already possible to have these values auto-generated into the AndroidManifest.xml file without changing the source? If so, then I'd love to know how (and where in the documentation I can contribute), and also then I guess the only problem left to solve is finding a way to link the <path-permission> tag to a tauri configuration file.


In response to @bpevs

So that's why when I modified the generated AndroidManifest.xml in my app, it won't solve this issue

Yeah, I tried updating it manually too and ran pnpm tauri android dev which updated the file to not include the permissions. Which means that to resolve this we need to find a way to generate the permission upon build, in a way that suits the tauri way

@amrbashir
Copy link
Member

amrbashir commented Apr 2, 2024

Then would it be possible to link it to the src-tauri/tauri.conf.json file? I remember seeing an android section in tauri.conf.json with a minSdk option, similar to the one found in the AndroidManifest.xml file

That could be done, will need changes in the cli but probably not needed

In this case, this is just a documentation issue and wouldn't require any changes in the default template.

Oh wow, is it already possible to have these values auto-generated into the AndroidManifest.xml file without changing the source? If so, then I'd love to know how (and where in the documentation I can contribute), and also then I guess the only problem left to solve is finding a way to link the <path-permission> tag to a tauri configuration file.

I believe you users can modify AnrdoidManifest.xml as they see fit. if it gets overrident, then it is a bug and should be fixed.

@otamam818
Copy link
Author

otamam818 commented Apr 3, 2024

I believe you users can modify AnrdoidManifest.xml as they see fit. if it gets overrident, then it is a bug and should be fixed.

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 AndroidManifest.xml files that exist within the project. Among those files, only one can be modified by the user.

# 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 AndroidManifest.xml issue. I checked this personally in my bug-reproduction project

Would it make sense to have this documented somewhere?

Runtime Permissions

Another thing I found out after doing some digging is that declaring the correct permissions in the AndroidManifest.xml is just one of the many steps that grant file access outside of the app's internal file-system. The others include requesting runtime permissions where the user consents to allow file access.

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:

  1. Declare the permission

Already discussed to potentially include a flag in the tauri.conf.json file, or to just update the documentation to include this information

  1. and 7. Permission already granted to your app?

Does tauri have a way to check whether runtime permissions have been granted on an Android device?

  1. Request the permission to show the system dialog

Would it make sense to implement in a separate permissions plugin (focused on android/ios/both) or would it make more sense for this to be included in plugins-workspace/dialog?

@otamam818
Copy link
Author

Whatever the decision that Tauri Working Group makes, I'm happy to try and send in a PR to resolve this issue

@amrbashir
Copy link
Member

Would it make sense to have this documented somewhere?

Yeah that should be documented on the fs and dialog plugins README and on the documentation website

The others include requesting runtime permissions where the user consents to allow file access.

This is what I hope the dialog plugin would do

Does tauri have a way to check whether runtime permissions have been granted on an Android device?

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

Would it make sense to implement in a separate permissions plugin (focused on android/ios/both) or would it make more sense for this to be included in plugins-workspace/dialog?

preferably in the dialog plugin

@otamam818
Copy link
Author

Okay then I'll send in a pull-request for updating the README for plugins-workspace/dialog and in the meantime look for a way to integrate runtime permissions into the dialog plugin

@AClon314
Copy link

please check https://github.com/getActivity/XXPermissions , integrated with massive permissions on Android while could consider sdk compabilities.

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

Successfully merging this pull request may close these issues.

[v2][Android]How to covert a content URI to an openable filepath?
4 participants