Skip to content

Disable username prohibited chars validator when email as username is… #31140

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pedroigor merged 2 commits into from
Jul 10, 2024
+51 −14
Merged

Disable username prohibited chars validator when email as username is… #31140

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bbe1295
Disable username prohibited chars validator when email as username is…
martin-kanis Jul 9, 2024
aa66b49
Unnecessary nullability checks and update to the docs
pedroigor Jul 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/documentation/server_admin/topics/users/user-profile.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,7 @@ The list below provides a list of all the built-in validators:

|username-prohibited-characters
| Check if the value is a valid username as an additional barrier for attacks such as script injection. The validation is based on a default RegEx pattern that blocks characters not common in usernames.
When the realm setting `Email as username` is enabled, this validator is skipped to allow email values.
|

*error-message*: the key of the error message in i18n bundle. If not set a generic message is used.
Expand Down
22 changes: 17 additions & 5 deletions ...c/main/java/org/keycloak/userprofile/validator/UsernameProhibitedCharactersValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
import java.util.List;
import java.util.regex.Pattern;

import org.keycloak.models.KeycloakSession;
import org.keycloak.models.RealmModel;
import org.keycloak.provider.ConfiguredProvider;
import org.keycloak.provider.ProviderConfigProperty;
import org.keycloak.validate.AbstractStringValidator;
Expand All @@ -40,11 +42,11 @@ public class UsernameProhibitedCharactersValidator extends AbstractStringValidat
public static final UsernameProhibitedCharactersValidator INSTANCE = new UsernameProhibitedCharactersValidator();

protected static final Pattern PATTERN = Pattern.compile("^[^<>&\"'\\s\\v\\h$%!#?§,;:*~/\\\\|^=\\[\\]{}()`\\p{Cntrl}]+$");

public static final String MESSAGE_NO_MATCH = "error-username-invalid-character";

public static final String CFG_ERROR_MESSAGE = "error-message";

private static final List<ProviderConfigProperty> configProperties = new ArrayList<>();

static {
Expand All @@ -64,12 +66,22 @@ public String getId() {

@Override
protected void doValidate(String value, String inputHint, ValidationContext context, ValidatorConfig config) {
KeycloakSession session = context.getSession();

if (session != null) {
RealmModel realm = session.getContext().getRealm();

if (realm.isRegistrationEmailAsUsername()) {
return;
}
}

if (!PATTERN.matcher(value).matches()) {
context.addError(new ValidationError(ID, inputHint, config.getStringOrDefault(CFG_ERROR_MESSAGE, MESSAGE_NO_MATCH)));
}
}


@Override
public String getHelpText() {
return "Basic Username validator disallowing bunch of characters we really do not expect in username.";
Expand Down
42 changes: 33 additions & 9 deletions ...uillian/tests/base/src/test/java/org/keycloak/testsuite/user/profile/UserProfileTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,7 @@ private static void testResolveProfile(KeycloakSession session) {
public void testValidation() {
getTestingClient().server(TEST_REALM_NAME).run((RunOnServer) UserProfileTest::failValidationWhenEmptyAttributes);
getTestingClient().server(TEST_REALM_NAME).run((RunOnServer) UserProfileTest::testAttributeValidation);
getTestingClient().server(TEST_REALM_NAME).run((RunOnServer) UserProfileTest::testEmailAsUsernameValidation);
}

private static void failValidationWhenEmptyAttributes(KeycloakSession session) {
Expand Down Expand Up @@ -398,7 +399,30 @@ private static void testAttributeValidation(KeycloakSession session) {
assertFalse(profile.getAttributes().validate(UserModel.EMAIL, (Consumer<ValidationError>) errors::add));
assertTrue(containsErrorMessage(errors, EmailValidator.MESSAGE_INVALID_EMAIL));
}


private static void testEmailAsUsernameValidation(KeycloakSession session) {
Map<String, Object> attributes = new HashMap<>();
UserProfileProvider provider = session.getProvider(UserProfileProvider.class);
provider.setConfiguration(null);
UserProfile profile;
RealmModel realm = session.getContext().getRealm();

try {
realm.setRegistrationEmailAsUsername(true);
attributes.clear();
attributes.put(UserModel.FIRST_NAME, "Joe");
attributes.put(UserModel.LAST_NAME, "Doe");
// valid email but invalid as username
attributes.put(UserModel.EMAIL, "foo%bar@example.com");
profile = provider.create(UserProfileContext.UPDATE_PROFILE, attributes);
profile.validate();
} catch (ValidationException ve) {
Assert.fail("Should be OK email as username");
} finally {
realm.setRegistrationEmailAsUsername(false);
}
}

private static boolean containsErrorMessage(List<ValidationError> errors, String message){
for(ValidationError err : errors) {
if(err.getMessage().equals(message)) {
Expand All @@ -407,7 +431,7 @@ private static boolean containsErrorMessage(List<ValidationError> errors, String
}
return false;
}


@Test
public void testValidateComplianceWithUserProfile() {
Expand Down Expand Up @@ -516,14 +540,14 @@ private static void testGetProfileAttributeGroups(KeycloakSession session) {

assertThat(attributes.nameSet(),
containsInAnyOrder(UserModel.USERNAME, UserModel.EMAIL, UserModel.FIRST_NAME, UserModel.LAST_NAME, UserModel.LOCALE, "address", "second"));


AttributeGroupMetadata companyAddressGroup = attributes.getMetadata("address").getAttributeGroupMetadata();
assertEquals("companyaddress", companyAddressGroup.getName());
assertEquals("header", companyAddressGroup.getDisplayHeader());
assertEquals("description", companyAddressGroup.getDisplayDescription());
assertNull(companyAddressGroup.getAnnotations());

AttributeGroupMetadata groupwithannoGroup = attributes.getMetadata("second").getAttributeGroupMetadata();
assertEquals("groupwithanno", groupwithannoGroup.getName());
assertNull(groupwithannoGroup.getDisplayHeader());
Expand Down Expand Up @@ -572,9 +596,9 @@ private static void testCreateAndUpdateUser(KeycloakSession session) {
attributesUpdatedOldValues.put(UserModel.FIRST_NAME, "Joe");
attributesUpdatedOldValues.put(UserModel.LAST_NAME, "Doe");
attributesUpdatedOldValues.put(UserModel.EMAIL, "user@keycloak.org");

profile.update((attributeName, userModel, oldValue) -> {
assertTrue(attributesUpdated.add(attributeName));
assertTrue(attributesUpdated.add(attributeName));
assertEquals(attributesUpdatedOldValues.get(attributeName), getSingleValue(oldValue));
assertEquals(attributes.get(attributeName), userModel.getFirstAttribute(attributeName));
});
Expand All @@ -593,13 +617,13 @@ private static void testCreateAndUpdateUser(KeycloakSession session) {

assertEquals("fixed-business-address", user.getFirstAttribute("business.address"));
}

private static String getSingleValue(List<String> vals) {
if(vals==null || vals.isEmpty())
return null;
return vals.get(0);
}

@Test
public void testReadonlyUpdates() {
getTestingClient().server(TEST_REALM_NAME).run((RunOnServer) UserProfileTest::testReadonlyUpdates);
Expand Down