Allow defining internal users with plaintext passwords

examples/java-client/pom.xml

@@ -21,7 +21,7 @@
       <dependency>
         <groupId>org.geoserver.acl</groupId>
         <artifactId>gs-acl-bom</artifactId>
-        <version>${acl.version}</version>
+        <version>${project.version}</version>
         <type>pom</type>
         <scope>import</scope>
       </dependency>

examples/pom.xml

@@ -16,7 +16,6 @@
     <module>java-client</module>
   </modules>
   <properties>
-    <acl.version>2.1-SNAPSHOT</acl.version>
     <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
     <project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
   </properties>

src/artifacts/api/src/main/java/org/geoserver/acl/autoconfigure/security/InternalSecurityConfiguration.java → src/artifacts/api/src/main/java/org/geoserver/acl/autoconfigure/security/InternalSecurityAutoConfiguration.java

@@ -6,6 +6,7 @@
 
 import static org.springframework.util.StringUtils.hasText;
 
+import lombok.NonNull;
 import lombok.extern.slf4j.Slf4j;
 
 import org.springframework.beans.factory.annotation.Qualifier;
@@ -19,7 +20,7 @@
 import org.springframework.security.core.userdetails.UserDetails;
 import org.springframework.security.core.userdetails.UserDetailsService;
 import org.springframework.security.crypto.factory.PasswordEncoderFactories;
-import org.springframework.security.crypto.password.DelegatingPasswordEncoder;
+import org.springframework.security.crypto.password.PasswordEncoder;
 import org.springframework.security.provisioning.InMemoryUserDetailsManager;
 
 import java.util.ArrayList;
@@ -30,26 +31,27 @@
 @ConditionalOnInternalAuthenticationEnabled
 @EnableConfigurationProperties(SecurityConfigProperties.class)
 @Slf4j(topic = "org.geoserver.acl.autoconfigure.security")
-public class InternalSecurityConfiguration {
+public class InternalSecurityAutoConfiguration {
 
     @Bean
     AuthenticationProvider internalAuthenticationProvider(
-            @Qualifier("internalUserDetailsService")
-                    UserDetailsService internalUserDetailsService) {
+            @Qualifier("internalUserDetailsService") UserDetailsService internalUserDetailsService,
+            PasswordEncoder encoder) {
 
         DaoAuthenticationProvider provider = new DaoAuthenticationProvider();
         provider.setAuthoritiesMapper(new NullAuthoritiesMapper());
         provider.setUserDetailsService(internalUserDetailsService);
 
-        DelegatingPasswordEncoder encoder =
-                (DelegatingPasswordEncoder)
-                        PasswordEncoderFactories.createDelegatingPasswordEncoder();
-
         provider.setPasswordEncoder(encoder);
 
         return provider;
     }
 
+    @Bean
+    PasswordEncoder passwordEncoder() {
+        return PasswordEncoderFactories.createDelegatingPasswordEncoder();
+    }
+
     @Bean("internalUserDetailsService")
     UserDetailsService internalUserDetailsService(SecurityConfigProperties config) {
 
@@ -58,14 +60,15 @@ UserDetailsService internalUserDetailsService(SecurityConfigProperties config) {
         Collection<UserDetails> authUsers = new ArrayList<>();
         users.forEach(
                 (username, userinfo) -> {
-                    validate(username, userinfo);
-                    log.info(
-                            "Loading internal user {}, admin: {}, enabled: {}",
-                            username,
-                            userinfo.isAdmin(),
-                            userinfo.isEnabled());
-                    UserDetails user = toUserDetails(username, userinfo);
-                    authUsers.add(user);
+                    if (userinfo.isEnabled()) {
+                        log.info(
+                                "Loading internal user {}, admin: {}, enabled: {}",
+                                username,
+                                userinfo.isAdmin(),
+                                userinfo.isEnabled());
+                        UserDetails user = toUserDetails(username, userinfo);
+                        authUsers.add(user);
+                    }
                 });
 
         long enabledUsers = authUsers.stream().filter(UserDetails::isEnabled).count();
@@ -79,20 +82,27 @@ UserDetailsService internalUserDetailsService(SecurityConfigProperties config) {
     }
 
     private UserDetails toUserDetails(
-            String username, SecurityConfigProperties.Internal.UserInfo u) {
+            String username, SecurityConfigProperties.Internal.UserInfo userinfo) {
+        validate(username, userinfo);
         return User.builder()
                 .username(username)
-                .password(u.getPassword())
-                .authorities(u.authorities())
-                .disabled(!u.isEnabled())
+                .password(ensurePrefixed(userinfo.getPassword()))
+                .authorities(userinfo.authorities())
+                .disabled(!userinfo.isEnabled())
                 .build();
     }
 
-    private void validate(final String name, SecurityConfigProperties.Internal.UserInfo info) {
-        if (info.isEnabled()) {
-            if (!hasText(name)) throw new IllegalArgumentException("User has no name: " + info);
-            if (!hasText(info.getPassword()))
-                throw new IllegalArgumentException("User has no password " + name + ": " + info);
+    private String ensurePrefixed(@NonNull String password) {
+        if (!password.matches("(\\{.+\\}).+")) {
+            return "{noop}%s".formatted(password);
         }
+
+        return password;
+    }
+
+    private void validate(final String name, SecurityConfigProperties.Internal.UserInfo info) {
+        if (!hasText(name)) throw new IllegalArgumentException("User has no name: " + info);
+        if (!hasText(info.getPassword()))
+            throw new IllegalArgumentException("User %s has no password".formatted(name));
     }
 }

src/artifacts/api/src/main/resources/META-INF/spring.factories

@@ -7,7 +7,7 @@ org.springframework.boot.autoconfigure.EnableAutoConfiguration=\
 org.geoserver.acl.autoconfigure.persistence.JPAIntegrationAutoConfiguration,\
 org.geoserver.acl.autoconfigure.api.RulesApiAutoConfiguration,\
 org.geoserver.acl.autoconfigure.security.AclServiceSecurityAutoConfiguration,\
-org.geoserver.acl.autoconfigure.security.InternalSecurityConfiguration,\
+org.geoserver.acl.autoconfigure.security.InternalSecurityAutoConfiguration,\
 org.geoserver.acl.autoconfigure.security.PreAuthenticationSecurityAutoConfiguration,\
 org.geoserver.acl.autoconfigure.security.AuthenticationManagerAutoConfiguration,\
 org.geoserver.acl.autoconfigure.springdoc.SpringDocAutoConfiguration,\

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

@@ -107,6 +107,7 @@ springdoc:
     enabled: true
     #results in /swagger-ui/index.html
     path: /index.html
+    disable-swagger-default-url: true
     try-it-out-enabled: true
     tags-sorter: alpha    
 
@@ -158,20 +159,23 @@ geoserver:
             admin: true
             enabled: ${acl.users.admin.enabled}
             password: "${acl.users.admin.password}"
-            # password is the bcrypt encoded value, for example, for pwd s3cr3t:
+            # the following sample password is the bcrypt encoded value, for example, for pwd s3cr3t:
             # password: "{bcrypt}$2a$10$eMyaZRLZBAZdor8nOX.qwuwOyWazXjR2hddGLCT6f6c382WiwdQGG"
           geoserver:
             # special user for GeoServer to ACL communication
             # Using a `{noop}` default credentials for performance, since bcrypt adds a significant per-request overhead
-            # in the orther of 100ms. In production it should be replaced by a docker/k8s secret
+            # in the orther of 100ms. In production it should be replaced by a docker/k8s secret. To simplify defining and
+            # reusing secrets for both the server and client config, a noop encrypted password is allowed not to have the
+            # {noop} prefix.
             admin: true
             enabled: ${acl.users.geoserver.enabled}
             password: "${acl.users.geoserver.password}"
+# Sample non-admin user: 
 #         user:
 #            admin: false
 #            enabled: true
 #            # password is the bcrypt encoded value for s3cr3t
-#            password: "{bcrypt}$2a$10$eMyaZRLZBAZdor8nOX.qwuwOyWazXjR2hddGLCT6f6c382WiwdQGG"
+#            password: "{noop}changeme"
 
 ---
 spring.config.activate.on-profile: local

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

@@ -29,7 +29,7 @@ acl.security.basic.enabled: true
 acl.users.admin.enabled: true
 acl.users.admin.password: "{bcrypt}$2a$10$eMyaZRLZBAZdor8nOX.qwuwOyWazXjR2hddGLCT6f6c382WiwdQGG"
 acl.users.geoserver.enabled: true
-acl.users.geoserver.password: "{noop}s3cr3t"
+acl.users.geoserver.password: "s3cr3t"
 
 #Example of how to add additional users:
 #geoserver:

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

@@ -0,0 +1,148 @@
+/* (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.junit.jupiter.api.Assertions.assertThrows;
+
+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.authentication.dao.DaoAuthenticationProvider;
+import org.springframework.security.core.GrantedAuthority;
+import org.springframework.security.core.userdetails.UserDetails;
+import org.springframework.security.core.userdetails.UserDetailsService;
+import org.springframework.security.core.userdetails.UsernameNotFoundException;
+import org.springframework.security.crypto.password.PasswordEncoder;
+import org.springframework.security.provisioning.InMemoryUserDetailsManager;
+
+import java.util.List;
+
+class InternalSecurityAutoConfigurationTest {
+
+    private ApplicationContextRunner runner =
+            new ApplicationContextRunner()
+                    .withConfiguration(
+                            AutoConfigurations.of(InternalSecurityAutoConfiguration.class));
+
+    @Test
+    void conditionalOnInternalAuthenticationEnabledIsDisabledByDefault() {
+        runner.run(
+                context ->
+                        assertThat(context)
+                                .hasNotFailed()
+                                .doesNotHaveBean(AuthenticationProvider.class));
+    }
+
+    @Test
+    void conditionalOnInternalAuthenticationEnabled() {
+        runner.withPropertyValues("geoserver.acl.security.internal.enabled=true")
+                .run(
+                        context ->
+                                assertThat(context)
+                                        .hasNotFailed()
+                                        .hasSingleBean(UserDetailsService.class)
+                                        .hasSingleBean(PasswordEncoder.class)
+                                        .hasSingleBean(AuthenticationProvider.class)
+                                        .getBean(AuthenticationProvider.class)
+                                        .isExactlyInstanceOf(DaoAuthenticationProvider.class));
+    }
+
+    @Test
+    void testUsers() {
+        runner.withPropertyValues(
+                        "geoserver.acl.security.internal.enabled=true",
+                        // define some users
+                        // admin user
+                        "geoserver.acl.security.internal.users.testadmin.enabled=true",
+                        "geoserver.acl.security.internal.users.testadmin.admin=true",
+                        "geoserver.acl.security.internal.users.testadmin.password={bcrypt}$2a$10$eMyaZRLZBAZdor8nOX.qwuwOyWazXjR2hddGLCT6f6c382WiwdQGG",
+                        // non-admin user
+                        "geoserver.acl.security.internal.users.testuser.enabled=true",
+                        "geoserver.acl.security.internal.users.testuser.admin=false",
+                        "geoserver.acl.security.internal.users.testuser.password={noop}changeme")
+                .run(
+                        context -> {
+                            assertThat(context)
+                                    .hasNotFailed()
+                                    .hasSingleBean(UserDetailsService.class)
+                                    .getBean(UserDetailsService.class)
+                                    .isInstanceOf(InMemoryUserDetailsManager.class);
+
+                            UserDetailsService service = context.getBean(UserDetailsService.class);
+                            UserDetails admin = service.loadUserByUsername("testadmin");
+                            assertThat(admin.getPassword())
+                                    .isEqualTo(
+                                            "{bcrypt}$2a$10$eMyaZRLZBAZdor8nOX.qwuwOyWazXjR2hddGLCT6f6c382WiwdQGG");
+                            assertThat(admin.getAuthorities())
+                                    .hasSize(1)
+                                    .map(GrantedAuthority::getAuthority)
+                                    .isEqualTo(List.of("ROLE_ADMIN"));
+
+                            UserDetails user = service.loadUserByUsername("testuser");
+                            assertThat(user.getPassword()).isEqualTo("{noop}changeme");
+                            assertThat(user.getAuthorities())
+                                    .hasSize(1)
+                                    .map(GrantedAuthority::getAuthority)
+                                    .isEqualTo(List.of("ROLE_USER"));
+                        });
+    }
+
+    @Test
+    void testFailureOnEnabledPasswordLessUser() {
+        runner.withPropertyValues(
+                        "geoserver.acl.security.internal.enabled=true",
+                        // ill-defined passwsord-less user
+                        "geoserver.acl.security.internal.users.baduser.enabled=true")
+                .run(
+                        context -> {
+                            assertThat(context)
+                                    .hasFailed()
+                                    .getFailure()
+                                    .hasMessageContaining("User baduser has no password");
+                        });
+    }
+
+    @Test
+    void testDisabledPasswordLessUserIgnored() {
+        runner.withPropertyValues(
+                        "geoserver.acl.security.internal.enabled=true",
+                        // ill-defined but disabled passwsord-less user
+                        "geoserver.acl.security.internal.users.baduser.enabled=false",
+                        "geoserver.acl.security.internal.users.baduser.admin=true")
+                .run(
+                        context -> {
+                            assertThat(context).hasNotFailed();
+
+                            UserDetailsService service = context.getBean(UserDetailsService.class);
+
+                            assertThrows(
+                                    UsernameNotFoundException.class,
+                                    () -> service.loadUserByUsername("baduser"));
+                        });
+    }
+
+    @Test
+    void testUsersPlainTextPasswordMappedAsNoop() {
+        runner.withPropertyValues(
+                        "geoserver.acl.security.internal.enabled=true",
+                        "geoserver.acl.security.internal.users.testuser.enabled=true",
+                        "geoserver.acl.security.internal.users.testuser.admin=false",
+                        "geoserver.acl.security.internal.users.testuser.password=changeme")
+                .run(
+                        context -> {
+                            UserDetailsService service = context.getBean(UserDetailsService.class);
+                            UserDetails user = service.loadUserByUsername("testuser");
+                            assertThat(user.getPassword())
+                                    .as(
+                                            """
+                                    		plain text password should have been mapped as \
+                                    		a '{noop}' prefixed literal
+                                    		""")
+                                    .isEqualTo("{noop}changeme");
+                        });
+    }
+}