Fix system context resolving in ACM (#2737)

Signed-off-by: Avgustin Marinov <Avgustin.Marinov@bosch.com>
This commit is contained in:
Avgustin Marinov
2025-10-10 12:02:16 +03:00
committed by GitHub
parent e7d9ee7990
commit 3447ac3b1b
17 changed files with 97 additions and 129 deletions

View File

@@ -9,8 +9,6 @@
*/
package org.eclipse.hawkbit.repository.jpa.acm;
import static org.eclipse.hawkbit.security.SecurityContextTenantAware.SYSTEM_USER;
import java.util.ArrayList;
import java.util.EnumMap;
import java.util.List;
@@ -24,6 +22,7 @@ import org.eclipse.hawkbit.ContextAware;
import org.eclipse.hawkbit.repository.QueryField;
import org.eclipse.hawkbit.repository.exception.InsufficientPermissionException;
import org.eclipse.hawkbit.repository.jpa.ql.QLSupport;
import org.eclipse.hawkbit.security.SystemSecurityContext;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.data.jpa.domain.Specification;
import org.springframework.security.core.GrantedAuthority;
@@ -58,8 +57,8 @@ public class DefaultAccessController<A extends Enum<A> & QueryField, T> implemen
@Override
public Optional<Specification<T>> getAccessRules(final Operation operation) {
if (contextAware.getCurrentTenant() != null && SYSTEM_USER.equals(contextAware.getCurrentUsername())) {
// as tenant, no restrictions
if (SystemSecurityContext.isCurrentThreadSystemCode()) {
// system code - no restrictions. this runs with SYSTEM_ROLE, so no restrictions apply anyway - not scopes, but this way should be faster
return Optional.empty();
}
@@ -73,8 +72,8 @@ public class DefaultAccessController<A extends Enum<A> & QueryField, T> implemen
@Override
public void assertOperationAllowed(final Operation operation, final T entity) throws InsufficientPermissionException {
if (contextAware.getCurrentTenant() != null && SYSTEM_USER.equals(contextAware.getCurrentUsername())) {
// as tenant, no restrictions
if (SystemSecurityContext.isCurrentThreadSystemCode()) {
// system code - no restrictions. this runs with SYSTEM_ROLE, so no restrictions apply anyway - not scopes, but this way should be faster
return;
}

View File

@@ -134,7 +134,6 @@ public class JpaDistributionSetInvalidationManagement implements DistributionSet
systemSecurityContext.runAsSystem(() -> {
log.debug("Cancel auto assignments after ds invalidation. ID: {}", setId);
targetFilterQueryManagement.cancelAutoAssignmentForDistributionSet(setId);
return null;
});
}
}

View File

@@ -73,7 +73,6 @@ public class RolloutScheduler {
handleAllAsync(tenant);
}
});
return null;
});
meterRegistry

View File

@@ -61,7 +61,6 @@ public class PauseRolloutGroupAction implements RolloutGroupActionEvaluator<Roll
// if only the latest state is != paused then pause
rolloutManagement.pauseRollout(rollout.getId());
}
return null;
});
}
}

View File

@@ -27,7 +27,6 @@ import org.eclipse.hawkbit.repository.jpa.autoassign.AutoAssignScheduler;
import org.eclipse.hawkbit.repository.model.TargetFilterQuery;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.data.domain.Pageable;
import org.springframework.integration.support.locks.LockRegistry;
class AutoAssignTest extends AbstractAccessControllerManagementTest {
@@ -77,8 +76,8 @@ class AutoAssignTest extends AbstractAccessControllerManagementTest {
// do the assignment
assigner.run();
assertThat(targetManagement.findByAssignedDistributionSet(targetFilterQuery.getAutoAssignDistributionSet().getId(), Pageable.unpaged())
.map(Identifiable::getId).toList())
assertThat(targetManagement.findByAssignedDistributionSet(targetFilterQuery.getAutoAssignDistributionSet().getId(), UNPAGED)
.map(Identifiable::getId).toList())
.as("Only updatable targets should be part of the rollout")
// all targets are distribution set type 2 compatible, but since user has UPDATE_TARGET only for targets of type 2
// only target2 and target3 shall be assigned

View File

@@ -36,10 +36,7 @@ class RolloutExecutionTest extends AbstractAccessControllerManagementTest {
@Test
void verifyOnlyUpdatableTargetsArePartOfRollout() {
verify(() -> systemSecurityContext.runAsSystem(() -> {
rolloutHandler.handleAll();
return null;
}));
verify(() -> systemSecurityContext.runAsSystem(rolloutHandler::handleAll));
}
private void verify(final Runnable run) {

View File

@@ -106,10 +106,7 @@ class SystemExecutionTest extends AbstractAccessControllerManagementTest {
final Specification mockAsSystem = mock(Specification.class);
for (Operation operation : Operation.values()) {
systemSecurityContext.runAsSystem(() -> {
accessController.appendAccessRules(operation, mockAsSystem);
return null;
});
systemSecurityContext.runAsSystem(() -> accessController.appendAccessRules(operation, mockAsSystem));
}
verifyNoInteractions(mockAsSystem);
}

View File

@@ -21,7 +21,6 @@ import static org.eclipse.hawkbit.im.authentication.SpPermission.READ_ROLLOUT;
import static org.eclipse.hawkbit.im.authentication.SpPermission.READ_TARGET;
import static org.eclipse.hawkbit.im.authentication.SpPermission.UPDATE_TARGET;
import static org.eclipse.hawkbit.repository.test.util.SecurityContextSwitch.runAs;
import static org.eclipse.hawkbit.repository.test.util.SecurityContextSwitch.withUser;
import java.util.Arrays;
import java.util.List;
@@ -37,12 +36,9 @@ import org.eclipse.hawkbit.repository.model.Action;
import org.eclipse.hawkbit.repository.model.DistributionSet;
import org.eclipse.hawkbit.repository.model.NamedEntity;
import org.eclipse.hawkbit.repository.model.Rollout;
import org.eclipse.hawkbit.repository.model.RolloutGroup;
import org.eclipse.hawkbit.repository.model.Target;
import org.eclipse.hawkbit.repository.model.TargetTag;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.springframework.data.domain.Pageable;
class TargetManagementTest extends AbstractAccessControllerManagementTest {
@@ -130,57 +126,69 @@ class TargetManagementTest extends AbstractAccessControllerManagementTest {
});
}
@Disabled
@Test
void verifyReadRolloutRelated() {
assertThat(assignDistributionSet(ds2Type2, List.of(target1Type1, target2Type2, target3Type2)).getAssigned()).isEqualTo(3);
void verifyReadCompatibleRelated() {
// assertThat(assignDistributionSet(ds2Type2, List.of(target1Type1, target2Type2, target3Type2)).getAssigned()).isEqualTo(3);
prepareFinishedUpdates(ds2Type2, target1Type1, target2Type2);
final Target target1Type1 = targetManagement.get(super.target1Type1.getId());
runAs(withAuthorities(
READ_TARGET + "/type.id==" + targetType1.getId(),
READ_DISTRIBUTION_SET,
CREATE_ROLLOUT, READ_ROLLOUT, HANDLE_ROLLOUT), () -> {
READ_TARGET + "/type.id==" + targetType2.getId(), // we want to have 2 updatable targets
READ_DISTRIBUTION_SET), () -> {
assertThat(targetManagement.findByInstalledDistributionSet(ds2Type2.getId(), UNPAGED))
.extracting(Identifiable::getId).containsExactly(target1Type1.getId());
.extracting(Identifiable::getId).containsExactly(target2Type2.getId());
assertThat(targetManagement.findByInstalledDistributionSetAndRsql(ds2Type2.getId(), "id==*", UNPAGED))
.extracting(Identifiable::getId).containsExactly(target1Type1.getId());
assertThat(targetManagement.findByTargetFilterQueryAndNonDSAndCompatibleAndUpdatable(
ds2Type2.getId(), "id==*", UNPAGED))
.hasSize(1)
.extracting(Identifiable::getId).containsOnly(target1Type1.getId());
.extracting(Identifiable::getId).containsExactly(target2Type2.getId());
});
runAs(withAuthorities(
READ_TARGET, UPDATE_TARGET + "/type.id==" + targetType2.getId(), // we want to have 2 updatable targets
READ_DISTRIBUTION_SET), () -> {
assertThat(targetManagement.findByTargetFilterQueryAndNonDSAndCompatibleAndUpdatable(ds2Type2.getId(), "id==*", UNPAGED))
.extracting(Identifiable::getId).containsExactly(target3Type2.getId());
assertThat(targetManagement.isTargetMatchingQueryAndDSNotAssignedAndCompatibleAndUpdatable(
target1Type1.getControllerId(), ds2Type2.getId(), "id==*")).isTrue();
target1Type1.getControllerId(), ds2Type2.getId(), "id==*")).isFalse();
assertThat(targetManagement.isTargetMatchingQueryAndDSNotAssignedAndCompatibleAndUpdatable(
target2Type2.getControllerId(), ds2Type2.getId(), "id==*")).isFalse();
assertThat(targetManagement.isTargetMatchingQueryAndDSNotAssignedAndCompatibleAndUpdatable(
target3Type2.getControllerId(), ds2Type2.getId(), "id==*")).isTrue();
});
}
@Test
void verifyReadRolloutRelated() {
final Target target1Type1 = targetManagement.get(super.target1Type1.getId());
runAs(withAuthorities(
READ_TARGET, UPDATE_TARGET + "/type.id==" + targetType1.getId(),
READ_DISTRIBUTION_SET,
CREATE_ROLLOUT, READ_ROLLOUT, HANDLE_ROLLOUT), () -> {
assertThat(targetManagement.findByRsqlAndNotInRolloutGroupsAndCompatibleAndUpdatable(
List.of(1L), "id==*", ds2Type2.getType(), UNPAGED))
.hasSize(1)
.extracting(Identifiable::getId).containsOnly(target1Type1.getId());
assertThat(targetManagement.countByRsqlAndNotInRolloutGroupsAndCompatibleAndUpdatable("id==*", List.of(1L),
ds2Type2.getType())).isEqualTo(1);
final Rollout rollout = testdataFactory.createRolloutByVariables(
"testRollout", "testDescription", 3, "id==*", ds2Type2, "50", "5");
final List<Long> foundTargetIds = rolloutGroupManagement.findByRollout(rollout.getId(), UNPAGED).getContent().stream()
.flatMap(rolloutGroup -> targetManagement.findByInRolloutGroupWithoutAction(rolloutGroup.getId(), UNPAGED)
.getContent().stream()).map(Identifiable::getId).toList();
assertThat(foundTargetIds).hasSize(1).contains(target1Type1.getId());
assertThat(targetManagement.findByRsqlAndNotInRolloutGroupsAndCompatibleAndUpdatable(
List.of(1L), "id==*", ds2Type2.getType(), UNPAGED))
.hasSize(1)
.extracting(Identifiable::getId).containsOnly(target1Type1.getId());
List.of(1L), "id==*", ds2Type2.getType(), UNPAGED).stream().toList())
.containsExactly(target1Type1);
assertThat(targetManagement.countByRsqlAndNotInRolloutGroupsAndCompatibleAndUpdatable("id==*", List.of(1L), ds2Type2.getType()))
.isEqualTo(1);
final Rollout rollout = testdataFactory.createRolloutByVariables("testRollout", "testDescription", 3, "id==*", ds2Type2, "50", "5");
final List<Long> groups = rolloutGroupManagement.findByRollout(rollout.getId(), UNPAGED).getContent().stream().
map(Identifiable::getId).toList();
assertThat(groups.stream().flatMap(
group -> targetManagement.findByInRolloutGroupWithoutAction(group, UNPAGED).get()).toList())
.containsExactly(target1Type1);
assertThat(groups.stream().flatMap(
group -> rolloutGroupManagement.findTargetsOfRolloutGroup(group, UNPAGED).get()).toList())
.containsExactly(target1Type1);
assertThat(targetManagement.findByRsqlAndNotInRolloutGroupsAndCompatibleAndUpdatable(groups, "id==*", ds2Type2.getType(), UNPAGED))
.isEmpty();
assertThat(targetManagement.countByRsqlAndNotInRolloutGroupsAndCompatibleAndUpdatable("id==*", groups, ds2Type2.getType()))
.isZero();
// as system in context - doesn't apply scopes
final Rollout runAsSystem = systemSecurityContext.runAsSystem(
() -> testdataFactory.createRolloutByVariables("testRollout", "testDescription", 3, "id==*", ds2Type2, "50", "5"));
() -> testdataFactory.createRolloutByVariables(
"testRolloutAsSystem", "testDescriptionAsSystem", 3, "id==*", ds2Type2, "50", "5"));
assertThat(rolloutGroupManagement.findByRollout(runAsSystem.getId(), UNPAGED).getContent().stream()
.flatMap(rolloutGroup -> targetManagement.findByInRolloutGroupWithoutAction(rolloutGroup.getId(), UNPAGED)
.getContent().stream()).map(Identifiable::getId).toList())
.flatMap(
group -> targetManagement.findByInRolloutGroupWithoutAction(group.getId(), UNPAGED).getContent().stream()).toList())
.hasSize(3);
});
}
@@ -288,44 +296,4 @@ class TargetManagementTest extends AbstractAccessControllerManagementTest {
Action.ActionStatusCreate.builder().actionId(action.getId()).status(Action.Status.FINISHED).build());
});
}
/**
* Verifies only manageable targets are part of the rollout
*/
@Test
void verifyRolloutTargetScope() {
final DistributionSet ds = testdataFactory.createDistributionSet("myDs");
distributionSetManagement.lock(ds);
final String[] updateTargetControllerIds = { "update1", "update2", "update3" };
final List<Target> updateTargets = testdataFactory.createTargets(updateTargetControllerIds);
final String[] readTargetControllerIds = { "read1", "read2", "read3", "read4" };
final List<Target> readTargets = testdataFactory.createTargets(readTargetControllerIds);
final List<Target> hiddenTargets = testdataFactory.createTargets("hidden1", "hidden2", "hidden3", "hidden4", "hidden5");
runAs(withUser("user",
READ_DISTRIBUTION_SET,
READ_TARGET + "/controllerId=in=(" + String.join(", ", List.of(updateTargetControllerIds)) + ")" +
" or controllerId=in=(" + String.join(", ", List.of(readTargetControllerIds)) + ")",
UPDATE_TARGET + "/controllerId=in=(" + String.join(", ", List.of(updateTargetControllerIds)) + ")",
CREATE_ROLLOUT, READ_ROLLOUT), () -> {
final Rollout rollout = testdataFactory.createRolloutByVariables(
"testRollout", "description", updateTargets.size(), "id==*", ds, "50", "5");
assertThat(rollout.getTotalTargets()).isEqualTo(updateTargets.size());
final List<RolloutGroup> content = rolloutGroupManagement.findByRollout(rollout.getId(), Pageable.unpaged()).getContent();
assertThat(content).hasSize(updateTargets.size());
final List<Target> rolloutTargets = content.stream().flatMap(
group -> rolloutGroupManagement.findTargetsOfRolloutGroup(group.getId(), Pageable.unpaged()).get())
.toList();
assertThat(rolloutTargets).hasSize(updateTargets.size()).allMatch(
target -> updateTargets.stream().anyMatch(readTarget -> readTarget.getId().equals(target.getId())))
.noneMatch(target -> readTargets.stream()
.anyMatch(readTarget -> readTarget.getId().equals(target.getId())))
.noneMatch(target -> hiddenTargets.stream()
.anyMatch(readTarget -> readTarget.getId().equals(target.getId())));
});
}
}

View File

@@ -91,7 +91,7 @@ class VirtualPropertyResolverTest {
@ParameterizedTest
@ValueSource(strings = { "${NOW_TS}", "${OVERDUE_TS}", "${overdue_ts}" })
void resolveNowTimestampPlaceholder(final String placeholder) {
when(securityContext.runAsSystem(Mockito.any())).thenAnswer(a -> ((Callable<?>) a.getArgument(0)).call());
when(securityContext.runAsSystem(Mockito.any(Callable.class))).thenAnswer(a -> ((Callable<?>) a.getArgument(0)).call());
final String testString = "lhs=lt=" + placeholder;
final String resolvedPlaceholders = substitutor.replace(testString);