Merge branch 'master' into Feature_Improve_Code_Quality

Signed-off-by: Dominic Schabel <dominic.schabel@bosch-si.com>

# Conflicts:
#	hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/util/WithSpringAuthorityRule.java
This commit is contained in:
Dominic Schabel
2016-08-08 16:40:00 +02:00
21 changed files with 274 additions and 179 deletions

View File

@@ -9,16 +9,16 @@
-->
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
<parent>
<groupId>org.eclipse.hawkbit</groupId>
<artifactId>hawkbit-repository</artifactId>
<version>0.2.0-SNAPSHOT</version>
</parent>
<artifactId>hawkbit-repository-api</artifactId>
<name>hawkBit :: Repository API</name>
<dependencies>
<modelVersion>4.0.0</modelVersion>
<parent>
<groupId>org.eclipse.hawkbit</groupId>
<artifactId>hawkbit-repository</artifactId>
<version>0.2.0-SNAPSHOT</version>
</parent>
<artifactId>hawkbit-repository-api</artifactId>
<name>hawkBit :: Repository API</name>
<dependencies>
<dependency>
<groupId>org.eclipse.hawkbit</groupId>
<artifactId>hawkbit-security-core</artifactId>
@@ -36,14 +36,33 @@
<groupId>cz.jirutka.rsql</groupId>
<artifactId>rsql-parser</artifactId>
</dependency>
<dependency>
<dependency>
<groupId>org.springframework.hateoas</groupId>
<artifactId>spring-hateoas</artifactId>
</dependency>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-configuration-processor</artifactId>
<optional>true</optional>
</dependency>
</dependencies>
<!-- Optional -->
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-configuration-processor</artifactId>
<optional>true</optional>
</dependency>
<!-- TEST -->
<dependency>
<groupId>ru.yandex.qatools.allure</groupId>
<artifactId>allure-junit-adaptor</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.easytesting</groupId>
<artifactId>fest-assert-core</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.easytesting</groupId>
<artifactId>fest-assert</artifactId>
<scope>test</scope>
</dependency>
</dependencies>
</project>

View File

@@ -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);
/**

View File

@@ -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<SoftwareModule> findSoftwareModulesById(@NotEmpty Collection<Long> ids);
/**

View File

@@ -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);
}
}

View File

@@ -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> 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<Class<?>> 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<Class<?>> 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);
}
}
}

View File

@@ -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<String> 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();
}
}
}
}

View File

@@ -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<Target> 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<Target> 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;

View File

@@ -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

View File

@@ -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,