From c20ee8bdf35fb8ea49f558ce9044ed685c1afeb2 Mon Sep 17 00:00:00 2001 From: Avgustin Marinov Date: Wed, 24 Sep 2025 09:40:27 +0300 Subject: [PATCH] Fix fine-grained permissions config (#2688) * disabled by default * evaluaton context considers fine-grained only when acm is enabled Signed-off-by: Avgustin Marinov --- .../app/ddi/PreAuthorizeEnabledTest.java | 2 +- .../app/mgmt/PreAuthorizeEnabledTest.java | 2 + .../hawkbit/app/PreAuthorizeEnabledTest.java | 2 + .../repository/RepositoryConfiguration.java | 77 +----------- ...ava => AccessControllerConfiguration.java} | 112 +++++++++++++++++- ...ot.autoconfigure.AutoConfiguration.imports | 1 + .../jpa/acm/ActionAccessControllerTest.java | 4 +- .../DistributionSetAccessControllerTest.java | 4 +- .../jpa/acm/TargetAccessControllerTest.java | 4 +- .../acm/TargetTypeAccessControllerTest.java | 4 +- 10 files changed, 128 insertions(+), 84 deletions(-) rename hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/acm/{DefaultAccessControllerConfiguration.java => AccessControllerConfiguration.java} (53%) create mode 100644 hawkbit-repository/hawkbit-repository-jpa/src/main/resources/META-INF/spring/org.springframework.boot.autoconfigure.AutoConfiguration.imports diff --git a/hawkbit-ddi/hawkbit-ddi-server/src/test/java/org/eclipse/hawkbit/app/ddi/PreAuthorizeEnabledTest.java b/hawkbit-ddi/hawkbit-ddi-server/src/test/java/org/eclipse/hawkbit/app/ddi/PreAuthorizeEnabledTest.java index 865ea9d8a..0c298134e 100644 --- a/hawkbit-ddi/hawkbit-ddi-server/src/test/java/org/eclipse/hawkbit/app/ddi/PreAuthorizeEnabledTest.java +++ b/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
* 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 { /** diff --git a/hawkbit-mgmt/hawkbit-mgmt-server/src/test/java/org/eclipse/hawkbit/app/mgmt/PreAuthorizeEnabledTest.java b/hawkbit-mgmt/hawkbit-mgmt-server/src/test/java/org/eclipse/hawkbit/app/mgmt/PreAuthorizeEnabledTest.java index 1fea696d6..9c7e346d2 100644 --- a/hawkbit-mgmt/hawkbit-mgmt-server/src/test/java/org/eclipse/hawkbit/app/mgmt/PreAuthorizeEnabledTest.java +++ b/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
* Story: PreAuthorized enabled */ +@TestPropertySource(properties = "hawkbit.acm.access-controller.enabled=true") class PreAuthorizeEnabledTest extends AbstractSecurityTest { /** diff --git a/hawkbit-monolith/hawkbit-update-server/src/test/java/org/eclipse/hawkbit/app/PreAuthorizeEnabledTest.java b/hawkbit-monolith/hawkbit-update-server/src/test/java/org/eclipse/hawkbit/app/PreAuthorizeEnabledTest.java index c9bcf5d97..c5ee98b5b 100644 --- a/hawkbit-monolith/hawkbit-update-server/src/test/java/org/eclipse/hawkbit/app/PreAuthorizeEnabledTest.java +++ b/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
* Story: PreAuthorized enabled */ +@TestPropertySource(properties = "hawkbit.acm.access-controller.enabled=true") class PreAuthorizeEnabledTest extends AbstractSecurityTest { /** diff --git a/hawkbit-repository/hawkbit-repository-core/src/main/java/org/eclipse/hawkbit/repository/RepositoryConfiguration.java b/hawkbit-repository/hawkbit-repository-core/src/main/java/org/eclipse/hawkbit/repository/RepositoryConfiguration.java index 72da94c40..25f91d8dd 100644 --- a/hawkbit-repository/hawkbit-repository-core/src/main/java/org/eclipse/hawkbit/repository/RepositoryConfiguration.java +++ b/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) { - final DefaultMethodSecurityExpressionHandler methodSecurityExpressionHandler = new DefaultMethodSecurityExpressionHandler() { - - @Override - public EvaluationContext createEvaluationContext(final Supplier 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); - } - }; + final DefaultMethodSecurityExpressionHandler methodSecurityExpressionHandler = new DefaultMethodSecurityExpressionHandler() {}; 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> 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(/). - // 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 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(); - } - } } \ No newline at end of file diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/acm/DefaultAccessControllerConfiguration.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/acm/AccessControllerConfiguration.java similarity index 53% rename from hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/acm/DefaultAccessControllerConfiguration.java rename to hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/acm/AccessControllerConfiguration.java index 7b9784d3a..0d54612d5 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/acm/DefaultAccessControllerConfiguration.java +++ b/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) -public class DefaultAccessControllerConfiguration { +@ConditionalOnProperty(name = "hawkbit.acm.access-controller.enabled", havingValue = "true") +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 distributionSetTypeAccessController() { return new DefaultAccessController<>(DistributionSetTypeFields.class, SpPermission.DISTRIBUTION_SET_TYPE); } -} \ No newline at end of file + + @Bean + @Primary + MethodSecurityExpressionHandler methodSecurityExpressionHandler( + final RoleHierarchy roleHierarchy, final PermissionEvaluator permissionEvaluator, + final Optional applicationContext) { + final DefaultMethodSecurityExpressionHandler methodSecurityExpressionHandler = new DefaultMethodSecurityExpressionHandler() { + + @Override + public EvaluationContext createEvaluationContext(final Supplier 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> 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(/). + // 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 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(); + } + } +} diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/resources/META-INF/spring/org.springframework.boot.autoconfigure.AutoConfiguration.imports b/hawkbit-repository/hawkbit-repository-jpa/src/main/resources/META-INF/spring/org.springframework.boot.autoconfigure.AutoConfiguration.imports new file mode 100644 index 000000000..77e1bb58e --- /dev/null +++ b/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 \ No newline at end of file diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/ActionAccessControllerTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/ActionAccessControllerTest.java index 6341096c5..161a28905 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/ActionAccessControllerTest.java +++ b/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; diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/DistributionSetAccessControllerTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/DistributionSetAccessControllerTest.java index d7c902e0e..30560b965 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/DistributionSetAccessControllerTest.java +++ b/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
* 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 { /** diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/TargetAccessControllerTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/TargetAccessControllerTest.java index 6e8069e9d..5f6d34793 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/TargetAccessControllerTest.java +++ b/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
* 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 diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/TargetTypeAccessControllerTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/TargetTypeAccessControllerTest.java index c7f261e9c..d1363af08 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/TargetTypeAccessControllerTest.java +++ b/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
* Story: Test Target Type Access Controller */ -@ContextConfiguration(classes = { DefaultAccessControllerConfiguration.class }) -@TestPropertySource(properties = { "hawkbit.acm.access-controller.target-type.enabled=true" }) +@ContextConfiguration(classes = { AccessControllerConfiguration.class }) +@TestPropertySource(properties = { "hawkbit.acm.access-controller.target-type.enabled=true", "hawkbit.acm.access-controller.enabled=true" }) class TargetTypeAccessControllerTest extends AbstractJpaIntegrationTest { /**