From e4fa0f83df038a596b34253bd6b3af3f6b200b3e Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Mon, 24 Mar 2025 10:09:43 +0000 Subject: [PATCH] Check if csrf protection is already disabled If the user has explicitly disabled CSRF protection in a custom SecurityFilterChain, we should be able to back off and not try to disable it just for the gRPC endpoints (accidentally switching it back on for the other endpoints). Fixes gh-142 --- .../src/main/resources/application.properties | 3 +- .../sample/CsrfDisabledApplicationTests.java | 110 ++++++++++++++++++ .../GrpcDisableCsrfHttpConfigurer.java | 20 +++- .../server/security/GrpcServletRequest.java | 6 + .../security/GrpcServletRequestTests.java | 16 +++ 5 files changed, 152 insertions(+), 3 deletions(-) create mode 100644 samples/grpc-tomcat-secure/src/test/java/org/springframework/grpc/sample/CsrfDisabledApplicationTests.java diff --git a/samples/grpc-tomcat-secure/src/main/resources/application.properties b/samples/grpc-tomcat-secure/src/main/resources/application.properties index 21ac05f..5e57f1d 100644 --- a/samples/grpc-tomcat-secure/src/main/resources/application.properties +++ b/samples/grpc-tomcat-secure/src/main/resources/application.properties @@ -1,4 +1,5 @@ spring.application.name=grpc-tomcat-secure server.port=9090 spring.security.user.name=user -spring.security.user.password=user \ No newline at end of file +spring.security.user.password=user +#logging.level.org.springframework.security=TRACE \ No newline at end of file diff --git a/samples/grpc-tomcat-secure/src/test/java/org/springframework/grpc/sample/CsrfDisabledApplicationTests.java b/samples/grpc-tomcat-secure/src/test/java/org/springframework/grpc/sample/CsrfDisabledApplicationTests.java new file mode 100644 index 0000000..5584c60 --- /dev/null +++ b/samples/grpc-tomcat-secure/src/test/java/org/springframework/grpc/sample/CsrfDisabledApplicationTests.java @@ -0,0 +1,110 @@ +package org.springframework.grpc.sample; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.springframework.security.config.Customizer.withDefaults; + +import java.util.List; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.boot.builder.SpringApplicationBuilder; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.context.TestConfiguration; +import org.springframework.boot.test.web.server.LocalServerPort; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Lazy; +import org.springframework.grpc.client.ChannelBuilderOptions; +import org.springframework.grpc.client.GrpcChannelFactory; +import org.springframework.grpc.client.interceptor.security.BasicAuthenticationInterceptor; +import org.springframework.grpc.sample.proto.HelloReply; +import org.springframework.grpc.sample.proto.HelloRequest; +import org.springframework.grpc.sample.proto.SimpleGrpc; +import org.springframework.security.config.annotation.web.builders.HttpSecurity; +import org.springframework.security.config.annotation.web.configurers.CsrfConfigurer; +import org.springframework.security.web.SecurityFilterChain; +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.web.bind.annotation.PostMapping; +import org.springframework.web.bind.annotation.RequestBody; +import org.springframework.web.bind.annotation.RestController; + +import io.grpc.Status.Code; +import io.grpc.StatusRuntimeException; + +@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT, + properties = "spring.grpc.client.default-channel.address=0.0.0.0:${local.server.port}") +public class CsrfDisabledApplicationTests { + + private static Log log = LogFactory.getLog(CsrfDisabledApplicationTests.class); + + public static void main(String[] args) { + new SpringApplicationBuilder(GrpcServerApplication.class, ExtraConfiguration.class).run(args); + } + + @Autowired + @Qualifier("simpleBlockingStub") + private SimpleGrpc.SimpleBlockingStub stub; + + @Autowired + @Qualifier("basic") + private SimpleGrpc.SimpleBlockingStub basic; + + @Test + @DirtiesContext + void contextLoads() { + } + + @Test + @DirtiesContext + void unauthenticated() { + StatusRuntimeException exception = assertThrows(StatusRuntimeException.class, + () -> stub.sayHello(HelloRequest.newBuilder().setName("Alien").build())); + assertEquals(Code.UNAUTHENTICATED, exception.getStatus().getCode()); + } + + @Test + @DirtiesContext + void authenticated() { + log.info("Testing"); + HelloReply response = basic.sayHello(HelloRequest.newBuilder().setName("Alien").build()); + assertEquals("Hello ==> Alien", response.getMessage()); + } + + @TestConfiguration + @RestController + static class ExtraConfiguration { + + @Bean + SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception { + return http.httpBasic(withDefaults()) + .csrf(CsrfConfigurer::disable) + .authorizeHttpRequests(requests -> requests.anyRequest().fullyAuthenticated()) + .build(); + } + + @Bean + @Lazy + SimpleGrpc.SimpleBlockingStub basic(GrpcChannelFactory channels, @LocalServerPort int port) { + return SimpleGrpc.newBlockingStub(channels.createChannel("default", ChannelBuilderOptions.defaults() + .withInterceptors(List.of(new BasicAuthenticationInterceptor("user", "user"))))); + } + + @PostMapping + GreetingResponse postGreeting(@RequestBody GreetingRequest request) { + var helloRequest = HelloRequest.newBuilder().setName(request.name()).build(); + var helloReply = "Hello ==> " + helloRequest.getName(); + return new GreetingResponse(helloReply); + } + + record GreetingRequest(String name) { + } + + record GreetingResponse(String message) { + } + + } + +} diff --git a/spring-grpc-spring-boot-autoconfigure/src/main/java/org/springframework/grpc/autoconfigure/server/security/GrpcDisableCsrfHttpConfigurer.java b/spring-grpc-spring-boot-autoconfigure/src/main/java/org/springframework/grpc/autoconfigure/server/security/GrpcDisableCsrfHttpConfigurer.java index 5571950..64862a1 100644 --- a/spring-grpc-spring-boot-autoconfigure/src/main/java/org/springframework/grpc/autoconfigure/server/security/GrpcDisableCsrfHttpConfigurer.java +++ b/spring-grpc-spring-boot-autoconfigure/src/main/java/org/springframework/grpc/autoconfigure/server/security/GrpcDisableCsrfHttpConfigurer.java @@ -16,8 +16,13 @@ package org.springframework.grpc.autoconfigure.server.security; import org.springframework.context.ApplicationContext; +import org.springframework.grpc.server.service.GrpcServiceDiscoverer; import org.springframework.security.config.annotation.web.builders.HttpSecurity; import org.springframework.security.config.annotation.web.configurers.AbstractHttpConfigurer; +import org.springframework.security.config.annotation.web.configurers.CsrfConfigurer; +import org.springframework.security.web.csrf.CsrfFilter; +import org.springframework.security.web.util.matcher.AndRequestMatcher; +import org.springframework.security.web.util.matcher.NegatedRequestMatcher; /** * A custom {@link AbstractHttpConfigurer} that disables CSRF protection for gRPC @@ -38,11 +43,22 @@ public class GrpcDisableCsrfHttpConfigurer extends AbstractHttpConfigurer csrf.ignoringRequestMatchers(GrpcServletRequest.all())); + if (context != null && context.getBeanNamesForType(GrpcServiceDiscoverer.class).length == 1 + && isServletEnabledAndCsrfDisabled(context) && isCsrfConfigurerPresent(http)) { + http.csrf(this::disable); } } + @SuppressWarnings("unchecked") + private boolean isCsrfConfigurerPresent(HttpSecurity http) { + return http.getConfigurer(CsrfConfigurer.class) != null; + } + + private void disable(CsrfConfigurer csrf) { + csrf.requireCsrfProtectionMatcher(new AndRequestMatcher(CsrfFilter.DEFAULT_CSRF_MATCHER, + new NegatedRequestMatcher(GrpcServletRequest.all()))); + } + private boolean isServletEnabledAndCsrfDisabled(ApplicationContext context) { return context.getEnvironment().getProperty("spring.grpc.server.servlet.enabled", Boolean.class, true) && !context.getEnvironment() diff --git a/spring-grpc-spring-boot-autoconfigure/src/main/java/org/springframework/grpc/autoconfigure/server/security/GrpcServletRequest.java b/spring-grpc-spring-boot-autoconfigure/src/main/java/org/springframework/grpc/autoconfigure/server/security/GrpcServletRequest.java index 6b44daa..80c615e 100644 --- a/spring-grpc-spring-boot-autoconfigure/src/main/java/org/springframework/grpc/autoconfigure/server/security/GrpcServletRequest.java +++ b/spring-grpc-spring-boot-autoconfigure/src/main/java/org/springframework/grpc/autoconfigure/server/security/GrpcServletRequest.java @@ -28,6 +28,7 @@ import org.springframework.security.web.util.matcher.AntPathRequestMatcher; import org.springframework.security.web.util.matcher.OrRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.util.Assert; +import org.springframework.web.context.WebApplicationContext; import jakarta.servlet.http.HttpServletRequest; @@ -102,6 +103,11 @@ public class GrpcServletRequest { this.delegate = matchers.isEmpty() ? request -> false : new OrRequestMatcher(matchers); } + @Override + protected boolean ignoreApplicationContext(WebApplicationContext context) { + return context.getBeanNamesForType(GrpcServiceDiscoverer.class).length != 1; + } + private Stream getDelegateMatchers(GrpcServiceDiscoverer context) { return getPatterns(context).map(AntPathRequestMatcher::new); } diff --git a/spring-grpc-spring-boot-autoconfigure/src/test/java/org/springframework/grpc/autoconfigure/server/security/GrpcServletRequestTests.java b/spring-grpc-spring-boot-autoconfigure/src/test/java/org/springframework/grpc/autoconfigure/server/security/GrpcServletRequestTests.java index 32532b9..44a7663 100644 --- a/spring-grpc-spring-boot-autoconfigure/src/test/java/org/springframework/grpc/autoconfigure/server/security/GrpcServletRequestTests.java +++ b/spring-grpc-spring-boot-autoconfigure/src/test/java/org/springframework/grpc/autoconfigure/server/security/GrpcServletRequestTests.java @@ -67,6 +67,22 @@ public class GrpcServletRequestTests { assertThat(matcher.matches(request)).isFalse(); }; + @Test + void noServices() { + GrpcServletRequestMatcher matcher = GrpcServletRequest.all(); + MockHttpServletRequest request = mockRequestNoServices("/my-service/Method"); + assertThat(matcher.matches(request)).isFalse(); + }; + + private MockHttpServletRequest mockRequestNoServices(String path) { + MockServletContext servletContext = new MockServletContext(); + servletContext.setAttribute(WebApplicationContext.ROOT_WEB_APPLICATION_CONTEXT_ATTRIBUTE, + new StaticWebApplicationContext()); + MockHttpServletRequest request = new MockHttpServletRequest(servletContext); + request.setPathInfo(path); + return request; + } + private MockHttpServletRequest mockRequest(String path) { MockServletContext servletContext = new MockServletContext(); servletContext.setAttribute(WebApplicationContext.ROOT_WEB_APPLICATION_CONTEXT_ATTRIBUTE, this.context);