diff --git a/binder/src/androidTest/java/io/grpc/binder/internal/BinderTransportTest.java b/binder/src/androidTest/java/io/grpc/binder/internal/BinderTransportTest.java index c06bf88955e..53a724fc7e0 100644 --- a/binder/src/androidTest/java/io/grpc/binder/internal/BinderTransportTest.java +++ b/binder/src/androidTest/java/io/grpc/binder/internal/BinderTransportTest.java @@ -71,7 +71,8 @@ protected InternalServer newServer(List streamTracer executorServicePool, streamTracerFactories, BinderInternal.createPolicyChecker(SecurityPolicies.serverInternalOnly()), - InboundParcelablePolicy.DEFAULT); + InboundParcelablePolicy.DEFAULT, + /* shutdownListener=*/ () -> {}); HostServices.configureService(addr, HostServices.serviceParamsBuilder() diff --git a/binder/src/main/java/io/grpc/binder/BinderServerBuilder.java b/binder/src/main/java/io/grpc/binder/BinderServerBuilder.java index dafb884d6b1..158f7947ee8 100644 --- a/binder/src/main/java/io/grpc/binder/BinderServerBuilder.java +++ b/binder/src/main/java/io/grpc/binder/BinderServerBuilder.java @@ -33,9 +33,14 @@ import io.grpc.internal.ServerImplBuilder; import io.grpc.internal.ObjectPool; import io.grpc.internal.SharedResourcePool; + +import java.io.Closeable; import java.io.File; +import java.util.concurrent.Executor; import java.util.concurrent.ScheduledExecutorService; +import javax.annotation.Nullable; + /** * Builder for a server that services requests from an Android Service. */ @@ -72,6 +77,7 @@ public static BinderServerBuilder forPort(int port) { private ServerSecurityPolicy securityPolicy; private InboundParcelablePolicy inboundParcelablePolicy; private boolean isBuilt; + @Nullable private BinderTransportSecurity.ShutdownListener shutdownListener = null; private BinderServerBuilder( AndroidComponentAddress listenAddress, @@ -85,7 +91,9 @@ private BinderServerBuilder( schedulerPool, streamTracerFactories, BinderInternal.createPolicyChecker(securityPolicy), - inboundParcelablePolicy); + inboundParcelablePolicy, + // 'shutdownListener' should have been set by build() + checkNotNull(shutdownListener)); BinderInternal.setIBinder(binderReceiver, server.getHostBinder()); return server; }); @@ -171,7 +179,10 @@ public Server build() { checkState(!isBuilt, "BinderServerBuilder can only be used to build one server instance."); isBuilt = true; // We install the security interceptor last, so it's closest to the transport. - BinderTransportSecurity.installAuthInterceptor(this); + ObjectPool executorPool = serverImplBuilder.getExecutorPool(); + Executor executor = executorPool.getObject(); + BinderTransportSecurity.installAuthInterceptor(this, executor); + shutdownListener = () -> executorPool.returnObject(executor); return super.build(); } } diff --git a/binder/src/main/java/io/grpc/binder/internal/BinderServer.java b/binder/src/main/java/io/grpc/binder/internal/BinderServer.java index e72f2851b29..8d6fe08fa94 100644 --- a/binder/src/main/java/io/grpc/binder/internal/BinderServer.java +++ b/binder/src/main/java/io/grpc/binder/internal/BinderServer.java @@ -60,6 +60,7 @@ public final class BinderServer implements InternalServer, LeakSafeOneWayBinder. private final LeakSafeOneWayBinder hostServiceBinder; private final BinderTransportSecurity.ServerPolicyChecker serverPolicyChecker; private final InboundParcelablePolicy inboundParcelablePolicy; + private final BinderTransportSecurity.ShutdownListener transportSecurityShutdownListener; @GuardedBy("this") private ServerListener listener; @@ -70,18 +71,24 @@ public final class BinderServer implements InternalServer, LeakSafeOneWayBinder. @GuardedBy("this") private boolean shutdown; + /** + * @param transportSecurityShutdownListener represents resources that should be cleaned up once + * the server shuts down. + */ public BinderServer( AndroidComponentAddress listenAddress, ObjectPool executorServicePool, List streamTracerFactories, BinderTransportSecurity.ServerPolicyChecker serverPolicyChecker, - InboundParcelablePolicy inboundParcelablePolicy) { + InboundParcelablePolicy inboundParcelablePolicy, + BinderTransportSecurity.ShutdownListener transportSecurityShutdownListener) { this.listenAddress = listenAddress; this.executorServicePool = executorServicePool; this.streamTracerFactories = ImmutableList.copyOf(checkNotNull(streamTracerFactories, "streamTracerFactories")); this.serverPolicyChecker = checkNotNull(serverPolicyChecker, "serverPolicyChecker"); this.inboundParcelablePolicy = inboundParcelablePolicy; + this.transportSecurityShutdownListener = transportSecurityShutdownListener; hostServiceBinder = new LeakSafeOneWayBinder(this); } @@ -125,6 +132,7 @@ public synchronized void shutdown() { hostServiceBinder.detach(); listener.serverShutdown(); executorService = executorServicePool.returnObject(executorService); + transportSecurityShutdownListener.onServerShutdown(); } } diff --git a/binder/src/main/java/io/grpc/binder/internal/BinderTransportSecurity.java b/binder/src/main/java/io/grpc/binder/internal/BinderTransportSecurity.java index 022ae7f6d20..1866bf54a47 100644 --- a/binder/src/main/java/io/grpc/binder/internal/BinderTransportSecurity.java +++ b/binder/src/main/java/io/grpc/binder/internal/BinderTransportSecurity.java @@ -16,7 +16,10 @@ package io.grpc.binder.internal; +import com.google.common.util.concurrent.FutureCallback; +import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; + import io.grpc.Attributes; import io.grpc.Internal; import io.grpc.Metadata; @@ -28,9 +31,12 @@ import io.grpc.ServerInterceptor; import io.grpc.Status; import io.grpc.internal.GrpcAttributes; + import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutionException; +import java.util.concurrent.Executor; import javax.annotation.CheckReturnValue; +import javax.annotation.Nullable; /** * Manages security for an Android Service hosted gRPC server. @@ -49,10 +55,11 @@ private BinderTransportSecurity() {} * Install a security policy on an about-to-be created server. * * @param serverBuilder The ServerBuilder being used to create the server. + * @param executor The executor in which the authorization result will be handled. */ @Internal - public static void installAuthInterceptor(ServerBuilder serverBuilder) { - serverBuilder.intercept(new ServerAuthInterceptor()); + public static void installAuthInterceptor(ServerBuilder serverBuilder, Executor executor) { + serverBuilder.intercept(new ServerAuthInterceptor(executor)); } /** @@ -78,13 +85,36 @@ public static void attachAuthAttrs( * Authentication state is fetched from the call attributes, inherited from the transport. */ private static final class ServerAuthInterceptor implements ServerInterceptor { + + private final Executor executor; + + ServerAuthInterceptor(Executor executor) { + this.executor = executor; + } + @Override public ServerCall.Listener interceptCall( ServerCall call, Metadata headers, ServerCallHandler next) { - Status authStatus = + ListenableFuture authStatusFuture = call.getAttributes() .get(TRANSPORT_AUTHORIZATION_STATE) .checkAuthorization(call.getMethodDescriptor()); + + // Most SecurityPolicy will have synchronous implementations that provide an + // immediately-resolved Future. In that case, short-circuit to avoid unnecessary allocations + // and asynchronous code if the authorization result is already present. + if (!authStatusFuture.isDone()) { + return newServerCallListenerForPendingAuthResult(authStatusFuture, call, headers, next); + } + + Status authStatus; + try { + authStatus = Futures.getDone(authStatusFuture); + } catch (ExecutionException e) { + // Failed futures are treated as an internal error rather than a security rejection. + authStatus = Status.INTERNAL.withCause(e); + } + if (authStatus.isOk()) { return next.startCall(call, headers); } else { @@ -92,16 +122,45 @@ public ServerCall.Listener interceptCall( return new ServerCall.Listener() {}; } } + + private ServerCall.Listener newServerCallListenerForPendingAuthResult( + ListenableFuture authStatusFuture, + ServerCall call, + Metadata headers, + ServerCallHandler next) { + PendingAuthListener listener = new PendingAuthListener<>(); + Futures.addCallback( + authStatusFuture, + new FutureCallback() { + @Override + public void onSuccess(Status authStatus) { + if (!authStatus.isOk()) { + call.close(authStatus, new Metadata()); + return; + } + + listener.startCall(call, headers, next); + } + + @Override + public void onFailure(Throwable t) { + call.close( + Status.INTERNAL.withCause(t).withDescription("Authorization future failed"), + new Metadata()); + } + }, executor); + return listener; + } } /** - * Maintaines the authorization state for a single transport instance. This class lives for the + * Maintains the authorization state for a single transport instance. This class lives for the * lifetime of a single transport. */ private static final class TransportAuthorizationState { private final int uid; private final ServerPolicyChecker serverPolicyChecker; - private final ConcurrentHashMap serviceAuthorization; + private final ConcurrentHashMap> serviceAuthorization; TransportAuthorizationState(int uid, ServerPolicyChecker serverPolicyChecker) { this.uid = uid; @@ -111,32 +170,27 @@ private static final class TransportAuthorizationState { /** Get whether we're authorized to make this call. */ @CheckReturnValue - Status checkAuthorization(MethodDescriptor method) { + ListenableFuture checkAuthorization(MethodDescriptor method) { String serviceName = method.getServiceName(); // Only cache decisions if the method can be sampled for tracing, - // which is true for all generated methods. Otherwise, programatically - // created methods could casue this cahe to grow unbounded. + // which is true for all generated methods. Otherwise, programmatically + // created methods could cause this cache to grow unbounded. boolean useCache = method.isSampledToLocalTracing(); - Status authorization; if (useCache) { - authorization = serviceAuthorization.get(serviceName); + @Nullable ListenableFuture authorization = serviceAuthorization.get(serviceName); if (authorization != null) { return authorization; } } - try { - // TODO(10566): provide a synchronous version of "checkAuthorization" to avoid blocking the - // calling thread on the completion of the future. - authorization = - serverPolicyChecker.checkAuthorizationForServiceAsync(uid, serviceName).get(); - } catch (ExecutionException e) { - // Do not cache this failure since it may be transient. - return Status.fromThrowable(e); - } catch (InterruptedException e) { - // Do not cache this failure since it may be transient. - Thread.currentThread().interrupt(); - return Status.CANCELLED.withCause(e); - } + // Under high load, this may trigger a large number of concurrent authorization checks that + // perform essentially the same work and have the potential of exhausting the resources they + // depend on. This was a non-issue in the past with synchronous policy checks due to the + // fixed-size nature of the thread pool this method runs under. + // + // TODO(10669): evaluate if there should be at most a single pending authorization check per + // (uid, serviceName) pair at any given time. + ListenableFuture authorization = + serverPolicyChecker.checkAuthorizationForServiceAsync(uid, serviceName); if (useCache) { serviceAuthorization.putIfAbsent(serviceName, authorization); } @@ -167,4 +221,12 @@ public interface ServerPolicyChecker { */ ListenableFuture checkAuthorizationForServiceAsync(int uid, String serviceName); } + + /** + * A listener invoked when the {@link io.grpc.binder.internal.BinderServer} shuts down, allowing + * resources to be potentially cleaned up. + */ + public interface ShutdownListener { + void onServerShutdown(); + } } diff --git a/binder/src/main/java/io/grpc/binder/internal/PendingAuthListener.java b/binder/src/main/java/io/grpc/binder/internal/PendingAuthListener.java new file mode 100644 index 00000000000..cdafc9c9191 --- /dev/null +++ b/binder/src/main/java/io/grpc/binder/internal/PendingAuthListener.java @@ -0,0 +1,102 @@ +package io.grpc.binder.internal; + +import io.grpc.Metadata; +import io.grpc.ServerCall; +import io.grpc.ServerCallHandler; +import io.grpc.Status; + +import java.util.concurrent.ConcurrentLinkedQueue; +import java.util.concurrent.atomic.AtomicReference; + +import javax.annotation.Nullable; + +/** + * A {@link ServerCall.Listener} that can be returned by a {@link io.grpc.ServerInterceptor} to + * asynchronously advance the gRPC pending resolving a possibly asynchronous security policy check. + */ +final class PendingAuthListener extends ServerCall.Listener { + + private final ConcurrentLinkedQueue> pendingSteps = + new ConcurrentLinkedQueue<>(); + private final AtomicReference> delegateRef = + new AtomicReference<>(null); + + PendingAuthListener() {} + + void startCall(ServerCall call, + Metadata headers, + ServerCallHandler next) { + ServerCall.Listener delegate; + try { + delegate = next.startCall(call, headers); + } catch (RuntimeException e) { + call.close( + Status + .INTERNAL + .withCause(e) + .withDescription("Failed to start server call after authorization check"), + new Metadata()); + return; + } + delegateRef.set(delegate); + maybeRunPendingSteps(); + } + + /** + * Runs any enqueued step in this ServerCall listener as long as the authorization check is + * complete. Otherwise, no-op and returns immediately. + */ + private void maybeRunPendingSteps() { + @Nullable ServerCall.Listener delegate = delegateRef.get(); + if (delegate == null) { + return; + } + + // This section is synchronized so that no 2 threads may attempt to retrieve elements from the + // queue in order but end up executing the steps out of order. + synchronized (this) { + ListenerConsumer nextStep; + while ((nextStep = pendingSteps.poll()) != null) { + nextStep.accept(delegate); + } + } + } + + @Override + public void onCancel() { + pendingSteps.offer(ServerCall.Listener::onCancel); + maybeRunPendingSteps(); + } + + @Override + public void onComplete() { + pendingSteps.offer(ServerCall.Listener::onComplete); + maybeRunPendingSteps(); + } + + @Override + public void onHalfClose() { + pendingSteps.offer(ServerCall.Listener::onHalfClose); + maybeRunPendingSteps(); + } + + @Override + public void onMessage(ReqT message) { + pendingSteps.offer(delegate -> delegate.onMessage(message)); + maybeRunPendingSteps(); + } + + @Override + public void onReady() { + pendingSteps.offer(ServerCall.Listener::onReady); + maybeRunPendingSteps(); + } + + /** + * Similar to Java8's {@link java.util.function.Consumer}, but redeclared in order to support + * Android SDK 21. + */ + private interface ListenerConsumer { + void accept(ServerCall.Listener listener); + } +} diff --git a/binder/src/test/java/io/grpc/binder/ServerSecurityPolicyTest.java b/binder/src/test/java/io/grpc/binder/ServerSecurityPolicyTest.java index 9393aab1e7f..fb7e8e05566 100644 --- a/binder/src/test/java/io/grpc/binder/ServerSecurityPolicyTest.java +++ b/binder/src/test/java/io/grpc/binder/ServerSecurityPolicyTest.java @@ -17,6 +17,7 @@ package io.grpc.binder; import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.fail; import android.os.Process; @@ -25,14 +26,18 @@ import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.ListeningExecutorService; import com.google.common.util.concurrent.MoreExecutors; +import com.google.common.util.concurrent.Uninterruptibles; + import io.grpc.Status; -import io.grpc.StatusException; +import io.grpc.Status.Code; import org.junit.Test; import org.junit.runner.RunWith; import org.robolectric.RobolectricTestRunner; import java.util.concurrent.BrokenBarrierException; +import java.util.concurrent.CancellationException; import java.util.concurrent.CountDownLatch; import java.util.concurrent.CyclicBarrier; +import java.util.concurrent.ExecutionException; import java.util.concurrent.Executors; @RunWith(RobolectricTestRunner.class) @@ -48,7 +53,16 @@ public final class ServerSecurityPolicyTest { ServerSecurityPolicy policy; @Test - public void testDefaultInternalOnly() { + public void testDefaultInternalOnly() throws Exception { + policy = new ServerSecurityPolicy(); + assertThat(checkAuthorizationForServiceAsync(policy, MY_UID, SERVICE1)) + .isEqualTo(Status.OK.getCode()); + assertThat(checkAuthorizationForServiceAsync(policy, MY_UID, SERVICE2)) + .isEqualTo(Status.OK.getCode()); + } + + @Test + public void testDefaultInternalOnly_legacyApi() { policy = new ServerSecurityPolicy(); assertThat(policy.checkAuthorizationForService(MY_UID, SERVICE1).getCode()) .isEqualTo(Status.OK.getCode()); @@ -57,7 +71,16 @@ public void testDefaultInternalOnly() { } @Test - public void testInternalOnly_AnotherUid() { + public void testInternalOnly_AnotherUid() throws Exception { + policy = new ServerSecurityPolicy(); + assertThat(checkAuthorizationForServiceAsync(policy, OTHER_UID, SERVICE1)) + .isEqualTo(Status.PERMISSION_DENIED.getCode()); + assertThat(checkAuthorizationForServiceAsync(policy, OTHER_UID, SERVICE2)) + .isEqualTo(Status.PERMISSION_DENIED.getCode()); + } + + @Test + public void testInternalOnly_AnotherUid_legacyApi() { policy = new ServerSecurityPolicy(); assertThat(policy.checkAuthorizationForService(OTHER_UID, SERVICE1).getCode()) .isEqualTo(Status.PERMISSION_DENIED.getCode()); @@ -66,7 +89,16 @@ public void testInternalOnly_AnotherUid() { } @Test - public void testBuilderDefault() { + public void testBuilderDefault() throws Exception { + policy = ServerSecurityPolicy.newBuilder().build(); + assertThat(checkAuthorizationForServiceAsync(policy, MY_UID, SERVICE1)) + .isEqualTo(Status.OK.getCode()); + assertThat(checkAuthorizationForServiceAsync(policy, OTHER_UID, SERVICE1)) + .isEqualTo(Status.PERMISSION_DENIED.getCode()); + } + + @Test + public void testBuilderDefault_legacyApi() { policy = ServerSecurityPolicy.newBuilder().build(); assertThat(policy.checkAuthorizationForService(MY_UID, SERVICE1).getCode()) .isEqualTo(Status.OK.getCode()); @@ -75,7 +107,25 @@ public void testBuilderDefault() { } @Test - public void testPerService() { + public void testPerService() throws Exception { + policy = + ServerSecurityPolicy.newBuilder() + .servicePolicy(SERVICE2, policy((uid) -> Status.OK)) + .build(); + + assertThat(checkAuthorizationForServiceAsync(policy, MY_UID, SERVICE1)) + .isEqualTo(Status.OK.getCode()); + assertThat(checkAuthorizationForServiceAsync(policy, OTHER_UID, SERVICE1)) + .isEqualTo(Status.PERMISSION_DENIED.getCode()); + assertThat(checkAuthorizationForServiceAsync(policy, MY_UID, SERVICE2)) + .isEqualTo(Status.OK.getCode()); + assertThat(checkAuthorizationForServiceAsync(policy, OTHER_UID, SERVICE2)) + .isEqualTo(Status.OK.getCode()); + } + + + @Test + public void testPerService_legacyApi() { policy = ServerSecurityPolicy.newBuilder() .servicePolicy(SERVICE2, policy((uid) -> Status.OK)) @@ -92,7 +142,7 @@ public void testPerService() { } @Test - public void testPerServiceAsync() { + public void testPerServiceAsync() throws Exception { policy = ServerSecurityPolicy.newBuilder() .servicePolicy(SERVICE2, asyncPolicy(uid -> { @@ -104,29 +154,31 @@ public void testPerServiceAsync() { })) .build(); - assertThat(policy.checkAuthorizationForService(MY_UID, SERVICE1).getCode()) + assertThat(checkAuthorizationForServiceAsync(policy, MY_UID, SERVICE1)) .isEqualTo(Status.OK.getCode()); - assertThat(policy.checkAuthorizationForService(OTHER_UID, SERVICE1).getCode()) + assertThat(checkAuthorizationForServiceAsync(policy, OTHER_UID, SERVICE1)) .isEqualTo(Status.PERMISSION_DENIED.getCode()); - assertThat(policy.checkAuthorizationForService(MY_UID, SERVICE2).getCode()) + assertThat(checkAuthorizationForServiceAsync(policy, MY_UID, SERVICE2)) .isEqualTo(Status.OK.getCode()); - assertThat(policy.checkAuthorizationForService(OTHER_UID, SERVICE2).getCode()) + assertThat(checkAuthorizationForServiceAsync(policy, OTHER_UID, SERVICE2)) .isEqualTo(Status.OK.getCode()); } @Test - public void testPerService_throwingExceptionAsynchronously_propagatesStatusFromException() { + public void testPerService_failedSecurityPolicyFuture_returnsAFailedFuture() { policy = ServerSecurityPolicy.newBuilder() .servicePolicy(SERVICE1, asyncPolicy(uid -> Futures .immediateFailedFuture( - new StatusException(Status.fromCode(Status.Code.ALREADY_EXISTS))) + new IllegalStateException("something went wrong")) )) .build(); - assertThat(policy.checkAuthorizationForService(MY_UID, SERVICE1).getCode()) - .isEqualTo(Status.ALREADY_EXISTS.getCode()); + ListenableFuture statusFuture = + policy.checkAuthorizationForServiceAsync(MY_UID, SERVICE1); + + assertThrows(ExecutionException.class, statusFuture::get); } @Test @@ -136,12 +188,14 @@ public void testPerServiceAsync_cancelledFuture_propagatesStatus() { .servicePolicy(SERVICE1, asyncPolicy(unused -> Futures.immediateCancelledFuture())) .build(); - assertThat(policy.checkAuthorizationForService(MY_UID, SERVICE1).getCode()) - .isEqualTo(Status.CANCELLED.getCode()); + ListenableFuture statusFuture = + policy.checkAuthorizationForServiceAsync(MY_UID, SERVICE1); + + assertThrows(CancellationException.class, statusFuture::get); } @Test - public void testPerServiceAsync_interrupted_cancelledStatus() { + public void testPerServiceAsync_interrupted_cancelledFuture() { ListeningExecutorService listeningExecutorService = MoreExecutors.listeningDecorator(Executors.newSingleThreadExecutor()); CountDownLatch unsatisfiedLatch = new CountDownLatch(1); @@ -164,15 +218,40 @@ public void testPerServiceAsync_interrupted_cancelledStatus() { return toBeInterruptedFuture; })) .build(); + ListenableFuture statusFuture = + policy.checkAuthorizationForServiceAsync(MY_UID, SERVICE1); - assertThat(policy.checkAuthorizationForService(MY_UID, SERVICE1).getCode()) - .isEqualTo(Status.CANCELLED.getCode()); - assertThat(Thread.currentThread().isInterrupted()).isTrue(); + assertThrows(InterruptedException.class, statusFuture::get); listeningExecutorService.shutdownNow(); } @Test - public void testPerServiceNoDefault() { + public void testPerServiceNoDefault() throws Exception { + policy = + ServerSecurityPolicy.newBuilder() + .servicePolicy(SERVICE1, policy((uid) -> Status.INTERNAL)) + .servicePolicy( + SERVICE2, policy((uid) -> uid == OTHER_UID ? Status.OK : Status.PERMISSION_DENIED)) + .build(); + + // Uses the specified policy for service1. + assertThat(checkAuthorizationForServiceAsync(policy, MY_UID, SERVICE1)) + .isEqualTo(Status.INTERNAL.getCode()); + assertThat(checkAuthorizationForServiceAsync(policy, OTHER_UID, SERVICE1)) + .isEqualTo(Status.INTERNAL.getCode()); + + // Uses the specified policy for service2. + assertThat(checkAuthorizationForServiceAsync(policy, MY_UID, SERVICE2)) + .isEqualTo(Status.PERMISSION_DENIED.getCode()); + assertThat(checkAuthorizationForServiceAsync(policy, OTHER_UID, SERVICE2)).isEqualTo(Status.OK.getCode()); + + // Falls back to the default. + assertThat(checkAuthorizationForServiceAsync(policy, MY_UID, SERVICE3)).isEqualTo(Status.OK.getCode()); + assertThat(checkAuthorizationForServiceAsync(policy, OTHER_UID, SERVICE3)) + .isEqualTo(Status.PERMISSION_DENIED.getCode()); + } + @Test + public void testPerServiceNoDefault_legacyApi() { policy = ServerSecurityPolicy.newBuilder() .servicePolicy(SERVICE1, policy((uid) -> Status.INTERNAL)) @@ -200,7 +279,7 @@ SERVICE2, policy((uid) -> uid == OTHER_UID ? Status.OK : Status.PERMISSION_DENIE } @Test - public void testPerServiceNoDefaultAsync() { + public void testPerServiceNoDefaultAsync() throws Exception { policy = ServerSecurityPolicy.newBuilder() .servicePolicy( @@ -224,24 +303,37 @@ SERVICE2, asyncPolicy((uid) -> { .build(); // Uses the specified policy for service1. - assertThat(policy.checkAuthorizationForService(MY_UID, SERVICE1).getCode()) + assertThat(checkAuthorizationForServiceAsync(policy, MY_UID, SERVICE1)) .isEqualTo(Status.INTERNAL.getCode()); - assertThat(policy.checkAuthorizationForService(OTHER_UID, SERVICE1).getCode()) + assertThat(checkAuthorizationForServiceAsync(policy, OTHER_UID, SERVICE1)) .isEqualTo(Status.INTERNAL.getCode()); // Uses the specified policy for service2. - assertThat(policy.checkAuthorizationForService(MY_UID, SERVICE2).getCode()) + assertThat(checkAuthorizationForServiceAsync(policy, MY_UID, SERVICE2)) .isEqualTo(Status.PERMISSION_DENIED.getCode()); - assertThat(policy.checkAuthorizationForService(OTHER_UID, SERVICE2).getCode()) + assertThat(checkAuthorizationForServiceAsync(policy, OTHER_UID, SERVICE2)) .isEqualTo(Status.OK.getCode()); // Falls back to the default. - assertThat(policy.checkAuthorizationForService(MY_UID, SERVICE3).getCode()) + assertThat(checkAuthorizationForServiceAsync(policy, MY_UID, SERVICE3)) .isEqualTo(Status.OK.getCode()); - assertThat(policy.checkAuthorizationForService(OTHER_UID, SERVICE3).getCode()) + assertThat(checkAuthorizationForServiceAsync(policy, OTHER_UID, SERVICE3)) .isEqualTo(Status.PERMISSION_DENIED.getCode()); } + /** + * Shortcut for invoking {@link ServerSecurityPolicy#checkAuthorizationForServiceAsync} without + * dealing with concurrency details. Returns a {link @Status.Code} for convenience. + */ + private static Status.Code checkAuthorizationForServiceAsync( + ServerSecurityPolicy policy, + int callerUid, + String service) throws ExecutionException { + ListenableFuture statusFuture = + policy.checkAuthorizationForServiceAsync(callerUid, service); + return Uninterruptibles.getUninterruptibly(statusFuture).getCode(); + } + private static SecurityPolicy policy(Function func) { return new SecurityPolicy() { @Override