From 9173ee91216aed1dfaa75ad59ff775b4f1cf6247 Mon Sep 17 00:00:00 2001 From: Thibault Deckers Date: Tue, 16 Feb 2021 12:18:59 +0900 Subject: [PATCH] android 11: improved handling and feedback for restricted directories --- android/app/src/main/AndroidManifest.xml | 4 -- .../aves/channel/calls/MetadataHandler.kt | 4 +- .../aves/channel/calls/StorageHandler.kt | 8 +++- .../thibault/aves/utils/PermissionManager.kt | 43 +++++++++++++++-- lib/model/source/album.dart | 48 ++++++++++--------- lib/services/android_file_service.dart | 16 +++++-- lib/utils/android_file_utils.dart | 48 +++++++++++++++++++ .../collection/entry_set_action_delegate.dart | 18 ++++++- .../common/action_mixins/feedback.dart | 5 +- .../action_mixins/permission_aware.dart | 37 ++++++++++---- lib/widgets/dialogs/create_album_dialog.dart | 2 +- lib/widgets/dialogs/rename_album_dialog.dart | 2 +- 12 files changed, 181 insertions(+), 54 deletions(-) diff --git a/android/app/src/main/AndroidManifest.xml b/android/app/src/main/AndroidManifest.xml index 66f30eaf6..3d853ef60 100644 --- a/android/app/src/main/AndroidManifest.xml +++ b/android/app/src/main/AndroidManifest.xml @@ -14,10 +14,6 @@ https://developer.android.com/preview/privacy/storage#media-file-access - raw path access: https://developer.android.com/preview/privacy/storage#media-files-raw-paths - - Android R issues: - - users cannot grant directory access to the root Downloads directory, - - users cannot grant directory access to the root directory of each reliable SD card volume --> diff --git a/android/app/src/main/kotlin/deckers/thibault/aves/channel/calls/MetadataHandler.kt b/android/app/src/main/kotlin/deckers/thibault/aves/channel/calls/MetadataHandler.kt index 6046c7508..d35b00134 100644 --- a/android/app/src/main/kotlin/deckers/thibault/aves/channel/calls/MetadataHandler.kt +++ b/android/app/src/main/kotlin/deckers/thibault/aves/channel/calls/MetadataHandler.kt @@ -325,7 +325,7 @@ class MetadataHandler(private val context: Context) : MethodCallHandler { if (xmpMeta.doesPropertyExist(XMP.DC_SCHEMA_NS, XMP.SUBJECT_PROP_NAME)) { val count = xmpMeta.countArrayItems(XMP.DC_SCHEMA_NS, XMP.SUBJECT_PROP_NAME) val values = (1 until count + 1).map { xmpMeta.getArrayItem(XMP.DC_SCHEMA_NS, XMP.SUBJECT_PROP_NAME, it).value } - metadataMap[KEY_XMP_SUBJECTS] = values.joinToString(separator = XMP_SUBJECTS_SEPARATOR) + metadataMap[KEY_XMP_SUBJECTS] = values.joinToString(XMP_SUBJECTS_SEPARATOR) } xmpMeta.getSafeLocalizedText(XMP.DC_SCHEMA_NS, XMP.TITLE_PROP_NAME, acceptBlank = false) { metadataMap[KEY_XMP_TITLE_DESCRIPTION] = it } if (!metadataMap.containsKey(KEY_XMP_TITLE_DESCRIPTION)) { @@ -350,7 +350,7 @@ class MetadataHandler(private val context: Context) : MethodCallHandler { // XMP fallback to IPTC if (!metadataMap.containsKey(KEY_XMP_SUBJECTS)) { for (dir in metadata.getDirectoriesOfType(IptcDirectory::class.java)) { - dir.keywords?.let { metadataMap[KEY_XMP_SUBJECTS] = it.joinToString(separator = XMP_SUBJECTS_SEPARATOR) } + dir.keywords?.let { metadataMap[KEY_XMP_SUBJECTS] = it.joinToString(XMP_SUBJECTS_SEPARATOR) } } } diff --git a/android/app/src/main/kotlin/deckers/thibault/aves/channel/calls/StorageHandler.kt b/android/app/src/main/kotlin/deckers/thibault/aves/channel/calls/StorageHandler.kt index 26acc1bf6..9199bb43c 100644 --- a/android/app/src/main/kotlin/deckers/thibault/aves/channel/calls/StorageHandler.kt +++ b/android/app/src/main/kotlin/deckers/thibault/aves/channel/calls/StorageHandler.kt @@ -26,6 +26,7 @@ class StorageHandler(private val context: Context) : MethodCallHandler { "getFreeSpace" -> safe(call, result, ::getFreeSpace) "getGrantedDirectories" -> safe(call, result, ::getGrantedDirectories) "getInaccessibleDirectories" -> safe(call, result, ::getInaccessibleDirectories) + "getRestrictedDirectories" -> safe(call, result, ::getRestrictedDirectories) "revokeDirectoryAccess" -> safe(call, result, ::revokeDirectoryAccess) "scanFile" -> GlobalScope.launch(Dispatchers.IO) { safe(call, result, ::scanFile) } else -> result.notImplemented() @@ -104,8 +105,11 @@ class StorageHandler(private val context: Context) : MethodCallHandler { return } - val dirs = PermissionManager.getInaccessibleDirectories(context, dirPaths) - result.success(dirs) + result.success(PermissionManager.getInaccessibleDirectories(context, dirPaths)) + } + + private fun getRestrictedDirectories(@Suppress("UNUSED_PARAMETER") call: MethodCall, result: MethodChannel.Result) { + result.success(PermissionManager.getRestrictedDirectories(context)) } private fun revokeDirectoryAccess(call: MethodCall, result: MethodChannel.Result) { diff --git a/android/app/src/main/kotlin/deckers/thibault/aves/utils/PermissionManager.kt b/android/app/src/main/kotlin/deckers/thibault/aves/utils/PermissionManager.kt index 49d1b005b..e703392e5 100644 --- a/android/app/src/main/kotlin/deckers/thibault/aves/utils/PermissionManager.kt +++ b/android/app/src/main/kotlin/deckers/thibault/aves/utils/PermissionManager.kt @@ -5,12 +5,14 @@ import android.content.Context import android.content.Intent import android.net.Uri import android.os.Build +import android.os.Environment import android.os.storage.StorageManager import android.util.Log import deckers.thibault.aves.utils.StorageUtils.PathSegments import java.io.File import java.util.* import java.util.concurrent.ConcurrentHashMap +import kotlin.collections.ArrayList object PermissionManager { private val LOG_TAG = LogUtils.createTag(PermissionManager::class.java) @@ -66,9 +68,20 @@ object PermissionManager { val dirSet = dirsPerVolume[volumePath] ?: HashSet() if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) { // request primary directory on volume from Android R - segments.relativeDir?.apply { - val primaryDir = split(File.separator).firstOrNull { it.isNotEmpty() } - primaryDir?.let { dirSet.add(it) } + val relativeDir = segments.relativeDir + if (relativeDir != null) { + val dirSegments = relativeDir.split(File.separator).takeWhile { it.isNotEmpty() } + val primaryDir = dirSegments.firstOrNull() + if (primaryDir == Environment.DIRECTORY_DOWNLOADS && dirSegments.size > 1) { + // request secondary directory (if any) for restricted primary directory + dirSet.add(dirSegments.take(2).joinToString(File.separator)) + } else { + primaryDir?.let { dirSet.add(it) } + } + } else { + // the requested path is the volume root itself + // which cannot be granted, due to Android R restrictions + dirSet.add("") } } else { // request volume root until Android Q @@ -92,6 +105,30 @@ object PermissionManager { } } + fun getRestrictedDirectories(context: Context): List> { + return if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) { + // cf https://developer.android.com/about/versions/11/privacy/storage#directory-access + val volumePaths = StorageUtils.getVolumePaths(context) + ArrayList>().apply { + addAll(volumePaths.map { + hashMapOf( + "volumePath" to it, + "relativeDir" to "", + ) + }) + addAll(volumePaths.map { + hashMapOf( + "volumePath" to it, + "relativeDir" to Environment.DIRECTORY_DOWNLOADS, + ) + }) + } + } else { + // TODO TLAD add KitKat restriction (no SD card root access) if min version goes to API 19-20 + ArrayList() + } + } + fun revokeDirectoryAccess(context: Context, path: String): Boolean { return StorageUtils.convertDirPathToTreeUri(context, path)?.let { val flags = Intent.FLAG_GRANT_READ_URI_PERMISSION or Intent.FLAG_GRANT_WRITE_URI_PERMISSION diff --git a/lib/model/source/album.dart b/lib/model/source/album.dart index 2ae41ee85..5fecb0848 100644 --- a/lib/model/source/album.dart +++ b/lib/model/source/album.dart @@ -23,34 +23,36 @@ mixin AlbumMixin on SourceBase { void _notifyAlbumChange() => eventBus.fire(AlbumsChangedEvent()); - String getUniqueAlbumName(String album) { - final otherAlbums = _directories.where((item) => item != album); - final parts = album.split(separator); - var partCount = 0; - String testName; - do { - testName = separator + parts.skip(parts.length - ++partCount).join(separator); - } while (otherAlbums.any((item) => item.endsWith(testName))); - final uniqueName = parts.skip(parts.length - partCount).join(separator); - - final volume = androidFileUtils.getStorageVolume(album); - if (volume == null) { + String getUniqueAlbumName(String dirPath) { + String unique(String dirPath, [bool Function(String) test]) { + final otherAlbums = _directories.where(test ?? (_) => true).where((item) => item != dirPath); + final parts = dirPath.split(separator); + var partCount = 0; + String testName; + do { + testName = separator + parts.skip(parts.length - ++partCount).join(separator); + } while (otherAlbums.any((item) => item.endsWith(testName))); + final uniqueName = parts.skip(parts.length - partCount).join(separator); return uniqueName; } - final volumeRootLength = volume.path.length; - if (album.length < volumeRootLength) { - // `album` is at the root, without trailing '/' - return uniqueName; - } + final dir = VolumeRelativeDirectory.fromPath(dirPath); + if (dir == null) return dirPath; - final albumRelativePath = album.substring(volumeRootLength); - if (uniqueName.length < albumRelativePath.length) { - return uniqueName; - } else if (volume.isPrimary) { - return albumRelativePath; + final uniqueNameInDevice = unique(dirPath); + final relativeDir = dir.relativeDir; + if (relativeDir.isEmpty) return uniqueNameInDevice; + + if (uniqueNameInDevice.length < relativeDir.length) { + return uniqueNameInDevice; } else { - return '$albumRelativePath (${volume.description})'; + final uniqueNameInVolume = unique(dirPath, (item) => item.startsWith(dir.volumePath)); + final volume = androidFileUtils.getStorageVolume(dirPath); + if (volume.isPrimary) { + return uniqueNameInVolume; + } else { + return '$uniqueNameInVolume (${volume.description})'; + } } } diff --git a/lib/services/android_file_service.dart b/lib/services/android_file_service.dart index a53448809..3b2096f42 100644 --- a/lib/services/android_file_service.dart +++ b/lib/services/android_file_service.dart @@ -52,20 +52,28 @@ class AndroidFileService { return; } - // returns a list of directories, - // each directory is a map with "volumePath", "relativeDir" - static Future> getInaccessibleDirectories(Iterable dirPaths) async { + static Future> getInaccessibleDirectories(Iterable dirPaths) async { try { final result = await platform.invokeMethod('getInaccessibleDirectories', { 'dirPaths': dirPaths.toList(), }); - return (result as List).cast(); + return (result as List).cast().map((map) => VolumeRelativeDirectory.fromMap(map)).toSet(); } on PlatformException catch (e) { debugPrint('getInaccessibleDirectories failed with code=${e.code}, exception=${e.message}, details=${e.details}}'); } return null; } + static Future> getRestrictedDirectories() async { + try { + final result = await platform.invokeMethod('getRestrictedDirectories'); + return (result as List).cast().map((map) => VolumeRelativeDirectory.fromMap(map)).toSet(); + } on PlatformException catch (e) { + debugPrint('getRestrictedDirectories failed with code=${e.code}, exception=${e.message}, details=${e.details}}'); + } + return null; + } + // returns whether user granted access to volume root at `volumePath` static Future requestVolumeAccess(String volumePath) async { try { diff --git a/lib/utils/android_file_utils.dart b/lib/utils/android_file_utils.dart index f97ccb3a3..f0de8f962 100644 --- a/lib/utils/android_file_utils.dart +++ b/lib/utils/android_file_utils.dart @@ -2,6 +2,7 @@ import 'package:aves/services/android_app_service.dart'; import 'package:aves/services/android_file_service.dart'; import 'package:aves/utils/change_notifier.dart'; import 'package:flutter/foundation.dart'; +import 'package:flutter/widgets.dart'; import 'package:path/path.dart'; final AndroidFileUtils androidFileUtils = AndroidFileUtils._private(); @@ -112,6 +113,7 @@ class Package { String toString() => '$runtimeType#${shortHash(this)}{packageName=$packageName, categoryLauncher=$categoryLauncher, isSystem=$isSystem, currentLabel=$currentLabel, englishLabel=$englishLabel, ownedDirs=$ownedDirs}'; } +@immutable class StorageVolume { final String description, path, state; final bool isPrimary, isRemovable; @@ -135,3 +137,49 @@ class StorageVolume { ); } } + +@immutable +class VolumeRelativeDirectory { + final String volumePath, relativeDir; + + const VolumeRelativeDirectory({ + this.volumePath, + this.relativeDir, + }); + + factory VolumeRelativeDirectory.fromMap(Map map) { + return VolumeRelativeDirectory( + volumePath: map['volumePath'], + relativeDir: map['relativeDir'] ?? '', + ); + } + + // prefer static method over a null returning factory constructor + static VolumeRelativeDirectory fromPath(String dirPath) { + final volume = androidFileUtils.getStorageVolume(dirPath); + if (volume == null) return null; + + final root = volume.path; + final rootLength = root.length; + return VolumeRelativeDirectory( + volumePath: root, + relativeDir: dirPath.length < rootLength ? '' : dirPath.substring(rootLength), + ); + } + + String get directoryDescription => relativeDir.isEmpty ? 'root' : '“$relativeDir”'; + + String get volumeDescription { + final volume = androidFileUtils.storageVolumes.firstWhere((volume) => volume.path == volumePath, orElse: () => null); + return volume?.description ?? volumePath; + } + + @override + bool operator ==(Object other) { + if (other.runtimeType != runtimeType) return false; + return other is VolumeRelativeDirectory && other.volumePath == volumePath && other.relativeDir == relativeDir; + } + + @override + int get hashCode => hashValues(volumePath, relativeDir); +} diff --git a/lib/widgets/collection/entry_set_action_delegate.dart b/lib/widgets/collection/entry_set_action_delegate.dart index 2d41d0bce..d8a4544fd 100644 --- a/lib/widgets/collection/entry_set_action_delegate.dart +++ b/lib/widgets/collection/entry_set_action_delegate.dart @@ -7,8 +7,10 @@ import 'package:aves/model/entry.dart'; import 'package:aves/model/source/collection_lens.dart'; import 'package:aves/model/source/collection_source.dart'; import 'package:aves/services/android_app_service.dart'; +import 'package:aves/services/android_file_service.dart'; import 'package:aves/services/image_file_service.dart'; import 'package:aves/services/image_op_events.dart'; +import 'package:aves/utils/android_file_utils.dart'; import 'package:aves/widgets/common/action_mixins/feedback.dart'; import 'package:aves/widgets/common/action_mixins/permission_aware.dart'; import 'package:aves/widgets/common/action_mixins/size_aware.dart'; @@ -63,6 +65,20 @@ class EntrySetActionDelegate with FeedbackMixin, PermissionAwareMixin, SizeAware } Future _moveSelection(BuildContext context, {@required MoveType moveType}) async { + final selectionDirs = selection.where((e) => e.path != null).map((e) => e.directory).toSet(); + if (moveType == MoveType.move) { + // check whether moving is possible given OS restrictions, + // before asking to pick a destination album + final restrictedDirs = await AndroidFileService.getRestrictedDirectories(); + for (final selectionDir in selectionDirs) { + final dir = VolumeRelativeDirectory.fromPath(selectionDir); + if (restrictedDirs.contains(dir)) { + await showRestrictedDirectoryDialog(context, dir); + return; + } + } + } + final destinationAlbum = await Navigator.push( context, MaterialPageRoute( @@ -73,7 +89,7 @@ class EntrySetActionDelegate with FeedbackMixin, PermissionAwareMixin, SizeAware if (destinationAlbum == null || destinationAlbum.isEmpty) return; if (!await checkStoragePermissionForAlbums(context, {destinationAlbum})) return; - if (!await checkStoragePermission(context, selection)) return; + if (moveType == MoveType.move && !await checkStoragePermissionForAlbums(context, selectionDirs)) return; if (!await checkFreeSpaceForMove(context, selection, destinationAlbum, moveType)) return; diff --git a/lib/widgets/common/action_mixins/feedback.dart b/lib/widgets/common/action_mixins/feedback.dart index 4a1743e68..06dcbce87 100644 --- a/lib/widgets/common/action_mixins/feedback.dart +++ b/lib/widgets/common/action_mixins/feedback.dart @@ -87,10 +87,7 @@ class _ReportOverlayState extends State> with SingleTickerPr Future onComplete() => _animationController.reverse().then((_) => widget.onDone(processed)); opStream.listen( processed.add, - onError: (error) { - debugPrint('_showOpReport error=$error'); - onComplete(); - }, + onError: (error) => debugPrint('_showOpReport error=$error'), onDone: onComplete, ); } diff --git a/lib/widgets/common/action_mixins/permission_aware.dart b/lib/widgets/common/action_mixins/permission_aware.dart index d57373c8b..cbd8ce7c9 100644 --- a/lib/widgets/common/action_mixins/permission_aware.dart +++ b/lib/widgets/common/action_mixins/permission_aware.dart @@ -10,26 +10,26 @@ mixin PermissionAwareMixin { } Future checkStoragePermissionForAlbums(BuildContext context, Set albumPaths) async { + final restrictedDirs = await AndroidFileService.getRestrictedDirectories(); while (true) { final dirs = await AndroidFileService.getInaccessibleDirectories(albumPaths); if (dirs == null) return false; if (dirs.isEmpty) return true; + final restrictedInaccessibleDir = dirs.firstWhere(restrictedDirs.contains, orElse: () => null); + if (restrictedInaccessibleDir != null) { + await showRestrictedDirectoryDialog(context, restrictedInaccessibleDir); + return false; + } + final dir = dirs.first; - final volumePath = dir['volumePath'] as String; - final relativeDir = dir['relativeDir'] as String; - - final volume = androidFileUtils.storageVolumes.firstWhere((volume) => volume.path == volumePath, orElse: () => null); - final volumeDescription = volume?.description ?? volumePath; - final dirDisplayName = relativeDir.isEmpty ? 'root' : '“$relativeDir”'; - final confirmed = await showDialog( context: context, builder: (context) { return AvesDialog( context: context, title: 'Storage Volume Access', - content: Text('Please select the $dirDisplayName directory of “$volumeDescription” in the next screen, so that this app can access it and complete your request.'), + content: Text('Please select the ${dir.directoryDescription} directory of “${dir.volumeDescription}” in the next screen, so that this app can access it and complete your request.'), actions: [ TextButton( onPressed: () => Navigator.pop(context), @@ -46,11 +46,30 @@ mixin PermissionAwareMixin { // abort if the user cancels in Flutter if (confirmed == null || !confirmed) return false; - final granted = await AndroidFileService.requestVolumeAccess(volumePath); + final granted = await AndroidFileService.requestVolumeAccess(dir.volumePath); if (!granted) { // abort if the user denies access from the native dialog return false; } } } + + Future showRestrictedDirectoryDialog(BuildContext context, VolumeRelativeDirectory dir) { + return showDialog( + context: context, + builder: (context) { + return AvesDialog( + context: context, + title: 'Restricted Access', + content: Text('This app is not allowed to modify files in the ${dir.directoryDescription} directory of “${dir.volumeDescription}”.\n\nPlease use a pre-installed file manager or gallery app to move the items to another directory.'), + actions: [ + TextButton( + onPressed: () => Navigator.pop(context), + child: Text('OK'.toUpperCase()), + ), + ], + ); + }, + ); + } } diff --git a/lib/widgets/dialogs/create_album_dialog.dart b/lib/widgets/dialogs/create_album_dialog.dart index 2ac658691..26c41dad1 100644 --- a/lib/widgets/dialogs/create_album_dialog.dart +++ b/lib/widgets/dialogs/create_album_dialog.dart @@ -74,7 +74,7 @@ class _CreateAlbumDialogState extends State { focusNode: _nameFieldFocusNode, decoration: InputDecoration( labelText: 'Album name', - helperText: exists ? 'Album already exists' : '', + helperText: exists ? 'Directory already exists' : '', ), autofocus: _allVolumes.length == 1, onChanged: (_) => _validate(), diff --git a/lib/widgets/dialogs/rename_album_dialog.dart b/lib/widgets/dialogs/rename_album_dialog.dart index f9e3f10f4..7997464fb 100644 --- a/lib/widgets/dialogs/rename_album_dialog.dart +++ b/lib/widgets/dialogs/rename_album_dialog.dart @@ -47,7 +47,7 @@ class _RenameAlbumDialogState extends State { controller: _nameController, decoration: InputDecoration( labelText: 'New name', - helperText: exists ? 'Album already exists' : '', + helperText: exists ? 'Directory already exists' : '', ), autofocus: true, onChanged: (_) => _validate(),