Make invalid password message clearer

src/integrationTest/java/org/opensearch/security/api/CreateResetPasswordTest.java

@@ -0,0 +1,108 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ *
+ */
+package org.opensearch.security.api;
+
+import java.util.List;
+import java.util.Map;
+
+import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.opensearch.security.dlic.rest.validation.RequestContentValidator;
+import org.opensearch.security.support.ConfigConstants;
+import org.opensearch.test.framework.TestSecurityConfig.User;
+import org.opensearch.test.framework.cluster.ClusterManager;
+import org.opensearch.test.framework.cluster.LocalCluster;
+import org.opensearch.test.framework.cluster.TestRestClient;
+import org.opensearch.test.framework.cluster.TestRestClient.HttpResponse;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.equalTo;
+import static org.opensearch.security.SecurityConfigurationTests.ADDITIONAL_USER_1;
+import static org.opensearch.security.SecurityConfigurationTests.CREATE_USER_BODY;
+import static org.opensearch.security.SecurityConfigurationTests.INTERNAL_USERS_RESOURCE;
+import static org.opensearch.security.support.ConfigConstants.SECURITY_BACKGROUND_INIT_IF_SECURITYINDEX_NOT_EXIST;
+import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_ROLES_ENABLED;
+import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL;
+import static org.opensearch.test.framework.TestSecurityConfig.Role.ALL_ACCESS;
+
+@RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class)
+@ThreadLeakScope(ThreadLeakScope.Scope.NONE)
+public class CreateResetPasswordTest {
+
+    private static final User USER_ADMIN = new User("admin").roles(ALL_ACCESS);
+
+    public static final String INVALID_PASSWORD_REGEX = "user 1 fair password";
+
+    public static final String VALID_WEAK_PASSWORD = "Asdfghjkl1!";
+
+    public static final String VALID_SIMILAR_PASSWORD = "456Additional00001_1234!";
+
+    private static final String CUSTOM_PASSWORD_MESSAGE =
+        "Password must be minimum 5 characters long and must contain at least one uppercase letter, one lowercase letter, one digit, and one special character.";
+
+    private static final String CUSTOM_PASSWORD_REGEX = "(?=.*[A-Z])(?=.*[^a-zA-Z\\d])(?=.*[0-9])(?=.*[a-z]).{5,}";
+
+    @ClassRule
+    public static LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.THREE_CLUSTER_MANAGERS)
+        .authc(AUTHC_HTTPBASIC_INTERNAL)
+        .users(USER_ADMIN)
+        .anonymousAuth(false)
+        .nodeSettings(
+            Map.of(
+                SECURITY_RESTAPI_ROLES_ENABLED,
+                List.of("user_" + USER_ADMIN.getName() + "__" + ALL_ACCESS.getName()),
+                SECURITY_BACKGROUND_INIT_IF_SECURITYINDEX_NOT_EXIST,
+                true,
+                ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_REGEX,
+                CUSTOM_PASSWORD_REGEX,
+                ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_ERROR_MESSAGE,
+                CUSTOM_PASSWORD_MESSAGE
+            )
+        )
+        .build();
+
+    @Test
+    public void shouldValidateCreateUserAPIErrorMessages() {
+        try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
+            HttpResponse httpResponse = client.putJson(
+                INTERNAL_USERS_RESOURCE + ADDITIONAL_USER_1,
+                String.format(CREATE_USER_BODY, INVALID_PASSWORD_REGEX)
+            );
+
+            assertThat(httpResponse.getStatusCode(), equalTo(400));
+            assertThat(httpResponse.getBody(), containsString(CUSTOM_PASSWORD_MESSAGE));
+        }
+
+        try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
+            HttpResponse httpResponse = client.putJson(
+                INTERNAL_USERS_RESOURCE + ADDITIONAL_USER_1,
+                String.format(CREATE_USER_BODY, VALID_WEAK_PASSWORD)
+            );
+
+            assertThat(httpResponse.getStatusCode(), equalTo(400));
+            assertThat(httpResponse.getBody(), containsString(RequestContentValidator.ValidationError.WEAK_PASSWORD.message()));
+        }
+
+        try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
+            HttpResponse httpResponse = client.putJson(
+                INTERNAL_USERS_RESOURCE + ADDITIONAL_USER_1,
+                String.format(CREATE_USER_BODY, VALID_SIMILAR_PASSWORD)
+            );
+
+            assertThat(httpResponse.getStatusCode(), equalTo(400));
+            assertThat(httpResponse.getBody(), containsString(RequestContentValidator.ValidationError.SIMILAR_PASSWORD.message()));
+        }
+    }
+
+}

src/main/java/org/opensearch/security/dlic/rest/validation/PasswordValidator.java

@@ -84,19 +84,19 @@ public RequestContentValidator.ValidationError validate(final String username, f
                 minPasswordLength,
                 password.length()
             );
-            return RequestContentValidator.ValidationError.INVALID_PASSWORD;
+            return RequestContentValidator.ValidationError.INVALID_PASSWORD_TOO_SHORT;
         }
         if (password.length() > MAX_LENGTH) {
             logger.debug(
                 "Password is too long, the maximum required length is {}, but current length is {}",
                 MAX_LENGTH,
                 password.length()
             );
-            return RequestContentValidator.ValidationError.INVALID_PASSWORD;
+            return RequestContentValidator.ValidationError.INVALID_PASSWORD_TOO_LONG;
         }
         if (Objects.nonNull(passwordRegexpPattern) && !passwordRegexpPattern.matcher(password).matches()) {
             logger.debug("Regex does not match password");
-            return RequestContentValidator.ValidationError.INVALID_PASSWORD;
+            return RequestContentValidator.ValidationError.INVALID_PASSWORD_INVALID_REGEX;
         }
         final Strength strength = zxcvbn.measure(password, ImmutableList.of(username));
         if (strength.getScore() < scoreStrength.score()) {

src/main/java/org/opensearch/security/dlic/rest/validation/RequestContentValidator.java

@@ -245,7 +245,7 @@ private ValidationResult validatePassword(final RestRequest request, final JsonN
             final PasswordValidator passwordValidator = PasswordValidator.of(validationContext.settings());
             final String password = jsonContent.get("password").asText();
             if (Strings.isNullOrEmpty(password)) {
-                this.validationError = ValidationError.INVALID_PASSWORD;
+                this.validationError = ValidationError.INVALID_PASSWORD_TOO_SHORT;
                 return ValidationResult.error(this);
             }
             final String username = Utils.coalesce(
@@ -278,22 +278,14 @@ public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params par
                 addErrorMessage(builder, MISSING_MANDATORY_KEYS_KEY, missingMandatoryKeys);
                 addErrorMessage(builder, MISSING_MANDATORY_OR_KEYS_KEY, missingMandatoryOrKeys);
                 break;
-            case INVALID_PASSWORD:
+            case INVALID_PASSWORD_INVALID_REGEX:
                 builder.field("status", "error");
                 builder.field(
                     "reason",
                     validationContext.settings()
                         .get(SECURITY_RESTAPI_PASSWORD_VALIDATION_ERROR_MESSAGE, "Password does not match minimum criteria")
                 );
                 break;
-            case WEAK_PASSWORD:
-            case SIMILAR_PASSWORD:
-                builder.field("status", "error");
-                builder.field(
-                    "reason",
-                    validationContext.settings().get(SECURITY_RESTAPI_PASSWORD_VALIDATION_ERROR_MESSAGE, validationError.message())
-                );
-                break;
             case WRONG_DATATYPE:
                 builder.field("status", "error");
                 builder.field("reason", ValidationError.WRONG_DATATYPE.message());
@@ -357,7 +349,9 @@ public Map<String, DataType> allowedKeys() {
     public enum ValidationError {
         NONE("ok"),
         INVALID_CONFIGURATION("Invalid configuration"),
-        INVALID_PASSWORD("Invalid password"),
+        INVALID_PASSWORD_TOO_SHORT("Password is too short"),
+        INVALID_PASSWORD_TOO_LONG("Password is too long"),
+        INVALID_PASSWORD_INVALID_REGEX("Password does not match validation regex"),
         NO_USERNAME("No username is given"),
         WEAK_PASSWORD("Weak password"),
         SIMILAR_PASSWORD("Password is similar to user name"),

src/test/java/org/opensearch/security/dlic/rest/api/UserApiTest.java

@@ -665,7 +665,7 @@ public void testRegExpPasswordRules() throws Exception {
 
         verifyCouldNotCreatePasswords(HttpStatus.SC_BAD_REQUEST);
         verifyCanCreatePasswords();
-        verifySimilarity("xxx");
+        verifySimilarity(RequestContentValidator.ValidationError.SIMILAR_PASSWORD.message());
 
         addUserWithPasswordAndHash("empty_password", "", "$%^123", HttpStatus.SC_BAD_REQUEST);
         addUserWithPasswordAndHash("null_password", null, "$%^123", HttpStatus.SC_BAD_REQUEST);
@@ -808,7 +808,12 @@ public void testScoreBasedPasswordRules() throws Exception {
             RequestContentValidator.ValidationError.WEAK_PASSWORD.message()
         );
 
-        addUserWithPassword("admin", "pas", HttpStatus.SC_BAD_REQUEST, "Password does not match minimum criteria");
+        addUserWithPassword(
+            "admin",
+            "pas",
+            HttpStatus.SC_BAD_REQUEST,
+            RequestContentValidator.ValidationError.INVALID_PASSWORD_TOO_SHORT.message()
+        );
 
         verifySimilarity(RequestContentValidator.ValidationError.SIMILAR_PASSWORD.message());
 

src/test/java/org/opensearch/security/dlic/rest/validation/PasswordValidatorTest.java

@@ -126,12 +126,12 @@ public void testRegExpBasedValidation() {
                 .put(SECURITY_RESTAPI_PASSWORD_VALIDATION_REGEX, "(?=.*[A-Z])(?=.*[^a-zA-Z\\\\d])(?=.*[0-9])(?=.*[a-z]).{8,}")
                 .build()
         );
-        verifyWeakPasswords(passwordValidator, RequestContentValidator.ValidationError.INVALID_PASSWORD);
-        verifyFairPasswords(passwordValidator, RequestContentValidator.ValidationError.INVALID_PASSWORD);
+        verifyWeakPasswords(passwordValidator, RequestContentValidator.ValidationError.INVALID_PASSWORD_INVALID_REGEX);
+        verifyFairPasswords(passwordValidator, RequestContentValidator.ValidationError.INVALID_PASSWORD_INVALID_REGEX);
         for (final String password : GOOD_PASSWORDS.subList(0, GOOD_PASSWORDS.size() - 2))
             assertEquals(
                 password,
-                RequestContentValidator.ValidationError.INVALID_PASSWORD,
+                RequestContentValidator.ValidationError.INVALID_PASSWORD_INVALID_REGEX,
                 passwordValidator.validate("some_user_name", password)
             );
         for (final String password : GOOD_PASSWORDS.subList(GOOD_PASSWORDS.size() - 2, GOOD_PASSWORDS.size()))
@@ -151,7 +151,10 @@ public void testMinLength() {
             Settings.builder().put(SECURITY_RESTAPI_PASSWORD_MIN_LENGTH, 15).build()
         );
         for (final String password : STRONG_PASSWORDS) {
-            assertEquals(RequestContentValidator.ValidationError.INVALID_PASSWORD, passwordValidator.validate(password, "some_user_name"));
+            assertEquals(
+                RequestContentValidator.ValidationError.INVALID_PASSWORD_TOO_SHORT,
+                passwordValidator.validate(password, "some_user_name")
+            );
         }
 
     }