From 6cf4a74384befa06d08ec6e9a011b4ea473db01a Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Thu, 23 Jan 2025 13:39:38 +0000 Subject: [PATCH] Reorganize interceptor --- .../sample/GrpcServerApplicationTests.java | 2 +- ... => AuthenticationProcessInterceptor.java} | 61 +++++-------------- .../grpc/server/security/GrpcSecurity.java | 8 +-- .../server/security/HttpBasicConfigurer.java | 4 +- .../server/security/PreAuthConfigurer.java | 4 +- .../security/RequestMapperConfigurer.java | 9 +-- 6 files changed, 29 insertions(+), 59 deletions(-) rename spring-grpc-core/src/main/java/org/springframework/grpc/server/security/{AuthenticationServerInterceptor.java => AuthenticationProcessInterceptor.java} (62%) diff --git a/samples/grpc-secure/src/test/java/org/springframework/grpc/sample/GrpcServerApplicationTests.java b/samples/grpc-secure/src/test/java/org/springframework/grpc/sample/GrpcServerApplicationTests.java index 286efb7..913ecbf 100644 --- a/samples/grpc-secure/src/test/java/org/springframework/grpc/sample/GrpcServerApplicationTests.java +++ b/samples/grpc-secure/src/test/java/org/springframework/grpc/sample/GrpcServerApplicationTests.java @@ -151,7 +151,7 @@ public class GrpcServerApplicationTests { @Lazy SimpleGrpc.SimpleBlockingStub basic(GrpcChannelFactory channels) { return SimpleGrpc.newBlockingStub(channels.createChannel("basic", ChannelBuilderOptions.defaults() - .withInterceptors(List.of(new BasicAuthenticationInterceptor("user", "user"))))); + .withInterceptors(List.of(new BasicAuthenticationInterceptor("user", "user"))))); } @Bean diff --git a/spring-grpc-core/src/main/java/org/springframework/grpc/server/security/AuthenticationServerInterceptor.java b/spring-grpc-core/src/main/java/org/springframework/grpc/server/security/AuthenticationProcessInterceptor.java similarity index 62% rename from spring-grpc-core/src/main/java/org/springframework/grpc/server/security/AuthenticationServerInterceptor.java rename to spring-grpc-core/src/main/java/org/springframework/grpc/server/security/AuthenticationProcessInterceptor.java index 16ec5e7..cc25574 100644 --- a/spring-grpc-core/src/main/java/org/springframework/grpc/server/security/AuthenticationServerInterceptor.java +++ b/spring-grpc-core/src/main/java/org/springframework/grpc/server/security/AuthenticationProcessInterceptor.java @@ -26,7 +26,6 @@ import org.springframework.security.core.authority.AuthorityUtils; import org.springframework.security.core.context.SecurityContext; import org.springframework.security.core.context.SecurityContextHolder; -import io.grpc.ForwardingServerCallListener; import io.grpc.Metadata; import io.grpc.ServerCall; import io.grpc.ServerCall.Listener; @@ -37,11 +36,11 @@ import io.grpc.ServerInterceptor; * An interceptor that extracts the authentication credentials from the gRPC request * headers and metadata, authenticates the user, and sets the authentication in the * SecurityContext. This interceptor should be registered with the gRPC server to handle - * authentication. + * authentication and authorization for gRPC requests. * * @author Dave Syer */ -public class AuthenticationServerInterceptor implements ServerInterceptor, Ordered { +public class AuthenticationProcessInterceptor implements ServerInterceptor, Ordered { private final AuthenticationManager authenticationManager; @@ -54,7 +53,7 @@ public class AuthenticationServerInterceptor implements ServerInterceptor, Order return GrpcSecurity.CONTEXT_FILTER_ORDER - 10; } - public AuthenticationServerInterceptor(AuthenticationManager authenticationManager, + public AuthenticationProcessInterceptor(AuthenticationManager authenticationManager, GrpcAuthenticationExtractor extractor, AuthorizationManager authorizationManager) { this.authenticationManager = authenticationManager; this.extractor = extractor; @@ -72,56 +71,26 @@ public class AuthenticationServerInterceptor implements ServerInterceptor, Order } if (this.authorizationManager != null) { + CallContext context = new CallContext(headers, call.getAttributes(), call.getMethodDescriptor()); if (user == null) { + // Maybe just throw BadCredentialsException (authentication manager would + // have to make the anonymous user)? user = new AnonymousAuthenticationToken("anonymous", "anonymous", AuthorityUtils.createAuthorityList("ROLE_ANONYMOUS")); } - return new AuthenticatedListener(next.startCall(call, headers), this.authorizationManager, - new CallContext(headers, call.getAttributes(), call.getMethodDescriptor()), user); - } - return new AuthenticatedListener(next.startCall(call, headers), null, - new CallContext(headers, call.getAttributes(), call.getMethodDescriptor()), user); - } - - static class AuthenticatedListener extends ForwardingServerCallListener { - - private final Listener delegate; - - private final CallContext context; - - private final Authentication authentication; - - private final AuthorizationManager authorizationManager; - - AuthenticatedListener(io.grpc.ServerCall.Listener delegate, - AuthorizationManager authorizationManager, CallContext context, Authentication user) { - this.delegate = delegate; - this.authorizationManager = authorizationManager; - this.context = context; - this.authentication = user; - } - - @Override - public void onReady() { - if (this.authentication == null || !this.authentication.isAuthenticated()) { - throw new BadCredentialsException("not authenticated"); - } - else { - if (!this.authorizationManager.authorize(() -> this.authentication, this.context).isGranted()) { - if (this.authentication instanceof AnonymousAuthenticationToken) { - throw new BadCredentialsException("not authenticated"); - } - throw new AccessDeniedException("not allowed"); + Authentication authentication = user; + if (!this.authorizationManager.authorize(() -> authentication, context).isGranted()) { + if (user instanceof AnonymousAuthenticationToken) { + throw new BadCredentialsException("not authenticated"); } + throw new AccessDeniedException("not allowed"); } - super.onReady(); - } - - @Override - protected Listener delegate() { - return this.delegate; + } + else if (user == null || !user.isAuthenticated()) { + throw new BadCredentialsException("not authenticated"); } + return next.startCall(call, headers); } } diff --git a/spring-grpc-core/src/main/java/org/springframework/grpc/server/security/GrpcSecurity.java b/spring-grpc-core/src/main/java/org/springframework/grpc/server/security/GrpcSecurity.java index 78f5fd2..989d991 100644 --- a/spring-grpc-core/src/main/java/org/springframework/grpc/server/security/GrpcSecurity.java +++ b/spring-grpc-core/src/main/java/org/springframework/grpc/server/security/GrpcSecurity.java @@ -52,7 +52,7 @@ import io.micrometer.observation.ObservationRegistry; * @author Dave Syer */ public final class GrpcSecurity - extends AbstractConfiguredSecurityBuilder { + extends AbstractConfiguredSecurityBuilder { /** * A constant key used for storing and retrieving the "Authorization" header from gRPC @@ -100,7 +100,7 @@ public final class GrpcSecurity } @Override - protected AuthenticationServerInterceptor performBuild() throws Exception { + protected AuthenticationProcessInterceptor performBuild() throws Exception { if (this.authenticationManager != null) { setSharedObject(AuthenticationManager.class, this.authenticationManager); } @@ -115,7 +115,7 @@ public final class GrpcSecurity } } this.authenticationExtractors.sort(AnnotationAwareOrderComparator.INSTANCE); - return new AuthenticationServerInterceptor(getSharedObject(AuthenticationManager.class), + return new AuthenticationProcessInterceptor(getSharedObject(AuthenticationManager.class), new CompositeAuthenticationExtractor(this.authenticationExtractors), this.authorizationManager); } @@ -147,7 +147,7 @@ public final class GrpcSecurity } @SuppressWarnings({ "unchecked", "removal" }) - private > C getOrApply( + private > C getOrApply( C configurer) throws Exception { C existingConfig = (C) getConfigurer(configurer.getClass()); if (existingConfig != null) { diff --git a/spring-grpc-core/src/main/java/org/springframework/grpc/server/security/HttpBasicConfigurer.java b/spring-grpc-core/src/main/java/org/springframework/grpc/server/security/HttpBasicConfigurer.java index 15c9189..0b21a1d 100644 --- a/spring-grpc-core/src/main/java/org/springframework/grpc/server/security/HttpBasicConfigurer.java +++ b/spring-grpc-core/src/main/java/org/springframework/grpc/server/security/HttpBasicConfigurer.java @@ -22,8 +22,8 @@ import org.springframework.security.config.annotation.SecurityConfigurerAdapter; import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder; import org.springframework.security.core.userdetails.UserDetailsService; -public final class HttpBasicConfigurer> - extends SecurityConfigurerAdapter { +public final class HttpBasicConfigurer> + extends SecurityConfigurerAdapter { private final ApplicationContext context; diff --git a/spring-grpc-core/src/main/java/org/springframework/grpc/server/security/PreAuthConfigurer.java b/spring-grpc-core/src/main/java/org/springframework/grpc/server/security/PreAuthConfigurer.java index 208b86b..678c2e6 100644 --- a/spring-grpc-core/src/main/java/org/springframework/grpc/server/security/PreAuthConfigurer.java +++ b/spring-grpc-core/src/main/java/org/springframework/grpc/server/security/PreAuthConfigurer.java @@ -24,8 +24,8 @@ import org.springframework.security.core.userdetails.UserDetailsService; import org.springframework.security.web.authentication.preauth.PreAuthenticatedAuthenticationProvider; import org.springframework.security.web.authentication.preauth.PreAuthenticatedAuthenticationToken; -public final class PreAuthConfigurer> - extends SecurityConfigurerAdapter { +public final class PreAuthConfigurer> + extends SecurityConfigurerAdapter { private final ApplicationContext context; diff --git a/spring-grpc-core/src/main/java/org/springframework/grpc/server/security/RequestMapperConfigurer.java b/spring-grpc-core/src/main/java/org/springframework/grpc/server/security/RequestMapperConfigurer.java index d9662ae..615ef43 100644 --- a/spring-grpc-core/src/main/java/org/springframework/grpc/server/security/RequestMapperConfigurer.java +++ b/spring-grpc-core/src/main/java/org/springframework/grpc/server/security/RequestMapperConfigurer.java @@ -35,7 +35,7 @@ import org.springframework.util.Assert; import org.springframework.util.PatternMatchUtils; import org.springframework.util.function.SingletonSupplier; -public class RequestMapperConfigurer extends SecurityConfigurerAdapter { +public class RequestMapperConfigurer extends SecurityConfigurerAdapter { private List authorizedCalls = new ArrayList<>(); @@ -46,12 +46,12 @@ public class RequestMapperConfigurer extends SecurityConfigurerAdapter 0) { this.publisher = context.getBean(AuthorizationEventPublisher.class); - } else { + } + else { this.publisher = new SpringAuthorizationEventPublisher(context); } this.roleHierarchy = SingletonSupplier.of(() -> (context.getBeanNamesForType(RoleHierarchy.class).length > 0) - ? context.getBean(RoleHierarchy.class) - : new NullRoleHierarchy()); + ? context.getBean(RoleHierarchy.class) : new NullRoleHierarchy()); } @Override @@ -150,6 +150,7 @@ public class RequestMapperConfigurer extends SecurityConfigurerAdapter { private final List authorizedCalls; + private final AuthorizationEventPublisher publisher; public RequestMapperAuthorizationManager(List authorizedCalls,