From 4239357fe163c680e945148cc486e8167e4bd420 Mon Sep 17 00:00:00 2001 From: Felipe Leme Date: Thu, 18 Feb 2021 18:05:57 -0800 Subject: [PATCH] Fixed OneTimeSafetyChecker callback. It was inverting the "isSafe" value. Test: atest DeviceOwnerTest#testDevicePolicySafetyCheckerIntegration_onOperationSafetyStateChanged Bug: 173541467 Bug: 178494483 Change-Id: Ia2dd44e1ae413bc3358c419d2f59c82152aada57 --- core/java/android/app/admin/DeviceAdminReceiver.java | 2 +- .../server/devicepolicy/DevicePolicyManagerService.java | 2 -- .../android/server/devicepolicy/OneTimeSafetyChecker.java | 8 ++++---- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/core/java/android/app/admin/DeviceAdminReceiver.java b/core/java/android/app/admin/DeviceAdminReceiver.java index 4dbff0c064237..721a444f0f15b 100644 --- a/core/java/android/app/admin/DeviceAdminReceiver.java +++ b/core/java/android/app/admin/DeviceAdminReceiver.java @@ -1073,6 +1073,7 @@ public class DeviceAdminReceiver extends BroadcastReceiver { private void onOperationSafetyStateChanged(Context context, Intent intent) { if (!hasRequiredExtra(intent, EXTRA_OPERATION_SAFETY_REASON) || !hasRequiredExtra(intent, EXTRA_OPERATION_SAFETY_STATE)) { + Log.w(TAG, "Igoring intent that's missing required extras"); return; } @@ -1084,7 +1085,6 @@ public class DeviceAdminReceiver extends BroadcastReceiver { } boolean isSafe = intent.getBooleanExtra(EXTRA_OPERATION_SAFETY_STATE, /* defaultValue=*/ false); - onOperationSafetyStateChanged(context, reason, isSafe); } diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java index eb7a4d4217a9d..bad3892efc19c 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java @@ -12341,7 +12341,6 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { Slog.v(LOG_TAG, String.format("notifyUnsafeOperationStateChanged(): %s=%b", DevicePolicyManager.operationSafetyReasonToString(reason), isSafe)); } - Preconditions.checkArgument(mSafetyChecker == checker, "invalid checker: should be %s, was %s", mSafetyChecker, checker); @@ -12349,7 +12348,6 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { extras.putInt(DeviceAdminReceiver.EXTRA_OPERATION_SAFETY_REASON, reason); extras.putBoolean(DeviceAdminReceiver.EXTRA_OPERATION_SAFETY_STATE, isSafe); - // TODO(b/178494483): add CTS test sendDeviceOwnerCommand(DeviceAdminReceiver.ACTION_OPERATION_SAFETY_STATE_CHANGED, extras); for (int profileOwnerId : mOwners.getProfileOwnerKeys()) { diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/OneTimeSafetyChecker.java b/services/devicepolicy/java/com/android/server/devicepolicy/OneTimeSafetyChecker.java index 6ec6f915ea0f6..776b444456787 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/OneTimeSafetyChecker.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/OneTimeSafetyChecker.java @@ -72,11 +72,11 @@ final class OneTimeSafetyChecker implements DevicePolicySafetyChecker { DevicePolicyManagerInternal dpmi = LocalServices .getService(DevicePolicyManagerInternal.class); - Slog.i(TAG, "notifying " + reasonName + " is active"); - dpmi.notifyUnsafeOperationStateChanged(this, reason, true); + Slog.i(TAG, "notifying " + reasonName + " is UNSAFE"); + dpmi.notifyUnsafeOperationStateChanged(this, reason, /* isSafe= */ false); - Slog.i(TAG, "notifying " + reasonName + " is inactive"); - dpmi.notifyUnsafeOperationStateChanged(this, reason, false); + Slog.i(TAG, "notifying " + reasonName + " is SAFE"); + dpmi.notifyUnsafeOperationStateChanged(this, reason, /* isSafe= */ true); Slog.i(TAG, "returning " + reasonName);