From 1640025a25d58bd3505e86712b08d9a3979619fa Mon Sep 17 00:00:00 2001 From: Avgustin Marinov Date: Thu, 7 Mar 2024 18:52:50 +0200 Subject: [PATCH] Apply role hierarchy in hasPermission checks (#1675) Signed-off-by: Marinov Avgustin --- .../security/SecurityAutoConfiguration.java | 5 +- .../repository/jpa/model/JpaTarget.java | 8 +-- .../jpa/model/helper/SecurityChecker.java | 52 ------------------- .../jpa/management/TargetManagementTest.java | 11 ++++ .../repository/test/TestConfiguration.java | 6 ++- .../security/SystemSecurityContext.java | 39 +++++++++++++- 6 files changed, 61 insertions(+), 60 deletions(-) delete mode 100644 hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/model/helper/SecurityChecker.java diff --git a/hawkbit-autoconfigure/src/main/java/org/eclipse/hawkbit/autoconfigure/security/SecurityAutoConfiguration.java b/hawkbit-autoconfigure/src/main/java/org/eclipse/hawkbit/autoconfigure/security/SecurityAutoConfiguration.java index 52d0afd14..d3047664f 100644 --- a/hawkbit-autoconfigure/src/main/java/org/eclipse/hawkbit/autoconfigure/security/SecurityAutoConfiguration.java +++ b/hawkbit-autoconfigure/src/main/java/org/eclipse/hawkbit/autoconfigure/security/SecurityAutoConfiguration.java @@ -127,8 +127,9 @@ public class SecurityAutoConfiguration { */ @Bean @ConditionalOnMissingBean - public SystemSecurityContext systemSecurityContext(final TenantAware tenantAware) { - return new SystemSecurityContext(tenantAware); + public SystemSecurityContext systemSecurityContext( + final TenantAware tenantAware, final RoleHierarchy roleHierarchy) { + return new SystemSecurityContext(tenantAware, roleHierarchy); } /** diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/model/JpaTarget.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/model/JpaTarget.java index b49abca5e..300dee323 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/model/JpaTarget.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/model/JpaTarget.java @@ -51,7 +51,6 @@ import org.eclipse.hawkbit.im.authentication.SpPermission; import org.eclipse.hawkbit.repository.event.remote.TargetDeletedEvent; import org.eclipse.hawkbit.repository.event.remote.entity.TargetCreatedEvent; import org.eclipse.hawkbit.repository.event.remote.entity.TargetUpdatedEvent; -import org.eclipse.hawkbit.repository.jpa.model.helper.SecurityChecker; import org.eclipse.hawkbit.repository.jpa.model.helper.SecurityTokenGeneratorHolder; import org.eclipse.hawkbit.repository.model.Action; import org.eclipse.hawkbit.repository.model.AutoConfirmationStatus; @@ -65,6 +64,7 @@ import org.eclipse.hawkbit.repository.model.TargetUpdateStatus; import org.eclipse.hawkbit.repository.model.helper.EventPublisherHolder; import org.eclipse.hawkbit.repository.model.helper.SystemSecurityContextHolder; import org.eclipse.hawkbit.repository.model.helper.TenantConfigurationManagementHolder; +import org.eclipse.hawkbit.security.SystemSecurityContext; import org.eclipse.hawkbit.tenancy.configuration.DurationHelper; import org.eclipse.hawkbit.tenancy.configuration.TenantConfigurationProperties.TenantConfigurationKey; import org.eclipse.persistence.annotations.CascadeOnDelete; @@ -320,8 +320,10 @@ public class JpaTarget extends AbstractJpaNamedEntity implements Target, EventAw */ @Override public String getSecurityToken() { - if (SystemSecurityContextHolder.getInstance().getSystemSecurityContext().isCurrentThreadSystemCode() - || SecurityChecker.hasPermission(SpPermission.READ_TARGET_SEC_TOKEN)) { + final SystemSecurityContext systemSecurityContext = + SystemSecurityContextHolder.getInstance().getSystemSecurityContext(); + if (systemSecurityContext.isCurrentThreadSystemCode() || + systemSecurityContext.hasPermission(SpPermission.READ_TARGET_SEC_TOKEN)) { return securityToken; } return null; diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/model/helper/SecurityChecker.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/model/helper/SecurityChecker.java deleted file mode 100644 index 3b173c4dd..000000000 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/model/helper/SecurityChecker.java +++ /dev/null @@ -1,52 +0,0 @@ -/** - * Copyright (c) 2015 Bosch Software Innovations GmbH and others - * - * This program and the accompanying materials are made - * available under the terms of the Eclipse Public License 2.0 - * which is available at https://www.eclipse.org/legal/epl-2.0/ - * - * SPDX-License-Identifier: EPL-2.0 - */ -package org.eclipse.hawkbit.repository.jpa.model.helper; - -import org.springframework.security.core.Authentication; -import org.springframework.security.core.GrantedAuthority; -import org.springframework.security.core.context.SecurityContext; -import org.springframework.security.core.context.SecurityContextHolder; - -/** - * A helper class which allows to do runtime security checks. - * - * - * - * - */ -public final class SecurityChecker { - - private SecurityChecker() { - - } - - /** - * Checks the current {@link SecurityContext} for the permission. - * - * @param permission - * the permission to check against the security context - * @return {@code true} if the given permission is present in the security - * context otherwise {@code false} - */ - public static boolean hasPermission(final String permission) { - final SecurityContext context = SecurityContextHolder.getContext(); - if (context != null) { - final Authentication authentication = context.getAuthentication(); - if (authentication != null) { - for (final GrantedAuthority authority : authentication.getAuthorities()) { - if (authority.getAuthority().equals(permission)) { - return true; - } - } - } - } - return false; - } -} diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/TargetManagementTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/TargetManagementTest.java index 71609a077..19a727c31 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/TargetManagementTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/TargetManagementTest.java @@ -30,6 +30,7 @@ import jakarta.validation.ConstraintViolationException; import org.apache.commons.lang3.RandomStringUtils; import org.awaitility.Awaitility; import org.eclipse.hawkbit.im.authentication.SpPermission; +import org.eclipse.hawkbit.im.authentication.SpRole; import org.eclipse.hawkbit.repository.FilterParams; import org.eclipse.hawkbit.repository.Identifiable; import org.eclipse.hawkbit.repository.builder.TargetUpdate; @@ -189,6 +190,14 @@ class TargetManagementTest extends AbstractJpaIntegrationTest { final String securityTokenWithReadPermission = SecurityContextSwitch.runAs( SecurityContextSwitch.withUser("OnlyTargetReadPermission", false, SpPermission.READ_TARGET_SEC_TOKEN), createdTarget::getSecurityToken); + // retrieve security token only with ROLE_TARGET_ADMIN permission + final String securityTokenWithTargetAdminPermission = SecurityContextSwitch.runAs( + SecurityContextSwitch.withUser("OnlyTargetAdminPermission", false, SpRole.TARGET_ADMIN), + createdTarget::getSecurityToken); + // retrieve security token only with ROLE_TENANT_ADMIN permission + final String securityTokenWithTenantAdminPermission = SecurityContextSwitch.runAs( + SecurityContextSwitch.withUser("OnlyTenantAdminPermission", false, SpRole.TENANT_ADMIN), + createdTarget::getSecurityToken); // retrieve security token as system code execution final String securityTokenAsSystemCode = systemSecurityContext.runAsSystem(createdTarget::getSecurityToken); @@ -199,6 +208,8 @@ class TargetManagementTest extends AbstractJpaIntegrationTest { assertThat(createdTarget.getSecurityToken()).isEqualTo("token"); assertThat(securityTokenWithReadPermission).isNotNull(); + assertThat(securityTokenWithTargetAdminPermission).isNotNull(); + assertThat(securityTokenWithTenantAdminPermission).isNotNull(); assertThat(securityTokenAsSystemCode).isNotNull(); assertThat(securityTokenWithoutPermission).isNull(); diff --git a/hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/TestConfiguration.java b/hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/TestConfiguration.java index b29f110ff..c3532f669 100644 --- a/hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/TestConfiguration.java +++ b/hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/TestConfiguration.java @@ -28,6 +28,7 @@ import org.eclipse.hawkbit.cache.DefaultDownloadIdCache; import org.eclipse.hawkbit.cache.DownloadIdCache; import org.eclipse.hawkbit.cache.TenantAwareCacheManager; import org.eclipse.hawkbit.event.BusProtoStuffMessageConverter; +import org.eclipse.hawkbit.im.authentication.SpRole; import org.eclipse.hawkbit.repository.RolloutApprovalStrategy; import org.eclipse.hawkbit.repository.RolloutStatusCache; import org.eclipse.hawkbit.repository.event.ApplicationEventFilter; @@ -67,6 +68,7 @@ import org.springframework.integration.support.locks.DefaultLockRegistry; import org.springframework.integration.support.locks.LockRegistry; import org.springframework.messaging.converter.MessageConverter; import org.springframework.scheduling.annotation.AsyncConfigurer; +import org.springframework.security.access.hierarchicalroles.RoleHierarchyImpl; import org.springframework.security.concurrent.DelegatingSecurityContextExecutorService; import org.springframework.security.concurrent.DelegatingSecurityContextScheduledExecutorService; import org.springframework.security.config.annotation.method.configuration.EnableGlobalMethodSecurity; @@ -106,7 +108,9 @@ public class TestConfiguration implements AsyncConfigurer { @Bean SystemSecurityContext systemSecurityContext(final TenantAware tenantAware) { - return new SystemSecurityContext(tenantAware); + final RoleHierarchyImpl hierarchy = new RoleHierarchyImpl(); + hierarchy.setHierarchy(SpRole.DEFAULT_ROLE_HIERARCHY); + return new SystemSecurityContext(tenantAware, hierarchy); } @Bean diff --git a/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/security/SystemSecurityContext.java b/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/security/SystemSecurityContext.java index ca48dd4f8..bdabd17a3 100644 --- a/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/security/SystemSecurityContext.java +++ b/hawkbit-security-core/src/main/java/org/eclipse/hawkbit/security/SystemSecurityContext.java @@ -12,6 +12,7 @@ package org.eclipse.hawkbit.security; import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.Set; import java.util.UUID; import java.util.concurrent.Callable; @@ -22,6 +23,7 @@ import lombok.extern.slf4j.Slf4j; import org.eclipse.hawkbit.im.authentication.TenantAwareAuthenticationDetails; import org.eclipse.hawkbit.im.authentication.SpPermission.SpringEvalExpressions; import org.eclipse.hawkbit.tenancy.TenantAware; +import org.springframework.security.access.hierarchicalroles.RoleHierarchy; import org.springframework.security.authentication.AnonymousAuthenticationToken; import org.springframework.security.core.Authentication; import org.springframework.security.core.GrantedAuthority; @@ -29,6 +31,7 @@ import org.springframework.security.core.authority.SimpleGrantedAuthority; import org.springframework.security.core.context.SecurityContext; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.core.context.SecurityContextImpl; +import org.springframework.util.ObjectUtils; /** * A Service which provide to run system code. @@ -37,15 +40,26 @@ import org.springframework.security.core.context.SecurityContextImpl; public class SystemSecurityContext { private final TenantAware tenantAware; + private final RoleHierarchy roleHierarchy; /** * Autowired constructor. * - * @param tenantAware - * the tenant aware bean to retrieve the current tenant + * @param tenantAware the tenant aware bean to retrieve the current tenant */ public SystemSecurityContext(final TenantAware tenantAware) { + this(tenantAware, null); + } + + /** + * Autowired constructor. + * + * @param tenantAware the tenant aware bean to retrieve the current tenant + * @param roleHierarchy the roleHierarchy that is applied + */ + public SystemSecurityContext(final TenantAware tenantAware, final RoleHierarchy roleHierarchy) { this.tenantAware = tenantAware; + this.roleHierarchy = roleHierarchy; } /** @@ -160,6 +174,27 @@ public class SystemSecurityContext { return SecurityContextHolder.getContext().getAuthentication() instanceof SystemCodeAuthentication; } + public boolean hasPermission(final String permission) { + final SecurityContext context = SecurityContextHolder.getContext(); + if (context != null) { + final Authentication authentication = context.getAuthentication(); + if (authentication != null) { + Collection grantedAuthorities = authentication.getAuthorities(); + if (!ObjectUtils.isEmpty(grantedAuthorities)) { + if (roleHierarchy != null) { + grantedAuthorities = roleHierarchy.getReachableGrantedAuthorities(grantedAuthorities); + } + for (final GrantedAuthority authority : grantedAuthorities) { + if (authority.getAuthority().equals(permission)) { + return true; + } + } + } + } + } + return false; + } + private void setCustomSecurityContext(final String tenantId, final Object principal, final Collection authorities) { final AnonymousAuthenticationToken authenticationToken = new AnonymousAuthenticationToken(