From 00b129ad95416192952637e1af37327034ae7779 Mon Sep 17 00:00:00 2001 From: Avgustin Marinov Date: Tue, 15 Apr 2025 13:44:15 +0300 Subject: [PATCH] [#2343] Deny scheduled rollout creation when no HANDLE_ROLLOUT permission (#2357) Signed-off-by: Avgustin Marinov --- .../jpa/management/JpaRolloutManagement.java | 18 +++++- .../jpa/management/JpaSystemManagement.java | 2 +- .../jpa/management/RolloutManagementTest.java | 61 +++++++++++++++++++ 3 files changed, 77 insertions(+), 4 deletions(-) diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaRolloutManagement.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaRolloutManagement.java index 039f348d4..8a6fffe70 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaRolloutManagement.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaRolloutManagement.java @@ -29,6 +29,7 @@ import jakarta.validation.constraints.NotNull; import lombok.extern.slf4j.Slf4j; import org.eclipse.hawkbit.ContextAware; +import org.eclipse.hawkbit.im.authentication.SpPermission; import org.eclipse.hawkbit.repository.DistributionSetManagement; import org.eclipse.hawkbit.repository.QuotaManagement; import org.eclipse.hawkbit.repository.RepositoryProperties; @@ -47,6 +48,7 @@ import org.eclipse.hawkbit.repository.builder.RolloutUpdate; import org.eclipse.hawkbit.repository.event.remote.entity.RolloutGroupCreatedEvent; import org.eclipse.hawkbit.repository.exception.EntityNotFoundException; import org.eclipse.hawkbit.repository.exception.EntityReadOnlyException; +import org.eclipse.hawkbit.repository.exception.InsufficientPermissionException; import org.eclipse.hawkbit.repository.exception.RolloutIllegalStateException; import org.eclipse.hawkbit.repository.jpa.JpaManagementHelper; import org.eclipse.hawkbit.repository.jpa.builder.JpaRolloutGroupCreate; @@ -195,6 +197,7 @@ public class JpaRolloutManagement implements RolloutManagement { final RolloutGroupConditions conditions, final DynamicRolloutGroupTemplate dynamicRolloutGroupTemplate) { return create0(rollout, amountGroup, confirmationRequired, conditions, dynamicRolloutGroupTemplate); } + private Rollout create0( final RolloutCreate rollout, final int amountGroup, final boolean confirmationRequired, final RolloutGroupConditions conditions, final DynamicRolloutGroupTemplate dynamicRolloutGroupTemplate) { @@ -210,6 +213,12 @@ public class JpaRolloutManagement implements RolloutManagement { } final JpaRollout rolloutRequest = (JpaRollout) rollout.build(); + // scheduled rollout, the creator shall have permissions to start rollout + if (rolloutRequest.getStartAt() != null && rolloutRequest.getStartAt() != Long.MAX_VALUE && // if scheduled rollout + !systemSecurityContext.hasPermission(SpPermission.HANDLE_ROLLOUT) && + !systemSecurityContext.hasPermission(SpPermission.SpringEvalExpressions.SYSTEM_ROLE)) { + throw new InsufficientPermissionException("You need permission to start rollouts to create a scheduled rollout"); + } if (dynamicRolloutGroupTemplate != null && !rolloutRequest.isDynamic()) { throw new ValidationException("Dynamic group template is only allowed for dynamic rollouts"); } @@ -266,7 +275,8 @@ public class JpaRolloutManagement implements RolloutManagement { @Override public Slice findAllWithDetailedStatus(final Pageable pageable, final boolean deleted) { final Slice rollouts = JpaManagementHelper.convertPage( - rolloutRepository.findAll(RolloutSpecification.isDeleted(deleted, pageable.getSort()), JpaRollout_.GRAPH_ROLLOUT_DS, pageable), pageable); + rolloutRepository.findAll(RolloutSpecification.isDeleted(deleted, pageable.getSort()), JpaRollout_.GRAPH_ROLLOUT_DS, pageable), + pageable); setRolloutStatusDetails(rollouts); return rollouts; } @@ -375,6 +385,7 @@ public class JpaRolloutManagement implements RolloutManagement { public Rollout approveOrDeny(final long rolloutId, final Rollout.ApprovalDecision decision, final String remark) { return approveOrDeny0(rolloutId, decision, remark); } + private Rollout approveOrDeny0(final long rolloutId, final Rollout.ApprovalDecision decision, final String remark) { log.debug("approveOrDeny rollout called for rollout {} with decision {}", rolloutId, decision); final JpaRollout rollout = getRolloutOrThrowExceptionIfNotFound(rolloutId); @@ -720,7 +731,7 @@ public class JpaRolloutManagement implements RolloutManagement { .orElseThrow(() -> new EntityNotFoundException(Rollout.class, rolloutId)); } - private @NotNull Map> getStatusCountItemForRollout(final List rollouts) { + private @NotNull Map> getStatusCountItemForRollout(final List rollouts) { if (rollouts.isEmpty()) { return Collections.emptyMap(); } @@ -819,7 +830,8 @@ public class JpaRolloutManagement implements RolloutManagement { } } - private long calculateRemainingTargets(final List groups, final String targetFilter, final Long createdAt, final Long dsTypeId) { + private long calculateRemainingTargets(final List groups, final String targetFilter, final Long createdAt, + final Long dsTypeId) { final TargetCount targets = calculateTargets(targetFilter, createdAt, dsTypeId); final long totalTargets = targets.total(); diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaSystemManagement.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaSystemManagement.java index d9efc9b84..886772a78 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaSystemManagement.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaSystemManagement.java @@ -312,7 +312,7 @@ public class JpaSystemManagement implements CurrentTenantCacheKeyGenerator, Syst new JpaSoftwareModuleType( org.eclipse.hawkbit.repository.Constants.SMT_DEFAULT_OS_KEY, org.eclipse.hawkbit.repository.Constants.SMT_DEFAULT_OS_NAME, - "Core firmware or operation system", 1)); + "Core firmware or operating system", 1)); // make sure the module types get their IDs entityManager.flush(); diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/RolloutManagementTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/RolloutManagementTest.java index 8751efc22..d39e7c5c9 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/RolloutManagementTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/RolloutManagementTest.java @@ -35,6 +35,7 @@ import io.qameta.allure.Story; import org.assertj.core.api.Assertions; import org.assertj.core.api.Condition; import org.awaitility.Awaitility; +import org.eclipse.hawkbit.im.authentication.SpPermission; import org.eclipse.hawkbit.repository.Identifiable; import org.eclipse.hawkbit.repository.OffsetBasedPageRequest; import org.eclipse.hawkbit.repository.builder.RolloutCreate; @@ -59,6 +60,7 @@ import org.eclipse.hawkbit.repository.exception.AssignmentQuotaExceededException import org.eclipse.hawkbit.repository.exception.EntityAlreadyExistsException; import org.eclipse.hawkbit.repository.exception.EntityReadOnlyException; import org.eclipse.hawkbit.repository.exception.IncompleteDistributionSetException; +import org.eclipse.hawkbit.repository.exception.InsufficientPermissionException; import org.eclipse.hawkbit.repository.exception.InvalidDistributionSetException; import org.eclipse.hawkbit.repository.exception.RolloutIllegalStateException; import org.eclipse.hawkbit.repository.jpa.AbstractJpaIntegrationTest; @@ -85,6 +87,7 @@ import org.eclipse.hawkbit.repository.model.TotalTargetCountStatus; import org.eclipse.hawkbit.repository.test.matcher.Expect; import org.eclipse.hawkbit.repository.test.matcher.ExpectEvents; import org.eclipse.hawkbit.repository.test.util.SecurityContextSwitch; +import org.eclipse.hawkbit.repository.test.util.WithUser; import org.eclipse.hawkbit.tenancy.configuration.TenantConfigurationProperties.TenantConfigurationKey; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -1378,6 +1381,64 @@ class RolloutManagementTest extends AbstractJpaIntegrationTest { validateRolloutActionStatus(myRolloutId, expectedTargetCountStatus); } + @Test + @Description("Verify the creation and the start of a rollout.") + void createScheduledRollout() throws Exception { + final String rolloutName = "scheduledRolloutTest"; + testdataFactory.createTargets(50, rolloutName + "-", rolloutName); + final DistributionSet distributionSet = testdataFactory.createDistributionSet("dsFor" + rolloutName); + distributionSetManagement.lock(distributionSet.getId()); + + final WithUser userWithoutHandleRollout = SecurityContextSwitch.withUser( + "user_without_handle_rollout", + SpPermission.READ_REPOSITORY, SpPermission.READ_TARGET, SpPermission.CREATE_ROLLOUT); + final WithUser userWithHandleRollout = SecurityContextSwitch.withUser( + "user_with_handle_rollout", + SpPermission.READ_REPOSITORY, SpPermission.READ_TARGET, SpPermission.CREATE_ROLLOUT, SpPermission.HANDLE_ROLLOUT); + final WithUser userWithSystemRole = SecurityContextSwitch.withUser( + "user_with_system_role", + SpPermission.SpringEvalExpressions.SYSTEM_ROLE); + + final String filter = "controllerId==" + rolloutName + "-*"; + // create scheduled rollout fails without handle rollout permission + assertThatExceptionOfType(InsufficientPermissionException.class) + .as("Insufficient permission exception when startAt and no handle rollout permission") + .isThrownBy(() -> SecurityContextSwitch.runAs( + userWithoutHandleRollout, () -> createRolloutWithStartAt(rolloutName, filter, distributionSet, 1L))); + // same action succeeds with handle rollout permission + SecurityContextSwitch.runAs( + userWithHandleRollout, + () -> createRolloutWithStartAt(rolloutName + "_withStartTime", filter, distributionSet, 1L)); + // same action succeeds with system role permission + SecurityContextSwitch.runAs( + userWithSystemRole, + () -> createRolloutWithStartAt(rolloutName + "_withStartTimeSystemRole", filter, distributionSet, 1L)); + // same action succeeds without handle rollout permission but with null start at + SecurityContextSwitch.runAs( + userWithoutHandleRollout, + () -> createRolloutWithStartAt(rolloutName + "_withoutStartTime", filter, distributionSet, null)); + // same action succeeds without handle rollout permission but with Long.MAX_VALUE start at + SecurityContextSwitch.runAs( + userWithoutHandleRollout, + () -> createRolloutWithStartAt(rolloutName + "_withLongMax", filter, distributionSet, Long.MAX_VALUE)); + } + private Rollout createRolloutWithStartAt(final String rolloutName, final String filter, final DistributionSet distributionSet, + final Long startAt) { + return rolloutManagement.create( + entityFactory.rollout().create() + .name(rolloutName) + .description("desc") + .targetFilterQuery(filter) + .distributionSetId(distributionSet) + .weight(1000) + .startAt(startAt) + .dynamic(false), + 5, false, new RolloutGroupConditionBuilder().withDefaults() + .successCondition(RolloutGroupSuccessCondition.THRESHOLD, "80") + .errorCondition(RolloutGroupErrorCondition.THRESHOLD, "50") + .errorAction(RolloutGroupErrorAction.PAUSE, null).build(), null); + } + @Test @Description("Verify that a rollout cannot be created if the 'max targets per rollout group' quota is violated.") void createRolloutFailsIfQuotaGroupQuotaIsViolated() {