Fixed possible constraint violation with an existed organization. (#6674)

* Fixed possible constraint violation with an existed organization.

Usecase is following:
If we have already an organization with name, let's say "org"  and the new user also has name "org" then we will have constraint violation.
 In this case, we will try to create the user with name "org"+random string.
6.19.x
Sergii Kabashniuk 2017-10-11 16:21:59 +03:00 committed by GitHub
parent a6faa9a0bd
commit 1bea871d0a
4 changed files with 47 additions and 24 deletions

View File

@ -74,6 +74,10 @@
<groupId>org.eclipse.che.core</groupId>
<artifactId>che-core-commons-auth</artifactId>
</dependency>
<dependency>
<groupId>org.eclipse.che.core</groupId>
<artifactId>che-core-commons-lang</artifactId>
</dependency>
<dependency>
<groupId>org.eclipse.che.multiuser</groupId>
<artifactId>che-multiuser-api-authorization</artifactId>

View File

@ -11,6 +11,7 @@
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;
@ -79,15 +80,21 @@ public class KeycloakEnvironmentInitalizationFilter extends AbstractKeycloakFilt
throw new ServletException("Cannot detect or instantiate user.");
}
Claims claims = (Claims) jwtToken.getBody();
User user =
getOrCreateUser(
claims.getSubject(),
claims.get("email", String.class),
claims.get("preferred_username", String.class));
subject =
new AuthorizedSubject(
new SubjectImpl(user.getName(), user.getId(), token, false), permissionChecker);
session.setAttribute("che_subject", subject);
try {
User user =
getOrCreateUser(
claims.getSubject(),
claims.get("email", String.class),
claims.get("preferred_username", String.class));
subject =
new AuthorizedSubject(
new SubjectImpl(user.getName(), user.getId(), token, false), permissionChecker);
session.setAttribute("che_subject", subject);
} catch (ServerException | ConflictException e) {
throw new ServletException(
"Unable to identify user " + claims.getSubject() + " in Che database", e);
}
}
try {
@ -98,19 +105,20 @@ public class KeycloakEnvironmentInitalizationFilter extends AbstractKeycloakFilt
}
}
private synchronized User getOrCreateUser(String id, String email, String username)
throws ServletException {
private User getOrCreateUser(String id, String email, String username)
throws ServerException, ConflictException {
try {
return userManager.getById(id);
} catch (NotFoundException e) {
try {
final UserImpl cheUser = new UserImpl(id, email, username, "secret", emptyList());
return userManager.create(cheUser, false);
} catch (ServerException | ConflictException ex) {
throw new ServletException("Unable to create new user", ex);
synchronized (this) {
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);
}
}
} catch (ServerException e) {
throw new ServletException("Unable to get user", e);
}
}

View File

@ -14,7 +14,6 @@ import static java.lang.String.format;
import static java.util.Objects.requireNonNull;
import com.google.inject.persist.Transactional;
import java.util.Optional;
import javax.inject.Inject;
import javax.inject.Provider;
import javax.inject.Singleton;
@ -111,7 +110,9 @@ public class JpaAccountDao implements AccountDao {
@Transactional
protected void doCreate(AccountImpl account) {
managerProvider.get().persist(account);
final EntityManager manager = managerProvider.get();
manager.persist(account);
manager.flush();
}
@Transactional
@ -127,10 +128,12 @@ public class JpaAccountDao implements AccountDao {
}
@Transactional
protected Optional<AccountImpl> doRemove(String id) {
protected void doRemove(String id) {
final EntityManager manager = managerProvider.get();
final Optional<AccountImpl> account = Optional.ofNullable(manager.find(AccountImpl.class, id));
account.ifPresent(manager::remove);
return account;
AccountImpl account = manager.find(AccountImpl.class, id);
if (account != null) {
manager.remove(account);
manager.flush();
}
}
}

View File

@ -96,6 +96,14 @@ public class AccountDaoTest {
accountDao.update(account);
}
@Test(expectedExceptions = ConflictException.class)
public void shouldThrowConflictExceptionWhenCreatingAccountWithExistingName() throws Exception {
AccountImpl account =
new AccountImpl(NameGenerator.generate("account", 5), accounts[0].getName(), "test");
accountDao.create(account);
}
@Test(expectedExceptions = NotFoundException.class)
public void shouldThrowNotFoundExceptionWhenUpdatingNonExistingAccount() throws Exception {
AccountImpl account = accounts[0];