diff --git a/hawkbit-autoconfigure/src/main/java/org/eclipse/hawkbit/autoconfigure/security/SecurityManagedConfiguration.java b/hawkbit-autoconfigure/src/main/java/org/eclipse/hawkbit/autoconfigure/security/SecurityManagedConfiguration.java index 07ea15a8a..66beeec5a 100644 --- a/hawkbit-autoconfigure/src/main/java/org/eclipse/hawkbit/autoconfigure/security/SecurityManagedConfiguration.java +++ b/hawkbit-autoconfigure/src/main/java/org/eclipse/hawkbit/autoconfigure/security/SecurityManagedConfiguration.java @@ -73,6 +73,7 @@ import org.springframework.security.config.annotation.web.servlet.configuration. import org.springframework.security.config.http.SessionCreationPolicy; import org.springframework.security.core.Authentication; import org.springframework.security.core.authority.SimpleGrantedAuthority; +import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.web.access.intercept.FilterSecurityInterceptor; import org.springframework.security.web.authentication.AnonymousAuthenticationFilter; import org.springframework.security.web.authentication.LoginUrlAuthenticationEntryPoint; @@ -292,6 +293,9 @@ public class SecurityManagedConfiguration { @Autowired private SecurityProperties springSecurityProperties; + @Autowired + private SystemSecurityContext systemSecurityContext; + @Override protected void configure(final HttpSecurity http) throws Exception { @@ -320,14 +324,14 @@ public class SecurityManagedConfiguration { userAuthenticationFilter.destroy(); } }, RequestHeaderAuthenticationFilter.class) - .addFilterAfter( - new AuthenticationSuccessTenantMetadataCreationFilter(tenantAware, systemManagement), - SessionManagementFilter.class) + .addFilterAfter(new AuthenticationSuccessTenantMetadataCreationFilter(systemManagement, + systemSecurityContext), SessionManagementFilter.class) .authorizeRequests().anyRequest().authenticated() .antMatchers(MgmtRestConstants.BASE_SYSTEM_MAPPING + "/admin/**") .hasAnyAuthority(SpPermission.SYSTEM_ADMIN); httpSec.httpBasic().and().exceptionHandling().authenticationEntryPoint(basicAuthEntryPoint); + httpSec.anonymous().disable(); } } @@ -524,12 +528,15 @@ class TenantMetadataSavedRequestAwareVaadinAuthenticationSuccessHandler extends @Autowired private SystemManagement systemManagement; + @Autowired + private SystemSecurityContext systemSecurityContext; + @Override public void onAuthenticationSuccess(final Authentication authentication) throws Exception { if (authentication.getClass().equals(TenantUserPasswordAuthenticationToken.class)) { - systemManagement - .getTenantMetadata(((TenantUserPasswordAuthenticationToken) authentication).getTenant().toString()); + systemSecurityContext.runAsSystemAsTenant(() -> systemManagement.getTenantMetadata(), + ((TenantUserPasswordAuthenticationToken) authentication).getTenant().toString()); } else if (authentication.getClass().equals(UsernamePasswordAuthenticationToken.class)) { // TODO: vaadin4spring-ext-security does not give us the // fullyAuthenticatedToken @@ -538,7 +545,8 @@ class TenantMetadataSavedRequestAwareVaadinAuthenticationSuccessHandler extends // LoginView. This needs to be changed with the update of // vaadin4spring 0.0.7 because it // has been fixed. - systemManagement.getTenantMetadata("DEFAULT"); + final String defaultTenant = "DEFAULT"; + systemSecurityContext.runAsSystemAsTenant(() -> systemManagement.getTenantMetadata(), defaultTenant); } super.onAuthenticationSuccess(authentication); @@ -550,13 +558,13 @@ class TenantMetadataSavedRequestAwareVaadinAuthenticationSuccessHandler extends */ class AuthenticationSuccessTenantMetadataCreationFilter implements Filter { - private final TenantAware tenantAware; private final SystemManagement systemManagement; + private final SystemSecurityContext systemSecurityContext; - AuthenticationSuccessTenantMetadataCreationFilter(final TenantAware tenantAware, - final SystemManagement systemManagement) { - this.tenantAware = tenantAware; + AuthenticationSuccessTenantMetadataCreationFilter(final SystemManagement systemManagement, + final SystemSecurityContext systemSecurityContext) { this.systemManagement = systemManagement; + this.systemSecurityContext = systemSecurityContext; } @Override @@ -567,14 +575,16 @@ class AuthenticationSuccessTenantMetadataCreationFilter implements Filter { @Override public void doFilter(final ServletRequest request, final ServletResponse response, final FilterChain chain) throws IOException, ServletException { - - final String currentTenant = tenantAware.getCurrentTenant(); - if (currentTenant != null) { - // lazy initialize tenant meta data after successful authentication - systemManagement.getTenantMetadata(currentTenant); - } - + lazyCreateTenantMetadata(); chain.doFilter(request, response); + + } + + private void lazyCreateTenantMetadata() { + final Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); + if (authentication != null && authentication.isAuthenticated()) { + systemSecurityContext.runAsSystem(() -> systemManagement.getTenantMetadata()); + } } @Override diff --git a/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiRootControllerTest.java b/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiRootControllerTest.java index 31ec37c58..ba5e3d2f6 100644 --- a/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiRootControllerTest.java +++ b/hawkbit-ddi-resource/src/test/java/org/eclipse/hawkbit/ddi/rest/resource/DdiRootControllerTest.java @@ -8,6 +8,10 @@ */ package org.eclipse.hawkbit.ddi.rest.resource; +import static org.eclipse.hawkbit.im.authentication.SpPermission.SpringEvalExpressions.CONTROLLER_ROLE; +import static org.eclipse.hawkbit.im.authentication.SpPermission.SpringEvalExpressions.CONTROLLER_ROLE_ANONYMOUS; +import static org.eclipse.hawkbit.im.authentication.SpPermission.SpringEvalExpressions.HAS_AUTH_TENANT_CONFIGURATION; +import static org.eclipse.hawkbit.im.authentication.SpPermission.SpringEvalExpressions.SYSTEM_ROLE; import static org.fest.assertions.api.Assertions.assertThat; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.Matchers.startsWith; @@ -23,7 +27,6 @@ import java.util.ArrayList; import java.util.List; import org.eclipse.hawkbit.im.authentication.SpPermission; -import org.eclipse.hawkbit.im.authentication.SpPermission.SpringEvalExpressions; import org.eclipse.hawkbit.repository.model.Action; import org.eclipse.hawkbit.repository.model.DistributionSet; import org.eclipse.hawkbit.repository.model.Target; @@ -57,7 +60,8 @@ public class DdiRootControllerTest extends AbstractRestIntegrationTestWithMongoD @Test @Description("Ensures that targets cannot be created e.g. in plug'n play scenarios when tenant does not exists but can be created if the tenant exists.") - @WithUser(tenantId = "tenantDoesNotExists", allSpPermissions = true, authorities = "ROLE_CONTROLLER", autoCreateTenant = false) + @WithUser(tenantId = "tenantDoesNotExists", allSpPermissions = true, authorities = { CONTROLLER_ROLE, + SYSTEM_ROLE }, autoCreateTenant = false) public void targetCannotBeRegisteredIfTenantDoesNotExistsButWhenExists() throws Exception { mvc.perform(get("/default-tenant/", tenantAware.getCurrentTenant())).andDo(MockMvcResultPrinter.print()) @@ -91,13 +95,11 @@ public class DdiRootControllerTest extends AbstractRestIntegrationTestWithMongoD // make a poll, audit information should not be changed, run as // controller principal! - securityRule.runAs( - WithSpringAuthorityRule.withUser("controller", SpringEvalExpressions.CONTROLLER_ROLE_ANONYMOUS), () -> { - mvc.perform( - get("/{tenant}/controller/v1/" + knownTargetControllerId, tenantAware.getCurrentTenant())) - .andDo(MockMvcResultPrinter.print()).andExpect(status().isOk()); - return null; - }); + securityRule.runAs(WithSpringAuthorityRule.withUser("controller", CONTROLLER_ROLE_ANONYMOUS), () -> { + mvc.perform(get("/{tenant}/controller/v1/" + knownTargetControllerId, tenantAware.getCurrentTenant())) + .andDo(MockMvcResultPrinter.print()).andExpect(status().isOk()); + return null; + }); // verify that audit information has not changed final Target targetVerify = targetManagement.findTargetByControllerID(knownTargetControllerId); @@ -143,22 +145,19 @@ public class DdiRootControllerTest extends AbstractRestIntegrationTestWithMongoD @Description("Ensures that tenant specific polling time, which is saved in the db, is delivered to the controller.") @WithUser(principal = "knownpricipal", allSpPermissions = false) public void pollWithModifiedGloablPollingTime() throws Exception { - securityRule.runAs( - WithSpringAuthorityRule.withUser("tenantadmin", SpringEvalExpressions.HAS_AUTH_TENANT_CONFIGURATION), - () -> { - tenantConfigurationManagement.addOrUpdateConfiguration(TenantConfigurationKey.POLLING_TIME_INTERVAL, - "00:02:00"); - return null; - }); + securityRule.runAs(WithSpringAuthorityRule.withUser("tenantadmin", HAS_AUTH_TENANT_CONFIGURATION), () -> { + tenantConfigurationManagement.addOrUpdateConfiguration(TenantConfigurationKey.POLLING_TIME_INTERVAL, + "00:02:00"); + return null; + }); - securityRule.runAs( - WithSpringAuthorityRule.withUser("controller", SpringEvalExpressions.CONTROLLER_ROLE_ANONYMOUS), () -> { - mvc.perform(get("/{tenant}/controller/v1/4711", tenantAware.getCurrentTenant())) - .andDo(MockMvcResultPrinter.print()).andExpect(status().isOk()) - .andExpect(content().contentType(MediaTypes.HAL_JSON)) - .andExpect(jsonPath("$config.polling.sleep", equalTo("00:02:00"))); - return null; - }); + securityRule.runAs(WithSpringAuthorityRule.withUser("controller", CONTROLLER_ROLE_ANONYMOUS), () -> { + mvc.perform(get("/{tenant}/controller/v1/4711", tenantAware.getCurrentTenant())) + .andDo(MockMvcResultPrinter.print()).andExpect(status().isOk()) + .andExpect(content().contentType(MediaTypes.HAL_JSON)) + .andExpect(jsonPath("$config.polling.sleep", equalTo("00:02:00"))); + return null; + }); } @Test diff --git a/hawkbit-mgmt-resource/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtDistributionSetResource.java b/hawkbit-mgmt-resource/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtDistributionSetResource.java index e99a33c5f..721995d88 100644 --- a/hawkbit-mgmt-resource/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtDistributionSetResource.java +++ b/hawkbit-mgmt-resource/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtDistributionSetResource.java @@ -39,7 +39,7 @@ import org.eclipse.hawkbit.repository.model.DistributionSetMetadata; import org.eclipse.hawkbit.repository.model.SoftwareModule; import org.eclipse.hawkbit.repository.model.Target; import org.eclipse.hawkbit.repository.model.TargetWithActionType; -import org.eclipse.hawkbit.tenancy.TenantAware; +import org.eclipse.hawkbit.security.SystemSecurityContext; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; @@ -72,15 +72,15 @@ public class MgmtDistributionSetResource implements MgmtDistributionSetRestApi { @Autowired private SystemManagement systemManagement; - @Autowired - private TenantAware currentTenant; - @Autowired private EntityFactory entityFactory; @Autowired private DistributionSetManagement distributionSetManagement; + @Autowired + private SystemSecurityContext systemSecurityContext; + @Override public ResponseEntity> getDistributionSets( @RequestParam(value = MgmtRestConstants.REQUEST_PARAMETER_PAGING_OFFSET, defaultValue = MgmtRestConstants.REQUEST_PARAMETER_PAGING_DEFAULT_OFFSET) final int pagingOffsetParam, @@ -119,8 +119,9 @@ public class MgmtDistributionSetResource implements MgmtDistributionSetRestApi { LOG.debug("creating {} distribution sets", sets.size()); // set default Ds type if ds type is null - sets.stream().filter(ds -> ds.getType() == null).forEach(ds -> ds.setType(this.systemManagement - .getTenantMetadata(this.currentTenant.getCurrentTenant()).getDefaultDsType().getKey())); + final String defaultDsKey = systemSecurityContext + .runAsSystem(() -> this.systemManagement.getTenantMetadata().getDefaultDsType().getKey()); + sets.stream().filter(ds -> ds.getType() == null).forEach(ds -> ds.setType(defaultDsKey)); final Iterable createdDSets = this.distributionSetManagement .createDistributionSets(MgmtDistributionSetMapper.dsFromRequest(sets, this.softwareManagement, diff --git a/hawkbit-repository/hawkbit-repository-api/pom.xml b/hawkbit-repository/hawkbit-repository-api/pom.xml index 050a9a070..e809ce29d 100644 --- a/hawkbit-repository/hawkbit-repository-api/pom.xml +++ b/hawkbit-repository/hawkbit-repository-api/pom.xml @@ -9,16 +9,16 @@ --> - 4.0.0 - - org.eclipse.hawkbit - hawkbit-repository - 0.2.0-SNAPSHOT - - hawkbit-repository-api - hawkBit :: Repository API - - + 4.0.0 + + org.eclipse.hawkbit + hawkbit-repository + 0.2.0-SNAPSHOT + + hawkbit-repository-api + hawkBit :: Repository API + + org.eclipse.hawkbit hawkbit-security-core @@ -36,14 +36,33 @@ cz.jirutka.rsql rsql-parser - + org.springframework.hateoas spring-hateoas - - org.springframework.boot - spring-boot-configuration-processor - true - - + + + + org.springframework.boot + spring-boot-configuration-processor + true + + + + + ru.yandex.qatools.allure + allure-junit-adaptor + test + + + org.easytesting + fest-assert-core + test + + + org.easytesting + fest-assert + test + + \ No newline at end of file diff --git a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/ControllerManagement.java b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/ControllerManagement.java index e5dcb8f74..f06621fd4 100644 --- a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/ControllerManagement.java +++ b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/ControllerManagement.java @@ -183,6 +183,8 @@ public interface ControllerManagement { * @return the security context of the target, in case no target exists for * the given controllerId {@code null} is returned */ + @PreAuthorize(SpringEvalExpressions.IS_CONTROLLER + SpringEvalExpressions.HAS_AUTH_OR + + SpringEvalExpressions.HAS_AUTH_READ_TARGET_SEC_TOKEN) String getSecurityTokenByControllerId(@NotEmpty String controllerId); /** diff --git a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/SoftwareManagement.java b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/SoftwareManagement.java index 0934e871e..bc3c50004 100644 --- a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/SoftwareManagement.java +++ b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/SoftwareManagement.java @@ -340,6 +340,7 @@ public interface SoftwareManagement { * to search for * @return {@link List} of found {@link SoftwareModule}s */ + @PreAuthorize(SpringEvalExpressions.HAS_AUTH_READ_REPOSITORY) List findSoftwareModulesById(@NotEmpty Collection ids); /** diff --git a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/SystemManagement.java b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/SystemManagement.java index a44b13d92..cd414b09a 100644 --- a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/SystemManagement.java +++ b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/SystemManagement.java @@ -61,6 +61,9 @@ public interface SystemManagement { /** * @return {@link TenantMetaData} of {@link TenantAware#getCurrentTenant()} */ + @PreAuthorize(SpringEvalExpressions.HAS_AUTH_READ_REPOSITORY + SpringEvalExpressions.HAS_AUTH_OR + + SpringEvalExpressions.HAS_AUTH_READ_TARGET + SpringEvalExpressions.HAS_AUTH_OR + + SpringEvalExpressions.HAS_AUTH_TENANT_CONFIGURATION) TenantMetaData getTenantMetadata(); /** @@ -77,6 +80,7 @@ public interface SystemManagement { * to retrieve data for * @return {@link TenantMetaData} of given tenant */ + @PreAuthorize(SpringEvalExpressions.IS_SYSTEM_CODE) TenantMetaData getTenantMetadata(@NotNull String tenant); /** @@ -86,6 +90,7 @@ public interface SystemManagement { * to update * @return updated {@link TenantMetaData} entity */ + @PreAuthorize(SpringEvalExpressions.HAS_AUTH_TENANT_CONFIGURATION) TenantMetaData updateTenantMetadata(@NotNull TenantMetaData metaData); -} \ No newline at end of file +} diff --git a/hawkbit-repository/hawkbit-repository-api/src/test/java/org/eclipse/hawkbit/repository/RepositoryManagementMethodPreAuthorizeAnnotatedTest.java b/hawkbit-repository/hawkbit-repository-api/src/test/java/org/eclipse/hawkbit/repository/RepositoryManagementMethodPreAuthorizeAnnotatedTest.java new file mode 100644 index 000000000..1a0d85d30 --- /dev/null +++ b/hawkbit-repository/hawkbit-repository-api/src/test/java/org/eclipse/hawkbit/repository/RepositoryManagementMethodPreAuthorizeAnnotatedTest.java @@ -0,0 +1,97 @@ +/** + * Copyright (c) 2015 Bosch Software Innovations GmbH and others. + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + */ +package org.eclipse.hawkbit.repository; + +import static org.fest.assertions.Assertions.assertThat; + +import java.io.IOException; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.net.URISyntaxException; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.regex.Pattern; +import java.util.stream.Collectors; + +import org.junit.Test; +import org.springframework.security.access.prepost.PreAuthorize; + +import com.google.common.reflect.ClassPath; + +import ru.yandex.qatools.allure.annotations.Description; +import ru.yandex.qatools.allure.annotations.Features; +import ru.yandex.qatools.allure.annotations.Stories; + +@Features("Unit Tests - Repository") +@Stories("Security Test") +public class RepositoryManagementMethodPreAuthorizeAnnotatedTest { + + private static final Set METHOD_SECURITY_EXCLUSION = new HashSet<>(); + + static { + METHOD_SECURITY_EXCLUSION.add(getMethod(SystemManagement.class, "currentTenant")); + } + + @Test + @Description("Verfies that repository methods are @PreAuthorize annotated") + public void repositoryManagementMethodsArePreAuthorizedAnnotated() + throws ClassNotFoundException, URISyntaxException, IOException { + final List> findInterfacesInPackage = findInterfacesInPackage(getClass().getPackage(), + Pattern.compile(".*Management")); + + assertThat(findInterfacesInPackage).isNotEmpty(); + for (final Class interfaceToCheck : findInterfacesInPackage) { + assertDeclaredMethodsContainsPreAuthorizeAnnotaions(interfaceToCheck); + } + + // all exclusion should be used, otherwise the method exlusion should be + // cleaned up again + assertThat(METHOD_SECURITY_EXCLUSION).isEmpty(); + } + + /** + * asserts that the given methods are annotated with the + * {@link PreAuthorize} annotation for security. Inherited methods are not + * checked. The following methods are excluded due inherited from + * {@link Object}, like equals() or toString(). + * + * @param clazz + * the class to retrieve the public declared methods + */ + private static void assertDeclaredMethodsContainsPreAuthorizeAnnotaions(final Class clazz) { + final Method[] declaredMethods = clazz.getDeclaredMethods(); + for (final Method method : declaredMethods) { + final boolean methodExcluded = METHOD_SECURITY_EXCLUSION.contains(method); + if (methodExcluded || method.isSynthetic() || Modifier.isPublic(method.getModifiers())) { + // skip method because it should be excluded + METHOD_SECURITY_EXCLUSION.remove(method); + continue; + } + final PreAuthorize annotation = method.getAnnotation(PreAuthorize.class); + assertThat(annotation).as("The public method " + method.getName() + " in class " + clazz.getName() + + " is not annoated with @PreAuthorize, security leak?").isNotNull(); + } + } + + private List> findInterfacesInPackage(final Package p, final Pattern includeFilter) + throws URISyntaxException, IOException, ClassNotFoundException { + return ClassPath.from(Thread.currentThread().getContextClassLoader()).getTopLevelClasses(p.getName()).stream() + .filter(clazzInfo -> includeFilter.matcher(clazzInfo.getSimpleName()).matches()) + .map(clazzInfo -> clazzInfo.load()).filter(clazz -> clazz.isInterface()).collect(Collectors.toList()); + } + + private static Method getMethod(final Class clazz, final String methodName, final Class... parameterTypes) { + try { + return clazz.getMethod(methodName, parameterTypes); + } catch (NoSuchMethodException | SecurityException e) { + throw new RuntimeException(e.getMessage(), e); + } + } +} diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/MethodSecurityUtil.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/MethodSecurityUtil.java deleted file mode 100644 index 032289e87..000000000 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/MethodSecurityUtil.java +++ /dev/null @@ -1,63 +0,0 @@ -/** - * Copyright (c) 2015 Bosch Software Innovations GmbH and others. - * - * All rights reserved. This program and the accompanying materials - * are made available under the terms of the Eclipse Public License v1.0 - * which accompanies this distribution, and is available at - * http://www.eclipse.org/legal/epl-v10.html - */ -package org.eclipse.hawkbit.repository.jpa; - -import static org.fest.assertions.Assertions.assertThat; - -import java.lang.reflect.Method; -import java.lang.reflect.Modifier; -import java.util.HashSet; -import java.util.Set; - -import org.springframework.security.access.prepost.PreAuthorize; - -public class MethodSecurityUtil { - - private static final Set METHOD_SECURITY_EXCLUSION = new HashSet<>(); - - static { - METHOD_SECURITY_EXCLUSION.add("equals"); - METHOD_SECURITY_EXCLUSION.add("toString"); - METHOD_SECURITY_EXCLUSION.add("hashCode"); - METHOD_SECURITY_EXCLUSION.add("clone"); - METHOD_SECURITY_EXCLUSION.add("setEnvironment"); - // this method shouldn't be public on the DeploymentManagemeht but it is - METHOD_SECURITY_EXCLUSION.add("setOverrideObsoleteUpdateActions"); - METHOD_SECURITY_EXCLUSION.add("isOverrideObsoleteUpdateActions"); - // this method must be public accessible without security because it's - // necessary to acccess - // the security-token of a target without being authenticated because - // the security-token is - // the authentication process - // ControllerManagement#getSecurityTokenByControllerId() - METHOD_SECURITY_EXCLUSION.add("getSecurityTokenByControllerId"); - } - - /** - * asserts that the given methods are annotated with the - * {@link PreAuthorize} annotation for security. Inherited methods are not - * checked. The following methods are excluded due inherited from - * {@link Object}, like equals() or toString(). - * - * @param clazz - * the class to retrieve the public declared methods - */ - public static void assertDeclaredMethodsContainsPreAuthorizeAnnotaions(final Class clazz) { - final Method[] declaredMethods = clazz.getDeclaredMethods(); - for (final Method method : declaredMethods) { - if (!METHOD_SECURITY_EXCLUSION.contains(method.getName()) && !method.isSynthetic() - && Modifier.isPublic(method.getModifiers())) { - final PreAuthorize annotation = method.getAnnotation(PreAuthorize.class); - assertThat(annotation).as( - "The public method " + method.getName() + " is not annoated with @PreAuthorize, security leak?") - .isNotNull(); - } - } - } -} diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/SystemManagementTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/SystemManagementTest.java index a9f004a73..ac2b30d23 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/SystemManagementTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/SystemManagementTest.java @@ -14,10 +14,10 @@ import java.io.ByteArrayInputStream; import java.util.List; import java.util.Random; +import org.eclipse.hawkbit.im.authentication.SpPermission.SpringEvalExpressions; import org.eclipse.hawkbit.repository.jpa.model.JpaSoftwareModule; import org.eclipse.hawkbit.repository.model.DistributionSet; import org.eclipse.hawkbit.repository.model.Target; -import org.eclipse.hawkbit.repository.model.TenantMetaData; import org.eclipse.hawkbit.repository.report.model.TenantUsage; import org.eclipse.hawkbit.repository.test.util.WithSpringAuthorityRule; import org.junit.Test; @@ -30,15 +30,6 @@ import ru.yandex.qatools.allure.annotations.Stories; @Stories("System Management") public class SystemManagementTest extends AbstractJpaIntegrationTestWithMongoDB { - @Test - @Description("Ensures that you can create a tenant without setting the necessary security context which holds a current tenant") - public void createInitialTenantWithoutSecurityContext() { - securityRule.clear(); - final String tenantToBeCreated = "newTenantToCreate"; - final TenantMetaData tenantMetadata = systemManagement.getTenantMetadata(tenantToBeCreated); - assertThat(tenantMetadata).isNotNull(); - } - @Test @Description("Ensures that findTenants returns all tenants and not only restricted to the tenant which currently is logged in") public void findTenantsReturnsAllTenantsNotOnlyWhichLoggedIn() throws Exception { @@ -109,26 +100,27 @@ public class SystemManagementTest extends AbstractJpaIntegrationTestWithMongoDB for (int i = 0; i < tenants; i++) { final String tenantname = "tenant" + i; - securityRule.runAs(WithSpringAuthorityRule.withUserAndTenant("bumlux", tenantname), () -> { - systemManagement.getTenantMetadata(tenantname); - if (artifactSize > 0) { - createTestArtifact(random); - createDeletedTestArtifact(random); - } - if (targets > 0) { - final List createdTargets = createTestTargets(targets); - if (updates > 0) { - for (int x = 0; x < updates; x++) { - final DistributionSet ds = testdataFactory.createDistributionSet("to be deployed" + x, - true); - - deploymentManagement.assignDistributionSet(ds, createdTargets); + securityRule.runAs(WithSpringAuthorityRule.withUserAndTenant("bumlux", tenantname, true, true, + SpringEvalExpressions.SYSTEM_ROLE), () -> { + systemManagement.getTenantMetadata(tenantname); + if (artifactSize > 0) { + createTestArtifact(random); + createDeletedTestArtifact(random); } - } - } + if (targets > 0) { + final List createdTargets = createTestTargets(targets); + if (updates > 0) { + for (int x = 0; x < updates; x++) { + final DistributionSet ds = testdataFactory + .createDistributionSet("to be deployed" + x, true); - return null; - }); + deploymentManagement.assignDistributionSet(ds, createdTargets); + } + } + } + + return null; + }); } return random; 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 650a16c96..fe8e40da0 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 @@ -8,6 +8,9 @@ */ package org.eclipse.hawkbit.repository.test.util; +import static org.eclipse.hawkbit.im.authentication.SpPermission.SpringEvalExpressions.CONTROLLER_ROLE; +import static org.eclipse.hawkbit.im.authentication.SpPermission.SpringEvalExpressions.SYSTEM_ROLE; + import org.eclipse.hawkbit.ExcludePathAwareShallowETagFilter; import org.eclipse.hawkbit.repository.ArtifactManagement; import org.eclipse.hawkbit.repository.ControllerManagement; @@ -58,7 +61,7 @@ import org.springframework.web.context.WebApplicationContext; @RunWith(SpringJUnit4ClassRunner.class) @WebAppConfiguration @ActiveProfiles({ "test" }) -@WithUser(principal = "bumlux", allSpPermissions = true, authorities = "ROLE_CONTROLLER") +@WithUser(principal = "bumlux", allSpPermissions = true, authorities = { CONTROLLER_ROLE, SYSTEM_ROLE }) @SpringApplicationConfiguration(classes = { TestConfiguration.class }) // destroy the context after each test class because otherwise we get problem // when context is diff --git a/hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/util/WithSpringAuthorityRule.java b/hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/util/WithSpringAuthorityRule.java index 7c588febe..ce5c39843 100644 --- a/hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/util/WithSpringAuthorityRule.java +++ b/hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/util/WithSpringAuthorityRule.java @@ -52,10 +52,10 @@ public class WithSpringAuthorityRule implements TestRule { annotation = description.getTestClass().getAnnotation(WithUser.class); } if (annotation != null) { - setSecurityContext(annotation); if (annotation.autoCreateTenant()) { - SystemManagementHolder.getInstance().getSystemManagement().getTenantMetadata(annotation.tenantId()); + createTenant(annotation.tenantId()); } + setSecurityContext(annotation); } return oldContext; } @@ -139,7 +139,7 @@ public class WithSpringAuthorityRule implements TestRule { } /** - * + * * @param withUser * @param callable * @return callable result @@ -149,7 +149,7 @@ public class WithSpringAuthorityRule implements TestRule { final SecurityContext oldContext = SecurityContextHolder.getContext(); setSecurityContext(withUser); if (withUser.autoCreateTenant()) { - SystemManagementHolder.getInstance().getSystemManagement().getTenantMetadata(withUser.tenantId()); + createTenant(withUser.tenantId()); } try { return callable.call(); @@ -158,6 +158,16 @@ public class WithSpringAuthorityRule implements TestRule { } } + private void createTenant(final String tenantId) { + final SecurityContext oldContext = SecurityContextHolder.getContext(); + setSecurityContext(privilegedUser()); + try { + SystemManagementHolder.getInstance().getSystemManagement().getTenantMetadata(tenantId); + } finally { + after(oldContext); + } + } + public static WithUser withUser(final String principal, final String... authorities) { return withUserAndTenant(principal, "default", true, true, authorities); } @@ -180,7 +190,7 @@ public class WithSpringAuthorityRule implements TestRule { } private static WithUser privilegedUser() { - return createWithUser("bumlux", "default", true, true, new String[] { "ROLE_CONTROLLER" }); + return createWithUser("bumlux", "default", true, true, new String[] { "ROLE_CONTROLLER", "ROLE_SYSTEM_CODE" }); } private static WithUser createWithUser(final String principal, final String tenant, final boolean autoCreateTenant, diff --git a/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/im/authentication/SpPermission.java b/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/im/authentication/SpPermission.java index 52935a0cb..866ee16bc 100644 --- a/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/im/authentication/SpPermission.java +++ b/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/im/authentication/SpPermission.java @@ -294,6 +294,14 @@ public final class SpPermission { public static final String HAS_AUTH_READ_TARGET = HAS_AUTH_PREFIX + READ_TARGET + HAS_AUTH_SUFFIX + HAS_AUTH_OR + IS_SYSTEM_CODE; + /** + * Spring security eval hasAuthority expression to check if spring + * context contains {@link SpPermission#READ_TARGET_SEC_TOKEN} or + * {@link #IS_SYSTEM_CODE}. + */ + public static final String HAS_AUTH_READ_TARGET_SEC_TOKEN = HAS_AUTH_PREFIX + READ_TARGET_SEC_TOKEN + + HAS_AUTH_SUFFIX + HAS_AUTH_OR + IS_SYSTEM_CODE; + /** * Spring security eval hasAuthority expression to check if spring * context contains {@link SpPermission#CREATE_TARGET} or diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/common/detailslayout/SoftwareModuleDetailsTable.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/common/detailslayout/SoftwareModuleDetailsTable.java index 1af6aabe1..a7adb8711 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/common/detailslayout/SoftwareModuleDetailsTable.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/common/detailslayout/SoftwareModuleDetailsTable.java @@ -186,12 +186,17 @@ public class SoftwareModuleDetailsTable extends Table { private void unassignSW(final ClickEvent event, final DistributionSet distributionSet, final Set alreadyAssignedSwModules) { final SoftwareModule unAssignedSw = getSoftwareModule(event.getButton().getId(), alreadyAssignedSwModules); - final DistributionSet newDistributionSet = distributionSetManagement.unassignSoftwareModule(distributionSet, - unAssignedSw); - manageDistUIState.setLastSelectedEntity(DistributionSetIdName.generate(newDistributionSet)); - eventBus.publish(this, new DistributionTableEvent(BaseEntityEventType.SELECTED_ENTITY, newDistributionSet)); - eventBus.publish(this, DistributionsUIEvent.ORDER_BY_DISTRIBUTION); - uiNotification.displaySuccess(i18n.get("message.sw.unassigned", unAssignedSw.getName())); + if (distributionSetManagement.isDistributionSetInUse(distributionSet)) { + uiNotification.displayValidationError(i18n.get("message.error.notification.ds.target.assigned",distributionSet.getName(), distributionSet.getVersion())); + } else { + final DistributionSet newDistributionSet = distributionSetManagement.unassignSoftwareModule(distributionSet, + unAssignedSw); + manageDistUIState.setLastSelectedEntity(DistributionSetIdName.generate(newDistributionSet)); + eventBus.publish(this, new DistributionTableEvent(BaseEntityEventType.SELECTED_ENTITY, newDistributionSet)); + eventBus.publish(this, DistributionsUIEvent.ORDER_BY_DISTRIBUTION); + uiNotification.displaySuccess(i18n.get("message.sw.unassigned", unAssignedSw.getName())); + } + } private static boolean isSoftModAvaiableForSoftType(final Set swModulesSet, diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/common/tagdetails/AbstractTagToken.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/common/tagdetails/AbstractTagToken.java index 2160bdac2..e51f49200 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/common/tagdetails/AbstractTagToken.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/common/tagdetails/AbstractTagToken.java @@ -12,6 +12,7 @@ import java.io.Serializable; import java.util.HashMap; import java.util.Map; import java.util.Optional; +import java.util.concurrent.ConcurrentHashMap; import javax.annotation.PostConstruct; import javax.annotation.PreDestroy; @@ -57,7 +58,7 @@ public abstract class AbstractTagToken implements Serializ protected IndexedContainer container; - protected final transient Map tagDetails = new HashMap<>(); + protected final transient Map tagDetails = new ConcurrentHashMap<>(); protected final transient Map tokensAdded = new HashMap<>(); @@ -146,10 +147,12 @@ public abstract class AbstractTagToken implements Serializ } protected void setContainerPropertValues(final Long tagId, final String tagName, final String tagColor) { - tagDetails.put(tagId, new TagData(tagId, tagName, tagColor)); - final Item item = container.addItem(tagId); - item.getItemProperty("id").setValue(tagId); - updateItem(tagName, tagColor, item); + final TagData tagData = tagDetails.putIfAbsent(tagId, new TagData(tagId, tagName, tagColor)); + if (tagData == null) { + final Item item = container.addItem(tagId); + item.getItemProperty("id").setValue(tagId); + updateItem(tagName, tagColor, item); + } } protected void updateItem(final String tagName, final String tagColor, final Item item) { diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/common/tagdetails/DistributionTagToken.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/common/tagdetails/DistributionTagToken.java index 3b044be2c..13e8eda9c 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/common/tagdetails/DistributionTagToken.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/common/tagdetails/DistributionTagToken.java @@ -108,6 +108,7 @@ public class DistributionTagToken extends AbstractTagToken { @Override protected void populateContainer() { container.removeAllItems(); + tagDetails.clear(); for (final DistributionSetTag tag : tagManagement.findAllDistributionSetTags()) { setContainerPropertValues(tag.getId(), tag.getName(), tag.getColour()); } diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/common/tagdetails/TargetTagToken.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/common/tagdetails/TargetTagToken.java index 34b931c1b..2b4a853d0 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/common/tagdetails/TargetTagToken.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/common/tagdetails/TargetTagToken.java @@ -103,6 +103,7 @@ public class TargetTagToken extends AbstractTargetTagToken { @Override protected void populateContainer() { container.removeAllItems(); + tagDetails.clear(); for (final TargetTag tag : tagManagement.findAllTargetTags()) { setContainerPropertValues(tag.getId(), tag.getName(), tag.getColour()); } diff --git a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/distributions/dstable/DistributionSetTable.java b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/distributions/dstable/DistributionSetTable.java index 1547ef11b..fc415f4de 100644 --- a/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/distributions/dstable/DistributionSetTable.java +++ b/hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/distributions/dstable/DistributionSetTable.java @@ -334,9 +334,7 @@ public class DistributionSetTable extends AbstractNamedVersionTable