From 8c498c42f7873e0a4397a5000420705061ed5705 Mon Sep 17 00:00:00 2001 From: OZGCloud <ozgcloud@mgm-tp.com> Date: Thu, 29 Jun 2023 07:40:04 +0200 Subject: [PATCH] OZG-3961 - update user --- pom.xml | 8 +++ .../keycloak/user/KeycloakUserMapper.java | 35 ++++++------ .../keycloak/user/KeycloakUserReconciler.java | 2 +- .../user/KeycloakUserRemoteService.java | 7 ++- .../keycloak/user/KeycloakUserService.java | 14 +++-- .../keycloak/user/KeycloakUserMapperTest.java | 53 ++++++++++++++----- .../user/KeycloakUserReconcilerTest.java | 4 +- .../user/KeycloakUserRemoteServiceTest.java | 21 ++++++-- .../user/KeycloakUserServiceTest.java | 46 ++++++++-------- .../user/UserRepresentationTestFactory.java | 3 +- 10 files changed, 116 insertions(+), 77 deletions(-) diff --git a/pom.xml b/pom.xml index caf7c83..2f6f33d 100644 --- a/pom.xml +++ b/pom.xml @@ -9,6 +9,14 @@ <!-- <version>2.1.0</version>--> <!-- <relativePath/> lookup parent from repository --> <!-- </parent>--> + +<!-- <parent>--> +<!-- <groupId>de.itvsh.kop.common</groupId>--> +<!-- <artifactId>kop-common-dependency</artifactId>--> +<!-- <version>2.1.0</version>--> +<!-- <relativePath/> lookup parent from repository --> +<!-- </parent>--> + <parent> <groupId>org.springframework.boot</groupId> <artifactId>spring-boot-starter-parent</artifactId> diff --git a/src/main/java/de/ozgcloud/operator/keycloak/user/KeycloakUserMapper.java b/src/main/java/de/ozgcloud/operator/keycloak/user/KeycloakUserMapper.java index b792244..674c289 100644 --- a/src/main/java/de/ozgcloud/operator/keycloak/user/KeycloakUserMapper.java +++ b/src/main/java/de/ozgcloud/operator/keycloak/user/KeycloakUserMapper.java @@ -6,43 +6,40 @@ import java.util.stream.Collectors; import org.keycloak.representations.idm.CredentialRepresentation; import org.keycloak.representations.idm.UserRepresentation; +import org.mapstruct.CollectionMappingStrategy; import org.mapstruct.Mapper; import org.mapstruct.Mapping; +import org.mapstruct.MappingTarget; import org.mapstruct.Named; +import org.mapstruct.ReportingPolicy; import de.ozgcloud.operator.keycloak.user.OzgKeycloakUserSpec.KeycloakUserSpecClientRole; import de.ozgcloud.operator.keycloak.user.OzgKeycloakUserSpec.KeycloakUserSpecUserGroup; -@Mapper +@Mapper(unmappedTargetPolicy = ReportingPolicy.IGNORE, unmappedSourcePolicy = ReportingPolicy.IGNORE, collectionMappingStrategy = CollectionMappingStrategy.ACCESSOR_ONLY) interface KeycloakUserMapper { - @Mapping(target = "access", ignore = true) - @Mapping(target = "attributes", ignore = true) - @Mapping(target = "clientConsents", ignore = true) @Mapping(target = "clientRoles", source = "keycloakUser.clientRoles", qualifiedByName = "mapClientRoles") - @Mapping(target = "createdTimestamp", ignore = true) @Mapping(target = "credentials", source = ".", qualifiedByName = "mapPassword") - @Mapping(target = "disableableCredentialTypes", ignore = true) @Mapping(target = "email", source = "keycloakUser.email") @Mapping(target = "emailVerified", constant = "true") @Mapping(target = "enabled", constant = "true") - @Mapping(target = "federatedIdentities", ignore = true) - @Mapping(target = "federationLink", ignore = true) @Mapping(target = "groups", source = "keycloakUser.groups", qualifiedByName = "mapGroups") - @Mapping(target = "id", ignore = true) - @Mapping(target = "notBefore", ignore = true) - @Mapping(target = "origin", ignore = true) - @Mapping(target = "realmRoles", ignore = true) - @Mapping(target = "requiredActions", ignore = true) - @Mapping(target = "self", ignore = true) - @Mapping(target = "serviceAccountClientId", ignore = true) - @Mapping(target = "socialLinks", ignore = true) - @Mapping(target = "totp", ignore = true) - @Mapping(target = "applicationRoles", ignore = true) @Mapping(target = "username", source = "keycloakUser.username") @Mapping(target = "firstName", source = "keycloakUser.firstName") @Mapping(target = "lastName", source = "keycloakUser.lastName") - UserRepresentation toUserRepresentation(OzgKeycloakUserSpec user); + UserRepresentation map(OzgKeycloakUserSpec user); + + @Mapping(target = "clientRoles", source = "keycloakUser.clientRoles", qualifiedByName = "mapClientRoles") + @Mapping(target = "credentials", source = ".", qualifiedByName = "mapPassword") + @Mapping(target = "email", source = "keycloakUser.email") + @Mapping(target = "emailVerified", constant = "true") + @Mapping(target = "enabled", constant = "true") + @Mapping(target = "groups", source = "keycloakUser.groups", qualifiedByName = "mapGroups") + @Mapping(target = "username", source = "keycloakUser.username") + @Mapping(target = "firstName", source = "keycloakUser.firstName") + @Mapping(target = "lastName", source = "keycloakUser.lastName") + UserRepresentation update(@MappingTarget UserRepresentation userRepresentation, OzgKeycloakUserSpec userSpec); @Named("mapPassword") default List<CredentialRepresentation> mapPassword(OzgKeycloakUserSpec user) { diff --git a/src/main/java/de/ozgcloud/operator/keycloak/user/KeycloakUserReconciler.java b/src/main/java/de/ozgcloud/operator/keycloak/user/KeycloakUserReconciler.java index 57546d4..6b4f62e 100644 --- a/src/main/java/de/ozgcloud/operator/keycloak/user/KeycloakUserReconciler.java +++ b/src/main/java/de/ozgcloud/operator/keycloak/user/KeycloakUserReconciler.java @@ -35,7 +35,7 @@ public class KeycloakUserReconciler implements Reconciler<OzgKeycloakUser>, Clea String namespace = resource.getMetadata().getNamespace(); - keycloakUserService.addUser(resource.getSpec(), namespace); + keycloakUserService.createOrUpdateUser(resource.getSpec(), namespace); resource.setStatus(OzgKeycloakUserStatus.builder().status(OzgCustomResourceStatus.OK).errorMessage(null).build()); return UpdateControl.updateStatus(resource); diff --git a/src/main/java/de/ozgcloud/operator/keycloak/user/KeycloakUserRemoteService.java b/src/main/java/de/ozgcloud/operator/keycloak/user/KeycloakUserRemoteService.java index 44becc0..0cbc5ec 100644 --- a/src/main/java/de/ozgcloud/operator/keycloak/user/KeycloakUserRemoteService.java +++ b/src/main/java/de/ozgcloud/operator/keycloak/user/KeycloakUserRemoteService.java @@ -61,10 +61,6 @@ class KeycloakUserRemoteService { realmResource.users().get(userId).roles().clientLevel(appClient.getId()).add(Arrays.asList(clientRole)); } - boolean userExistsByName(String userName, String realm) { - return getUserByName(userName, realm).isPresent(); - } - Optional<UserRepresentation> getUserByName(String username, String realm) { return keycloakClient.getKeycloak().realm(realm).users().search(username, true).stream().findFirst(); } @@ -73,4 +69,7 @@ class KeycloakUserRemoteService { keycloakClient.getKeycloak().realm(realm).users().delete(userId); } + void updateUser(UserRepresentation user, String realm) { + keycloakClient.getKeycloak().realm(realm).users().get(user.getId()).update(user); + } } diff --git a/src/main/java/de/ozgcloud/operator/keycloak/user/KeycloakUserService.java b/src/main/java/de/ozgcloud/operator/keycloak/user/KeycloakUserService.java index 27b2203..bb53e6a 100644 --- a/src/main/java/de/ozgcloud/operator/keycloak/user/KeycloakUserService.java +++ b/src/main/java/de/ozgcloud/operator/keycloak/user/KeycloakUserService.java @@ -14,18 +14,16 @@ public class KeycloakUserService { @Autowired private KeycloakUserMapper userMapper; - void addUser(OzgKeycloakUserSpec userSpec, String namespace) { - Optional.of(userSpec) - .map(userMapper::toUserRepresentation) - .filter(keycloakUser -> !remoteService.userExistsByName(keycloakUser.getUsername(), namespace)) - .ifPresent(keycloakUser -> remoteService.createUser(keycloakUser, namespace)); + void createOrUpdateUser(OzgKeycloakUserSpec userSpec, String namespace) { + remoteService.getUserByName(userSpec.getKeycloakUser().getUsername(), namespace) + .ifPresentOrElse(existingUser -> remoteService.updateUser(userMapper.update(existingUser, userSpec), namespace), + () -> remoteService.createUser(userMapper.map(userSpec), namespace)); } void deleteUser(OzgKeycloakUserSpec userSpec, String namespace) { Optional.of(userSpec) - .map(userMapper::toUserRepresentation) - .map(keycloakUser -> remoteService.getUserByName(keycloakUser.getUsername(), namespace)) - .filter(Optional::isPresent).map(Optional::get) + .map(userMapper::map) + .flatMap(keycloakUser -> remoteService.getUserByName(keycloakUser.getUsername(), namespace)) .ifPresent(keycloakUser -> remoteService.deleteUser(keycloakUser.getId(), namespace)); } } diff --git a/src/test/java/de/ozgcloud/operator/keycloak/user/KeycloakUserMapperTest.java b/src/test/java/de/ozgcloud/operator/keycloak/user/KeycloakUserMapperTest.java index 6644ddc..9f3aa82 100644 --- a/src/test/java/de/ozgcloud/operator/keycloak/user/KeycloakUserMapperTest.java +++ b/src/test/java/de/ozgcloud/operator/keycloak/user/KeycloakUserMapperTest.java @@ -22,74 +22,74 @@ class KeycloakUserMapperTest { private KeycloakUserMapper mapper = Mappers.getMapper(KeycloakUserMapper.class); @Nested - class TesttoUserRepresentation { + class TestMap { @Test void shouldMapUsername() { - var keycloakUser = mapper.toUserRepresentation(OzgKeycloakUserSpecTestFactory.create()); + var keycloakUser = mapper.map(OzgKeycloakUserSpecTestFactory.create()); assertThat(keycloakUser.getUsername()).isEqualTo(KeycloakUserSpecUserTestFactory.USERNAME); } @Test void shouldMapFirstname() { - var keycloakUser = mapper.toUserRepresentation(OzgKeycloakUserSpecTestFactory.create()); + var keycloakUser = mapper.map(OzgKeycloakUserSpecTestFactory.create()); assertThat(keycloakUser.getFirstName()).isEqualTo(KeycloakUserSpecUserTestFactory.FIRSTNAME); } @Test void shouldMapLastname() { - var keycloakUser = mapper.toUserRepresentation(OzgKeycloakUserSpecTestFactory.create()); + var keycloakUser = mapper.map(OzgKeycloakUserSpecTestFactory.create()); assertThat(keycloakUser.getLastName()).isEqualTo(KeycloakUserSpecUserTestFactory.LASTNAME); } @Test void userShouldBeEnabled() { - var keycloakUser = mapper.toUserRepresentation(OzgKeycloakUserSpecTestFactory.create()); + var keycloakUser = mapper.map(OzgKeycloakUserSpecTestFactory.create()); assertThat(keycloakUser.isEnabled()).isTrue(); } @Test void userShouldBeEmailVerified() { - var keycloakUser = mapper.toUserRepresentation(OzgKeycloakUserSpecTestFactory.create()); + var keycloakUser = mapper.map(OzgKeycloakUserSpecTestFactory.create()); assertThat(keycloakUser.isEmailVerified()).isTrue(); } @Test void shouldMapEmail() { - var keycloakUser = mapper.toUserRepresentation(OzgKeycloakUserSpecTestFactory.create()); + var keycloakUser = mapper.map(OzgKeycloakUserSpecTestFactory.create()); assertThat(keycloakUser.getEmail()).isEqualTo(KeycloakUserSpecUserTestFactory.EMAIL); } @Test void shouldSetPasswordType() { - var keycloakUser = mapper.toUserRepresentation(OzgKeycloakUserSpecTestFactory.create()); + var keycloakUser = mapper.map(OzgKeycloakUserSpecTestFactory.create()); assertThat(keycloakUser.getCredentials().get(0).getType()).isEqualTo(CredentialRepresentation.PASSWORD); } @Test void shouldSetPassword() { - var keycloakUser = mapper.toUserRepresentation(OzgKeycloakUserSpecTestFactory.create()); + var keycloakUser = mapper.map(OzgKeycloakUserSpecTestFactory.create()); assertThat(keycloakUser.getCredentials().get(0).getValue()).isEqualTo(KeycloakUserSpecUserTestFactory.PASSWORD); } @Test void shouldSetPasswordNotTemporary() { - var keycloakUser = mapper.toUserRepresentation(OzgKeycloakUserSpecTestFactory.create()); + var keycloakUser = mapper.map(OzgKeycloakUserSpecTestFactory.create()); assertThat(keycloakUser.getCredentials().get(0).isTemporary()).isFalse(); } @Test void shouldMapGroups() { - var keycloakUser = mapper.toUserRepresentation(OzgKeycloakUserSpecTestFactory.create()); + var keycloakUser = mapper.map(OzgKeycloakUserSpecTestFactory.create()); assertThat(keycloakUser.getGroups()).isNotEmpty().contains(KeycloakUserSpecUserTestFactory.GROUP1.getName()) .contains(KeycloakUserSpecUserTestFactory.GROUP2.getName()); @@ -97,7 +97,7 @@ class KeycloakUserMapperTest { @Test void shouldMapClientRoles() { - var keycloakUser = mapper.toUserRepresentation(OzgKeycloakUserSpecTestFactory.create()); + var keycloakUser = mapper.map(OzgKeycloakUserSpecTestFactory.create()); assertThat(keycloakUser.getClientRoles()).hasSize(1); } @@ -118,7 +118,7 @@ class KeycloakUserMapperTest { @Test void shouldContainClientRoles() { - var keycloakUser = mapper.toUserRepresentation(OzgKeycloakUserSpecTestFactory.create()); + var keycloakUser = mapper.map(OzgKeycloakUserSpecTestFactory.create()); assertThat(keycloakUser.getClientRoles().get(KeycloakUserSpecUserTestFactory.CLIENT_NAME)).hasSize(2) .contains(KeycloakUserSpecUserTestFactory.ROLE1.getRoleName(), KeycloakUserSpecUserTestFactory.ROLE2.getRoleName()); @@ -148,4 +148,31 @@ class KeycloakUserMapperTest { assertThat(user.getClientRoles()).isNotNull().isEmpty(); } } + + @Nested + class TestUpdate { + + private static final String FIRSTNAME2 = "UpdateFirstName"; + private static final String USERID = "UserId"; + + @Test + void shouldMapFirstname() { + var userRepresentation = UserRepresentationTestFactory.create(); + userRepresentation.setFirstName(FIRSTNAME2); + + var keycloakUser = mapper.update(userRepresentation, OzgKeycloakUserSpecTestFactory.create()); + + assertThat(keycloakUser.getFirstName()).isEqualTo(OzgKeycloakUserSpecTestFactory.KEYCLOAK_USER.getFirstName()); + } + + @Test + void shouldKeepUserId() { + var userRepresentation = UserRepresentationTestFactory.create(); + userRepresentation.setId(USERID); + + var keycloakUser = mapper.update(userRepresentation, OzgKeycloakUserSpecTestFactory.create()); + + assertThat(keycloakUser.getId()).isEqualTo(USERID); + } + } } diff --git a/src/test/java/de/ozgcloud/operator/keycloak/user/KeycloakUserReconcilerTest.java b/src/test/java/de/ozgcloud/operator/keycloak/user/KeycloakUserReconcilerTest.java index 1f88257..c2521af 100644 --- a/src/test/java/de/ozgcloud/operator/keycloak/user/KeycloakUserReconcilerTest.java +++ b/src/test/java/de/ozgcloud/operator/keycloak/user/KeycloakUserReconcilerTest.java @@ -32,7 +32,7 @@ class KeycloakUserReconcilerTest { conciler.reconcile(user, null); - verify(userService).addUser(user.getSpec(), OzgKeycloakUserTestFactory.METADATA_NAMESPACE); + verify(userService).createOrUpdateUser(user.getSpec(), OzgKeycloakUserTestFactory.METADATA_NAMESPACE); } @Test @@ -47,7 +47,7 @@ class KeycloakUserReconcilerTest { @Test void shouldSetErrorStatusOnException() { OzgKeycloakUser user = OzgKeycloakUserTestFactory.create(); - doThrow(new RuntimeException()).when(userService).addUser(any(), any()); + doThrow(new RuntimeException()).when(userService).createOrUpdateUser(any(), any()); conciler.reconcile(user, null); diff --git a/src/test/java/de/ozgcloud/operator/keycloak/user/KeycloakUserRemoteServiceTest.java b/src/test/java/de/ozgcloud/operator/keycloak/user/KeycloakUserRemoteServiceTest.java index b2f8722..0cfd7e3 100644 --- a/src/test/java/de/ozgcloud/operator/keycloak/user/KeycloakUserRemoteServiceTest.java +++ b/src/test/java/de/ozgcloud/operator/keycloak/user/KeycloakUserRemoteServiceTest.java @@ -189,15 +189,26 @@ class KeycloakUserRemoteServiceTest { } @Nested - class TestUserExistsByName { + class TestUpdateUser { + + private final static String USERID = "UserId"; + + @BeforeEach + void init() { + when(keycloakClient.getKeycloak()).thenReturn(keycloak); + when(keycloak.realm(TEST_REALM)).thenReturn(realmResource); + when(realmResource.users()).thenReturn(usersResource); + when(usersResource.get(USERID)).thenReturn(userResource); + } @Test - void shouldReturnTrue() { + void shouldCallKeycloakApiUpdateUser() { + var user = UserRepresentationTestFactory.create(); + user.setId(USERID); - doReturn(Optional.of(UserRepresentationTestFactory.create())).when(userRemoteService) - .getUserByName(UserRepresentationTestFactory.USERNAME, TEST_REALM); + userRemoteService.updateUser(user, TEST_REALM); - assertThat(userRemoteService.userExistsByName(UserRepresentationTestFactory.USERNAME, TEST_REALM)).isTrue(); + verify(userResource).update(user); } } diff --git a/src/test/java/de/ozgcloud/operator/keycloak/user/KeycloakUserServiceTest.java b/src/test/java/de/ozgcloud/operator/keycloak/user/KeycloakUserServiceTest.java index 8c0cecf..4a8ec61 100644 --- a/src/test/java/de/ozgcloud/operator/keycloak/user/KeycloakUserServiceTest.java +++ b/src/test/java/de/ozgcloud/operator/keycloak/user/KeycloakUserServiceTest.java @@ -7,6 +7,7 @@ import java.util.Optional; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import org.keycloak.representations.idm.UserRepresentation; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.Spy; @@ -34,44 +35,41 @@ class KeycloakUserServiceTest { var testUser = OzgKeycloakUserSpecTestFactory.create(); - userService.addUser(testUser, TEST_NAMESPACE); + userService.createOrUpdateUser(testUser, TEST_NAMESPACE); - verify(userMapper).toUserRepresentation(testUser); + verify(userMapper).map(testUser); } @Test - void shouldCallUserRemoteServiceUserExists() { - + void shouldCallUserRemoteServiceGetUserByName() { var userRepresentation = UserRepresentationTestFactory.create(); - when(userMapper.toUserRepresentation(any())).thenReturn(userRepresentation); - userService.addUser(OzgKeycloakUserSpecTestFactory.create(), TEST_NAMESPACE); + userService.createOrUpdateUser(OzgKeycloakUserSpecTestFactory.create(), TEST_NAMESPACE); - verify(userRemoteService).userExistsByName(eq(userRepresentation.getUsername()), eq(TEST_NAMESPACE)); + verify(userRemoteService).getUserByName(eq(userRepresentation.getUsername()), eq(TEST_NAMESPACE)); } @Test - void shouldCallUserRemoteServiceAddUser() { - - var userRepresentation = UserRepresentationTestFactory.create(); - when(userMapper.toUserRepresentation(any())).thenReturn(userRepresentation); - when(userRemoteService.userExistsByName(any(), anyString())).thenReturn(false); + void shouldCreateUserIfNotExists() { + when(userRemoteService.getUserByName(OzgKeycloakUserSpecTestFactory.KEYCLOAK_USER.getUsername(), TEST_NAMESPACE)) + .thenReturn(Optional.empty()); + when(userMapper.map(any())).thenReturn(mock(UserRepresentation.class)); - userService.addUser(OzgKeycloakUserSpecTestFactory.create(), TEST_NAMESPACE); + userService.createOrUpdateUser(OzgKeycloakUserSpecTestFactory.create(), TEST_NAMESPACE); - verify(userRemoteService).createUser(eq(userRepresentation), eq(TEST_NAMESPACE)); + verify(userRemoteService).createUser(any(UserRepresentation.class), eq(TEST_NAMESPACE)); } @Test - void shouldNotAddUserIfUserExists() { - - var userRepresentation = UserRepresentationTestFactory.create(); - when(userMapper.toUserRepresentation(any())).thenReturn(userRepresentation); - when(userRemoteService.userExistsByName(any(), anyString())).thenReturn(true); + void shouldUpdateUserIfExists() { + var userRepresentation = mock(UserRepresentation.class); + when(userRemoteService.getUserByName(OzgKeycloakUserSpecTestFactory.KEYCLOAK_USER.getUsername(), TEST_NAMESPACE)) + .thenReturn(Optional.of(userRepresentation)); + when(userMapper.update(eq(userRepresentation), any())).thenReturn(userRepresentation); - userService.addUser(OzgKeycloakUserSpecTestFactory.create(), TEST_NAMESPACE); + userService.createOrUpdateUser(OzgKeycloakUserSpecTestFactory.create(), TEST_NAMESPACE); - verify(userRemoteService, never()).createUser(eq(userRepresentation), eq(TEST_NAMESPACE)); + verify(userRemoteService).updateUser(eq(userRepresentation), eq(TEST_NAMESPACE)); } } @@ -85,14 +83,14 @@ class KeycloakUserServiceTest { userService.deleteUser(testUser, TEST_NAMESPACE); - verify(userMapper).toUserRepresentation(testUser); + verify(userMapper).map(testUser); } @Test void shouldCallUserRemoteServiceGetUser() { var userRepresentation = UserRepresentationTestFactory.create(); - when(userMapper.toUserRepresentation(any())).thenReturn(userRepresentation); + when(userMapper.map(any())).thenReturn(userRepresentation); userService.deleteUser(OzgKeycloakUserSpecTestFactory.create(), TEST_NAMESPACE); @@ -103,7 +101,7 @@ class KeycloakUserServiceTest { void shouldCallUserRemoteServiceDeleteUser() { var userRepresentation = UserRepresentationTestFactory.create(); - when(userMapper.toUserRepresentation(any())).thenReturn(userRepresentation); + when(userMapper.map(any())).thenReturn(userRepresentation); when(userRemoteService.getUserByName(UserRepresentationTestFactory.USERNAME, TEST_NAMESPACE)).thenReturn(Optional.of(userRepresentation)); userRepresentation.setId(TEST_USERID); diff --git a/src/test/java/de/ozgcloud/operator/keycloak/user/UserRepresentationTestFactory.java b/src/test/java/de/ozgcloud/operator/keycloak/user/UserRepresentationTestFactory.java index c899ff2..530787b 100644 --- a/src/test/java/de/ozgcloud/operator/keycloak/user/UserRepresentationTestFactory.java +++ b/src/test/java/de/ozgcloud/operator/keycloak/user/UserRepresentationTestFactory.java @@ -1,5 +1,6 @@ package de.ozgcloud.operator.keycloak.user; +import java.util.HashMap; import java.util.List; import java.util.Map; @@ -19,7 +20,7 @@ public class UserRepresentationTestFactory { user.setUsername(USERNAME); user.setFirstName(FIRSTNAME); user.setLastName(LASTNAME); - user.setClientRoles(Map.of(CLIENT_NAME, List.of(ROLE1))); + user.setClientRoles(new HashMap<>(Map.of(CLIENT_NAME, List.of(ROLE1)))); return user; } } -- GitLab