diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java index 2df58ccf88642..d1294fe5c5967 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java @@ -1032,9 +1032,7 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { Slog.w(LOG_TAG, "factoryReset(): " + reason); final long identity = Binder.clearCallingIdentity(); try { - FactoryResetter.factoryReset(mContext, /* shutdown= */ false, reason, - /* force= */ false, /* wipeEuicc= */ false, /* wipeAdoptableStorage= */ false, - /* wipeFactoryResetProtection= */ false); + FactoryResetter.newBuilder(mContext).setReason(reason).build().factoryReset(); } catch (IOException e) { // Shouldn't happen. Slog.wtf(LOG_TAG, "Could not factory reset", e); @@ -1295,8 +1293,11 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { void recoverySystemRebootWipeUserData(boolean shutdown, String reason, boolean force, boolean wipeEuicc, boolean wipeExtRequested, boolean wipeResetProtectionData) throws IOException { - FactoryResetter.factoryReset(mContext, shutdown, reason, force, wipeEuicc, - wipeExtRequested, wipeResetProtectionData); + FactoryResetter.newBuilder(mContext).setReason(reason).setShutdown(shutdown) + .setForce(force).setWipeEuicc(wipeEuicc) + .setWipeAdoptableStorage(wipeExtRequested) + .setWipeFactoryResetProtection(wipeResetProtectionData) + .build().factoryReset(); } boolean systemPropertiesGetBoolean(String key, boolean def) { diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/FactoryResetter.java b/services/devicepolicy/java/com/android/server/devicepolicy/FactoryResetter.java index 1ee6f8f83428d..a0cf7a31650d5 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/FactoryResetter.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/FactoryResetter.java @@ -16,6 +16,7 @@ package com.android.server.devicepolicy; +import android.annotation.Nullable; import android.content.Context; import android.content.pm.PackageManager; import android.os.RecoverySystem; @@ -27,37 +28,43 @@ import android.util.Log; import com.android.internal.util.Preconditions; import java.io.IOException; +import java.util.Objects; /** * Entry point for "factory reset" requests. */ -final class FactoryResetter { +public final class FactoryResetter { private static final String TAG = FactoryResetter.class.getSimpleName(); - // TODO(b/171603586): use an object that's constructed with a Builder instead (then update - // javadoc) - /** - * Factory reset the device. - */ - public static void factoryReset(Context context, boolean shutdown, String reason, - boolean force, boolean wipeEuicc, boolean wipeAdoptableStorage, - boolean wipeFactoryResetProtection) throws IOException { - Log.i(TAG, "factoryReset(): shutdown=" + shutdown + ", force=" + force - + ", wipeEuicc=" + wipeEuicc + ", wipeAdoptableStorage=" + wipeAdoptableStorage - + ", wipeFRP=" + wipeFactoryResetProtection); + private final Context mContext; + private final @Nullable String mReason; + private final boolean mShutdown; + private final boolean mForce; + private final boolean mWipeEuicc; + private final boolean mWipeAdoptableStorage; + private final boolean mWipeFactoryResetProtection; - Preconditions.checkCallAuthorization(context.checkCallingOrSelfPermission( + + /** + * Factory reset the device according to the builder's arguments. + */ + public void factoryReset() throws IOException { + Log.i(TAG, String.format("factoryReset(): reason=%s, shutdown=%b, force=%b, wipeEuicc=%b" + + ", wipeAdoptableStorage=%b, wipeFRP=%b", mReason, mShutdown, mForce, mWipeEuicc, + mWipeAdoptableStorage, mWipeFactoryResetProtection)); + + Preconditions.checkCallAuthorization(mContext.checkCallingOrSelfPermission( android.Manifest.permission.MASTER_CLEAR) == PackageManager.PERMISSION_GRANTED); - UserManager um = (UserManager) context.getSystemService(Context.USER_SERVICE); - if (!force && um.hasUserRestriction(UserManager.DISALLOW_FACTORY_RESET)) { + UserManager um = mContext.getSystemService(UserManager.class); + if (!mForce && um.hasUserRestriction(UserManager.DISALLOW_FACTORY_RESET)) { throw new SecurityException("Factory reset is not allowed for this user."); } - if (wipeFactoryResetProtection) { - PersistentDataBlockManager manager = (PersistentDataBlockManager) - context.getSystemService(Context.PERSISTENT_DATA_BLOCK_SERVICE); + if (mWipeFactoryResetProtection) { + PersistentDataBlockManager manager = mContext + .getSystemService(PersistentDataBlockManager.class); if (manager != null) { Log.w(TAG, "Wiping factory reset protection"); manager.wipe(); @@ -66,17 +73,104 @@ final class FactoryResetter { } } - if (wipeAdoptableStorage) { + if (mWipeAdoptableStorage) { Log.w(TAG, "Wiping adoptable storage"); - StorageManager sm = (StorageManager) context.getSystemService( - Context.STORAGE_SERVICE); + StorageManager sm = mContext.getSystemService(StorageManager.class); sm.wipeAdoptableDisks(); } - RecoverySystem.rebootWipeUserData(context, shutdown, reason, force, wipeEuicc); + RecoverySystem.rebootWipeUserData(mContext, mShutdown, mReason, mForce, mWipeEuicc); } - private FactoryResetter() { - throw new UnsupportedOperationException("Contains only static methods"); + private FactoryResetter(Builder builder) { + mContext = builder.mContext; + mShutdown = builder.mShutdown; + mReason = builder.mReason; + mForce = builder.mForce; + mWipeEuicc = builder.mWipeEuicc; + mWipeAdoptableStorage = builder.mWipeAdoptableStorage; + mWipeFactoryResetProtection = builder.mWipeFactoryResetProtection; + } + + /** + * Creates a new builder. + */ + public static Builder newBuilder(Context context) { + return new Builder(context); + } + + /** + * Builder for {@link FactoryResetter} instances. + */ + public static final class Builder { + + private final Context mContext; + private @Nullable String mReason; + private boolean mShutdown; + private boolean mForce; + private boolean mWipeEuicc; + private boolean mWipeAdoptableStorage; + private boolean mWipeFactoryResetProtection; + + private Builder(Context context) { + mContext = Objects.requireNonNull(context); + } + + /** + * Sets the (non-null) reason for the factory reset that is visible in the logs + */ + public Builder setReason(String reason) { + mReason = Objects.requireNonNull(reason); + return this; + } + + /** + * Sets whether the device will be powered down after the wipe completes, rather than being + * rebooted back to the regular system. + */ + public Builder setShutdown(boolean value) { + mShutdown = value; + return this; + } + + /** + * Sets whether the {@link UserManager.DISALLOW_FACTORY_RESET} user restriction should be + * ignored. + */ + public Builder setForce(boolean value) { + mForce = value; + return this; + } + + /** + * Sets whether to wipe the {@code euicc} data. + */ + public Builder setWipeEuicc(boolean value) { + mWipeEuicc = value; + return this; + } + + /** + * Sets whether to wipe the adoptable external storage (if any). + */ + public Builder setWipeAdoptableStorage(boolean value) { + mWipeAdoptableStorage = value; + return this; + } + + /** + * Sets whether to reset the factory reset protection. + */ + public Builder setWipeFactoryResetProtection(boolean value) { + mWipeFactoryResetProtection = value; + return this; + } + + /** + * Builds the {@link FactoryResetter} instance. + */ + public FactoryResetter build() { + return new FactoryResetter(this); + } } } diff --git a/services/tests/mockingservicestests/src/com/android/server/devicepolicy/FactoryResetterTest.java b/services/tests/mockingservicestests/src/com/android/server/devicepolicy/FactoryResetterTest.java index 45160dc4c1d71..829e3124dc393 100644 --- a/services/tests/mockingservicestests/src/com/android/server/devicepolicy/FactoryResetterTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/devicepolicy/FactoryResetterTest.java @@ -50,18 +50,7 @@ public final class FactoryResetterTest { private static final String TAG = FactoryResetterTest.class.getSimpleName(); - // Fixed parameters private static final String REASON = "self-destruct"; - private static final boolean SHUTDOWN = true; - private static final boolean WIPE_EUICC = true; - - // Parameters under test - private static final boolean FORCE = true; - private static final boolean NO_FORCE = false; - private static final boolean WIPE_ADOPTABLE_STORAGE = true; - private static final boolean NO_WIPE_ADOPTABLE_STORAGE = false; - private static final boolean WIPE_FACTORY_RESET_PROTECTION = true; - private static final boolean NO_WIPE_FACTORY_RESET_PROTECTION = false; private MockitoSession mSession; @@ -78,19 +67,13 @@ public final class FactoryResetterTest { .strictness(Strictness.LENIENT) .startMocking(); - when(mContext.getSystemService(any(String.class))).thenAnswer((inv) -> { + when(mContext.getSystemService(any(Class.class))).thenAnswer((inv) -> { Log.d(TAG, "Mocking " + inv); - String service = (String) inv.getArguments()[0]; - switch (service) { - case Context.PERSISTENT_DATA_BLOCK_SERVICE: - return mPdbm; - case Context.STORAGE_SERVICE: - return mSm; - case Context.USER_SERVICE: - return mUm; - default: - throw new IllegalArgumentException("Not expecting call for " + service); - } + Class serviceClass = (Class) inv.getArguments()[0]; + if (serviceClass.equals(PersistentDataBlockManager.class)) return mPdbm; + if (serviceClass.equals(StorageManager.class)) return mSm; + if (serviceClass.equals(UserManager.class)) return mUm; + throw new IllegalArgumentException("Not expecting call for " + serviceClass); }); doAnswer((inv) -> { @@ -110,13 +93,23 @@ public final class FactoryResetterTest { } @Test - public void testFactoryReset_noMasterClearPermission() throws Exception { + public void testFactoryResetBuilder_nullContext() throws Exception { + assertThrows(NullPointerException.class, () -> FactoryResetter.newBuilder(null)); + } + + @Test + public void testFactoryResetBuilder_nullReason() throws Exception { + assertThrows(NullPointerException.class, + () -> FactoryResetter.newBuilder(mContext).setReason(null)); + } + + @Test + public void testFactoryReset_minimumArgs_noMasterClearPermission() throws Exception { revokeMasterClearPermission(); - setFactoryResetRestriction(/* allowed= */ true); + allowFactoryReset(); assertThrows(SecurityException.class, - () -> FactoryResetter.factoryReset(mContext, SHUTDOWN, REASON, NO_FORCE, - WIPE_EUICC, WIPE_ADOPTABLE_STORAGE, WIPE_FACTORY_RESET_PROTECTION)); + () -> FactoryResetter.newBuilder(mContext).build().factoryReset()); verifyWipeAdoptableStorageNotCalled(); verifyWipeFactoryResetProtectionNotCalled(); @@ -124,13 +117,11 @@ public final class FactoryResetterTest { } @Test - public void testFactoryReset_noForceDisallowed() - throws Exception { - setFactoryResetRestriction(/* allowed= */ false); + public void testFactoryReset_minimumArgs_withRestriction_notForced() throws Exception { + disallowFactoryReset(); assertThrows(SecurityException.class, - () -> FactoryResetter.factoryReset(mContext, SHUTDOWN, REASON, NO_FORCE, - WIPE_EUICC, WIPE_ADOPTABLE_STORAGE, WIPE_FACTORY_RESET_PROTECTION)); + () -> FactoryResetter.newBuilder(mContext).build().factoryReset()); verifyWipeAdoptableStorageNotCalled(); verifyWipeFactoryResetProtectionNotCalled(); @@ -138,57 +129,69 @@ public final class FactoryResetterTest { } @Test - public void testFactoryReset_noForceAllowed() throws Exception { - setFactoryResetRestriction(/* allowed= */ true); + public void testFactoryReset_minimumArgs_noRestriction_notForced() throws Exception { + allowFactoryReset(); - FactoryResetter.factoryReset(mContext, SHUTDOWN, REASON, NO_FORCE, - WIPE_EUICC, WIPE_ADOPTABLE_STORAGE, WIPE_FACTORY_RESET_PROTECTION); - - verifyWipeAdoptableStorageCalled(); - verifyWipeFactoryResetProtectionCalled(); - verifyRebootWipeUserDataCalled(NO_FORCE); - } - - @Test - public void testFactoryReset_forceDisallowed() throws Exception { - setFactoryResetRestriction(/* allowed= */ false); - - FactoryResetter.factoryReset(mContext, SHUTDOWN, REASON, FORCE, - WIPE_EUICC, WIPE_ADOPTABLE_STORAGE, WIPE_FACTORY_RESET_PROTECTION); - - verifyWipeAdoptableStorageCalled(); - verifyWipeFactoryResetProtectionCalled(); - verifyRebootWipeUserDataCalled(FORCE); - } - - @Test - public void testFactoryReset_bothFalse() throws Exception { - FactoryResetter.factoryReset(mContext, SHUTDOWN, REASON, FORCE, - WIPE_EUICC, NO_WIPE_ADOPTABLE_STORAGE, NO_WIPE_FACTORY_RESET_PROTECTION); + FactoryResetter.newBuilder(mContext).build().factoryReset(); verifyWipeAdoptableStorageNotCalled(); verifyWipeFactoryResetProtectionNotCalled(); - verifyRebootWipeUserDataCalled(FORCE); + verifyRebootWipeUserDataMinimumArgsCalled(); + } + + @Test + public void testFactoryReset_minimumArgs_withRestriction_forced() throws Exception { + disallowFactoryReset(); + + FactoryResetter.newBuilder(mContext).setForce(true).build().factoryReset(); + + verifyWipeAdoptableStorageNotCalled(); + verifyWipeFactoryResetProtectionNotCalled(); + verifyRebootWipeUserDataMinimumArgsButForceCalled(); } @Test public void testFactoryReset_storageOnly() throws Exception { - FactoryResetter.factoryReset(mContext, SHUTDOWN, REASON, FORCE, - WIPE_EUICC, WIPE_ADOPTABLE_STORAGE, NO_WIPE_FACTORY_RESET_PROTECTION); + allowFactoryReset(); + + FactoryResetter.newBuilder(mContext) + .setWipeAdoptableStorage(true).build() + .factoryReset(); verifyWipeAdoptableStorageCalled(); verifyWipeFactoryResetProtectionNotCalled(); - verifyRebootWipeUserDataCalled(FORCE); + verifyRebootWipeUserDataMinimumArgsCalled(); } @Test public void testFactoryReset_frpOnly() throws Exception { - FactoryResetter.factoryReset(mContext, SHUTDOWN, REASON, FORCE, - WIPE_EUICC, NO_WIPE_ADOPTABLE_STORAGE, WIPE_FACTORY_RESET_PROTECTION); + allowFactoryReset(); + + FactoryResetter.newBuilder(mContext) + .setWipeFactoryResetProtection(true) + .build().factoryReset(); verifyWipeAdoptableStorageNotCalled(); verifyWipeFactoryResetProtectionCalled(); - verifyRebootWipeUserDataCalled(FORCE); + verifyRebootWipeUserDataMinimumArgsCalled(); + } + + @Test + public void testFactoryReset_allArgs() throws Exception { + allowFactoryReset(); + + FactoryResetter.newBuilder(mContext) + .setReason(REASON) + .setForce(true) + .setShutdown(true) + .setWipeEuicc(true) + .setWipeAdoptableStorage(true) + .setWipeFactoryResetProtection(true) + .build().factoryReset(); + + verifyWipeAdoptableStorageCalled(); + verifyWipeFactoryResetProtectionCalled(); + verifyRebootWipeUserDataAllArgsCalled(); } private void revokeMasterClearPermission() { @@ -196,8 +199,12 @@ public final class FactoryResetterTest { .thenReturn(PackageManager.PERMISSION_DENIED); } - private void setFactoryResetRestriction(boolean allowed) { - when(mUm.hasUserRestriction(UserManager.DISALLOW_FACTORY_RESET)).thenReturn(!allowed); + private void allowFactoryReset() { + when(mUm.hasUserRestriction(UserManager.DISALLOW_FACTORY_RESET)).thenReturn(false); + } + + private void disallowFactoryReset() { + when(mUm.hasUserRestriction(UserManager.DISALLOW_FACTORY_RESET)).thenReturn(true); } private void verifyRebootWipeUserDataNotCalled() { @@ -205,9 +212,19 @@ public final class FactoryResetterTest { anyBoolean()), never()); } - private void verifyRebootWipeUserDataCalled(boolean force) { - verify(() -> RecoverySystem.rebootWipeUserData(mContext, SHUTDOWN, REASON, force, - WIPE_EUICC)); + private void verifyRebootWipeUserDataMinimumArgsCalled() { + verify(() -> RecoverySystem.rebootWipeUserData(mContext, /* shutdown= */ false, + /* reason= */ null, /* force= */ false, /* wipeEuicc= */ false)); + } + + private void verifyRebootWipeUserDataMinimumArgsButForceCalled() { + verify(() -> RecoverySystem.rebootWipeUserData(mContext, /* shutdown= */ false, + /* reason= */ null, /* force= */ true, /* wipeEuicc= */ false)); + } + + private void verifyRebootWipeUserDataAllArgsCalled() { + verify(() -> RecoverySystem.rebootWipeUserData(mContext, /* shutdown= */ true, + /* reason= */ REASON, /* force= */ true, /* wipeEuicc= */ true)); } private void verifyWipeAdoptableStorageNotCalled() {