From 8abf7275c4ee659e62678df43749c5733137acee Mon Sep 17 00:00:00 2001 From: Avgustin Marinov Date: Wed, 13 Aug 2025 12:12:16 +0300 Subject: [PATCH] Unified secman test (#2606) * Unified Security Management Test Signed-off-by: Avgustin Marinov * Add unified ManagementSecurityTest Signed-off-by: Avgustin Marinov --------- Signed-off-by: Avgustin Marinov --- .../resource/MgmtDistributionSetResource.java | 12 +- ...DistributionSetInvalidationManagement.java | 40 +- .../hawkbit-repository-jpa/pom.xml | 5 + .../jpa/JpaRepositoryConfiguration.java | 4 +- .../repository/jpa/JpaRolloutHandler.java | 5 +- .../AbstractJpaRepositoryManagement.java | 2 +- ...DistributionSetInvalidationManagement.java | 50 +- ...ributionSetInvalidationManagementTest.java | 75 ++- .../management/ManagementSecurityTest.java | 458 ++++++++++++++++++ 9 files changed, 538 insertions(+), 113 deletions(-) create mode 100644 hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/ManagementSecurityTest.java diff --git a/hawkbit-mgmt/hawkbit-mgmt-resource/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtDistributionSetResource.java b/hawkbit-mgmt/hawkbit-mgmt-resource/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtDistributionSetResource.java index 6004ab33f..79c364c51 100644 --- a/hawkbit-mgmt/hawkbit-mgmt-resource/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtDistributionSetResource.java +++ b/hawkbit-mgmt/hawkbit-mgmt-resource/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtDistributionSetResource.java @@ -14,7 +14,6 @@ import static org.eclipse.hawkbit.mgmt.rest.resource.util.PagingUtility.sanitize import java.text.MessageFormat; import java.util.AbstractMap.SimpleEntry; import java.util.Collection; -import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -69,7 +68,6 @@ import org.eclipse.hawkbit.security.SystemSecurityContext; import org.eclipse.hawkbit.utils.TenantConfigHelper; import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; -import org.springframework.data.domain.Slice; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.RestController; @@ -316,7 +314,8 @@ public class MgmtDistributionSetResource implements MgmtDistributionSetRestApi { final int pagingOffsetParam, final int pagingLimitParam, final String sortParam) { final Pageable pageable = PagingUtility.toPageable(pagingOffsetParam, pagingLimitParam, sanitizeDistributionSetSortParam(sortParam)); final Page softwareModules = softwareModuleManagement.findByAssignedTo(distributionSetId, pageable); - return ResponseEntity.ok(new PagedList<>(MgmtSoftwareModuleMapper.toResponse(softwareModules.getContent()), softwareModules.getTotalElements())); + return ResponseEntity.ok( + new PagedList<>(MgmtSoftwareModuleMapper.toResponse(softwareModules.getContent()), softwareModules.getTotalElements())); } @Override @@ -357,10 +356,9 @@ public class MgmtDistributionSetResource implements MgmtDistributionSetRestApi { @AuditLog(entity = "DistributionSet", type = AuditLog.Type.DELETE, description = "Invalidate Distribution Set") public ResponseEntity invalidateDistributionSet( final Long distributionSetId, final MgmtInvalidateDistributionSetRequestBody invalidateRequestBody) { - distributionSetInvalidationManagement - .invalidateDistributionSet( - new DistributionSetInvalidation(Collections.singletonList(distributionSetId), - MgmtRestModelMapper.convertCancelationType(invalidateRequestBody.getActionCancelationType()))); + distributionSetInvalidationManagement.invalidateDistributionSet(new DistributionSetInvalidation( + List.of(distributionSetId), + MgmtRestModelMapper.convertCancelationType(invalidateRequestBody.getActionCancelationType()))); return ResponseEntity.ok().build(); } } \ No newline at end of file diff --git a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/DistributionSetInvalidationManagement.java b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/DistributionSetInvalidationManagement.java index 2a92efce0..115cf50b3 100644 --- a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/DistributionSetInvalidationManagement.java +++ b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/DistributionSetInvalidationManagement.java @@ -9,43 +9,31 @@ */ package org.eclipse.hawkbit.repository; +import org.eclipse.hawkbit.im.authentication.SpPermission; +import org.eclipse.hawkbit.im.authentication.SpringEvalExpressions; import org.eclipse.hawkbit.repository.model.DistributionSet; import org.eclipse.hawkbit.repository.model.DistributionSetInvalidation; import org.eclipse.hawkbit.repository.model.DistributionSetInvalidationCount; import org.springframework.security.access.prepost.PreAuthorize; /** - * A DistributionSetInvalidationManagement service provides operations to - * invalidate {@link DistributionSet}s. + * A DistributionSetInvalidationManagement service provides operations to invalidate {@link DistributionSet}s. */ -public interface DistributionSetInvalidationManagement { +public interface DistributionSetInvalidationManagement extends PermissionSupport { + + @Override + default String permissionGroup() { + return SpPermission.DISTRIBUTION_SET; + } /** - * Invalidates a given {@link DistributionSet}. The invalidation always - * cancels all auto assignments referring this {@link DistributionSet} and - * can not be undone. Optionally, all rollouts and actions referring this - * {@link DistributionSet} can be canceled. + * Invalidates a given {@link DistributionSet}. The invalidation always cancels all auto assignments referring this {@link DistributionSet} + * and can not be undone. Optionally, all rollouts and actions referring this {@link DistributionSet} can be canceled. *

* {@link PreAuthorize} missing intentionally as it relies on the permission set defined in the management api methods that it calls internally. * - * @param distributionSetInvalidation defines the {@link DistributionSet} and options what should be - * canceled + * @param distributionSetInvalidation defines the {@link DistributionSet} and options what should be canceled */ + @PreAuthorize(SpringEvalExpressions.HAS_UPDATE_REPOSITORY) void invalidateDistributionSet(final DistributionSetInvalidation distributionSetInvalidation); - - /** - * Counts all entities for a list of {@link DistributionSet}s that will be - * canceled when invalidation is called for those {@link DistributionSet}s. - *

- * {@link PreAuthorize} missing intentionally as it relies on the permission set defined in the management api methods that it calls internally. - * - * @param distributionSetInvalidation defines the {@link DistributionSet} and options what should be - * canceled - * @return The {@link DistributionSetInvalidationCount} object that holds - * information about the count of affected rollouts, - * auto-assignments and actions - */ - DistributionSetInvalidationCount countEntitiesForInvalidation( - final DistributionSetInvalidation distributionSetInvalidation); - -} +} \ No newline at end of file diff --git a/hawkbit-repository/hawkbit-repository-jpa/pom.xml b/hawkbit-repository/hawkbit-repository-jpa/pom.xml index 8426cdb0b..c33f23a9e 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/pom.xml +++ b/hawkbit-repository/hawkbit-repository-jpa/pom.xml @@ -159,5 +159,10 @@ hibernate-micrometer test + + io.github.classgraph + classgraph + test + diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaRepositoryConfiguration.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaRepositoryConfiguration.java index a5760f49d..82c807036 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaRepositoryConfiguration.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaRepositoryConfiguration.java @@ -359,8 +359,8 @@ public class JpaRepositoryConfiguration { @ConditionalOnMissingBean RolloutHandler rolloutHandler(final TenantAware tenantAware, final RolloutManagement rolloutManagement, final RolloutExecutor rolloutExecutor, final LockRegistry lockRegistry, - final PlatformTransactionManager txManager, final ContextAware contextAware, final Optional meterRegistry) { - return new JpaRolloutHandler(tenantAware, rolloutManagement, rolloutExecutor, lockRegistry, txManager, contextAware, meterRegistry); + final PlatformTransactionManager txManager, final Optional meterRegistry) { + return new JpaRolloutHandler(tenantAware, rolloutManagement, rolloutExecutor, lockRegistry, txManager, meterRegistry); } @Bean diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaRolloutHandler.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaRolloutHandler.java index 1bf35f3fe..4d378cc1e 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaRolloutHandler.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaRolloutHandler.java @@ -37,7 +37,6 @@ public class JpaRolloutHandler implements RolloutHandler { private final RolloutExecutor rolloutExecutor; private final LockRegistry lockRegistry; private final PlatformTransactionManager txManager; - private final ContextAware contextAware; private final Optional meterRegistry; /** @@ -51,14 +50,12 @@ public class JpaRolloutHandler implements RolloutHandler { */ public JpaRolloutHandler(final TenantAware tenantAware, final RolloutManagement rolloutManagement, final RolloutExecutor rolloutExecutor, final LockRegistry lockRegistry, - final PlatformTransactionManager txManager, - final ContextAware contextAware, final Optional meterRegistry) { + final PlatformTransactionManager txManager, final Optional meterRegistry) { this.tenantAware = tenantAware; this.rolloutManagement = rolloutManagement; this.rolloutExecutor = rolloutExecutor; this.lockRegistry = lockRegistry; this.txManager = txManager; - this.contextAware = contextAware; this.meterRegistry = meterRegistry; } diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/AbstractJpaRepositoryManagement.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/AbstractJpaRepositoryManagement.java index 6d3ad1b73..a2dea5bf3 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/AbstractJpaRepositoryManagement.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/AbstractJpaRepositoryManagement.java @@ -69,7 +69,7 @@ import org.springframework.validation.annotation.Validated; */ // Spring AOP doesn't support bridge methods and the AspectJ advices as ExceptionMappingAspectHandler could not handle the // thrown exception (e.g. to convert AuthorizationDeniedException to InsufficientPermissionException). -// That's why we explictly handle the insufficient permission exception with this @HandleAuthorizationDenied annotation. +// That's why we explicitly handle the insufficient permission exception with this @HandleAuthorizationDenied annotation. @HandleAuthorizationDenied(handlerClass = JpaRepositoryConfiguration.ManagementExceptionThrowingMethodAuthorizationDeniedHandler.class) @Transactional(readOnly = true) @Validated diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaDistributionSetInvalidationManagement.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaDistributionSetInvalidationManagement.java index 389db8ef9..45a95fa38 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaDistributionSetInvalidationManagement.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaDistributionSetInvalidationManagement.java @@ -49,7 +49,6 @@ public class JpaDistributionSetInvalidationManagement implements DistributionSet private final RolloutManagement rolloutManagement; private final DeploymentManagement deploymentManagement; private final TargetFilterQueryManagement targetFilterQueryManagement; - private final ActionRepository actionRepository; private final PlatformTransactionManager txManager; private final RepositoryProperties repositoryProperties; private final TenantAware tenantAware; @@ -60,7 +59,7 @@ public class JpaDistributionSetInvalidationManagement implements DistributionSet protected JpaDistributionSetInvalidationManagement( final DistributionSetManagement distributionSetManagement, final RolloutManagement rolloutManagement, final DeploymentManagement deploymentManagement, - final TargetFilterQueryManagement targetFilterQueryManagement, final ActionRepository actionRepository, + final TargetFilterQueryManagement targetFilterQueryManagement, final PlatformTransactionManager txManager, final RepositoryProperties repositoryProperties, final TenantAware tenantAware, final LockRegistry lockRegistry, final SystemSecurityContext systemSecurityContext) { @@ -68,7 +67,6 @@ public class JpaDistributionSetInvalidationManagement implements DistributionSet this.rolloutManagement = rolloutManagement; this.deploymentManagement = deploymentManagement; this.targetFilterQueryManagement = targetFilterQueryManagement; - this.actionRepository = actionRepository; this.txManager = txManager; this.repositoryProperties = repositoryProperties; this.tenantAware = tenantAware; @@ -103,26 +101,11 @@ public class JpaDistributionSetInvalidationManagement implements DistributionSet } } - @Override - public DistributionSetInvalidationCount countEntitiesForInvalidation( - final DistributionSetInvalidation distributionSetInvalidation) { - return systemSecurityContext.runAsSystem(() -> { - final Collection setIds = distributionSetInvalidation.getDistributionSetIds(); - final long rolloutsCount = shouldRolloutsBeCanceled(distributionSetInvalidation.getActionCancellationType()) ? countRolloutsForInvalidation(setIds) : 0; - final long autoAssignmentsCount = countAutoAssignmentsForInvalidation(setIds); - final long actionsCount = countActionsForInvalidation(setIds, - distributionSetInvalidation.getActionCancellationType()); - - return new DistributionSetInvalidationCount(rolloutsCount, autoAssignmentsCount, actionsCount); - }); - } - private static boolean shouldRolloutsBeCanceled(final ActionCancellationType cancelationType) { return cancelationType != ActionCancellationType.NONE; } - private void invalidateDistributionSetsInTransaction(final DistributionSetInvalidation distributionSetInvalidation, - final String tenant) { + private void invalidateDistributionSetsInTransaction(final DistributionSetInvalidation distributionSetInvalidation, final String tenant) { DeploymentHelper.runInNewTransaction(txManager, tenant + "-invalidateDS", status -> { distributionSetInvalidation.getDistributionSetIds().forEach(setId -> invalidateDistributionSet(setId, distributionSetInvalidation.getActionCancellationType())); @@ -158,33 +141,4 @@ public class JpaDistributionSetInvalidationManagement implements DistributionSet return null; }); } - - private long countRolloutsForInvalidation(final Collection setIds) { - return setIds.stream().mapToLong(rolloutManagement::countByDistributionSetIdAndRolloutIsStoppable).sum(); - } - - private long countAutoAssignmentsForInvalidation(final Collection setIds) { - return setIds.stream().mapToLong(targetFilterQueryManagement::countByAutoAssignDistributionSetId).sum(); - } - - private long countActionsForInvalidation(final Collection setIds, final ActionCancellationType cancelationType) { - long affectedActionsByDSInvalidation = 0; - if (cancelationType == ActionCancellationType.FORCE) { - affectedActionsByDSInvalidation = countActionsForForcedInvalidation(setIds); - } else if (cancelationType == ActionCancellationType.SOFT) { - affectedActionsByDSInvalidation = countActionsForSoftInvalidation(setIds); - } - return affectedActionsByDSInvalidation; - } - - private long countActionsForForcedInvalidation(final Collection setIds) { - return setIds.stream().mapToLong(actionRepository::countByDistributionSetIdAndActiveIsTrue).sum(); - } - - private long countActionsForSoftInvalidation(final Collection setIds) { - return setIds.stream() - .mapToLong(distributionSet -> actionRepository - .countByDistributionSetIdAndActiveIsTrueAndStatusIsNot(distributionSet, Status.CANCELING)) - .sum(); - } } \ No newline at end of file diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/DistributionSetInvalidationManagementTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/DistributionSetInvalidationManagementTest.java index d72ae8c12..90a060461 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/DistributionSetInvalidationManagementTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/DistributionSetInvalidationManagementTest.java @@ -12,9 +12,11 @@ package org.eclipse.hawkbit.repository.jpa.management; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import java.util.Collection; import java.util.Collections; import java.util.List; +import lombok.Data; import lombok.extern.slf4j.Slf4j; import org.eclipse.hawkbit.repository.TargetFilterQueryManagement; import org.eclipse.hawkbit.repository.exception.IncompleteDistributionSetException; @@ -55,8 +57,7 @@ class DistributionSetInvalidationManagementTest extends AbstractJpaIntegrationTe final DistributionSetInvalidation distributionSetInvalidation = new DistributionSetInvalidation( Collections.singletonList(invalidationTestData.getDistributionSet().getId()), ActionCancellationType.NONE); - final DistributionSetInvalidationCount distributionSetInvalidationCount = distributionSetInvalidationManagement - .countEntitiesForInvalidation(distributionSetInvalidation); + final DistributionSetInvalidationCount distributionSetInvalidationCount = countEntitiesForInvalidation(distributionSetInvalidation); assertDistributionSetInvalidationCount(distributionSetInvalidationCount, 1, 0, 0); distributionSetInvalidationManagement.invalidateDistributionSet(distributionSetInvalidation); @@ -85,8 +86,7 @@ class DistributionSetInvalidationManagementTest extends AbstractJpaIntegrationTe final DistributionSetInvalidation distributionSetInvalidation = new DistributionSetInvalidation( Collections.singletonList(invalidationTestData.getDistributionSet().getId()), ActionCancellationType.NONE); - final DistributionSetInvalidationCount distributionSetInvalidationCount = distributionSetInvalidationManagement - .countEntitiesForInvalidation(distributionSetInvalidation); + final DistributionSetInvalidationCount distributionSetInvalidationCount = countEntitiesForInvalidation(distributionSetInvalidation); assertDistributionSetInvalidationCount(distributionSetInvalidationCount, 1, 0, 0); distributionSetInvalidationManagement.invalidateDistributionSet(distributionSetInvalidation); @@ -118,8 +118,7 @@ class DistributionSetInvalidationManagementTest extends AbstractJpaIntegrationTe final DistributionSetInvalidation distributionSetInvalidation = new DistributionSetInvalidation( Collections.singletonList(invalidationTestData.getDistributionSet().getId()), ActionCancellationType.FORCE); - final DistributionSetInvalidationCount distributionSetInvalidationCount = distributionSetInvalidationManagement - .countEntitiesForInvalidation(distributionSetInvalidation); + final DistributionSetInvalidationCount distributionSetInvalidationCount = countEntitiesForInvalidation(distributionSetInvalidation); assertDistributionSetInvalidationCount(distributionSetInvalidationCount, 1, 5, 1); distributionSetInvalidationManagement.invalidateDistributionSet(distributionSetInvalidation); @@ -142,8 +141,7 @@ class DistributionSetInvalidationManagementTest extends AbstractJpaIntegrationTe final DistributionSetInvalidation distributionSetInvalidation = new DistributionSetInvalidation( Collections.singletonList(invalidationTestData.getDistributionSet().getId()), ActionCancellationType.SOFT); - final DistributionSetInvalidationCount distributionSetInvalidationCount = distributionSetInvalidationManagement - .countEntitiesForInvalidation(distributionSetInvalidation); + final DistributionSetInvalidationCount distributionSetInvalidationCount = countEntitiesForInvalidation(distributionSetInvalidation); assertDistributionSetInvalidationCount(distributionSetInvalidationCount, 1, 5, 1); distributionSetInvalidationManagement.invalidateDistributionSet(distributionSetInvalidation); @@ -278,6 +276,48 @@ class DistributionSetInvalidationManagementTest extends AbstractJpaIntegrationTe return actionRepository.findAll(ActionSpecifications.byTargetControllerId(target.getControllerId())); } + private DistributionSetInvalidationCount countEntitiesForInvalidation( + final DistributionSetInvalidation distributionSetInvalidation) { + return systemSecurityContext.runAsSystem(() -> { + final Collection setIds = distributionSetInvalidation.getDistributionSetIds(); + final long rolloutsCount = distributionSetInvalidation.getActionCancellationType() != ActionCancellationType.NONE ? countRolloutsForInvalidation(setIds) : 0; + final long autoAssignmentsCount = countAutoAssignmentsForInvalidation(setIds); + final long actionsCount = countActionsForInvalidation(setIds, distributionSetInvalidation.getActionCancellationType()); + + return new DistributionSetInvalidationCount(rolloutsCount, autoAssignmentsCount, actionsCount); + }); + } + + private long countRolloutsForInvalidation(final Collection setIds) { + return setIds.stream().mapToLong(rolloutManagement::countByDistributionSetIdAndRolloutIsStoppable).sum(); + } + + private long countAutoAssignmentsForInvalidation(final Collection setIds) { + return setIds.stream().mapToLong(targetFilterQueryManagement::countByAutoAssignDistributionSetId).sum(); + } + + private long countActionsForInvalidation(final Collection setIds, final ActionCancellationType cancelationType) { + long affectedActionsByDSInvalidation = 0; + if (cancelationType == ActionCancellationType.FORCE) { + affectedActionsByDSInvalidation = countActionsForForcedInvalidation(setIds); + } else if (cancelationType == ActionCancellationType.SOFT) { + affectedActionsByDSInvalidation = countActionsForSoftInvalidation(setIds); + } + return affectedActionsByDSInvalidation; + } + + private long countActionsForForcedInvalidation(final Collection setIds) { + return setIds.stream().mapToLong(actionRepository::countByDistributionSetIdAndActiveIsTrue).sum(); + } + + private long countActionsForSoftInvalidation(final Collection setIds) { + return setIds.stream() + .mapToLong(distributionSet -> actionRepository + .countByDistributionSetIdAndActiveIsTrueAndStatusIsNot(distributionSet, Status.CANCELING)) + .sum(); + } + + @Data private static class InvalidationTestData { private final DistributionSet distributionSet; @@ -285,7 +325,8 @@ class DistributionSetInvalidationManagementTest extends AbstractJpaIntegrationTe private final TargetFilterQuery targetFilterQuery; private final Rollout rollout; - public InvalidationTestData(final DistributionSet distributionSet, final List targets, + public InvalidationTestData( + final DistributionSet distributionSet, final List targets, final TargetFilterQuery targetFilterQuery, final Rollout rollout) { super(); this.distributionSet = distributionSet; @@ -293,21 +334,5 @@ class DistributionSetInvalidationManagementTest extends AbstractJpaIntegrationTe this.targetFilterQuery = targetFilterQuery; this.rollout = rollout; } - - public DistributionSet getDistributionSet() { - return distributionSet; - } - - public List getTargets() { - return targets; - } - - public TargetFilterQuery getTargetFilterQuery() { - return targetFilterQuery; - } - - public Rollout getRollout() { - return rollout; - } } } diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/ManagementSecurityTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/ManagementSecurityTest.java new file mode 100644 index 000000000..622b1e411 --- /dev/null +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/ManagementSecurityTest.java @@ -0,0 +1,458 @@ +/** + * Copyright (c) 2025 Contributors to the Eclipse Foundation + * + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + */ +package org.eclipse.hawkbit.repository.jpa.management; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.fail; + +import java.io.ByteArrayInputStream; +import java.io.InputStream; +import java.io.Serializable; +import java.lang.reflect.Array; +import java.lang.reflect.Constructor; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.net.URI; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.Callable; +import java.util.function.Consumer; +import java.util.stream.Stream; + +import io.github.classgraph.ClassGraph; +import io.github.classgraph.ClassInfo; +import io.github.classgraph.ScanResult; +import lombok.SneakyThrows; +import lombok.extern.slf4j.Slf4j; +import org.eclipse.hawkbit.repository.PermissionSupport; +import org.eclipse.hawkbit.repository.TenantStatsManagement; +import org.eclipse.hawkbit.repository.exception.InsufficientPermissionException; +import org.eclipse.hawkbit.repository.jpa.AbstractJpaIntegrationTest; +import org.eclipse.hawkbit.repository.test.util.AbstractIntegrationTest; +import org.eclipse.hawkbit.repository.test.util.SecurityContextSwitch; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.data.domain.Pageable; +import org.springframework.expression.spel.SpelNode; +import org.springframework.expression.spel.ast.MethodReference; +import org.springframework.expression.spel.ast.OpAnd; +import org.springframework.expression.spel.ast.OpOr; +import org.springframework.expression.spel.ast.StringLiteral; +import org.springframework.expression.spel.ast.VariableReference; +import org.springframework.expression.spel.standard.SpelExpression; +import org.springframework.expression.spel.standard.SpelExpressionParser; +import org.springframework.security.access.prepost.PreAuthorize; +import org.springframework.security.authorization.AuthorizationDeniedException; +import org.springframework.test.context.TestPropertySource; +import org.springframework.util.ClassUtils; +import org.springframework.util.ObjectUtils; + +@Slf4j +@TestPropertySource(properties = { "logging.level.org.eclipse.hawkbit.repository.test.util=off" }) +class ManagementSecurityTest extends AbstractJpaIntegrationTest { + + private static final SpelExpressionParser SPEL_EXPRESSION_PARSER = new SpelExpressionParser(); + + @Autowired + protected TenantStatsManagement tenantStatsManagement; + + @Override + @BeforeEach + public void beforeAll() throws Exception { + // override - shall not do anything + } + + @ParameterizedTest + @MethodSource("testMethods") + void testMethod(final Class managementInterface, final Method managementInterfaceMethod) { + final Object managementObject = TenantStatsManagement.class == managementInterface + ? tenantStatsManagement // it's not a field of AbstractIntegrationTest, so we need to use the autowired instance + : Stream + .of(AbstractIntegrationTest.class.getDeclaredFields()) + .filter(field -> managementInterface.isAssignableFrom(field.getType())) + .findFirst() + .map(field -> { + field.setAccessible(true); + try { + return field.get(this); + } catch (final IllegalAccessException e) { + throw new AssertionError("Could not access field " + field.getName(), e); + } + }) + .orElseThrow(() -> new AssertionError("No management implementation found for " + managementInterface)); + final Class managedClass = ClassUtils.getUserClass(managementObject); // managed class is a proxy + final Method implementationMethod = findImplementationMethod(managedClass, managementInterfaceMethod); + if (implementationMethod == null) { + throw new AssertionError("No management implementation found for " + managementInterfaceMethod + " in " + managedClass.getName()); + } + final String permissionGroup = managementObject instanceof PermissionSupport permissionSupport + ? permissionSupport.permissionGroup() + : null; + Set preAuthorizedPermissions = collectPreAuthorizedPermissions(implementationMethod, permissionGroup); + if (ObjectUtils.isEmpty(preAuthorizedPermissions)) { + preAuthorizedPermissions = collectPreAuthorizedPermissions(managementInterfaceMethod, permissionGroup); + } + if (ObjectUtils.isEmpty(preAuthorizedPermissions)) { + fail("No PreAuthorize annotation found for " + managementInterface.getSimpleName() + " -> " + implementationMethod); + } else { + assertPermissionsCheck(managementInterfaceMethod, managementObject, preAuthorizedPermissions.toArray(new String[0])); + } + } + + private static Stream testMethods() { + final String packageName = "org.eclipse.hawkbit.repository"; + try (final ScanResult scanResult = new ClassGraph().acceptPackages(packageName).scan()) { + return scanResult.getAllClasses() + .stream() + // scan scans subpackages as well, so we need to filter out the classes that are not in the package (e.g. JpaActionManagement) + .filter(classInPackage -> classInPackage.getPackageName().equals(packageName)) + .filter(classInPackage -> classInPackage.getSimpleName().endsWith("Management")) + // RepositoryManagement is not a management interface but a super of such interfaces + .filter(classInPackage -> !classInPackage.getSimpleName().equals("RepositoryManagement")) + // QuotaManagement and its implementation PropertiesQuotaManagement is not protected using @PreAuthorize + // it is not an exposed db service but internally used + .filter(classInPackage -> !classInPackage.getSimpleName().equals("QuotaManagement") && + !classInPackage.getSimpleName().equals("PropertiesQuotaManagement")) + .map(ClassInfo::loadClass) + .flatMap(clazz -> collectMethods(clazz, new ArrayList<>()).stream() + // permissionGroup is an internal method and should not be protected by @PreAuthorize + .filter(method -> !"permissionGroup".equals(method.getName()) || + method.getParameterCount() != 0 || + method.getReturnType() != String.class) + // jacoco adds some methods with bytecode instrumentation + .filter(method -> !"$jacocoInit".equals(method.getName())) + .map(method -> Arguments.of(clazz, method))) + // consumes the stream because scan result couldn't be used after being closed + .toList() + .stream(); + } + } + + private static List collectMethods(final Class clazz, final List methods) { + methods.addAll(Arrays.asList(clazz.getDeclaredMethods())); + for (final Class interfaceClass : clazz.getInterfaces()) { + collectMethods(interfaceClass, methods); + } + if (clazz.getSuperclass() != null) { + collectMethods(clazz.getSuperclass(), methods); + } + return methods; + } + + private static Method findImplementationMethod(final Class managementClass, final Method managementInterfaceMethod) { + final Method classMethod = findClassImplementationMethod(managementClass, managementInterfaceMethod); + if (classMethod == null) { + return findInterfaceDefaultMethod(managementClass, managementInterfaceMethod); + } else { + return classMethod; + } + } + + private static Method findClassImplementationMethod(final Class managementClass, final Method managementInterfaceMethod) { + return Stream.of(managementClass.getDeclaredMethods()) + .filter(m -> match(m, managementInterfaceMethod)) + .findFirst() + .orElseGet(() -> { + final Class superClass = managementClass.getSuperclass(); + if (superClass == null) { + return null; + } else { + return findImplementationMethod(superClass, managementInterfaceMethod); + } + }); + } + + private static Method findInterfaceDefaultMethod(final Class managementClassOrInterface, final Method managementInterfaceMethod) { + if (!managementInterfaceMethod.getDeclaringClass().isAssignableFrom(managementClassOrInterface)) { + return null; + } + Method interfaceMethod = null; + for (final Class superInterface : managementClassOrInterface.getInterfaces()) { + final Method method = Stream.of(superInterface.getDeclaredMethods()) + .filter(Method::isDefault) + .filter(m -> match(m, managementInterfaceMethod)) + .findFirst() + .orElseGet(() -> findInterfaceDefaultMethod(superInterface, managementInterfaceMethod)); + if (method != null) { // found + if (interfaceMethod != null) { + // should not happen, but check anyway + throw new IllegalStateException( + "Multiple default methods found for " + managementInterfaceMethod + " in interfaces: " + interfaceMethod + " and " + method); + } + interfaceMethod = method; + } + } + return interfaceMethod; + } + + private static boolean match(final Method method, final Method managementInterfaceMethod) { + return method.getName().equals(managementInterfaceMethod.getName()) && + // TODO - check for generics + Arrays.equals(method.getParameterTypes(), managementInterfaceMethod.getParameterTypes()); + } + + private Set collectPreAuthorizedPermissions(final Method method, final String permissionGroup) { + if (method.isAnnotationPresent(PreAuthorize.class)) { + final PreAuthorize preAuthorize = method.getAnnotation(PreAuthorize.class); + final SpelExpression expr = (SpelExpression) SPEL_EXPRESSION_PARSER.parseExpression(preAuthorize.value()); + final Set expressionPermissions = new HashSet<>(); + addSufficientPermissions(expr.getAST(), expressionPermissions, permissionGroup); + if (expressionPermissions.isEmpty()) { + throw new IllegalStateException("No permissions found in expression: " + preAuthorize.value()); + } + return expressionPermissions; + } else { + return null; + } + } + + private void addSufficientPermissions(final SpelNode spelNode, final Set preAuthorizedPermissions, final String permissionGroup) { + if (spelNode instanceof OpOr) { + addSufficientPermissions(spelNode.getChild(0), preAuthorizedPermissions, permissionGroup); + } else if (spelNode instanceof OpAnd) { + for (int i = 0; i < spelNode.getChildCount(); i++) { + addSufficientPermissions(spelNode.getChild(i), preAuthorizedPermissions, permissionGroup); + } + } else if (spelNode instanceof MethodReference methodReference) { + final String method = methodReference.getName(); + switch (method) { + case "hasAuthority" -> { + for (int i = 0; i < spelNode.getChildCount(); i++) { + addSufficientPermissions(spelNode.getChild(i), preAuthorizedPermissions, permissionGroup); + } + } + case "hasAnyRole" -> { + final SpelNode child = spelNode.getChild(0); + if (child instanceof StringLiteral literal) { + final String permission = (String) literal.getLiteralValue().getValue(); + preAuthorizedPermissions.add(permission.toUpperCase().startsWith("ROLE_") ? permission : "ROLE_" + permission); + } else { + addSufficientPermissions(child, preAuthorizedPermissions, permissionGroup); + } + } + case "hasPermission" -> { + assertThat(spelNode.getChildCount()).isEqualTo(2); + assertThat(spelNode.getChild(0) instanceof VariableReference varRef && varRef.toStringAST().equals("#root")).isTrue(); + assertThat(spelNode.getChild(1)).isInstanceOf(StringLiteral.class); + final StringLiteral literal = (StringLiteral) spelNode.getChild(1); + preAuthorizedPermissions.add(literal.getLiteralValue().getValue() + "_" + permissionGroup); + } + default -> throw new IllegalStateException("Unexpected MethodReference: " + method); + } + } else if (spelNode instanceof StringLiteral literal) { + preAuthorizedPermissions.add((String) literal.getLiteralValue().getValue()); + } else { + throw new IllegalStateException("Unexpected SpelNode: " + spelNode + " of type " + spelNode.getClass()); + } + } + + @SneakyThrows + @SuppressWarnings("rawtypes") + private Object instance(final Class clazz) { + if (clazz.isArray()) { + return Array.newInstance(clazz.getComponentType(), 0); + } + + if (Collection.class.isAssignableFrom(clazz)) { + if (clazz == List.class || clazz == Collection.class) { + return new ArrayList<>(); + } else if (clazz == Set.class) { + return new HashSet<>(); + } else { + throw new IllegalStateException("No instance for collection interface " + clazz); + } + } + + if (clazz == Map.class) { + return new HashMap<>(); + } + + if (clazz.isInterface()) { + if (clazz == Pageable.class) { + return Pageable.ofSize(10); + } else if (clazz == Serializable.class) { + return ""; + } else if (clazz == Consumer.class) { + return (Consumer) s -> {}; + } else if (clazz.getPackageName().startsWith("org.eclipse.hawkbit.repository")) { + if (clazz.getSimpleName().endsWith("Management")) { + return Stream + .of(AbstractIntegrationTest.class.getDeclaredFields()) + .filter(field -> clazz.isAssignableFrom(field.getType())) + .findFirst() + .map(field -> { + field.setAccessible(true); + try { + return field.get(this); + } catch (final IllegalAccessException e) { + throw new AssertionError("Could not access field " + field.getName(), e); + } + }) + .orElseThrow(() -> new IllegalStateException("No management implementation found for " + clazz)); + } + try (final ScanResult scanResult = new ClassGraph().acceptPackages("org.eclipse.hawkbit.repository").scan()) { + return scanResult.getClassesImplementing(clazz) + .stream() + .filter(impl -> !impl.isAbstract()) + .findFirst() + .map(impl -> instance(impl.loadClass())) + .orElseThrow(() -> new IllegalStateException("No instance for interface " + clazz)); + } + } else { + throw new IllegalStateException("No instance for interface " + clazz); + } + } + + if (clazz.isEnum()) { + return clazz.getEnumConstants()[0]; + } + + if (clazz == boolean.class || clazz == Boolean.class) { + return false; + } else if (clazz == int.class || clazz == Integer.class) { + return 1; + } else if (clazz == long.class || clazz == Long.class) { + return 1L; + } else if (clazz == float.class || clazz == Float.class) { + return 1.0f; + } else if (clazz == double.class || clazz == Double.class) { + return 1.0; + } else if (clazz == short.class || clazz == Short.class) { + return (short) 1; + } else if (clazz == byte.class || clazz == Byte.class) { + return (byte) 1; + } else if (clazz == char.class || clazz == Character.class) { + return 'a'; + } else if (clazz == String.class) { + return "id==0"; + } else if (clazz == InputStream.class) { + return new ByteArrayInputStream(new byte[1]); + } else if (clazz == URI.class) { + return new URI("http://localhost"); + } else if (clazz == Class.class) { + return String.class; + } else { + try { + final Constructor[] constructors = clazz.getDeclaredConstructors(); + if (ObjectUtils.isEmpty(constructors)) { + throw new IllegalStateException("No public constructor found for " + clazz); + } + // prefer empty constructor + for (final Constructor constructor : constructors) { + if (constructor.getParameterCount() == 0) { + constructor.setAccessible(true); + return constructor.newInstance(); + } + } + constructors[0].setAccessible(true); + return constructors[0].newInstance(Stream.of(constructors[0].getParameterTypes()) + .map(this::instance) + .toArray()); + } catch (final InstantiationException e) { + // try builder pattern + try { + final Object builder = clazz.getDeclaredMethod("builder").invoke(null); + final Method build = builder.getClass().getDeclaredMethod("build"); + build.setAccessible(true); + return build.invoke(builder); + } catch (final NoSuchMethodException | IllegalAccessException | InvocationTargetException e1) { + log.debug("{} is not a builder. Throws could not instantiate", clazz.getName()); + } + log.error("Could not instantiate {}", clazz.getName(), e); + throw e; + } + } + } + + private static final Set EXPECTED_EXCEPTIONS_TYPES = new HashSet<>(); + + @AfterAll + static void afterAll() { + final List exceptions = new ArrayList<>(EXPECTED_EXCEPTIONS_TYPES); + Collections.sort(exceptions); + log.info("Expected exceptions occurred during tests:\n\t{}", String.join("\n\t", exceptions)); + } + + @SneakyThrows + protected void assertPermissionsCheck(final Method managementInterfaceMethod, final Object managementObject, final String... permissions) { + final Callable callable = () -> { + try { + final Object[] params = new Object[managementInterfaceMethod.getParameterCount()]; + for (int i = 0; i < params.length; i++) { + params[i] = instance(managementInterfaceMethod.getParameterTypes()[i]); + } + return managementInterfaceMethod.invoke(managementObject, params); + } catch (final InvocationTargetException e) { + if (e.getCause() instanceof RuntimeException re) { + throw re; + } else { + throw new AssertionError(e.getCause()); + } + } + }; + + // check if the user has the correct permissions + SecurityContextSwitch.runAs(SecurityContextSwitch.withUser("user_with_permissions", permissions), () -> { + try { + callable.call(); + } catch (final Throwable th) { + if (th instanceof InsufficientPermissionException || th instanceof AuthorizationDeniedException) { + throw new AssertionError( + "Expected no InsufficientPermissionException or AuthorizationDeniedException to be thrown, but got: " + th + + " (permissions: " + Arrays.toString(permissions) + ")", th); + } else { + Stream.of(th.getStackTrace()) + .filter(stackTraceElement -> { + // if the method seem to exist in the stack trace + try { + final Class clazz = Class.forName(stackTraceElement.getClassName()); + return clazz.isAssignableFrom(managementObject.getClass()) && // in class or implementation in hierarchy + stackTraceElement.getMethodName().equals(managementInterfaceMethod.getName()); // + } catch (final ClassNotFoundException e) { + return false; + } + }) + .findAny() + .orElseThrow(() -> new AssertionError( + "Unexpected Exception is thrown (permissions: " + Arrays.toString(permissions) + ")", th)); + EXPECTED_EXCEPTIONS_TYPES.add(th.getClass().getName()); + log.debug("Expected catch: {}", th.getMessage()); + } + } + }); + + // check if the user has not the correct permissions + final String[] permissionsWithoutOne = new String[permissions.length - 1]; + System.arraycopy(permissions, 0, permissionsWithoutOne, 0, permissionsWithoutOne.length); + SecurityContextSwitch.runAs(SecurityContextSwitch.withUser("user_without_permissions", permissionsWithoutOne), () -> { + try { + callable.call(); + throw new AssertionError( + "Expected Exception InsufficientPermissionException to be thrown, but request passed with no exception" + + " (permissions: " + Arrays.toString(permissionsWithoutOne) + ", needed: " + Arrays.asList(permissions) + ")"); + } catch (final Exception ex) { + // default interface methods as TargetManagement.getWithAutoConfigurationStatus are not handled to + // throw InsufficientPermissionException + assertThat(ex).isInstanceOfAny(InsufficientPermissionException.class, AuthorizationDeniedException.class); + } + }); + } +} \ No newline at end of file