From 5e4dfaf4eaceda3fa4c55e5cdb59faf5acbab460 Mon Sep 17 00:00:00 2001 From: Beverly Date: Fri, 28 Jan 2022 22:06:14 +0000 Subject: [PATCH] DO NOT MERGE Don't show notification private view too early Don't show the notification private view immediately after authenticating with a biometric that can bypass the lockscreen. We don't want to flash the private notification right before we hide the notification shade because it looks janky. Instead, wait to update it until after the keyguard is done going away. Test: atest SensitiveContentCoordinatorTest NotificationViewHierarchyManagerTest Test: manually test with and w/o the new notification pipeline enabled 1. add FP for security 2. Settings > Notifications > Disable "Sensitive notifications" 3. Post a notification that would be partially hidden as a sensitive notification 4. Authenticate with FP on the LS (repeat 10x) times Expected: notifications don't flash their private views when entering the device Fixes: 212835832 Merged-In: I5ce4460043250aad4a5b0b0838e6f3b5ae6feb1d Change-Id: I5ce4460043250aad4a5b0b0838e6f3b5ae6feb1d (cherry picked from commit 670240dc87f914e73f513e579a54f869e380171c) --- .../keyguard/KeyguardUpdateMonitor.java | 15 ++++++++ .../NotificationViewHierarchyManager.java | 17 ++++++++-- .../dagger/StatusBarDependenciesModule.java | 10 ++++-- .../coordinator/KeyguardCoordinator.java | 2 ++ .../SensitiveContentCoordinator.kt | 30 ++++++++++++++-- .../NotificationViewHierarchyManagerTest.java | 6 +++- .../SensitiveContentCoordinatorTest.kt | 34 ++++++++++++++++++- 7 files changed, 105 insertions(+), 9 deletions(-) diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitor.java b/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitor.java index 2789e27178cca..42f0fccd61b05 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitor.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitor.java @@ -1186,6 +1186,21 @@ public class KeyguardUpdateMonitor implements TrustManager.TrustListener, Dumpab return fingerprintAllowed || faceAllowed; } + /** + * Returns whether the user is unlocked with a biometric that is currently bypassing + * the lock screen. + */ + public boolean getUserUnlockedWithBiometricAndIsBypassing(int userId) { + BiometricAuthenticated fingerprint = mUserFingerprintAuthenticated.get(userId); + BiometricAuthenticated face = mUserFaceAuthenticated.get(userId); + // fingerprint always bypasses + boolean fingerprintAllowed = fingerprint != null && fingerprint.mAuthenticated + && isUnlockingWithBiometricAllowed(fingerprint.mIsStrongBiometric); + boolean faceAllowed = face != null && face.mAuthenticated + && isUnlockingWithBiometricAllowed(face.mIsStrongBiometric); + return fingerprintAllowed || faceAllowed && mKeyguardBypassController.canBypass(); + } + public boolean getUserTrustIsManaged(int userId) { return mUserTrustIsManaged.get(userId) && !isTrustDisabled(userId); } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java index 464b2b69c58e8..e0e928073336d 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java @@ -25,6 +25,7 @@ import android.util.Log; import android.view.View; import android.view.ViewGroup; +import com.android.keyguard.KeyguardUpdateMonitor; import com.android.systemui.R; import com.android.systemui.dagger.qualifiers.Main; import com.android.systemui.flags.FeatureFlags; @@ -42,6 +43,7 @@ import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow import com.android.systemui.statusbar.notification.stack.ForegroundServiceSectionController; import com.android.systemui.statusbar.notification.stack.NotificationListContainer; import com.android.systemui.statusbar.phone.KeyguardBypassController; +import com.android.systemui.statusbar.policy.KeyguardStateController; import com.android.systemui.util.Assert; import com.android.wm.shell.bubbles.Bubbles; @@ -92,6 +94,8 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle private final KeyguardBypassController mBypassController; private final ForegroundServiceSectionController mFgsSectionController; private AssistantFeedbackController mAssistantFeedbackController; + private final KeyguardStateController mKeyguardStateController; + private final KeyguardUpdateMonitor mKeyguardUpdateMonitor; private final Context mContext; private NotificationPresenter mPresenter; @@ -121,7 +125,9 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle ForegroundServiceSectionController fgsSectionController, DynamicChildBindController dynamicChildBindController, LowPriorityInflationHelper lowPriorityInflationHelper, - AssistantFeedbackController assistantFeedbackController) { + AssistantFeedbackController assistantFeedbackController, + KeyguardUpdateMonitor keyguardUpdateMonitor, + KeyguardStateController keyguardStateController) { mContext = context; mHandler = mainHandler; mFeatureFlags = featureFlags; @@ -140,6 +146,8 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle mDynamicChildBindController = dynamicChildBindController; mLowPriorityInflationHelper = lowPriorityInflationHelper; mAssistantFeedbackController = assistantFeedbackController; + mKeyguardUpdateMonitor = keyguardUpdateMonitor; + mKeyguardStateController = keyguardStateController; } public void setUpWithPresenter(NotificationPresenter presenter, @@ -163,6 +171,11 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle beginUpdate(); + boolean dynamicallyUnlocked = mDynamicPrivacyController.isDynamicallyUnlocked() + && !(mStatusBarStateController.getState() == StatusBarState.KEYGUARD + && mKeyguardUpdateMonitor.getUserUnlockedWithBiometricAndIsBypassing( + KeyguardUpdateMonitor.getCurrentUser())) + && !mKeyguardStateController.isKeyguardGoingAway(); List activeNotifications = mEntryManager.getVisibleNotifications(); ArrayList toShow = new ArrayList<>(activeNotifications.size()); final int N = activeNotifications.size(); @@ -181,7 +194,7 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle boolean devicePublic = mLockscreenUserManager.isLockscreenPublicMode(currentUserId); boolean userPublic = devicePublic || mLockscreenUserManager.isLockscreenPublicMode(userId); - if (userPublic && mDynamicPrivacyController.isDynamicallyUnlocked() + if (userPublic && dynamicallyUnlocked && (userId == currentUserId || userId == UserHandle.USER_ALL || !mLockscreenUserManager.needsSeparateWorkChallenge(userId))) { userPublic = false; diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/dagger/StatusBarDependenciesModule.java b/packages/SystemUI/src/com/android/systemui/statusbar/dagger/StatusBarDependenciesModule.java index f2d926d97108a..4c5522fc66c89 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/dagger/StatusBarDependenciesModule.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/dagger/StatusBarDependenciesModule.java @@ -23,6 +23,7 @@ import android.os.Handler; import android.service.dreams.IDreamManager; import com.android.internal.statusbar.IStatusBarService; +import com.android.keyguard.KeyguardUpdateMonitor; import com.android.systemui.animation.ActivityLaunchAnimator; import com.android.systemui.animation.DialogLaunchAnimator; import com.android.systemui.dagger.SysUISingleton; @@ -69,6 +70,7 @@ import com.android.systemui.statusbar.phone.StatusBarIconControllerImpl; import com.android.systemui.statusbar.phone.StatusBarRemoteInputCallback; import com.android.systemui.statusbar.phone.ongoingcall.OngoingCallController; import com.android.systemui.statusbar.phone.ongoingcall.OngoingCallLogger; +import com.android.systemui.statusbar.policy.KeyguardStateController; import com.android.systemui.statusbar.policy.RemoteInputUriController; import com.android.systemui.statusbar.window.StatusBarWindowController; import com.android.systemui.tracing.ProtoTracer; @@ -201,7 +203,9 @@ public interface StatusBarDependenciesModule { ForegroundServiceSectionController fgsSectionController, DynamicChildBindController dynamicChildBindController, LowPriorityInflationHelper lowPriorityInflationHelper, - AssistantFeedbackController assistantFeedbackController) { + AssistantFeedbackController assistantFeedbackController, + KeyguardUpdateMonitor keyguardUpdateMonitor, + KeyguardStateController keyguardStateController) { return new NotificationViewHierarchyManager( context, mainHandler, @@ -217,7 +221,9 @@ public interface StatusBarDependenciesModule { fgsSectionController, dynamicChildBindController, lowPriorityInflationHelper, - assistantFeedbackController); + assistantFeedbackController, + keyguardUpdateMonitor, + keyguardStateController); } /** diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/KeyguardCoordinator.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/KeyguardCoordinator.java index fe1cd7b98cf9b..43710916628ac 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/KeyguardCoordinator.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/KeyguardCoordinator.java @@ -157,6 +157,8 @@ public class KeyguardCoordinator implements Coordinator { } } + // TODO(b/206118999): merge this class with SensitiveContentCoordinator which also depends on + // these same updates private void setupInvalidateNotifListCallbacks() { // register onKeyguardShowing callback mKeyguardStateController.addCallback(mKeyguardCallback); diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/SensitiveContentCoordinator.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/SensitiveContentCoordinator.kt index a115e0400de30..9c82cb64a0d76 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/SensitiveContentCoordinator.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/SensitiveContentCoordinator.kt @@ -17,7 +17,10 @@ package com.android.systemui.statusbar.notification.collection.coordinator import android.os.UserHandle +import com.android.keyguard.KeyguardUpdateMonitor +import com.android.systemui.plugins.statusbar.StatusBarStateController import com.android.systemui.statusbar.NotificationLockscreenUserManager +import com.android.systemui.statusbar.StatusBarState import com.android.systemui.statusbar.notification.DynamicPrivacyController import com.android.systemui.statusbar.notification.collection.GroupEntry import com.android.systemui.statusbar.notification.collection.ListEntry @@ -26,6 +29,7 @@ import com.android.systemui.statusbar.notification.collection.NotificationEntry import com.android.systemui.statusbar.notification.collection.coordinator.dagger.CoordinatorScope import com.android.systemui.statusbar.notification.collection.listbuilder.OnBeforeRenderListListener import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.Invalidator +import com.android.systemui.statusbar.policy.KeyguardStateController import dagger.Module import dagger.Provides @@ -36,9 +40,13 @@ object SensitiveContentCoordinatorModule { @CoordinatorScope fun provideCoordinator( dynamicPrivacyController: DynamicPrivacyController, - lockscreenUserManager: NotificationLockscreenUserManager + lockscreenUserManager: NotificationLockscreenUserManager, + keyguardUpdateMonitor: KeyguardUpdateMonitor, + statusBarStateController: StatusBarStateController, + keyguardStateController: KeyguardStateController ): SensitiveContentCoordinator = - SensitiveContentCoordinatorImpl(dynamicPrivacyController, lockscreenUserManager) + SensitiveContentCoordinatorImpl(dynamicPrivacyController, lockscreenUserManager, + keyguardUpdateMonitor, statusBarStateController, keyguardStateController) } /** Coordinates re-inflation and post-processing of sensitive notification content. */ @@ -46,7 +54,10 @@ interface SensitiveContentCoordinator : Coordinator private class SensitiveContentCoordinatorImpl( private val dynamicPrivacyController: DynamicPrivacyController, - private val lockscreenUserManager: NotificationLockscreenUserManager + private val lockscreenUserManager: NotificationLockscreenUserManager, + private val keyguardUpdateMonitor: KeyguardUpdateMonitor, + private val statusBarStateController: StatusBarStateController, + private val keyguardStateController: KeyguardStateController ) : Invalidator("SensitiveContentInvalidator"), SensitiveContentCoordinator, DynamicPrivacyController.Listener, @@ -61,6 +72,19 @@ private class SensitiveContentCoordinatorImpl( override fun onDynamicPrivacyChanged(): Unit = invalidateList() override fun onBeforeRenderList(entries: List) { + if (keyguardStateController.isKeyguardGoingAway() || + statusBarStateController.getState() == StatusBarState.KEYGUARD && + keyguardUpdateMonitor.getUserUnlockedWithBiometricAndIsBypassing( + KeyguardUpdateMonitor.getCurrentUser())) { + // don't update yet if: + // - the keyguard is currently going away + // - LS is about to be dismissed by a biometric that bypasses LS (avoid notif flash) + + // TODO(b/206118999): merge this class with KeyguardCoordinator which ensures the + // dependent state changes invalidate the pipeline + return + } + val currentUserId = lockscreenUserManager.currentUserId val devicePublic = lockscreenUserManager.isLockscreenPublicMode(currentUserId) val deviceSensitive = devicePublic && diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationViewHierarchyManagerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationViewHierarchyManagerTest.java index cf58c63e3d263..a3e0107c17169 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationViewHierarchyManagerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationViewHierarchyManagerTest.java @@ -35,6 +35,7 @@ import android.widget.LinearLayout; import androidx.test.filters.SmallTest; +import com.android.keyguard.KeyguardUpdateMonitor; import com.android.systemui.SysuiTestCase; import com.android.systemui.flags.FeatureFlags; import com.android.systemui.plugins.statusbar.NotificationSwipeActionHelper; @@ -54,6 +55,7 @@ import com.android.systemui.statusbar.notification.row.NotificationTestHelper; import com.android.systemui.statusbar.notification.stack.ForegroundServiceSectionController; import com.android.systemui.statusbar.notification.stack.NotificationListContainer; import com.android.systemui.statusbar.phone.KeyguardBypassController; +import com.android.systemui.statusbar.policy.KeyguardStateController; import com.android.wm.shell.bubbles.Bubbles; import com.google.android.collect.Lists; @@ -118,7 +120,9 @@ public class NotificationViewHierarchyManagerTest extends SysuiTestCase { mock(ForegroundServiceSectionController.class), mock(DynamicChildBindController.class), mock(LowPriorityInflationHelper.class), - mock(AssistantFeedbackController.class)); + mock(AssistantFeedbackController.class), + mock(KeyguardUpdateMonitor.class), + mock(KeyguardStateController.class)); mViewHierarchyManager.setUpWithPresenter(mPresenter, mListContainer); } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/SensitiveContentCoordinatorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/SensitiveContentCoordinatorTest.kt index 5fd4174af164e..3f84c161db200 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/SensitiveContentCoordinatorTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/SensitiveContentCoordinatorTest.kt @@ -19,8 +19,11 @@ package com.android.systemui.statusbar.notification.collection.coordinator import android.os.UserHandle import android.service.notification.StatusBarNotification import androidx.test.filters.SmallTest +import com.android.keyguard.KeyguardUpdateMonitor import com.android.systemui.SysuiTestCase +import com.android.systemui.plugins.statusbar.StatusBarStateController import com.android.systemui.statusbar.NotificationLockscreenUserManager +import com.android.systemui.statusbar.StatusBarState import com.android.systemui.statusbar.notification.DynamicPrivacyController import com.android.systemui.statusbar.notification.collection.ListEntry import com.android.systemui.statusbar.notification.collection.NotifPipeline @@ -28,9 +31,12 @@ import com.android.systemui.statusbar.notification.collection.NotificationEntry import com.android.systemui.statusbar.notification.collection.listbuilder.OnBeforeRenderListListener import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.Invalidator import com.android.systemui.statusbar.notification.collection.listbuilder.pluggable.Pluggable +import com.android.systemui.statusbar.policy.KeyguardStateController import com.android.systemui.util.mockito.withArgCaptor +import com.android.systemui.util.mockito.any import com.android.systemui.util.mockito.mock import org.junit.Test +import org.mockito.Mockito.never import org.mockito.Mockito.verify import org.mockito.Mockito.`when` as whenever @@ -40,9 +46,13 @@ class SensitiveContentCoordinatorTest : SysuiTestCase() { val dynamicPrivacyController: DynamicPrivacyController = mock() val lockscreenUserManager: NotificationLockscreenUserManager = mock() val pipeline: NotifPipeline = mock() + val keyguardUpdateMonitor: KeyguardUpdateMonitor = mock() + val statusBarStateController: StatusBarStateController = mock() + val keyguardStateController: KeyguardStateController = mock() val coordinator: SensitiveContentCoordinator = SensitiveContentCoordinatorModule - .provideCoordinator(dynamicPrivacyController, lockscreenUserManager) + .provideCoordinator(dynamicPrivacyController, lockscreenUserManager, + keyguardUpdateMonitor, statusBarStateController, keyguardStateController) @Test fun onDynamicPrivacyChanged_invokeInvalidationListener() { @@ -190,6 +200,28 @@ class SensitiveContentCoordinatorTest : SysuiTestCase() { verify(entry.representativeEntry!!).setSensitive(true, true) } + @Test + fun onBeforeRenderList_deviceDynamicallyUnlocked_deviceBiometricBypassingLockScreen() { + coordinator.attach(pipeline) + val onBeforeRenderListListener = withArgCaptor { + verify(pipeline).addOnBeforeRenderListListener(capture()) + } + + whenever(lockscreenUserManager.currentUserId).thenReturn(1) + whenever(lockscreenUserManager.isLockscreenPublicMode(1)).thenReturn(true) + whenever(lockscreenUserManager.userAllowsPrivateNotificationsInPublic(1)).thenReturn(false) + whenever(dynamicPrivacyController.isDynamicallyUnlocked).thenReturn(true) + whenever(statusBarStateController.getState()).thenReturn(StatusBarState.KEYGUARD) + whenever(keyguardUpdateMonitor.getUserUnlockedWithBiometricAndIsBypassing(any())) + .thenReturn(true) + + val entry = fakeNotification(2, true) + + onBeforeRenderListListener.onBeforeRenderList(listOf(entry)) + + verify(entry.representativeEntry!!, never()).setSensitive(any(), any()) + } + private fun fakeNotification(notifUserId: Int, needsRedaction: Boolean): ListEntry { val mockUserHandle = mock().apply { whenever(identifier).thenReturn(notifUserId)