Improved security tests - some tests failing with expected InsufficientPermissionException but not from the initial call, but after unexpectedly call permissions are resolved, content of method throws expected InsufficientPermissionException. (#2171)

Added more tests for methods with @PreAuthorize with combination of 'OR' permissions.

Co-authored-by: vasilchev <vasil.ilchev@bosch.com>
This commit is contained in:
Vasil Ilchev
2025-01-07 17:03:00 +02:00
committed by GitHub
parent ad5c4a2389
commit 5177409fb4
10 changed files with 55 additions and 12 deletions

View File

@@ -87,7 +87,7 @@ public abstract class AbstractJpaIntegrationTest extends AbstractIntegrationTest
protected static final String NOT_EXIST_ID = "12345678990";
protected static final long NOT_EXIST_IDL = Long.parseLong(NOT_EXIST_ID);
protected static final List<String> REPOSITORY_AND_TARGET_PERMISSIONS = List.of(SpPermission.READ_REPOSITORY, SpPermission.CREATE_REPOSITORY, SpPermission.UPDATE_REPOSITORY, SpPermission.DELETE_REPOSITORY, SpPermission.READ_TARGET, SpPermission.CREATE_TARGET, SpPermission.UPDATE_TARGET, SpPermission.DELETE_TARGET);
private static final List<String> REPOSITORY_AND_TARGET_PERMISSIONS = List.of(SpPermission.READ_REPOSITORY, SpPermission.CREATE_REPOSITORY, SpPermission.UPDATE_REPOSITORY, SpPermission.DELETE_REPOSITORY, SpPermission.READ_TARGET, SpPermission.CREATE_TARGET, SpPermission.UPDATE_TARGET, SpPermission.DELETE_TARGET);
@PersistenceContext
protected EntityManager entityManager;
@@ -247,9 +247,22 @@ public abstract class AbstractJpaIntegrationTest extends AbstractIntegrationTest
*
* @param callable the callable to call
*/
@SneakyThrows
protected void assertPermissions(final Callable<?> callable, List<String> requiredPermissions) {
final List<String> insufficiantPermissions = REPOSITORY_AND_TARGET_PERMISSIONS.stream()
assertPermissions(callable, requiredPermissions, null);
}
/**
* Asserts that the given callable throws an InsufficientPermissionException.
* @param callable the callable to call
* @param requiredPermissions required permissions for the callable
* @param insufficientPermissions can be null, if null, it will be resolved automatically. But in some cases (e.g. @PreAuthorized Permissions with OR, it is safer to pass directly the insufficient permissions)
*/
@SneakyThrows
protected void assertPermissions(final Callable<?> callable, final List<String> requiredPermissions, final List<String> insufficientPermissions) {
// if READ_PERMISSION is required and required permissions are multiple, give only READ_PERMISSION to eliminate internal read_permission check failure that would confuse the actual test
final List<String> resolvedInsufficientPermissions = insufficientPermissions != null ? insufficientPermissions :
requiredPermissions.contains(SpPermission.READ_REPOSITORY) && requiredPermissions.size() > 1 ?
List.of(SpPermission.READ_REPOSITORY) : REPOSITORY_AND_TARGET_PERMISSIONS.stream()
.filter(p -> !requiredPermissions.contains(p)).toList();
// check if the user has the correct permissions
SecurityContextSwitch.runAs(SecurityContextSwitch.withUser("user_with_permissions", requiredPermissions.toArray(new String[0])), () -> {
@@ -259,13 +272,14 @@ public abstract class AbstractJpaIntegrationTest extends AbstractIntegrationTest
});
// check if the user has the insufficient permissions
SecurityContextSwitch.runAs(SecurityContextSwitch.withUser("user_without_permissions", insufficiantPermissions.toArray(new String[0])), () -> {
SecurityContextSwitch.runAs(SecurityContextSwitch.withUser("user_without_permissions", resolvedInsufficientPermissions.toArray(new String[0])), () -> {
assertInsufficientPermission(callable);
log.info("assertInsufficientPermission Passed");
return null;
});
}
/**
* Asserts that the given callable throws an InsufficientPermissionException.
* If callable succeeds without any exception or exception other than InsufficientPermissionException, it will be considered as an assert failure.

View File

@@ -52,25 +52,30 @@ class ArtifactManagementSecurityTest extends AbstractJpaIntegrationTest {
@Description("Tests ArtifactManagement#get() method")
void getPermissionCheck() {
assertPermissions(() -> artifactManagement.get(1L), List.of(SpPermission.READ_REPOSITORY));
assertPermissions(() -> artifactManagement.get(1L), List.of(SpPermission.SpringEvalExpressions.CONTROLLER_ROLE), List.of(SpPermission.CREATE_REPOSITORY));
}
@Test
@Description("Tests ArtifactManagement#getByFilenameAndSoftwareModule() method")
void getByFilenameAndSoftwareModulePermissionCheck() {
assertPermissions(() -> artifactManagement.getByFilenameAndSoftwareModule("filename", 1L),
List.of(SpPermission.READ_REPOSITORY));
List.of(SpPermission.READ_REPOSITORY), List.of(SpPermission.CREATE_REPOSITORY));
assertPermissions(() -> artifactManagement.getByFilenameAndSoftwareModule("filename", 1L),
List.of(SpPermission.SpringEvalExpressions.CONTROLLER_ROLE), List.of(SpPermission.CREATE_REPOSITORY));
}
@Test
@Description("Tests ArtifactManagement#findFirstBySHA1() method")
void findFirstBySHA1PermissionCheck() {
assertPermissions(() -> artifactManagement.findFirstBySHA1("sha1"), List.of(SpPermission.READ_REPOSITORY));
assertPermissions(() -> artifactManagement.findFirstBySHA1("sha1"), List.of(SpPermission.SpringEvalExpressions.CONTROLLER_ROLE), List.of(SpPermission.CREATE_REPOSITORY));
}
@Test
@Description("Tests ArtifactManagement#getByFilename() method")
void getByFilenamePermissionCheck() {
assertPermissions(() -> artifactManagement.getByFilename("filename"), List.of(SpPermission.READ_REPOSITORY));
assertPermissions(() -> artifactManagement.getByFilename("filename"), List.of(SpPermission.SpringEvalExpressions.CONTROLLER_ROLE), List.of(SpPermission.CREATE_REPOSITORY));
}
@Test
@@ -88,7 +93,8 @@ class ArtifactManagementSecurityTest extends AbstractJpaIntegrationTest {
@Test
@Description("Tests ArtifactManagement#loadArtifactBinary() method")
void loadArtifactBinaryPermissionCheck() {
assertPermissions(() -> artifactManagement.loadArtifactBinary("sha1", 1L, false), List.of(SpPermission.DOWNLOAD_REPOSITORY_ARTIFACT));
assertPermissions(() -> artifactManagement.loadArtifactBinary("sha1", 1L, false), List.of(SpPermission.DOWNLOAD_REPOSITORY_ARTIFACT), List.of(SpPermission.CREATE_REPOSITORY));
assertPermissions(() -> artifactManagement.loadArtifactBinary("sha1", 1L, false), List.of(SpPermission.SpringEvalExpressions.CONTROLLER_ROLE), List.of(SpPermission.CREATE_REPOSITORY));
}
}

View File

@@ -38,7 +38,9 @@ class ConfirmationManagementSecurityTest extends AbstractJpaIntegrationTest {
@Test
@Description("Tests ConfirmationManagement#getStatus() method")
void getStatusPermissionsCheck() {
assertPermissions(() -> confirmationManagement.getStatus("controllerId"), List.of(SpPermission.READ_TARGET));
assertPermissions(() -> confirmationManagement.getStatus("controllerId"), List.of(SpPermission.READ_TARGET),
List.of(SpPermission.CREATE_TARGET));
assertPermissions(() -> confirmationManagement.getStatus("controllerId"), List.of(SpPermission.SpringEvalExpressions.CONTROLLER_ROLE), List.of(SpPermission.CREATE_TARGET));
}
@Test

View File

@@ -159,12 +159,15 @@ class ControllerManagementSecurityTest extends AbstractJpaIntegrationTest {
void getByControllerIdPermissionsCheck() {
assertPermissions(() -> controllerManagement.getByControllerId("controllerId"),
List.of(SpPermission.SpringEvalExpressions.CONTROLLER_ROLE));
assertPermissions(() -> controllerManagement.getByControllerId("controllerId"),
List.of(SpPermission.SpringEvalExpressions.SYSTEM_ROLE));
}
@Test
@Description("Tests ControllerManagement#get() method")
void getPermissionsCheck() {
assertPermissions(() -> controllerManagement.get(1L), List.of(SpPermission.SpringEvalExpressions.CONTROLLER_ROLE));
assertPermissions(() -> controllerManagement.get(1L), List.of(SpPermission.SpringEvalExpressions.SYSTEM_ROLE));
}
@Test

View File

@@ -10,6 +10,7 @@
package org.eclipse.hawkbit.repository.jpa.management;
import java.util.List;
import java.util.Random;
import io.qameta.allure.Description;
import io.qameta.allure.Feature;
@@ -244,6 +245,7 @@ class DistributionSetManagementSecurityTest
@Test
@Description("Tests ManagementAPI PreAuthorized method with correct and insufficient permissions.")
void invalidatePermissionsCheck() {
distributionSetTypeManagement.create(entityFactory.distributionSetType().create().key("type").name("name"));
assertPermissions(() -> {
distributionSetManagement.invalidate(entityFactory.distributionSet().create().name("name").version("1.0").type("type").build());
return null;

View File

@@ -10,6 +10,7 @@
package org.eclipse.hawkbit.repository.jpa.management;
import java.util.List;
import java.util.Random;
import io.qameta.allure.Description;
import io.qameta.allure.Feature;
@@ -35,7 +36,7 @@ public class DistributionSetTagManagementSecurityTest
@Override
protected TagCreate getCreateObject() {
return entityFactory.tag().create().name("tag");
return entityFactory.tag().create().name(String.format("tag-%d", new Random().nextInt()));
}
@Override

View File

@@ -10,6 +10,7 @@
package org.eclipse.hawkbit.repository.jpa.management;
import java.util.List;
import java.util.Random;
import io.qameta.allure.Description;
import io.qameta.allure.Feature;
@@ -34,7 +35,7 @@ public class DistributionSetTypeManagementSecurityTest
@Override
protected DistributionSetTypeCreate getCreateObject() {
return entityFactory.distributionSetType().create().key("key").name("name");
return entityFactory.distributionSetType().create().key(String.format("key-%d", new Random().nextInt())).name(String.format("name-%d", new Random().nextInt()));
}
@Override

View File

@@ -22,9 +22,11 @@ import org.eclipse.hawkbit.repository.builder.DistributionSetCreate;
import org.eclipse.hawkbit.repository.builder.DistributionSetTypeCreate;
import org.eclipse.hawkbit.repository.builder.DynamicRolloutGroupTemplate;
import org.eclipse.hawkbit.repository.jpa.AbstractJpaIntegrationTest;
import org.eclipse.hawkbit.repository.jpa.model.JpaRollout;
import org.eclipse.hawkbit.repository.model.DistributionSet;
import org.eclipse.hawkbit.repository.model.Rollout;
import org.eclipse.hawkbit.repository.model.RolloutGroupConditionBuilder;
import org.eclipse.hawkbit.repository.model.SoftwareModule;
import org.eclipse.hawkbit.repository.test.util.WithUser;
import org.junit.jupiter.api.Test;
import org.springframework.data.domain.PageImpl;
@@ -158,8 +160,16 @@ public class RolloutManagementSecurityTest extends AbstractJpaIntegrationTest {
@Test
@Description("Tests ManagementAPI PreAuthorized method with correct and insufficient permissions.")
void setRolloutStatusDetailsPermissionsCheck() {
final String rolloutName = "rollout-std";
final int amountGroups = 5; // static only
final String targetPrefix = "controller-rollout-std-";
final DistributionSet distributionSet = testdataFactory.createDistributionSet("dsFor" + rolloutName);
testdataFactory.createTargets(targetPrefix, 0, amountGroups * 3);
final Rollout rollout = testdataFactory.createRolloutByVariables(rolloutName, rolloutName, amountGroups,
"controllerid==" + targetPrefix + "*", distributionSet, "60", "30", false, false);
assertPermissions(() -> {
rolloutManagement.setRolloutStatusDetails(new PageImpl<>(List.of(entityFactory.rollout().create().distributionSetId(1L).build())));
rolloutManagement.setRolloutStatusDetails(new PageImpl<>(List.of(rollout)));
return null;
}, List.of(SpPermission.UPDATE_ROLLOUT, SpPermission.READ_REPOSITORY));
}

View File

@@ -10,6 +10,7 @@
package org.eclipse.hawkbit.repository.jpa.management;
import java.util.List;
import java.util.Random;
import io.qameta.allure.Description;
import io.qameta.allure.Feature;
@@ -34,7 +35,7 @@ public class SoftwareModuleTypeManagementSecurityTest
@Override
protected SoftwareModuleTypeCreate getCreateObject() {
return entityFactory.softwareModuleType().create().key("key").name("name");
return entityFactory.softwareModuleType().create().key(String.format("key-%d", new Random().nextInt())).name(String.format("name-%d", new Random().nextInt()));
}
@Override

View File

@@ -63,7 +63,10 @@ public class SystemManagementSecurityTest extends AbstractJpaIntegrationTest {
@Test
@Description("Tests ManagementAPI PreAuthorized method with correct and insufficient permissions.")
void getTenantMetadataPermissionsCheck() {
assertPermissions(() -> systemManagement.getTenantMetadata(), List.of(SpPermission.READ_REPOSITORY, SpPermission.READ_TARGET, SpPermission.READ_TENANT_CONFIGURATION));
assertPermissions(() -> systemManagement.getTenantMetadata(), List.of(SpPermission.READ_REPOSITORY), List.of(SpPermission.CREATE_REPOSITORY));
assertPermissions(() -> systemManagement.getTenantMetadata(), List.of(SpPermission.READ_TARGET), List.of(SpPermission.CREATE_REPOSITORY));
assertPermissions(() -> systemManagement.getTenantMetadata(), List.of(SpPermission.READ_TENANT_CONFIGURATION), List.of(SpPermission.CREATE_REPOSITORY));
assertPermissions(() -> systemManagement.getTenantMetadata(), List.of(SpPermission.SpringEvalExpressions.CONTROLLER_ROLE), List.of(SpPermission.CREATE_REPOSITORY));
}
@Test