From b510dc51acdb9f7dd7ec41dabe62cd637bbd16c5 Mon Sep 17 00:00:00 2001
From: topjohnwu <topjohnwu@gmail.com>
Date: Fri, 11 Sep 2020 03:09:01 -0700
Subject: [PATCH] Fix su request auto response

---
 .../magisk/core/su/SuRequestHandler.kt        |  16 +--
 .../{BiometricDialog.kt => BiometricEvent.kt} |   2 +-
 .../magisk/ui/settings/SettingsViewModel.kt   |   4 +-
 .../magisk/ui/superuser/SuperuserViewModel.kt |   6 +-
 .../magisk/ui/surequest/SuRequestActivity.kt  |   2 +-
 .../magisk/ui/surequest/SuRequestViewModel.kt | 121 +++++++++---------
 6 files changed, 74 insertions(+), 77 deletions(-)
 rename app/src/main/java/com/topjohnwu/magisk/events/dialog/{BiometricDialog.kt => BiometricEvent.kt} (97%)

diff --git a/app/src/main/java/com/topjohnwu/magisk/core/su/SuRequestHandler.kt b/app/src/main/java/com/topjohnwu/magisk/core/su/SuRequestHandler.kt
index aefbe2895..cc5123840 100644
--- a/app/src/main/java/com/topjohnwu/magisk/core/su/SuRequestHandler.kt
+++ b/app/src/main/java/com/topjohnwu/magisk/core/su/SuRequestHandler.kt
@@ -18,17 +18,16 @@ import java.io.*
 import java.util.concurrent.TimeUnit
 import java.util.concurrent.TimeUnit.SECONDS
 
-abstract class SuRequestHandler(
-    private val packageManager: PackageManager,
+class SuRequestHandler(
+    private val pm: PackageManager,
     private val policyDB: PolicyDao
 ) : Closeable {
 
     private lateinit var output: DataOutputStream
-    protected lateinit var policy: SuPolicy
+    lateinit var policy: SuPolicy
         private set
 
-    abstract fun onStart()
-
+    // Return true to indicate undetermined policy, require user interaction
     suspend fun start(intent: Intent): Boolean {
         if (!init(intent))
             return false
@@ -40,15 +39,14 @@ abstract class SuRequestHandler(
         when (Config.suAutoReponse) {
             Config.Value.SU_AUTO_DENY -> {
                 respond(SuPolicy.DENY, 0)
-                return true
+                return false
             }
             Config.Value.SU_AUTO_ALLOW -> {
                 respond(SuPolicy.ALLOW, 0)
-                return true
+                return false
             }
         }
 
-        onStart()
         return true
     }
 
@@ -82,7 +80,7 @@ abstract class SuRequestHandler(
                 val map = async { input.readRequest() }.timedAwait() ?: throw SuRequestError()
                 uid = map["uid"]?.toIntOrNull() ?: throw SuRequestError()
             }
-            policy = uid.toPolicy(packageManager)
+            policy = uid.toPolicy(pm)
             true
         } catch (e: Exception) {
             when (e) {
diff --git a/app/src/main/java/com/topjohnwu/magisk/events/dialog/BiometricDialog.kt b/app/src/main/java/com/topjohnwu/magisk/events/dialog/BiometricEvent.kt
similarity index 97%
rename from app/src/main/java/com/topjohnwu/magisk/events/dialog/BiometricDialog.kt
rename to app/src/main/java/com/topjohnwu/magisk/events/dialog/BiometricEvent.kt
index 6a8302d68..63a4f247d 100644
--- a/app/src/main/java/com/topjohnwu/magisk/events/dialog/BiometricDialog.kt
+++ b/app/src/main/java/com/topjohnwu/magisk/events/dialog/BiometricEvent.kt
@@ -5,7 +5,7 @@ import com.topjohnwu.magisk.arch.BaseUIActivity
 import com.topjohnwu.magisk.arch.ViewEvent
 import com.topjohnwu.magisk.core.utils.BiometricHelper
 
-class BiometricDialog(
+class BiometricEvent(
     builder: Builder.() -> Unit
 ) : ViewEvent(), ActivityExecutor {
 
diff --git a/app/src/main/java/com/topjohnwu/magisk/ui/settings/SettingsViewModel.kt b/app/src/main/java/com/topjohnwu/magisk/ui/settings/SettingsViewModel.kt
index eead2a620..3ec02f8c3 100644
--- a/app/src/main/java/com/topjohnwu/magisk/ui/settings/SettingsViewModel.kt
+++ b/app/src/main/java/com/topjohnwu/magisk/ui/settings/SettingsViewModel.kt
@@ -23,7 +23,7 @@ import com.topjohnwu.magisk.core.tasks.PatchAPK
 import com.topjohnwu.magisk.data.database.RepoDao
 import com.topjohnwu.magisk.events.AddHomeIconEvent
 import com.topjohnwu.magisk.events.RecreateEvent
-import com.topjohnwu.magisk.events.dialog.BiometricDialog
+import com.topjohnwu.magisk.events.dialog.BiometricEvent
 import com.topjohnwu.magisk.utils.Utils
 import com.topjohnwu.superuser.Shell
 import kotlinx.coroutines.launch
@@ -125,7 +125,7 @@ class SettingsViewModel(
     }
 
     private fun authenticate(callback: () -> Unit) {
-        BiometricDialog {
+        BiometricEvent {
             // allow the change on success
             onSuccess { callback() }
         }.publish()
diff --git a/app/src/main/java/com/topjohnwu/magisk/ui/superuser/SuperuserViewModel.kt b/app/src/main/java/com/topjohnwu/magisk/ui/superuser/SuperuserViewModel.kt
index 7d55de1d3..4ac7cafc2 100644
--- a/app/src/main/java/com/topjohnwu/magisk/ui/superuser/SuperuserViewModel.kt
+++ b/app/src/main/java/com/topjohnwu/magisk/ui/superuser/SuperuserViewModel.kt
@@ -16,7 +16,7 @@ import com.topjohnwu.magisk.core.utils.BiometricHelper
 import com.topjohnwu.magisk.core.utils.currentLocale
 import com.topjohnwu.magisk.databinding.ComparableRvItem
 import com.topjohnwu.magisk.events.SnackbarEvent
-import com.topjohnwu.magisk.events.dialog.BiometricDialog
+import com.topjohnwu.magisk.events.dialog.BiometricEvent
 import com.topjohnwu.magisk.events.dialog.SuperuserRevokeDialog
 import com.topjohnwu.magisk.view.TappableHeadlineItem
 import com.topjohnwu.magisk.view.TextItem
@@ -86,7 +86,7 @@ class SuperuserViewModel(
         }
 
         if (BiometricHelper.isEnabled) {
-            BiometricDialog {
+            BiometricEvent {
                 onSuccess { updateState() }
             }.publish()
         } else {
@@ -130,7 +130,7 @@ class SuperuserViewModel(
         }
 
         if (BiometricHelper.isEnabled) {
-            BiometricDialog {
+            BiometricEvent {
                 onSuccess { updateState() }
             }.publish()
         } else {
diff --git a/app/src/main/java/com/topjohnwu/magisk/ui/surequest/SuRequestActivity.kt b/app/src/main/java/com/topjohnwu/magisk/ui/surequest/SuRequestActivity.kt
index a28572013..d5f49cb69 100644
--- a/app/src/main/java/com/topjohnwu/magisk/ui/surequest/SuRequestActivity.kt
+++ b/app/src/main/java/com/topjohnwu/magisk/ui/surequest/SuRequestActivity.kt
@@ -29,7 +29,7 @@ open class SuRequestActivity : BaseUIActivity<SuRequestViewModel, ActivityReques
         supportRequestWindowFeature(Window.FEATURE_NO_TITLE)
         lockOrientation()
         window.setFlags(WindowManager.LayoutParams.FLAG_SECURE,
-                WindowManager.LayoutParams.FLAG_SECURE)
+            WindowManager.LayoutParams.FLAG_SECURE)
         super.onCreate(savedInstanceState)
 
         fun showRequest() {
diff --git a/app/src/main/java/com/topjohnwu/magisk/ui/surequest/SuRequestViewModel.kt b/app/src/main/java/com/topjohnwu/magisk/ui/surequest/SuRequestViewModel.kt
index 1ef14ee99..e6280e76f 100644
--- a/app/src/main/java/com/topjohnwu/magisk/ui/surequest/SuRequestViewModel.kt
+++ b/app/src/main/java/com/topjohnwu/magisk/ui/surequest/SuRequestViewModel.kt
@@ -13,12 +13,14 @@ import com.topjohnwu.magisk.R
 import com.topjohnwu.magisk.arch.BaseViewModel
 import com.topjohnwu.magisk.core.Config
 import com.topjohnwu.magisk.core.magiskdb.PolicyDao
+import com.topjohnwu.magisk.core.model.su.SuPolicy
 import com.topjohnwu.magisk.core.model.su.SuPolicy.Companion.ALLOW
 import com.topjohnwu.magisk.core.model.su.SuPolicy.Companion.DENY
 import com.topjohnwu.magisk.core.su.SuRequestHandler
 import com.topjohnwu.magisk.core.utils.BiometricHelper
 import com.topjohnwu.magisk.events.DieEvent
 import com.topjohnwu.magisk.events.ShowUIEvent
+import com.topjohnwu.magisk.events.dialog.BiometricEvent
 import com.topjohnwu.magisk.ui.superuser.SpinnerRvItem
 import com.topjohnwu.magisk.utils.set
 import kotlinx.coroutines.launch
@@ -28,7 +30,7 @@ import java.util.concurrent.TimeUnit.SECONDS
 
 class SuRequestViewModel(
     private val pm: PackageManager,
-    private val policyDB: PolicyDao,
+    policyDB: PolicyDao,
     private val timeoutPrefs: SharedPreferences,
     private val res: Resources
 ) : BaseViewModel() {
@@ -57,89 +59,86 @@ class SuRequestViewModel(
         setItems(items)
     }
 
-    private val handler = Handler()
+    private val handler = SuRequestHandler(pm, policyDB)
+    private lateinit var timer: CountDownTimer
 
     fun grantPressed() {
-        handler.cancelTimer()
+        cancelTimer()
         if (BiometricHelper.isEnabled) {
-            withView {
-                BiometricHelper.authenticate(this) {
-                    handler.respond(ALLOW)
+            BiometricEvent {
+                onSuccess {
+                    respond(ALLOW)
                 }
-            }
+            }.publish()
         } else {
-            handler.respond(ALLOW)
+            respond(ALLOW)
         }
     }
 
     fun denyPressed() {
-        handler.respond(DENY)
+        respond(DENY)
     }
 
     fun spinnerTouched(): Boolean {
-        handler.cancelTimer()
+        cancelTimer()
         return false
     }
 
     fun handleRequest(intent: Intent) {
         viewModelScope.launch {
-            if (!handler.start(intent))
+            if (handler.start(intent))
+                showDialog(handler.policy)
+            else
                 DieEvent().publish()
         }
     }
 
-    private inner class Handler : SuRequestHandler(pm, policyDB) {
+    private fun showDialog(policy: SuPolicy) {
+        icon = policy.applicationInfo.loadIcon(pm)
+        title = policy.appName
+        packageName = policy.packageName
+        selectedItemPosition = timeoutPrefs.getInt(policy.packageName, 0)
 
-        private lateinit var timer: CountDownTimer
+        // Set timer
+        val millis = SECONDS.toMillis(Config.suDefaultTimeout.toLong())
+        timer = SuTimer(millis, 1000).apply { start() }
 
-        fun respond(action: Int) {
-            timer.cancel()
-
-            val pos = selectedItemPosition
-            timeoutPrefs.edit().putInt(policy.packageName, pos).apply()
-            respond(action, Config.Value.TIMEOUT_LIST[pos])
-
-            // Kill activity after response
-            DieEvent().publish()
-        }
-
-        fun cancelTimer() {
-            timer.cancel()
-            denyText = res.getString(R.string.deny)
-        }
-
-        override fun onStart() {
-            icon = policy.applicationInfo.loadIcon(pm)
-            title = policy.appName
-            packageName = policy.packageName
-            selectedItemPosition = timeoutPrefs.getInt(policy.packageName, 0)
-
-            // Set timer
-            val millis = SECONDS.toMillis(Config.suDefaultTimeout.toLong())
-            timer = SuTimer(millis, 1000).apply { start() }
-
-            // Actually show the UI
-            ShowUIEvent().publish()
-        }
-
-        private inner class SuTimer(
-            private val millis: Long,
-            interval: Long
-        ) : CountDownTimer(millis, interval) {
-
-            override fun onTick(remains: Long) {
-                if (!grantEnabled && remains <= millis - 1000) {
-                    grantEnabled = true
-                }
-                denyText = "${res.getString(R.string.deny)} (${(remains / 1000) + 1})"
-            }
-
-            override fun onFinish() {
-                denyText = res.getString(R.string.deny)
-                respond(DENY)
-            }
-
-        }
+        // Actually show the UI
+        ShowUIEvent().publish()
     }
 
+    private fun respond(action: Int) {
+        timer.cancel()
+
+        val pos = selectedItemPosition
+        timeoutPrefs.edit().putInt(handler.policy.packageName, pos).apply()
+        handler.respond(action, Config.Value.TIMEOUT_LIST[pos])
+
+        // Kill activity after response
+        DieEvent().publish()
+    }
+
+    private fun cancelTimer() {
+        timer.cancel()
+        denyText = res.getString(R.string.deny)
+    }
+
+    private inner class SuTimer(
+        private val millis: Long,
+        interval: Long
+    ) : CountDownTimer(millis, interval) {
+
+        override fun onTick(remains: Long) {
+            if (!grantEnabled && remains <= millis - 1000) {
+                grantEnabled = true
+            }
+            denyText = "${res.getString(R.string.deny)} (${(remains / 1000) + 1})"
+        }
+
+        override fun onFinish() {
+            denyText = res.getString(R.string.deny)
+            respond(DENY)
+        }
+
+    }
 }