From 1063af076dbf61ed7e7db2478f8727277b510c7a Mon Sep 17 00:00:00 2001
From: Lukas Malte Monnerjahn <lukasmalte.monnerjahn@dataport.de>
Date: Wed, 27 Mar 2024 09:57:13 +0100
Subject: [PATCH] OZG-5176 PR Anmerkungen
---
.../de/ozgcloud/admin/RootModelAssembler.java | 2 +-
.../ozgcloud/admin/common/user/UserRole.java | 2 +-
.../admin/security/SecurityConfiguration.java | 8 +++----
.../java/de/ozgcloud/admin/ApiRootITCase.java | 2 +-
.../admin/RootModelAssemblerTest.java | 6 ++---
.../common/user/CurrentUserHelperTest.java | 22 +++++++++----------
.../common/user/CurrentUserServiceTest.java | 4 ++--
.../admin/security/JwtTestFactory.java | 11 ----------
.../security/SecurityConfigurationTest.java | 12 +++++-----
.../ozgcloud/admin/setting/SettingITCase.java | 2 +-
10 files changed, 30 insertions(+), 41 deletions(-)
diff --git a/src/main/java/de/ozgcloud/admin/RootModelAssembler.java b/src/main/java/de/ozgcloud/admin/RootModelAssembler.java
index 479177c2..319cb4d3 100644
--- a/src/main/java/de/ozgcloud/admin/RootModelAssembler.java
+++ b/src/main/java/de/ozgcloud/admin/RootModelAssembler.java
@@ -56,7 +56,7 @@ public class RootModelAssembler implements RepresentationModelAssembler<Root, En
List<Link> links = new ArrayList<>();
var rootLinkBuilder = WebMvcLinkBuilder.linkTo(RootController.class);
links.add(rootLinkBuilder.withSelfRel());
- if (currentUserService.hasRole(UserRole.ADMIN_USER)) {
+ if (currentUserService.hasRole(UserRole.ADMIN_ADMIN)) {
links.add(buildConfigLink());
}
return links;
diff --git a/src/main/java/de/ozgcloud/admin/common/user/UserRole.java b/src/main/java/de/ozgcloud/admin/common/user/UserRole.java
index d369b3b3..1b0db41b 100644
--- a/src/main/java/de/ozgcloud/admin/common/user/UserRole.java
+++ b/src/main/java/de/ozgcloud/admin/common/user/UserRole.java
@@ -28,5 +28,5 @@ import lombok.NoArgsConstructor;
@NoArgsConstructor(access = AccessLevel.PRIVATE)
public class UserRole {
- public static final String ADMIN_USER = "ADMIN_ADMIN";
+ public static final String ADMIN_ADMIN = "ADMIN_ADMIN";
}
diff --git a/src/main/java/de/ozgcloud/admin/security/SecurityConfiguration.java b/src/main/java/de/ozgcloud/admin/security/SecurityConfiguration.java
index 4a87241b..b1c6280e 100644
--- a/src/main/java/de/ozgcloud/admin/security/SecurityConfiguration.java
+++ b/src/main/java/de/ozgcloud/admin/security/SecurityConfiguration.java
@@ -73,8 +73,8 @@ public class SecurityConfiguration {
http.authorizeHttpRequests(requests -> requests
.requestMatchers(HttpMethod.GET, "/api/environment").permitAll()
- .requestMatchers("/api/configuration").hasRole(UserRole.ADMIN_USER)
- .requestMatchers("/api/configuration/**").hasRole(UserRole.ADMIN_USER)
+ .requestMatchers("/api/configuration").hasRole(UserRole.ADMIN_ADMIN)
+ .requestMatchers("/api/configuration/**").hasRole(UserRole.ADMIN_ADMIN)
.requestMatchers("/api").authenticated()
.requestMatchers("/api/**").authenticated()
.requestMatchers("/actuator").permitAll()
@@ -95,7 +95,7 @@ public class SecurityConfiguration {
}
Set<GrantedAuthority> convertJwtToGrantedAuthorities(Jwt jwt) {
- return getKeycloakRolesFromJwt(jwt)
+ return getRolesFromJwt(jwt)
.stream()
.map(this::mapRoleStringToGrantedAuthority)
.collect(toSet());
@@ -105,7 +105,7 @@ public class SecurityConfiguration {
return new SimpleGrantedAuthority(SIMPLE_GRANT_AUTHORITY_PREFIX + role);
}
- List<String> getKeycloakRolesFromJwt(Jwt jwt) {
+ List<String> getRolesFromJwt(Jwt jwt) {
return Optional.ofNullable(jwt.getClaimAsMap(RESOURCE_ACCESS_KEY))
.flatMap(resourceAccessMap -> getMap(resourceAccessMap, oAuth2Properties.getResource()))
.flatMap(adminClientMap -> getList(adminClientMap, ROLES_KEY))
diff --git a/src/test/java/de/ozgcloud/admin/ApiRootITCase.java b/src/test/java/de/ozgcloud/admin/ApiRootITCase.java
index 3f9624d0..87b3da37 100644
--- a/src/test/java/de/ozgcloud/admin/ApiRootITCase.java
+++ b/src/test/java/de/ozgcloud/admin/ApiRootITCase.java
@@ -40,7 +40,7 @@ import lombok.SneakyThrows;
@ITCase
@AutoConfigureMockMvc
-@WithMockUser(roles = UserRole.ADMIN_USER)
+@WithMockUser(roles = UserRole.ADMIN_ADMIN)
class ApiRootITCase {
@Autowired
diff --git a/src/test/java/de/ozgcloud/admin/RootModelAssemblerTest.java b/src/test/java/de/ozgcloud/admin/RootModelAssemblerTest.java
index a08b15d5..9e797f60 100644
--- a/src/test/java/de/ozgcloud/admin/RootModelAssemblerTest.java
+++ b/src/test/java/de/ozgcloud/admin/RootModelAssemblerTest.java
@@ -57,7 +57,7 @@ class RootModelAssemblerTest {
class TestEntityModel {
@BeforeEach
void beforeEach() {
- Mockito.when(currentUserService.hasRole(UserRole.ADMIN_USER)).thenReturn(true);
+ Mockito.when(currentUserService.hasRole(UserRole.ADMIN_ADMIN)).thenReturn(true);
Mockito.when(restProperties.getBasePath()).thenReturn(BASE_PATH);
}
@@ -90,7 +90,7 @@ class RootModelAssemblerTest {
@Test
void shouldHaveHrefToBasePathIfAuthorized() {
Mockito.when(restProperties.getBasePath()).thenReturn(BASE_PATH);
- Mockito.when(currentUserService.hasRole(UserRole.ADMIN_USER)).thenReturn(true);
+ Mockito.when(currentUserService.hasRole(UserRole.ADMIN_ADMIN)).thenReturn(true);
List<Link> links = modelAssembler.buildRootModelLinks();
@@ -101,7 +101,7 @@ class RootModelAssemblerTest {
@Test
void shouldNotHaveHrefToBasePathIfUnauthorized() {
- Mockito.when(currentUserService.hasRole(UserRole.ADMIN_USER)).thenReturn(false);
+ Mockito.when(currentUserService.hasRole(UserRole.ADMIN_ADMIN)).thenReturn(false);
List<Link> links = modelAssembler.buildRootModelLinks();
diff --git a/src/test/java/de/ozgcloud/admin/common/user/CurrentUserHelperTest.java b/src/test/java/de/ozgcloud/admin/common/user/CurrentUserHelperTest.java
index ceffbade..42866857 100644
--- a/src/test/java/de/ozgcloud/admin/common/user/CurrentUserHelperTest.java
+++ b/src/test/java/de/ozgcloud/admin/common/user/CurrentUserHelperTest.java
@@ -59,7 +59,7 @@ class CurrentUserHelperTest {
Mockito.CALLS_REAL_METHODS)) {
mockUserHelper.when(CurrentUserHelper::getAuthentication).thenReturn(null);
- boolean hasRole = CurrentUserHelper.hasRole(UserRole.ADMIN_USER);
+ boolean hasRole = CurrentUserHelper.hasRole(UserRole.ADMIN_ADMIN);
assertThat(hasRole).isFalse();
}
@@ -73,7 +73,7 @@ class CurrentUserHelperTest {
Mockito.CALLS_REAL_METHODS)) {
mockUserHelper.when(CurrentUserHelper::getAuthentication).thenReturn(mockAuthentication);
- boolean hasRole = CurrentUserHelper.hasRole(UserRole.ADMIN_USER);
+ boolean hasRole = CurrentUserHelper.hasRole(UserRole.ADMIN_ADMIN);
assertThat(hasRole).isFalse();
}
@@ -93,9 +93,9 @@ class CurrentUserHelperTest {
mockUserHelper.when(() -> CurrentUserHelper.containsRole(Mockito.anyList(), Mockito.anyString()))
.thenReturn(containsRoleValue);
- boolean hasRole = CurrentUserHelper.hasRole(UserRole.ADMIN_USER);
+ boolean hasRole = CurrentUserHelper.hasRole(UserRole.ADMIN_ADMIN);
- mockUserHelper.verify(() -> CurrentUserHelper.containsRole(mockAuthentication.getAuthorities(), UserRole.ADMIN_USER));
+ mockUserHelper.verify(() -> CurrentUserHelper.containsRole(mockAuthentication.getAuthorities(), UserRole.ADMIN_ADMIN));
assertThat(hasRole).isEqualTo(containsRoleValue);
}
}
@@ -106,7 +106,7 @@ class CurrentUserHelperTest {
class TestContainsRole {
@Test
void shouldNotContainRoleIfAuthoritiesIsNull() {
- boolean containsRole = CurrentUserHelper.containsRole(null, UserRole.ADMIN_USER);
+ boolean containsRole = CurrentUserHelper.containsRole(null, UserRole.ADMIN_ADMIN);
assertThat(containsRole).isFalse();
}
@@ -116,7 +116,7 @@ class CurrentUserHelperTest {
List<GrantedAuthority> authorities = List.of(
new SimpleGrantedAuthority(CurrentUserHelper.ROLE_PREFIX + "OTHER"));
- boolean containsRole = CurrentUserHelper.containsRole(authorities, UserRole.ADMIN_USER);
+ boolean containsRole = CurrentUserHelper.containsRole(authorities, UserRole.ADMIN_ADMIN);
assertThat(containsRole).isFalse();
}
@@ -124,9 +124,9 @@ class CurrentUserHelperTest {
@Test
void shouldContainRole() {
Collection<? extends GrantedAuthority> authorities = List.of(
- new SimpleGrantedAuthority(CurrentUserHelper.ROLE_PREFIX + UserRole.ADMIN_USER));
+ new SimpleGrantedAuthority(CurrentUserHelper.ROLE_PREFIX + UserRole.ADMIN_ADMIN));
- boolean containsRole = CurrentUserHelper.containsRole(authorities, UserRole.ADMIN_USER);
+ boolean containsRole = CurrentUserHelper.containsRole(authorities, UserRole.ADMIN_ADMIN);
assertThat(containsRole).isTrue();
}
@@ -138,16 +138,16 @@ class CurrentUserHelperTest {
@Test
void shouldAddPrefixIfMissing() {
- var roleWithoutPrefix = UserRole.ADMIN_USER;
+ var roleWithoutPrefix = UserRole.ADMIN_ADMIN;
var role = CurrentUserHelper.addRolePrefixIfMissing(roleWithoutPrefix);
- assertThat(role).isEqualTo(CurrentUserHelper.ROLE_PREFIX + UserRole.ADMIN_USER);
+ assertThat(role).isEqualTo(CurrentUserHelper.ROLE_PREFIX + UserRole.ADMIN_ADMIN);
}
@Test
void shouldReturnRoleIfPrefixAlreadyExists() {
- var roleWithPrefix = CurrentUserHelper.ROLE_PREFIX + UserRole.ADMIN_USER;
+ var roleWithPrefix = CurrentUserHelper.ROLE_PREFIX + UserRole.ADMIN_ADMIN;
var role = CurrentUserHelper.addRolePrefixIfMissing(roleWithPrefix);
diff --git a/src/test/java/de/ozgcloud/admin/common/user/CurrentUserServiceTest.java b/src/test/java/de/ozgcloud/admin/common/user/CurrentUserServiceTest.java
index 20e75bfc..878292bc 100644
--- a/src/test/java/de/ozgcloud/admin/common/user/CurrentUserServiceTest.java
+++ b/src/test/java/de/ozgcloud/admin/common/user/CurrentUserServiceTest.java
@@ -47,9 +47,9 @@ class CurrentUserServiceTest {
mockUserHelper.when(() -> CurrentUserHelper.hasRole(Mockito.anyString()))
.thenReturn(hasRoleValue);
- boolean hasRole = currentUserService.hasRole(UserRole.ADMIN_USER);
+ boolean hasRole = currentUserService.hasRole(UserRole.ADMIN_ADMIN);
- mockUserHelper.verify(() -> CurrentUserHelper.hasRole(UserRole.ADMIN_USER));
+ mockUserHelper.verify(() -> CurrentUserHelper.hasRole(UserRole.ADMIN_ADMIN));
assertThat(hasRole).isEqualTo(hasRoleValue);
}
}
diff --git a/src/test/java/de/ozgcloud/admin/security/JwtTestFactory.java b/src/test/java/de/ozgcloud/admin/security/JwtTestFactory.java
index 1638bd85..719c94af 100644
--- a/src/test/java/de/ozgcloud/admin/security/JwtTestFactory.java
+++ b/src/test/java/de/ozgcloud/admin/security/JwtTestFactory.java
@@ -44,17 +44,6 @@ public class JwtTestFactory {
return createBuilder().claim(RESOURCE_ACCESS_KEY, Map.of(AUTH_RESOURCE, Map.of(ROLES_KEY, roles)));
}
- // private static Map<String, Object> readResourceAccessClaim() {
- // var claimsJson = TestUtils.loadTextFile("jsonTemplates/security/resource_access.template.json");
- // var mapper = new ObjectMapper();
- // try {
- // return mapper.readValue(claimsJson, new TypeReference<Map<String, Object>>() {
- // });
- // } catch (IOException e) {
- // throw new RuntimeException(e);
- // }
- // }
-
public static Jwt.Builder createBuilder() {
return Jwt.withTokenValue("token-value").header("header-key", "header-value").claim("claim-key", "claim-value");
}
diff --git a/src/test/java/de/ozgcloud/admin/security/SecurityConfigurationTest.java b/src/test/java/de/ozgcloud/admin/security/SecurityConfigurationTest.java
index 5fa89431..9acdae93 100644
--- a/src/test/java/de/ozgcloud/admin/security/SecurityConfigurationTest.java
+++ b/src/test/java/de/ozgcloud/admin/security/SecurityConfigurationTest.java
@@ -109,7 +109,7 @@ class SecurityConfigurationTest {
void mock() {
var keycloakRoles = List.of(ROLE_1, JwtTestFactory.ROLE_2, JwtTestFactory.ROLE_3);
expectedSecurityRoleStrings = keycloakRoles.stream().map(role -> SIMPLE_GRANT_AUTHORITY_PREFIX + role).toList();
- doReturn(keycloakRoles).when(securityConfiguration).getKeycloakRolesFromJwt(any());
+ doReturn(keycloakRoles).when(securityConfiguration).getRolesFromJwt(any());
}
@DisplayName("should call get keycloak roles from jwt")
@@ -119,7 +119,7 @@ class SecurityConfigurationTest {
securityConfiguration.convertJwtToGrantedAuthorities(jwt);
- verify(securityConfiguration).getKeycloakRolesFromJwt(jwt);
+ verify(securityConfiguration).getRolesFromJwt(jwt);
}
@DisplayName("should return granted authorities with ROLE_ prefix")
@@ -136,9 +136,9 @@ class SecurityConfigurationTest {
}
}
- @DisplayName("get keycloak roles from jwt")
+ @DisplayName("get roles from jwt")
@Nested
- class TestGetKeycloakRolesFromJwt {
+ class TestGetRolesFromJwt {
@BeforeEach
void mock() {
@@ -149,7 +149,7 @@ class SecurityConfigurationTest {
@ParameterizedTest
@MethodSource("getIncompleteJwt")
void shouldReturnEmptyListIfResourceAccessAdminRolesPathIsMissing(Jwt incompleteJwt) {
- var roleStrings = securityConfiguration.getKeycloakRolesFromJwt(incompleteJwt);
+ var roleStrings = securityConfiguration.getRolesFromJwt(incompleteJwt);
assertThat(roleStrings).isEmpty();
}
@@ -168,7 +168,7 @@ class SecurityConfigurationTest {
var expectedRoles = List.of(ROLE_1, JwtTestFactory.ROLE_2, JwtTestFactory.ROLE_3);
var jwtWithRoles = JwtTestFactory.createWithRoles(expectedRoles).build();
- var roleStrings = securityConfiguration.getKeycloakRolesFromJwt(jwtWithRoles);
+ var roleStrings = securityConfiguration.getRolesFromJwt(jwtWithRoles);
assertThat(roleStrings).isEqualTo(expectedRoles);
}
diff --git a/src/test/java/de/ozgcloud/admin/setting/SettingITCase.java b/src/test/java/de/ozgcloud/admin/setting/SettingITCase.java
index d4a72b63..04aaf44f 100644
--- a/src/test/java/de/ozgcloud/admin/setting/SettingITCase.java
+++ b/src/test/java/de/ozgcloud/admin/setting/SettingITCase.java
@@ -56,7 +56,7 @@ import lombok.SneakyThrows;
@DataITCase
@AutoConfigureMockMvc
-@WithMockUser(roles = UserRole.ADMIN_USER)
+@WithMockUser(roles = UserRole.ADMIN_ADMIN)
class SettingITCase {
@Autowired
--
GitLab