From ee54a220f496fcfd4420b40f7923ba3af0979f09 Mon Sep 17 00:00:00 2001 From: Mykhailo Kuznietsov Date: Tue, 3 Apr 2018 12:10:20 +0000 Subject: [PATCH] Fix recreation of user in Che DB, when it has been recreated on Keycloak (#9280) --- .../che-multiuser-keycloak-server/pom.xml | 12 ++ ...eycloakEnvironmentInitalizationFilter.java | 58 +----- .../keycloak/server/KeycloakUserManager.java | 164 ++++++++++++++++ .../server/dao/KeycloakProfileDao.java | 2 +- .../server/deploy/KeycloakModule.java | 3 + ...oakEnvironmentInitalizationFilterTest.java | 62 +----- .../server/KeycloakUserManagerTest.java | 185 ++++++++++++++++++ .../{logback.xml => logback-test.xml} | 0 .../che/api/user/server/UserManager.java | 10 +- 9 files changed, 375 insertions(+), 121 deletions(-) create mode 100644 multiuser/keycloak/che-multiuser-keycloak-server/src/main/java/org/eclipse/che/multiuser/keycloak/server/KeycloakUserManager.java create mode 100644 multiuser/keycloak/che-multiuser-keycloak-server/src/test/java/org/eclipse/che/multiuser/keycloak/server/KeycloakUserManagerTest.java rename multiuser/keycloak/che-multiuser-keycloak-server/src/test/resources/{logback.xml => logback-test.xml} (100%) diff --git a/multiuser/keycloak/che-multiuser-keycloak-server/pom.xml b/multiuser/keycloak/che-multiuser-keycloak-server/pom.xml index 3210d38bf7..4c53d55c54 100644 --- a/multiuser/keycloak/che-multiuser-keycloak-server/pom.xml +++ b/multiuser/keycloak/che-multiuser-keycloak-server/pom.xml @@ -49,6 +49,10 @@ com.google.inject guice + + com.google.inject.extensions + guice-persist + com.google.inject.extensions guice-servlet @@ -69,6 +73,10 @@ javax.ws.rs javax.ws.rs-api + + org.eclipse.che.core + che-core-api-account + org.eclipse.che.core che-core-api-auth-shared @@ -113,6 +121,10 @@ org.eclipse.che.multiuser che-multiuser-keycloak-shared + + org.eclipse.che.multiuser + che-multiuser-personal-account + org.slf4j slf4j-api diff --git a/multiuser/keycloak/che-multiuser-keycloak-server/src/main/java/org/eclipse/che/multiuser/keycloak/server/KeycloakEnvironmentInitalizationFilter.java b/multiuser/keycloak/che-multiuser-keycloak-server/src/main/java/org/eclipse/che/multiuser/keycloak/server/KeycloakEnvironmentInitalizationFilter.java index 3b6230a2c4..ca07eb035b 100644 --- a/multiuser/keycloak/che-multiuser-keycloak-server/src/main/java/org/eclipse/che/multiuser/keycloak/server/KeycloakEnvironmentInitalizationFilter.java +++ b/multiuser/keycloak/che-multiuser-keycloak-server/src/main/java/org/eclipse/che/multiuser/keycloak/server/KeycloakEnvironmentInitalizationFilter.java @@ -10,14 +10,10 @@ */ package org.eclipse.che.multiuser.keycloak.server; -import static java.util.Collections.emptyList; -import static org.eclipse.che.commons.lang.NameGenerator.generate; - import io.jsonwebtoken.Claims; import io.jsonwebtoken.Jwt; import java.io.IOException; import java.security.Principal; -import java.util.Optional; import javax.inject.Inject; import javax.inject.Singleton; import javax.servlet.FilterChain; @@ -28,11 +24,8 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequestWrapper; import javax.servlet.http.HttpSession; import org.eclipse.che.api.core.ConflictException; -import org.eclipse.che.api.core.NotFoundException; import org.eclipse.che.api.core.ServerException; import org.eclipse.che.api.core.model.user.User; -import org.eclipse.che.api.user.server.UserManager; -import org.eclipse.che.api.user.server.model.impl.UserImpl; import org.eclipse.che.commons.auth.token.RequestTokenExtractor; import org.eclipse.che.commons.env.EnvironmentContext; import org.eclipse.che.commons.subject.Subject; @@ -48,13 +41,13 @@ import org.eclipse.che.multiuser.api.permission.server.PermissionChecker; @Singleton public class KeycloakEnvironmentInitalizationFilter extends AbstractKeycloakFilter { - private final UserManager userManager; + private final KeycloakUserManager userManager; private final RequestTokenExtractor tokenExtractor; private final PermissionChecker permissionChecker; @Inject public KeycloakEnvironmentInitalizationFilter( - UserManager userManager, + KeycloakUserManager userManager, RequestTokenExtractor tokenExtractor, PermissionChecker permissionChecker) { this.userManager = userManager; @@ -84,7 +77,7 @@ public class KeycloakEnvironmentInitalizationFilter extends AbstractKeycloakFilt try { User user = - getOrCreateUser( + userManager.getOrCreateUser( claims.getSubject(), claims.get("email", String.class), claims.get("preferred_username", String.class)); @@ -106,51 +99,6 @@ public class KeycloakEnvironmentInitalizationFilter extends AbstractKeycloakFilt } } - private User getOrCreateUser(String id, String email, String username) - throws ServerException, ConflictException { - Optional user = getUser(id); - if (!user.isPresent()) { - synchronized (this) { - user = getUser(id); - if (!user.isPresent()) { - final UserImpl cheUser = new UserImpl(id, email, username, generate("", 12), emptyList()); - try { - return userManager.create(cheUser, false); - } catch (ConflictException ex) { - cheUser.setName(generate(cheUser.getName(), 4)); - return userManager.create(cheUser, false); - } - } - } - } - return actualizeUser(user.get(), email); - } - /** Performs check that emails in JWT and local DB are match, and synchronize them otherwise */ - private User actualizeUser(User actualUser, String email) throws ServerException { - if (actualUser.getEmail().equals(email)) { - return actualUser; - } - UserImpl update = new UserImpl(actualUser); - update.setEmail(email); - try { - userManager.update(update); - } catch (NotFoundException e) { - throw new ServerException("Unable to actualize user email. User not found.", e); - } catch (ConflictException e) { - throw new ServerException( - "Unable to actualize user email. Another user with such email exists", e); - } - return update; - } - - private Optional getUser(String id) throws ServerException { - try { - return Optional.of(userManager.getById(id)); - } catch (NotFoundException e) { - return Optional.empty(); - } - } - private HttpServletRequest addUserInRequest( final HttpServletRequest httpRequest, final Subject subject) { return new HttpServletRequestWrapper(httpRequest) { diff --git a/multiuser/keycloak/che-multiuser-keycloak-server/src/main/java/org/eclipse/che/multiuser/keycloak/server/KeycloakUserManager.java b/multiuser/keycloak/che-multiuser-keycloak-server/src/main/java/org/eclipse/che/multiuser/keycloak/server/KeycloakUserManager.java new file mode 100644 index 0000000000..c88e5ef4d5 --- /dev/null +++ b/multiuser/keycloak/che-multiuser-keycloak-server/src/main/java/org/eclipse/che/multiuser/keycloak/server/KeycloakUserManager.java @@ -0,0 +1,164 @@ +/* + * Copyright (c) 2012-2018 Red Hat, Inc. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ +package org.eclipse.che.multiuser.keycloak.server; + +import static java.lang.System.currentTimeMillis; +import static java.util.Collections.emptyList; +import static org.eclipse.che.commons.lang.NameGenerator.generate; + +import com.google.common.collect.ImmutableMap; +import com.google.inject.Inject; +import com.google.inject.persist.Transactional; +import java.util.Optional; +import javax.inject.Named; +import javax.inject.Singleton; +import org.eclipse.che.account.api.AccountManager; +import org.eclipse.che.api.core.ApiException; +import org.eclipse.che.api.core.ConflictException; +import org.eclipse.che.api.core.NotFoundException; +import org.eclipse.che.api.core.ServerException; +import org.eclipse.che.api.core.model.user.User; +import org.eclipse.che.api.core.notification.EventService; +import org.eclipse.che.api.user.server.event.BeforeUserRemovedEvent; +import org.eclipse.che.api.user.server.event.PostUserPersistedEvent; +import org.eclipse.che.api.user.server.model.impl.UserImpl; +import org.eclipse.che.api.user.server.spi.PreferenceDao; +import org.eclipse.che.api.user.server.spi.ProfileDao; +import org.eclipse.che.api.user.server.spi.UserDao; +import org.eclipse.che.multiuser.api.account.personal.PersonalAccountUserManager; + +/** + * Extension of User Manager, providing utility operations related to Keycloak User management, and + * overriding create/remove operations to be compatible with {@link + * org.eclipse.che.multiuser.keycloak.server.dao.KeycloakProfileDao} + * + * @author Mykhailo Kuznietsov + */ +@Singleton +public class KeycloakUserManager extends PersonalAccountUserManager { + + @Inject + public KeycloakUserManager( + UserDao userDao, + ProfileDao profileDao, + PreferenceDao preferencesDao, + AccountManager accountManager, + EventService eventService, + @Named("che.auth.reserved_user_names") String[] reservedNames) { + super(userDao, profileDao, preferencesDao, reservedNames, accountManager, eventService); + } + + @Transactional(rollbackOn = {RuntimeException.class, ApiException.class}) + protected void doCreate(UserImpl user, boolean isTemporary) + throws ConflictException, ServerException { + userDao.create(user); + eventService.publish(new PostUserPersistedEvent(new UserImpl(user))).propagateException(); + preferencesDao.setPreferences( + user.getId(), + ImmutableMap.of( + "temporary", Boolean.toString(isTemporary), + "codenvy:created", Long.toString(currentTimeMillis()))); + } + + @Transactional(rollbackOn = {RuntimeException.class, ApiException.class}) + protected void doRemove(String id) throws ServerException { + UserImpl user; + try { + user = userDao.getById(id); + } catch (NotFoundException ignored) { + return; + } + preferencesDao.remove(id); + eventService.publish(new BeforeUserRemovedEvent(user)).propagateException(); + userDao.remove(id); + } + + /** + * Method is used to retrieve user object from Che DB for given user {@code id}, {@code email}, + * and {@code username}. Various actualization operations may be performed: + * + *

- if user is found in Che DB by the given {@code id}, then it will check, if it's email, + * matches the {@code email} , and update it in DB if necessary. + * + *

- if user is not found in Che DB by the given {@code id} , then attempt to create one. But + * also, it will attempt to get one by {@code email}. If such is found, he will be removed. That + * way, there will be no conflict with existing user id or email upon recreation. In case of + * conflict with user name, it may be prepended randomized symbols + * + * @param id - user id from + * @param email - user email + * @param username - user name + * @return user object from Che Database, with all needed actualization operations performed on + * him + * @throws ServerException if this exception during user creation, removal, or retrieval + * @throws ConflictException if this exception occurs during user creation or removal + */ + public User getOrCreateUser(String id, String email, String username) + throws ServerException, ConflictException { + Optional userById = getUserById(id); + if (!userById.isPresent()) { + synchronized (this) { + userById = getUserById(id); + if (!userById.isPresent()) { + Optional userByEmail = getUserByEmail(email); + if (userByEmail.isPresent()) { + remove(userByEmail.get().getId()); + } + final UserImpl cheUser = new UserImpl(id, email, username, generate("", 12), emptyList()); + try { + return create(cheUser, false); + } catch (ConflictException ex) { + cheUser.setName(generate(cheUser.getName(), 4)); + return create(cheUser, false); + } + } + } + } + return actualizeUserEmail(userById.get(), email); + } + + /** + * Performs check that {@code email} matches with the one in local DB, and synchronize them + * otherwise + */ + private User actualizeUserEmail(User actualUser, String email) throws ServerException { + if (actualUser.getEmail().equals(email)) { + return actualUser; + } + UserImpl update = new UserImpl(actualUser); + update.setEmail(email); + try { + update(update); + } catch (NotFoundException e) { + throw new ServerException("Unable to actualize user email. User not found.", e); + } catch (ConflictException e) { + throw new ServerException( + "Unable to actualize user email. Another user with such email exists", e); + } + return update; + } + + private Optional getUserById(String id) throws ServerException { + try { + return Optional.of(getById(id)); + } catch (NotFoundException e) { + return Optional.empty(); + } + } + + private Optional getUserByEmail(String email) throws ServerException { + try { + return Optional.of(getByEmail(email)); + } catch (NotFoundException e) { + return Optional.empty(); + } + } +} diff --git a/multiuser/keycloak/che-multiuser-keycloak-server/src/main/java/org/eclipse/che/multiuser/keycloak/server/dao/KeycloakProfileDao.java b/multiuser/keycloak/che-multiuser-keycloak-server/src/main/java/org/eclipse/che/multiuser/keycloak/server/dao/KeycloakProfileDao.java index db5c73084d..ee03b24c30 100644 --- a/multiuser/keycloak/che-multiuser-keycloak-server/src/main/java/org/eclipse/che/multiuser/keycloak/server/dao/KeycloakProfileDao.java +++ b/multiuser/keycloak/che-multiuser-keycloak-server/src/main/java/org/eclipse/che/multiuser/keycloak/server/dao/KeycloakProfileDao.java @@ -51,7 +51,7 @@ public class KeycloakProfileDao implements ProfileDao { @Override public void create(ProfileImpl profile) throws ServerException, ConflictException { - // this method intentionally left blank + throw new ServerException("Given operation doesn't supported on current configured storage."); } @Override diff --git a/multiuser/keycloak/che-multiuser-keycloak-server/src/main/java/org/eclipse/che/multiuser/keycloak/server/deploy/KeycloakModule.java b/multiuser/keycloak/che-multiuser-keycloak-server/src/main/java/org/eclipse/che/multiuser/keycloak/server/deploy/KeycloakModule.java index 1fb71b0672..9adc0596cf 100644 --- a/multiuser/keycloak/che-multiuser-keycloak-server/src/main/java/org/eclipse/che/multiuser/keycloak/server/deploy/KeycloakModule.java +++ b/multiuser/keycloak/che-multiuser-keycloak-server/src/main/java/org/eclipse/che/multiuser/keycloak/server/deploy/KeycloakModule.java @@ -14,8 +14,10 @@ import com.google.inject.AbstractModule; import org.eclipse.che.api.core.rest.HttpJsonRequestFactory; import org.eclipse.che.api.user.server.TokenValidator; import org.eclipse.che.api.user.server.spi.ProfileDao; +import org.eclipse.che.multiuser.api.account.personal.PersonalAccountUserManager; import org.eclipse.che.multiuser.keycloak.server.KeycloakConfigurationService; import org.eclipse.che.multiuser.keycloak.server.KeycloakTokenValidator; +import org.eclipse.che.multiuser.keycloak.server.KeycloakUserManager; import org.eclipse.che.multiuser.keycloak.server.dao.KeycloakProfileDao; import org.eclipse.che.multiuser.keycloak.server.oauth2.KeycloakOAuthAuthenticationService; @@ -30,5 +32,6 @@ public class KeycloakModule extends AbstractModule { bind(KeycloakOAuthAuthenticationService.class); bind(ProfileDao.class).to(KeycloakProfileDao.class); + bind(PersonalAccountUserManager.class).to(KeycloakUserManager.class); } } diff --git a/multiuser/keycloak/che-multiuser-keycloak-server/src/test/java/org/eclipse/che/multiuser/keycloak/server/KeycloakEnvironmentInitalizationFilterTest.java b/multiuser/keycloak/che-multiuser-keycloak-server/src/test/java/org/eclipse/che/multiuser/keycloak/server/KeycloakEnvironmentInitalizationFilterTest.java index 457d1f4b6e..7b8a669364 100644 --- a/multiuser/keycloak/che-multiuser-keycloak-server/src/test/java/org/eclipse/che/multiuser/keycloak/server/KeycloakEnvironmentInitalizationFilterTest.java +++ b/multiuser/keycloak/che-multiuser-keycloak-server/src/test/java/org/eclipse/che/multiuser/keycloak/server/KeycloakEnvironmentInitalizationFilterTest.java @@ -11,7 +11,6 @@ package org.eclipse.che.multiuser.keycloak.server; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.spy; @@ -30,9 +29,6 @@ import javax.servlet.FilterChain; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpSession; -import org.eclipse.che.api.core.NotFoundException; -import org.eclipse.che.api.core.model.user.User; -import org.eclipse.che.api.user.server.UserManager; import org.eclipse.che.api.user.server.model.impl.UserImpl; import org.eclipse.che.commons.auth.token.RequestTokenExtractor; import org.eclipse.che.commons.env.EnvironmentContext; @@ -51,7 +47,7 @@ import org.testng.annotations.Test; @Listeners(value = {MockitoTestNGListener.class}) public class KeycloakEnvironmentInitalizationFilterTest { - @Mock private UserManager userManager; + @Mock private KeycloakUserManager userManager; @Mock private RequestTokenExtractor tokenExtractor; @Mock private PermissionChecker permissionChecker; @Mock private FilterChain chain; @@ -101,7 +97,7 @@ public class KeycloakEnvironmentInitalizationFilterTest { when(tokenExtractor.getToken(any(HttpServletRequest.class))).thenReturn("token2"); when(request.getAttribute("token")).thenReturn(createJwt()); when(session.getAttribute(eq("che_subject"))).thenReturn(existingSubject); - when(userManager.getById(anyString())).thenReturn(user); + when(userManager.getOrCreateUser(anyString(), anyString(), anyString())).thenReturn(user); EnvironmentContext context = spy(EnvironmentContext.getCurrent()); EnvironmentContext.setCurrent(context); @@ -119,60 +115,6 @@ public class KeycloakEnvironmentInitalizationFilterTest { assertEquals(expectedSubject.getUserName(), captor.getAllValues().get(1).getUserName()); } - @Test - public void shouldCreateUserIfNoneExists() throws Exception { - - UserImpl user = new UserImpl(); - DefaultJwt jwt = createJwt(); - user.setEmail((String) jwt.getBody().get("email")); - user.setId(jwt.getBody().getSubject()); - user.setName((String) jwt.getBody().get("preferred_username")); - - ArgumentCaptor captor = ArgumentCaptor.forClass(UserImpl.class); - - // given - when(tokenExtractor.getToken(any(HttpServletRequest.class))).thenReturn("token2"); - when(request.getScheme()).thenReturn("http"); - when(request.getSession()).thenReturn(session); - when(request.getAttribute("token")).thenReturn(jwt); - when(session.getAttribute(eq("che_subject"))).thenReturn(null); - when(userManager.getById(anyString())).thenThrow(NotFoundException.class); - when(userManager.create(any(User.class), anyBoolean())).thenReturn(user); - - // when - filter.doFilter(request, response, chain); - - // then - verify(session).setAttribute(eq("che_subject"), captor.capture()); - verify(userManager).create(captor.capture(), eq(false)); - assertEquals(jwt.getBody().getSubject(), captor.getValue().getId()); - assertEquals(jwt.getBody().get("email"), captor.getValue().getEmail()); - assertEquals(jwt.getBody().get("preferred_username"), captor.getValue().getName()); - } - - @Test - public void shouldUpdateUserWhenEmailsNotMatch() throws Exception { - - Subject existingSubject = new SubjectImpl("name", "id1", "token", false); - UserImpl user = new UserImpl("id2", "test2@test.com", "username2"); - DefaultJwt jwt = createJwt(); - - ArgumentCaptor captor = ArgumentCaptor.forClass(UserImpl.class); - - // given - when(tokenExtractor.getToken(any(HttpServletRequest.class))).thenReturn("token2"); - when(request.getAttribute("token")).thenReturn(jwt); - when(session.getAttribute(eq("che_subject"))).thenReturn(existingSubject); - when(userManager.getById(anyString())).thenReturn(user); - - // when - filter.doFilter(request, response, chain); - - // then - verify(userManager).update(captor.capture()); - assertEquals(jwt.getBody().get("email"), captor.getValue().getEmail()); - } - private DefaultJwt createJwt() { Map claimParams = new HashMap<>(); claimParams.put("email", "test@test.com"); diff --git a/multiuser/keycloak/che-multiuser-keycloak-server/src/test/java/org/eclipse/che/multiuser/keycloak/server/KeycloakUserManagerTest.java b/multiuser/keycloak/che-multiuser-keycloak-server/src/test/java/org/eclipse/che/multiuser/keycloak/server/KeycloakUserManagerTest.java new file mode 100644 index 0000000000..fb3b029db7 --- /dev/null +++ b/multiuser/keycloak/che-multiuser-keycloak-server/src/test/java/org/eclipse/che/multiuser/keycloak/server/KeycloakUserManagerTest.java @@ -0,0 +1,185 @@ +/* + * Copyright (c) 2012-2018 Red Hat, Inc. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ +package org.eclipse.che.multiuser.keycloak.server; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import static org.mockito.MockitoAnnotations.initMocks; +import static org.testng.Assert.assertNotEquals; +import static org.testng.AssertJUnit.assertEquals; + +import org.eclipse.che.account.api.AccountManager; +import org.eclipse.che.api.core.ConflictException; +import org.eclipse.che.api.core.NotFoundException; +import org.eclipse.che.api.core.model.user.User; +import org.eclipse.che.api.core.notification.EventService; +import org.eclipse.che.api.user.server.event.BeforeUserRemovedEvent; +import org.eclipse.che.api.user.server.event.PostUserPersistedEvent; +import org.eclipse.che.api.user.server.model.impl.UserImpl; +import org.eclipse.che.api.user.server.spi.PreferenceDao; +import org.eclipse.che.api.user.server.spi.ProfileDao; +import org.eclipse.che.api.user.server.spi.UserDao; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.testng.MockitoTestNGListener; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Listeners; +import org.testng.annotations.Test; + +/** @author Mykhailo Kuznietsov */ +@Listeners(value = {MockitoTestNGListener.class}) +public class KeycloakUserManagerTest { + + @Mock private UserDao userDao; + @Mock private ProfileDao profileDao; + @Mock private PreferenceDao preferenceDao; + @Mock private AccountManager accountManager; + @Mock private EventService eventService; + @Mock private PostUserPersistedEvent postUserPersistedEvent; + @Mock private BeforeUserRemovedEvent beforeUserRemovedEvent; + + KeycloakUserManager keycloakUserManager; + + @BeforeMethod + public void setUp() { + initMocks(this); + keycloakUserManager = + new KeycloakUserManager( + userDao, profileDao, preferenceDao, accountManager, eventService, new String[] {}); + + when(eventService.publish(any())) + .thenAnswer( + invocationOnMock -> { + Object arg = invocationOnMock.getArguments()[0]; + if (arg instanceof BeforeUserRemovedEvent) { + return beforeUserRemovedEvent; + } else { + return postUserPersistedEvent; + } + }); + } + + @Test + public void shouldReturnExistingUser() throws Exception { + UserImpl userImpl = new UserImpl("id", "user@mail.com", "name"); + when(userDao.getById(eq("id"))).thenReturn(userImpl); + + User user = keycloakUserManager.getOrCreateUser("id", "user@mail.com", "name"); + + verify(userDao).getById("id"); + assertEquals("id", user.getId()); + assertEquals("user@mail.com", user.getEmail()); + assertEquals("name", user.getName()); + } + + @Test + public void shouldReturnUserAndUpdateHisEmail() throws Exception { + // given + ArgumentCaptor captor = ArgumentCaptor.forClass(UserImpl.class); + UserImpl userImpl = new UserImpl("id", "user@mail.com", "name"); + when(userDao.getById(eq("id"))).thenReturn(userImpl); + + // when + User user = keycloakUserManager.getOrCreateUser("id", "new@mail.com", "name"); + + // then + verify(userDao, times(2)).getById("id"); + verify(userDao).update(captor.capture()); + assertEquals("id", captor.getValue().getId()); + assertEquals("new@mail.com", captor.getValue().getEmail()); + assertEquals("name", captor.getValue().getName()); + + assertEquals("id", user.getId()); + assertEquals("new@mail.com", user.getEmail()); + assertEquals("name", user.getName()); + } + + @Test + public void shoudCreateNewUserIfHeIsNotFoundByIdOrEmail() throws Exception { + // given + ArgumentCaptor captor = ArgumentCaptor.forClass(UserImpl.class); + when(userDao.getById(eq("id"))).thenThrow(NotFoundException.class); + when(userDao.getByEmail(eq("user@mail.com"))).thenThrow(NotFoundException.class); + + // when + keycloakUserManager.getOrCreateUser("id", "user@mail.com", "name"); + + // then + verify(userDao, times(2)).getById(eq("id")); + verify(userDao).getByEmail(eq("user@mail.com")); + + verify(userDao).create((captor.capture())); + assertEquals("id", captor.getValue().getId()); + assertEquals("user@mail.com", captor.getValue().getEmail()); + assertEquals("name", captor.getValue().getName()); + } + + @Test + public void shoudRecreateUserIfHeIsntFoundByIdButFoundByEmail() throws Exception { + // given + UserImpl newUserImpl = new UserImpl("id", "user@mail.com", "name"); + UserImpl oldUserImpl = new UserImpl("oldId", "user@mail.com", "name"); + ArgumentCaptor captor = ArgumentCaptor.forClass(UserImpl.class); + when(userDao.getById(eq("id"))).thenThrow(NotFoundException.class); + when(userDao.getByEmail(eq("user@mail.com"))).thenReturn(oldUserImpl); + + // when + keycloakUserManager.getOrCreateUser("id", "user@mail.com", "name"); + + // then + verify(userDao, times(2)).getById(eq("id")); + verify(userDao).getByEmail(eq("user@mail.com")); + + verify(userDao).remove(eq(oldUserImpl.getId())); + + verify(userDao).create((captor.capture())); + assertEquals(newUserImpl.getId(), captor.getValue().getId()); + assertEquals(newUserImpl.getEmail(), captor.getValue().getEmail()); + assertEquals(newUserImpl.getName(), captor.getValue().getName()); + } + + @Test + public void shoudRecreateUserWithDifferentNameIfConflictOccures() throws Exception { + // given + UserImpl newUserImpl = new UserImpl("id", "user@mail.com", "name"); + + ArgumentCaptor captor = ArgumentCaptor.forClass(UserImpl.class); + ArgumentCaptor captor2 = ArgumentCaptor.forClass(UserImpl.class); + when(userDao.getById(eq("id"))).thenThrow(NotFoundException.class); + when(userDao.getByEmail(eq("user@mail.com"))).thenThrow(NotFoundException.class); + doAnswer( + invocation -> { + if (((UserImpl) invocation.getArgument(0)).getName().equals("name")) { + throw new ConflictException(""); + } + return newUserImpl; + }) + .when(userDao) + .create(any()); + + // when + keycloakUserManager.getOrCreateUser("id", "user@mail.com", "name"); + + // then + verify(userDao, times(2)).getById(eq("id")); + verify(userDao).getByEmail(eq("user@mail.com")); + + verify(userDao, atLeastOnce()).create((captor.capture())); + assertEquals(newUserImpl.getId(), captor.getValue().getId()); + assertEquals(newUserImpl.getEmail(), captor.getValue().getEmail()); + assertNotEquals(newUserImpl.getName(), captor.getValue().getName()); + } +} diff --git a/multiuser/keycloak/che-multiuser-keycloak-server/src/test/resources/logback.xml b/multiuser/keycloak/che-multiuser-keycloak-server/src/test/resources/logback-test.xml similarity index 100% rename from multiuser/keycloak/che-multiuser-keycloak-server/src/test/resources/logback.xml rename to multiuser/keycloak/che-multiuser-keycloak-server/src/test/resources/logback-test.xml diff --git a/wsmaster/che-core-api-user/src/main/java/org/eclipse/che/api/user/server/UserManager.java b/wsmaster/che-core-api-user/src/main/java/org/eclipse/che/api/user/server/UserManager.java index 5d2e5a92ca..f72e905bfc 100644 --- a/wsmaster/che-core-api-user/src/main/java/org/eclipse/che/api/user/server/UserManager.java +++ b/wsmaster/che-core-api-user/src/main/java/org/eclipse/che/api/user/server/UserManager.java @@ -55,11 +55,11 @@ public class UserManager { public static final String PERSONAL_ACCOUNT = "personal"; - private final UserDao userDao; - private final ProfileDao profileDao; - private final PreferenceDao preferencesDao; - private final Set reservedNames; - private final EventService eventService; + protected final UserDao userDao; + protected final ProfileDao profileDao; + protected final PreferenceDao preferencesDao; + protected final Set reservedNames; + protected final EventService eventService; @Inject public UserManager(