Merge pull request #41 from groldan/qa

plugin: Readability improvements in org.geoserver.acl.plugin.accessmanager

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

@@ -6,7 +6,6 @@
 
 import lombok.extern.slf4j.Slf4j;
 
-import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.autoconfigure.AutoConfiguration;
 import org.springframework.boot.context.properties.EnableConfigurationProperties;
 import org.springframework.context.annotation.Bean;
@@ -18,20 +17,21 @@
 import org.springframework.security.web.SecurityFilterChain;
 import org.springframework.security.web.authentication.preauth.RequestHeaderAuthenticationFilter;
 
+import java.util.Optional;
+
 @AutoConfiguration
 @EnableWebSecurity
 @EnableConfigurationProperties(SecurityConfigProperties.class)
 @EnableGlobalMethodSecurity(prePostEnabled = true)
 @Slf4j(topic = "org.geoserver.acl.autoconfigure.security")
 public class AclServiceSecurityAutoConfiguration {
 
-    private @Autowired(required = false) RequestHeaderAuthenticationFilter preAuthFilter;
-
     @Bean
-    public SecurityFilterChain securityFilterChain(
+    SecurityFilterChain securityFilterChain(
             HttpSecurity http,
             AuthenticationManager authenticationManager,
-            SecurityConfigProperties config)
+            SecurityConfigProperties config,
+            Optional<RequestHeaderAuthenticationFilter> preAuthFilter)
             throws Exception {
 
         http.csrf().disable();
@@ -43,15 +43,16 @@ public SecurityFilterChain securityFilterChain(
 
         http.authenticationManager(authenticationManager);
 
-        if (null == preAuthFilter) {
-            log.info("Pre-authentication headers disabled");
-        } else {
+        if (preAuthFilter.isPresent()) {
+            RequestHeaderAuthenticationFilter preAuth = preAuthFilter.orElseThrow();
             log.info(
                     "Pre-authentication headers enabled for {}/{}. Admin roles: {}",
                     config.getHeaders().getUserHeader(),
                     config.getHeaders().getRolesHeader(),
                     config.getHeaders().getAdminRoles());
-            http.addFilterAfter(preAuthFilter, RequestHeaderAuthenticationFilter.class);
+            http = http.addFilterAfter(preAuth, RequestHeaderAuthenticationFilter.class);
+        } else {
+            log.info("Pre-authentication headers disabled");
         }
 
         http =

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

@@ -51,7 +51,7 @@ AuthenticationProvider internalAuthenticationProvider(
     }
 
     @Bean("internalUserDetailsService")
-    public UserDetailsService internalUserDetailsService(SecurityConfigProperties config) {
+    UserDetailsService internalUserDetailsService(SecurityConfigProperties config) {
 
         Map<String, SecurityConfigProperties.Internal.UserInfo> users =
                 config.getInternal().getUsers();

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

@@ -34,7 +34,7 @@
 public class PreAuthenticationSecurityAutoConfiguration {
 
     @Bean
-    public RequestHeaderAuthenticationFilter requestHeaderAuthenticationFilter(
+    RequestHeaderAuthenticationFilter requestHeaderAuthenticationFilter(
             AuthenticationManager authenticationManager, SecurityConfigProperties config)
             throws Exception {
         RequestHeaderAuthenticationFilter filter = new RequestHeaderAuthenticationFilter();

src/artifacts/api/src/main/java/org/geoserver/acl/autoconfigure/springdoc/SpringDocHomeRedirectController.java

@@ -5,7 +5,7 @@
 package org.geoserver.acl.autoconfigure.springdoc;
 
 import org.springframework.stereotype.Controller;
-import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.GetMapping;
 
 @Controller
 public class SpringDocHomeRedirectController {
@@ -19,7 +19,7 @@ public SpringDocHomeRedirectController(String basePath) {
         this.basePath = basePath;
     }
 
-    @RequestMapping(value = "/")
+    @GetMapping(value = "/")
     public String redirectToSwaggerUI() {
         return "redirect:" + basePath;
     }

src/integration/openapi/java-client/src/main/java/org/geoserver/acl/api/client/integration/ClientExceptionHelper.java

@@ -4,16 +4,22 @@
  */
 package org.geoserver.acl.api.client.integration;
 
+import lombok.experimental.UtilityClass;
+
 import org.springframework.web.client.HttpClientErrorException;
 
+import java.util.Optional;
+
+@UtilityClass
 class ClientExceptionHelper {
 
     static String reason(HttpClientErrorException e) {
         return reason(e, e.getMessage());
     }
 
     static String reason(HttpClientErrorException e, String defaultValue) {
-        String reason = e.getResponseHeaders().getFirst("X-Reason");
-        return reason == null ? defaultValue : reason;
+        return Optional.ofNullable(e.getResponseHeaders())
+                .map(h -> h.getFirst("X-Reason"))
+                .orElse(defaultValue);
     }
 }

src/integration/persistence-jpa/integration/src/main/java/org/geoserver/acl/integration/jpa/config/JPAIntegrationConfiguration.java

@@ -32,7 +32,7 @@
 public class JPAIntegrationConfiguration {
 
     @Bean
-    public RuleRepository aclRuleRepositoryJpaAdaptor(
+    RuleRepository aclRuleRepositoryJpaAdaptor(
             EntityManager em,
             JpaRuleRepository jpaRuleRepository,
             RuleJpaMapper modelMapper,
@@ -46,7 +46,7 @@ public RuleRepository aclRuleRepositoryJpaAdaptor(
     }
 
     @Bean
-    public AdminRuleRepository aclAdminRuleRepositoryJpaAdaptor(
+    AdminRuleRepository aclAdminRuleRepositoryJpaAdaptor(
             EntityManager em,
             JpaAdminRuleRepository jpaAdminRuleRepo,
             AdminRuleJpaMapper modelMapper,

src/integration/persistence-jpa/model/src/main/java/org/geoserver/acl/jpa/config/AclDataSourceConfiguration.java

@@ -41,7 +41,7 @@ public class AclDataSourceConfiguration {
      */
     @Primary
     @Bean("authorizationDataSource")
-    public DataSource authorizationDataSource(AclJpaProperties props) {
+    DataSource authorizationDataSource(AclJpaProperties props) {
         DataSourceProperties dsprops = props.getDatasource();
         final String jndiName = dsprops.getJndiName();
         if (StringUtils.hasText(jndiName)) {

src/integration/persistence-jpa/model/src/main/java/org/geoserver/acl/jpa/config/AuthorizationJPAConfiguration.java

@@ -50,7 +50,7 @@ HibernateJpaVendorAdapter authorizationVendorAdapter(AclJpaProperties configProp
 
     @Bean("authorizationEntityManagerFactory")
     @DependsOn({"authorizationDataSource", "authorizationVendorAdapter"})
-    public LocalContainerEntityManagerFactoryBean authorizationEntityManagerFactory( //
+    LocalContainerEntityManagerFactoryBean authorizationEntityManagerFactory( //
             @Qualifier("authorizationVendorAdapter")
                     HibernateJpaVendorAdapter authorizationVendorAdapter,
             @Qualifier("authorizationDataSource") DataSource dataSource,
@@ -68,7 +68,7 @@ public LocalContainerEntityManagerFactoryBean authorizationEntityManagerFactory(
     }
 
     @Bean("authorizationTransactionManager")
-    public JpaTransactionManager authorizationTransactionManager(
+    JpaTransactionManager authorizationTransactionManager(
             @Qualifier("authorizationEntityManagerFactory") final EntityManagerFactory emf) {
 
         JpaTransactionManager transactionManager = new JpaTransactionManager();

src/integration/spring/domain/src/main/java/org/geoserver/acl/config/domain/AdminRuleAdminServiceConfiguration.java

@@ -15,7 +15,7 @@
 public class AdminRuleAdminServiceConfiguration {
 
     @Bean
-    public AdminRuleAdminService adminRuleAdminService(
+    AdminRuleAdminService adminRuleAdminService(
             AdminRuleRepository repository, ApplicationEventPublisher eventPublisher) {
         AdminRuleAdminService service = new AdminRuleAdminServiceImpl(repository);
         service.setEventPublisher(eventPublisher::publishEvent);

src/integration/spring/domain/src/main/java/org/geoserver/acl/config/domain/AuthorizationServiceConfiguration.java

@@ -15,7 +15,7 @@
 public class AuthorizationServiceConfiguration {
 
     @Bean
-    public AuthorizationService aclAuthorizationService(
+    AuthorizationService aclAuthorizationService(
             AdminRuleAdminService adminRuleService, RuleAdminService ruleService) {
         return new AuthorizationServiceImpl(adminRuleService, ruleService);
     }

src/integration/spring/domain/src/main/java/org/geoserver/acl/config/domain/RuleAdminServiceConfiguration.java

@@ -15,7 +15,7 @@
 public class RuleAdminServiceConfiguration {
 
     @Bean
-    public RuleAdminService ruleAdminService(
+    RuleAdminService ruleAdminService(
             RuleRepository ruleRepository, ApplicationEventPublisher eventPublisher) {
         RuleAdminService service = new RuleAdminServiceImpl(ruleRepository);
         service.setEventPublisher(eventPublisher::publishEvent);

src/plugin/accessmanager/src/main/java/org/geoserver/acl/plugin/accessmanager/ACLDispatcherCallback.java

@@ -31,7 +31,6 @@
 import org.geotools.api.style.Style;
 import org.geotools.factory.CommonFactoryFinder;
 import org.geotools.util.logging.Logging;
-import org.springframework.security.authentication.AnonymousAuthenticationToken;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.context.SecurityContextHolder;
 
@@ -81,21 +80,10 @@ public Operation operationDispatched(Request gsRequest, Operation operation) {
 
         // get the user
         Authentication user = SecurityContextHolder.getContext().getAuthentication();
-        String username = null;
-        if ((user != null) && !(user instanceof AnonymousAuthenticationToken)) {
-            // shortcut, if the user is the admin, he can do everything
-            if (ACLResourceAccessManager.isAdmin(user)) {
-                LOGGER.log(
-                        Level.FINE,
-                        "Admin level access, not applying default style for this request");
-
-                return operation;
-            } else {
-                username = user.getName();
-                if (username != null && username.isEmpty()) {
-                    username = null;
-                }
-            }
+        // shortcut, if the user is the admin, he can do everything
+        if (ACLResourceAccessManager.isAdmin(user)) {
+            LOGGER.finer("Admin level access, not applying default style for this request");
+            return operation;
         }
 
         if ((request != null)
@@ -154,7 +142,6 @@ void overrideGetLegendGraphicRequest(
             ResourceInfo resource = layer.getResource();
 
             // get the rule, it contains default and allowed styles
-            //            RuleFilter ruleFilter = new RuleFilter(SpecialFilterType.DEFAULT);
             AccessRequest ruleFilter =
                     new AccessRequestBuilder(configProvider.get())
                             .user(user)
@@ -164,7 +151,7 @@ void overrideGetLegendGraphicRequest(
                             .layer(resource.getName())
                             .build();
 
-            LOGGER.log(Level.FINE, "Getting access limits for getLegendGraphic", ruleFilter);
+            LOGGER.log(Level.FINEST, "Getting access limits for getLegendGraphic: {0}", ruleFilter);
             AccessInfo grant = aclService.getAccessInfo(ruleFilter);
 
             // get the requested style
@@ -224,18 +211,6 @@ void overrideGetMapRequest(
             }
 
             // get the rule, it contains default and allowed styles
-            //            RuleFilter ruleFilter = new RuleFilter(SpecialFilterType.DEFAULT);
-            //
-            // ruleFilter.setInstance(configurationManager.getConfiguration().getInstanceName());
-            //            ruleFilter.setService(service);
-            //            ruleFilter.setRequest(request);
-            //            if (info != null) {
-            //                ruleFilter.setWorkspace(info.getStore().getWorkspace().getName());
-            //                ruleFilter.setLayer(info.getName());
-            //            } else {
-            //                ruleFilter.setWorkspace(SpecialFilterType.DEFAULT);
-            //                ruleFilter.setLayer(SpecialFilterType.DEFAULT);
-            //            }
             AccessRequest ruleFilter;
             {
                 String workspace = info == null ? null : info.getStore().getWorkspace().getName();
@@ -249,7 +224,7 @@ void overrideGetMapRequest(
                                 .layer(layerName)
                                 .build();
             }
-            LOGGER.log(Level.FINE, "Getting access limits for getMap", ruleFilter);
+            LOGGER.log(Level.FINEST, "Getting access limits for getMap {0}:", ruleFilter);
 
             AccessInfo rule = aclService.getAccessInfo(ruleFilter);