Fix fine-grained permissions config (#2688)

* disabled by default
* evaluaton context considers fine-grained only when acm is enabled

Signed-off-by: Avgustin Marinov <Avgustin.Marinov@bosch.com>

hawkbit-ddi/hawkbit-ddi-server/src/test/java/org/eclipse/hawkbit/app/ddi/PreAuthorizeEnabledTest.java

@@ -23,7 +23,7 @@ import org.springframework.test.context.TestPropertySource;
  * Feature: Integration Test - Security<br/>
  * Story: PreAuthorized enabled
  */
-@TestPropertySource(properties = { "spring.flyway.enabled=true" })
+@TestPropertySource(properties = { "spring.flyway.enabled=true", "hawkbit.acm.access-controller.enabled=false" })
 class PreAuthorizeEnabledTest extends AbstractSecurityTest {
 
     /**

hawkbit-mgmt/hawkbit-mgmt-server/src/test/java/org/eclipse/hawkbit/app/mgmt/PreAuthorizeEnabledTest.java

@@ -21,11 +21,13 @@ import org.eclipse.hawkbit.im.authentication.SpRole;
 import org.eclipse.hawkbit.repository.test.util.WithUser;
 import org.junit.jupiter.api.Test;
 import org.springframework.http.HttpStatus;
+import org.springframework.test.context.TestPropertySource;
 
 /**
  * Feature: Integration Test - Security<br/>
  * Story: PreAuthorized enabled
  */
+@TestPropertySource(properties = "hawkbit.acm.access-controller.enabled=true")
 class PreAuthorizeEnabledTest extends AbstractSecurityTest {
 
     /**

hawkbit-monolith/hawkbit-update-server/src/test/java/org/eclipse/hawkbit/app/PreAuthorizeEnabledTest.java

@@ -21,11 +21,13 @@ import org.eclipse.hawkbit.im.authentication.SpRole;
 import org.eclipse.hawkbit.repository.test.util.WithUser;
 import org.junit.jupiter.api.Test;
 import org.springframework.http.HttpStatus;
+import org.springframework.test.context.TestPropertySource;
 
 /**
  * Feature: Integration Test - Security<br/>
  * Story: PreAuthorized enabled
  */
+@TestPropertySource(properties = "hawkbit.acm.access-controller.enabled=true")
 class PreAuthorizeEnabledTest extends AbstractSecurityTest {
 
     /**

hawkbit-repository/hawkbit-repository-core/src/main/java/org/eclipse/hawkbit/repository/RepositoryConfiguration.java

@@ -9,8 +9,6 @@
  */
 package org.eclipse.hawkbit.repository;
 
-import java.util.Collection;
-import java.util.List;
 import java.util.Optional;
 import java.util.function.Supplier;
 
@@ -38,7 +36,6 @@ import org.springframework.security.access.hierarchicalroles.RoleHierarchyImpl;
 import org.springframework.security.config.annotation.method.configuration.EnableMethodSecurity;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.GrantedAuthority;
-import org.springframework.security.core.authority.SimpleGrantedAuthority;
 import org.springframework.util.ObjectUtils;
 import org.springframework.util.function.SingletonSupplier;
 
@@ -95,84 +92,14 @@ public class RepositoryConfiguration {
     }
 
     @Bean
-    @Primary
+    @ConditionalOnMissingBean
     MethodSecurityExpressionHandler methodSecurityExpressionHandler(
             final RoleHierarchy roleHierarchy, final PermissionEvaluator permissionEvaluator,
             final Optional<ApplicationContext> applicationContext) {
-        final DefaultMethodSecurityExpressionHandler methodSecurityExpressionHandler = new DefaultMethodSecurityExpressionHandler() {
+        final DefaultMethodSecurityExpressionHandler methodSecurityExpressionHandler = new DefaultMethodSecurityExpressionHandler() {};
-
-            @Override
-            public EvaluationContext createEvaluationContext(final Supplier<Authentication> authentication, final MethodInvocation mi) {
-                return super.createEvaluationContext(SingletonSupplier.of(() -> new RawAuthoritiesAuthentication(authentication.get())), mi);
-            }
-
-            @Override
-            protected MethodSecurityExpressionOperations createSecurityExpressionRoot(
-                    final Authentication authentication, final MethodInvocation mi) {
-                return super.createSecurityExpressionRoot(new RawAuthoritiesAuthentication(authentication), mi);
-            }
-        };
         methodSecurityExpressionHandler.setRoleHierarchy(roleHierarchy);
         methodSecurityExpressionHandler.setPermissionEvaluator(permissionEvaluator);
         applicationContext.ifPresent(methodSecurityExpressionHandler::setApplicationContext);
         return methodSecurityExpressionHandler;
     }
-
-    private static class RawAuthoritiesAuthentication implements Authentication {
-
-        private final Authentication authentication;
-        private final transient SingletonSupplier<List<? extends GrantedAuthority>> rawAuthoritiesSupplier;
-
-        public RawAuthoritiesAuthentication(final Authentication authentication) {
-            this.authentication = authentication;
-            rawAuthoritiesSupplier = SingletonSupplier.of(
-                    () -> authentication.getAuthorities().stream()
-                            .map(GrantedAuthority::getAuthority)// get the authority
-                            .map(authority -> {
-                                // permissions are in the format UPDATE_TARGET(/<rsql query>).
-                                // here we remove the rsql query - not supported by expression evaluation
-                                // the rsql evaluation will be done later by the access controller
-                                final int index = authority.indexOf('/');
-                                return index < 0 ? authority : authority.substring(0, index);
-                            })
-                            .distinct() // remove duplicates if any
-                            .map(SimpleGrantedAuthority::new)
-                            .toList());
-        }
-
-        @Override
-        public Collection<? extends GrantedAuthority> getAuthorities() {
-            return rawAuthoritiesSupplier.get();
-        }
-
-        @Override
-        public Object getCredentials() {
-            return authentication.getCredentials();
-        }
-
-        @Override
-        public Object getDetails() {
-            return authentication.getDetails();
-        }
-
-        @Override
-        public Object getPrincipal() {
-            return authentication.getPrincipal();
-        }
-
-        @Override
-        public boolean isAuthenticated() {
-            return authentication.isAuthenticated();
-        }
-
-        @Override
-        public void setAuthenticated(final boolean isAuthenticated) throws IllegalArgumentException {
-            throw new UnsupportedOperationException();
-        }
-
-        @Override
-        public String getName() {
-            return authentication.getName();
-        }
-    }
 }

hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/acm/AccessControllerConfiguration.java

@@ -10,13 +10,17 @@
 package org.eclipse.hawkbit.repository.jpa.acm;
 
 import java.lang.reflect.Proxy;
+import java.util.Collection;
+import java.util.List;
 import java.util.Optional;
+import java.util.function.Supplier;
 
 import jakarta.persistence.criteria.From;
 import jakarta.persistence.criteria.Join;
 import jakarta.persistence.criteria.Root;
 import jakarta.persistence.metamodel.EntityType;
 
+import org.aopalliance.intercept.MethodInvocation;
 import org.eclipse.hawkbit.im.authentication.SpPermission;
 import org.eclipse.hawkbit.repository.DistributionSetFields;
 import org.eclipse.hawkbit.repository.DistributionSetTypeFields;
@@ -33,14 +37,34 @@ import org.eclipse.hawkbit.repository.jpa.model.JpaSoftwareModule;
 import org.eclipse.hawkbit.repository.jpa.model.JpaSoftwareModuleType;
 import org.eclipse.hawkbit.repository.jpa.model.JpaTarget;
 import org.eclipse.hawkbit.repository.jpa.model.JpaTargetType;
+import org.eclipse.hawkbit.security.SecurityContextSerializer;
+import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
+import org.springframework.context.ApplicationContext;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
+import org.springframework.context.annotation.Primary;
 import org.springframework.data.jpa.domain.Specification;
+import org.springframework.expression.EvaluationContext;
+import org.springframework.security.access.PermissionEvaluator;
+import org.springframework.security.access.expression.method.DefaultMethodSecurityExpressionHandler;
+import org.springframework.security.access.expression.method.MethodSecurityExpressionHandler;
+import org.springframework.security.access.expression.method.MethodSecurityExpressionOperations;
+import org.springframework.security.access.hierarchicalroles.RoleHierarchy;
+import org.springframework.security.core.Authentication;
+import org.springframework.security.core.GrantedAuthority;
+import org.springframework.security.core.authority.SimpleGrantedAuthority;
+import org.springframework.util.function.SingletonSupplier;
 
 @Configuration
-@ConditionalOnProperty(name = "hawkbit.acm.access-controller.enabled", havingValue = "true", matchIfMissing = true)
+@ConditionalOnProperty(name = "hawkbit.acm.access-controller.enabled", havingValue = "true")
-public class DefaultAccessControllerConfiguration {
+public class AccessControllerConfiguration {
+
+    @Bean
+    @ConditionalOnMissingBean
+    SecurityContextSerializer securityContextSerializer() {
+        return SecurityContextSerializer.JSON_SERIALIZATION;
+    }
 
     @Bean
     @ConditionalOnProperty(name = "hawkbit.acm.access-controller.target.enabled", havingValue = "true", matchIfMissing = true)
@@ -111,4 +135,86 @@ public class DefaultAccessControllerConfiguration {
     AccessController<JpaDistributionSetType> distributionSetTypeAccessController() {
         return new DefaultAccessController<>(DistributionSetTypeFields.class, SpPermission.DISTRIBUTION_SET_TYPE);
     }
-}
+
+    @Bean
+    @Primary
+    MethodSecurityExpressionHandler methodSecurityExpressionHandler(
+            final RoleHierarchy roleHierarchy, final PermissionEvaluator permissionEvaluator,
+            final Optional<ApplicationContext> applicationContext) {
+        final DefaultMethodSecurityExpressionHandler methodSecurityExpressionHandler = new DefaultMethodSecurityExpressionHandler() {
+
+            @Override
+            public EvaluationContext createEvaluationContext(final Supplier<Authentication> authentication, final MethodInvocation mi) {
+                return super.createEvaluationContext(SingletonSupplier.of(() -> new RawAuthoritiesAuthentication(authentication.get())), mi);
+            }
+
+            @Override
+            protected MethodSecurityExpressionOperations createSecurityExpressionRoot(
+                    final Authentication authentication, final MethodInvocation mi) {
+                return super.createSecurityExpressionRoot(new RawAuthoritiesAuthentication(authentication), mi);
+            }
+        };
+        methodSecurityExpressionHandler.setRoleHierarchy(roleHierarchy);
+        methodSecurityExpressionHandler.setPermissionEvaluator(permissionEvaluator);
+        applicationContext.ifPresent(methodSecurityExpressionHandler::setApplicationContext);
+        return methodSecurityExpressionHandler;
+    }
+
+    private static class RawAuthoritiesAuthentication implements Authentication {
+
+        private final Authentication authentication;
+        private final transient SingletonSupplier<List<? extends GrantedAuthority>> rawAuthoritiesSupplier;
+
+        public RawAuthoritiesAuthentication(final Authentication authentication) {
+            this.authentication = authentication;
+            rawAuthoritiesSupplier = SingletonSupplier.of(
+                    () -> authentication.getAuthorities().stream()
+                            .map(GrantedAuthority::getAuthority)// get the authority
+                            .map(authority -> {
+                                // permissions are in the format UPDATE_TARGET(/<rsql query>).
+                                // here we remove the rsql query - not supported by expression evaluation
+                                // the rsql evaluation will be done later by the access controller
+                                final int index = authority.indexOf('/');
+                                return index < 0 ? authority : authority.substring(0, index);
+                            })
+                            .distinct() // remove duplicates if any
+                            .map(SimpleGrantedAuthority::new)
+                            .toList());
+        }
+
+        @Override
+        public Collection<? extends GrantedAuthority> getAuthorities() {
+            return rawAuthoritiesSupplier.get();
+        }
+
+        @Override
+        public Object getCredentials() {
+            return authentication.getCredentials();
+        }
+
+        @Override
+        public Object getDetails() {
+            return authentication.getDetails();
+        }
+
+        @Override
+        public Object getPrincipal() {
+            return authentication.getPrincipal();
+        }
+
+        @Override
+        public boolean isAuthenticated() {
+            return authentication.isAuthenticated();
+        }
+
+        @Override
+        public void setAuthenticated(final boolean isAuthenticated) throws IllegalArgumentException {
+            throw new UnsupportedOperationException();
+        }
+
+        @Override
+        public String getName() {
+            return authentication.getName();
+        }
+    }
+}

hawkbit-repository/hawkbit-repository-jpa/src/main/resources/META-INF/spring/org.springframework.boot.autoconfigure.AutoConfiguration.imports

@@ -0,0 +1 @@
+org.eclipse.hawkbit.repository.jpa.acm.AccessControllerConfiguration

hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/ActionAccessControllerTest.java

@@ -25,8 +25,10 @@ import org.eclipse.hawkbit.repository.model.TargetType;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.springframework.test.context.ContextConfiguration;
+import org.springframework.test.context.TestPropertySource;
 
-@ContextConfiguration(classes = { DefaultAccessControllerConfiguration.class })
+@ContextConfiguration(classes = { AccessControllerConfiguration.class })
+@TestPropertySource(properties = "hawkbit.acm.access-controller.enabled=true")
 class ActionAccessControllerTest extends AbstractJpaIntegrationTest {
 
     private TargetType targetType1;

hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/DistributionSetAccessControllerTest.java

@@ -38,6 +38,7 @@ import org.eclipse.hawkbit.repository.model.TargetFilterQuery;
 import org.junit.jupiter.api.Test;
 import org.springframework.data.domain.Pageable;
 import org.springframework.test.context.ContextConfiguration;
+import org.springframework.test.context.TestPropertySource;
 
 /**
  * Note: Still all test gets READ_REPOSITORY since find methods are inherited with request for READ_REPOSITORY. However,
@@ -46,7 +47,8 @@ import org.springframework.test.context.ContextConfiguration;
  * Feature: Component Tests - Access Control<br/>
  * Story: Test Distribution Set Access Controller
  */
-@ContextConfiguration(classes = { DefaultAccessControllerConfiguration.class })
+@ContextConfiguration(classes = { AccessControllerConfiguration.class })
+@TestPropertySource(properties = "hawkbit.acm.access-controller.enabled=true")
 class DistributionSetAccessControllerTest extends AbstractJpaIntegrationTest {
 
     /**

hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/TargetAccessControllerTest.java

@@ -44,12 +44,14 @@ import org.junit.jupiter.api.Test;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.data.domain.Pageable;
 import org.springframework.test.context.ContextConfiguration;
+import org.springframework.test.context.TestPropertySource;
 
 /**
  * Feature: Component Tests - Access Control<br/>
  * Story: Test Target Access Controller
  */
-@ContextConfiguration(classes = { DefaultAccessControllerConfiguration.class, AcmTestConfiguration.class })
+@ContextConfiguration(classes = { AccessControllerConfiguration.class, AcmTestConfiguration.class })
+@TestPropertySource(properties = "hawkbit.acm.access-controller.enabled=true")
 class TargetAccessControllerTest extends AbstractJpaIntegrationTest {
 
     @Autowired

hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/TargetTypeAccessControllerTest.java

@@ -37,8 +37,8 @@ import org.springframework.test.context.TestPropertySource;
  * Feature: Component Tests - Access Control<br/>
  * Story: Test Target Type Access Controller
  */
-@ContextConfiguration(classes = { DefaultAccessControllerConfiguration.class })
+@ContextConfiguration(classes = { AccessControllerConfiguration.class })
-@TestPropertySource(properties = { "hawkbit.acm.access-controller.target-type.enabled=true" })
+@TestPropertySource(properties = { "hawkbit.acm.access-controller.target-type.enabled=true", "hawkbit.acm.access-controller.enabled=true" })
 class TargetTypeAccessControllerTest extends AbstractJpaIntegrationTest {
 
     /**