diff --git a/binder/src/androidTest/java/io/grpc/binder/BinderSecurityTest.java b/binder/src/androidTest/java/io/grpc/binder/BinderSecurityTest.java index 56e8f93c14..27fc293e2a 100644 --- a/binder/src/androidTest/java/io/grpc/binder/BinderSecurityTest.java +++ b/binder/src/androidTest/java/io/grpc/binder/BinderSecurityTest.java @@ -29,9 +29,12 @@ import com.google.common.base.Function; import com.google.protobuf.Empty; import io.grpc.CallOptions; import io.grpc.ManagedChannel; +import io.grpc.Metadata; import io.grpc.MethodDescriptor; import io.grpc.Server; +import io.grpc.ServerCall; import io.grpc.ServerCallHandler; +import io.grpc.ServerInterceptor; import io.grpc.ServerServiceDefinition; import io.grpc.Status; import io.grpc.StatusRuntimeException; @@ -59,6 +62,7 @@ public final class BinderSecurityTest { @Nullable ManagedChannel channel; Map> methods = new HashMap<>(); List> calls = new ArrayList<>(); + CountingServerInterceptor countingServerInterceptor; @Before public void setupServiceDefinitionsAndMethods() { @@ -86,6 +90,7 @@ public final class BinderSecurityTest { } serviceDefinitions.add(builder.build()); } + countingServerInterceptor = new CountingServerInterceptor(); } @After @@ -120,6 +125,7 @@ public final class BinderSecurityTest { ServerSecurityPolicy serverPolicy) { BinderServerBuilder serverBuilder = BinderServerBuilder.forAddress(listenAddr, receiver); serverBuilder.securityPolicy(serverPolicy); + serverBuilder.intercept(countingServerInterceptor); for (ServerServiceDefinition serviceDefinition : serviceDefinitions) { serverBuilder.addService(serviceDefinition); @@ -195,6 +201,27 @@ public final class BinderSecurityTest { } } + @Test + public void testSecurityInterceptorIsClosestToTransport() throws Exception { + createChannel( + ServerSecurityPolicy.newBuilder() + .servicePolicy("foo", policy((uid) -> true)) + .servicePolicy("bar", policy((uid) -> false)) + .servicePolicy("baz", policy((uid) -> false)) + .build(), + SecurityPolicies.internalOnly()); + assertThat(countingServerInterceptor.numInterceptedCalls).isEqualTo(0); + for (MethodDescriptor method : methods.values()) { + try { + ClientCalls.blockingUnaryCall(channel, method, CallOptions.DEFAULT, null); + } catch (StatusRuntimeException sre) { + // Ignore. + } + } + // Only the foo calls should have made it to the user interceptor. + assertThat(countingServerInterceptor.numInterceptedCalls).isEqualTo(2); + } + private static SecurityPolicy policy(Function func) { return new SecurityPolicy() { @Override @@ -203,4 +230,17 @@ public final class BinderSecurityTest { } }; } + + private final class CountingServerInterceptor implements ServerInterceptor { + int numInterceptedCalls; + + @Override + public ServerCall.Listener interceptCall( + ServerCall call, + Metadata headers, + ServerCallHandler next) { + numInterceptedCalls += 1; + return next.startCall(call, headers); + } + } } diff --git a/binder/src/main/java/io/grpc/binder/BinderServerBuilder.java b/binder/src/main/java/io/grpc/binder/BinderServerBuilder.java index 383bd3f8e4..c4674a76bb 100644 --- a/binder/src/main/java/io/grpc/binder/BinderServerBuilder.java +++ b/binder/src/main/java/io/grpc/binder/BinderServerBuilder.java @@ -81,6 +81,7 @@ public final class BinderServerBuilder SharedResourcePool.forResource(GrpcUtil.TIMER_SERVICE); private ServerSecurityPolicy securityPolicy; private InboundParcelablePolicy inboundParcelablePolicy; + private boolean isBuilt; private BinderServerBuilder( AndroidComponentAddress listenAddress, @@ -107,8 +108,6 @@ public final class BinderServerBuilder // Disable stats and tracing by default. serverImplBuilder.setStatsEnabled(false); serverImplBuilder.setTracingEnabled(false); - - BinderTransportSecurity.installAuthInterceptor(this); } @Override @@ -177,6 +176,11 @@ public final class BinderServerBuilder */ @Override // For javadoc refinement only. public Server build() { + // Since we install a final interceptor here, we need to ensure we're only built once. + 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); return super.build(); } }