diff --git a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/multiuser/oauth/OpenshiftTokenInitializationFilter.java b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/multiuser/oauth/OpenshiftTokenInitializationFilter.java index 8aa42afefc..4e4c4bcd19 100644 --- a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/multiuser/oauth/OpenshiftTokenInitializationFilter.java +++ b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/multiuser/oauth/OpenshiftTokenInitializationFilter.java @@ -91,19 +91,11 @@ public class OpenshiftTokenInitializationFilter protected Subject extractSubject(String token, io.fabric8.openshift.api.model.User osu) { try { ObjectMeta userMeta = osu.getMetadata(); - User user = - userManager.getOrCreateUser( - getUserId(osu), openshiftUserEmail(userMeta), userMeta.getName()); + User user = userManager.getOrCreateUser(getUserId(osu), userMeta.getName()); return new AuthorizedSubject( new SubjectImpl(user.getName(), user.getId(), token, false), permissionChecker); } catch (ServerException | ConflictException e) { throw new RuntimeException(e); } } - - protected String openshiftUserEmail(ObjectMeta userMeta) { - // OpenShift User does not have data about user's email. However, we need some email. For now, - // we can use fake email, but probably we will need to find better solution. - return userMeta.getName() + "@che"; - } } diff --git a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/multiuser/oauth/OpenshiftTokenInitializationFilterTest.java b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/multiuser/oauth/OpenshiftTokenInitializationFilterTest.java index 547c37f419..4c1f134a4d 100644 --- a/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/multiuser/oauth/OpenshiftTokenInitializationFilterTest.java +++ b/infrastructures/openshift/src/test/java/org/eclipse/che/workspace/infrastructure/openshift/multiuser/oauth/OpenshiftTokenInitializationFilterTest.java @@ -86,8 +86,7 @@ public class OpenshiftTokenInitializationFilterTest { when(openshiftUser.getMetadata()).thenReturn(openshiftUserMeta); when(openshiftUserMeta.getUid()).thenReturn(null); when(openshiftUserMeta.getName()).thenReturn(KUBE_ADMIN_USERNAME); - when(userManager.getOrCreateUser( - KUBE_ADMIN_USERNAME, KUBE_ADMIN_USERNAME + "@che", KUBE_ADMIN_USERNAME)) + when(userManager.getOrCreateUser(KUBE_ADMIN_USERNAME, KUBE_ADMIN_USERNAME)) .thenReturn( new UserImpl(KUBE_ADMIN_USERNAME, KUBE_ADMIN_USERNAME + "@che", KUBE_ADMIN_USERNAME)); @@ -106,7 +105,7 @@ public class OpenshiftTokenInitializationFilterTest { when(openshiftUser.getMetadata()).thenReturn(openshiftUserMeta); when(openshiftUserMeta.getUid()).thenReturn(USER_UID); when(openshiftUserMeta.getName()).thenReturn(USERNAME); - when(userManager.getOrCreateUser(USER_UID, USERNAME + "@che", USERNAME)) + when(userManager.getOrCreateUser(USER_UID, USERNAME)) .thenReturn(new UserImpl(USER_UID, USERNAME + "@che", USERNAME)); User u = openshiftTokenInitializationFilter.processToken(TOKEN).get(); 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 2eca6eb9a0..b7f52ee3e1 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 @@ -315,6 +315,44 @@ public class UserManager { return actualizeUserEmail(userById.get(), email); } + /** + * Method is used to retrieve user object from Che DB for given user {@code id} and {@code + * username} + * + *

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

- if user is not found in Che DB by the given {@code id} , then attempt to create one. In + * case of conflict with user name, it may be prepended randomized symbols + * + * @param id - user id from + * @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 username) + throws ServerException, ConflictException { + Optional userById = getUserById(id); + if (!userById.isPresent()) { + synchronized (this) { + userById = getUserById(id); + if (!userById.isPresent()) { + final UserImpl cheUser = + new UserImpl(id, username + "@che", username, generate("", 12), emptyList()); + try { + return create(cheUser, false); + } catch (ConflictException ex) { + cheUser.setName(generate(cheUser.getName(), 4)); + return create(cheUser, false); + } + } + } + } + return userById.get(); + } + @Transactional( rollbackOn = {RuntimeException.class, ServerException.class, ConflictException.class}) protected void doRemove(String id) throws ConflictException, ServerException { diff --git a/wsmaster/che-core-api-user/src/test/java/org/eclipse/che/api/user/server/UserManagerTest.java b/wsmaster/che-core-api-user/src/test/java/org/eclipse/che/api/user/server/UserManagerTest.java index 6289800171..6303b06d85 100644 --- a/wsmaster/che-core-api-user/src/test/java/org/eclipse/che/api/user/server/UserManagerTest.java +++ b/wsmaster/che-core-api-user/src/test/java/org/eclipse/che/api/user/server/UserManagerTest.java @@ -27,6 +27,7 @@ import static org.testng.Assert.fail; import java.util.Arrays; import java.util.Collections; import org.eclipse.che.api.core.ConflictException; +import org.eclipse.che.api.core.NotFoundException; import org.eclipse.che.api.core.Page; import org.eclipse.che.api.core.ServerException; import org.eclipse.che.api.core.model.user.User; @@ -434,4 +435,77 @@ public class UserManagerTest { assertTrue( firedEvents.getValue() instanceof PostUserPersistedEvent, "Not a PostUserPersistedEvent"); } + + @Test + public void shouldBeAbleToGetOrCreate_existed() throws Exception { + // given + final User user = + new UserImpl( + "identifier", + "test@email.com", + "testName", + "password", + Collections.singletonList("alias")); + when(manager.getById(user.getId())).thenReturn(user); + + // when + User actual = manager.getOrCreateUser("identifier", "testName@che", "testName"); + + // then + assertNotNull(actual); + assertEquals(actual.getEmail(), "testName@che"); + assertEquals(actual.getId(), "identifier"); + assertEquals(actual.getName(), "testName"); + } + + @Test + public void shouldBeAbleToGetOrCreate_nonexisted() throws Exception { + // given + when(manager.getById("identifier")).thenThrow(NotFoundException.class); + when(manager.getByEmail("testName@che")).thenThrow(NotFoundException.class); + + // when + User actual = manager.getOrCreateUser("identifier", "testName@che", "testName"); + // then + assertNotNull(actual); + assertEquals(actual.getEmail(), "testName@che"); + assertEquals(actual.getId(), "identifier"); + assertEquals(actual.getName(), "testName"); + } + + @Test + public void shouldBeAbleToGetOrCreateWithoutEmail_existed() throws Exception { + // given + final User user = + new UserImpl( + "identifier", + "test@email.com", + "testName", + "password", + Collections.singletonList("alias")); + when(manager.getById(user.getId())).thenReturn(user); + + // when + User actual = manager.getOrCreateUser("identifier", "testName"); + + // then + assertNotNull(actual); + assertEquals(actual.getEmail(), "test@email.com"); + assertEquals(actual.getId(), "identifier"); + assertEquals(actual.getName(), "testName"); + } + + @Test + public void shouldBeAbleToGetOrCreateWithoutEmail_nonexisted() throws Exception { + // given + when(manager.getById("identifier")).thenThrow(NotFoundException.class); + + // when + User actual = manager.getOrCreateUser("identifier", "testName"); + // then + assertNotNull(actual); + assertEquals(actual.getEmail(), "testName@che"); + assertEquals(actual.getId(), "identifier"); + assertEquals(actual.getName(), "testName"); + } }