From ddf171b4e1c5fd85f347fc24fd3bf1bd301cd9e2 Mon Sep 17 00:00:00 2001 From: AgraVator Date: Mon, 13 Jul 2026 15:04:31 +0530 Subject: [PATCH] binder: describe security policy evaluation failures uniformly as 'Authorization future failed' --- .../io/grpc/binder/ServerSecurityPolicy.java | 7 ++-- .../internal/BinderTransportSecurity.java | 6 +-- .../binder/RobolectricBinderSecurityTest.java | 23 ++++++++++- .../grpc/binder/ServerSecurityPolicyTest.java | 38 +++++++++++++++++++ 4 files changed, 63 insertions(+), 11 deletions(-) diff --git a/binder/src/main/java/io/grpc/binder/ServerSecurityPolicy.java b/binder/src/main/java/io/grpc/binder/ServerSecurityPolicy.java index 4786a5e6cc4..500e5527c79 100644 --- a/binder/src/main/java/io/grpc/binder/ServerSecurityPolicy.java +++ b/binder/src/main/java/io/grpc/binder/ServerSecurityPolicy.java @@ -72,11 +72,10 @@ public Status checkAuthorizationForService(int uid, String serviceName) { @CheckReturnValue ListenableFuture checkAuthorizationForServiceAsync(int uid, String serviceName) { SecurityPolicy securityPolicy = perServicePolicies.getOrDefault(serviceName, defaultPolicy); - if (securityPolicy instanceof AsyncSecurityPolicy) { - return ((AsyncSecurityPolicy) securityPolicy).checkAuthorizationAsync(uid); - } - try { + if (securityPolicy instanceof AsyncSecurityPolicy) { + return ((AsyncSecurityPolicy) securityPolicy).checkAuthorizationAsync(uid); + } Status status = securityPolicy.checkAuthorization(uid); return Futures.immediateFuture(status); } catch (Exception e) { 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 6f95ef8a83c..1a84a4ed225 100644 --- a/binder/src/main/java/io/grpc/binder/internal/BinderTransportSecurity.java +++ b/binder/src/main/java/io/grpc/binder/internal/BinderTransportSecurity.java @@ -110,11 +110,7 @@ public ServerCall.Listener interceptCall( authStatus = Futures.getDone(authStatusFuture); } catch (ExecutionException | CancellationException e) { // Failed futures are treated as an internal error rather than a security rejection. - authStatus = Status.INTERNAL.withCause(e); - @Nullable String message = e.getMessage(); - if (message != null) { - authStatus = authStatus.withDescription(message); - } + authStatus = Status.INTERNAL.withCause(e).withDescription("Authorization future failed"); } if (authStatus.isOk()) { diff --git a/binder/src/test/java/io/grpc/binder/RobolectricBinderSecurityTest.java b/binder/src/test/java/io/grpc/binder/RobolectricBinderSecurityTest.java index ffd1d89e69c..571542075af 100644 --- a/binder/src/test/java/io/grpc/binder/RobolectricBinderSecurityTest.java +++ b/binder/src/test/java/io/grpc/binder/RobolectricBinderSecurityTest.java @@ -161,10 +161,29 @@ public void testAsyncServerSecurityPolicy_failed_returnsFailureStatus() throws E @Test public void testAsyncServerSecurityPolicy_failedFuture_failsWithCodeInternal() throws Exception { - ListenableFuture status = makeCall(); + ListenableFuture status1 = makeCall(); statusesToSet.take().setException(new IllegalStateException("oops")); - assertThat(status.get().getCode()).isEqualTo(Status.Code.INTERNAL); + Status result1 = status1.get(); + assertThat(result1.getCode()).isEqualTo(Status.Code.INTERNAL); + assertThat(result1.getDescription()).isEqualTo("Authorization future failed"); + + // Subsequent call on the same channel should also get uniform opaque failure description + ListenableFuture status2 = makeCall(); + Status result2 = status2.get(); + assertThat(result2.getCode()).isEqualTo(Status.Code.INTERNAL); + assertThat(result2.getDescription()).isEqualTo("Authorization future failed"); + } + + @Test + public void testAsyncServerSecurityPolicy_cancelledFuture_failsWithUniformOpaqueDescription() + throws Exception { + ListenableFuture status1 = makeCall(); + statusesToSet.take().cancel(true); + + Status result1 = status1.get(); + assertThat(result1.getCode()).isEqualTo(Status.Code.INTERNAL); + assertThat(result1.getDescription()).isEqualTo("Authorization future failed"); } @Test diff --git a/binder/src/test/java/io/grpc/binder/ServerSecurityPolicyTest.java b/binder/src/test/java/io/grpc/binder/ServerSecurityPolicyTest.java index eedc3f590cd..7ca1bf156ab 100644 --- a/binder/src/test/java/io/grpc/binder/ServerSecurityPolicyTest.java +++ b/binder/src/test/java/io/grpc/binder/ServerSecurityPolicyTest.java @@ -333,6 +333,44 @@ public void testPerServiceNoDefaultAsync() throws Exception { .isEqualTo(Status.PERMISSION_DENIED.getCode()); } + @Test + public void testAsyncSecurityPolicy_throwsExceptionSynchronously_returnsFailedFuture() { + policy = + ServerSecurityPolicy.newBuilder() + .servicePolicy( + SERVICE1, + asyncPolicy( + uid -> { + throw new IllegalStateException("Sync policy error"); + })) + .build(); + + ListenableFuture future = policy.checkAuthorizationForServiceAsync(MY_UID, SERVICE1); + assertThat(future.isDone()).isTrue(); + ExecutionException thrown = assertThrows(ExecutionException.class, future::get); + assertThat(thrown.getCause()).isInstanceOf(IllegalStateException.class); + assertThat(thrown.getCause().getMessage()).isEqualTo("Sync policy error"); + } + + @Test + public void testSecurityPolicy_throwsExceptionSynchronously_returnsFailedFuture() { + policy = + ServerSecurityPolicy.newBuilder() + .servicePolicy( + SERVICE1, + policy( + uid -> { + throw new IllegalArgumentException("Sync standard policy error"); + })) + .build(); + + ListenableFuture future = policy.checkAuthorizationForServiceAsync(MY_UID, SERVICE1); + assertThat(future.isDone()).isTrue(); + ExecutionException thrown = assertThrows(ExecutionException.class, future::get); + assertThat(thrown.getCause()).isInstanceOf(IllegalArgumentException.class); + assertThat(thrown.getCause().getMessage()).isEqualTo("Sync standard policy error"); + } + /** * Shortcut for invoking {@link ServerSecurityPolicy#checkAuthorizationForServiceAsync} without * dealing with concurrency details. Returns a {link @Status.Code} for convenience.