Remove SYSTEM_ADMIN (#2936)
Not needed. Overlaping with system role. Could be added on top of others if needed Signed-off-by: Avgustin Marinov <Avgustin.Marinov@bosch.com>
This commit is contained in:
@@ -96,9 +96,6 @@ public final class SpPermission {
|
|||||||
/** Permission to start/stop/resume a rollout. */
|
/** Permission to start/stop/resume a rollout. */
|
||||||
public static final String HANDLE_ROLLOUT = "HANDLE_" + 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 = " > ";
|
||||||
public static final String IMPLY_CREATE = IMPLY + CREATE_PREFIX;
|
public static final String IMPLY_CREATE = IMPLY + CREATE_PREFIX;
|
||||||
public static final String IMPLY_READ = IMPLY + READ_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_UPDATE + TENANT_CONFIGURATION + LINE_BREAK +
|
||||||
TENANT_CONFIGURATION + IMPLY_DELETE + TENANT_CONFIGURATION + LINE_BREAK +
|
TENANT_CONFIGURATION + IMPLY_DELETE + TENANT_CONFIGURATION + LINE_BREAK +
|
||||||
TENANT_CONFIGURATION + IMPLY + READ_GATEWAY_SECURITY_TOKEN + LINE_BREAK;
|
TENANT_CONFIGURATION + IMPLY + READ_GATEWAY_SECURITY_TOKEN + LINE_BREAK;
|
||||||
|
|
||||||
// @formatter:on
|
// @formatter:on
|
||||||
private static final SingletonSupplier<Set<String>> ALL_AUTHORITIES = SingletonSupplier.of(() -> getAuthorities(false));
|
|
||||||
private static final SingletonSupplier<Set<String>> ALL_TENANT_AUTHORITIES = SingletonSupplier.of(() -> getAuthorities(true));
|
|
||||||
|
|
||||||
private static Set<String> getAuthorities(final boolean tenant) {
|
private static final SingletonSupplier<Set<String>> ALL_TENANT_AUTHORITIES = SingletonSupplier.of(SpPermission::getAuthorities);
|
||||||
|
|
||||||
|
private static Set<String> getAuthorities() {
|
||||||
final Set<String> allPermissions = new HashSet<>();
|
final Set<String> allPermissions = new HashSet<>();
|
||||||
|
|
||||||
// groups with access, canonical
|
// groups with access, canonical
|
||||||
@@ -155,19 +151,10 @@ public final class SpPermission {
|
|||||||
|
|
||||||
allPermissions.add(TENANT_CONFIGURATION);
|
allPermissions.add(TENANT_CONFIGURATION);
|
||||||
|
|
||||||
if (!tenant) {
|
|
||||||
// system permission, (!) take care with
|
|
||||||
allPermissions.add(SYSTEM_ADMIN);
|
|
||||||
}
|
|
||||||
|
|
||||||
return Collections.unmodifiableSet(allPermissions);
|
return Collections.unmodifiableSet(allPermissions);
|
||||||
}
|
}
|
||||||
|
|
||||||
public static Set<String> getAllAuthorities() {
|
public static Set<String> getAllAuthorities() {
|
||||||
return ALL_AUTHORITIES.get();
|
|
||||||
}
|
|
||||||
|
|
||||||
public static Set<String> getAllTenantAuthorities() {
|
|
||||||
return ALL_TENANT_AUTHORITIES.get();
|
return ALL_TENANT_AUTHORITIES.get();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -78,7 +78,7 @@ public final class SpRole {
|
|||||||
TENANT_ADMIN + IMPLIES + SpPermission.TENANT_CONFIGURATION + LINE_BREAK;
|
TENANT_ADMIN + IMPLIES + SpPermission.TENANT_CONFIGURATION + LINE_BREAK;
|
||||||
public static final String SYSTEM_ROLE_HIERARCHY =
|
public static final String SYSTEM_ROLE_HIERARCHY =
|
||||||
SYSTEM_ROLE + IMPLIES + TENANT_ADMIN + LINE_BREAK +
|
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 =
|
public static final String DEFAULT_ROLE_HIERARCHY =
|
||||||
TARGET_ADMIN_HIERARCHY +
|
TARGET_ADMIN_HIERARCHY +
|
||||||
|
|||||||
@@ -37,7 +37,6 @@ import org.springframework.security.access.prepost.PreAuthorize;
|
|||||||
public final class SpringEvalExpressions {
|
public final class SpringEvalExpressions {
|
||||||
|
|
||||||
public static final String IS_SYSTEM_CODE = "hasAuthority('ROLE_SYSTEM_CODE')";
|
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}";
|
public static final String PERMISSION_GROUP_PLACEHOLDER = "${permissionGroup}";
|
||||||
// evaluated to <permission>_<permissionGroup> (e.g. CREATE_DISTRIBUTION_SET)
|
// evaluated to <permission>_<permissionGroup> (e.g. CREATE_DISTRIBUTION_SET)
|
||||||
|
|||||||
@@ -111,10 +111,8 @@ public class MgmtSecurityConfiguration {
|
|||||||
@Autowired(required = false) @Qualifier("hawkbitHttpSecurityCustomizer") final Customizer<HttpSecurity> httpSecurityCustomizer,
|
@Autowired(required = false) @Qualifier("hawkbitHttpSecurityCustomizer") final Customizer<HttpSecurity> httpSecurityCustomizer,
|
||||||
final SystemManagement systemManagement) throws Exception {
|
final SystemManagement systemManagement) throws Exception {
|
||||||
http
|
http
|
||||||
.securityMatcher(MgmtRestConstants.REST + "/**", "/system/admin/**")
|
.securityMatcher(MgmtRestConstants.REST + "/**")
|
||||||
.authorizeHttpRequests(amrmRegistry -> amrmRegistry
|
.authorizeHttpRequests(amrmRegistry -> amrmRegistry
|
||||||
.requestMatchers("/system/admin/**")
|
|
||||||
.hasAnyAuthority(SpPermission.SYSTEM_ADMIN)
|
|
||||||
.anyRequest()
|
.anyRequest()
|
||||||
.authenticated())
|
.authenticated())
|
||||||
.anonymous(AbstractHttpConfigurer::disable)
|
.anonymous(AbstractHttpConfigurer::disable)
|
||||||
|
|||||||
@@ -264,7 +264,7 @@ public interface ControllerManagement {
|
|||||||
* @return {@link Target} or {@code null} if it does not exist
|
* @return {@link Target} or {@code null} if it does not exist
|
||||||
* @see Target#getControllerId()
|
* @see Target#getControllerId()
|
||||||
*/
|
*/
|
||||||
@PreAuthorize(SpringEvalExpressions.IS_CONTROLLER + " or " + SpringEvalExpressions.IS_SYSTEM_CODE)
|
@PreAuthorize(SpringEvalExpressions.IS_CONTROLLER)
|
||||||
Optional<Target> findByControllerId(@NotEmpty String controllerId);
|
Optional<Target> findByControllerId(@NotEmpty String controllerId);
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -274,7 +274,7 @@ public interface ControllerManagement {
|
|||||||
* @return {@link Target} or {@code null} if it does not exist
|
* @return {@link Target} or {@code null} if it does not exist
|
||||||
* @see Target#getId()
|
* @see Target#getId()
|
||||||
*/
|
*/
|
||||||
@PreAuthorize(SpringEvalExpressions.IS_CONTROLLER + " or " + SpringEvalExpressions.IS_SYSTEM_CODE)
|
@PreAuthorize(SpringEvalExpressions.IS_CONTROLLER)
|
||||||
Optional<Target> find(long targetId);
|
Optional<Target> find(long targetId);
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@@ -403,6 +403,6 @@ public interface DeploymentManagement extends PermissionSupport {
|
|||||||
@PreAuthorize(HAS_UPDATE_REPOSITORY)
|
@PreAuthorize(HAS_UPDATE_REPOSITORY)
|
||||||
void cancelActionsForDistributionSet(final ActionCancellationType cancelationType, final DistributionSet set);
|
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);
|
void handleMaxAssignmentsExceeded(Long targetId, Long requested, AssignmentQuotaExceededException quotaExceededException);
|
||||||
}
|
}
|
||||||
@@ -78,6 +78,6 @@ public interface SystemManagement {
|
|||||||
*
|
*
|
||||||
* @param tenant to delete
|
* @param tenant to delete
|
||||||
*/
|
*/
|
||||||
@PreAuthorize(SpringEvalExpressions.HAS_AUTH_SYSTEM_ADMIN)
|
@PreAuthorize(SpringEvalExpressions.IS_SYSTEM_CODE)
|
||||||
void deleteTenant(@NotNull String tenant);
|
void deleteTenant(@NotNull String tenant);
|
||||||
}
|
}
|
||||||
@@ -25,9 +25,6 @@ public interface TenantStatsManagement {
|
|||||||
*
|
*
|
||||||
* @return collected statistics
|
* @return collected statistics
|
||||||
*/
|
*/
|
||||||
@PreAuthorize(
|
@PreAuthorize("hasAuthority('" + SpRole.TENANT_ADMIN + "')")
|
||||||
"hasAuthority('" + SpRole.TENANT_ADMIN + "')" + " or " +
|
|
||||||
SpringEvalExpressions.HAS_AUTH_SYSTEM_ADMIN + " or " +
|
|
||||||
SpringEvalExpressions.IS_SYSTEM_CODE)
|
|
||||||
TenantUsage getStatsOfTenant();
|
TenantUsage getStatsOfTenant();
|
||||||
}
|
}
|
||||||
@@ -29,14 +29,12 @@ import org.eclipse.hawkbit.repository.qfields.TargetTagFields;
|
|||||||
// utility class to validate authorities when ACM is enabled
|
// utility class to validate authorities when ACM is enabled
|
||||||
@NoArgsConstructor(access = AccessLevel.PRIVATE)
|
@NoArgsConstructor(access = AccessLevel.PRIVATE)
|
||||||
public final class AuthorityChecker {
|
public final class AuthorityChecker {
|
||||||
|
;
|
||||||
|
|
||||||
private static final Set<String> ALL_AUTHORITIES = SpPermission.getAllTenantAuthorities();
|
public static void validateAuthorities(final String... authorities) {
|
||||||
|
|
||||||
public static String[] validateAuthorities(final String... authorities) {
|
|
||||||
for (final String authority : authorities) {
|
for (final String authority : authorities) {
|
||||||
validateAuthority(authority);
|
validateAuthority(authority);
|
||||||
}
|
}
|
||||||
return authorities;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
public static void validateAuthority(final String authority) {
|
public static void validateAuthority(final String authority) {
|
||||||
@@ -45,7 +43,7 @@ public final class AuthorityChecker {
|
|||||||
if (index > 0) {
|
if (index > 0) {
|
||||||
validateScope(group(unscopedPermission), authority.substring(index + 1), authority);
|
validateScope(group(unscopedPermission), authority.substring(index + 1), authority);
|
||||||
}
|
}
|
||||||
if (!ALL_AUTHORITIES.contains(unscopedPermission)) {
|
if (!SpPermission.getAllAuthorities().contains(unscopedPermission)) {
|
||||||
throw new IllegalArgumentException(
|
throw new IllegalArgumentException(
|
||||||
"Unknown permission: " + unscopedPermission + (index > 0 ? " (unscoped of " + authority + ")" : ""));
|
"Unknown permission: " + unscopedPermission + (index > 0 ? " (unscoped of " + authority + ")" : ""));
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -86,30 +86,13 @@ class MultiTenancyEntityTest extends AbstractJpaIntegrationTest {
|
|||||||
assertThat(findTargetsForTenant).hasSize(1);
|
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.
|
* Ensures that tenant metadata is retrieved for the current tenant.
|
||||||
*/
|
*/
|
||||||
@Test
|
@Test
|
||||||
@WithUser(tenantId = "mytenant", autoCreateTenant = false, allSpPermissions = true)
|
@WithUser(tenantId = "mytenant", autoCreateTenant = false, allSpPermissions = true)
|
||||||
void getTenantMetdata() throws Exception {
|
void getTenantMetdata() throws Exception {
|
||||||
// logged in tenant mytenant - check if tenant default data is
|
// logged in tenant mytenant - check if tenant default data is autogenerated
|
||||||
// autogenerated
|
|
||||||
assertThat(distributionSetTypeManagement.findAll(PAGE)).isEmpty();
|
assertThat(distributionSetTypeManagement.findAll(PAGE)).isEmpty();
|
||||||
SecurityContextSwitch.asPrivileged(() ->
|
SecurityContextSwitch.asPrivileged(() ->
|
||||||
assertThat(systemManagement.createTenantMetadata("mytenant").getTenant().toUpperCase()).isEqualTo("mytenant".toUpperCase()));
|
assertThat(systemManagement.createTenantMetadata("mytenant").getTenant().toUpperCase()).isEqualTo("mytenant".toUpperCase()));
|
||||||
|
|||||||
@@ -31,8 +31,6 @@ import org.springframework.test.context.support.AbstractTestExecutionListener;
|
|||||||
@Slf4j
|
@Slf4j
|
||||||
public class CleanupTestExecutionListener extends AbstractTestExecutionListener {
|
public class CleanupTestExecutionListener extends AbstractTestExecutionListener {
|
||||||
|
|
||||||
private static final Pageable PAGE = PageRequest.of(0, 400, Sort.by(Sort.Direction.ASC, "id"));
|
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public void afterTestMethod(@NotNull final TestContext testContext) throws Exception {
|
public void afterTestMethod(@NotNull final TestContext testContext) throws Exception {
|
||||||
SecurityContextSwitch.asPrivileged(() -> {
|
SecurityContextSwitch.asPrivileged(() -> {
|
||||||
|
|||||||
Reference in New Issue
Block a user