diff options
16 files changed, 269 insertions, 128 deletions
diff --git a/Android.bp b/Android.bp index 4a436da6c..bf7437814 100644 --- a/Android.bp +++ b/Android.bp @@ -30,7 +30,6 @@ apex_defaults { systemserverclasspath_fragments: ["com.android.permission-systemserverclasspath-fragment"], prebuilts: [ "current_sdkinfo", - "privapp_allowlist_com.android.permissioncontroller.xml", ], key: "com.android.permission.key", certificate: ":com.android.permission.certificate", diff --git a/PermissionController/Android.bp b/PermissionController/Android.bp index 9b20819f0..892f12f2b 100644 --- a/PermissionController/Android.bp +++ b/PermissionController/Android.bp @@ -82,7 +82,7 @@ android_app { privileged: true, certificate: "platform", rename_resources_package: false, - required: ["privapp_allowlist_com.android.permissioncontroller.xml"], + privapp_allowlist: ":privapp_allowlist_com.android.permissioncontroller.xml", srcs: [":permissioncontroller-sources"], diff --git a/PermissionController/src/com/android/permissioncontroller/permission/service/RuntimePermissionsUpgradeController.kt b/PermissionController/src/com/android/permissioncontroller/permission/service/RuntimePermissionsUpgradeController.kt index 43b79a7f8..97f817cff 100644 --- a/PermissionController/src/com/android/permissioncontroller/permission/service/RuntimePermissionsUpgradeController.kt +++ b/PermissionController/src/com/android/permissioncontroller/permission/service/RuntimePermissionsUpgradeController.kt @@ -42,7 +42,6 @@ import com.android.permissioncontroller.permission.model.livedatatypes.LightPerm import com.android.permissioncontroller.permission.utils.IPC import com.android.permissioncontroller.permission.utils.KotlinUtils.grantBackgroundRuntimePermissions import com.android.permissioncontroller.permission.utils.KotlinUtils.grantForegroundRuntimePermissions -import com.android.permissioncontroller.permission.utils.PermissionMapping import com.android.permissioncontroller.permission.utils.PermissionMapping.getPlatformPermissionNamesOfGroup import com.android.permissioncontroller.permission.utils.PermissionMapping.getRuntimePlatformPermissionNames import com.android.permissioncontroller.permission.utils.application @@ -56,9 +55,7 @@ internal object RuntimePermissionsUpgradeController { private val LOG_TAG = RuntimePermissionsUpgradeController::class.java.simpleName // The latest version of the runtime permissions database - private val LATEST_VERSION = if (SdkLevel.isAtLeastU()) { - 11 - } else if (SdkLevel.isAtLeastT()) { + private val LATEST_VERSION = if (SdkLevel.isAtLeastT()) { 10 } else { 9 @@ -137,7 +134,6 @@ internal object RuntimePermissionsUpgradeController { val needBackgroundAppPermGroups = sdkUpgradedFromP && currentVersion <= 6 val needAccessMediaAppPermGroups = !isNewUser && currentVersion <= 7 val needGrantedExternalStorage = currentVersion <= 9 && SdkLevel.isAtLeastT() - val needGrantedReadMediaVisual = currentVersion <= 10 && SdkLevel.isAtLeastU() val isDeviceUpgrading = context.packageManager.isDeviceUpgrading // All data needed by this method. @@ -160,17 +156,24 @@ internal object RuntimePermissionsUpgradeController { private val pkgInfoProvider = UserPackageInfosLiveData[myUserHandle()] /** Provides all {@link LightAppPermGroup} this upgrade needs */ - private var permGroupProviders: MutableSet<LightAppPermGroupLiveData>? = null + private var permGroupProviders: MutableList<LightAppPermGroupLiveData>? = null /** {@link #permGroupProviders} that already provided a result */ private val permGroupProvidersDone = mutableSetOf<LightAppPermGroupLiveData>() init { // First step: Load packages + perm infos + // TODO ntmyren: remove once b/154796729 is fixed + Log.i("RuntimePermissions", "observing UserPackageInfoLiveData for " + + "${myUserHandle().identifier} in RuntimePermissionsUpgradeController") addSource(pkgInfoProvider) { pkgInfos -> if (pkgInfos != null) { removeSource(pkgInfoProvider) + // TODO ntmyren: remove once b/154796729 is fixed + Log.i("RuntimePermissions", "observing " + + "PreinstalledUserPackageInfoLiveData for ${myUserHandle().identifier}" + + " in RuntimePermissionsUpgradeController") addSource(preinstalledPkgInfoProvider) { preinstalledPkgInfos -> if (preinstalledPkgInfos != null) { removeSource(preinstalledPkgInfoProvider) @@ -200,16 +203,15 @@ internal object RuntimePermissionsUpgradeController { if (permGroupProviders == null && pkgInfoProvider.value != null) { // Second step: Trigger load of app-perm-groups - permGroupProviders = mutableSetOf() + permGroupProviders = mutableListOf() // Only load app-perm-groups needed for this upgrade if (needBackgroundAppPermGroups || needAccessMediaAppPermGroups || - needGrantedExternalStorage || needGrantedReadMediaVisual) { + needGrantedExternalStorage) { for ((pkgName, _, requestedPerms, requestedPermFlags) in pkgInfoProvider.value!!) { var requestsAccessMediaLocation = false var hasGrantedExternalStorage = false - var hasGrantedReadMediaVisual = false for ((perm, flags) in requestedPerms.zip(requestedPermFlags)) { if (needBackgroundAppPermGroups && @@ -218,22 +220,17 @@ internal object RuntimePermissionsUpgradeController { permission_group.LOCATION, myUserHandle()]) } - if (needAccessMediaAppPermGroups || needGrantedExternalStorage || - needGrantedReadMediaVisual) { + if (needAccessMediaAppPermGroups || needGrantedExternalStorage) { if (needAccessMediaAppPermGroups && perm == permission.ACCESS_MEDIA_LOCATION) { requestsAccessMediaLocation = true } - val isGranted = - flags and PackageInfo.REQUESTED_PERMISSION_GRANTED != 0 - if (perm == permission.READ_EXTERNAL_STORAGE && isGranted) { + if (perm == permission.READ_EXTERNAL_STORAGE && + flags and PackageInfo.REQUESTED_PERMISSION_GRANTED + != 0) { hasGrantedExternalStorage = true } - if (PermissionMapping.getGroupOfPlatformPermission(perm) - == permission_group.READ_MEDIA_VISUAL && isGranted) { - hasGrantedReadMediaVisual = true - } } } @@ -258,10 +255,6 @@ internal object RuntimePermissionsUpgradeController { accessMediaLocationPermGroup, myUserHandle()]) } } - if (hasGrantedReadMediaVisual && needGrantedReadMediaVisual) { - permGroupProviders!!.add(LightAppPermGroupLiveData[pkgName, - permission_group.READ_MEDIA_VISUAL, myUserHandle()]) - } } } @@ -543,24 +536,6 @@ internal object RuntimePermissionsUpgradeController { currentVersion = 10 } - if (currentVersion == 10 && SdkLevel.isAtLeastU()) { - // On U, if the app is granted READ_MEDIA_VISUAL, expand the grant to - // READ_MEDIA_VISUAL_USER_SELECTED - if (isDeviceUpgrading && !isNewUser) { - Log.i(LOG_TAG, "Grandfathering READ_MEDIA_VISUAL_USER_SELECTED to apps already " + - "granted visual permissions") - val visualAppPermGroups = storageAndMediaAppPermGroups.filter { - it.packageInfo.targetSdkVersion >= Build.VERSION_CODES.TIRAMISU && - it.permGroupInfo.name == permission_group.READ_MEDIA_VISUAL && - it.isGranted && it.isUserSet - } - visualAppPermGroups.forEach { - grants.add(Grant(false, it)) - } - } - currentVersion = 11 - } - // XXX: Add new upgrade steps above this point. return Triple(currentVersion, exemptions, grants) @@ -616,7 +591,7 @@ internal object RuntimePermissionsUpgradeController { private val isBackground: Boolean, /** Group to be granted */ private val group: LightAppPermGroup, - /** Which of the permissions in the group should be granted */ + /** Which of th permissions in the group should be granted */ private val permissions: List<String> = group.permissions.keys.toList() ) { /** diff --git a/PermissionController/tests/mocking/src/com/android/permissioncontroller/tests/mocking/permission/service/RuntimePermissionsUpgradeControllerTest.kt b/PermissionController/tests/mocking/src/com/android/permissioncontroller/tests/mocking/permission/service/RuntimePermissionsUpgradeControllerTest.kt index 86aab9b60..d576f2924 100644 --- a/PermissionController/tests/mocking/src/com/android/permissioncontroller/tests/mocking/permission/service/RuntimePermissionsUpgradeControllerTest.kt +++ b/PermissionController/tests/mocking/src/com/android/permissioncontroller/tests/mocking/permission/service/RuntimePermissionsUpgradeControllerTest.kt @@ -24,7 +24,6 @@ import android.Manifest.permission.READ_EXTERNAL_STORAGE import android.Manifest.permission.READ_MEDIA_AUDIO import android.Manifest.permission.READ_MEDIA_IMAGES import android.Manifest.permission.READ_MEDIA_VIDEO -import android.Manifest.permission.READ_MEDIA_VISUAL_USER_SELECTED import android.Manifest.permission.SEND_SMS import android.Manifest.permission.WRITE_EXTERNAL_STORAGE import android.app.ActivityManager @@ -548,63 +547,6 @@ class RuntimePermissionsUpgradeControllerTest { verifyGranted(TEST_PKG_NAME, READ_MEDIA_IMAGES) } - @Test - fun userSelectedGrantedIfReadMediaVisualGrantedWhenVersionIs10() { - Assume.assumeTrue(SdkLevel.isAtLeastU()) - whenever(packageManager.isDeviceUpgrading).thenReturn(true) - setInitialDatabaseVersion(10) - setPackages( - Package(TEST_PKG_NAME, - Permission(READ_MEDIA_VIDEO, isGranted = true, flags = FLAG_PERMISSION_USER_SET), - Permission(READ_MEDIA_IMAGES, isGranted = true, flags = FLAG_PERMISSION_USER_SET), - Permission(READ_MEDIA_VISUAL_USER_SELECTED, isGranted = false), - targetSdkVersion = 33 - ) - ) - - upgradeIfNeeded() - - verifyGranted(TEST_PKG_NAME, READ_MEDIA_VISUAL_USER_SELECTED) - } - - @Test - fun userSelectedNotGrantedIfDeviceNotUpgradingWhenVersionIs10() { - Assume.assumeTrue(SdkLevel.isAtLeastU()) - whenever(packageManager.isDeviceUpgrading).thenReturn(false) - setInitialDatabaseVersion(10) - setPackages( - Package(TEST_PKG_NAME, - Permission(READ_MEDIA_VIDEO, isGranted = true, flags = FLAG_PERMISSION_USER_SET), - Permission(READ_MEDIA_IMAGES, isGranted = true, flags = FLAG_PERMISSION_USER_SET), - Permission(READ_MEDIA_VISUAL_USER_SELECTED, isGranted = false), - targetSdkVersion = 33 - ) - ) - - upgradeIfNeeded() - - verifyNotGranted(TEST_PKG_NAME, READ_MEDIA_VISUAL_USER_SELECTED) - } - - @Test - fun userSelectedNotGrantedIfReadMediaVisualNotGrantedWhenVersionIs10() { - Assume.assumeTrue(SdkLevel.isAtLeastU()) - whenever(packageManager.isDeviceUpgrading).thenReturn(false) - setInitialDatabaseVersion(10) - setPackages( - Package(TEST_PKG_NAME, - Permission(READ_MEDIA_VIDEO, isGranted = false, flags = FLAG_PERMISSION_USER_SET), - Permission(READ_MEDIA_IMAGES, isGranted = false, flags = FLAG_PERMISSION_USER_SET), - Permission(READ_MEDIA_VISUAL_USER_SELECTED, isGranted = false), - targetSdkVersion = 33 - ) - ) - - upgradeIfNeeded() - - verifyNotGranted(TEST_PKG_NAME, READ_MEDIA_VISUAL_USER_SELECTED) - } - @After fun resetSystem() { // Send low memory notifications for all data repositories which will clear cached data diff --git a/permissions/Android.bp b/permissions/Android.bp index 8c7dab40b..6c1fdb8f8 100644 --- a/permissions/Android.bp +++ b/permissions/Android.bp @@ -19,9 +19,7 @@ package { default_visibility: ["//packages/modules/Permission:__subpackages__"], } -prebuilt_etc { +filegroup { name: "privapp_allowlist_com.android.permissioncontroller.xml", - sub_dir: "permissions", - src: "com.android.permissioncontroller.xml", - installable: false, + srcs: ["com.android.permissioncontroller.xml"], } diff --git a/service/java/com/android/safetycenter/SafetyCenterService.java b/service/java/com/android/safetycenter/SafetyCenterService.java index 3400dad2f..1a9f60781 100644 --- a/service/java/com/android/safetycenter/SafetyCenterService.java +++ b/service/java/com/android/safetycenter/SafetyCenterService.java @@ -75,7 +75,9 @@ import com.android.internal.annotations.GuardedBy; import com.android.modules.utils.BackgroundThread; import com.android.permission.util.ForegroundThread; import com.android.permission.util.UserUtils; +import com.android.safetycenter.data.AndroidLockScreenFix; import com.android.safetycenter.data.SafetyCenterDataManager; +import com.android.safetycenter.data.SafetyEventFix; import com.android.safetycenter.internaldata.SafetyCenterIds; import com.android.safetycenter.internaldata.SafetyCenterIssueActionId; import com.android.safetycenter.internaldata.SafetyCenterIssueId; @@ -278,6 +280,16 @@ public final class SafetyCenterService extends SystemService { UserProfileGroup userProfileGroup = UserProfileGroup.fromUser(getContext(), userId); synchronized (mApiLock) { + safetySourceData = + AndroidLockScreenFix.maybeOverrideSafetySourceData( + getContext(), safetySourceId, safetySourceData); + safetyEvent = + SafetyEventFix.maybeOverrideSafetyEvent( + mSafetyCenterDataManager, + safetySourceId, + safetySourceData, + safetyEvent, + userId); boolean hasUpdate = mSafetyCenterDataManager.setSafetySourceData( safetySourceData, safetySourceId, safetyEvent, packageName, userId); diff --git a/service/java/com/android/safetycenter/data/AndroidLockScreenFix.java b/service/java/com/android/safetycenter/data/AndroidLockScreenFix.java index a52ad0aca..22af4ed4d 100644 --- a/service/java/com/android/safetycenter/data/AndroidLockScreenFix.java +++ b/service/java/com/android/safetycenter/data/AndroidLockScreenFix.java @@ -41,9 +41,11 @@ import java.util.List; /** * A class to work around an issue with the {@code AndroidLockScreen} safety source, by potentially * overriding its {@link SafetySourceData}. + * + * @hide */ @RequiresApi(TIRAMISU) -final class AndroidLockScreenFix { +public final class AndroidLockScreenFix { private static final String TAG = "AndroidLockScreenFix"; @@ -73,7 +75,7 @@ final class AndroidLockScreenFix { * created (the key does take into account the request code). */ @Nullable - static SafetySourceData maybeOverrideSafetySourceData( + public static SafetySourceData maybeOverrideSafetySourceData( Context context, String sourceId, @Nullable SafetySourceData safetySourceData) { if (safetySourceData == null) { return null; diff --git a/service/java/com/android/safetycenter/data/SafetyCenterDataManager.java b/service/java/com/android/safetycenter/data/SafetyCenterDataManager.java index 917af9b91..9fd593ed9 100644 --- a/service/java/com/android/safetycenter/data/SafetyCenterDataManager.java +++ b/service/java/com/android/safetycenter/data/SafetyCenterDataManager.java @@ -29,6 +29,7 @@ import android.safetycenter.SafetySourceErrorDetails; import android.safetycenter.SafetySourceIssue; import android.safetycenter.config.SafetyCenterConfig; import android.safetycenter.config.SafetySourcesGroup; +import android.util.ArraySet; import android.util.Log; import androidx.annotation.Nullable; @@ -404,6 +405,11 @@ public final class SafetyCenterDataManager { safetyCenterIssueActionId); } + /** Returns a list of IDs of in-flight actions for the given source and user */ + ArraySet<SafetyCenterIssueActionId> getInFlightActions(String sourceId, @UserIdInt int userId) { + return mSafetyCenterInFlightIssueActionRepository.getInFlightActions(sourceId, userId); + } + /////////////////////////////////////////////////////////////////////////////////////////////// ////////////////////// SafetySourceDataRepository /////////////////////////////////////////// /////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/service/java/com/android/safetycenter/data/SafetyCenterInFlightIssueActionRepository.java b/service/java/com/android/safetycenter/data/SafetyCenterInFlightIssueActionRepository.java index b4db99069..fd8a12fc4 100644 --- a/service/java/com/android/safetycenter/data/SafetyCenterInFlightIssueActionRepository.java +++ b/service/java/com/android/safetycenter/data/SafetyCenterInFlightIssueActionRepository.java @@ -25,6 +25,7 @@ import android.content.Context; import android.os.SystemClock; import android.safetycenter.SafetySourceIssue; import android.util.ArrayMap; +import android.util.ArraySet; import android.util.Log; import androidx.annotation.Nullable; @@ -113,6 +114,19 @@ final class SafetyCenterInFlightIssueActionRepository { return mSafetyCenterIssueActionsInFlight.containsKey(safetyCenterIssueActionId); } + /** Returns a list of IDs of in-flight actions for the given source and user */ + ArraySet<SafetyCenterIssueActionId> getInFlightActions(String sourceId, @UserIdInt int userId) { + ArraySet<SafetyCenterIssueActionId> result = new ArraySet<>(); + for (int i = 0; i < mSafetyCenterIssueActionsInFlight.size(); i++) { + SafetyCenterIssueActionId actionId = mSafetyCenterIssueActionsInFlight.keyAt(i); + SafetyCenterIssueKey issueKey = actionId.getSafetyCenterIssueKey(); + if (sourceId.equals(issueKey.getSafetySourceId()) && issueKey.getUserId() == userId) { + result.add(actionId); + } + } + return result; + } + /** * Returns {@link SafetySourceIssue.Action} identified by the given {@link * SafetyCenterIssueActionId} and {@link SafetySourceIssue}. diff --git a/service/java/com/android/safetycenter/data/SafetyCenterIssueRepository.java b/service/java/com/android/safetycenter/data/SafetyCenterIssueRepository.java index 566c28c1e..ac2fc96eb 100644 --- a/service/java/com/android/safetycenter/data/SafetyCenterIssueRepository.java +++ b/service/java/com/android/safetycenter/data/SafetyCenterIssueRepository.java @@ -92,19 +92,6 @@ final class SafetyCenterIssueRepository { * Updates the class as per the current state of issues. Should be called after any state update * that can affect issues. */ - void updateIssues(UserProfileGroup userProfileGroup) { - updateIssues(userProfileGroup.getProfileParentUserId(), /* isManagedProfile= */ false); - - int[] managedProfileUserIds = userProfileGroup.getManagedProfilesUserIds(); - for (int i = 0; i < managedProfileUserIds.length; i++) { - updateIssues(managedProfileUserIds[i], /* isManagedProfile= */ true); - } - } - - /** - * Updates the class as per the current state of issues. Should be called after any state update - * that can affect issues. - */ void updateIssues(@UserIdInt int userId) { updateIssues(userId, UserUtils.isManagedProfile(userId, mContext)); } diff --git a/service/java/com/android/safetycenter/data/SafetyEventFix.java b/service/java/com/android/safetycenter/data/SafetyEventFix.java new file mode 100644 index 000000000..658b5f078 --- /dev/null +++ b/service/java/com/android/safetycenter/data/SafetyEventFix.java @@ -0,0 +1,121 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.safetycenter.data; + +import static android.os.Build.VERSION_CODES.TIRAMISU; +import static android.safetycenter.SafetyEvent.SAFETY_EVENT_TYPE_RESOLVING_ACTION_SUCCEEDED; +import static android.safetycenter.SafetyEvent.SAFETY_EVENT_TYPE_SOURCE_STATE_CHANGED; + +import android.annotation.UserIdInt; +import android.safetycenter.SafetyEvent; +import android.safetycenter.SafetySourceData; +import android.safetycenter.SafetySourceIssue; +import android.util.ArraySet; +import android.util.Log; + +import androidx.annotation.Nullable; +import androidx.annotation.RequiresApi; + +import com.android.safetycenter.internaldata.SafetyCenterIssueActionId; + +import java.util.List; + +/** + * Works around sources sending unexpected {@link SafetyEvent}s by optionally replacing them using + * heuristics based on the incoming {@link SafetySourceData} and Safety Center's current state. + * + * @hide + */ +@RequiresApi(TIRAMISU) +public final class SafetyEventFix { + + private static final String TAG = "SafetyEventFix"; + + private SafetyEventFix() {} + + /** + * Optionally returns a new {@link SafetyEvent} if heuristics indicate that the one provided by + * the source is inappropriate, otherwise returns the source-provided event unchanged. + * + * <p>If the incoming event has type {@link SafetyEvent#SAFETY_EVENT_TYPE_SOURCE_STATE_CHANGED} + * but the {@link SafetySourceData} no longer includes an issue, for which Safety Center has a + * record of an in-flight, resolving action, then the event will be exchanged for a new one of + * type {@link SafetyEvent#SAFETY_EVENT_TYPE_RESOLVING_ACTION_SUCCEEDED}. + */ + public static SafetyEvent maybeOverrideSafetyEvent( + SafetyCenterDataManager dataManager, + String safetySourceId, + @Nullable SafetySourceData safetySourceData, + SafetyEvent safetyEvent, + @UserIdInt int userId) { + if (safetySourceData == null + || safetyEvent.getType() != SAFETY_EVENT_TYPE_SOURCE_STATE_CHANGED) { + return safetyEvent; + } + + ArraySet<SafetyCenterIssueActionId> possiblySuccessfulActions = + dataManager.getInFlightActions(safetySourceId, userId); + + if (possiblySuccessfulActions.isEmpty()) { + return safetyEvent; + } + + // Discard any actions for which the issue is still present in the latest source data, they + // cannot have been resolved successfully! + ArraySet<String> presentSourceIssueIds = getSourceIssueIds(safetySourceData); + for (int i = possiblySuccessfulActions.size() - 1; i >= 0; i--) { + String sourceIssueId = + possiblySuccessfulActions + .valueAt(i) + .getSafetyCenterIssueKey() + .getSafetySourceIssueId(); + if (presentSourceIssueIds.contains(sourceIssueId)) { + possiblySuccessfulActions.removeAt(i); + } + } + + if (possiblySuccessfulActions.isEmpty()) { + return safetyEvent; + } + + if (possiblySuccessfulActions.size() > 1) { + Log.i(TAG, "Multiple actions resolved, not overriding " + safetyEvent); + return safetyEvent; + } + + SafetyCenterIssueActionId successfulAction = possiblySuccessfulActions.valueAt(0); + SafetyEvent replacement = newActionSucceededEvent(successfulAction); + Log.i(TAG, "Replacing incoming " + safetyEvent + " with " + replacement); + return replacement; + } + + private static ArraySet<String> getSourceIssueIds(SafetySourceData safetySourceData) { + List<SafetySourceIssue> issues = safetySourceData.getIssues(); + ArraySet<String> issueIds = new ArraySet<>(issues.size()); + for (int i = 0; i < issues.size(); i++) { + issueIds.add(issues.get(i).getId()); + } + return issueIds; + } + + private static SafetyEvent newActionSucceededEvent(SafetyCenterIssueActionId actionId) { + return new SafetyEvent.Builder(SAFETY_EVENT_TYPE_RESOLVING_ACTION_SUCCEEDED) + .setSafetySourceIssueId(actionId.getSafetyCenterIssueKey().getSafetySourceIssueId()) + .setSafetySourceIssueActionId(actionId.getSafetySourceIssueActionId()) + .build(); + } +} diff --git a/service/java/com/android/safetycenter/data/SafetySourceDataRepository.java b/service/java/com/android/safetycenter/data/SafetySourceDataRepository.java index 8e3364c33..d437245b4 100644 --- a/service/java/com/android/safetycenter/data/SafetySourceDataRepository.java +++ b/service/java/com/android/safetycenter/data/SafetySourceDataRepository.java @@ -102,10 +102,6 @@ final class SafetySourceDataRepository { String safetySourceId, @UserIdInt int userId) { SafetySourceKey key = SafetySourceKey.of(safetySourceId, userId); - safetySourceData = - AndroidLockScreenFix.maybeOverrideSafetySourceData( - mContext, safetySourceId, safetySourceData); - boolean sourceDataDiffers = !Objects.equals(safetySourceData, mSafetySourceData.get(key)); boolean removedSourceError = mSafetySourceErrors.remove(key); diff --git a/service/java/com/android/safetycenter/notifications/SafetyCenterNotificationReceiver.java b/service/java/com/android/safetycenter/notifications/SafetyCenterNotificationReceiver.java index 41c54a9fd..bb4977bb5 100644 --- a/service/java/com/android/safetycenter/notifications/SafetyCenterNotificationReceiver.java +++ b/service/java/com/android/safetycenter/notifications/SafetyCenterNotificationReceiver.java @@ -173,6 +173,7 @@ public final class SafetyCenterNotificationReceiver extends BroadcastReceiver { } if (!SafetyCenterFlags.getNotificationsEnabled()) { + // TODO(b/284271124): Decide what to do with existing notifications Log.i(TAG, "Received notification broadcast but notifications are disabled"); return; } diff --git a/service/java/com/android/safetycenter/notifications/SafetyCenterNotificationSender.java b/service/java/com/android/safetycenter/notifications/SafetyCenterNotificationSender.java index 77dd80c47..90be3618e 100644 --- a/service/java/com/android/safetycenter/notifications/SafetyCenterNotificationSender.java +++ b/service/java/com/android/safetycenter/notifications/SafetyCenterNotificationSender.java @@ -145,6 +145,11 @@ public final class SafetyCenterNotificationSender { */ public void notifyActionSuccess( String sourceId, SafetyEvent safetyEvent, @UserIdInt int userId) { + if (!SafetyCenterFlags.getNotificationsEnabled()) { + // TODO(b/284271124): Decide what to do with existing notifications + return; + } + if (safetyEvent.getType() != SAFETY_EVENT_TYPE_RESOLVING_ACTION_SUCCEEDED) { Log.w(TAG, "Received safety event of wrong type"); return; @@ -224,6 +229,7 @@ public final class SafetyCenterNotificationSender { */ public void updateNotifications(@UserIdInt int userId) { if (!SafetyCenterFlags.getNotificationsEnabled()) { + // TODO(b/284271124): Decide what to do with existing notifications return; } diff --git a/tests/functional/safetycenter/singleuser/src/android/safetycenter/functional/SafetyCenterNotificationTest.kt b/tests/functional/safetycenter/singleuser/src/android/safetycenter/functional/SafetyCenterNotificationTest.kt index eb51df697..5d73d53e3 100644 --- a/tests/functional/safetycenter/singleuser/src/android/safetycenter/functional/SafetyCenterNotificationTest.kt +++ b/tests/functional/safetycenter/singleuser/src/android/safetycenter/functional/SafetyCenterNotificationTest.kt @@ -32,6 +32,7 @@ import androidx.test.core.app.ApplicationProvider.getApplicationContext import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.filters.SdkSuppress import com.android.safetycenter.pendingintents.PendingIntentSender +import com.android.safetycenter.testing.Coroutines import com.android.safetycenter.testing.Coroutines.TIMEOUT_SHORT import com.android.safetycenter.testing.NotificationCharacteristics import com.android.safetycenter.testing.SafetyCenterActivityLauncher.executeBlockAndExit @@ -528,6 +529,22 @@ class SafetyCenterNotificationTest { TestNotificationListener.waitForZeroNotifications() } + // TODO(b/284271124): Decide what to do with existing notifications when flag flipped off + @Test + fun setSafetySourceData_removingAnIssue_afterFlagTurnedOff_noNotificationChanges() { + val data1 = safetySourceTestData.recommendationWithAccountIssue + val data2 = safetySourceTestData.information + + safetyCenterTestHelper.setData(SINGLE_SOURCE_ID, data1) + + TestNotificationListener.waitForSingleNotification() + + SafetyCenterFlags.notificationsEnabled = false + safetyCenterTestHelper.setData(SINGLE_SOURCE_ID, data2) + + TestNotificationListener.waitForZeroNotificationEvents() + } + @Test fun reportSafetySourceError_sourceWithNotification_cancelsNotification() { val data = safetySourceTestData.recommendationWithAccountIssue @@ -896,6 +913,58 @@ class SafetyCenterNotificationTest { ) } + // TODO(b/284271124): Decide what to do with existing notifications when flag flipped off + @Test + fun sendActionPendingIntent_flagDisabled_pendingIntentNotSentToSource() { + safetyCenterTestHelper.setData( + SINGLE_SOURCE_ID, + safetySourceTestData.criticalWithResolvingIssueWithSuccessMessage + ) + val notificationWithChannel = TestNotificationListener.waitForSingleNotification() + val action = + notificationWithChannel.statusBarNotification.notification.actions.firstOrNull() + checkNotNull(action) { "Notification action unexpectedly null" } + SafetySourceReceiver.setResponse( + Request.ResolveAction(SINGLE_SOURCE_ID), + Response.SetData(safetySourceTestData.information) + ) + SafetyCenterFlags.notificationsEnabled = false + + assertFailsWith(TimeoutCancellationException::class) { + sendActionPendingIntentAndWaitWithPermission(action, timeout = TIMEOUT_SHORT) + } + } + + @Test + fun sendActionPendingIntent_sourceStateChangedSafetyEvent_successNotification() { + safetyCenterTestHelper.setData( + SINGLE_SOURCE_ID, + safetySourceTestData.criticalWithResolvingIssueWithSuccessMessage + ) + val notificationWithChannel = TestNotificationListener.waitForSingleNotification() + val action = + notificationWithChannel.statusBarNotification.notification.actions.firstOrNull() + checkNotNull(action) { "Notification action unexpectedly null" } + SafetySourceReceiver.setResponse( + Request.ResolveAction(SINGLE_SOURCE_ID), + Response.SetData( + safetySourceTestData.information, + overrideSafetyEvent = + SafetyEvent.Builder(SafetyEvent.SAFETY_EVENT_TYPE_SOURCE_STATE_CHANGED).build() + ) + ) + + sendActionPendingIntentAndWaitWithPermission(action) + + TestNotificationListener.waitForSingleNotificationMatching( + NotificationCharacteristics( + "Issue solved", + "", + actions = emptyList(), + ) + ) + } + @Test fun sendActionPendingIntent_error_updatesListenerDoesNotRemoveNotification() { // Here we cause a notification with an action to be posted and prepare the fake receiver @@ -966,12 +1035,15 @@ class SafetyCenterNotificationTest { private val SafetyCenterData.inFlightActions: List<SafetyCenterIssue.Action> get() = issues.flatMap { it.actions }.filter { it.isInFlight } - private fun sendActionPendingIntentAndWaitWithPermission(action: Notification.Action) { + private fun sendActionPendingIntentAndWaitWithPermission( + action: Notification.Action, + timeout: Duration = Coroutines.TIMEOUT_LONG + ) { callWithShellPermissionIdentity(SEND_SAFETY_CENTER_UPDATE) { PendingIntentSender.send(action.actionIntent) // Sending the action's PendingIntent above is asynchronous and we need to wait for // it to be received by the fake receiver below. - SafetySourceReceiver.receiveResolveAction() + SafetySourceReceiver.receiveResolveAction(timeout) } } diff --git a/tests/utils/safetycenter/java/com/android/safetycenter/testing/SafetySourceIntentHandler.kt b/tests/utils/safetycenter/java/com/android/safetycenter/testing/SafetySourceIntentHandler.kt index 2bd662ee8..8386228b8 100644 --- a/tests/utils/safetycenter/java/com/android/safetycenter/testing/SafetySourceIntentHandler.kt +++ b/tests/utils/safetycenter/java/com/android/safetycenter/testing/SafetySourceIntentHandler.kt @@ -221,7 +221,7 @@ class SafetySourceIntentHandler { safetyEventForResponse: (Response) -> SafetyEvent ) { val response = mutex.withLock { requestsToResponses[request] } ?: return - val safetyEvent = safetyEventForResponse(response) + val safetyEvent = response.overrideSafetyEvent ?: safetyEventForResponse(response) when (response) { is Response.Error -> reportSafetySourceError(request.sourceId, SafetySourceErrorDetails(safetyEvent)) @@ -270,6 +270,13 @@ class SafetySourceIntentHandler { */ sealed interface Response { + /** + * If non-null, the [SafetyEvent] to use when calling any applicable [SafetyCenterManager] + * methods. + */ + val overrideSafetyEvent: SafetyEvent? + get() = null + /** Creates an error [Response]. */ object Error : Response @@ -282,10 +289,13 @@ class SafetySourceIntentHandler { * @param overrideBroadcastId an optional override of the broadcast id to use in the * [SafetyEvent] sent to the [SafetyCenterManager], in case of [Request.Refresh] or * [Request.Rescan]. This is used to simulate a misuse of the [SafetyCenterManager] APIs + * @param overrideSafetyEvent like [overrideBroadcastId] but allows the whole [SafetyEvent] + * to be override to send different types of [SafetyEvent]. */ data class SetData( val safetySourceData: SafetySourceData, - val overrideBroadcastId: String? = null + val overrideBroadcastId: String? = null, + override val overrideSafetyEvent: SafetyEvent? = null ) : Response } |