Skip to content

Commit ae3a004

Browse files
avgustinmmavgustinmm
authored
Finalize and polish fine-grained permission (#2660)
* Remove _REPOSITORY_ permissions -> replaced with _SOFTWARE_MODULE_, _SOFTWARE_MODULE_TYPE_, _DISTRIBUTION_SET_, _DISTRIBUTION_SET_TYPE_ permissions * Still kept _ROLE_REPOSITORY_ADMIN_ role granting all repository fine-graned permissions * Added dedicated _TARGET_TYPE_ permission set - the _TARGET_ permissions just grant _READ_TARGET_TYPE_ (analogically _SOFTWARE_MODULE_ permissions grant _READ_SOFTWARE_MODULE_TYPE_ and _DISTRIBUTION_SET_ grants _READ_DISTRIBUTON_SET_TYPE_ * Hierarcy is not configurable - could be completely replaced by setting spring application property org.eclipse.hawkbit.hierarchy or could be extended by adding rules using org.eclipse.hawkbit.hierarchy.ext Signed-off-by: Avgustin Marinov <Avgustin.Marinov@bosch.com>
1 parent f2e6344 commit ae3a004

File tree

16 files changed

+179
-216
lines changed

16 files changed

+179
-216
lines changed

‎hawkbit-mgmt/hawkbit-mgmt-resource/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtDistributionSetResource.java

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
import java.util.List;
1818
import java.util.Map;
1919
import java.util.Map.Entry;
20-
import java.util.Optional;
20+
import java.util.function.Function;
2121
import java.util.stream.Collectors;
2222

2323
import jakarta.validation.ValidationException;
@@ -88,7 +88,6 @@ public class MgmtDistributionSetResource implements MgmtDistributionSetRestApi {
8888
private final DeploymentManagement deployManagement;
8989
private final SystemManagement systemManagement;
9090
private final MgmtDistributionSetMapper mgmtDistributionSetMapper;
91-
private final SystemSecurityContext systemSecurityContext;
9291
private final TenantConfigHelper tenantConfigHelper;
9392

9493
@SuppressWarnings("java:S107")
@@ -112,7 +111,6 @@ public class MgmtDistributionSetResource implements MgmtDistributionSetRestApi {
112111
this.tenantConfigHelper = TenantConfigHelper.usingContext(systemSecurityContext, tenantConfigurationManagement);
113112
this.mgmtDistributionSetMapper = mgmtDistributionSetMapper;
114113
this.systemManagement = systemManagement;
115-
this.systemSecurityContext = systemSecurityContext;
116114
}
117115

118116
@Override
@@ -144,23 +142,27 @@ public ResponseEntity<MgmtDistributionSet> getDistributionSet(final Long distrib
144142
public ResponseEntity<List<MgmtDistributionSet>> createDistributionSets(final List<MgmtDistributionSetRequestBodyPost> sets) {
145143
log.debug("creating {} distribution sets", sets.size());
146144
// set default Ds type if ds type is null
147-
final String defaultDsKey = systemSecurityContext.runAsSystem(systemManagement.getTenantMetadata().getDefaultDsType()::getKey);
145+
final String defaultDsKey = systemManagement.getTenantMetadata().getDefaultDsType().getKey();
148146
sets.stream().filter(ds -> ds.getType() == null).forEach(ds -> ds.setType(defaultDsKey));
149147

150-
//check if there is already deleted DS Type
151-
for (final MgmtDistributionSetRequestBodyPost ds : sets) {
152-
final Optional<? extends DistributionSetType> opt = distributionSetTypeManagement.findByKey(ds.getType());
153-
opt.ifPresent(dsType -> {
154-
if (dsType.isDeleted()) {
155-
final String text = "Cannot create Distribution Set from type with key {0}. Distribution Set Type already deleted!";
156-
final String message = MessageFormat.format(text, dsType.getKey());
157-
throw new ValidationException(message);
158-
}
159-
});
160-
}
161-
162-
final Collection<? extends DistributionSet> createdDSets = distributionSetManagement
163-
.create(mgmtDistributionSetMapper.fromRequest(sets));
148+
// check if target ds types exist and are not deleted, also caches them
149+
final Map<String, DistributionSetType> dsTypeKeyToDsType = sets.stream()
150+
.map(MgmtDistributionSetRequestBodyPost::getType)
151+
.distinct()
152+
.collect(Collectors.toMap(
153+
Function.identity(),
154+
dsTypeKey ->
155+
distributionSetTypeManagement.findByKey(dsTypeKey).map(dsType -> {
156+
if (dsType.isDeleted()) {
157+
throw new ValidationException(MessageFormat.format(
158+
"Cannot create Distribution Set from type with key {0}. Distribution Set Type already deleted!",
159+
dsTypeKey));
160+
}
161+
return dsType;
162+
}).orElseThrow(() -> new EntityNotFoundException(DistributionSetType.class, dsTypeKey))));
163+
164+
final Collection<? extends DistributionSet> createdDSets =
165+
distributionSetManagement.create(mgmtDistributionSetMapper.fromRequest(sets, defaultDsKey, dsTypeKeyToDsType));
164166

165167
log.debug("{} distribution sets created, return status {}", sets.size(), HttpStatus.CREATED);
166168
return new ResponseEntity<>(MgmtDistributionSetMapper.toResponseDistributionSets(createdDSets), HttpStatus.CREATED);

‎hawkbit-mgmt/hawkbit-mgmt-resource/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/mapper/MgmtDistributionSetMapper.java

Lines changed: 33 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import java.util.HashSet;
1818
import java.util.List;
1919
import java.util.Map;
20+
import java.util.Objects;
2021
import java.util.Optional;
2122
import java.util.Set;
2223
import java.util.stream.Collectors;
@@ -63,8 +64,38 @@ public class MgmtDistributionSetMapper {
6364
this.systemManagement = systemManagement;
6465
}
6566

66-
public List<DistributionSetManagement.Create> fromRequest(final Collection<MgmtDistributionSetRequestBodyPost> sets) {
67-
return sets.stream().map(this::fromRequest).toList();
67+
public List<DistributionSetManagement.Create> fromRequest(
68+
final Collection<MgmtDistributionSetRequestBodyPost> sets,
69+
final String defaultDsKey, final Map<String, DistributionSetType> dsTypeKeyToDsType) {
70+
return sets.stream().<DistributionSetManagement.Create>map(dsRest -> {
71+
final Set<Long> modules = new HashSet<>();
72+
if (dsRest.getOs() != null) {
73+
modules.add(dsRest.getOs().getId());
74+
}
75+
if (dsRest.getApplication() != null) {
76+
modules.add(dsRest.getApplication().getId());
77+
}
78+
if (dsRest.getRuntime() != null) {
79+
modules.add(dsRest.getRuntime().getId());
80+
}
81+
if (dsRest.getModules() != null) {
82+
dsRest.getModules().forEach(module -> modules.add(module.getId()));
83+
}
84+
// distribution set type, if null by the REST call shall be replaced with the default tenant DS type
85+
final String dsTypeKey = Objects.requireNonNull(dsRest.getType(), "Distribution set type must not be null");
86+
final DistributionSetType dsType = dsTypeKeyToDsType.get(dsTypeKey);
87+
if (dsType == null) {
88+
// type should never null, cache is prefilled with all types
89+
throw new EntityNotFoundException(DistributionSetType.class, defaultDsKey);
90+
}
91+
return DistributionSetManagement.Create.builder()
92+
.type(dsType)
93+
.name(dsRest.getName()).version(dsRest.getVersion())
94+
.description(dsRest.getDescription())
95+
.modules(findSoftwareModuleWithExceptionIfNotFound(modules))
96+
.requiredMigrationStep(dsRest.getRequiredMigrationStep())
97+
.build();
98+
}).toList();
6899
}
69100

70101
public static MgmtDistributionSet toResponse(final DistributionSet distributionSet) {
@@ -170,36 +201,6 @@ public static List<MgmtDistributionSet> toResponseFromDsList(final List<? extend
170201
return sets.stream().map(MgmtDistributionSetMapper::toResponse).toList();
171202
}
172203

173-
private DistributionSetManagement.Create fromRequest(final MgmtDistributionSetRequestBodyPost dsRest) {
174-
final Set<Long> modules = new HashSet<>();
175-
if (dsRest.getOs() != null) {
176-
modules.add(dsRest.getOs().getId());
177-
}
178-
if (dsRest.getApplication() != null) {
179-
modules.add(dsRest.getApplication().getId());
180-
}
181-
if (dsRest.getRuntime() != null) {
182-
modules.add(dsRest.getRuntime().getId());
183-
}
184-
if (dsRest.getModules() != null) {
185-
dsRest.getModules().forEach(module -> modules.add(module.getId()));
186-
}
187-
return DistributionSetManagement.Create.builder()
188-
.type(Optional.ofNullable(dsRest.getType())
189-
// if the type is supplied the type MUST exist
190-
.map(typeKey -> distributionSetTypeManagement
191-
.findByKey(typeKey)
192-
.orElseThrow(() -> new EntityNotFoundException(DistributionSetType.class, typeKey)))
193-
.map(DistributionSetType.class::cast)
194-
// if here, the type is not supplied, use the default type
195-
.orElseGet(() -> systemManagement.getTenantMetadata().getDefaultDsType()))
196-
.name(dsRest.getName()).version(dsRest.getVersion())
197-
.description(dsRest.getDescription())
198-
.modules(findSoftwareModuleWithExceptionIfNotFound(modules))
199-
.requiredMigrationStep(dsRest.getRequiredMigrationStep())
200-
.build();
201-
}
202-
203204
private Set<? extends SoftwareModule> findSoftwareModuleWithExceptionIfNotFound(final Set<Long> softwareModuleIds) {
204205
if (CollectionUtils.isEmpty(softwareModuleIds)) {
205206
return Collections.emptySet();

‎hawkbit-mgmt/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtTargetTypeResourceTest.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,6 @@ class MgmtTargetTypeResourceTest extends AbstractManagementApiIntegrationTest {
7373
principal = "targetTypeTester", allSpPermissions = true,
7474
removeFromAllPermission = {
7575
SpPermission.CREATE_TARGET, SpPermission.READ_TARGET, SpPermission.UPDATE_TARGET, SpPermission.DELETE_TARGET,
76-
SpPermission.CREATE_REPOSITORY, SpPermission.READ_REPOSITORY, SpPermission.UPDATE_REPOSITORY, SpPermission.DELETE_REPOSITORY,
7776
SpPermission.READ_TARGET_TYPE })
7877
void getTargetTypesWithoutPermission() throws Exception {
7978
mvc.perform(get(TARGETTYPES_ENDPOINT).accept(MediaType.APPLICATION_JSON))

‎hawkbit-mgmt/hawkbit-mgmt-server/src/test/java/org/eclipse/hawkbit/app/mgmt/PreAuthorizeEnabledTest.java

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -29,32 +29,31 @@
2929
class PreAuthorizeEnabledTest extends AbstractSecurityTest {
3030

3131
/**
32-
* Tests whether request fail if a role is forbidden for the user
32+
* Tests whether request succeed if a role is granted for the user
3333
*/
3434
@Test
35-
@WithUser(authorities = { SpPermission.READ_TARGET }, autoCreateTenant = false)
36-
void failIfNoRole() throws Exception {
35+
@WithUser(authorities = { SpPermission.READ_DISTRIBUTION_SET }, autoCreateTenant = false)
36+
void successIfHasRole() throws Exception {
3737
mvc.perform(get("/rest/v1/distributionsets")).andExpect(result ->
38-
assertThat(result.getResponse().getStatus()).isEqualTo(HttpStatus.FORBIDDEN.value()));
38+
assertThat(result.getResponse().getStatus()).isEqualTo(HttpStatus.OK.value()));
3939
}
4040

4141
/**
42-
* Tests whether request succeed if a role is granted for the user
42+
* Tests whether request fail if a role is forbidden for the user
4343
*/
4444
@Test
45-
@WithUser(authorities = { SpPermission.READ_REPOSITORY }, autoCreateTenant = false)
46-
void successIfHasRole() throws Exception {
45+
@WithUser(authorities = { SpPermission.READ_TARGET }, autoCreateTenant = false)
46+
void failIfNoRole() throws Exception {
4747
mvc.perform(get("/rest/v1/distributionsets")).andExpect(result ->
48-
assertThat(result.getResponse().getStatus()).isEqualTo(HttpStatus.OK.value()));
48+
assertThat(result.getResponse().getStatus()).isEqualTo(HttpStatus.FORBIDDEN.value()));
4949
}
5050

5151
/**
5252
* Tests whether request returns distribution set if a role with scope is granted for the user
5353
*/
5454
@Test
5555
@WithUser(authorities = {
56-
SpPermission.CREATE_REPOSITORY,
57-
SpPermission.READ_REPOSITORY,
56+
"CREATE_DISTRIBUTION_SET", "READ_DISTRIBUTION_SET_TYPE",
5857
SpPermission.READ_DISTRIBUTION_SET + "/name==DsOne" }, autoCreateTenant = false)
5958
void successIfHasRoleWithScope() throws Exception {
6059
createDsOne("successIfHasRoleWithScope");
@@ -69,8 +68,7 @@ void successIfHasRoleWithScope() throws Exception {
6968
*/
7069
@Test
7170
@WithUser(authorities = {
72-
SpPermission.CREATE_REPOSITORY,
73-
SpPermission.READ_REPOSITORY,
71+
"CREATE_DISTRIBUTION_SET", "READ_DISTRIBUTION_SET_TYPE",
7472
SpPermission.READ_DISTRIBUTION_SET + "/name==DsOne2" }, autoCreateTenant = false)
7573
void failIfHasNoForbiddingScope() throws Exception {
7674
createDsOne("failIfHasNoForbiddingScope");
@@ -99,8 +97,7 @@ void onlyDSIfNoTenantConfig() throws Exception {
9997
mvc.perform(get("/rest/v1/system/configs")).andExpect(result -> {
10098
// returns default DS type because of READ_TARGET
10199
assertThat(result.getResponse().getStatus()).isEqualTo(HttpStatus.OK.value());
102-
assertThat(new ObjectMapper().reader().readValue(result.getResponse().getContentAsString(), HashMap.class))
103-
.hasSize(1);
100+
assertThat(new ObjectMapper().reader().readValue(result.getResponse().getContentAsString(), HashMap.class)).hasSize(1);
104101
});
105102
}
106103

‎hawkbit-monolith/hawkbit-update-server/src/test/java/org/eclipse/hawkbit/app/PreAuthorizeEnabledTest.java

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -29,32 +29,31 @@
2929
class PreAuthorizeEnabledTest extends AbstractSecurityTest {
3030

3131
/**
32-
* Tests whether request fail if a role is forbidden for the user
32+
* Tests whether request succeed if a role is granted for the user
3333
*/
3434
@Test
35-
@WithUser(authorities = { SpPermission.READ_TARGET }, autoCreateTenant = false)
36-
void failIfNoRole() throws Exception {
35+
@WithUser(authorities = { SpPermission.READ_DISTRIBUTION_SET }, autoCreateTenant = false)
36+
void successIfHasRole() throws Exception {
3737
mvc.perform(get("/rest/v1/distributionsets"))
38-
.andExpect(result -> assertThat(result.getResponse().getStatus()).isEqualTo(HttpStatus.FORBIDDEN.value()));
38+
.andExpect(result -> assertThat(result.getResponse().getStatus()).isEqualTo(HttpStatus.OK.value()));
3939
}
4040

4141
/**
42-
* Tests whether request succeed if a role is granted for the user
42+
* Tests whether request fail if a role is forbidden for the user
4343
*/
4444
@Test
45-
@WithUser(authorities = { SpPermission.READ_REPOSITORY }, autoCreateTenant = false)
46-
void successIfHasRole() throws Exception {
45+
@WithUser(authorities = { SpPermission.READ_TARGET }, autoCreateTenant = false)
46+
void failIfNoRole() throws Exception {
4747
mvc.perform(get("/rest/v1/distributionsets"))
48-
.andExpect(result -> assertThat(result.getResponse().getStatus()).isEqualTo(HttpStatus.OK.value()));
48+
.andExpect(result -> assertThat(result.getResponse().getStatus()).isEqualTo(HttpStatus.FORBIDDEN.value()));
4949
}
5050

5151
/**
5252
* Tests whether request returns distribution set if a role with scope is granted for the user
5353
*/
5454
@Test
5555
@WithUser(authorities = {
56-
SpPermission.CREATE_REPOSITORY,
57-
SpPermission.READ_REPOSITORY,
56+
"CREATE_DISTRIBUTION_SET", "READ_DISTRIBUTION_SET_TYPE",
5857
SpPermission.READ_DISTRIBUTION_SET + "/name==DsOne" }, autoCreateTenant = false)
5958
void successIfHasRoleWithScope() throws Exception {
6059
createDsOne("successIfHasRoleWithScope");
@@ -69,8 +68,7 @@ void successIfHasRoleWithScope() throws Exception {
6968
*/
7069
@Test
7170
@WithUser(authorities = {
72-
SpPermission.CREATE_REPOSITORY,
73-
SpPermission.READ_REPOSITORY,
71+
"CREATE_DISTRIBUTION_SET", "READ_DISTRIBUTION_SET_TYPE",
7472
SpPermission.READ_DISTRIBUTION_SET + "/name==DsOne2" }, autoCreateTenant = false)
7573
void failIfHasNoForbiddingScope() throws Exception {
7674
createDsOne("failIfHasNoForbiddingScope");
@@ -100,8 +98,7 @@ void onlyDSIfNoTenantConfig() throws Exception {
10098
.andExpect(result -> {
10199
// returns default DS type because of READ_TARGET
102100
assertThat(result.getResponse().getStatus()).isEqualTo(HttpStatus.OK.value());
103-
assertThat(new ObjectMapper().reader().readValue(result.getResponse().getContentAsString(), HashMap.class))
104-
.hasSize(1);
101+
assertThat(new ObjectMapper().reader().readValue(result.getResponse().getContentAsString(), HashMap.class)).hasSize(1);
105102
});
106103
}
107104

‎hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/ArtifactManagement.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ default String permissionGroup() {
6262
* @param isEncrypted flag to indicate if artifact is encrypted.
6363
* @return loaded {@link StoredArtifactInfo}
6464
*/
65-
@PreAuthorize("hasAuthority('" + SpPermission.DOWNLOAD_REPOSITORY_ARTIFACT + "')" + " or " + SpringEvalExpressions.IS_CONTROLLER)
65+
@PreAuthorize("hasAuthority('DOWNLOAD_REPOSITORY_ARTIFACT') or hasAuthority('" + SpPermission.SOFTWARE_MODULE_DOWNLOAD_ARTIFACT + "')" + " or " + SpringEvalExpressions.IS_CONTROLLER)
6666
ArtifactStream getArtifactStream(@NotEmpty String sha1Hash, long softwareModuleId, final boolean isEncrypted);
6767

6868
/**

‎hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/SystemManagement.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ public interface SystemManagement {
7474
/**
7575
* @return {@link TenantMetaData} of {@link TenantAware#getCurrentTenant()}
7676
*/
77-
@PreAuthorize("hasAuthority('" + SpPermission.READ_REPOSITORY + "')" + " or "
77+
@PreAuthorize("hasAuthority('" + SpPermission.READ_DISTRIBUTION_SET + "')" + " or "
7878
+ "hasAuthority('READ_" + SpPermission.TARGET + "')" + " or "
7979
+ "hasAuthority('READ_" + SpPermission.TENANT_CONFIGURATION + "')" + " or "
8080
+ SpringEvalExpressions.IS_CONTROLLER)
@@ -83,7 +83,7 @@ public interface SystemManagement {
8383
/**
8484
* @return {@link TenantMetaData} of {@link TenantAware#getCurrentTenant()} without details ({@link TenantMetaData#getDefaultDsType()})
8585
*/
86-
@PreAuthorize("hasAuthority('" + SpPermission.READ_REPOSITORY + "')" + " or "
86+
@PreAuthorize("hasAuthority('" + SpPermission.READ_DISTRIBUTION_SET + "')" + " or "
8787
+ "hasAuthority('READ_" + SpPermission.TARGET + "')" + " or "
8888
+ "hasAuthority('READ_" + SpPermission.TENANT_CONFIGURATION + "')" + " or "
8989
+ SpringEvalExpressions.IS_CONTROLLER)

‎hawkbit-repository/hawkbit-repository-core/src/main/java/org/eclipse/hawkbit/repository/RepositoryConfiguration.java

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import org.eclipse.hawkbit.im.authentication.Hierarchy;
2020
import org.eclipse.hawkbit.tenancy.configuration.ControllerPollProperties;
2121
import org.eclipse.hawkbit.tenancy.configuration.TenantConfigurationProperties;
22+
import org.springframework.beans.factory.annotation.Value;
2223
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
2324
import org.springframework.boot.context.properties.EnableConfigurationProperties;
2425
import org.springframework.context.ApplicationContext;
@@ -38,6 +39,7 @@
3839
import org.springframework.security.core.Authentication;
3940
import org.springframework.security.core.GrantedAuthority;
4041
import org.springframework.security.core.authority.SimpleGrantedAuthority;
42+
import org.springframework.util.ObjectUtils;
4143
import org.springframework.util.function.SingletonSupplier;
4244

4345
/**
@@ -52,8 +54,16 @@ public class RepositoryConfiguration {
5254

5355
@Bean
5456
@ConditionalOnMissingBean
55-
static RoleHierarchy roleHierarchy() {
56-
return RoleHierarchyImpl.fromHierarchy(Hierarchy.DEFAULT);
57+
@SuppressWarnings("java:S3358") // java:S3358 better readable this way
58+
RoleHierarchy roleHierarchy(
59+
// if configured replaces the hierarchy completely
60+
@Value("${org.eclipse.hawkbit.hierarchy:}") final String hierarchy,
61+
// if the "hierarchy" property is empty, and this property is configured it is appended to the default hierarchy
62+
@Value("${org.eclipse.hawkbit.hierarchy.ext:}") final String hierarchyExt) {
63+
return RoleHierarchyImpl.fromHierarchy(
64+
ObjectUtils.isEmpty(hierarchy)
65+
? (ObjectUtils.isEmpty(hierarchyExt) ? Hierarchy.DEFAULT : Hierarchy.DEFAULT + hierarchyExt)
66+
: hierarchy);
5767
}
5868

5969
@Bean

0 commit comments

Comments
 (0)