do not catch exceptions from inner interceptors in SecurityInterceptor

fixes LogNet#231
jGleitz · Jul 14, 2021 · 1f227bc · 1f227bc
1 parent fe4650e
commit 1f227bc
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 22 deletions.
diff --git a/...arter-demo/src/test/java/org/lognet/springboot/grpc/security/SecurityInterceptorTest.java b/...arter-demo/src/test/java/org/lognet/springboot/grpc/security/SecurityInterceptorTest.java
@@ -7,14 +7,19 @@
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyZeroInteractions;
 import static org.mockito.Mockito.when;
 
 import java.util.Optional;
 
 import io.grpc.Metadata;
 import io.grpc.ServerCall;
+import io.grpc.ServerCallHandler;
 import io.grpc.Status;
 import io.grpc.StatusRuntimeException;
+import io.grpc.examples.custom.Custom.CustomReply;
+import io.grpc.examples.custom.Custom.CustomRequest;
+import io.grpc.examples.custom.CustomServiceGrpc;
 import org.junit.Test;
 import org.lognet.springboot.grpc.GRpcErrorHandler;
 
@@ -36,9 +41,32 @@ public void authSchemeSelectionException() {
         Metadata headers = new Metadata();
         headers.put(Metadata.Key.of("Authorization", Metadata.ASCII_STRING_MARSHALLER), "test");
 
-        assertThrows(StatusRuntimeException.class, () ->
-            securityInterceptor.interceptCall(mock(ServerCall.class), headers, (c, h) -> null)
+        assertThrows(
+            StatusRuntimeException.class,
+            () -> securityInterceptor.interceptCall(mock(ServerCall.class), headers, (c, h) -> null)
         );
         verify(errorHandler).handle(eq(Status.UNAUTHENTICATED), same(testException), same(headers), any());
     }
+
+    @SuppressWarnings("unchecked")
+    @Test
+    public void innerInterceptorException() {
+        GRpcErrorHandler errorHandler = spy(new GRpcErrorHandler());
+        SecurityInterceptor securityInterceptor =
+            new SecurityInterceptor(mock(GrpcSecurityMetadataSource.class), mock(AuthenticationSchemeSelector.class));
+        securityInterceptor.setErrorHandler(Optional.of(errorHandler));
+        securityInterceptor.setConfig(null);
+        Metadata headers = new Metadata();
+
+        ServerCallHandler<CustomRequest, CustomReply> throwingInnerInterceptor =
+            (c, h) -> {throw new IllegalStateException("inner exception");};
+        ServerCall<CustomRequest, CustomReply> call = mock(ServerCall.class);
+        when(call.getMethodDescriptor()).thenReturn(CustomServiceGrpc.getCustomMethod());
+
+        assertThrows(
+            IllegalStateException.class,
+            () -> securityInterceptor.interceptCall(call, headers, throwingInnerInterceptor)
+        );
+        verifyZeroInteractions(errorHandler);
+    }
 }
diff --git a/...g-boot-starter/src/main/java/org/lognet/springboot/grpc/security/SecurityInterceptor.java b/...g-boot-starter/src/main/java/org/lognet/springboot/grpc/security/SecurityInterceptor.java
@@ -76,33 +76,38 @@ public <ReqT, RespT> ServerCall.Listener<ReqT> interceptCall(
 
 
         final CharSequence authorization = Optional.ofNullable(headers.get(Metadata.Key.of("Authorization" + Metadata.BINARY_HEADER_SUFFIX, Metadata.BINARY_BYTE_MARSHALLER)))
-                .map(auth -> (CharSequence) StandardCharsets.UTF_8.decode(ByteBuffer.wrap(auth)))
-                .orElse(headers.get(Metadata.Key.of("Authorization", Metadata.ASCII_STRING_MARSHALLER)));
+            .map(auth -> (CharSequence) StandardCharsets.UTF_8.decode(ByteBuffer.wrap(auth)))
+            .orElse(headers.get(Metadata.Key.of("Authorization", Metadata.ASCII_STRING_MARSHALLER)));
 
         try {
-            final Authentication authentication = null == authorization ? null :
-                schemeSelector.getAuthScheme(authorization)
-                        .orElseThrow(() -> new RuntimeException("Can't get authentication from authorization header"));
-
-            SecurityContext context = SecurityContextHolder.createEmptyContext();
-            context.setAuthentication(authentication);
-            SecurityContextHolder.setContext(context);
-
-            beforeInvocation(call.getMethodDescriptor());
-
-            Context ctx = Context.current()
-                    .withValue(GrpcSecurity.AUTHENTICATION_CONTEXT_KEY, SecurityContextHolder.getContext().getAuthentication());
-
-            return Contexts.interceptCall(ctx, call, headers, next);
-        } catch (AccessDeniedException e) {
-            return fail(next, call, headers, Status.PERMISSION_DENIED, e);
-        } catch (Exception e) {
-            return fail(next, call, headers, Status.UNAUTHENTICATED, e);
+            final Context grpcSecurityContext;
+            try {
+                grpcSecurityContext = setupGRpcSecurityContext(call, authorization);
+            } catch (AccessDeniedException e) {
+                return fail(next, call, headers, Status.PERMISSION_DENIED, e);
+            } catch (Exception e) {
+                return fail(next, call, headers, Status.UNAUTHENTICATED, e);
+            }
+
+            return Contexts.interceptCall(grpcSecurityContext, call, headers, next);
         } finally {
             SecurityContextHolder.getContext().setAuthentication(null);
         }
+    }
+
+    private Context setupGRpcSecurityContext(ServerCall<?, ?> call, CharSequence authorization) {
+        final Authentication authentication = null == authorization ? null :
+            schemeSelector.getAuthScheme(authorization)
+                .orElseThrow(() -> new RuntimeException("Can't get authentication from authorization header"));
+
+        SecurityContext context = SecurityContextHolder.createEmptyContext();
+        context.setAuthentication(authentication);
+        SecurityContextHolder.setContext(context);
 
+        beforeInvocation(call.getMethodDescriptor());
 
+        return Context.current()
+            .withValue(GrpcSecurity.AUTHENTICATION_CONTEXT_KEY, SecurityContextHolder.getContext().getAuthentication());
     }
 
     private <RespT, ReqT> ServerCall.Listener<ReqT> fail(ServerCallHandler<ReqT, RespT> next, ServerCall<ReqT, RespT> call, Metadata headers,final Status status, Exception exception) {