Skip to content

[camerax] Revert "Explicitly remove READ_EXTERNAL_STORAGE permission" #7826

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

Merged
merged 5 commits into from
Oct 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions packages/camera/camera_android_camerax/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
## 0.6.10

* Removes logic that explicitly removes `READ_EXTERNAL_STORAGE` permission that may be implied
from `WRITE_EXTERNAL_STORAGE` and updates the README to tell users how to manually
remove it from their app's merged manifest if they wish.

## 0.6.9+2

* Updates Java compatibility version to 11.
Expand Down
18 changes: 16 additions & 2 deletions packages/camera/camera_android_camerax/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,21 @@ and thus that parameter will silently be ignored.

In order to save captured images and videos to files on Android 10 and below, CameraX
requires specifying the `WRITE_EXTERNAL_STORAGE` permission (see [the CameraX documentation][10]).
This is already done in the plugin, so no further action is required on your end. To understand
the implications of specificying this permission, see [the `WRITE_EXTERNAL_STORAGE` documentation][11].
This is already done in the plugin, so no further action is required on your end.

To understand the privacy impact of specifying the `WRITE_EXTERNAL_STORAGE` permission, see the
[`WRITE_EXTERNAL_STORAGE` documentation][11]. We have seen apps also have the [`READ_EXTERNAL_STORAGE`][13]
permission automatically added to the merged Android manifest; it appears to be implied from
`WRITE_EXTERNAL_STORAGE`. If you do not want the `READ_EXTERNAL_STORAGE` permission to be included
in the merged Android manifest of your app, then take the following steps to remove it:

1. Ensure that your app nor any of the plugins that it depends on require the `READ_EXTERNAL_STORAGE` permission.
2. Add the following to your app's `your_app/android/app/src/main/AndroidManifest.xml`:

```xml
<uses-permission android:name="android.permission.READ_EXTERNAL_STORAGE"
tools:node="remove" />
```

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please link to the documentation for camerax and the reasons for why this permission is included.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is linked above this part:

In order to save captured images and videos to files on Android 10 and below, CameraX
requires specifying the `WRITE_EXTERNAL_STORAGE` permission (see [the CameraX documentation][10]).
This is already done in the plugin, so no further action is required on your end. To understand
the implications of specificying this permission, see [the `WRITE_EXTERNAL_STORAGE` documentation][11].

Is that sufficient?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"To understand the implications of specificying(sp?) this permission, see [the WRITE_EXTERNAL_STORAGE documentation][11]."

Is technically correct but not as useful. Developers like the original reporter of flutter/flutter#131116 are being asked by their legal team to fill out a justification for a permission. One they didnt add do not actually call.

I think it should be changed to. "If you want to understand the privacy impact of WRITE_EXTERNAL_STORAGE see see [the WRITE_EXTERNAL_STORAGE documentation][11]. We have seen apps also have READ_EXTERNAL_STORAGE permission automatically added this appears to be implied from WRITE_EXTERNAL_STORAGE."

### Allowing image streaming in the background

Expand Down Expand Up @@ -91,4 +104,5 @@ For more information on contributing to this plugin, see [`CONTRIBUTING.md`](CON
[10]: https://developer.android.com/media/camera/camerax/architecture#permissions
[11]: https://developer.android.com/reference/android/Manifest.permission#WRITE_EXTERNAL_STORAGE
[12]: https://developer.android.com/reference/android/Manifest.permission#FOREGROUND_SERVICE_CAMERA
[13]: https://developer.android.com/reference/android/Manifest.permission#READ_EXTERNAL_STORAGE
[148013]: https://github.com/flutter/flutter/issues/148013
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:tools="http://schemas.android.com/tools"
package="io.flutter.plugins.camerax">
<uses-feature android:name="android.hardware.camera.any" />
<uses-permission android:name="android.permission.CAMERA" />
<uses-permission android:name="android.permission.RECORD_AUDIO" />
<uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE"
android:maxSdkVersion="28" />
<uses-permission android:name="android.permission.READ_EXTERNAL_STORAGE"
tools:node="remove" />
</manifest>
2 changes: 1 addition & 1 deletion packages/camera/camera_android_camerax/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: camera_android_camerax
description: Android implementation of the camera plugin using the CameraX library.
repository: https://github.com/flutter/packages/tree/main/packages/camera/camera_android_camerax
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+camera%22
version: 0.6.9+2
version: 0.6.10

environment:
sdk: ^3.5.0
Expand Down