Merge pull request #64 from groldan/fix/default_preauth_roles_config

Fix config of default admin role names for http headers preauthentication

src/artifacts/api/src/main/java/org/geoserver/acl/autoconfigure/security/AuthenticationManagerAutoConfiguration.java

@@ -4,15 +4,15 @@
  */
 package org.geoserver.acl.autoconfigure.security;
 
+import org.springframework.boot.autoconfigure.AutoConfiguration;
 import org.springframework.context.annotation.Bean;
-import org.springframework.context.annotation.Configuration;
 import org.springframework.security.authentication.AuthenticationManager;
 import org.springframework.security.authentication.AuthenticationProvider;
 import org.springframework.security.authentication.ProviderManager;
 
 import java.util.List;
 
-@Configuration
+@AutoConfiguration
 public class AuthenticationManagerAutoConfiguration {
 
     @Bean

src/artifacts/api/src/main/java/org/geoserver/acl/autoconfigure/security/PreAuthenticationSecurityAutoConfiguration.java

@@ -7,9 +7,9 @@
 import lombok.NonNull;
 import lombok.RequiredArgsConstructor;
 
+import org.springframework.boot.autoconfigure.AutoConfiguration;
 import org.springframework.boot.context.properties.EnableConfigurationProperties;
 import org.springframework.context.annotation.Bean;
-import org.springframework.context.annotation.Configuration;
 import org.springframework.security.authentication.AuthenticationManager;
 import org.springframework.security.core.authority.SimpleGrantedAuthority;
 import org.springframework.security.core.userdetails.AuthenticationUserDetailsService;
@@ -28,7 +28,7 @@
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
 
-@Configuration
+@AutoConfiguration(before = AuthenticationManagerAutoConfiguration.class)
 @ConditionalOnPreAuthenticationEnabled
 @EnableConfigurationProperties(SecurityConfigProperties.class)
 public class PreAuthenticationSecurityAutoConfiguration {

src/artifacts/api/src/main/resources/values.yml

@@ -49,7 +49,8 @@ acl.users.geoserver.password: "s3cr3t"
 acl.security.headers.enabled: false
 acl.security.headers.user-header: sec-username
 acl.security.headers.roles-header: sec-roles
-acl.security.headers.admin-roles: ["ROLE_ADMINISTRATOR"]
+# comma-separated list of role names provided in the roles header to be given admin rights
+acl.security.headers.admin-roles: ROLE_ADMINISTRATOR
 
 
 ---

src/artifacts/api/src/test/java/org/geoserver/acl/app/AccesControlListApplicationTest.java

@@ -6,7 +6,10 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 
+import org.geoserver.acl.autoconfigure.security.SecurityConfigProperties;
+import org.geoserver.acl.autoconfigure.security.SecurityConfigProperties.PreauthHeaders;
 import org.junit.jupiter.api.Test;
+import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.test.context.SpringBootTest;
 import org.springframework.boot.test.context.SpringBootTest.WebEnvironment;
 import org.springframework.http.HttpHeaders;
@@ -18,6 +21,8 @@
 @ActiveProfiles("dev")
 class AccesControlListApplicationTest extends AbstractAccesControlListApplicationTest {
 
+    @Autowired private SecurityConfigProperties securityConfig;
+
     @Test
     void rootRedirectsToSwaggerUI() {
         String expected = "/acl/openapi/swagger-ui/index.html";
@@ -37,4 +42,12 @@ void rootRedirectsToSwaggerUIWithXForwardedHeaders() {
         assertThat(response.getStatusCode()).isEqualTo(HttpStatus.SEE_OTHER);
         assertThat(response.getHeaders().get("Location")).containsExactly(expected);
     }
+
+    @Test
+    void preAuthDefaultconfig() {
+        PreauthHeaders config = securityConfig.getHeaders();
+        assertThat(config.getUserHeader()).isEqualTo("sec-username");
+        assertThat(config.getRolesHeader()).isEqualTo("sec-roles");
+        assertThat(config.getAdminRoles()).containsExactly("ROLE_ADMINISTRATOR");
+    }
 }

src/artifacts/api/src/test/java/org/geoserver/acl/autoconfigure/security/PreAuthenticationSecurityAutoConfigurationTest.java

@@ -0,0 +1,78 @@
+/* (c) 2023  Open Source Geospatial Foundation - all rights reserved
+ * This code is licensed under the GPL 2.0 license, available at the root
+ * application directory.
+ */
+package org.geoserver.acl.autoconfigure.security;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+
+import org.geoserver.acl.autoconfigure.security.SecurityConfigProperties.PreauthHeaders;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.springframework.boot.autoconfigure.AutoConfigurations;
+import org.springframework.boot.test.context.runner.ApplicationContextRunner;
+import org.springframework.security.authentication.AuthenticationProvider;
+import org.springframework.security.web.authentication.preauth.PreAuthenticatedAuthenticationProvider;
+import org.springframework.security.web.authentication.preauth.RequestHeaderAuthenticationFilter;
+
+class PreAuthenticationSecurityAutoConfigurationTest {
+
+    private ApplicationContextRunner runner =
+            new ApplicationContextRunner()
+                    .withConfiguration(
+                            AutoConfigurations.of(
+                                    AuthenticationManagerAutoConfiguration.class,
+                                    PreAuthenticationSecurityAutoConfiguration.class));
+
+    @BeforeEach
+    void setUp() throws Exception {}
+
+    @Test
+    void testConditionalOnPreAuthenticationEnabledIsDisabledByDefault() {
+        // contribute a mocked up AuthenticationProvider for AuthenticationManagerAutoConfiguration
+        // not to fail due to at least one AuthenticationProvider existing
+        AuthenticationProvider mockProvider = mock(AuthenticationProvider.class);
+        runner.withBean(AuthenticationProvider.class, () -> mockProvider)
+                .run(
+                        context ->
+                                assertThat(context)
+                                        .hasNotFailed()
+                                        .doesNotHaveBean(SecurityConfigProperties.class)
+                                        .doesNotHaveBean(RequestHeaderAuthenticationFilter.class)
+                                        .doesNotHaveBean(
+                                                PreAuthenticatedAuthenticationProvider.class));
+    }
+
+    @Test
+    void testConditionalOnPreAuthenticationEnabled() {
+        runner.withPropertyValues("geoserver.acl.security.headers.enabled=true")
+                .run(
+                        context ->
+                                assertThat(context)
+                                        .hasNotFailed()
+                                        .hasSingleBean(SecurityConfigProperties.class)
+                                        .hasSingleBean(RequestHeaderAuthenticationFilter.class)
+                                        .hasSingleBean(
+                                                PreAuthenticatedAuthenticationProvider.class));
+    }
+
+    @Test
+    void testPreAuthenticationConfig() {
+        runner.withPropertyValues(
+                        "geoserver.acl.security.headers.enabled: true",
+                        "geoserver.acl.security.headers.user-header: sec-username",
+                        "geoserver.acl.security.headers.roles-header: sec-roles",
+                        "geoserver.acl.security.headers.admin-roles: ROLE_ADMINISTRATOR,ADMIN")
+                .run(
+                        context -> {
+                            assertThat(context).hasNotFailed();
+                            PreauthHeaders config =
+                                    context.getBean(SecurityConfigProperties.class).getHeaders();
+                            assertThat(config.getUserHeader()).isEqualTo("sec-username");
+                            assertThat(config.getRolesHeader()).isEqualTo("sec-roles");
+                            assertThat(config.getAdminRoles())
+                                    .containsExactly("ROLE_ADMINISTRATOR", "ADMIN");
+                        });
+    }
+}