Merge pull request #594 from yidongnan/feature/security-api-refactor

Improve authentication capabilities

Author: yidongnan

grpc-server-spring-boot-autoconfigure/src/main/java/net/devh/boot/grpc/server/security/authentication/GrpcAuthenticationReader.java

@@ -26,12 +26,17 @@
 
 import io.grpc.Metadata;
 import io.grpc.ServerCall;
+import io.grpc.Status;
 
 /**
  * Reads the authentication data from the given {@link ServerCall} and {@link Metadata}. The returned
  * {@link Authentication} is not yet validated and needs to be passed to an {@link AuthenticationManager}.
  *
  * <p>
+ * This is similar to the {@code org.springframework.security.web.authentication.AuthenticationConverter}.
+ * </p>
+ *
+ * <p>
  * <b>Note:</b> The authentication manager needs a corresponding {@link AuthenticationProvider} to actually verify the
  * {@link Authentication}.
  * </p>
@@ -47,7 +52,9 @@ public interface GrpcAuthenticationReader {
      * <p>
      * <b>Note:</b> Implementations are free to throw an {@link AuthenticationException} if no credentials could be
      * found in the call. If an exception is thrown by an implementation then the authentication attempt should be
-     * considered as failed and no subsequent {@link GrpcAuthenticationReader}s should be called.
+     * considered as failed and no subsequent {@link GrpcAuthenticationReader}s should be called. Additionally, the call
+     * will fail as {@link Status#UNAUTHENTICATED}. If the call instead returns {@code null}, then the call processing
+     * will proceed unauthenticated.
      * </p>
      *
      * @param call The call to get that send the request.

grpc-server-spring-boot-autoconfigure/src/main/java/net/devh/boot/grpc/server/security/interceptors/AuthenticatingServerInterceptor.java

@@ -18,6 +18,7 @@
 package net.devh.boot.grpc.server.security.interceptors;
 
 import org.springframework.security.core.Authentication;
+import org.springframework.security.core.context.SecurityContext;
 
 import io.grpc.Context;
 import io.grpc.Contexts;
@@ -46,6 +47,14 @@ public interface AuthenticatingServerInterceptor extends ServerInterceptor {
     /**
      * The context key that can be used to retrieve the associated {@link Authentication}.
      */
-    public static final Context.Key<Authentication> AUTHENTICATION_CONTEXT_KEY = Context.key("authentication");
+    Context.Key<SecurityContext> SECURITY_CONTEXT_KEY = Context.key("security-context");
+
+    /**
+     * The context key that can be used to retrieve the originally associated {@link Authentication}.
+     *
+     * @deprecated Use {@link #SECURITY_CONTEXT_KEY} instead.
+     */
+    @Deprecated
+    Context.Key<Authentication> AUTHENTICATION_CONTEXT_KEY = Context.key("authentication");
 
 }

grpc-server-spring-boot-autoconfigure/src/main/java/net/devh/boot/grpc/server/security/interceptors/DefaultAuthenticatingServerInterceptor.java

@@ -28,6 +28,7 @@
 import org.springframework.security.authentication.BadCredentialsException;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.AuthenticationException;
+import org.springframework.security.core.context.SecurityContext;
 import org.springframework.security.core.context.SecurityContextHolder;
 
 import io.grpc.Context;
@@ -47,6 +48,10 @@
  * authentication to both grpc's {@link Context} and {@link SecurityContextHolder}.
  *
  * <p>
+ * This works similar to the {@code org.springframework.security.web.authentication.AuthenticationFilter}.
+ * </p>
+ *
+ * <p>
  * <b>Note:</b> This interceptor works similar to
  * {@link Contexts#interceptCall(Context, ServerCall, Metadata, ServerCallHandler)}.
  * </p>
@@ -89,7 +94,7 @@ public <ReqT, RespT> ServerCall.Listener<ReqT> interceptCall(final ServerCall<Re
             try {
                 return next.startCall(call, headers);
             } catch (final AccessDeniedException e) {
-                throw new BadCredentialsException("No credentials found in the request", e);
+                throw newNoCredentialsException(e);
             }
         }
         if (authentication.getDetails() == null && authentication instanceof AbstractAuthenticationToken) {
@@ -103,29 +108,101 @@ public <ReqT, RespT> ServerCall.Listener<ReqT> interceptCall(final ServerCall<Re
             authentication = this.authenticationManager.authenticate(authentication);
         } catch (final AuthenticationException e) {
             log.debug("Authentication request failed: {}", e.getMessage());
+            onUnsuccessfulAuthentication(call, headers, e);
             throw e;
         }
 
-        final Context context = Context.current().withValue(AUTHENTICATION_CONTEXT_KEY, authentication);
-        final Context previousContext = context.attach();
-        SecurityContextHolder.getContext().setAuthentication(authentication);
+        final SecurityContext securityContext = SecurityContextHolder.createEmptyContext();
+        securityContext.setAuthentication(authentication);
+        SecurityContextHolder.setContext(securityContext);
+        @SuppressWarnings("deprecation")
+        final Context grpcContext = Context.current().withValues(
+                SECURITY_CONTEXT_KEY, securityContext,
+                AUTHENTICATION_CONTEXT_KEY, authentication);
+        final Context previousContext = grpcContext.attach();
         log.debug("Authentication successful: Continuing as {} ({})", authentication.getName(),
                 authentication.getAuthorities());
+        onSuccessfulAuthentication(call, headers, authentication);
         try {
-            return new AuthenticatingServerCallListener<>(next.startCall(call, headers), context, authentication);
+            return new AuthenticatingServerCallListener<>(next.startCall(call, headers), grpcContext, securityContext);
         } catch (final AccessDeniedException e) {
             if (authentication instanceof AnonymousAuthenticationToken) {
-                throw new BadCredentialsException("No credentials found in the request", e);
+                throw newNoCredentialsException(e);
             } else {
                 throw e;
             }
         } finally {
             SecurityContextHolder.clearContext();
-            context.detach(previousContext);
+            grpcContext.detach(previousContext);
             log.debug("startCall - Authentication cleared");
         }
     }
 
+    /**
+     * Hook that will be called on successful authentication. Implementations may only use the call instance in a
+     * non-disruptive manor, that is accessing call attributes or the call descriptor. Implementations must not pollute
+     * the current thread/context with any call-related state, including authentication, beyond the duration of the
+     * method invocation. At the time of calling both the grpc context and the security context have been updated to
+     * reflect the state of the authentication and thus don't have to be setup manually.
+     *
+     * <p>
+     * <b>Note:</b> This method is called regardless of whether the authenticated user is authorized or not to perform
+     * the requested action.
+     * </p>
+     *
+     * <p>
+     * By default, this method does nothing.
+     * </p>
+     *
+     * @param call The call instance to receive response messages.
+     * @param headers The headers associated with the call.
+     * @param authentication The successful authentication instance.
+     */
+    protected void onSuccessfulAuthentication(
+            final ServerCall<?, ?> call,
+            final Metadata headers,
+            final Authentication authentication) {
+        // Overwrite to add custom behavior.
+    }
+
+    /**
+     * Hook that will be called on unsuccessful authentication. Implementations must use the call instance only in a
+     * non-disruptive manner, i.e. to access call attributes or the call descriptor. Implementations must not close the
+     * call and must not pollute the current thread/context with any call-related state, including authentication,
+     * beyond the duration of the method invocation.
+     *
+     * <p>
+     * <b>Note:</b> This method is called only if the request contains an authentication but the
+     * {@link AuthenticationManager} considers it invalid. This method is not called if an authenticated user is not
+     * authorized to perform the requested action.
+     * </p>
+     *
+     * <p>
+     * By default, this method does nothing.
+     * </p>
+     *
+     * @param call The call instance to receive response messages.
+     * @param headers The headers associated with the call.
+     * @param failed The exception related to the unsuccessful authentication.
+     */
+    protected void onUnsuccessfulAuthentication(
+            final ServerCall<?, ?> call,
+            final Metadata headers,
+            final AuthenticationException failed) {
+        // Overwrite to add custom behavior.
+    }
+
+    /**
+     * Wraps the given {@link AccessDeniedException} in an {@link AuthenticationException} to reflect, that no
+     * authentication was originally present in the request.
+     *
+     * @param denied The caught exception.
+     * @return The newly created {@link AuthenticationException}.
+     */
+    private static AuthenticationException newNoCredentialsException(final AccessDeniedException denied) {
+        return new BadCredentialsException("No credentials found in the request", denied);
+    }
+
     /**
      * A call listener that will set the authentication context using {@link SecurityContextHolder} before each
      * invocation and clear it afterwards.
@@ -134,25 +211,25 @@ public <ReqT, RespT> ServerCall.Listener<ReqT> interceptCall(final ServerCall<Re
      */
     private static class AuthenticatingServerCallListener<ReqT> extends AbstractAuthenticatingServerCallListener<ReqT> {
 
-        private final Authentication authentication;
+        private final SecurityContext securityContext;
 
         /**
          * Creates a new AuthenticatingServerCallListener which will attach the given security context before delegating
          * to the given listener.
          *
          * @param delegate The listener to delegate to.
-         * @param context The context to attach.
-         * @param authentication The authentication instance to attach.
+         * @param grpcContext The context to attach.
+         * @param securityContext The security context instance to attach.
          */
-        public AuthenticatingServerCallListener(final Listener<ReqT> delegate, final Context context,
-                final Authentication authentication) {
-            super(delegate, context);
-            this.authentication = authentication;
+        public AuthenticatingServerCallListener(final Listener<ReqT> delegate, final Context grpcContext,
+                final SecurityContext securityContext) {
+            super(delegate, grpcContext);
+            this.securityContext = securityContext;
         }
 
         @Override
         protected void attachAuthenticationContext() {
-            SecurityContextHolder.getContext().setAuthentication(this.authentication);
+            SecurityContextHolder.setContext(this.securityContext);
         }
 
         @Override
@@ -165,8 +242,8 @@ public void onHalfClose() {
             try {
                 super.onHalfClose();
             } catch (final AccessDeniedException e) {
-                if (this.authentication instanceof AnonymousAuthenticationToken) {
-                    throw new BadCredentialsException("No credentials found in the request", e);
+                if (this.securityContext.getAuthentication() instanceof AnonymousAuthenticationToken) {
+                    throw newNoCredentialsException(e);
                 } else {
                     throw e;
                 }

tests/src/test/java/net/devh/boot/grpc/test/interceptor/DefaultServerInterceptorTest.java

@@ -48,7 +48,7 @@
         ManualSecurityConfiguration.class})
 @EnableAutoConfiguration
 @DirtiesContext
-public class DefaultServerInterceptorTest {
+class DefaultServerInterceptorTest {
 
     @Autowired
     private ApplicationContext applicationContext;

tests/src/test/java/net/devh/boot/grpc/test/server/TestServiceImpl.java

@@ -184,7 +184,7 @@ protected void assertSameAuthenticatedGrcContextCancellation(final String method
     protected Authentication assertSameAuthenticatedGrcContextOnly(final String method, final Authentication expected,
             final Context context) {
         return assertSameAuthenticated(method, expected,
-                AuthenticatingServerInterceptor.AUTHENTICATION_CONTEXT_KEY.get(context));
+                AuthenticatingServerInterceptor.SECURITY_CONTEXT_KEY.get(context).getAuthentication());
     }
 
     protected Authentication assertSameAuthenticated(final String method, final Authentication expected) {