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

Permission Requesting reason #872

Closed
dovahkiin98 opened this issue Nov 12, 2021 · 14 comments
Closed

Permission Requesting reason #872

dovahkiin98 opened this issue Nov 12, 2021 · 14 comments
Labels
feature-candidate This issue might result in a feature to be implemented suggestion New feature or request

Comments

@dovahkiin98
Copy link

Why does the plugin request permission to External Storage? Even though reading a file or picking a folder does not require External Storage permission.

For example, in Android, picking a file gives you a URI, that you can use to read data from, without needing permission.

Same for writing to file if you pick a directory, though you do need more code to do that.

If the user of this plugin wants to write to the file, and that permission is denied, that's the developer's responsibility, not the Plugin's.

@dovahkiin98 dovahkiin98 added the suggestion New feature or request label Nov 12, 2021
@miguelpruivo
Copy link
Owner

To handle files in the Flutter side you always need to work with absolute paths to create a File instance, hence the reason why they are cached — the URI is only provided for those that want it for some reason.

Because of that, I'm afraid that you will always need to request read permissions.

@dovahkiin98
Copy link
Author

You can still use native code to create a temporary file with the same data, without having to request access to the entire external storage.

As someone who uses this plugin in a very simple app, having the user see the External Storage Permission will most likely scare some users away

@miguelpruivo
Copy link
Owner

Does using 4.2.2 solve it for you? That version removed the read external permission from the plugin but then reverted.

@dovahkiin98
Copy link
Author

@miguelpruivo It removed the permission declaration in the manifest, but that doesn't remove the native code requesting the permission

@miguelpruivo
Copy link
Owner

Oh, I got you. As an alternative I can refactor to add a FilePicker.platform.requestReadPermissions() so they have to be explicitly asked if you want to read external storage (that nowadays most of devices use) but I suspect that most devs would have to use it anyway, so that's an additional step.

@dovahkiin98
Copy link
Author

I don't think this plugin should handle permissions at all. It's the developer's responsibility, not the plugin's.

If the application crashes or sends an error message, then the developer would know what to do, instead of adding extra responsibility to a plugin

@miguelpruivo
Copy link
Owner

@dovahkiin98 I understand your point but most will need this permission anyway, I can see it being in a separate method just to provide a way to ask for the permission, however, I'm quite afraid that I'm starting to receive a lot of new issues just because people won't read the changelog/docs. But I agree with you nevertheless, I'll probably refactor it into a separate method.

@miguelpruivo miguelpruivo added the feature-candidate This issue might result in a feature to be implemented label Nov 15, 2021
@XNeverGiveUp
Copy link

Hey @miguelpruivo , I commented out the code related to the detection permissions, and the plugin still works fine. I am not sure why we need to check the permission?
And the reason why I commented it out is we usually follow the minimum permissions policy, which means that if we don't need permissions, we shouldn't apply for them.
Would you consider not applying for permissions in a future version?

@miguelpruivo
Copy link
Owner

Hey @miguelpruivo , I commented out the code related to the detection permissions, and the plugin still works fine. I am not sure why we need to check the permission?
And the reason why I commented it out is we usually follow the minimum permissions policy, which means that if we don't need permissions, we shouldn't apply for them.
Would you consider not applying for permissions in a future version?

I think that some Android versions don't require permissions and others require. I'm not sure which is which right now, I'd have to look into the docs.

@sevenzees
Copy link

You shouldn't need READ_EXTERNAL_STORAGE permissions to cache the file. Is there a reason why you can't use the app-specific directories that don't require any permissions to cache the file?

https://developer.android.com/training/data-storage/app-specific#internal-access-files

image

image

The image_picker Flutter package does the same thing as this package (except that you can only browse for images) without requiring READ_EXTERNAL_STORAGE.

@kutear
Copy link

kutear commented Jan 7, 2023

Do you have plan to remvoe READ_EXTERNAL_STORAGE ? @miguelpruivo

@rcaner
Copy link

rcaner commented Feb 17, 2023

Do you have any update this issue @miguelpruivo

@Liloupar
Copy link

@miguelpruivo
is any update? READ_EXTERNAL_STORAGE is not need. thanks

@miguelpruivo
Copy link
Owner

Hi, feel free to issue a PR if you're in a rush as I've been quite busy lately. I'd review it ASAP.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-candidate This issue might result in a feature to be implemented suggestion New feature or request
Projects
None yet
Development

No branches or pull requests

7 participants