From 670240dc87f914e73f513e579a54f869e380171c Mon Sep 17 00:00:00 2001 From: Beverly Date: Fri, 28 Jan 2022 22:06:14 +0000 Subject: [PATCH] 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 Bug: 212835832 Change-Id: I5ce4460043250aad4a5b0b0838e6f3b5ae6feb1d --- .../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 f2d0427c39d3e..cc10b02a8bb6d 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitor.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitor.java @@ -1196,6 +1196,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 abfdfaf2115f1..391525e118662 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java @@ -27,6 +27,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; @@ -47,6 +48,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; @@ -98,6 +100,8 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle private final ForegroundServiceSectionController mFgsSectionController; private final NotifPipelineFlags mNotifPipelineFlags; private AssistantFeedbackController mAssistantFeedbackController; + private final KeyguardStateController mKeyguardStateController; + private final KeyguardUpdateMonitor mKeyguardUpdateMonitor; private final Context mContext; private NotificationPresenter mPresenter; @@ -129,7 +133,9 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle DynamicChildBindController dynamicChildBindController, LowPriorityInflationHelper lowPriorityInflationHelper, AssistantFeedbackController assistantFeedbackController, - NotifPipelineFlags notifPipelineFlags) { + NotifPipelineFlags notifPipelineFlags, + KeyguardUpdateMonitor keyguardUpdateMonitor, + KeyguardStateController keyguardStateController) { mContext = context; mHandler = mainHandler; mFeatureFlags = featureFlags; @@ -149,6 +155,8 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle mDynamicChildBindController = dynamicChildBindController; mLowPriorityInflationHelper = lowPriorityInflationHelper; mAssistantFeedbackController = assistantFeedbackController; + mKeyguardUpdateMonitor = keyguardUpdateMonitor; + mKeyguardStateController = keyguardStateController; } public void setUpWithPresenter(NotificationPresenter presenter, @@ -174,6 +182,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(); @@ -192,7 +205,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 d574cdabced62..e3d0d9802b8d0 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; @@ -72,6 +73,7 @@ import com.android.systemui.statusbar.phone.StatusBarRemoteInputCallback; import com.android.systemui.statusbar.phone.ongoingcall.OngoingCallController; import com.android.systemui.statusbar.phone.ongoingcall.OngoingCallFlags; 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; @@ -214,7 +216,9 @@ public interface StatusBarDependenciesModule { DynamicChildBindController dynamicChildBindController, LowPriorityInflationHelper lowPriorityInflationHelper, AssistantFeedbackController assistantFeedbackController, - NotifPipelineFlags notifPipelineFlags) { + NotifPipelineFlags notifPipelineFlags, + KeyguardUpdateMonitor keyguardUpdateMonitor, + KeyguardStateController keyguardStateController) { return new NotificationViewHierarchyManager( context, mainHandler, @@ -231,7 +235,9 @@ public interface StatusBarDependenciesModule { dynamicChildBindController, lowPriorityInflationHelper, assistantFeedbackController, - notifPipelineFlags); + notifPipelineFlags, + 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 0ce07cb99d52f..bc4ed6653d902 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 @@ -164,6 +164,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 83f1d87133c6c..7fafb24317b7d 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; @@ -56,6 +57,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; @@ -123,7 +125,9 @@ public class NotificationViewHierarchyManagerTest extends SysuiTestCase { mock(DynamicChildBindController.class), mock(LowPriorityInflationHelper.class), mock(AssistantFeedbackController.class), - mNotifPipelineFlags); + mNotifPipelineFlags, + mock(KeyguardUpdateMonitor.class), + mock(KeyguardStateController.class)); mViewHierarchyManager.setUpWithPresenter(mPresenter, mStackController, 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)