Fix bugs in security autoconfig conditions

2025-03-06 14:52:20 +00:00
parent 2578d99cd3
commit f580197169
6 changed files with 266 additions and 52 deletions
--- a/samples/grpc-secure/src/main/java/org/springframework/grpc/sample/GrpcServerApplication.java
+++ b/samples/grpc-secure/src/main/java/org/springframework/grpc/sample/GrpcServerApplication.java
@@ -5,11 +5,8 @@ import static org.springframework.security.config.Customizer.withDefaults;
 import org.springframework.boot.SpringApplication;
 import org.springframework.boot.autoconfigure.SpringBootApplication;
 import org.springframework.context.annotation.Bean;
-import org.springframework.context.annotation.Import;
 import org.springframework.grpc.server.GlobalServerInterceptor;
 import org.springframework.grpc.server.security.GrpcSecurity;
-import org.springframework.security.config.annotation.authentication.configuration.AuthenticationConfiguration;
-import org.springframework.security.config.annotation.method.configuration.EnableMethodSecurity;
 import org.springframework.security.core.userdetails.User;
 import org.springframework.security.provisioning.InMemoryUserDetailsManager;

@@ -17,8 +14,6 @@ import io.grpc.Metadata;
 import io.grpc.ServerInterceptor;

@SpringBootApplication
-@EnableMethodSecurity
-@Import(AuthenticationConfiguration.class)
 public class GrpcServerApplication {

 	public static final Metadata.Key<String> USER_KEY = Metadata.Key.of("X-USER", Metadata.ASCII_STRING_MARSHALLER);
--- a/spring-grpc-spring-boot-autoconfigure/src/main/java/org/springframework/grpc/autoconfigure/server/GrpcServerFactoryAutoConfiguration.java
+++ b/spring-grpc-spring-boot-autoconfigure/src/main/java/org/springframework/grpc/autoconfigure/server/GrpcServerFactoryAutoConfiguration.java
@@ -75,7 +75,7 @@ public class GrpcServerFactoryAutoConfiguration {

 	@Configuration(proxyBeanMethods = false)
 	@Conditional(OnGrpcServletCondition.class)
-	static class GrpcServletConfiguration {
+	public static class GrpcServletConfiguration {

 		private static Log logger = LogFactory.getLog(GrpcServletConfiguration.class);

--- a/spring-grpc-spring-boot-autoconfigure/src/main/java/org/springframework/grpc/autoconfigure/server/security/GrpcSecurityAutoConfiguration.java
+++ b/spring-grpc-spring-boot-autoconfigure/src/main/java/org/springframework/grpc/autoconfigure/server/security/GrpcSecurityAutoConfiguration.java
@@ -23,6 +23,7 @@ import org.springframework.context.ApplicationContext;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Conditional;
 import org.springframework.context.annotation.Configuration;
+import org.springframework.context.annotation.Import;
 import org.springframework.grpc.autoconfigure.server.ConditionalOnGrpcServerEnabled;
 import org.springframework.grpc.autoconfigure.server.GrpcServerFactoryAutoConfiguration;
 import org.springframework.grpc.autoconfigure.server.exception.GrpcExceptionHandlerAutoConfiguration;
@@ -44,52 +45,56 @@ import io.grpc.internal.GrpcUtil;
@ConditionalOnClass(ObjectPostProcessor.class)
@ConditionalOnGrpcServerEnabled
@AutoConfiguration(before = GrpcExceptionHandlerAutoConfiguration.class, after = SecurityAutoConfiguration.class)
+
+@Import({ ExceptionHandlerAutoConfiguration.class, GrpcNativeSecurityConfigurerAutoConfiguration.class,
+		GrpcServletSecurityConfigurerAutoConfiguration.class })
 public class GrpcSecurityAutoConfiguration {

-	@Configuration(proxyBeanMethods = false)
-	static class ExceptionHandlerAutoConfiguration {
+}

-		@Bean
-		public GrpcExceptionHandler accessExceptionHandler() {
-			return new SecurityGrpcExceptionHandler();
-		}
-
-	}
-
-	@ConditionalOnBean(ObjectPostProcessor.class)
-	@Configuration(proxyBeanMethods = false)
-	@Conditional(GrpcServerFactoryAutoConfiguration.OnNativeGrpcServerCondition.class)
-	static class GrpcNativeSecurityConfigurerAutoConfiguration {
-
-		@Bean
-		public GrpcSecurity grpcSecurity(ObjectPostProcessor<Object> objectPostProcessor,
-				AuthenticationConfiguration authenticationConfiguration, ApplicationContext context) throws Exception {
-			AuthenticationManagerBuilder authenticationManagerBuilder = authenticationConfiguration
-				.authenticationManagerBuilder(objectPostProcessor, context);
-			authenticationManagerBuilder
-				.parentAuthenticationManager(authenticationConfiguration.getAuthenticationManager());
-			return new GrpcSecurity(objectPostProcessor, authenticationManagerBuilder, context);
-		}
-
-	}
-
-	@ConditionalOnBean(SecurityFilterChain.class)
-	@Conditional(GrpcServerFactoryAutoConfiguration.OnGrpcServletCondition.class)
-	@Configuration(proxyBeanMethods = false)
-	static class GrpcServletSecurityConfigurerAutoConfiguration {
-
-		@Bean
-		@GlobalServerInterceptor
-		public SecurityContextServerInterceptor securityContextInterceptor() {
-			return new SecurityContextServerInterceptor();
-		}
-
-		@Bean
-		public <T extends ServerBuilder<T>> ServerBuilderCustomizer<T> securityContextExecutorCustomizer() {
-			return (serverBuilder) -> serverBuilder
-				.executor(new DelegatingSecurityContextExecutor(GrpcUtil.SHARED_CHANNEL_EXECUTOR.create()));
-		}
+@Configuration(proxyBeanMethods = false)
+@Import(AuthenticationConfiguration.class)
+class ExceptionHandlerAutoConfiguration {

+	@Bean
+	public GrpcExceptionHandler accessExceptionHandler() {
+		return new SecurityGrpcExceptionHandler();
+	}
+
+}
+
+@ConditionalOnBean(ObjectPostProcessor.class)
+@Configuration(proxyBeanMethods = false)
+@Conditional(GrpcServerFactoryAutoConfiguration.OnNativeGrpcServerCondition.class)
+class GrpcNativeSecurityConfigurerAutoConfiguration {
+
+	@Bean
+	public GrpcSecurity grpcSecurity(ObjectPostProcessor<Object> objectPostProcessor,
+			AuthenticationConfiguration authenticationConfiguration, ApplicationContext context) throws Exception {
+		AuthenticationManagerBuilder authenticationManagerBuilder = authenticationConfiguration
+			.authenticationManagerBuilder(objectPostProcessor, context);
+		authenticationManagerBuilder
+			.parentAuthenticationManager(authenticationConfiguration.getAuthenticationManager());
+		return new GrpcSecurity(objectPostProcessor, authenticationManagerBuilder, context);
+	}
+
+}
+
+@ConditionalOnBean(SecurityFilterChain.class)
+@Conditional(GrpcServerFactoryAutoConfiguration.OnGrpcServletCondition.class)
+@Configuration(proxyBeanMethods = false)
+class GrpcServletSecurityConfigurerAutoConfiguration {
+
+	@Bean
+	@GlobalServerInterceptor
+	public SecurityContextServerInterceptor securityContextInterceptor() {
+		return new SecurityContextServerInterceptor();
+	}
+
+	@Bean
+	public <T extends ServerBuilder<T>> ServerBuilderCustomizer<T> securityContextExecutorCustomizer() {
+		return (serverBuilder) -> serverBuilder
+			.executor(new DelegatingSecurityContextExecutor(GrpcUtil.SHARED_CHANNEL_EXECUTOR.create()));
 	}

 }
--- a/spring-grpc-spring-boot-autoconfigure/src/main/java/org/springframework/grpc/autoconfigure/server/security/OAuth2ResourceServerAutoConfiguration.java
+++ b/spring-grpc-spring-boot-autoconfigure/src/main/java/org/springframework/grpc/autoconfigure/server/security/OAuth2ResourceServerAutoConfiguration.java
@@ -44,6 +44,8 @@ import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Conditional;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.context.annotation.Import;
+import org.springframework.grpc.autoconfigure.server.GrpcServerFactoryAutoConfiguration;
+import org.springframework.grpc.autoconfigure.server.GrpcServerFactoryAutoConfiguration.GrpcServletConfiguration;
 import org.springframework.grpc.server.GlobalServerInterceptor;
 import org.springframework.grpc.server.security.AuthenticationProcessInterceptor;
 import org.springframework.grpc.server.security.GrpcSecurity;
@@ -65,12 +67,17 @@ import org.springframework.security.oauth2.server.resource.introspection.OpaqueT
 import org.springframework.security.oauth2.server.resource.introspection.SpringOpaqueTokenIntrospector;
 import org.springframework.util.CollectionUtils;

+import io.grpc.BindableService;
+
 // All copied from Spring Boot (https://github.com/spring-projects/spring-boot/issues/43978), except the
 // 2 @Beans of type AuthenticationProcessInterceptor
@AutoConfiguration(before = { GrpcSecurityAutoConfiguration.class, UserDetailsServiceAutoConfiguration.class },
-		after = org.springframework.boot.autoconfigure.security.oauth2.resource.servlet.OAuth2ResourceServerAutoConfiguration.class)
+		after = { GrpcServerFactoryAutoConfiguration.class,
+				org.springframework.boot.autoconfigure.security.oauth2.resource.servlet.OAuth2ResourceServerAutoConfiguration.class })
@EnableConfigurationProperties(OAuth2ResourceServerProperties.class)
@ConditionalOnClass(BearerTokenAuthenticationToken.class)
+@ConditionalOnMissingBean(GrpcServletConfiguration.class)
+@ConditionalOnBean(BindableService.class)
@Import({ Oauth2ResourceServerConfiguration.JwtConfiguration.class,
 		Oauth2ResourceServerConfiguration.OpaqueTokenConfiguration.class })
 class OAuth2ResourceServerAutoConfiguration {
@@ -122,7 +129,8 @@ class OAuth2ResourceServerOpaqueTokenConfiguration {
 		@Bean
 		@ConditionalOnBean(OpaqueTokenIntrospector.class)
 		@GlobalServerInterceptor
-		AuthenticationProcessInterceptor opaqueTokenSecurityFilterChain(GrpcSecurity http) throws Exception {
+		AuthenticationProcessInterceptor opaqueTokenAuthenticationProcessInterceptor(GrpcSecurity http)
+				throws Exception {
 			http.authorizeRequests((requests) -> requests.allRequests().authenticated());
 			http.oauth2ResourceServer((resourceServer) -> resourceServer.opaqueToken(withDefaults()));
 			return http.build();
@@ -241,7 +249,7 @@ class OAuth2ResourceServerJwtConfiguration {
 		@Bean
 		@ConditionalOnBean(JwtDecoder.class)
 		@GlobalServerInterceptor
-		AuthenticationProcessInterceptor jwtSecurityFilterChain(GrpcSecurity http) throws Exception {
+		AuthenticationProcessInterceptor jwtAuthenticationProcessInterceptor(GrpcSecurity http) throws Exception {
 			http.authorizeRequests((requests) -> requests.allRequests().authenticated());
 			http.oauth2ResourceServer((resourceServer) -> resourceServer.jwt(withDefaults()));
 			return http.build();
--- a/spring-grpc-spring-boot-autoconfigure/src/test/java/org/springframework/grpc/autoconfigure/server/security/GrpcSecurityAutoConfigurationTests.java
+++ b/spring-grpc-spring-boot-autoconfigure/src/test/java/org/springframework/grpc/autoconfigure/server/security/GrpcSecurityAutoConfigurationTests.java
@@ -18,18 +18,24 @@ package org.springframework.grpc.autoconfigure.server.security;

 import static org.assertj.core.api.Assertions.assertThat;

+import java.util.Arrays;
+
 import org.junit.jupiter.api.Test;
 import org.mockito.Mockito;
-
 import org.springframework.boot.autoconfigure.AutoConfigurations;
+import org.springframework.boot.autoconfigure.logging.ConditionEvaluationReportLoggingListener;
+import org.springframework.boot.logging.LogLevel;
 import org.springframework.boot.test.context.FilteredClassLoader;
 import org.springframework.boot.test.context.runner.ApplicationContextRunner;
+import org.springframework.context.annotation.Configuration;
 import org.springframework.grpc.autoconfigure.server.GrpcServerAutoConfiguration;
 import org.springframework.grpc.server.exception.GrpcExceptionHandler;
 import org.springframework.grpc.server.lifecycle.GrpcServerLifecycle;
 import org.springframework.grpc.server.security.AuthenticationProcessInterceptor;
+import org.springframework.grpc.server.security.GrpcSecurity;
 import org.springframework.grpc.server.security.SecurityGrpcExceptionHandler;
 import org.springframework.security.config.ObjectPostProcessor;
+import org.springframework.security.config.annotation.method.configuration.EnableMethodSecurity;

 /**
 * Tests for {@link GrpcServerAutoConfiguration}.
@@ -52,6 +58,17 @@ class GrpcSecurityAutoConfigurationTests {
 			.run((context) -> assertThat(context).doesNotHaveBean(GrpcSecurityAutoConfiguration.class));
 	}

+	@Test
+	void whenObjectPostProcessorPresentGrpcSecurityIsCreated() {
+		new ApplicationContextRunner()
+			.withInitializer(ConditionEvaluationReportLoggingListener.forLogLevel(LogLevel.INFO))
+			.withConfiguration(AutoConfigurations.of(GrpcSecurityAutoConfiguration.class))
+			.run((context) -> {
+				System.err.println(Arrays.asList(context.getBeanDefinitionNames()));
+				assertThat(context).hasSingleBean(GrpcSecurity.class);
+			});
+	}
+
 	@Test
 	void whenServerEnabledPropertySetFalseThenAutoConfigurationIsSkipped() {
 		this.contextRunner()
@@ -79,4 +96,10 @@ class GrpcSecurityAutoConfigurationTests {
 		});
 	}

+	@EnableMethodSecurity
+	@Configuration(proxyBeanMethods = false)
+	static class ExtraConfiguration {
+
+	}
+
 }
--- a/spring-grpc-spring-boot-autoconfigure/src/test/java/org/springframework/grpc/autoconfigure/server/security/OAuth2ResourceServerAutoConfigurationTests.java
+++ b/spring-grpc-spring-boot-autoconfigure/src/test/java/org/springframework/grpc/autoconfigure/server/security/OAuth2ResourceServerAutoConfigurationTests.java
@@ -0,0 +1,183 @@
+/*
+ * Copyright 2023-2024 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.grpc.autoconfigure.server.security;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
+import org.springframework.boot.SpringApplication;
+import org.springframework.boot.autoconfigure.AutoConfigurations;
+import org.springframework.boot.autoconfigure.logging.ConditionEvaluationReportLoggingListener;
+import org.springframework.boot.autoconfigure.security.servlet.SecurityAutoConfiguration;
+import org.springframework.boot.autoconfigure.ssl.SslAutoConfiguration;
+import org.springframework.boot.context.annotation.UserConfigurations;
+import org.springframework.boot.context.event.ApplicationFailedEvent;
+import org.springframework.boot.logging.LogLevel;
+import org.springframework.boot.test.context.runner.ApplicationContextRunner;
+import org.springframework.boot.test.context.runner.WebApplicationContextRunner;
+import org.springframework.boot.web.servlet.context.AnnotationConfigServletWebApplicationContext;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.grpc.autoconfigure.server.GrpcServerAutoConfiguration;
+import org.springframework.grpc.autoconfigure.server.GrpcServerFactoryAutoConfiguration;
+import org.springframework.grpc.server.GlobalServerInterceptor;
+import org.springframework.grpc.server.lifecycle.GrpcServerLifecycle;
+import org.springframework.grpc.server.security.AuthenticationProcessInterceptor;
+import org.springframework.grpc.server.security.GrpcSecurity;
+import org.springframework.security.config.Customizer;
+
+import io.grpc.BindableService;
+import io.grpc.ServerServiceDefinition;
+
+/**
+ * Tests for {@link GrpcServerAutoConfiguration}.
+ *
+ * @author Chris Bono
+ */
+class OAuth2ResourceServerAutoConfigurationTests {
+
+	private BindableService service = mock();
+
+	{
+		ServerServiceDefinition serviceDefinition = ServerServiceDefinition.builder("my-service").build();
+		when(service.bindService()).thenReturn(serviceDefinition);
+
+	}
+
+	private ApplicationContextRunner contextRunner() {
+		// NOTE: we use noop server lifecycle to avoid startup
+		return new ApplicationContextRunner()
+			.withConfiguration(AutoConfigurations.of(OAuth2ResourceServerAutoConfiguration.class,
+					GrpcSecurityAutoConfiguration.class))
+			.withBean(BindableService.class, () -> service)
+			.withBean("noopServerLifecycle", GrpcServerLifecycle.class, Mockito::mock);
+	}
+
+	@Test
+	void notConfiguredWhenIssuerNotProvided() {
+		this.contextRunner().run((context) -> {
+			assertThat(context).doesNotHaveBean(AuthenticationProcessInterceptor.class);
+		});
+	}
+
+	@Test
+	void notConfiguredInWebApplication() {
+		new WebApplicationContextRunner().withConfiguration(AutoConfigurations.of(
+				GrpcServerFactoryAutoConfiguration.class, GrpcServerAutoConfiguration.class,
+				SecurityAutoConfiguration.class,
+				org.springframework.boot.autoconfigure.security.oauth2.resource.servlet.OAuth2ResourceServerAutoConfiguration.class,
+				OAuth2ResourceServerAutoConfiguration.class, GrpcSecurityAutoConfiguration.class))
+			.withBean(BindableService.class, () -> service)
+			.withPropertyValues("spring.security.oauth2.resourceserver.jwt.issuer-uri=http://localhost:9000")
+			.run((context) -> {
+				assertThat(context).doesNotHaveBean(AuthenticationProcessInterceptor.class);
+			});
+	}
+
+	@Test
+	void notConfiguredInWebApplicationWithNoBindableService() {
+		new WebApplicationContextRunner(WebApplicationContextRunner.withMockServletContext(MyContext::new))
+			.withInitializer(ConditionEvaluationReportLoggingListener.forLogLevel(LogLevel.INFO))
+			.withConfiguration(AutoConfigurations.of(GrpcServerFactoryAutoConfiguration.class,
+					GrpcServerAutoConfiguration.class, SecurityAutoConfiguration.class,
+					org.springframework.boot.autoconfigure.security.oauth2.resource.servlet.OAuth2ResourceServerAutoConfiguration.class,
+					OAuth2ResourceServerAutoConfiguration.class, GrpcSecurityAutoConfiguration.class))
+			.withPropertyValues("spring.security.oauth2.resourceserver.jwt.issuer-uri=http://localhost:9000")
+			.run((context) -> {
+				assertThat(context).doesNotHaveBean(AuthenticationProcessInterceptor.class);
+			});
+	}
+
+	@Test
+	void configuredInWebApplicationWithGrpcNative() {
+		new WebApplicationContextRunner(WebApplicationContextRunner.withMockServletContext(MyContext::new))
+			.withConfiguration(AutoConfigurations.of(GrpcServerFactoryAutoConfiguration.class,
+					GrpcServerAutoConfiguration.class, SslAutoConfiguration.class, SecurityAutoConfiguration.class,
+					org.springframework.boot.autoconfigure.security.oauth2.resource.servlet.OAuth2ResourceServerAutoConfiguration.class,
+					OAuth2ResourceServerAutoConfiguration.class, GrpcSecurityAutoConfiguration.class))
+			.withInitializer(ConditionEvaluationReportLoggingListener.forLogLevel(LogLevel.INFO))
+			.withBean(BindableService.class, () -> service)
+			.withBean("noopServerLifecycle", GrpcServerLifecycle.class, Mockito::mock)
+			.withPropertyValues("spring.security.oauth2.resourceserver.jwt.issuer-uri=http://localhost:9000",
+					"spring.grpc.server.servlet.enabled=false")
+			.run((context) -> {
+				assertThat(context).hasSingleBean(AuthenticationProcessInterceptor.class);
+			});
+	}
+
+	// Utility class to ensure ApplicationFailedEvent is published
+	static class MyContext extends AnnotationConfigServletWebApplicationContext {
+
+		@Override
+		public void refresh() {
+			try {
+				super.refresh();
+			}
+			catch (Throwable ex) {
+				publishEvent(new ApplicationFailedEvent(new SpringApplication(this), new String[0], this, ex));
+				throw ex;
+			}
+		}
+
+	}
+
+	@Test
+	void jwtConfiguredWhenIssuerIsProvided() {
+		this.contextRunner()
+			.withPropertyValues("spring.security.oauth2.resourceserver.jwt.issuer-uri=http://localhost:9000")
+			.run((context) -> {
+				assertThat(context).hasSingleBean(AuthenticationProcessInterceptor.class);
+			});
+	}
+
+	@Test
+	void jwtConfiguredWhenJwkSetIsProvided() {
+		this.contextRunner()
+			.withPropertyValues("spring.security.oauth2.resourceserver.jwt.jwk-set-uri=http://localhost:9000")
+			.run((context) -> {
+				assertThat(context).hasSingleBean(AuthenticationProcessInterceptor.class);
+			});
+	}
+
+	@Test
+	void customInterceptorWhenJwkSetIsProvided() {
+		this.contextRunner()
+			.withInitializer(ConditionEvaluationReportLoggingListener.forLogLevel(LogLevel.INFO))
+			.withConfiguration(UserConfigurations.of(CustomInterceptorConfiguration.class))
+			.withPropertyValues("spring.security.oauth2.resourceserver.jwt.jwk-set-uri=http://localhost:9000")
+			.run((context) -> {
+				assertThat(context).hasSingleBean(AuthenticationProcessInterceptor.class);
+			});
+	}
+
+	@Configuration(proxyBeanMethods = false)
+	static class CustomInterceptorConfiguration {
+
+		@Bean
+		@GlobalServerInterceptor
+		AuthenticationProcessInterceptor jwtSecurityFilterChain(GrpcSecurity grpc) throws Exception {
+			return grpc.authorizeRequests(requests -> requests.allRequests().authenticated())
+				.oauth2ResourceServer((resourceServer) -> resourceServer.jwt(Customizer.withDefaults()))
+				.build();
+		}
+
+	}
+
+}