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>

4 files changed, 45 insertions, 33 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")) {
diff --git a/app/src/main/res/values-ru/strings.xml b/app/src/main/res/values-ru/strings.xml
index 051ab024..07c4dca7 100644
--- a/app/src/main/res/values-ru/strings.xml
+++ b/app/src/main/res/values-ru/strings.xml
@@ -303,7 +303,6 @@
     <string name="biometric_auth_summary">Когда ключено, Password Store будет запрашивать ваш опечаток пальца при каждом запуске приложения</string>
     <string name="biometric_auth_summary_error">Сенсор отпечатка пальца не доступен или отсутствует</string>
     <string name="ssh_openkeystore_clear_keyid">Очистить сохраненный SSH Key идентификатор OpenKystortore</string>
-    <string name="access_sdcard_text">Хранилище находится на sd-карте, но приложение не имеет прав доступа к ней. Пожалуйста, дайте приложению необходимые разрешения.</string>
     <string name="your_public_key">Ваш публичный ключ</string>
     <string name="error_generate_ssh_key">Возникла ошибка при попытке генерации ssh ключа</string>
     <string name="pref_show_hidden_title">Показать скрытые папки</string>
diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml
index 1c1f480d..bc7f0a6f 100644
--- a/app/src/main/res/values/strings.xml
+++ b/app/src/main/res/values/strings.xml
@@ -333,7 +333,7 @@
     <string name="biometric_auth_summary">When enabled, Password Store will prompt you for your fingerprint when launching the app</string>
     <string name="biometric_auth_summary_error">Fingerprint hardware not accessible or missing</string>
     <string name="ssh_openkeystore_clear_keyid">Clear remembered OpenKeystore SSH Key ID</string>
-    <string name="access_sdcard_text">The store is on the sdcard but the app does not have permission to access it. Please give permission.</string>
+    <string name="access_sdcard_text">The store location is in your SD Card or Internal storage, but the app does not have permission to access it.</string>
     <string name="your_public_key">Your public key</string>
     <string name="error_generate_ssh_key">Error while trying to generate the ssh-key</string>
     <string name="pref_show_hidden_title">Show hidden folders</string>
@@ -364,4 +364,5 @@
     <string name="bottom_sheet_create_new_password">Create new password</string>
     <string name="autofill_onboarding_dialog_title">New, revamped Autofill!</string>
     <string name="autofill_onboarding_dialog_message">In this release, Autofill support has been massively improved with advanced features like anti-phishing protection and enhanced reliability. If you have been holding out on using it because of the shortcomings on the previous version, you\'ll likely love the new iteration. Give it a shot!</string>
+    <string name="snackbar_action_grant">Grant</string>
 </resources>