diff --git a/hawkbit-dmf/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerService.java b/hawkbit-dmf/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerService.java index 46c0a1cd3..a83af238b 100644 --- a/hawkbit-dmf/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerService.java +++ b/hawkbit-dmf/hawkbit-dmf-amqp/src/main/java/org/eclipse/hawkbit/amqp/AmqpMessageHandlerService.java @@ -169,11 +169,11 @@ public class AmqpMessageHandlerService extends BaseAmqpService { SecurityContextHolder.setContext(securityContextImpl); } - private static void setTenantSecurityContext(final String tenantId) { + private static void setTenantSecurityContext(final String tenant) { final AnonymousAuthenticationToken authenticationToken = new AnonymousAuthenticationToken( UUID.randomUUID().toString(), "AMQP-Controller", List.of(new SimpleGrantedAuthority(SpRole.CONTROLLER_ROLE_ANONYMOUS))); - authenticationToken.setDetails(new TenantAwareAuthenticationDetails(tenantId, true)); + authenticationToken.setDetails(new TenantAwareAuthenticationDetails(tenant, true)); setSecurityContext(authenticationToken); } diff --git a/hawkbit-mgmt/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtRolloutResourceTest.java b/hawkbit-mgmt/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtRolloutResourceTest.java index 610f6c6d1..59eb056f0 100644 --- a/hawkbit-mgmt/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtRolloutResourceTest.java +++ b/hawkbit-mgmt/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtRolloutResourceTest.java @@ -41,7 +41,6 @@ import org.eclipse.hawkbit.mgmt.json.model.rollout.MgmtRolloutResponseBody; import org.eclipse.hawkbit.mgmt.rest.api.MgmtRestConstants; import org.eclipse.hawkbit.mgmt.rest.api.MgmtRolloutRestApi; import org.eclipse.hawkbit.mgmt.rest.resource.mapper.MgmtRestModelMapper; -import org.eclipse.hawkbit.repository.DistributionSetManagement; import org.eclipse.hawkbit.repository.RolloutGroupManagement; import org.eclipse.hawkbit.repository.RolloutManagement; import org.eclipse.hawkbit.repository.RolloutManagement.Create; @@ -345,6 +344,37 @@ class MgmtRolloutResourceTest extends AbstractManagementApiIntegrationTest { .andExpect(jsonPath("errorCode", equalTo("hawkbit.server.error.rest.body.notReadable"))); } + /** + * Testing that creating rollout with insufficient permission returns forbidden + */ + @Test + @WithUser(principal = "bumlux", authorities = {SpRole.TARGET_ADMIN, SpRole.REPOSITORY_ADMIN, "CREATE_ROLLOUT"}) + void createRolloutWithSufficientPermissionsIsOk() throws Exception { + createRolloutWithPermissions("rollout-suff", 201); + } + + @Test + @WithUser(authorities = {SpRole.TARGET_ADMIN, SpRole.REPOSITORY_ADMIN}) + void createRolloutWithInsufficientPermissionReturnsForbidden() throws Exception { + createRolloutWithPermissions("rollout-insuff", 403); + } + + private void createRolloutWithPermissions(final String name, final int expectedStatus) throws Exception { + testdataFactory.createTargets(20, "target", "rollout"); + final DistributionSet dsA = testdataFactory.createDistributionSet(""); + final String actionType = MgmtRestModelMapper.convertActionType(Action.ActionType.FORCED).getName(); + final String rollout = JsonBuilder.rollout(name, "desc", 5, dsA.getId(), "id==target*", + new RolloutGroupConditionBuilder().withDefaults().build(), null, actionType, null, null, null, + null, false, null, 0); + + mvc.perform(post("/rest/v1/rollouts") + .contentType(MediaType.APPLICATION_JSON).accept(MediaType.APPLICATION_JSON) + .content(rollout)) + .andDo(MockMvcResultPrinter.print()) + .andExpect(status().is(expectedStatus)) + .andReturn(); + } + /** * Testing that creating rollout with not existing distribution set returns not found */ @@ -377,9 +407,7 @@ class MgmtRolloutResourceTest extends AbstractManagementApiIntegrationTest { */ @Test void missingTargetFilterQueryInRollout() throws Exception { - final String targetFilterQuery = null; - final DistributionSet dsA = testdataFactory.createDistributionSet(""); mvc.perform(post("/rest/v1/rollouts") .content(JsonBuilder.rollout("rollout1", "desc", 10, dsA.getId(), targetFilterQuery, null)) @@ -397,7 +425,6 @@ class MgmtRolloutResourceTest extends AbstractManagementApiIntegrationTest { @Test void createRollout() throws Exception { testdataFactory.createTargets(20, "target", "rollout"); - final DistributionSet dsA = testdataFactory.createDistributionSet(""); postRollout("rollout1", 5, dsA.getId(), "id==target*", 20, Action.ActionType.FORCED); } diff --git a/hawkbit-mgmt/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTargetResourceTest.java b/hawkbit-mgmt/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTargetResourceTest.java index fd415c454..84fbc80b1 100644 --- a/hawkbit-mgmt/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTargetResourceTest.java +++ b/hawkbit-mgmt/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTargetResourceTest.java @@ -415,7 +415,6 @@ class MgmtTargetResourceTest extends AbstractManagementApiIntegrationTest { @Test @WithUser(authorities = { SpPermission.READ_TARGET, SpPermission.CREATE_TARGET }) void securityTokenIsNotInResponseIfMissingPermission() throws Exception { - final String knownControllerId = "knownControllerId"; testdataFactory.createTarget(knownControllerId); mvc.perform(get(TARGETS_V1 + "/{targetId}", knownControllerId)) diff --git a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/TenantStatsManagement.java b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/TenantStatsManagement.java index 8dba38498..2ee87dd81 100644 --- a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/TenantStatsManagement.java +++ b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/TenantStatsManagement.java @@ -10,7 +10,6 @@ package org.eclipse.hawkbit.repository; import org.eclipse.hawkbit.auth.SpRole; -import org.eclipse.hawkbit.auth.SpringEvalExpressions; import org.eclipse.hawkbit.repository.model.report.TenantUsage; import org.springframework.security.access.prepost.PreAuthorize; diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/ArtifactManagementTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/ArtifactManagementTest.java index 6d63b143d..55e92b8ac 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/ArtifactManagementTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/ArtifactManagementTest.java @@ -50,7 +50,6 @@ import org.eclipse.hawkbit.repository.model.SoftwareModule; 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.junit.jupiter.api.Test; /** @@ -411,14 +410,6 @@ class ArtifactManagementTest extends AbstractJpaIntegrationTest { } } - @Test - @WithUser(authorities = {}) - void getArtifactBinaryWithoutDownloadArtifactThrowsPermissionDenied() { - assertThatExceptionOfType(InsufficientPermissionException.class) - .as("Should not have worked with missing permission.") - .isThrownBy(() -> artifactManagement.getArtifactStream("123", 1, false)); - } - /** * Verifies that creation of an artifact with none matching hashes fails. */ @@ -537,7 +528,7 @@ class ArtifactManagementTest extends AbstractJpaIntegrationTest { } private T runAsTenant(final String tenant, final Callable callable) throws Exception { - return SecurityContextSwitch.callAs(SecurityContextSwitch.withUserAndTenantAllSpPermissions("user", tenant), callable); + return SecurityContextSwitch.callAs(SecurityContextSwitch.withTenantAndUserAndAllPermissions(tenant, "user"), callable); } private SoftwareModule createSoftwareModuleForTenant(final String tenant) throws Exception { diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/SystemManagementTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/SystemManagementTest.java index fcb82e63b..4a2dfa805 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/SystemManagementTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/SystemManagementTest.java @@ -10,7 +10,7 @@ package org.eclipse.hawkbit.repository.jpa.management; import static org.assertj.core.api.Assertions.assertThat; -import static org.eclipse.hawkbit.repository.test.util.SecurityContextSwitch.withUserAndTenant; +import static org.eclipse.hawkbit.repository.test.util.SecurityContextSwitch.withTenantAndUser; import java.io.ByteArrayInputStream; import java.util.ArrayList; @@ -57,7 +57,7 @@ class SystemManagementTest extends AbstractJpaIntegrationTest { for (int i = 0; i < tenants; i++) { final String tenantname = "TENANT" + i; SecurityContextSwitch.getAs( - withUserAndTenant(tenantname, "bumlux", new String[] { SpRole.SYSTEM_ROLE }, true, true), + withTenantAndUser(tenantname, "bumlux", new String[] { SpRole.SYSTEM_ROLE }, true, true), () -> { systemManagement.getTenantMetadataWithoutDetails(); if (artifactSize > 0) { diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/tenancy/MultiTenancyEntityTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/tenancy/MultiTenancyEntityTest.java index 7763b8f8f..ae92b521c 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/tenancy/MultiTenancyEntityTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/tenancy/MultiTenancyEntityTest.java @@ -71,7 +71,7 @@ class MultiTenancyEntityTest extends AbstractJpaIntegrationTest { * Ensures that targets created by a tenant are not visible by another tenant. */ @Test - @WithUser(tenantId = "mytenant", authorities = SpRole.TENANT_ADMIN) + @WithUser(tenant = "mytenant", authorities = SpRole.TENANT_ADMIN) void queryTargetFromDifferentTenantIsNotVisible() throws Exception { // create target for another tenant final String anotherTenant = "anotherTenant"; @@ -93,7 +93,7 @@ class MultiTenancyEntityTest extends AbstractJpaIntegrationTest { * Ensures that tenant with proper permissions can read and delete other tenants. */ @Test - @WithUser(tenantId = "mytenant", authorities = SpRole.TENANT_ADMIN) + @WithUser(tenant = "mytenant", authorities = SpRole.TENANT_ADMIN) void deleteAnotherTenantNotPossibleWithTenantPermissions() throws Exception { // create target for another tenant final String anotherTenant = "anotherTenant"; @@ -106,7 +106,7 @@ class MultiTenancyEntityTest extends AbstractJpaIntegrationTest { } @Test - @WithUser(tenantId = "mytenant", authorities = { SpRole.SYSTEM_ROLE }) + @WithUser(tenant = "mytenant", authorities = { SpRole.SYSTEM_ROLE }) void deleteAnotherTenantPossibleWithSystemRole() throws Exception { // create target for another tenant final String anotherTenant = "anotherTenant"; @@ -122,7 +122,7 @@ class MultiTenancyEntityTest extends AbstractJpaIntegrationTest { * Ensures that tenant metadata is retrieved for the current tenant. */ @Test - @WithUser(tenantId = "mytenant", autoCreateTenant = false, authorities = SpRole.TENANT_ADMIN) + @WithUser(tenant = "mytenant", autoCreateTenant = false, authorities = SpRole.TENANT_ADMIN) void getTenantMetdata() throws Exception { // logged in tenant mytenant - check if tenant default data is autogenerated assertThat(distributionSetTypeManagement.findAll(PAGE)).isEmpty(); @@ -133,7 +133,7 @@ class MultiTenancyEntityTest extends AbstractJpaIntegrationTest { // check that the cache is not getting in the way, i.e. "bumlux" results in bumlux and not mytenant assertThat(SecurityContextSwitch.getAs( - SecurityContextSwitch.withUserAndTenantAllSpPermissions("user", "bumlux"), + SecurityContextSwitch.withTenantAndUserAndAllPermissions("bumlux", "user"), () -> systemManagement.getTenantMetadataWithoutDetails().getTenant().toUpperCase())) .isEqualTo("bumlux".toUpperCase()); } @@ -142,7 +142,7 @@ class MultiTenancyEntityTest extends AbstractJpaIntegrationTest { * Ensures that targets created from a different tenant cannot be deleted from other tenants */ @Test - @WithUser(tenantId = "mytenant", authorities = SpRole.TENANT_ADMIN) + @WithUser(tenant = "mytenant", authorities = SpRole.TENANT_ADMIN) void deleteTargetFromOtherTenantIsNotPossible() throws Exception { // create target for another tenant final String anotherTenant = "anotherTenant"; @@ -191,7 +191,7 @@ class MultiTenancyEntityTest extends AbstractJpaIntegrationTest { } private T runAsTenant(final String tenant, final Callable callable) throws Exception { - return SecurityContextSwitch.callAs(SecurityContextSwitch.withUserAndTenantAllSpPermissions("user", tenant), callable); + return SecurityContextSwitch.callAs(SecurityContextSwitch.withTenantAndUserAndAllPermissions(tenant, "user"), callable); } private Target createTargetForTenant(final String controllerId, final String tenant) throws Exception { diff --git a/hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/util/AbstractIntegrationTest.java b/hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/util/AbstractIntegrationTest.java index 85e4d0275..43f7df003 100644 --- a/hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/util/AbstractIntegrationTest.java +++ b/hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/util/AbstractIntegrationTest.java @@ -38,7 +38,6 @@ import org.awaitility.Awaitility; import org.awaitility.core.ConditionFactory; import org.eclipse.hawkbit.artifact.ArtifactStorage; import org.eclipse.hawkbit.artifact.exception.ArtifactStoreException; -import org.eclipse.hawkbit.auth.SpRole; import org.eclipse.hawkbit.repository.ArtifactManagement; import org.eclipse.hawkbit.repository.ConfirmationManagement; import org.eclipse.hawkbit.repository.ControllerManagement; diff --git a/hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/util/SecurityContextSwitch.java b/hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/util/SecurityContextSwitch.java index 0b57dbf9a..23cb85ac4 100644 --- a/hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/util/SecurityContextSwitch.java +++ b/hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/util/SecurityContextSwitch.java @@ -67,7 +67,7 @@ public class SecurityContextSwitch { setSecurityContext(withUser); try { if (withUser.autoCreateTenant()) { - createTenant(withUser.tenantId()); + createTenant(withUser.tenant()); } return callable.call(); } finally { @@ -93,18 +93,18 @@ public class SecurityContextSwitch { } public static WithUser withController(final String principal, final String... authorities) { - return withUserAndTenant(DEFAULT_TENANT, principal, authorities, true, true); + return withTenantAndUser(DEFAULT_TENANT, principal, authorities, true, true); } public static WithUser withUser(final String principal, final String... authorities) { - return withUserAndTenant(DEFAULT_TENANT, principal, authorities, false, true); + return withTenantAndUser(DEFAULT_TENANT, principal, authorities, false, true); } - public static WithUser withUserAndTenantAllSpPermissions(final String principal, final String tenant) { - return withUserAndTenant(tenant, principal, new String[] { SpRole.TENANT_ADMIN }, false, true); + public static WithUser withTenantAndUserAndAllPermissions(final String tenant, final String principal) { + return withTenantAndUser(tenant, principal, new String[] { SpRole.TENANT_ADMIN }, false, true); } - public static WithUser withUserAndTenant( + public static WithUser withTenantAndUser( final String tenant, final String principal, final String[] authorities, final boolean controller, final boolean autoCreateTenant) { return new WithUserImpl(tenant, principal, authorities, controller, autoCreateTenant); @@ -114,11 +114,11 @@ public class SecurityContextSwitch { SecurityContextHolder.setContext(new WithUserSecurityContext(annotation)); } - private static void createTenant(final String tenantId) { + private static void createTenant(final String tenant) { final SecurityContext oldContext = SecurityContextHolder.getContext(); setSecurityContext(PRIVILEGED_USER); try { - systemManagement.createTenantMetadata(tenantId); + systemManagement.createTenantMetadata(tenant); } finally { SecurityContextHolder.setContext(oldContext); } @@ -140,17 +140,17 @@ public class SecurityContextSwitch { WithUserSecurityContext(final WithUser annotation) { this.annotation = annotation; if (annotation.autoCreateTenant()) { - createTenant(annotation.tenantId()); + createTenant(annotation.tenant()); } } @Override public Authentication getAuthentication() { final TestingAuthenticationToken testingAuthenticationToken = new TestingAuthenticationToken( - new TenantAwareUser(annotation.principal(), "***", null, annotation.tenantId()), + new TenantAwareUser(annotation.principal(), "***", null, annotation.tenant()), annotation.credentials(), annotation.authorities()); testingAuthenticationToken.setDetails( - new TenantAwareAuthenticationDetails(annotation.tenantId(), annotation.controller())); + new TenantAwareAuthenticationDetails(annotation.tenant(), annotation.controller())); return testingAuthenticationToken; } @@ -201,7 +201,7 @@ public class SecurityContextSwitch { } @Override - public String tenantId() { + public String tenant() { return tenant; } diff --git a/hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/util/WithUser.java b/hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/util/WithUser.java index aeb1c499a..d4f0ede2c 100644 --- a/hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/util/WithUser.java +++ b/hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/util/WithUser.java @@ -33,7 +33,7 @@ public @interface WithUser { * * @return test tenant id */ - String tenantId() default "DEFAULT"; + String tenant() default "DEFAULT"; /** * Gets the test principal. diff --git a/hawkbit-sdk/hawkbit-sdk-dmf/src/main/java/org/eclipse/hawkbit/sdk/dmf/amqp/VHost.java b/hawkbit-sdk/hawkbit-sdk-dmf/src/main/java/org/eclipse/hawkbit/sdk/dmf/amqp/VHost.java index 9420f8d02..861422813 100644 --- a/hawkbit-sdk/hawkbit-sdk-dmf/src/main/java/org/eclipse/hawkbit/sdk/dmf/amqp/VHost.java +++ b/hawkbit-sdk/hawkbit-sdk-dmf/src/main/java/org/eclipse/hawkbit/sdk/dmf/amqp/VHost.java @@ -119,7 +119,12 @@ public final class VHost extends DmfSender implements MessageListener { } } - protected void handleAttributeUpdateRequest(final Message message, final String controllerId) { + void stop() { + container.stop(); + rabbitTemplate.destroy(); + } + + private void handleAttributeUpdateRequest(final Message message, final String controllerId) { final String tenantId = getTenant(message); Optional.ofNullable(dmfTenants.get(tenantId)) .flatMap(dmfTenant -> dmfTenant.getController(controllerId)) @@ -127,12 +132,12 @@ public final class VHost extends DmfSender implements MessageListener { updateAttributes(tenantId, controllerId, DmfUpdateMode.MERGE, controller.getAttributes())); } - protected void handleCancelDownloadAction(final Message message, final String thingId) { + private void handleCancelDownloadAction(final Message message, final String thingId) { final Long actionId = extractActionIdFrom(message); processCancelDownloadAction(thingId, actionId); } - protected void handleUpdateProcess(final Message message, final String controllerId, final EventTopic actionType) { + private void handleUpdateProcess(final Message message, final String controllerId, final EventTopic actionType) { final String tenant = getTenant(message); final DmfDownloadAndUpdateRequest downloadAndUpdateRequest = convertMessage(message, DmfDownloadAndUpdateRequest.class); @@ -141,11 +146,6 @@ public final class VHost extends DmfSender implements MessageListener { processUpdate(tenant, controllerId, actionType, downloadAndUpdateRequest); } - void stop() { - container.stop(); - rabbitTemplate.destroy(); - } - private static String getTenant(final Message message) { final MessageProperties messageProperties = message.getMessageProperties(); final Map headers = messageProperties.getHeaders(); @@ -252,9 +252,9 @@ public final class VHost extends DmfSender implements MessageListener { openActions.remove(actionId); } - private void processUpdate(final String tenantId, final String controllerId, final EventTopic actionType, + private void processUpdate(final String tenant, final String controllerId, final EventTopic actionType, final DmfDownloadAndUpdateRequest updateRequest) { - Optional.ofNullable(dmfTenants.get(tenantId)) + Optional.ofNullable(dmfTenants.get(tenant)) .flatMap(dmfTenant -> dmfTenant.getController(controllerId)) .ifPresent(controller -> controller.processUpdate(actionType, updateRequest)); }