From 56da119979303279620bd7b891b9f24934bfb7ee Mon Sep 17 00:00:00 2001 From: Avgustin Marinov Date: Wed, 25 Feb 2026 14:00:25 +0200 Subject: [PATCH] Remove SYSTEM_ADMIN (#2936) Not needed. Overlaping with system role. Could be added on top of others if needed Signed-off-by: Avgustin Marinov --- .../eclipse/hawkbit/auth/SpPermission.java | 19 +++---------------- .../java/org/eclipse/hawkbit/auth/SpRole.java | 2 +- .../hawkbit/auth/SpringEvalExpressions.java | 1 - .../mgmt/MgmtSecurityConfiguration.java | 4 +--- .../repository/ControllerManagement.java | 4 ++-- .../repository/DeploymentManagement.java | 2 +- .../hawkbit/repository/SystemManagement.java | 2 +- .../repository/TenantStatsManagement.java | 5 +---- .../repository/jpa/acm/AuthorityChecker.java | 8 +++----- .../jpa/tenancy/MultiTenancyEntityTest.java | 19 +------------------ .../util/CleanupTestExecutionListener.java | 2 -- 11 files changed, 14 insertions(+), 54 deletions(-) diff --git a/hawkbit-core/src/main/java/org/eclipse/hawkbit/auth/SpPermission.java b/hawkbit-core/src/main/java/org/eclipse/hawkbit/auth/SpPermission.java index 6ead58135..cfff7f782 100644 --- a/hawkbit-core/src/main/java/org/eclipse/hawkbit/auth/SpPermission.java +++ b/hawkbit-core/src/main/java/org/eclipse/hawkbit/auth/SpPermission.java @@ -96,9 +96,6 @@ public final class SpPermission { /** Permission to start/stop/resume a rollout. */ public static final String HANDLE_ROLLOUT = "HANDLE_" + ROLLOUT; - /** Permission to administrate the system on a global, i.e. tenant independent scale. That includes the deletion of tenants. */ - public static final String SYSTEM_ADMIN = "SYSTEM_ADMIN"; - public static final String IMPLY = " > "; public static final String IMPLY_CREATE = IMPLY + CREATE_PREFIX; public static final String IMPLY_READ = IMPLY + READ_PREFIX; @@ -128,12 +125,11 @@ public final class SpPermission { TENANT_CONFIGURATION + IMPLY_UPDATE + TENANT_CONFIGURATION + LINE_BREAK + TENANT_CONFIGURATION + IMPLY_DELETE + TENANT_CONFIGURATION + LINE_BREAK + TENANT_CONFIGURATION + IMPLY + READ_GATEWAY_SECURITY_TOKEN + LINE_BREAK; - // @formatter:on - private static final SingletonSupplier> ALL_AUTHORITIES = SingletonSupplier.of(() -> getAuthorities(false)); - private static final SingletonSupplier> ALL_TENANT_AUTHORITIES = SingletonSupplier.of(() -> getAuthorities(true)); - private static Set getAuthorities(final boolean tenant) { + private static final SingletonSupplier> ALL_TENANT_AUTHORITIES = SingletonSupplier.of(SpPermission::getAuthorities); + + private static Set getAuthorities() { final Set allPermissions = new HashSet<>(); // groups with access, canonical @@ -155,19 +151,10 @@ public final class SpPermission { allPermissions.add(TENANT_CONFIGURATION); - if (!tenant) { - // system permission, (!) take care with - allPermissions.add(SYSTEM_ADMIN); - } - return Collections.unmodifiableSet(allPermissions); } public static Set getAllAuthorities() { - return ALL_AUTHORITIES.get(); - } - - public static Set getAllTenantAuthorities() { return ALL_TENANT_AUTHORITIES.get(); } diff --git a/hawkbit-core/src/main/java/org/eclipse/hawkbit/auth/SpRole.java b/hawkbit-core/src/main/java/org/eclipse/hawkbit/auth/SpRole.java index c9cf04307..a7d8d5693 100644 --- a/hawkbit-core/src/main/java/org/eclipse/hawkbit/auth/SpRole.java +++ b/hawkbit-core/src/main/java/org/eclipse/hawkbit/auth/SpRole.java @@ -78,7 +78,7 @@ public final class SpRole { TENANT_ADMIN + IMPLIES + SpPermission.TENANT_CONFIGURATION + LINE_BREAK; public static final String SYSTEM_ROLE_HIERARCHY = SYSTEM_ROLE + IMPLIES + TENANT_ADMIN + LINE_BREAK + - SYSTEM_ROLE + IMPLIES + SpPermission.SYSTEM_ADMIN + LINE_BREAK; + SYSTEM_ROLE + IMPLIES + CONTROLLER_ROLE + LINE_BREAK; public static final String DEFAULT_ROLE_HIERARCHY = TARGET_ADMIN_HIERARCHY + diff --git a/hawkbit-core/src/main/java/org/eclipse/hawkbit/auth/SpringEvalExpressions.java b/hawkbit-core/src/main/java/org/eclipse/hawkbit/auth/SpringEvalExpressions.java index 14f618702..b2c6578d6 100644 --- a/hawkbit-core/src/main/java/org/eclipse/hawkbit/auth/SpringEvalExpressions.java +++ b/hawkbit-core/src/main/java/org/eclipse/hawkbit/auth/SpringEvalExpressions.java @@ -37,7 +37,6 @@ import org.springframework.security.access.prepost.PreAuthorize; public final class SpringEvalExpressions { public static final String IS_SYSTEM_CODE = "hasAuthority('ROLE_SYSTEM_CODE')"; - public static final String HAS_AUTH_SYSTEM_ADMIN = "hasAuthority('SYSTEM_ADMIN')"; public static final String PERMISSION_GROUP_PLACEHOLDER = "${permissionGroup}"; // evaluated to _ (e.g. CREATE_DISTRIBUTION_SET) diff --git a/hawkbit-mgmt/hawkbit-mgmt-starter/src/main/java/org/eclipse/hawkbit/autoconfigure/mgmt/MgmtSecurityConfiguration.java b/hawkbit-mgmt/hawkbit-mgmt-starter/src/main/java/org/eclipse/hawkbit/autoconfigure/mgmt/MgmtSecurityConfiguration.java index fd4bcb016..8bc9af0af 100644 --- a/hawkbit-mgmt/hawkbit-mgmt-starter/src/main/java/org/eclipse/hawkbit/autoconfigure/mgmt/MgmtSecurityConfiguration.java +++ b/hawkbit-mgmt/hawkbit-mgmt-starter/src/main/java/org/eclipse/hawkbit/autoconfigure/mgmt/MgmtSecurityConfiguration.java @@ -111,10 +111,8 @@ public class MgmtSecurityConfiguration { @Autowired(required = false) @Qualifier("hawkbitHttpSecurityCustomizer") final Customizer httpSecurityCustomizer, final SystemManagement systemManagement) throws Exception { http - .securityMatcher(MgmtRestConstants.REST + "/**", "/system/admin/**") + .securityMatcher(MgmtRestConstants.REST + "/**") .authorizeHttpRequests(amrmRegistry -> amrmRegistry - .requestMatchers("/system/admin/**") - .hasAnyAuthority(SpPermission.SYSTEM_ADMIN) .anyRequest() .authenticated()) .anonymous(AbstractHttpConfigurer::disable) 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 cf25cc349..c6be2fb89 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 @@ -264,7 +264,7 @@ public interface ControllerManagement { * @return {@link Target} or {@code null} if it does not exist * @see Target#getControllerId() */ - @PreAuthorize(SpringEvalExpressions.IS_CONTROLLER + " or " + SpringEvalExpressions.IS_SYSTEM_CODE) + @PreAuthorize(SpringEvalExpressions.IS_CONTROLLER) Optional findByControllerId(@NotEmpty String controllerId); /** @@ -274,7 +274,7 @@ public interface ControllerManagement { * @return {@link Target} or {@code null} if it does not exist * @see Target#getId() */ - @PreAuthorize(SpringEvalExpressions.IS_CONTROLLER + " or " + SpringEvalExpressions.IS_SYSTEM_CODE) + @PreAuthorize(SpringEvalExpressions.IS_CONTROLLER) Optional find(long targetId); /** diff --git a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/DeploymentManagement.java b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/DeploymentManagement.java index bbb99599c..63b2858e3 100644 --- a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/DeploymentManagement.java +++ b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/DeploymentManagement.java @@ -403,6 +403,6 @@ public interface DeploymentManagement extends PermissionSupport { @PreAuthorize(HAS_UPDATE_REPOSITORY) void cancelActionsForDistributionSet(final ActionCancellationType cancelationType, final DistributionSet set); - @PreAuthorize(HAS_UPDATE_REPOSITORY + " or " + SpringEvalExpressions.IS_SYSTEM_CODE) + @PreAuthorize(HAS_UPDATE_REPOSITORY) void handleMaxAssignmentsExceeded(Long targetId, Long requested, AssignmentQuotaExceededException quotaExceededException); } \ No newline at end of file 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 5d48947ea..214eb09d5 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 @@ -78,6 +78,6 @@ public interface SystemManagement { * * @param tenant to delete */ - @PreAuthorize(SpringEvalExpressions.HAS_AUTH_SYSTEM_ADMIN) + @PreAuthorize(SpringEvalExpressions.IS_SYSTEM_CODE) void deleteTenant(@NotNull String tenant); } \ No newline at end of file diff --git a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/TenantStatsManagement.java b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/TenantStatsManagement.java index 97d6e40a6..8dba38498 100644 --- a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/TenantStatsManagement.java +++ b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/TenantStatsManagement.java @@ -25,9 +25,6 @@ public interface TenantStatsManagement { * * @return collected statistics */ - @PreAuthorize( - "hasAuthority('" + SpRole.TENANT_ADMIN + "')" + " or " + - SpringEvalExpressions.HAS_AUTH_SYSTEM_ADMIN + " or " + - SpringEvalExpressions.IS_SYSTEM_CODE) + @PreAuthorize("hasAuthority('" + SpRole.TENANT_ADMIN + "')") TenantUsage getStatsOfTenant(); } \ No newline at end of file diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/acm/AuthorityChecker.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/acm/AuthorityChecker.java index 33e00f415..5fff493e0 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/acm/AuthorityChecker.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/acm/AuthorityChecker.java @@ -29,14 +29,12 @@ import org.eclipse.hawkbit.repository.qfields.TargetTagFields; // utility class to validate authorities when ACM is enabled @NoArgsConstructor(access = AccessLevel.PRIVATE) public final class AuthorityChecker { +; - private static final Set ALL_AUTHORITIES = SpPermission.getAllTenantAuthorities(); - - public static String[] validateAuthorities(final String... authorities) { + public static void validateAuthorities(final String... authorities) { for (final String authority : authorities) { validateAuthority(authority); } - return authorities; } public static void validateAuthority(final String authority) { @@ -45,7 +43,7 @@ public final class AuthorityChecker { if (index > 0) { validateScope(group(unscopedPermission), authority.substring(index + 1), authority); } - if (!ALL_AUTHORITIES.contains(unscopedPermission)) { + if (!SpPermission.getAllAuthorities().contains(unscopedPermission)) { throw new IllegalArgumentException( "Unknown permission: " + unscopedPermission + (index > 0 ? " (unscoped of " + authority + ")" : "")); } diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/tenancy/MultiTenancyEntityTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/tenancy/MultiTenancyEntityTest.java index c9cdbbc2c..93847562c 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/tenancy/MultiTenancyEntityTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/tenancy/MultiTenancyEntityTest.java @@ -86,30 +86,13 @@ class MultiTenancyEntityTest extends AbstractJpaIntegrationTest { assertThat(findTargetsForTenant).hasSize(1); } - /** - * Ensures that tenant with proper permissions can read and delete other tenants. - */ - @Test - @WithUser(tenantId = "mytenant", allSpPermissions = true) - void deleteAnotherTenantPossible() throws Exception { - // create target for another tenant - final String anotherTenant = "anotherTenant"; - final String controllerAnotherTenant = "anotherController"; - createTargetForTenant(controllerAnotherTenant, anotherTenant); - - assertThat(listTenants()).as("Expected number if tenants before deletion is").hasSize(3); - systemManagement.deleteTenant(anotherTenant); - assertThat(listTenants()).as("Expected number if tenants after deletion is").hasSize(2); - } - /** * Ensures that tenant metadata is retrieved for the current tenant. */ @Test @WithUser(tenantId = "mytenant", autoCreateTenant = false, allSpPermissions = true) void getTenantMetdata() throws Exception { - // logged in tenant mytenant - check if tenant default data is - // autogenerated + // logged in tenant mytenant - check if tenant default data is autogenerated assertThat(distributionSetTypeManagement.findAll(PAGE)).isEmpty(); SecurityContextSwitch.asPrivileged(() -> assertThat(systemManagement.createTenantMetadata("mytenant").getTenant().toUpperCase()).isEqualTo("mytenant".toUpperCase())); diff --git a/hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/util/CleanupTestExecutionListener.java b/hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/util/CleanupTestExecutionListener.java index d5a3a80ec..ef0d69bb1 100644 --- a/hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/util/CleanupTestExecutionListener.java +++ b/hawkbit-repository/hawkbit-repository-test/src/main/java/org/eclipse/hawkbit/repository/test/util/CleanupTestExecutionListener.java @@ -31,8 +31,6 @@ import org.springframework.test.context.support.AbstractTestExecutionListener; @Slf4j public class CleanupTestExecutionListener extends AbstractTestExecutionListener { - private static final Pageable PAGE = PageRequest.of(0, 400, Sort.by(Sort.Direction.ASC, "id")); - @Override public void afterTestMethod(@NotNull final TestContext testContext) throws Exception { SecurityContextSwitch.asPrivileged(() -> {