fix: Do not override existed user's email with dummy on OpenShift (#198)
* fix: Do not override existed user's email with dummy on OpenShift Signed-off-by: Sergii Kabashniuk <skabashniuk@redhat.com>pull/197/head
parent
5a7c249897
commit
896eb01a63
|
|
@ -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";
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
|
|
|
|||
|
|
@ -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}
|
||||
*
|
||||
* <p>- if user is found in Che DB by the given {@code id}, then it will update it in DB if
|
||||
* necessary.
|
||||
*
|
||||
* <p>- 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<User> 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 {
|
||||
|
|
|
|||
|
|
@ -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");
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue