libre.sh/keycloak-scim
3
0
Fork 0
Code Issues 15 Pull requests Releases 1 Packages Activity Actions

Check email and username for duplicated if isLoginWithEmailAllowed

Browse source 
Closes #27297

Signed-off-by: rmartinc <rmartinc@redhat.com>
This commit is contained in:
rmartinc 2024-02-29 18:53:18 +01:00 committed by Pedro Igor
parent 137907f5ef
commit f970803738
9 changed files with 123 additions and 31 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

7
services/src/main/java/org/keycloak/services/resources/admin/UserResource.java
View file

@ -78,6 +78,7 @@ import org.keycloak.services.managers.AuthenticationManager;
import org.keycloak.services.managers.BruteForceProtector;
import org.keycloak.services.managers.UserConsentManager;
import org.keycloak.services.managers.UserSessionManager;
import org.keycloak.services.messages.Messages;
import org.keycloak.services.resources.KeycloakOpenAPI;
import org.keycloak.services.resources.LoginActionsService;
import org.keycloak.services.resources.admin.permissions.AdminPermissionEvaluator;
@ -241,6 +242,12 @@ public class UserResource {
} catch (ValidationException pve) {
List<ErrorRepresentation> errors = new ArrayList<>();
for (ValidationException.Error error : pve.getErrors()) {
// some messages are managed directly as before
switch (error.getMessage()) {
case Messages.MISSING_USERNAME -> throw ErrorResponse.error("User name is missing", Response.Status.BAD_REQUEST);
case Messages.USERNAME_EXISTS -> throw ErrorResponse.exists("User exists with same username");
case Messages.EMAIL_EXISTS -> throw ErrorResponse.exists("User exists with same email");
}
errors.add(new ErrorRepresentation(error.getAttribute(), error.getMessage(), error.getMessageParameters()));
}

18
services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java
View file

@ -24,7 +24,6 @@ import org.jboss.logging.Logger;
import org.jboss.resteasy.reactive.NoCache;
import org.keycloak.common.ClientConnection;
import org.keycloak.common.Profile;
import org.keycloak.common.util.ObjectUtil;
import org.keycloak.events.admin.OperationType;
import org.keycloak.events.admin.ResourceType;
import org.keycloak.models.Constants;
@ -133,23 +132,6 @@ public class UsersResource {
if(realm.isRegistrationEmailAsUsername()) {
username = rep.getEmail();
}
if (ObjectUtil.isBlank(username)) {
throw ErrorResponse.error("User name is missing", Response.Status.BAD_REQUEST);
}
// Double-check duplicated username and email here due to federation
if (session.users().getUserByUsername(realm, username) != null) {
throw ErrorResponse.exists("User exists with same username");
}
if (rep.getEmail() != null && !realm.isDuplicateEmailsAllowed()) {
try {
if(session.users().getUserByEmail(realm, rep.getEmail()) != null) {
throw ErrorResponse.exists("User exists with same email");
}
} catch (ModelDuplicateException e) {
throw ErrorResponse.exists("User exists with same email");
}
}
UserProfileProvider profileProvider = session.getProvider(UserProfileProvider.class);

9
services/src/main/java/org/keycloak/userprofile/DeclarativeUserProfileProviderFactory.java
View file

@ -440,9 +440,14 @@ public class DeclarativeUserProfileProviderFactory implements UserProfileProvide
UserProfileMetadata metadata = new UserProfileMetadata(USER_API);
metadata.addAttribute(UserModel.USERNAME, -2, new AttributeValidatorMetadata(UsernameHasValueValidator.ID))
metadata.addAttribute(UserModel.USERNAME, -2,
new AttributeValidatorMetadata(UsernameHasValueValidator.ID),
new AttributeValidatorMetadata(DuplicateUsernameValidator.ID))
.addWriteCondition(DeclarativeUserProfileProviderFactory::editUsernameCondition);
metadata.addAttribute(UserModel.EMAIL, -1, new AttributeValidatorMetadata(EmailValidator.ID, ValidatorConfig.builder().config(EmailValidator.IGNORE_EMPTY_VALUE, true).build()))
metadata.addAttribute(UserModel.EMAIL, -1,
new AttributeValidatorMetadata(DuplicateEmailValidator.ID),
new AttributeValidatorMetadata(EmailExistsAsUsernameValidator.ID),
new AttributeValidatorMetadata(EmailValidator.ID, ValidatorConfig.builder().config(EmailValidator.IGNORE_EMPTY_VALUE, true).build()))
.addWriteCondition(DeclarativeUserProfileProviderFactory::editEmailCondition);
List<AttributeValidatorMetadata> readonlyValidators = new ArrayList<>();

7
services/src/main/java/org/keycloak/userprofile/validator/DuplicateEmailValidator.java
View file

@ -70,6 +70,13 @@ public class DuplicateEmailValidator implements SimpleValidator {
if (userByEmail != null && (user == null || !userByEmail.getId().equals(user.getId()))) {
context.addError(new ValidationError(ID, inputHint, Messages.EMAIL_EXISTS)
.setStatusCode(Response.Status.CONFLICT));
} else if (realm.isLoginWithEmailAllowed()) {
// check for duplicated username
userByEmail = session.users().getUserByUsername(realm, value);
if (userByEmail != null && (user == null || !userByEmail.getId().equals(user.getId()))) {
context.addError(new ValidationError(ID, inputHint, Messages.EMAIL_EXISTS)
.setStatusCode(Response.Status.CONFLICT));
}
}
}

13
services/src/main/java/org/keycloak/userprofile/validator/DuplicateUsernameValidator.java
View file

@ -64,12 +64,21 @@ public class DuplicateUsernameValidator implements SimpleValidator {
KeycloakSession session = context.getSession();
UserModel existing = session.users().getUserByUsername(session.getContext().getRealm(), value);
UserModel user = UserProfileAttributeValidationContext.from(context).getAttributeContext().getUser();
String valueLowercased = value.toLowerCase();
if (! KeycloakModelUtils.isUsernameCaseSensitive(session.getContext().getRealm())) value = value.toLowerCase();
if (! KeycloakModelUtils.isUsernameCaseSensitive(session.getContext().getRealm())) value = valueLowercased;
if (user != null && !value.equals(user.getFirstAttribute(UserModel.USERNAME)) && (existing != null && !existing.getId().equals(user.getId()))) {
RealmModel realm = session.getContext().getRealm();
if (existing != null && (user == null || !existing.getId().equals(user.getId()))) {
context.addError(new ValidationError(ID, inputHint, Messages.USERNAME_EXISTS)
.setStatusCode(Response.Status.CONFLICT));
} else if (realm.isLoginWithEmailAllowed() && value.indexOf('@') > 0) {
// check the username does not collide with an email
existing = session.users().getUserByEmail(realm, valueLowercased);
if (existing != null && (user == null || !existing.getId().equals(user.getId()))) {
context.addError(new ValidationError(ID, inputHint, Messages.USERNAME_EXISTS)
.setStatusCode(Response.Status.CONFLICT));
}
}
return context;

50
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionUpdateProfileTest.java
View file

@ -332,6 +332,56 @@ public class RequiredActionUpdateProfileTest extends AbstractTestRealmKeycloakTe
events.assertEmpty();
}
@Test
public void updateProfileDuplicateUsernameWithEmail() {
getCleanup().addUserId(createUser(TEST_REALM_NAME, "user1@local.com", "password", "user1", "user1", "user1@local.org"));
loginPage.open();
loginPage.login("john-doh@localhost", "password");
updateProfilePage.assertCurrent();
updateProfilePage.prepareUpdate().username("user1@local.org").firstName("New first").lastName("New last").email("new@email.com").submit();
updateProfilePage.assertCurrent();
// assert that form holds submitted values during validation error
Assert.assertEquals("New first", updateProfilePage.getFirstName());
Assert.assertEquals("New last", updateProfilePage.getLastName());
Assert.assertEquals("new@email.com", updateProfilePage.getEmail());
Assert.assertEquals("user1@local.org", updateProfilePage.getUsername());
Assert.assertEquals("Username already exists.", updateProfilePage.getInputErrors().getUsernameError());
events.assertEmpty();
}
@Test
public void updateProfileDuplicatedEmailWithUsername() {
getCleanup().addUserId(createUser(TEST_REALM_NAME, "user1@local.com", "password", "user1", "user1", "user1@local.org"));
loginPage.open();
loginPage.login("test-user@localhost", "password");
updateProfilePage.assertCurrent();
updateProfilePage.prepareUpdate().username("test-user@localhost").firstName("New first").lastName("New last")
.email("user1@local.com").submit();
updateProfilePage.assertCurrent();
// assert that form holds submitted values during validation error
Assert.assertEquals("New first", updateProfilePage.getFirstName());
Assert.assertEquals("New last", updateProfilePage.getLastName());
Assert.assertEquals("user1@local.com", updateProfilePage.getEmail());
Assert.assertEquals("Email already exists.", updateProfilePage.getInputErrors().getEmailError());
events.assertEmpty();
}
@Test
public void updateProfileExpiredCookies() {
loginPage.open();

37
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserTest.java
View file

@ -325,6 +325,39 @@ public class UserTest extends AbstractAdminTest {
}
}
@Test
public void createDuplicatedUsernameWithEmail() {
createUser("user1@local.com", "user1@local.org");
UserRepresentation user = new UserRepresentation();
user.setUsername("user1@local.org");
user.setEmail("user2@localhost");
try (Response response = realm.users().create(user)) {
assertEquals(409, response.getStatus());
assertAdminEvents.assertEmpty();
ErrorRepresentation error = response.readEntity(ErrorRepresentation.class);
Assert.assertEquals("User exists with same username", error.getErrorMessage());
}
}
@Test
public void createDuplicatedEmailWithUsername() {
createUser("user1@local.com", "user1@local.org");
UserRepresentation user = new UserRepresentation();
user.setUsername("user2");
user.setEmail("user1@local.com");
try (Response response = realm.users().create(user)) {
assertEquals(409, response.getStatus());
assertAdminEvents.assertEmpty();
ErrorRepresentation error = response.readEntity(ErrorRepresentation.class);
Assert.assertEquals("User exists with same email", error.getErrorMessage());
}
}
//KEYCLOAK-14611
@Test
public void createDuplicateEmailWithExistingDuplicates() {
@ -352,7 +385,7 @@ public class UserTest extends AbstractAdminTest {
try (Response response = realm.users().create(user)) {
assertEquals(409, response.getStatus());
ErrorRepresentation error = response.readEntity(ErrorRepresentation.class);
Assert.assertEquals("User exists with same email", error.getErrorMessage());
Assert.assertEquals("User exists with same username or email", error.getErrorMessage());
assertAdminEvents.assertEmpty();
}
}
@ -2619,7 +2652,7 @@ public class UserTest extends AbstractAdminTest {
assertThat(e.getResponse().getStatus(), is(409));
ErrorRepresentation error = e.getResponse().readEntity(ErrorRepresentation.class);
Assert.assertEquals("User exists with same username or email", error.getErrorMessage());
Assert.assertEquals("User exists with same email", error.getErrorMessage());
assertAdminEvents.assertEmpty();
}
}

9
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/kerberos/KerberosStandaloneTest.java
View file

@ -26,23 +26,20 @@ import org.keycloak.testsuite.Assert;
import org.junit.ClassRule;
import org.junit.Test;
import org.keycloak.admin.client.resource.UserResource;
import org.keycloak.common.Profile;
import org.keycloak.common.constants.KerberosConstants;
import org.keycloak.federation.kerberos.CommonKerberosConfig;
import org.keycloak.federation.kerberos.KerberosConfig;
import org.keycloak.federation.kerberos.KerberosFederationProviderFactory;
import org.keycloak.models.UserModel;
import org.keycloak.representations.idm.ComponentRepresentation;
import org.keycloak.representations.idm.RealmRepresentation;
import org.keycloak.representations.idm.ErrorRepresentation;
import org.keycloak.representations.idm.UserProfileAttributeMetadata;
import org.keycloak.representations.idm.UserRepresentation;
import org.keycloak.storage.UserStorageProvider;
import org.keycloak.testsuite.ActionURIUtils;
import org.keycloak.testsuite.KerberosEmbeddedServer;
import org.keycloak.testsuite.admin.ApiUtil;
import org.keycloak.testsuite.arquillian.annotation.EnableFeature;
import org.keycloak.testsuite.arquillian.annotation.UncaughtServerErrorExpected;
import org.keycloak.testsuite.forms.VerifyProfileTest;
import org.keycloak.testsuite.util.KerberosRule;
import static org.keycloak.userprofile.UserProfileUtil.USER_METADATA_GROUP;
@ -181,7 +178,9 @@ public class KerberosStandaloneTest extends AbstractKerberosSingleRealmTest {
UserRepresentation john = new UserRepresentation();
john.setUsername("john");
Response response = testRealmResource().users().create(john);
Assert.assertEquals(500, response.getStatus());
Assert.assertEquals(400, response.getStatus());
ErrorRepresentation error = response.readEntity(ErrorRepresentation.class);
Assert.assertEquals("Could not create user", error.getErrorMessage());
response.close();
}

4
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/user/profile/UserProfileTest.java
View file

@ -202,7 +202,7 @@ public class UserProfileTest extends AbstractUserProfileTest {
private static void testCustomAttributeInAnyContext(KeycloakSession session) {
Map<String, Object> attributes = new HashMap<>();
attributes.put(UserModel.USERNAME, "profiled-user");
attributes.put(UserModel.USERNAME, org.keycloak.models.utils.KeycloakModelUtils.generateId());
attributes.put(UserModel.FIRST_NAME, "John");
attributes.put(UserModel.LAST_NAME, "Doe");
attributes.put(UserModel.EMAIL, org.keycloak.models.utils.KeycloakModelUtils.generateId() + "@keycloak.org");
@ -243,7 +243,7 @@ public class UserProfileTest extends AbstractUserProfileTest {
Map<String, Object> attributes = new HashMap<>();
attributes.put(UserModel.USERNAME, "profiled-user");
attributes.put(UserModel.USERNAME, org.keycloak.models.utils.KeycloakModelUtils.generateId() + "@keycloak.org");
attributes.put(UserModel.FIRST_NAME, "John");
attributes.put(UserModel.LAST_NAME, "Doe");
attributes.put(UserModel.EMAIL, org.keycloak.models.utils.KeycloakModelUtils.generateId() + "@keycloak.org");