Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit b1c9f827 authored by Rafael Prado's avatar Rafael Prado
Browse files

Properly use futures to wait on API result in SetPermissionGrantState.

Test: atest android.devicepolicy.cts.PermissionGrantTest with flag on/off
Flag: android.app.admin.flags.set_permission_grant_state_coexistence
Bug: 370472975
Bug: 336297680
Change-Id: I1f44d3de6aa7fbf61c85fe8e147fb381870def4a
parent fd1745d6
Loading
Loading
Loading
Loading
+22 −23
Original line number Diff line number Diff line
@@ -238,6 +238,8 @@ import static android.app.admin.PolicySizeVerifier.MAX_LONG_SUPPORT_MESSAGE_LENG
import static android.app.admin.PolicySizeVerifier.MAX_ORG_NAME_LENGTH;
import static android.app.admin.PolicySizeVerifier.MAX_PROFILE_NAME_LENGTH;
import static android.app.admin.PolicySizeVerifier.MAX_SHORT_SUPPORT_MESSAGE_LENGTH;
import static android.app.admin.PolicyUpdateResult.RESULT_POLICY_CLEARED;
import static android.app.admin.PolicyUpdateResult.RESULT_POLICY_SET;
import static android.app.admin.ProvisioningException.ERROR_ADMIN_PACKAGE_INSTALLATION_FAILED;
import static android.app.admin.ProvisioningException.ERROR_PRE_CONDITION_FAILED;
import static android.app.admin.ProvisioningException.ERROR_PROFILE_CREATION_FAILED;
@@ -16840,9 +16842,11 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
        }
        EnforcingAdmin enforcingAdmin;
        // TODO(b/370472975): enable when we stop policy enforecer callback from blocking the main
        //  thread
        if (Flags.setPermissionGrantStateCoexistence()) {
        if (Flags.setPermissionGrantStateCoexistence() && Flags.dpeBasedOnAsyncApisEnabled()) {
            // TODO(b/403539755):  If admin calls setPermissionGrantState and enforcement fails,
            //  subsequent calls will succeed without retrying enforcement. Need to somehow track
            //  actual enforcement status to disambiguate.
            enforcingAdmin = enforcePermissionAndGetEnforcingAdmin(
                    admin,
                    MANAGE_DEVICE_POLICY_RUNTIME_PERMISSIONS,
@@ -16872,26 +16876,21 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
            //  exist, or the permission isn't requested by the app, because we could end up with
            //  inconsistent state between the policy engine and package manager. Also a package
            //  might get removed or has it's permission updated after we've set the policy.
            if (grantState == PERMISSION_GRANT_STATE_DEFAULT) {
            CompletableFuture<Integer> setPolicyFuture =
                    grantState == PERMISSION_GRANT_STATE_DEFAULT ?
                            mDevicePolicyEngine.removeLocalPolicy(
                                    PolicyDefinition.PERMISSION_GRANT(packageName, permission),
                                    enforcingAdmin,
                        caller.getUserId());
            } else {
                mDevicePolicyEngine.setLocalPolicy(
                                    caller.getUserId())
                            : mDevicePolicyEngine.setLocalPolicy(
                                    PolicyDefinition.PERMISSION_GRANT(packageName, permission),
                                    enforcingAdmin,
                                    new IntegerPolicyValue(grantState),
                                    caller.getUserId());
            }
            int newState = mInjector.binderWithCleanCallingIdentity(() ->
                    getPermissionGrantStateForUser(
                            packageName, permission, caller, caller.getUserId()));
            if (newState == grantState) {
                callback.sendResult(Bundle.EMPTY);
            } else {
                callback.sendResult(null);
            }
            setPolicyFuture.thenAccept((result) ->
                    callback.sendResult(result == RESULT_POLICY_SET ||
                            result == RESULT_POLICY_CLEARED? Bundle.EMPTY : null));
        } else {
            Preconditions.checkCallAuthorization((caller.hasAdminComponent()
                    && (isProfileOwner(caller) || isDefaultDeviceOwner(caller)
+7 −30
Original line number Diff line number Diff line
@@ -68,9 +68,6 @@ import java.util.List;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicReference;

final class PolicyEnforcerCallbacks {

@@ -123,24 +120,21 @@ final class PolicyEnforcerCallbacks {
                    ? DevicePolicyManager.PERMISSION_GRANT_STATE_DEFAULT
                    : grantState;

            // TODO(b/278710449): stop blocking in the main thread
            BlockingCallback callback = new BlockingCallback();
            // TODO: remove canAdminGrantSensorPermissions once we expose a new method in
            //  permissionController that doesn't need it.
            AdminPermissionControlParams permissionParams = new AdminPermissionControlParams(
                    parsedKey.getPackageName(), parsedKey.getPermissionName(), value,
                    /* canAdminGrantSensorPermissions= */ true);

            CompletableFuture<Boolean> enforcementFuture = new AndroidFuture<>();

            getPermissionControllerManager(context, UserHandle.of(userId))
                    // TODO: remove callingPackage param and stop passing context.getPackageName()
                    .setRuntimePermissionGrantStateByDeviceAdmin(context.getPackageName(),
                            permissionParams, context.getMainExecutor(), callback::trigger);
            try {
                return AndroidFuture.completedFuture(
                        callback.await(20_000, TimeUnit.MILLISECONDS));
            } catch (Exception e) {
                // TODO: add logging
                return AndroidFuture.completedFuture(false);
            }
                            permissionParams, context.getMainExecutor(),
                            enforcementFuture::complete);

            return enforcementFuture;
        });
    }

@@ -231,23 +225,6 @@ final class PolicyEnforcerCallbacks {
        });
    }

    private static class BlockingCallback {
        private final CountDownLatch mLatch = new CountDownLatch(1);
        private final AtomicReference<Boolean> mValue = new AtomicReference<>();

        public void trigger(Boolean value) {
            mValue.set(value);
            mLatch.countDown();
        }

        public Boolean await(long timeout, TimeUnit unit) throws InterruptedException {
            if (!mLatch.await(timeout, unit)) {
                Slogf.e(LOG_TAG, "Callback was not received");
            }
            return mValue.get();
        }
    }

    // TODO: when a local policy exists for a user, this callback will be invoked for this user
    // individually as well as for USER_ALL. This can be optimized by separating local and global
    // enforcement in the policy engine.