Rework storage permission handling on Android to serve permission req…

…uest in context
opengisch · Dec 27, 2024 · e7b94e8 · e7b94e8 · qfield-fairy · Dec 27, 2024
1 parent 782e9e1
commit e7b94e8
Show file tree

Hide file tree

Showing 7 changed files with 55 additions and 43 deletions.
diff --git a/platform/android/src/ch/opengis/qfield/QFieldActivity.java b/platform/android/src/ch/opengis/qfield/QFieldActivity.java
@@ -503,7 +503,6 @@ private void prepareQtActivity() {
             getSharedPreferences("QField", Context.MODE_PRIVATE);
         sharedPreferenceEditor = sharedPreferences.edit();
 
-        checkPermissions();
         checkAllFileAccess(); // Storage access permission handling for Android
                               // 11+
 
@@ -1242,7 +1241,7 @@ public void run() {
         });
     }
 
-    private void checkPermissions() {
+    private void checkStoragePermissions() {
         List<String> permissionsList = new ArrayList<String>();
         if (ContextCompat.checkSelfPermission(
                 QFieldActivity.this,
@@ -1282,8 +1281,8 @@ private void checkAllFileAccess() {
         if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R &&
             !Environment.isExternalStorageManager() &&
             !sharedPreferences.getBoolean("DontAskAllFilesPermission", false)) {
-            // if MANAGE_EXTERNAL_STORAGE permission isn't in the manifest, bail
-            // out
+            // if MANAGE_EXTERNAL_STORAGE permission isn't in the manifest,
+            // bail out
             String[] requestedPermissions;
             try {
                 PackageInfo pi = getPackageManager().getPackageInfo(
@@ -1299,6 +1298,8 @@ private void checkAllFileAccess() {
                 return;
             }
 
+            checkStoragePermissions();
+
             AlertDialog.Builder builder =
                 new AlertDialog.Builder(this, R.style.DialogTheme);
             builder.setTitle(getString(R.string.grant_permission));

diff --git a/src/core/platforms/android/androidplatformutilities.cpp b/src/core/platforms/android/androidplatformutilities.cpp
@@ -516,8 +516,6 @@ ViewStatus *AndroidPlatformUtilities::open( const QString &filePath, bool isEdit
   if ( QFileInfo( filePath ).isDir() )
     return nullptr;
 
-  checkWriteExternalStoragePermissions();
-
   QMimeDatabase db;
   const QString mimeType = db.mimeTypeForFile( filePath ).name();
 
@@ -547,51 +545,72 @@ ViewStatus *AndroidPlatformUtilities::open( const QString &filePath, bool isEdit
   return viewStatus;
 }
 
+void AndroidPlatformUtilities::requestStoragePermission() const
+{
+  if ( !QSettings().value( QStringLiteral( "QField/storagePermissionChecked" ), false ).toBool() )
+  {
+    const int sdkVersion = QCoreApplication::instance()->nativeInterface<QNativeInterface::QAndroidApplication>()->sdkVersion();
+
+    QStringList permissions;
+    permissions << "android.permission.READ_EXTERNAL_STORAGE"
+                << "android.permission.WRITE_EXTERNAL_STORAGE"
+                << "android.permission.ACCESS_MEDIA_LOCATION";
+    if ( sdkVersion >= 33 )
+    {
+      permissions << "android.permission.READ_MEDIA_IMAGES"
+                  << "android.permission.READ_MEDIA_VIDEO";
+    }
+
+    checkAndAcquirePermissions( permissions, true );
+    QSettings().setValue( QStringLiteral( "QField/storagePermissionChecked" ), true );
+  }
+}
+
 bool AndroidPlatformUtilities::checkPositioningPermissions() const
 {
   // First check for coarse permissions. If the user configured QField to only get coarse permissions
   // it's his wish and we just let it be.
   auto r = QtAndroidPrivate::checkPermission( "android.permission.ACCESS_COARSE_LOCATION" ).result();
   if ( r == QtAndroidPrivate::Denied )
   {
-    return checkAndAcquirePermissions( "android.permission.ACCESS_FINE_LOCATION" );
+    return checkAndAcquirePermissions( QStringList() << "android.permission.ACCESS_FINE_LOCATION" );
   }
   return true;
 }
 
 bool AndroidPlatformUtilities::checkCameraPermissions() const
 {
-  return checkAndAcquirePermissions( "android.permission.CAMERA" );
+  return checkAndAcquirePermissions( QStringList() << "android.permission.CAMERA" );
 }
 
 bool AndroidPlatformUtilities::checkMicrophonePermissions() const
 {
-  return checkAndAcquirePermissions( "android.permission.RECORD_AUDIO" );
-}
-
-bool AndroidPlatformUtilities::checkWriteExternalStoragePermissions() const
-{
-  return checkAndAcquirePermissions( "android.permission.WRITE_EXTERNAL_STORAGE" );
+  return checkAndAcquirePermissions( QStringList() << "android.permission.RECORD_AUDIO" );
 }
 
-bool AndroidPlatformUtilities::checkAndAcquirePermissions( const QString &permissions ) const
+bool AndroidPlatformUtilities::checkAndAcquirePermissions( QStringList permissions, bool forceAsk ) const
 {
-  QStringList requestedPermissions = permissions.split( ';' );
-  requestedPermissions.erase( std::remove_if( requestedPermissions.begin(), requestedPermissions.end(),
-                                              []( const QString &permission ) {
-                                                auto r = QtAndroidPrivate::checkPermission( permission ).result();
-                                                return r != QtAndroidPrivate::Denied;
-                                              } ),
-                              requestedPermissions.end() );
+  if ( !forceAsk )
+  {
+    permissions.erase( std::remove_if( permissions.begin(), permissions.end(),
+                                       []( const QString &permission ) {
+                                         auto r = QtAndroidPrivate::checkPermission( permission ).result();
+                                         return r != QtAndroidPrivate::Denied;
+                                       } ),
+                       permissions.end() );
+  }
 
-  if ( !requestedPermissions.isEmpty() )
+  if ( !permissions.isEmpty() )
   {
-    for ( const QString &permission : requestedPermissions )
+    for ( const QString &permission : permissions )
     {
       auto r = QtAndroidPrivate::requestPermission( permission ).result();
       if ( r == QtAndroidPrivate::Denied )
       {
-        return false;
+        if ( !forceAsk )
+        {
+          return false;
+        }
       }
     }
   }
@@ -695,7 +714,7 @@ QVariantMap AndroidPlatformUtilities::sceneMargins( QQuickWindow *window ) const
 void AndroidPlatformUtilities::uploadPendingAttachments( QFieldCloudConnection *connection ) const
 {
   // Request notification permission
-  checkAndAcquirePermissions( QStringLiteral( "android.permission.POST_NOTIFICATIONS" ) );
+  checkAndAcquirePermissions( QStringList() << QStringLiteral( "android.permission.POST_NOTIFICATIONS" ) );
 
   QTimer::singleShot( 500, [connection]() {
     if ( connection )

diff --git a/src/core/platforms/android/androidplatformutilities.h b/src/core/platforms/android/androidplatformutilities.h
@@ -62,10 +62,10 @@ class AndroidPlatformUtilities : public PlatformUtilities
 
     ViewStatus *open( const QString &filePath, bool isEditing, QObject *parent = nullptr ) override;
 
+    void requestStoragePermission() const override;
     bool checkPositioningPermissions() const override;
     bool checkCameraPermissions() const override;
     bool checkMicrophonePermissions() const override;
-    bool checkWriteExternalStoragePermissions() const override;
 
     void setScreenLockPermission( const bool allowLock ) override;
 
@@ -86,7 +86,7 @@ class AndroidPlatformUtilities : public PlatformUtilities
 
   private:
     // separate multiple permissions using a semi-column (;)
-    bool checkAndAcquirePermissions( const QString &permissions ) const;
+    bool checkAndAcquirePermissions( QStringList permissions, bool forceAsk = false ) const;
     ResourceSource *processCameraActivity( const QString &prefix, const QString &filePath, const QString &suffix, bool isVideo, QObject *parent = nullptr );
     ResourceSource *processGalleryActivity( const QString &prefix, const QString &filePath, const QString &mimeType, QObject *parent = nullptr );
 

diff --git a/src/core/platforms/platformutilities.cpp b/src/core/platforms/platformutilities.cpp
@@ -373,11 +373,6 @@ bool PlatformUtilities::checkMicrophonePermissions() const
   return true;
 }
 
-bool PlatformUtilities::checkWriteExternalStoragePermissions() const
-{
-  return true;
-}
-
 void PlatformUtilities::copyTextToClipboard( const QString &string ) const
 {
   QGuiApplication::clipboard()->setText( string );

diff --git a/src/core/platforms/platformutilities.h b/src/core/platforms/platformutilities.h
@@ -247,14 +247,6 @@ class QFIELD_CORE_EXPORT PlatformUtilities : public QObject
      */
     Q_DECL_DEPRECATED Q_INVOKABLE virtual bool checkMicrophonePermissions() const;
 
-    /**
-     * Checks for permissions to write exeternal storage.
-     * If the permissions are not given, the user will be asked to grant
-     * permissions.
-     * \deprecated Since QField 3.1
-     */
-    Q_DECL_DEPRECATED Q_INVOKABLE virtual bool checkWriteExternalStoragePermissions() const;
-
     /**
      * Sets whether the device screen is allowed to go in lock mode.
      * @param allowLock if set to FALSE, the screen will not be allowed to lock.
@@ -311,13 +303,14 @@ class QFIELD_CORE_EXPORT PlatformUtilities : public QObject
      */
     Q_INVOKABLE virtual void vibrate( int milliseconds ) const { Q_UNUSED( milliseconds ) }
 
-    static PlatformUtilities *instance();
-
+    Q_INVOKABLE virtual void requestStoragePermission() const {};
     virtual Qt::PermissionStatus checkCameraPermission() const;
     virtual void requestCameraPermission( std::function<void( Qt::PermissionStatus )> func );
     virtual Qt::PermissionStatus checkMicrophonePermission() const;
     virtual void requestMicrophonePermission( std::function<void( Qt::PermissionStatus )> func );
 
+    static PlatformUtilities *instance();
+
   signals:
     //! Emitted when a resource has been received.
     void resourceReceived( const QString &path );

diff --git a/src/qml/WelcomeScreen.qml b/src/qml/WelcomeScreen.qml
@@ -446,6 +446,7 @@ Page {
             Layout.fillWidth: true
             text: qsTr("Open local file")
             onClicked: {
+              platformUtilities.requestStoragePermission();
               openLocalDataPicker();
             }
           }

diff --git a/src/qml/editorwidgets/ExternalResource.qml b/src/qml/editorwidgets/ExternalResource.qml
@@ -314,6 +314,7 @@ EditorWidgetBase {
 
       onClicked: {
         if (FileUtils.fileExists(prefixToRelativePath + value)) {
+          platformUtilities.requestStoragePermission();
           __viewStatus = platformUtilities.open(prefixToRelativePath + value, isEnabled, this);
         }
       }
@@ -631,6 +632,7 @@ EditorWidgetBase {
 
   function attachFile() {
     Qt.inputMethod.hide();
+    platformUtilities.requestStoragePermission();
     var filepath = getResourceFilePath();
     if (documentViewer == document_AUDIO) {
       __resourceSource = platformUtilities.getFile(qgisProject.homePath + '/', filepath, PlatformUtilities.AudioFiles, this);
@@ -641,6 +643,7 @@ EditorWidgetBase {
 
   function attachGallery() {
     Qt.inputMethod.hide();
+    platformUtilities.requestStoragePermission();
     var filepath = getResourceFilePath();
     if (documentViewer == document_VIDEO) {
       __resourceSource = platformUtilities.getGalleryVideo(qgisProject.homePath + '/', filepath, this);