Merge pull request #11242 from nextcloud/fix/passcode-double-check

PassCodeManager: add some logic to avoid counting the same activity twice

app/src/androidTest/java/com/owncloud/android/authentication/PassCodeManagerIT.kt

@@ -0,0 +1,79 @@
+/*
+ * Nextcloud Android client application
+ *
+ *  @author Álvaro Brey
+ *  Copyright (C) 2023 Álvaro Brey
+ *  Copyright (C) 2023 Nextcloud GmbH
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU AFFERO GENERAL PUBLIC LICENSE for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public
+ * License along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+package com.owncloud.android.authentication
+
+import androidx.test.core.app.launchActivity
+import com.nextcloud.client.TestActivity
+import com.nextcloud.client.core.Clock
+import com.nextcloud.client.preferences.AppPreferences
+import com.owncloud.android.ui.activity.SettingsActivity
+import io.mockk.MockKAnnotations
+import io.mockk.every
+import io.mockk.impl.annotations.MockK
+import org.junit.Assert.assertTrue
+import org.junit.Before
+import org.junit.Test
+
+/**
+ * This class should really be unit tests, but PassCodeManager needs a refactor
+ * to decouple the locking logic from the platform classes
+ */
+class PassCodeManagerIT {
+    @MockK
+    lateinit var appPreferences: AppPreferences
+
+    @MockK
+    lateinit var clockImpl: Clock
+
+    lateinit var sut: PassCodeManager
+
+    @Before
+    fun before() {
+        MockKAnnotations.init(this, relaxed = true)
+        sut = PassCodeManager(appPreferences, clockImpl)
+    }
+
+    @Test
+    fun testResumeDuplicateActivity() {
+        // set locked state
+        every { appPreferences.lockPreference } returns SettingsActivity.LOCK_PASSCODE
+        every { appPreferences.lockTimestamp } returns 200
+        every { clockImpl.millisSinceBoot } returns 10000
+
+        launchActivity<TestActivity>().use { scenario ->
+            scenario.onActivity { activity ->
+                // resume activity twice
+                var askedForPin = sut.onActivityResumed(activity)
+                assertTrue("Passcode not requested on first launch", askedForPin)
+                sut.onActivityResumed(activity)
+
+                // stop it once
+                sut.onActivityStopped(activity)
+
+                // resume again. should ask for passcode
+                askedForPin = sut.onActivityResumed(activity)
+                assertTrue("Passcode not requested on subsequent launch after stop", askedForPin)
+            }
+        }
+    }
+}

app/src/main/java/com/owncloud/android/authentication/PassCodeManager.kt

@@ -25,6 +25,7 @@ import android.content.Intent
 import android.os.PowerManager
 import android.view.View
 import android.view.WindowManager
+import androidx.annotation.VisibleForTesting
 import com.nextcloud.client.core.Clock
 import com.nextcloud.client.preferences.AppPreferences
 import com.owncloud.android.MainApp
@@ -48,9 +49,12 @@ class PassCodeManager(private val preferences: AppPreferences, private val clock
          * the pass code being requested on screen rotations.
          */
         private const val PASS_CODE_TIMEOUT = 5000
+
+        private const val TAG = "PassCodeManager"
     }
 
     private var visibleActivitiesCounter = 0
+    private var lastResumedActivity: Activity? = null
 
     private fun isExemptActivity(activity: Activity): Boolean {
         return exemptOfPasscodeActivities.contains(activity.javaClass)
@@ -64,20 +68,17 @@ class PassCodeManager(private val preferences: AppPreferences, private val clock
         if (!isExemptActivity(activity)) {
             val passcodeRequested = passCodeShouldBeRequested(timestamp)
             val credentialsRequested = deviceCredentialsShouldBeRequested(timestamp, activity)
-            if (passcodeRequested || credentialsRequested) {
-                getActivityRootView(activity)?.visibility = View.GONE
-            } else {
-                getActivityRootView(activity)?.visibility = View.VISIBLE
-            }
+            val shouldHideView = passcodeRequested || credentialsRequested
+            toggleActivityVisibility(shouldHideView, activity)
+            askedForPin = shouldHideView
+
             if (passcodeRequested) {
-                askedForPin = true
-                preferences.lockTimestamp = 0
                 requestPasscode(activity)
+            } else if (credentialsRequested) {
+                requestCredentials(activity)
             }
-            if (credentialsRequested) {
-                askedForPin = true
+            if (askedForPin) {
                 preferences.lockTimestamp = 0
-                requestCredentials(activity)
             }
         }
 
@@ -86,12 +87,39 @@ class PassCodeManager(private val preferences: AppPreferences, private val clock
         }
 
         if (!isExemptActivity(activity)) {
-            visibleActivitiesCounter++ // keep it AFTER passCodeShouldBeRequested was checked
+            addVisibleActivity(activity) // keep it AFTER passCodeShouldBeRequested was checked
         }
 
         return askedForPin
     }
 
+    /**
+     * Used to hide root view while transitioning to passcode activity
+     */
+    private fun toggleActivityVisibility(
+        hide: Boolean,
+        activity: Activity
+    ) {
+        if (hide) {
+            getActivityRootView(activity)?.visibility = View.GONE
+        } else {
+            getActivityRootView(activity)?.visibility = View.VISIBLE
+        }
+    }
+
+    private fun addVisibleActivity(activity: Activity) {
+        // don't count the same activity twice
+        if (lastResumedActivity != activity) {
+            visibleActivitiesCounter++
+            lastResumedActivity = activity
+        }
+    }
+
+    private fun removeVisibleActivity() {
+        visibleActivitiesCounter--
+        lastResumedActivity = null
+    }
+
     private fun setSecureFlag(activity: Activity) {
         val window = activity.window
         if (window != null) {
@@ -118,7 +146,7 @@ class PassCodeManager(private val preferences: AppPreferences, private val clock
 
     fun onActivityStopped(activity: Activity) {
         if (visibleActivitiesCounter > 0 && !isExemptActivity(activity)) {
-            visibleActivitiesCounter--
+            removeVisibleActivity()
         }
         val powerMgr = activity.getSystemService(Context.POWER_SERVICE) as PowerManager
         if ((isPassCodeEnabled() || deviceCredentialsAreEnabled(activity)) && !powerMgr.isScreenOn) {
@@ -137,7 +165,8 @@ class PassCodeManager(private val preferences: AppPreferences, private val clock
         abs(clock.millisSinceBoot - timestamp) > PASS_CODE_TIMEOUT &&
             visibleActivitiesCounter <= 0
 
-    private fun passCodeShouldBeRequested(timestamp: Long): Boolean {
+    @VisibleForTesting
+    fun passCodeShouldBeRequested(timestamp: Long): Boolean {
         return shouldBeLocked(timestamp) && isPassCodeEnabled()
     }
 
@@ -153,7 +182,7 @@ class PassCodeManager(private val preferences: AppPreferences, private val clock
     }
 
     private fun getActivityRootView(activity: Activity): View? {
-        return activity.window.findViewById(android.R.id.content)
-            ?: activity.window.decorView.findViewById(android.R.id.content)
+        return activity.window?.findViewById(android.R.id.content)
+            ?: activity.window?.decorView?.findViewById(android.R.id.content)
     }
 }

app/src/test/java/com/owncloud/android/authentication/PassCodeManagerTest.kt

@@ -0,0 +1,71 @@
+/*
+ * Nextcloud Android client application
+ *
+ *  @author Álvaro Brey
+ *  Copyright (C) 2023 Álvaro Brey
+ *  Copyright (C) 2023 Nextcloud GmbH
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU AFFERO GENERAL PUBLIC LICENSE for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public
+ * License along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+package com.owncloud.android.authentication
+
+import com.nextcloud.client.core.Clock
+import com.nextcloud.client.preferences.AppPreferences
+import com.owncloud.android.ui.activity.SettingsActivity
+import io.mockk.MockKAnnotations
+import io.mockk.every
+import io.mockk.impl.annotations.MockK
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
+import org.junit.Before
+import org.junit.Test
+
+class PassCodeManagerTest {
+    @MockK
+    lateinit var appPreferences: AppPreferences
+
+    @MockK
+    lateinit var clockImpl: Clock
+
+    lateinit var sut: PassCodeManager
+
+    @Before
+    fun before() {
+        MockKAnnotations.init(this, relaxed = true)
+        sut = PassCodeManager(appPreferences, clockImpl)
+    }
+
+    @Test
+    fun testLocked() {
+        every { appPreferences.lockPreference } returns SettingsActivity.LOCK_PASSCODE
+        every { clockImpl.millisSinceBoot } returns 10000
+        assertTrue("Passcode not requested", sut.passCodeShouldBeRequested(200))
+    }
+
+    @Test
+    fun testPasscodeNotRequested_notEnabled() {
+        every { appPreferences.lockPreference } returns ""
+        every { clockImpl.millisSinceBoot } returns 10000
+        assertFalse("Passcode requested but it shouldn't have been", sut.passCodeShouldBeRequested(200))
+    }
+
+    @Test
+    fun testPasscodeNotRequested_unlockedRecently() {
+        every { appPreferences.lockPreference } returns SettingsActivity.LOCK_PASSCODE
+        every { clockImpl.millisSinceBoot } returns 210
+        assertFalse("Passcode requested but it shouldn't have been", sut.passCodeShouldBeRequested(200))
+    }
+}