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.

Sign up for GitHub

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

Improve Storage Permission Handling #14229

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Improve Storage Permission Handling #14229

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8c4fa44
simplify logic
alperozturk96 Dec 23, 2024
61e1e88
remove unnecessary check and use more accurate function name
alperozturk96 Dec 23, 2024
ae27629
simplify
alperozturk96 Dec 23, 2024
f5c3fe4
remove unnecessary parameter
alperozturk96 Dec 23, 2024
b60aea1
add READ_MEDIA_VISUAL_USER_SELECTED
alperozturk96 Dec 23, 2024
bf0c209
Analysis: update lint results to reflect reduced error/warning count
invalid-email-address Jan 14, 2025
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
41 changes: 24 additions & 17 deletions app/src/main/AndroidManifest.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,6 @@
android:name="android.hardware.camera2"
android:required="false"
tools:node="replace" />
<!--
WRITE_EXTERNAL_STORAGE may be enabled or disabled by the user after installation in
API >= 23; the app needs to handle this
-->
<uses-permission
android:name="android.permission.WRITE_EXTERNAL_STORAGE"
android:maxSdkVersion="29"
tools:ignore="ScopedStorage" />
<uses-permission
android:name="android.permission.MANAGE_EXTERNAL_STORAGE"
tools:ignore="ScopedStorage" />
<uses-permission
android:name="android.permission.READ_EXTERNAL_STORAGE"
android:maxSdkVersion="32" />
<uses-permission android:name="android.permission.CAMERA" />
<uses-permission android:name="android.permission.VIBRATE" /> <!-- Next permissions are always approved in installation time, the apps needs to do nothing special in runtime -->
<uses-permission android:name="android.permission.INTERNET" />
Expand All @@ -52,11 +38,32 @@
Apps that target Android 9 (API level 28) or higher and use foreground services
must request the FOREGROUND_SERVICE permission
-->
<uses-permission android:name="android.permission.FOREGROUND_SERVICE" /> <!-- Runtime permissions introduced in Android 13 (API level 33) -->
<uses-permission android:name="android.permission.READ_MEDIA_IMAGES" />
<uses-permission android:name="android.permission.READ_MEDIA_VIDEO" /> <!-- Needed for Android 14 (API level 34) -->
<uses-permission android:name="android.permission.FOREGROUND_SERVICE_MEDIA_PLAYBACK" />
<uses-permission android:name="android.permission.FOREGROUND_SERVICE_DATA_SYNC" />

<uses-permission android:name="android.permission.FOREGROUND_SERVICE" />


<!-- WRITE_EXTERNAL_STORAGE may be enabled or disabled by the user after installation in API >= 23; the app needs to handle this-->
<uses-permission
android:name="android.permission.WRITE_EXTERNAL_STORAGE"
android:maxSdkVersion="29"
tools:ignore="ScopedStorage" />

<uses-permission android:name="android.permission.MANAGE_EXTERNAL_STORAGE" tools:ignore="ScopedStorage" />

<!-- Devices running Android 12L (API level 32) or lower -->
<uses-permission android:name="android.permission.READ_EXTERNAL_STORAGE" android:maxSdkVersion="32" />

<!-- Devices running Android 13 (API level 33) or higher -->
<uses-permission android:name="android.permission.READ_MEDIA_IMAGES" />
<uses-permission android:name="android.permission.READ_MEDIA_VIDEO" />

<!-- To handle the reselection within the app on devices running Android 14
or higher if your app targets Android 14 (API level 34) or higher. -->
<uses-permission android:name="android.permission.READ_MEDIA_VISUAL_USER_SELECTED" />


<!--
Some Chromebooks don't support touch. Although not essential,
it's a good idea to explicitly include this declaration.
Expand Down
4 changes: 0 additions & 4 deletions app/src/main/java/com/nextcloud/client/preferences/AppPreferences.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,10 +378,6 @@ default void onDarkThemeModeChanged(DarkMode mode) {

int getPdfZoomTipShownCount();

boolean isStoragePermissionRequested();

void setStoragePermissionRequested(boolean value);

void setInAppReviewData(@NonNull AppReviewShownModel appReviewShownModel);

@Nullable
Expand Down
11 changes: 0 additions & 11 deletions app/src/main/java/com/nextcloud/client/preferences/AppPreferencesImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@ public final class AppPreferencesImpl implements AppPreferences {
private static final String PREF__PDF_ZOOM_TIP_SHOWN = "pdf_zoom_tip_shown";
private static final String PREF__MEDIA_FOLDER_LAST_PATH = "media_folder_last_path";

private static final String PREF__STORAGE_PERMISSION_REQUESTED = "storage_permission_requested";
private static final String PREF__IN_APP_REVIEW_DATA = "in_app_review_data";

private static final String PREF__TWO_WAY_STATUS = "two_way_sync_status";
Expand Down Expand Up @@ -760,16 +759,6 @@ public int getPdfZoomTipShownCount() {
return preferences.getInt(PREF__PDF_ZOOM_TIP_SHOWN, 0);
}

@Override
public boolean isStoragePermissionRequested() {
return preferences.getBoolean(PREF__STORAGE_PERMISSION_REQUESTED, false);
}

@Override
public void setStoragePermissionRequested(boolean value) {
preferences.edit().putBoolean(PREF__STORAGE_PERMISSION_REQUESTED, value).apply();
}

@VisibleForTesting
public int computeBruteForceDelay(int count) {
return (int) Math.min(count / 3d, 10);
Expand Down
2 changes: 1 addition & 1 deletion app/src/main/java/com/owncloud/android/MainApp.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -627,7 +627,7 @@ public static void initSyncOperations(
updateAutoUploadEntries(clock);

if (getAppContext() != null) {
if (PermissionUtil.checkExternalStoragePermission(getAppContext())) {
if (PermissionUtil.checkStoragePermission(getAppContext())) {
splitOutAutoUploadEntries(clock, viewThemeUtils);
} else {
preferences.setAutoUploadSplitEntriesEnabled(true);
Expand Down
9 changes: 4 additions & 5 deletions app/src/main/java/com/owncloud/android/datamodel/MediaProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public static List<MediaFolder> getImageFolders(ContentResolver contentResolver,

// query media/image folders
Cursor cursorFolders = null;
if (activity != null && PermissionUtil.checkExternalStoragePermission(activity.getApplicationContext())
if (activity != null && PermissionUtil.checkStoragePermission(activity.getApplicationContext())
|| getWithoutActivity) {
cursorFolders = ContentResolverHelper.queryResolver(contentResolver, IMAGES_MEDIA_URI,
IMAGES_FOLDER_PROJECTION, null, null,
Expand Down Expand Up @@ -161,9 +161,8 @@ private static boolean isValidAndExistingFilePath(String filePath) {

private static void checkPermissions(@Nullable AppCompatActivity activity,
final ViewThemeUtils viewThemeUtils) {
if (activity != null &&
!PermissionUtil.checkExternalStoragePermission(activity.getApplicationContext())) {
PermissionUtil.requestExternalStoragePermission(activity, viewThemeUtils, true);
if (activity != null) {
PermissionUtil.requestStoragePermissionIfNeeded(activity, viewThemeUtils, true);
}
}

Expand All @@ -177,7 +176,7 @@ public static List<MediaFolder> getVideoFolders(ContentResolver contentResolver,

// query media/image folders
Cursor cursorFolders = null;
if ((activity != null && PermissionUtil.checkExternalStoragePermission(activity.getApplicationContext()))
if ((activity != null && PermissionUtil.checkStoragePermission(activity.getApplicationContext()))
|| getWithoutActivity) {
cursorFolders = contentResolver.query(MediaStore.Video.Media.EXTERNAL_CONTENT_URI, VIDEOS_FOLDER_PROJECTION,
null, null, null);
Expand Down
6 changes: 3 additions & 3 deletions app/src/main/java/com/owncloud/android/ui/activity/FileDisplayActivity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ public void onConfigurationChanged(@NonNull Configuration newConfig) {
if (dialog != null && dialog.isShowing()) {
dialog.dismiss();
getSupportFragmentManager().beginTransaction().remove(fragment).commitNowAllowingStateLoss();
PermissionUtil.requestExternalStoragePermission(this, viewThemeUtils);
PermissionUtil.requestStoragePermissionIfNeeded(this, viewThemeUtils);
}
}
}
Expand All @@ -464,7 +464,7 @@ protected void onPostCreate(Bundle savedInstanceState) {
// storage permissions handled in onRequestPermissionsResult
PermissionUtil.requestNotificationPermission(this);
} else {
PermissionUtil.requestExternalStoragePermission(this, viewThemeUtils);
PermissionUtil.requestStoragePermissionIfNeeded(this, viewThemeUtils);
}

if (IntentExtensionsKt.getParcelableArgument(getIntent(), OCFileListFragment.SEARCH_EVENT, SearchEvent.class) != null) {
Expand Down Expand Up @@ -541,7 +541,7 @@ public void onRequestPermissionsResult(int requestCode, @NonNull String[] permis
case PermissionUtil.PERMISSIONS_POST_NOTIFICATIONS:
// handle notification permission on API level >= 33
// dialogue was dismissed -> prompt for storage permissions
PermissionUtil.requestExternalStoragePermission(this, viewThemeUtils);
PermissionUtil.requestStoragePermissionIfNeeded(this, viewThemeUtils);
break;
case PermissionUtil.PERMISSIONS_EXTERNAL_STORAGE:
// If request is cancelled, result arrays are empty.
Expand Down
6 changes: 3 additions & 3 deletions app/src/main/java/com/owncloud/android/ui/activity/SyncedFoldersActivity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ class SyncedFoldersActivity :
setTheme(R.style.FallbackThemingTheme)
}
binding.emptyList.emptyListViewAction.setOnClickListener { showHiddenItems() }
PermissionUtil.requestExternalStoragePermission(this, viewThemeUtils, true)
PermissionUtil.requestStoragePermissionIfNeeded(this, viewThemeUtils, true)
}

override fun onCreateOptionsMenu(menu: Menu): Boolean {
Expand Down Expand Up @@ -531,7 +531,7 @@ class SyncedFoldersActivity :
android.R.id.home -> finish()
R.id.action_create_custom_folder -> {
Log_OC.d(TAG, "Show custom folder dialog")
if (PermissionUtil.checkExternalStoragePermission(this)) {
if (PermissionUtil.checkStoragePermission(this)) {
val emptyCustomFolder = SyncedFolderDisplayItem(
SyncedFolder.UNPERSISTED_ID,
null,
Expand All @@ -554,7 +554,7 @@ class SyncedFoldersActivity :
)
onSyncFolderSettingsClick(0, emptyCustomFolder)
} else {
PermissionUtil.requestExternalStoragePermission(this, viewThemeUtils, true)
PermissionUtil.requestStoragePermissionIfNeeded(this, viewThemeUtils, true)
}
result = super.onOptionsItemSelected(item)
}
Expand Down
6 changes: 3 additions & 3 deletions app/src/main/java/com/owncloud/android/ui/activity/UploadFilesActivity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ public void onNothingSelected(AdapterView<?> parent) {
}

private void requestPermissions() {
PermissionUtil.requestExternalStoragePermission(this, viewThemeUtils, true);
PermissionUtil.requestStoragePermissionIfNeeded(this, viewThemeUtils, true);
}

public void showToolbarSpinner() {
Expand Down Expand Up @@ -319,7 +319,7 @@ public boolean onOptionsItemSelected(MenuItem item) {
}

private void checkLocalStoragePathPickerPermission() {
if (!PermissionUtil.checkExternalStoragePermission(this)) {
if (!PermissionUtil.checkStoragePermission(this)) {
requestPermissions();
} else {
showLocalStoragePathPickerDialog();
Expand Down Expand Up @@ -628,7 +628,7 @@ public void onClick(View v) {
finish();

} else if (v.getId() == R.id.upload_files_btn_upload) {
if (PermissionUtil.checkExternalStoragePermission(this)) {
if (PermissionUtil.checkStoragePermission(this)) {
if (mCurrentDir != null) {
preferences.setUploadFromLocalLastPath(mCurrentDir.getAbsolutePath());
}
Expand Down
52 changes: 25 additions & 27 deletions app/src/main/java/com/owncloud/android/ui/dialog/StoragePermissionDialogFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ package com.owncloud.android.ui.dialog
import android.app.Dialog
import android.os.Build
import android.os.Bundle
import android.os.Parcelable
import androidx.annotation.RequiresApi
import androidx.appcompat.app.AlertDialog
import androidx.core.os.bundleOf
Expand All @@ -19,27 +18,23 @@ import com.google.android.material.button.MaterialButton
import com.google.android.material.dialog.MaterialAlertDialogBuilder
import com.nextcloud.client.di.Injectable
import com.owncloud.android.R
import com.owncloud.android.utils.PermissionUtil
import com.owncloud.android.utils.PermissionUtil.REQUEST_CODE_MANAGE_ALL_FILES
import com.owncloud.android.utils.theme.ViewThemeUtils
import kotlinx.parcelize.Parcelize
import javax.inject.Inject

/**
* Dialog that shows permission options in SDK >= 30
*
* Allows choosing "full access" (MANAGE_ALL_FILES) or "read-only media" (READ_EXTERNAL_STORAGE)
*/
@RequiresApi(Build.VERSION_CODES.R)
class StoragePermissionDialogFragment : DialogFragment(), Injectable {

private var permissionRequired = false
private var showStrictText = false

@Inject
lateinit var viewThemeUtils: ViewThemeUtils

override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
arguments?.let {
permissionRequired = it.getBoolean(ARG_PERMISSION_REQUIRED, false)
showStrictText = it.getBoolean(ARG_SHOW_STRICT_TEXT, false)
}
}

Expand All @@ -61,11 +56,11 @@ class StoragePermissionDialogFragment : DialogFragment(), Injectable {

override fun onCreateDialog(savedInstanceState: Bundle?): Dialog {
val title = when {
permissionRequired -> R.string.file_management_permission
showStrictText -> R.string.file_management_permission
else -> R.string.file_management_permission_optional
}
val explanationResource = when {
permissionRequired -> R.string.file_management_permission_text
showStrictText -> R.string.file_management_permission_text
else -> R.string.file_management_permission_optional_text
}
val message = getString(explanationResource, getString(R.string.app_name))
Expand All @@ -74,15 +69,14 @@ class StoragePermissionDialogFragment : DialogFragment(), Injectable {
.setTitle(title)
.setMessage(message)
.setPositiveButton(R.string.storage_permission_full_access) { _, _ ->
setResult(Result.FULL_ACCESS)
requestManageAllFiles()
dismiss()
}
.setNegativeButton(R.string.storage_permission_media_read_only) { _, _ ->
setResult(Result.MEDIA_READ_ONLY)
requestMediaReadOnly()
dismiss()
}
.setNeutralButton(R.string.common_cancel) { _, _ ->
setResult(Result.CANCEL)
dismiss()
}

Expand All @@ -91,29 +85,33 @@ class StoragePermissionDialogFragment : DialogFragment(), Injectable {
return dialogBuilder.create()
}

private fun setResult(result: Result) {
parentFragmentManager.setFragmentResult(REQUEST_KEY, bundleOf(RESULT_KEY to result))
@Suppress("DEPRECATION")
private fun requestManageAllFiles() {
activity?.let {
val intent = PermissionUtil.getManageAllFilesIntent(it)
it.startActivityForResult(intent, REQUEST_CODE_MANAGE_ALL_FILES)
}
}

@Parcelize
enum class Result : Parcelable {
CANCEL,
FULL_ACCESS,
MEDIA_READ_ONLY
private fun requestMediaReadOnly() {
activity?.let {
PermissionUtil.showStoragePermissionsSnackbarOrRequest(
activity = it,
viewThemeUtils = viewThemeUtils
)
}
}

companion object {
private const val ARG_PERMISSION_REQUIRED = "ARG_PERMISSION_REQUIRED"
const val REQUEST_KEY = "REQUEST_KEY_STORAGE_PERMISSION"
const val RESULT_KEY = "RESULT"
private const val ARG_SHOW_STRICT_TEXT = "ARG_SHOW_STRICT_TEXT"

/**
* @param permissionRequired Whether the permission is absolutely required by the calling component.
* @param showStrictText Whether the permission is absolutely required by the calling component.
* This changes the texts to a more strict version.
*/
fun newInstance(permissionRequired: Boolean): StoragePermissionDialogFragment {
fun newInstance(showStrictText: Boolean): StoragePermissionDialogFragment {
return StoragePermissionDialogFragment().apply {
arguments = bundleOf(ARG_PERMISSION_REQUIRED to permissionRequired)
arguments = bundleOf(ARG_SHOW_STRICT_TEXT to showStrictText)
}
}
}
Expand Down
Loading
Loading