Skip to content

Commit 090f57a

Browse files
fix(ConfigurationRestRepository): refactor property access control and improve admin checks
credits: @saschaszott ref: #537, DSC-2686
1 parent 2e4baf3 commit 090f57a

2 files changed

Lines changed: 23 additions & 7 deletions

File tree

dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ConfigurationRestRepository.java

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -66,14 +66,13 @@ protected String[] getAdminExposedProperties() {
6666
@Override
6767
@PreAuthorize("permitAll()")
6868
public PropertyRest findOne(Context context, String property) {
69-
List<String> exposedProperties = Arrays.asList(getExposedProperties());
70-
List<String> adminExposedProperties = Arrays.asList(getAdminExposedProperties());
69+
if (
70+
!isAdminAllowed(context, property) && !isExposed(property)
71+
) {
72+
throw new ResourceNotFoundException("No such configuration property: " + property);
73+
}
7174

72-
if (!configurationService.hasProperty(property) ||
73-
(
74-
!exposedProperties.contains(property) &&
75-
(!isCurrentUserAdmin(context) || !adminExposedProperties.contains(property))
76-
)) {
75+
if (!configurationService.hasProperty(property)) {
7776
throw new ResourceNotFoundException("No such configuration property: " + property);
7877
}
7978

@@ -84,6 +83,16 @@ public PropertyRest findOne(Context context, String property) {
8483
return propertyRest;
8584
}
8685

86+
private boolean isExposed(String property) {
87+
List<String> exposedProperties = Arrays.asList(getExposedProperties());
88+
return exposedProperties.contains(property);
89+
}
90+
91+
private boolean isAdminAllowed(Context context, String property) {
92+
List<String> adminExposedProperties = Arrays.asList(getAdminExposedProperties());
93+
return adminExposedProperties.contains(property) && isCurrentUserAdmin(context);
94+
}
95+
8796
private boolean isCurrentUserAdmin(Context context) {
8897
try {
8998
return authorizeService.isAdmin(context);

dspace-server-webapp/src/test/java/org/dspace/app/rest/ConfigurationRestRepositoryIT.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,13 @@ public void getAdminRestrictedValueRetrieved() throws Exception {
6060
.andExpect(status().is2xxSuccessful());
6161
}
6262

63+
@Test
64+
public void getNonAdminRestrictedPropertyNotRetrieved() throws Exception {
65+
String tokenAdmin = getAuthToken(admin.getEmail(), password);
66+
getClient(tokenAdmin).perform(get("/api/config/properties/db.url"))
67+
.andExpect(status().isNotFound());
68+
}
69+
6370
@Test
6471
public void getAll() throws Exception {
6572
getClient().perform(get("/api/config/properties/"))

0 commit comments

Comments
 (0)