Improve permission handling logic (#732)

* Improve permission handling logic

Ensure we always ask for storage permissions when required

* Refactor storage permission checks and invert return value

* PasswordStore: improve permission grant flow

* strings: slightly reword permission grant request message

Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>

Co-authored-by: Fabian Henneke <fabian@henneke.me>
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>

2 files changed, 43 insertions, 31 deletions

diff --git a/app/src/main/java/com/zeapo/pwdstore/PasswordStore.kt b/app/src/main/java/com/zeapo/pwdstore/PasswordStore.kt
index 58e1f39c..fe18c6b7 100644
--- a/app/src/main/java/com/zeapo/pwdstore/PasswordStore.kt
+++ b/app/src/main/java/com/zeapo/pwdstore/PasswordStore.kt
@@ -12,7 +12,6 @@ import android.content.SharedPreferences
 import android.content.pm.PackageManager
 import android.content.pm.ShortcutInfo.Builder
 import android.content.pm.ShortcutManager
-import android.graphics.Color
 import android.graphics.drawable.Icon
 import android.net.Uri
 import android.os.Build
@@ -180,34 +179,7 @@ class PasswordStore : AppCompatActivity() {
         super.onResume()
         // do not attempt to checkLocalRepository() if no storage permission: immediate crash
         if (settings.getBoolean("git_external", false)) {
-            if (ContextCompat.checkSelfPermission(activity, Manifest.permission.WRITE_EXTERNAL_STORAGE)
-                    != PackageManager.PERMISSION_GRANTED) {
-                if (ActivityCompat.shouldShowRequestPermissionRationale(activity, Manifest.permission.WRITE_EXTERNAL_STORAGE)) {
-                    val snack = Snackbar.make(
-                                    findViewById(R.id.main_layout),
-                                    getString(R.string.access_sdcard_text),
-                                    Snackbar.LENGTH_INDEFINITE)
-                            .setAction(R.string.dialog_ok) {
-                                ActivityCompat.requestPermissions(
-                                        activity,
-                                        arrayOf(Manifest.permission.WRITE_EXTERNAL_STORAGE),
-                                        REQUEST_EXTERNAL_STORAGE)
-                            }
-                    snack.show()
-                    val view = snack.view
-                    val tv: AppCompatTextView = view.findViewById(com.google.android.material.R.id.snackbar_text)
-                    tv.setTextColor(Color.WHITE)
-                    tv.maxLines = 10
-                } else {
-                    // No explanation needed, we can request the permission.
-                    ActivityCompat.requestPermissions(
-                            activity,
-                            arrayOf(Manifest.permission.WRITE_EXTERNAL_STORAGE),
-                            REQUEST_EXTERNAL_STORAGE)
-                }
-            } else {
-                checkLocalRepository()
-            }
+            hasRequiredStoragePermissions(true)
         } else {
             checkLocalRepository()
         }
@@ -389,8 +361,12 @@ class PasswordStore : AppCompatActivity() {
     }
 
     private fun initializeRepositoryInfo() {
+        val externalRepo = settings.getBoolean("git_external", false)
         val externalRepoPath = settings.getString("git_external_repo", null)
-        if (settings.getBoolean("git_external", false) && externalRepoPath != null) {
+        if (externalRepo && !hasRequiredStoragePermissions()) {
+            return
+        }
+        if (externalRepo && externalRepoPath != null) {
             val dir = File(externalRepoPath)
             if (dir.exists() && dir.isDirectory &&
                     getPasswords(dir, getRepositoryDirectory(this), sortOrder).isNotEmpty()) {
@@ -413,6 +389,36 @@ class PasswordStore : AppCompatActivity() {
         createRepository()
     }
 
+    /**
+     * Validates if storage permission is granted, and requests for it if not. The return value
+     * is true if the permission has been granted.
+     */
+    private fun hasRequiredStoragePermissions(checkLocalRepo: Boolean = false): Boolean {
+        return if (ContextCompat.checkSelfPermission(activity, Manifest.permission.WRITE_EXTERNAL_STORAGE)
+            != PackageManager.PERMISSION_GRANTED) {
+            Snackbar.make(
+                    findViewById(R.id.main_layout),
+                    getString(R.string.access_sdcard_text),
+                    Snackbar.LENGTH_INDEFINITE
+            ).run {
+                setAction(getString(R.string.snackbar_action_grant)) {
+                    ActivityCompat.requestPermissions(
+                            activity,
+                            arrayOf(Manifest.permission.WRITE_EXTERNAL_STORAGE),
+                            REQUEST_EXTERNAL_STORAGE
+                    )
+                    dismiss()
+                }
+                show()
+            }
+            false
+        } else {
+            if (checkLocalRepo)
+                checkLocalRepository()
+            true
+        }
+    }
+
     private fun checkLocalRepository() {
         val repo = initialize(this)
         if (repo == null) {
@@ -631,7 +637,7 @@ class PasswordStore : AppCompatActivity() {
         get() = plist?.currentDir ?: getRepositoryDirectory(applicationContext)
 
     private fun commitChange(message: String) {
-        Companion.commitChange(activity, message)
+        commitChange(activity, message)
     }
 
     override fun onActivityResult(requestCode: Int, resultCode: Int, data: Intent?) {
diff --git a/app/src/main/java/com/zeapo/pwdstore/UserPreference.kt b/app/src/main/java/com/zeapo/pwdstore/UserPreference.kt
index 9d56845c..32d1e87a 100644
--- a/app/src/main/java/com/zeapo/pwdstore/UserPreference.kt
+++ b/app/src/main/java/com/zeapo/pwdstore/UserPreference.kt
@@ -461,6 +461,12 @@ class UserPreference : AppCompatActivity() {
         }
     }
 
+    override fun onBackPressed() {
+        super.onBackPressed()
+        setResult(Activity.RESULT_OK)
+        finish()
+    }
+
     public override fun onCreate(savedInstanceState: Bundle?) {
         super.onCreate(savedInstanceState)
         when (intent?.getStringExtra("operation")) {