Fixed attribute validation error message (#4)

kilmajster · web-flow · commit ffa2e17ab9f5 · 2021-06-06T00:58:34.000+02:00
* simplified tests

* simplified tests

* Cucumber tests!

* tests polish

* bumped Keycloak version to 13.0.1

* renamed job

* renamed job

* test fix

* fixed attribute validation message

* version of build for automation tests is set from commit SHA
diff --git a/.github/workflows/automation-tests.yml b/.github/workflows/automation-tests.yml
@@ -18,11 +18,14 @@ jobs:
         java-version: '11'
         distribution: 'adopt'
 
+    - name: Set version from git commit SHA
+      run: mvn -B -ntp versions:set -DgenerateBackupPoms=false -DnewVersion="${GITHUB_SHA::6}"
+
     - name: Build authenticator jar file
       run: mvn -B -ntp package
 
     - name: Build test docker container
-      run: docker-compose build
+      run: docker-compose build --build-arg VERSION="${GITHUB_SHA::6}"
 
     - name: Run automation tests
       run: mvn -B -ntp test -P automation-tests -D selenide.headless=true
diff --git a/README.md b/README.md
@@ -1,5 +1,5 @@
-# Keycloak username password attribute authenticator
-[![CI with 🐋 & Selenide](https://github.com/kilmajster/keycloak-username-password-attribute-authenticator/actions/workflows/automation-tests.yml/badge.svg)](https://github.com/kilmajster/keycloak-username-password-attribute-authenticator/actions/workflows/automation-tests.yml)
+# Keycloak username password attribute 
+[![automation tests](https://github.com/kilmajster/keycloak-username-password-attribute-authenticator/actions/workflows/automation-tests.yml/badge.svg)](https://github.com/kilmajster/keycloak-username-password-attribute-authenticator/actions/workflows/automation-tests.yml)
 ![Maven Central](https://img.shields.io/maven-central/v/io.github.kilmajster/keycloak-username-password-attribute-authenticator)
 ![Docker Image Version (latest by date)](https://img.shields.io/docker/v/kilmajster/keycloak-username-password-attribute-authenticator?label=docker%20hub)
 ![Docker Pulls](https://img.shields.io/docker/pulls/kilmajster/keycloak-username-password-attribute-authenticator)
@@ -9,11 +9,14 @@
 Keycloak default login form with user attribute validation.
 
 ## How 2 use
+To use this authenticator, it should be bundled together with Keycloak, here are two ways how to do that:
 ### using jar
 
 
 
 ### using docker init container
+If you want to use this authenticator in some cloud envirenement, here is ready init container. Jar file is placed in `/opt/jboss/keycloak/standalone/deployments`, 
+so same location as target one. Possible 
 ```
 kilmajster/keycloak-username-password-attribute-authenticator:latest
 ```
@@ -22,14 +25,14 @@ kilmajster/keycloak-username-password-attribute-authenticator:latest
 ## Configuration
 ### Authenticator config
 #### config via Keycloak UI / API
- - login_form_user_attribute
- - login_form_generate_label
- - login_form_attribute_label
+- login_form_user_attribute
+- login_form_generate_label
+- login_form_attribute_label
 
 #### config via env variables
- - LOGIN_FORM_USER_ATTRIBUTE
- - LOGIN_FORM_GENERATE_LABEL
- - LOGIN_FORM_ATTRIBUTE_LABEL
+- LOGIN_FORM_USER_ATTRIBUTE
+- LOGIN_FORM_GENERATE_LABEL
+- LOGIN_FORM_ATTRIBUTE_LABEL
 
 ### Theme config
 #### Using bundled default keycloak theme
diff --git a/src/main/docker/dev.Dockerfile b/src/main/docker/dev.Dockerfile
@@ -1,3 +1,5 @@
 FROM quay.io/keycloak/keycloak:latest
 
-ADD target/keycloak-username-password-attribute-authenticator-SNAPSHOT.jar /opt/jboss/keycloak/standalone/deployments
+ARG VERSION=SNAPSHOT
+
+ADD target/keycloak-username-password-attribute-authenticator-${VERSION}.jar /opt/jboss/keycloak/standalone/deployments
diff --git a/src/main/java/io.github.kilmajster.keycloak/UsernamePasswordAttributeForm.java b/src/main/java/io.github.kilmajster.keycloak/UsernamePasswordAttributeForm.java
@@ -9,13 +9,16 @@
 import org.keycloak.authentication.authenticators.browser.UsernamePasswordForm;
 import org.keycloak.events.Details;
 import org.keycloak.events.Errors;
+import org.keycloak.forms.login.LoginFormsProvider;
 import org.keycloak.models.ModelDuplicateException;
 import org.keycloak.models.UserModel;
+import org.keycloak.models.utils.FormMessage;
 import org.keycloak.models.utils.KeycloakModelUtils;
 import org.keycloak.services.ServicesLogger;
 import org.keycloak.services.managers.AuthenticationManager;
 import org.keycloak.services.messages.Messages;
 import org.keycloak.services.validation.Validation;
+import org.keycloak.theme.FreeMarkerUtil;
 
 import javax.ws.rs.core.MultivaluedMap;
 import javax.ws.rs.core.Response;
@@ -26,6 +29,34 @@ public class UsernamePasswordAttributeForm extends UsernamePasswordForm implemen
 
     protected static ServicesLogger log = ServicesLogger.LOGGER;
 
+    @Override
+    protected Response challenge(AuthenticationFlowContext context, String error, String field) {
+        LoginFormsProvider form = context.form().setExecution(context.getExecution().getId());
+        if (error != null) {
+            if (field != null) {
+                form.addError(new FormMessage(field, error));
+            } else {
+                form.setError(error, new Object[0]);
+            }
+        }
+
+        configureUserAttributeLabel(context);
+
+        return createLoginForm(form);
+    }
+
+    @Override
+    protected Response challenge(AuthenticationFlowContext context, MultivaluedMap<String, String> formData) {
+        LoginFormsProvider forms = context.form();
+        if (formData.size() > 0) {
+            forms.setFormData(formData);
+        }
+
+        configureUserAttributeLabel(context);
+
+        return forms.createLoginUsernamePassword();
+    }
+
     @Override
     public void authenticate(AuthenticationFlowContext context) {
         MultivaluedMap<String, String> formData = new MultivaluedMapImpl();
@@ -40,9 +71,7 @@ public void authenticate(AuthenticationFlowContext context) {
             }
         }
 
-        configureUserAttributeLabel(context);
-
-        Response challengeResponse = this.challenge(context, formData);
+        Response challengeResponse = challenge(context, formData);
         context.challenge(challengeResponse);
     }
 
@@ -55,7 +84,7 @@ private void configureUserAttributeLabel(AuthenticationFlowContext context) {
             if (userAttributeName != null && !userAttributeName.isEmpty()) {
                 context.form().setAttribute(USER_ATTRIBUTE_LABEL,
                         isGeneratePropertyLabelEnabled(context)
-                                ? UserAttributeLabelGenerator.from(userAttributeName)
+                                ? UserAttributeLabelGenerator.generateLabel(userAttributeName)
                                 : userAttributeName);
             } else {
                 log.warn("Configuration of keycloak-user-attribute-authenticator is incomplete! " +
@@ -96,7 +125,32 @@ private boolean isProvidedAttributeValid(AuthenticationFlowContext context, User
     private boolean invalidUserAttributeHandler(AuthenticationFlowContext context, UserModel user, boolean isAttributeEmpty) {
         context.getEvent().user(user);
         context.getEvent().error(Errors.INVALID_USER_CREDENTIALS);
-        Response challengeResponse = challenge(context, getDefaultChallengeMessage(context), USER_ATTRIBUTE);
+
+        String errorText;
+        final String userAttributeErrorLabel = configPropertyOf(context, USER_ATTRIBUTE_ERROR_LABEL);
+        if (userAttributeErrorLabel != null && !userAttributeErrorLabel.isBlank()) {
+            // error text directly from error label propertyModelException
+            errorText = userAttributeErrorLabel;
+        } else {
+            final String userAttributeLabel = configPropertyOf(context, USER_ATTRIBUTE_LABEL);
+            if (userAttributeLabel != null && !userAttributeLabel.isBlank()) {
+                // get message from message.properties in case USER_ATTRIBUTE_LABEL is a message key
+                final String message = context.form().getMessage(userAttributeLabel);
+                // generating error message based on provided user attribute label
+                errorText = UserAttributeLabelGenerator.generateErrorText(message != null ? message : userAttributeLabel);
+            } else {
+                // user attribute label not provided so generating text based on attribute name
+                errorText = isGeneratePropertyLabelEnabled(context) // generate pretty error if property is not disabled
+                        ? UserAttributeLabelGenerator.generateErrorText(configPropertyOf(context, USER_ATTRIBUTE))
+                        : "Invalid ".concat(configPropertyOf(context, USER_ATTRIBUTE)); // use raw attribute name
+            }
+        }
+
+        if (isClearUserOnFailedAttributeValidationEnabled(context)) {
+            context.clearUser();
+        }
+
+        Response challengeResponse = challenge(context, errorText, USER_ATTRIBUTE);
 
         if (isAttributeEmpty) {
             context.forceChallenge(challengeResponse);
diff --git a/src/main/java/io.github.kilmajster.keycloak/UsernamePasswordAttributeFormConfiguration.java b/src/main/java/io.github.kilmajster.keycloak/UsernamePasswordAttributeFormConfiguration.java
@@ -12,6 +12,8 @@ public interface UsernamePasswordAttributeFormConfiguration {
     String USER_ATTRIBUTE = "login_form_user_attribute";
     String GENERATE_FORM_LABEL = "login_form_generate_label";
     String USER_ATTRIBUTE_LABEL = "login_form_attribute_label";
+    String USER_ATTRIBUTE_ERROR_LABEL = "login_form_attribute_error_label";
+    String CLEAR_USER_ON_ATTRIBUTE_VALIDATION_FAIL = "clear_user_on_attribute_validation_fail";
 
     List<ProviderConfigProperty> PROPS = ProviderConfigurationBuilder.create()
 
@@ -30,13 +32,28 @@ public interface UsernamePasswordAttributeFormConfiguration {
             .helpText("TODO")
             .add()
 
+            .property()
+            .name(CLEAR_USER_ON_ATTRIBUTE_VALIDATION_FAIL)
+            .type(ProviderConfigProperty.BOOLEAN_TYPE)
+            .label("Clear user on attribute validation fail")
+            .defaultValue(true)
+            .helpText("TODO")
+            .add()
+
             .property()
             .name(USER_ATTRIBUTE_LABEL)
             .type(ProviderConfigProperty.STRING_TYPE)
             .label("User attribute form label")
             .helpText("TODO")
             .add()
 
+            .property()
+            .name(USER_ATTRIBUTE_ERROR_LABEL)
+            .type(ProviderConfigProperty.STRING_TYPE)
+            .label("Message for user attribute validation error")
+            .helpText("TODO")
+            .add()
+
             .build();
 
     static String configPropertyOf(final AuthenticationFlowContext context, final String configPropertyName) {
@@ -47,4 +64,8 @@ static String configPropertyOf(final AuthenticationFlowContext context, final St
     static boolean isGeneratePropertyLabelEnabled(final AuthenticationFlowContext context) {
         return Boolean.parseBoolean(configPropertyOf(context, GENERATE_FORM_LABEL));
     }
+
+    static boolean isClearUserOnFailedAttributeValidationEnabled(final AuthenticationFlowContext context) {
+        return Boolean.parseBoolean(configPropertyOf(context, CLEAR_USER_ON_ATTRIBUTE_VALIDATION_FAIL));
+    }
 }
diff --git a/src/main/java/io.github.kilmajster.keycloak/utils/UserAttributeLabelGenerator.java b/src/main/java/io.github.kilmajster.keycloak/utils/UserAttributeLabelGenerator.java
@@ -2,15 +2,20 @@
 
 public final class UserAttributeLabelGenerator {
 
-    public static String from(final String attributeName) {
+    public static String generateLabel(final String attributeName) {
         final String lowercaseWithSpaces = attributeName
                 .toLowerCase()
+                .replace(".", " ")
                 .replace("_", " ")
                 .replace("-", " ");
 
         return capitalizeFirstChar(lowercaseWithSpaces);
     }
 
+    public static String generateErrorText(final String attributeName)  {
+        return "Invalid " + generateLabel(attributeName).toLowerCase() + ".";
+    }
+
     private static String capitalizeFirstChar(final String lowercaseWithSpaces) {
         return lowercaseWithSpaces.substring(0,1).toUpperCase() + lowercaseWithSpaces.substring(1).toLowerCase();
     }
diff --git a/src/test/java/io.github.kilmajster.keycloak/KeycloakSteps.java b/src/test/java/io.github.kilmajster.keycloak/KeycloakSteps.java
@@ -6,12 +6,12 @@
 import io.cucumber.java.en.Given;
 import io.cucumber.java.en.Then;
 import io.cucumber.java.en.When;
-import org.junit.Before;
 import org.openqa.selenium.By;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.testcontainers.containers.output.Slf4jLogConsumer;
 
+import static com.codeborne.selenide.Condition.exactText;
 import static com.codeborne.selenide.Condition.text;
 import static com.codeborne.selenide.Selenide.$;
 import static com.codeborne.selenide.Selenide.open;
@@ -34,44 +34,58 @@ public void keycloak_is_running_with_default_setup() {
         }
     }
 
-    @Given("keycloak is running with LOGIN_FORM_ATTRIBUTE_LABEL = {string}")
-    public void keycloak_is_running_with_login_form_attribute_label_env(final String envLoginFormAttributeLabel) {
-        keycloak.addEnv("LOGIN_FORM_ATTRIBUTE_LABEL", envLoginFormAttributeLabel);
+    @Given("keycloak is running with {} = {}")
+    public void keycloak_is_running_with_env(final String envKey, final String envValue) {
+        keycloak.addEnv(envKey, envValue);
         if (!keycloak.isRunning()) {
-            log.info("Starting keycloak container with LOGIN_FORM_ATTRIBUTE_LABEL = " + envLoginFormAttributeLabel);
+            log.info("Starting keycloak container with " + envKey + " = " + envValue);
             keycloak.start();
         }
     }
 
     @When("user goes to the account console page")
     public void go_to_keycloak_account_page() {
         final String keycloakUrl = TestConstants.KEYCLOAK_LOCAL_URL_PREFIX + keycloak.getFirstMappedPort();
+
+        log.info("go_to_keycloak_account_page() :: keycloakUrl = " + keycloakUrl);
+
         open(keycloakUrl + "/auth/realms/dev-realm/account");
     }
 
     @Then("user should be not logged in")
     public void user_should_be_not_logged_in() {
+        log.info("user_should_be_not_logged_in()");
+
         final String loggedInUser = $(By.id("landingLoggedInUser")).val();
         assertThat(loggedInUser).isNullOrEmpty();
     }
 
     @When("user clicks a sign in button")
-    public static void click_sign_in_button() {
+    public void click_sign_in_button() {
         log.info("click_sign_in_button()");
 
         $(By.id("landingSignInButton")).click();
     }
 
+    @When("user navigates to login page")
+    public void user_navigates_to_login_page() {
+        log.info("user_navigates_to_login_page()");
+
+        go_to_keycloak_account_page();
+        user_should_be_not_logged_in();
+        click_sign_in_button();
+    }
+
     @Then("login form with attribute input labeled as {string} should be shown")
-    public static void verify_login_form_is_displayed_with_user_attribute_label(final String label) {
+    public void verify_login_form_is_displayed_with_user_attribute_label(final String label) {
         log.info("verify_login_form_is_displayed_with_user_attribute_label( label = " + label + " )");
 
         assertThat($(By.id("kc-form-login")).isDisplayed()).isTrue();
         userAttributeFormLabel().shouldHave(text(label));
     }
 
     @When("user log into account console with a valid credentials and user attribute equal {string}")
-    public static void log_into_account_console(final String attribute) {
+    public void log_into_account_console(final String attribute) {
         log.info("log_into_account_console()");
 
         $(By.id("username")).val(TEST_USERNAME);
@@ -81,13 +95,41 @@ public static void log_into_account_console(final String attribute) {
     }
 
     @Then("user should be logged into account console")
-    public static void verify_that_user_is_logged_in() {
+    public void verify_that_user_is_logged_in() {
         log.info("verify_that_user_is_logged_in()");
 
         $(By.id("landingLoggedInUser")).shouldHave(text("test"));
     }
 
-    private static SelenideElement userAttributeFormLabel() {
+    @Then("form error with message {string} is present")
+    public void form_error_with_message_is_present(final String errorMessage) {
+        log.info("form_error_with_message_is_present( " + errorMessage + " )");
+
+        $(By.className("alert-error")).shouldHave(text(errorMessage));
+    }
+
+    @And("attempted username is cleared")
+    public void attempted_username_is_cleared() {
+        log.info("attempted_username_is_cleared()");
+
+        assertThat($(By.id("kc-attempted-username")).exists()).isFalse();
+    }
+
+    @And("attempted username is set to {string}")
+    public void attempted_username_is_set_to(final String attemptedUsername) {
+        log.info("attempted_username_is_set_to( " + attemptedUsername + " )");
+
+        $(By.id("kc-attempted-username")).shouldHave(exactText(attemptedUsername));
+    }
+
+    @And("restart login link is visible")
+    public void restart_login_link_is_visible() {
+        log.info("restart_login_link_is_visible()");
+
+        assertThat($(By.className("kc-tooltip-text")).getOwnText()).isEqualTo("Restart login");
+    }
+
+    private SelenideElement userAttributeFormLabel() {
         return $(By.xpath("//input[@id='login_form_user_attribute']/preceding-sibling::label"));
     }
-}
+}
diff --git a/src/test/java/io.github.kilmajster.keycloak/utils/UserAttributeLabelGeneratorTest.java b/src/test/java/io.github.kilmajster.keycloak/utils/UserAttributeLabelGeneratorTest.java
@@ -10,10 +10,10 @@ public class UserAttributeLabelGeneratorTest {
 
     @Test
     public void shouldPrettifyString() {
-        final String attribute = "TEST_ATTRIBUTE-NAME";
+        final String attribute = "TEST_ATTRIBUTE-NAME.bLah";
 
-        final String label = UserAttributeLabelGenerator.from(attribute);
+        final String label = UserAttributeLabelGenerator.generateLabel(attribute);
 
-        assertThat(label).isEqualTo("Test attribute name");
+        assertThat(label).isEqualTo("Test attribute name blah");
     }
 }
diff --git a/src/test/resources/cucumber/login-form-env-var-config.feature b/src/test/resources/cucumber/login-form-env-var-config.feature
diff --git a/src/test/resources/dev-realm.json b/src/test/resources/dev-realm.json

Original file line number	Diff line number	Diff line change
`@@ -10,10 +10,10 @@ public class UserAttributeLabelGeneratorTest {`
`10`	`10`
`11`	`11`	`@Test`
`12`	`12`	`public void shouldPrettifyString() {`
`13`		`- final String attribute = "TEST_ATTRIBUTE-NAME";`
	`13`	`+ final String attribute = "TEST_ATTRIBUTE-NAME.bLah";`
`14`	`14`
`15`		`- final String label = UserAttributeLabelGenerator.from(attribute);`
	`15`	`+ final String label = UserAttributeLabelGenerator.generateLabel(attribute);`
`16`	`16`
`17`		`- assertThat(label).isEqualTo("Test attribute name");`
	`17`	`+ assertThat(label).isEqualTo("Test attribute name blah");`
`18`	`18`	`}`
`19`	`19`	`}`