Add missing PreAuthorize annotation to secure methods and write unit
test Signed-off-by: Michael Hirsch <michael.hirsch@bosch-si.com>
This commit is contained in:
@@ -1,24 +1,16 @@
|
|||||||
<!--
|
<!-- 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 -->
|
||||||
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
|
|
||||||
|
|
||||||
-->
|
|
||||||
<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">
|
<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>
|
<modelVersion>4.0.0</modelVersion>
|
||||||
<parent>
|
<parent>
|
||||||
<groupId>org.eclipse.hawkbit</groupId>
|
<groupId>org.eclipse.hawkbit</groupId>
|
||||||
<artifactId>hawkbit-repository</artifactId>
|
<artifactId>hawkbit-repository</artifactId>
|
||||||
<version>0.2.0-SNAPSHOT</version>
|
<version>0.2.0-SNAPSHOT</version>
|
||||||
</parent>
|
</parent>
|
||||||
<artifactId>hawkbit-repository-api</artifactId>
|
<artifactId>hawkbit-repository-api</artifactId>
|
||||||
<name>hawkBit :: Repository API</name>
|
<name>hawkBit :: Repository API</name>
|
||||||
|
|
||||||
<dependencies>
|
<dependencies>
|
||||||
<dependency>
|
<dependency>
|
||||||
<groupId>org.eclipse.hawkbit</groupId>
|
<groupId>org.eclipse.hawkbit</groupId>
|
||||||
<artifactId>hawkbit-security-core</artifactId>
|
<artifactId>hawkbit-security-core</artifactId>
|
||||||
@@ -36,14 +28,33 @@
|
|||||||
<groupId>cz.jirutka.rsql</groupId>
|
<groupId>cz.jirutka.rsql</groupId>
|
||||||
<artifactId>rsql-parser</artifactId>
|
<artifactId>rsql-parser</artifactId>
|
||||||
</dependency>
|
</dependency>
|
||||||
<dependency>
|
<dependency>
|
||||||
<groupId>org.springframework.hateoas</groupId>
|
<groupId>org.springframework.hateoas</groupId>
|
||||||
<artifactId>spring-hateoas</artifactId>
|
<artifactId>spring-hateoas</artifactId>
|
||||||
</dependency>
|
</dependency>
|
||||||
<dependency>
|
|
||||||
<groupId>org.springframework.boot</groupId>
|
<!-- Optional -->
|
||||||
<artifactId>spring-boot-configuration-processor</artifactId>
|
<dependency>
|
||||||
<optional>true</optional>
|
<groupId>org.springframework.boot</groupId>
|
||||||
</dependency>
|
<artifactId>spring-boot-configuration-processor</artifactId>
|
||||||
</dependencies>
|
<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>
|
</project>
|
||||||
@@ -183,6 +183,8 @@ public interface ControllerManagement {
|
|||||||
* @return the security context of the target, in case no target exists for
|
* @return the security context of the target, in case no target exists for
|
||||||
* the given controllerId {@code null} is returned
|
* 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);
|
String getSecurityTokenByControllerId(@NotEmpty String controllerId);
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@@ -340,6 +340,7 @@ public interface SoftwareManagement {
|
|||||||
* to search for
|
* to search for
|
||||||
* @return {@link List} of found {@link SoftwareModule}s
|
* @return {@link List} of found {@link SoftwareModule}s
|
||||||
*/
|
*/
|
||||||
|
@PreAuthorize(SpringEvalExpressions.HAS_AUTH_READ_REPOSITORY)
|
||||||
List<SoftwareModule> findSoftwareModulesById(@NotEmpty Collection<Long> ids);
|
List<SoftwareModule> findSoftwareModulesById(@NotEmpty Collection<Long> ids);
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@@ -0,0 +1,121 @@
|
|||||||
|
/**
|
||||||
|
* 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.File;
|
||||||
|
import java.io.IOException;
|
||||||
|
import java.lang.reflect.Method;
|
||||||
|
import java.lang.reflect.Modifier;
|
||||||
|
import java.net.URI;
|
||||||
|
import java.net.URISyntaxException;
|
||||||
|
import java.net.URL;
|
||||||
|
import java.util.ArrayList;
|
||||||
|
import java.util.Enumeration;
|
||||||
|
import java.util.HashSet;
|
||||||
|
import java.util.List;
|
||||||
|
import java.util.Set;
|
||||||
|
import java.util.regex.Pattern;
|
||||||
|
|
||||||
|
import org.junit.Test;
|
||||||
|
import org.springframework.security.access.prepost.PreAuthorize;
|
||||||
|
|
||||||
|
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"));
|
||||||
|
for (final Class<?> interfaceToCheck : findInterfacesInPackage) {
|
||||||
|
assertDeclaredMethodsContainsPreAuthorizeAnnotaions(interfaceToCheck);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* 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) {
|
||||||
|
if (!METHOD_SECURITY_EXCLUSION.contains(method) && !method.isSynthetic()
|
||||||
|
&& Modifier.isPublic(method.getModifiers())) {
|
||||||
|
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();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Finds all interfaces in a given packages which matches the given filter.
|
||||||
|
*
|
||||||
|
* @param p
|
||||||
|
* the package to search for interfaces in
|
||||||
|
* @param includeFilter
|
||||||
|
* the pattern which interfaces class names should be included
|
||||||
|
* @return a list of loaded interfaces in a specific package and matches the
|
||||||
|
* given filter
|
||||||
|
* @throws URISyntaxException
|
||||||
|
* @throws IOException
|
||||||
|
* @throws ClassNotFoundException
|
||||||
|
*/
|
||||||
|
private List<Class<?>> findInterfacesInPackage(final Package p, final Pattern includeFilter)
|
||||||
|
throws URISyntaxException, IOException, ClassNotFoundException {
|
||||||
|
final List<Class<?>> interfacesToReturn = new ArrayList<>();
|
||||||
|
final ClassLoader classLoader = Thread.currentThread().getContextClassLoader();
|
||||||
|
final Enumeration<URL> resources = classLoader.getResources(p.getName().replace(".", "/"));
|
||||||
|
while (resources.hasMoreElements()) {
|
||||||
|
final File packageDirectory = new File(new URI(resources.nextElement().toString()).getPath());
|
||||||
|
final File[] filesInPackage = packageDirectory.listFiles();
|
||||||
|
for (final File classFile : filesInPackage) {
|
||||||
|
final String classNameWithExtension = classFile.getName();
|
||||||
|
final int indexOfExtension = classNameWithExtension.indexOf(".class");
|
||||||
|
if (indexOfExtension > 0) {
|
||||||
|
final String classNameWithoutExtension = classNameWithExtension.substring(0, indexOfExtension);
|
||||||
|
if (includeFilter.matcher(classNameWithoutExtension).matches()) {
|
||||||
|
final Class<?> classInPackage = Class.forName(p.getName() + "." + classNameWithoutExtension);
|
||||||
|
if (classInPackage.isInterface()) {
|
||||||
|
interfacesToReturn.add(classInPackage);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return interfacesToReturn;
|
||||||
|
}
|
||||||
|
|
||||||
|
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);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -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();
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
@@ -290,6 +290,14 @@ public final class SpPermission {
|
|||||||
public static final String HAS_AUTH_READ_TARGET = HAS_AUTH_PREFIX + READ_TARGET + HAS_AUTH_SUFFIX + HAS_AUTH_OR
|
public static final String HAS_AUTH_READ_TARGET = HAS_AUTH_PREFIX + READ_TARGET + HAS_AUTH_SUFFIX + HAS_AUTH_OR
|
||||||
+ IS_SYSTEM_CODE;
|
+ 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
|
* Spring security eval hasAuthority expression to check if spring
|
||||||
* context contains {@link SpPermission#CREATE_TARGET} or
|
* context contains {@link SpPermission#CREATE_TARGET} or
|
||||||
|
|||||||
Reference in New Issue
Block a user