Merge pull request #175 from bsinno/fix_target_security_permission_check_for_system_code

allow the getTargetSecurityToken can be called as system code

hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/DeploymentManagement.java

@@ -54,6 +54,7 @@ import org.eclipse.hawkbit.repository.model.Target;
 import org.eclipse.hawkbit.repository.model.TargetInfo;
 import org.eclipse.hawkbit.repository.model.TargetUpdateStatus;
 import org.eclipse.hawkbit.repository.specifications.TargetSpecifications;
+import org.eclipse.hawkbit.security.SystemSecurityContext;
 import org.hibernate.validator.constraints.NotEmpty;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -118,6 +119,9 @@ public class DeploymentManagement {
     @Autowired
     private AfterTransactionCommitExecutor afterCommit;
 
+    @Autowired
+    private SystemSecurityContext systemSecurityContext;
+
     /**
      * method assigns the {@link DistributionSet} to all {@link Target}s.
      *
@@ -422,11 +426,14 @@ public class DeploymentManagement {
     private void assignDistributionSetEvent(final Target target, final Long actionId,
             final List<SoftwareModule> softwareModules) {
         target.getTargetInfo().setUpdateStatus(TargetUpdateStatus.PENDING);
+        final String targetSecurityToken = systemSecurityContext.runAsSystem(() -> {
+            return target.getSecurityToken();
+        });
         afterCommit.afterCommit(() -> {
             eventBus.post(new TargetInfoUpdateEvent(target.getTargetInfo()));
             eventBus.post(new TargetAssignDistributionSetEvent(target.getOptLockRevision(), target.getTenant(),
                     target.getControllerId(), actionId, softwareModules, target.getTargetInfo().getAddress(),
-                    target.getSecurityToken()));
+                    targetSecurityToken));
         });
     }
 

hawkbit-repository/src/main/java/org/eclipse/hawkbit/repository/model/Target.java

@@ -38,6 +38,7 @@ import javax.validation.constraints.Size;
 import org.eclipse.hawkbit.im.authentication.SpPermission;
 import org.eclipse.hawkbit.repository.model.helper.SecurityChecker;
 import org.eclipse.hawkbit.repository.model.helper.SecurityTokenGeneratorHolder;
+import org.eclipse.hawkbit.repository.model.helper.SystemSecurityContextHolder;
 import org.eclipse.persistence.annotations.CascadeOnDelete;
 import org.springframework.data.domain.Persistable;
 
@@ -193,10 +194,14 @@ public class Target extends NamedEntity implements Persistable<Long> {
     }
 
     /**
-     * @return the securityToken
+     * @return the securityToken if the current security context contains the
+     *         necessary permission {@link SpPermission#READ_TARGET_SEC_TOKEN}
+     *         or the current context is executed as system code, otherwise
+     *         {@code null}.
      */
     public String getSecurityToken() {
-        if (SecurityChecker.hasPermission(SpPermission.READ_TARGET_SEC_TOKEN)) {
+        if (SystemSecurityContextHolder.getInstance().getSystemSecurityContext().isCurrentThreadSystemCode()
+                || SecurityChecker.hasPermission(SpPermission.READ_TARGET_SEC_TOKEN)) {
             return securityToken;
         }
         return null;

hawkbit-repository/src/test/java/org/eclipse/hawkbit/AbstractIntegrationTest.java

@@ -48,6 +48,7 @@ import org.eclipse.hawkbit.repository.model.DistributionSetType;
 import org.eclipse.hawkbit.repository.model.SoftwareModuleType;
 import org.eclipse.hawkbit.repository.utils.RepositoryDataGenerator.DatabaseCleanupUtil;
 import org.eclipse.hawkbit.security.DosFilter;
+import org.eclipse.hawkbit.security.SystemSecurityContext;
 import org.eclipse.hawkbit.tenancy.TenantAware;
 import org.junit.After;
 import org.junit.AfterClass;
@@ -181,11 +182,10 @@ public abstract class AbstractIntegrationTest implements EnvironmentAware {
 
     @Autowired
     protected TenantAwareCacheManager cacheManager;
-    
+
     @Autowired
     protected TenantConfigurationManagement tenantConfigurationManagement;
 
-
     @Autowired
     protected RolloutManagement rolloutManagement;
 
@@ -198,6 +198,9 @@ public abstract class AbstractIntegrationTest implements EnvironmentAware {
     @Autowired
     protected RolloutRepository rolloutRepository;
 
+    @Autowired
+    protected SystemSecurityContext systemSecurityContext;
+
     protected MockMvc mvc;
 
     @Autowired

hawkbit-repository/src/test/java/org/eclipse/hawkbit/WithSpringAuthorityRule.java

@@ -160,19 +160,23 @@ public class WithSpringAuthorityRule implements TestRule {
     }
 
     public static WithUser withUser(final String principal, final String... authorities) {
-        return withUserAndTenant(principal, "default", true, authorities);
+        return withUserAndTenant(principal, "default", true, true, authorities);
+    }
+    
+    public static WithUser withUser(final String principal, final boolean allSpPermision, final String... authorities) {
+        return withUserAndTenant(principal, "default", true, allSpPermision, authorities);
     }
 
     public static WithUser withUser(final boolean autoCreateTenant) {
-        return withUserAndTenant("bumlux", "default", autoCreateTenant, new String[] {});
+        return withUserAndTenant("bumlux", "default", autoCreateTenant, true, new String[] {});
     }
 
     public static WithUser withUserAndTenant(final String principal, final String tenant, final String... authorities) {
-        return withUserAndTenant(principal, tenant, true, new String[] {});
+        return withUserAndTenant(principal, tenant, true, true, new String[] {});
     }
 
     public static WithUser withUserAndTenant(final String principal, final String tenant,
-            final boolean autoCreateTenant, final String... authorities) {
+            final boolean autoCreateTenant, final boolean allSpPermission, final String... authorities) {
         return new WithUser() {
 
             @Override
@@ -197,7 +201,7 @@ public class WithSpringAuthorityRule implements TestRule {
 
             @Override
             public boolean allSpPermissions() {
-                return true;
+                return allSpPermission;
             }
 
             @Override

hawkbit-repository/src/test/java/org/eclipse/hawkbit/repository/TargetManagementTest.java

@@ -32,6 +32,7 @@ import org.eclipse.hawkbit.AbstractIntegrationTest;
 import org.eclipse.hawkbit.TestDataUtil;
 import org.eclipse.hawkbit.WithSpringAuthorityRule;
 import org.eclipse.hawkbit.WithUser;
+import org.eclipse.hawkbit.im.authentication.SpPermission;
 import org.eclipse.hawkbit.repository.exception.EntityAlreadyExistsException;
 import org.eclipse.hawkbit.repository.exception.TenantNotExistException;
 import org.eclipse.hawkbit.repository.model.Action;
@@ -56,6 +57,36 @@ import ru.yandex.qatools.allure.annotations.Stories;
 @Stories("Target Management")
 public class TargetManagementTest extends AbstractIntegrationTest {
 
+    @Test
+    @Description("Ensures that retrieving the target security is only permitted with the necessary permissions.")
+    public void getTargetSecurityTokenOnlyWithCorrectPermission() throws Exception {
+        final Target createdTarget = targetManagement.createTarget(new Target("targetWithSecurityToken"));
+
+        // retrieve security token only with READ_TARGET_SEC_TOKEN permission
+        final String securityTokenWithReadPermission = securityRule.runAs(WithSpringAuthorityRule
+                .withUser("OnlyTargetReadPermission", false, SpPermission.READ_TARGET_SEC_TOKEN.toString()), () -> {
+                    return createdTarget.getSecurityToken();
+                });
+
+        // retrieve security token as system code execution
+        final String securityTokenAsSystemCode = systemSecurityContext.runAsSystem(() -> {
+            return createdTarget.getSecurityToken();
+        });
+
+        // retrieve security token without any permissions
+        final String securityTokenWithoutPermission = securityRule
+                .runAs(WithSpringAuthorityRule.withUser("NoPermission", false), () -> {
+                    return createdTarget.getSecurityToken();
+                });
+
+        assertThat(createdTarget.getSecurityToken()).isNotNull();
+        assertThat(securityTokenWithReadPermission).isNotNull();
+        assertThat(securityTokenAsSystemCode).isNotNull();
+
+        assertThat(securityTokenWithoutPermission).isNull();
+
+    }
+
     @Test
     @Description("Ensures that targets cannot be created e.g. in plug'n play scenarios when tenant does not exists.")
     @WithUser(tenantId = "tenantWhichDoesNotExists", allSpPermissions = true, autoCreateTenant = false)

hawkbit-security-core/src/main/java/org/eclipse/hawkbit/security/SystemSecurityContext.java

@@ -49,6 +49,21 @@ public class SystemSecurityContext {
         this.tenantAware = tenantAware;
     }
 
+    /**
+     * Runs a given {@link Callable} within a system security context, which is
+     * permitted to call secured system code. Often the system needs to call
+     * secured methods by it's own without relying on the current security
+     * context e.g. if the current security context does not contain the
+     * necessary permission it's necessary to execute code as system code to
+     * execute necessary methods and functionality.
+     * 
+     * The security context will be switched to the system code and back after
+     * the callable is called.
+     * 
+     * @param callable
+     *            the callable to call within the system security context
+     * @return the return value of the {@link Callable#call()} method.
+     */
     public <T> T runAsSystem(final Callable<T> callable) {
         final SecurityContext oldContext = SecurityContextHolder.getContext();
         try {
@@ -68,6 +83,14 @@ public class SystemSecurityContext {
         }
     }
 
+    /**
+     * @return {@code true} if the current running code is running as system
+     *         code block.
+     */
+    public boolean isCurrentThreadSystemCode() {
+        return SecurityContextHolder.getContext().getAuthentication() instanceof SystemCodeAuthentication;
+    }
+
     private static void setSystemContext() {
         final SecurityContextImpl securityContextImpl = new SecurityContextImpl();
         securityContextImpl.setAuthentication(new SystemCodeAuthentication());