From a50a0e6d6c903cb28d2d9fb86c181d4c8e784636 Mon Sep 17 00:00:00 2001 From: Andrey Pleskach Date: Mon, 22 May 2023 20:49:31 +0200 Subject: [PATCH] Add score based password verification (#2784) Signed-off-by: Andrey Pleskach --- build.gradle | 1 + .../security/OpenSearchSecurityPlugin.java | 14 ++ .../AbstractConfigurationValidator.java | 22 +- .../rest/validation/CredentialsValidator.java | 52 ++--- .../rest/validation/PasswordValidator.java | 185 +++++++++++++++ .../security/support/ConfigConstants.java | 2 + .../RestApiComplianceAuditlogTest.java | 13 +- .../rest/api/AbstractRestApiUnitTest.java | 13 ++ .../dlic/rest/api/AccountApiTest.java | 2 +- .../dlic/rest/api/ActionGroupsApiTest.java | 26 +-- .../security/dlic/rest/api/RolesApiTest.java | 22 +- .../dlic/rest/api/RolesMappingApiTest.java | 14 +- .../security/dlic/rest/api/UserApiTest.java | 213 ++++++++++++------ .../validation/PasswordValidatorTest.java | 197 ++++++++++++++++ 14 files changed, 627 insertions(+), 149 deletions(-) create mode 100644 src/main/java/org/opensearch/security/dlic/rest/validation/PasswordValidator.java create mode 100644 src/test/java/org/opensearch/security/dlic/rest/validation/PasswordValidatorTest.java diff --git a/build.gradle b/build.gradle index 1acafcf3c0..ee2e33131b 100644 --- a/build.gradle +++ b/build.gradle @@ -335,6 +335,7 @@ dependencies { implementation ('org.opensaml:opensaml-saml-impl:3.4.5') { exclude(group: 'org.apache.velocity', module: 'velocity') } + implementation "com.nulab-inc:zxcvbn:1.7.0" testImplementation 'org.opensaml:opensaml-messaging-impl:3.4.5' implementation 'org.opensaml:opensaml-messaging-api:3.4.5' runtimeOnly 'org.opensaml:opensaml-profile-api:3.4.5' diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index 1d89928d2b..878b37d4dc 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -137,6 +137,7 @@ import org.opensearch.security.configuration.Salt; import org.opensearch.security.configuration.SecurityFlsDlsIndexSearcherWrapper; import org.opensearch.security.dlic.rest.api.SecurityRestApiActions; +import org.opensearch.security.dlic.rest.validation.PasswordValidator; import org.opensearch.security.filter.SecurityFilter; import org.opensearch.security.filter.SecurityRestFilter; import org.opensearch.security.http.SecurityHttpServerTransport; @@ -1048,6 +1049,19 @@ public List> getSettings() { settings.add(Setting.simpleString(ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_REGEX, Property.NodeScope, Property.Filtered)); settings.add(Setting.simpleString(ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_ERROR_MESSAGE, Property.NodeScope, Property.Filtered)); + settings.add( + Setting.intSetting( + ConfigConstants.SECURITY_RESTAPI_PASSWORD_MIN_LENGTH, + -1, -1, Property.NodeScope, Property.Filtered) + ); + settings.add( + Setting.simpleString( + ConfigConstants.SECURITY_RESTAPI_PASSWORD_SCORE_BASED_VALIDATION_STRENGTH, + PasswordValidator.ScoreStrength.STRONG.name(), + PasswordValidator.ScoreStrength::fromConfiguration, + Property.NodeScope, Property.Filtered + ) + ); // Compliance settings.add(Setting.listSetting(ConfigConstants.OPENDISTRO_SECURITY_COMPLIANCE_HISTORY_WRITE_WATCHED_INDICES, Collections.emptyList(), Function.identity(), Property.NodeScope)); //not filtered here diff --git a/src/main/java/org/opensearch/security/dlic/rest/validation/AbstractConfigurationValidator.java b/src/main/java/org/opensearch/security/dlic/rest/validation/AbstractConfigurationValidator.java index 81942d9c11..e3221de7e6 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/validation/AbstractConfigurationValidator.java +++ b/src/main/java/org/opensearch/security/dlic/rest/validation/AbstractConfigurationValidator.java @@ -249,9 +249,19 @@ public XContentBuilder errorsAsXContent(RestChannel channel) { break; case INVALID_PASSWORD: builder.field("status", "error"); - builder.field("reason", opensearchSettings.get(ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_ERROR_MESSAGE, + builder.field("reason", opensearchSettings.get( + ConfigConstants.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", + opensearchSettings.get( + ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_ERROR_MESSAGE, + errorType.message)); + break; case WRONG_DATATYPE: builder.field("status", "error"); builder.field("reason", ErrorType.WRONG_DATATYPE.getMessage()); @@ -289,8 +299,14 @@ public static enum DataType { } public static enum ErrorType { - NONE("ok"), INVALID_CONFIGURATION("Invalid configuration"), INVALID_PASSWORD("Invalid password"), WRONG_DATATYPE("Wrong datatype"), - BODY_NOT_PARSEABLE("Could not parse content of request."), PAYLOAD_NOT_ALLOWED("Request body not allowed for this action."), + NONE("ok"), + INVALID_CONFIGURATION("Invalid configuration"), + INVALID_PASSWORD("Invalid password"), + WEAK_PASSWORD("Weak password"), + SIMILAR_PASSWORD("Password is similar to user name"), + WRONG_DATATYPE("Wrong datatype"), + BODY_NOT_PARSEABLE("Could not parse content of request."), + PAYLOAD_NOT_ALLOWED("Request body not allowed for this action."), PAYLOAD_MANDATORY("Request body required for this action."), SECURITY_NOT_INITIALIZED("Security index not initialized"), NULL_ARRAY_ELEMENT("`null` is not allowed as json array element"); diff --git a/src/main/java/org/opensearch/security/dlic/rest/validation/CredentialsValidator.java b/src/main/java/org/opensearch/security/dlic/rest/validation/CredentialsValidator.java index db95660448..a54f1947f0 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/validation/CredentialsValidator.java +++ b/src/main/java/org/opensearch/security/dlic/rest/validation/CredentialsValidator.java @@ -12,7 +12,6 @@ package org.opensearch.security.dlic.rest.validation; import java.util.Map; -import java.util.regex.Pattern; import org.opensearch.common.Strings; import org.opensearch.common.bytes.BytesReference; @@ -22,17 +21,21 @@ import org.opensearch.common.xcontent.XContentType; import org.opensearch.rest.RestRequest; import org.opensearch.security.ssl.util.Utils; -import org.opensearch.security.support.ConfigConstants; /** * Validator for validating password and hash present in the payload */ public class CredentialsValidator extends AbstractConfigurationValidator { - public CredentialsValidator(final RestRequest request, BytesReference ref, final Settings opensearchSettings, + private final PasswordValidator passwordValidator; + + public CredentialsValidator(final RestRequest request, + final BytesReference ref, + final Settings opensearchSettings, Object... param) { super(request, ref, opensearchSettings, param); this.payloadMandatory = true; + this.passwordValidator = PasswordValidator.of(opensearchSettings); allowedKeys.put("hash", DataType.STRING); allowedKeys.put("password", DataType.STRING); } @@ -46,49 +49,29 @@ public boolean validate() { if (!super.validate()) { return false; } - - final String regex = this.opensearchSettings.get(ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_REGEX, null); - if ((request.method() == RestRequest.Method.PUT || request.method() == RestRequest.Method.PATCH) && this.content != null && this.content.length() > 1) { try { final Map contentAsMap = XContentHelper.convertToMap(this.content, false, XContentType.JSON).v2(); - String password = (String) contentAsMap.get("password"); + final String password = (String) contentAsMap.get("password"); if (password != null) { // Password is not allowed to be empty if present. if (password.isEmpty()) { this.errorType = ErrorType.INVALID_PASSWORD; return false; } - - if (!Strings.isNullOrEmpty(regex)) { - // Password can be null for an existing user. Regex will validate password if present - if (!Pattern.compile("^"+regex+"$").matcher(password).matches()) { - if(log.isDebugEnabled()) { - log.debug("Regex does not match password"); - } - this.errorType = ErrorType.INVALID_PASSWORD; - return false; - } - - final String username = Utils.coalesce(request.param("name"), hasParams() ? (String) param[0] : null); - final boolean isDebugEnabled = log.isDebugEnabled(); - - if (username == null || username.isEmpty()) { - if (isDebugEnabled) { - log.debug("Unable to validate username because no user is given"); - } - return false; - } - - if (username.toLowerCase().equals(password.toLowerCase())) { - if (isDebugEnabled) { - log.debug("Username must not match password"); - } - this.errorType = ErrorType.INVALID_PASSWORD; - return false; + final String username = Utils.coalesce(request.param("name"), hasParams() ? (String) param[0] : null); + if (Strings.isNullOrEmpty(username)) { + if (log.isDebugEnabled()) { + log.debug("Unable to validate username because no user is given"); } + return false; + } + final ErrorType passwordValidationResult = passwordValidator.validate(username, password); + if (passwordValidationResult != ErrorType.NONE) { + this.errorType = passwordValidationResult; + return false; } } } catch (NotXContentException e) { @@ -99,4 +82,5 @@ public boolean validate() { } return true; } + } diff --git a/src/main/java/org/opensearch/security/dlic/rest/validation/PasswordValidator.java b/src/main/java/org/opensearch/security/dlic/rest/validation/PasswordValidator.java new file mode 100644 index 0000000000..a59a4e6d40 --- /dev/null +++ b/src/main/java/org/opensearch/security/dlic/rest/validation/PasswordValidator.java @@ -0,0 +1,185 @@ +/* + * 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. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.security.dlic.rest.validation; + +import java.util.List; +import java.util.Locale; +import java.util.Objects; +import java.util.StringJoiner; +import java.util.function.Predicate; +import java.util.regex.Pattern; + +import com.google.common.collect.ImmutableList; +import com.nulabinc.zxcvbn.Strength; +import com.nulabinc.zxcvbn.Zxcvbn; +import com.nulabinc.zxcvbn.matchers.Match; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +import org.opensearch.common.Strings; +import org.opensearch.common.settings.Settings; +import org.opensearch.security.dlic.rest.validation.AbstractConfigurationValidator.ErrorType; + +import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_PASSWORD_MIN_LENGTH; +import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_PASSWORD_SCORE_BASED_VALIDATION_STRENGTH; +import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_REGEX; + +public class PasswordValidator { + + private static final int MAX_LENGTH = 100; + + /** + * Checks a username similarity and a password + * names and passwords like: + * - some_user_name/456Some_uSer_Name_1234 + * - some_user_name/some_user_name_Ydfge + * - some_user_name/eman_resu_emos + * are similar + * "user_inputs" - is a default dictionary zxcvbn creates for checking similarity + */ + private final static Predicate USERNAME_SIMILARITY_CHECK = m -> + m.pattern == com.nulabinc.zxcvbn.Pattern.Dictionary && "user_inputs".equals(m.dictionaryName); + + private final Logger logger = LogManager.getLogger(this.getClass()); + + private final int minPasswordLength; + + private final Pattern passwordRegexpPattern; + + private final ScoreStrength scoreStrength; + + private final Zxcvbn zxcvbn; + + private PasswordValidator(final int minPasswordLength, + final Pattern passwordRegexpPattern, + final ScoreStrength scoreStrength) { + this.minPasswordLength = minPasswordLength; + this.passwordRegexpPattern = passwordRegexpPattern; + this.scoreStrength = scoreStrength; + this.zxcvbn = new Zxcvbn(); + } + + public static PasswordValidator of(final Settings settings) { + final String passwordRegex = settings.get(SECURITY_RESTAPI_PASSWORD_VALIDATION_REGEX, null); + final ScoreStrength scoreStrength = ScoreStrength.fromConfiguration( + settings.get(SECURITY_RESTAPI_PASSWORD_SCORE_BASED_VALIDATION_STRENGTH, ScoreStrength.STRONG.name()) + ); + final int minPasswordLength = settings.getAsInt(SECURITY_RESTAPI_PASSWORD_MIN_LENGTH, -1); + return new PasswordValidator( + minPasswordLength, + !Strings.isNullOrEmpty(passwordRegex) ? Pattern.compile(String.format("^%s$", passwordRegex)) : null, + scoreStrength); + } + + ErrorType validate(final String username, final String password) { + if (minPasswordLength > 0 && password.length() < minPasswordLength) { + logger.debug( + "Password is too short, the minimum required length is {}, but current length is {}", + minPasswordLength, + password.length() + ); + return ErrorType.INVALID_PASSWORD; + } + 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 ErrorType.INVALID_PASSWORD; + } + if (Objects.nonNull(passwordRegexpPattern) + && !passwordRegexpPattern.matcher(password).matches()) { + logger.debug("Regex does not match password"); + return ErrorType.INVALID_PASSWORD; + } + final Strength strength = zxcvbn.measure(password, ImmutableList.of(username)); + if (strength.getScore() < scoreStrength.score()) { + logger.debug( + "Password is weak the required score is {}, but current is {}", + scoreStrength, + ScoreStrength.fromScore(strength.getScore()) + ); + return ErrorType.WEAK_PASSWORD; + } + final boolean similar = strength.getSequence() + .stream() + .anyMatch(USERNAME_SIMILARITY_CHECK); + if (similar) { + logger.debug("Password is too similar to the user name {}", username); + return ErrorType.SIMILAR_PASSWORD; + } + return ErrorType.NONE; + } + + public enum ScoreStrength { + + // The weak score defines here only for debugging information + // and doesn't use as a configuration setting value. + WEAK(0, "too guessable: risky password"), + FAIR(1, "very guessable: protection from throttled online attacks"), + GOOD(2, "somewhat guessable: protection from unthrottled online attacks"), + STRONG(3, "safely unguessable: moderate protection from offline slow-hash scenario"), + VERY_STRONG(4, "very unguessable: strong protection from offline slow-hash scenario"); + + private final int score; + + private final String description; + + static final List CONFIGURATION_VALUES = ImmutableList.of(FAIR, STRONG, VERY_STRONG); + + static final String EXPECTED_CONFIGURATION_VALUES = + new StringJoiner(",") + .add(FAIR.name().toLowerCase(Locale.ROOT)) + .add(STRONG.name().toLowerCase(Locale.ROOT)) + .add(VERY_STRONG.name().toLowerCase(Locale.ROOT)) + .toString(); + + private ScoreStrength(final int score, final String description) { + this.score = score; + this.description = description; + } + + public static ScoreStrength fromScore(final int score) { + for (final ScoreStrength strength : values()) { + if (strength.score == score) + return strength; + } + throw new IllegalArgumentException("Unknown score " + score); + } + + public static ScoreStrength fromConfiguration(final String value) { + for (final ScoreStrength strength : CONFIGURATION_VALUES) { + if (strength.name().equalsIgnoreCase(value)) + return strength; + } + throw new IllegalArgumentException( + String.format( + "Setting [%s] cannot be used with the configured: %s. Expected one of [%s]", + SECURITY_RESTAPI_PASSWORD_SCORE_BASED_VALIDATION_STRENGTH, + value, + EXPECTED_CONFIGURATION_VALUES + ) + ); + } + + @Override + public String toString() { + return String.format("Password strength score %s. %s", score, description); + } + + public int score() { + return this.score; + } + + } +} diff --git a/src/main/java/org/opensearch/security/support/ConfigConstants.java b/src/main/java/org/opensearch/security/support/ConfigConstants.java index cc70d904a9..abcb342561 100644 --- a/src/main/java/org/opensearch/security/support/ConfigConstants.java +++ b/src/main/java/org/opensearch/security/support/ConfigConstants.java @@ -253,6 +253,8 @@ public enum RolesMappingResolution { public static final String SECURITY_RESTAPI_ENDPOINTS_DISABLED = "plugins.security.restapi.endpoints_disabled"; public static final String SECURITY_RESTAPI_PASSWORD_VALIDATION_REGEX = "plugins.security.restapi.password_validation_regex"; public static final String SECURITY_RESTAPI_PASSWORD_VALIDATION_ERROR_MESSAGE = "plugins.security.restapi.password_validation_error_message"; + public static final String SECURITY_RESTAPI_PASSWORD_MIN_LENGTH = "plugins.security.restapi.password_min_length"; + public static final String SECURITY_RESTAPI_PASSWORD_SCORE_BASED_VALIDATION_STRENGTH = "plugins.security.restapi.password_score_based_validation_strength"; // Illegal Opcodes from here on public static final String SECURITY_UNSUPPORTED_DISABLE_REST_AUTH_INITIALLY = "plugins.security.unsupported.disable_rest_auth_initially"; diff --git a/src/test/java/org/opensearch/security/auditlog/compliance/RestApiComplianceAuditlogTest.java b/src/test/java/org/opensearch/security/auditlog/compliance/RestApiComplianceAuditlogTest.java index 7d9ca05c2f..04bb41b91b 100644 --- a/src/test/java/org/opensearch/security/auditlog/compliance/RestApiComplianceAuditlogTest.java +++ b/src/test/java/org/opensearch/security/auditlog/compliance/RestApiComplianceAuditlogTest.java @@ -42,7 +42,7 @@ public void testRestApiRolesEnabled() throws Exception { setup(additionalSettings); TestAuditlogImpl.clear(); - String body = "{ \"password\":\"test\",\"backend_roles\":[\"role1\",\"role2\"] }"; + String body = "{ \"password\":\"some new password\",\"backend_roles\":[\"role1\",\"role2\"] }"; HttpResponse response = rh.executePutRequest("_opendistro/_security/api/internalusers/compuser?pretty", body, encodeBasicHeader("admin", "admin")); Thread.sleep(1500); System.out.println(TestAuditlogImpl.sb.toString()); @@ -71,7 +71,7 @@ public void testRestApiRolesDisabled() throws Exception { setup(additionalSettings); TestAuditlogImpl.clear(); - String body = "{ \"password\":\"test\",\"backend_roles\":[\"role1\",\"role2\"] }"; + String body = "{ \"password\":\"some new password\",\"backend_roles\":[\"role1\",\"role2\"] }"; rh.enableHTTPClientSSL = true; rh.trustHTTPServerCertificate = true; @@ -169,12 +169,13 @@ public void testRestApiNewUser() throws Exception { setup(additionalSettings); TestAuditlogImpl.clear(); - String body = "{ \"password\":\"test\",\"backend_roles\":[\"role1\",\"role2\"] }"; + String body = "{ \"password\":\"some new password\",\"backend_roles\":[\"role1\",\"role2\"] }"; System.out.println("exec"); - HttpResponse response = rh.executePutRequest("_opendistro/_security/api/internalusers/compuser?pretty", body, encodeBasicHeader("admin", "admin")); + HttpResponse response = rh.executePutRequest("_opendistro/_security/api/internalusers/compuser?pretty", + body, encodeBasicHeader("admin", "admin")); Thread.sleep(1500); System.out.println(TestAuditlogImpl.sb.toString()); - Assert.assertEquals(HttpStatus.SC_CREATED, response.getStatusCode()); + Assert.assertEquals(response.getBody(), HttpStatus.SC_CREATED, response.getStatusCode()); Assert.assertTrue(TestAuditlogImpl.messages.size()+"", TestAuditlogImpl.messages.isEmpty()); } @@ -241,7 +242,7 @@ public void testBCryptHashRedaction() throws Exception { // create internal user and verify no BCrypt hash is present in audit logs TestAuditlogImpl.clear(); - rh.executePutRequest("/_opendistro/_security/api/internalusers/test", "{ \"password\":\"test\"}"); + rh.executePutRequest("/_opendistro/_security/api/internalusers/test", "{ \"password\":\"some new user password\"}"); Assert.assertEquals(1, TestAuditlogImpl.messages.size()); Assert.assertFalse(AuditMessage.BCRYPT_HASH.matcher(TestAuditlogImpl.sb.toString()).matches()); } diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/AbstractRestApiUnitTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/AbstractRestApiUnitTest.java index 59e8feb198..20eb782853 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/api/AbstractRestApiUnitTest.java +++ b/src/test/java/org/opensearch/security/dlic/rest/api/AbstractRestApiUnitTest.java @@ -16,6 +16,7 @@ import java.util.Collection; import java.util.HashMap; import java.util.Map; +import java.util.Objects; import com.fasterxml.jackson.core.JsonParseException; import com.fasterxml.jackson.core.type.TypeReference; @@ -28,12 +29,15 @@ import org.opensearch.plugins.Plugin; import org.opensearch.security.DefaultObjectMapper; import org.opensearch.security.auditlog.AuditTestUtils; +import org.opensearch.security.dlic.rest.validation.PasswordValidator; import org.opensearch.security.test.DynamicSecurityConfig; import org.opensearch.security.test.SingleClusterTest; import org.opensearch.security.test.helper.file.FileHelper; import org.opensearch.security.test.helper.rest.RestHelper; import org.opensearch.security.test.helper.rest.RestHelper.HttpResponse; +import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_PASSWORD_SCORE_BASED_VALIDATION_STRENGTH; + public abstract class AbstractRestApiUnitTest extends SingleClusterTest { protected RestHelper rh = null; @@ -49,6 +53,7 @@ protected final void setup() throws Exception { Settings.Builder builder = Settings.builder(); builder.put("plugins.security.ssl.http.enabled", true) + .put(SECURITY_RESTAPI_PASSWORD_SCORE_BASED_VALIDATION_STRENGTH, PasswordValidator.ScoreStrength.FAIR.name()) .put("plugins.security.ssl.http.keystore_filepath", FileHelper.getAbsoluteFilePathFromClassPath("restapi/node-0-keystore.jks")) .put("plugins.security.ssl.http.truststore_filepath", @@ -85,6 +90,7 @@ protected final void setupWithRestRoles(Settings nodeOverride) throws Exception Settings.Builder builder = Settings.builder(); builder.put("plugins.security.ssl.http.enabled", true) + .put(SECURITY_RESTAPI_PASSWORD_SCORE_BASED_VALIDATION_STRENGTH, PasswordValidator.ScoreStrength.FAIR.name()) .put("plugins.security.ssl.http.keystore_filepath", FileHelper.getAbsoluteFilePathFromClassPath("restapi/node-0-keystore.jks")) .put("plugins.security.ssl.http.truststore_filepath", @@ -123,12 +129,19 @@ protected void deleteUser(String username) throws Exception { } protected void addUserWithPassword(String username, String password, int status) throws Exception { + addUserWithPassword(username, password, status, null); + } + + protected void addUserWithPassword(String username, String password, int status, String message) throws Exception { boolean sendAdminCertificate = rh.sendAdminCertificate; rh.sendAdminCertificate = true; HttpResponse response = rh.executePutRequest("/_opendistro/_security/api/internalusers/" + username, "{\"password\": \"" + password + "\"}", new Header[0]); Assert.assertEquals(status, response.getStatusCode()); rh.sendAdminCertificate = sendAdminCertificate; + if (Objects.nonNull(message)) { + Assert.assertTrue(response.getBody().contains(message)); + } } protected void addUserWithPassword(String username, String password, String[] roles, int status) throws Exception { diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/AccountApiTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/AccountApiTest.java index c1840524c9..88b8778888 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/api/AccountApiTest.java +++ b/src/test/java/org/opensearch/security/dlic/rest/api/AccountApiTest.java @@ -45,7 +45,7 @@ public void testGetAccount() throws Exception { // arrange setup(); final String testUser = "test-user"; - final String testPass = "test-pass"; + final String testPass = "some password for user"; addUserWithPassword(testUser, testPass, HttpStatus.SC_CREATED); // test - unauthorized access as credentials are missing. diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/ActionGroupsApiTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/ActionGroupsApiTest.java index 09efae9fbe..0e8a0a715f 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/api/ActionGroupsApiTest.java +++ b/src/test/java/org/opensearch/security/dlic/rest/api/ActionGroupsApiTest.java @@ -87,11 +87,11 @@ public void testActionGroupsApi() throws Exception { setupStarfleetIndex(); // add user picard, role starfleet, maps to opendistro_security_role_starfleet - addUserWithPassword("picard", "picard", new String[] { "starfleet" }, HttpStatus.SC_CREATED); - checkReadAccess(HttpStatus.SC_OK, "picard", "picard", "sf", "_doc", 0); + addUserWithPassword("picard", "picardpicardpicard", new String[] { "starfleet" }, HttpStatus.SC_CREATED); + checkReadAccess(HttpStatus.SC_OK, "picard", "picardpicardpicard", "sf", "_doc", 0); // TODO: only one doctype allowed for ES6 // checkReadAccess(HttpStatus.SC_OK, "picard", "picard", "sf", "public", 0); - checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picard", "sf", "_doc", 0); + checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicard", "sf", "_doc", 0); // TODO: only one doctype allowed for ES6 //checkWriteAccess(HttpStatus.SC_OK, "picard", "picard", "sf", "public", 0); @@ -109,22 +109,22 @@ public void testActionGroupsApi() throws Exception { Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); rh.sendAdminCertificate = false; - checkReadAccess(HttpStatus.SC_FORBIDDEN, "picard", "picard", "sf", "_doc", 0); + checkReadAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicard", "sf", "_doc", 0); // put picard in captains role. Role opendistro_security_role_captains uses the CRUD_UT // action group // which uses READ_UT and WRITE action groups. We removed READ_UT, so only // WRITE is possible - addUserWithPassword("picard", "picard", new String[] { "captains" }, HttpStatus.SC_OK); - checkWriteAccess(HttpStatus.SC_OK, "picard", "picard", "sf", "_doc", 0); - checkReadAccess(HttpStatus.SC_FORBIDDEN, "picard", "picard", "sf", "_doc", 0); + addUserWithPassword("picard", "picardpicardpicard", new String[] { "captains" }, HttpStatus.SC_OK); + checkWriteAccess(HttpStatus.SC_OK, "picard", "picardpicardpicard", "sf", "_doc", 0); + checkReadAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicard", "sf", "_doc", 0); // now remove also CRUD_UT groups, write also not possible anymore rh.sendAdminCertificate = true; response = rh.executeDeleteRequest(ENDPOINT+"/CRUD_UT", new Header[0]); rh.sendAdminCertificate = false; - checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picard", "sf", "_doc", 0); - checkReadAccess(HttpStatus.SC_FORBIDDEN, "picard", "picard", "sf", "_doc", 0); + checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicard", "sf", "_doc", 0); + checkReadAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicard", "sf", "_doc", 0); // -- PUT @@ -148,8 +148,8 @@ public void testActionGroupsApi() throws Exception { rh.sendAdminCertificate = false; // write access allowed again, read forbidden, since READ_UT group is still missing - checkReadAccess(HttpStatus.SC_FORBIDDEN, "picard", "picard", "sf", "_doc", 0); - checkWriteAccess(HttpStatus.SC_OK, "picard", "picard", "sf", "_doc", 0); + checkReadAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicard", "sf", "_doc", 0); + checkWriteAccess(HttpStatus.SC_OK, "picard", "picardpicardpicard", "sf", "_doc", 0); // restore READ_UT action groups rh.sendAdminCertificate = true; @@ -158,8 +158,8 @@ public void testActionGroupsApi() throws Exception { rh.sendAdminCertificate = false; // read/write allowed again - checkReadAccess(HttpStatus.SC_OK, "picard", "picard", "sf", "_doc", 0); - checkWriteAccess(HttpStatus.SC_OK, "picard", "picard", "sf", "_doc", 0); + checkReadAccess(HttpStatus.SC_OK, "picard", "picardpicardpicard", "sf", "_doc", 0); + checkWriteAccess(HttpStatus.SC_OK, "picard", "picardpicardpicard", "sf", "_doc", 0); // -- PUT, new JSON format including readonly flag, disallowed in REST API rh.sendAdminCertificate = true; diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/RolesApiTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/RolesApiTest.java index 8dc18f5043..e2359ecd8e 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/api/RolesApiTest.java +++ b/src/test/java/org/opensearch/security/dlic/rest/api/RolesApiTest.java @@ -159,9 +159,9 @@ public void testRolesApi() throws Exception { setupStarfleetIndex(); // add user picard, role starfleet, maps to opendistro_security_role_starfleet - addUserWithPassword("picard", "picard", new String[] { "starfleet", "captains" }, HttpStatus.SC_CREATED); - checkReadAccess(HttpStatus.SC_OK, "picard", "picard", "sf", "_doc", 0); - checkWriteAccess(HttpStatus.SC_OK, "picard", "picard", "sf", "_doc", 0); + addUserWithPassword("picard", "picardpicardpicardpicard", new String[] { "starfleet", "captains" }, HttpStatus.SC_CREATED); + checkReadAccess(HttpStatus.SC_OK, "picard", "picardpicardpicardpicard", "sf", "_doc", 0); + checkWriteAccess(HttpStatus.SC_OK, "picard", "picardpicardpicardpicard", "sf", "_doc", 0); // -- DELETE @@ -188,18 +188,18 @@ public void testRolesApi() throws Exception { rh.sendAdminCertificate = false; // user has only role starfleet left, role has READ access only - checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picard", "sf", "_doc", 1); + checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicardpicard", "sf", "_doc", 1); // ES7 only supports one doc type, but OpenSearch permission checks run first // So we also get a 403 FORBIDDEN when tring to add new document type - checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picard", "sf", "_doc", 0); + checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicardpicard", "sf", "_doc", 0); rh.sendAdminCertificate = true; // remove also starfleet role, nothing is allowed anymore response = rh.executeDeleteRequest(ENDPOINT + "/roles/opendistro_security_role_starfleet", new Header[0]); Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - checkReadAccess(HttpStatus.SC_FORBIDDEN, "picard", "picard", "sf", "_doc", 0); - checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picard", "sf", "_doc", 0); + checkReadAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicardpicard", "sf", "_doc", 0); + checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicardpicard", "sf", "_doc", 0); // -- PUT // put with empty roles, must fail @@ -249,14 +249,14 @@ public void testRolesApi() throws Exception { FileHelper.loadFile("restapi/roles_starfleet.json"), new Header[0]); Assert.assertEquals(HttpStatus.SC_CREATED, response.getStatusCode()); rh.sendAdminCertificate = false; - checkReadAccess(HttpStatus.SC_OK, "picard", "picard", "sf", "_doc", 0); + checkReadAccess(HttpStatus.SC_OK, "picard", "picardpicardpicardpicard", "sf", "_doc", 0); // now picard is only in opendistro_security_role_starfleet, which has write access to // all indices. We collapse all document types in ODFE7 so this permission in the // starfleet role grants all permissions: // _doc: // - 'indices:*' - checkWriteAccess(HttpStatus.SC_OK, "picard", "picard", "sf", "_doc", 0); + checkWriteAccess(HttpStatus.SC_OK, "picard", "picardpicardpicardpicard", "sf", "_doc", 0); rh.sendAdminCertificate = true; @@ -265,8 +265,8 @@ public void testRolesApi() throws Exception { FileHelper.loadFile("restapi/roles_captains.json"), new Header[0]); Assert.assertEquals(HttpStatus.SC_CREATED, response.getStatusCode()); rh.sendAdminCertificate = false; - checkReadAccess(HttpStatus.SC_OK, "picard", "picard", "sf", "_doc", 0); - checkWriteAccess(HttpStatus.SC_OK, "picard", "picard", "sf", "_doc", 0); + checkReadAccess(HttpStatus.SC_OK, "picard", "picardpicardpicardpicard", "sf", "_doc", 0); + checkWriteAccess(HttpStatus.SC_OK, "picard", "picardpicardpicardpicard", "sf", "_doc", 0); rh.sendAdminCertificate = true; response = rh.executePutRequest(ENDPOINT + "/roles/opendistro_security_role_starfleet_captains", diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/RolesMappingApiTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/RolesMappingApiTest.java index 2d1f10736d..6be5ceddb7 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/api/RolesMappingApiTest.java +++ b/src/test/java/org/opensearch/security/dlic/rest/api/RolesMappingApiTest.java @@ -96,8 +96,8 @@ public void testRolesMappingApi() throws Exception { // add user picard, role captains initially maps to // opendistro_security_role_starfleet_captains and opendistro_security_role_starfleet - addUserWithPassword("picard", "picard", new String[] { "captains" }, HttpStatus.SC_CREATED); - checkWriteAccess(HttpStatus.SC_CREATED, "picard", "picard", "sf", "_doc", 1); + addUserWithPassword("picard", "picardpicardpicardpicard", new String[] { "captains" }, HttpStatus.SC_CREATED); + checkWriteAccess(HttpStatus.SC_CREATED, "picard", "picardpicardpicardpicard", "sf", "_doc", 1); // TODO: only one doctype allowed for ES6 //checkWriteAccess(HttpStatus.SC_CREATED, "picard", "picard", "sf", "_doc", 1); @@ -128,7 +128,7 @@ public void testRolesMappingApi() throws Exception { // now picard is only in opendistro_security_role_starfleet, which has write access to // public, but not to _doc - checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picard", "sf", "_doc", 1); + checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicardpicard", "sf", "_doc", 1); // TODO: only one doctype allowed for ES6 // checkWriteAccess(HttpStatus.SC_OK, "picard", "picard", "sf", "_doc", 1); @@ -324,14 +324,14 @@ public void testRolesMappingApi() throws Exception { private void checkAllSfAllowed() throws Exception { rh.sendAdminCertificate = false; - checkReadAccess(HttpStatus.SC_OK, "picard", "picard", "sf", "_doc", 1); - checkWriteAccess(HttpStatus.SC_OK, "picard", "picard", "sf", "_doc", 1); + checkReadAccess(HttpStatus.SC_OK, "picard", "picardpicardpicardpicard", "sf", "_doc", 1); + checkWriteAccess(HttpStatus.SC_OK, "picard", "picardpicardpicardpicard", "sf", "_doc", 1); } private void checkAllSfForbidden() throws Exception { rh.sendAdminCertificate = false; - checkReadAccess(HttpStatus.SC_FORBIDDEN, "picard", "picard", "sf", "_doc", 1); - checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picard", "sf", "_doc", 1); + checkReadAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicardpicard", "sf", "_doc", 1); + checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicardpicard", "sf", "_doc", 1); } private HttpResponse deleteAndputNewMapping(String fileName) throws Exception { diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/UserApiTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/UserApiTest.java index ea6101e429..0c509e6bbd 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/api/UserApiTest.java +++ b/src/test/java/org/opensearch/security/dlic/rest/api/UserApiTest.java @@ -24,6 +24,7 @@ import org.opensearch.common.settings.Settings; import org.opensearch.common.xcontent.XContentType; import org.opensearch.security.dlic.rest.validation.AbstractConfigurationValidator; +import org.opensearch.security.dlic.rest.validation.PasswordValidator; import org.opensearch.security.securityconf.impl.CType; import org.opensearch.security.support.ConfigConstants; import org.opensearch.security.test.helper.file.FileHelper; @@ -88,14 +89,14 @@ public void testSecurityRoles() throws Exception { Assert.assertEquals(response.getBody(), HttpStatus.SC_OK, response.getStatusCode()); Settings settings = Settings.builder().loadFromSource(response.getBody(), XContentType.JSON).build(); Assert.assertEquals(USER_SETTING_SIZE, settings.size()); - response = rh.executePatchRequest(ENDPOINT + "/internalusers", "[{ \"op\": \"add\", \"path\": \"/newuser\", \"value\": {\"password\": \"newuser\", \"opendistro_security_roles\": [\"opendistro_security_all_access\"] } }]", new Header[0]); + response = rh.executePatchRequest(ENDPOINT + "/internalusers", "[{ \"op\": \"add\", \"path\": \"/newuser\", \"value\": {\"password\": \"fair password for the user\", \"opendistro_security_roles\": [\"opendistro_security_all_access\"] } }]", new Header[0]); Assert.assertEquals(response.getBody(), HttpStatus.SC_OK, response.getStatusCode()); response = rh.executeGetRequest(ENDPOINT + "/internalusers/newuser", new Header[0]); Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); Assert.assertTrue(response.getBody().contains("\"opendistro_security_roles\":[\"opendistro_security_all_access\"]")); - checkGeneralAccess(HttpStatus.SC_OK, "newuser", "newuser"); + checkGeneralAccess(HttpStatus.SC_OK, "newuser", "fair password for the user"); } @Test @@ -106,7 +107,7 @@ public void testParallelPutRequests() throws Exception { rh.keystore = "restapi/kirk-keystore.jks"; rh.sendAdminCertificate = true; - HttpResponse[] responses = rh.executeMultipleAsyncPutRequest(10, ENDPOINT + "/internalusers/test1", "{\"password\":\"test1\"}"); + HttpResponse[] responses = rh.executeMultipleAsyncPutRequest(10, ENDPOINT + "/internalusers/test1", "{\"password\":\"test1test1test1test1test1test1\"}"); boolean created = false; for (HttpResponse response : responses) { int sc = response.getStatusCode(); @@ -252,7 +253,7 @@ private void verifyPatch(final boolean sendAdminCert, Header... restAdminHeader) // PATCH password rh.sendAdminCertificate = sendAdminCert; - response = rh.executePatchRequest(ENDPOINT + "/internalusers/test", "[{ \"op\": \"add\", \"path\": \"/password\", \"value\": \"neu\" }]", restAdminHeader); + response = rh.executePatchRequest(ENDPOINT + "/internalusers/test", "[{ \"op\": \"add\", \"path\": \"/password\", \"value\": \"neu password 42\" }]", restAdminHeader); Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); response = rh.executeGetRequest(ENDPOINT + "/internalusers/test", restAdminHeader); Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); @@ -290,7 +291,7 @@ private void verifyPatch(final boolean sendAdminCert, Header... restAdminHeader) // PATCH rh.sendAdminCertificate = sendAdminCert; response = rh.executePatchRequest(ENDPOINT + "/internalusers", - "[{ \"op\": \"add\", \"path\": \"/bulknew1\", \"value\": {\"password\": \"bla\", \"backend_roles\": [\"vulcan\"] } }]", restAdminHeader); + "[{ \"op\": \"add\", \"path\": \"/bulknew1\", \"value\": {\"password\": \"bla bla bla password 42\", \"backend_roles\": [\"vulcan\"] } }]", restAdminHeader); Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); response = rh.executeGetRequest(ENDPOINT + "/internalusers/bulknew1", restAdminHeader); Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); @@ -488,24 +489,24 @@ private void verifyRoles(final boolean sendAdminCert, Header... header) throws E // use backendroles when creating user. User picard does not exist in // the internal user DB // and is also not assigned to any role by username - addUserWithPassword("picard", "picard", HttpStatus.SC_CREATED); + addUserWithPassword("picard", "picardpicardpicardpicardpicard", HttpStatus.SC_CREATED); // changed in ES5, you now need cluster:monitor/main which pucard does not have - checkGeneralAccess(HttpStatus.SC_FORBIDDEN, "picard", "picard"); + checkGeneralAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicardpicardpicard"); // check read access to starfleet index and _doc type, must fail - checkReadAccess(HttpStatus.SC_FORBIDDEN, "picard", "picard", "sf", "_doc", 0); + checkReadAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicardpicardpicard", "sf", "_doc", 0); // overwrite user picard, and give him role "starfleet". - addUserWithPassword("picard", "picard", new String[]{"starfleet"}, HttpStatus.SC_OK); + addUserWithPassword("picard", "picardpicardpicardpicardpicard", new String[]{"starfleet"}, HttpStatus.SC_OK); - checkReadAccess(HttpStatus.SC_OK, "picard", "picard", "sf", "_doc", 0); - checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picard", "sf", "_doc", 1); + checkReadAccess(HttpStatus.SC_OK, "picard", "picardpicardpicardpicardpicard", "sf", "_doc", 0); + checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicardpicardpicard", "sf", "_doc", 1); // overwrite user picard, and give him role "starfleet" plus "captains. Now // document can be created. - addUserWithPassword("picard", "picard", new String[]{"starfleet", "captains"}, HttpStatus.SC_OK); - checkReadAccess(HttpStatus.SC_OK, "picard", "picard", "sf", "_doc", 0); - checkWriteAccess(HttpStatus.SC_CREATED, "picard", "picard", "sf", "_doc", 1); + addUserWithPassword("picard", "picardpicardpicardpicardpicard", new String[]{"starfleet", "captains"}, HttpStatus.SC_OK); + checkReadAccess(HttpStatus.SC_OK, "picard", "picardpicardpicardpicardpicard", "sf", "_doc", 0); + checkWriteAccess(HttpStatus.SC_CREATED, "picard", "picardpicardpicardpicardpicard", "sf", "_doc", 1); rh.sendAdminCertificate = sendAdminCert; response = rh.executeGetRequest(ENDPOINT + "/internalusers/picard", header); @@ -518,8 +519,8 @@ private void verifyRoles(final boolean sendAdminCert, Header... header) throws E Assert.assertTrue(roles.contains("starfleet")); Assert.assertTrue(roles.contains("captains")); - addUserWithPassword("$1aAAAAAAAAC", "$1aAAAAAAAAC", HttpStatus.SC_CREATED); - addUserWithPassword("abc", "abc", HttpStatus.SC_CREATED); + addUserWithPassword("some_additional_user", "$1aAAAAAAAAC", HttpStatus.SC_CREATED); + addUserWithPassword("abc", "abcabcabcabc42", HttpStatus.SC_CREATED); // check tabs in json response = rh.executePutRequest(ENDPOINT + "/internalusers/userwithtabs", "\t{\"hash\": \t \"123\"\t} ", header); @@ -527,14 +528,15 @@ private void verifyRoles(final boolean sendAdminCert, Header... header) throws E } @Test - public void testPasswordRules() throws Exception { - + public void testRegExpPasswordRules() throws Exception { Settings nodeSettings = - Settings.builder() - .put(ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_ERROR_MESSAGE, "xxx") - .put(ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_REGEX, - "(?=.*[A-Z])(?=.*[^a-zA-Z\\\\d])(?=.*[0-9])(?=.*[a-z]).{8,}") - .build(); + Settings.builder() + .put(ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_ERROR_MESSAGE, "xxx") + .put(ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_REGEX, + "(?=.*[A-Z])(?=.*[^a-zA-Z\\\\d])(?=.*[0-9])(?=.*[a-z]).{8,}") + .put(ConfigConstants.SECURITY_RESTAPI_PASSWORD_SCORE_BASED_VALIDATION_STRENGTH, + PasswordValidator.ScoreStrength.FAIR.name()) + .build(); setup(nodeSettings); @@ -543,79 +545,142 @@ public void testPasswordRules() throws Exception { // initial configuration, 6 users HttpResponse response = rh - .executeGetRequest("_plugins/_security/api/" + CType.INTERNALUSERS.toLCString()); - Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); + .executeGetRequest("_plugins/_security/api/" + CType.INTERNALUSERS.toLCString()); + Assert.assertEquals(response.getBody(), HttpStatus.SC_OK, response.getStatusCode()); Settings settings = Settings.builder().loadFromSource(response.getBody(), XContentType.JSON).build(); Assert.assertEquals(USER_SETTING_SIZE, settings.size()); - addUserWithPassword("tooshoort", "", HttpStatus.SC_BAD_REQUEST); - addUserWithPassword("tooshoort", "123", HttpStatus.SC_BAD_REQUEST); - addUserWithPassword("tooshoort", "1234567", HttpStatus.SC_BAD_REQUEST); - addUserWithPassword("tooshoort", "1Aa%", HttpStatus.SC_BAD_REQUEST); - addUserWithPassword("no-nonnumeric", "123456789", HttpStatus.SC_BAD_REQUEST); - addUserWithPassword("no-uppercase", "a123456789", HttpStatus.SC_BAD_REQUEST); - addUserWithPassword("no-lowercase", "A123456789", HttpStatus.SC_BAD_REQUEST); - addUserWithPassword("ok1", "a%A123456789", HttpStatus.SC_CREATED); - addUserWithPassword("ok2", "$aA123456789", HttpStatus.SC_CREATED); - addUserWithPassword("ok3", "$Aa123456789", HttpStatus.SC_CREATED); - addUserWithPassword("ok4", "$1aAAAAAAAAA", HttpStatus.SC_CREATED); - addUserWithPassword("empty_password_no_hash", "", HttpStatus.SC_BAD_REQUEST); + verifyCouldNotCreatePasswords(HttpStatus.SC_BAD_REQUEST); + verifyCanCreatePasswords(); + verifySimilarity("xxx"); + addUserWithPasswordAndHash("empty_password", "", "$%^123", HttpStatus.SC_BAD_REQUEST); addUserWithPasswordAndHash("null_password", null, "$%^123", HttpStatus.SC_BAD_REQUEST); - response = rh.executePatchRequest(PLUGINS_PREFIX + "/api/internalusers", "[{ \"op\": \"add\", \"path\": \"/ok4\", \"value\": {\"password\": \"bla\", \"backend_roles\": [\"vulcan\"] } }]", new Header[0]); - Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode()); - - response = rh.executePatchRequest(PLUGINS_PREFIX + "/api/internalusers", "[{ \"op\": \"replace\", \"path\": \"/ok4\", \"value\": {\"password\": \"bla\", \"backend_roles\": [\"vulcan\"] } }]", new Header[0]); - Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode()); - - addUserWithPassword("ok4", "123", HttpStatus.SC_BAD_REQUEST); + response = rh.executeGetRequest(PLUGINS_PREFIX + "/api/internalusers/nothinghthere?pretty", new Header[0]); + Assert.assertEquals(HttpStatus.SC_NOT_FOUND, response.getStatusCode()); + Assert.assertTrue(response.getBody().contains("NOT_FOUND")); + } - response = rh.executePatchRequest(PLUGINS_PREFIX + "/api/internalusers", "[{ \"op\": \"add\", \"path\": \"/ok4\", \"value\": {\"password\": \"$1aAAAAAAAAB\", \"backend_roles\": [\"vulcan\"] } }]", new Header[0]); - Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - addUserWithPassword("ok4", "$1aAAAAAAAAC", HttpStatus.SC_OK); + private void verifyCouldNotCreatePasswords(final int expectedStatus) throws Exception { + addUserWithPassword("tooshoort", "", expectedStatus); + addUserWithPassword("tooshoort", "123",expectedStatus); + addUserWithPassword("tooshoort", "1234567", expectedStatus); + addUserWithPassword("tooshoort", "1Aa%", expectedStatus); + addUserWithPassword("no-nonnumeric", "123456789", expectedStatus); + addUserWithPassword("no-uppercase", "a123456789", expectedStatus); + addUserWithPassword("no-lowercase", "A123456789", expectedStatus); + addUserWithPassword("empty_password_no_hash", "", expectedStatus); + HttpResponse response = rh.executePatchRequest( + PLUGINS_PREFIX + "/api/internalusers", + "[{ \"op\": \"add\", \"path\": \"/ok4\", \"value\": {\"password\": \"bla\", \"backend_roles\": [\"vulcan\"] } }]", + new Header[0] + ); + Assert.assertEquals(response.getBody(), expectedStatus, response.getStatusCode()); + response = rh.executePatchRequest(PLUGINS_PREFIX + "/api/internalusers", "[{ \"op\": \"replace\", \"path\": \"/ok4\", \"value\": {\"password\": \"bla\", \"backend_roles\": [\"vulcan\"] } }]", new Header[0]); + Assert.assertEquals(response.getBody(), expectedStatus, response.getStatusCode()); + addUserWithPassword("ok4", "123", expectedStatus); //its not allowed to use the username as password (case insensitive) response = rh.executePatchRequest(PLUGINS_PREFIX + "/api/internalusers", "[{ \"op\": \"add\", \"path\": \"/$1aAAAAAAAAB\", \"value\": {\"password\": \"$1aAAAAAAAAB\", \"backend_roles\": [\"vulcan\"] } }]", new Header[0]); - Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode()); - addUserWithPassword("$1aAAAAAAAAC", "$1aAAAAAAAAC", HttpStatus.SC_BAD_REQUEST); - addUserWithPassword("$1aAAAAAAAac", "$1aAAAAAAAAC", HttpStatus.SC_BAD_REQUEST); - addUserWithPassword(URLEncoder.encode("$1aAAAAAAAac%", "UTF-8"), "$1aAAAAAAAAC%", HttpStatus.SC_BAD_REQUEST); - addUserWithPassword(URLEncoder.encode("$1aAAAAAAAac%!=\"/\\;:test&~@^", "UTF-8").replace("+", "%2B"), "$1aAAAAAAAac%!=\\\"/\\\\;:test&~@^", HttpStatus.SC_BAD_REQUEST); - addUserWithPassword(URLEncoder.encode("$1aAAAAAAAac%!=\"/\\;: test&", "UTF-8"), "$1aAAAAAAAac%!=\\\"/\\\\;: test&123", HttpStatus.SC_BAD_REQUEST); - - response = rh.executeGetRequest(PLUGINS_PREFIX + "/api/internalusers/nothinghthere?pretty", new Header[0]); - Assert.assertEquals(HttpStatus.SC_NOT_FOUND, response.getStatusCode()); - Assert.assertTrue(response.getBody().contains("NOT_FOUND")); - + Assert.assertEquals(response.getBody(), expectedStatus, response.getStatusCode()); + addUserWithPassword("$1aAAAAAAAAC", "$1aAAAAAAAAC", expectedStatus); + addUserWithPassword("$1aAAAAAAAac", "$1aAAAAAAAAC", expectedStatus); + addUserWithPassword(URLEncoder.encode("$1aAAAAAAAac%", "UTF-8"), "$1aAAAAAAAAC%", expectedStatus); + addUserWithPassword(URLEncoder.encode("$1aAAAAAAAac%!=\"/\\;:test&~@^", "UTF-8").replace("+", "%2B"), "$1aAAAAAAAac%!=\\\"/\\\\;:test&~@^", expectedStatus); + addUserWithPassword(URLEncoder.encode("$1aAAAAAAAac%!=\"/\\;: test&", "UTF-8"), "$1aAAAAAAAac%!=\\\"/\\\\;: test&123", expectedStatus); String patchPayload = "[ " + - "{ \"op\": \"add\", \"path\": \"/testuser1\", \"value\": { \"password\": \"$aA123456789\", \"backend_roles\": [\"testrole1\"] } }," + - "{ \"op\": \"add\", \"path\": \"/testuser2\", \"value\": { \"password\": \"testpassword2\", \"backend_roles\": [\"testrole2\"] } }" + - "]"; + "{ \"op\": \"add\", \"path\": \"/testuser1\", \"value\": { \"password\": \"$aA123456789\", \"backend_roles\": [\"testrole1\"] } }," + + "{ \"op\": \"add\", \"path\": \"/testuser2\", \"value\": { \"password\": \"testpassword2\", \"backend_roles\": [\"testrole2\"] } }" + + "]"; response = rh.executePatchRequest(PLUGINS_PREFIX + "/api/internalusers", patchPayload, new BasicHeader("Content-Type", "application/json")); - Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode()); + Assert.assertEquals(expectedStatus, response.getStatusCode()); Assert.assertTrue(response.getBody().contains("error")); Assert.assertTrue(response.getBody().contains("xxx")); response = rh.executePutRequest(PLUGINS_PREFIX + "/api/internalusers/ok1", "{\"backend_roles\":[\"my-backend-role\"],\"attributes\":{},\"password\":\"\"}", new Header[0]); - Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode()); + Assert.assertEquals(expectedStatus, response.getStatusCode()); + response = rh.executePutRequest(PLUGINS_PREFIX + "/api/internalusers/ok1", + "{\"backend_roles\":[\"my-backend-role\"],\"attributes\":{},\"password\":\"bla\"}", + new Header[0]); + Assert.assertEquals(expectedStatus, response.getStatusCode()); + } + + private void verifyCanCreatePasswords() throws Exception { + addUserWithPassword("ok1", "a%A123456789", HttpStatus.SC_CREATED); + addUserWithPassword("ok2", "$aA123456789", HttpStatus.SC_CREATED); + addUserWithPassword("ok3", "$Aa123456789", HttpStatus.SC_CREATED); + addUserWithPassword("ok4", "$1aAAAAAAAAA", HttpStatus.SC_CREATED); + addUserWithPassword("ok4", "$1aAAAAAAAAC", HttpStatus.SC_OK); + HttpResponse response = rh.executePatchRequest( + PLUGINS_PREFIX + "/api/internalusers", + "[{ \"op\": \"add\", \"path\": \"/ok4\", \"value\": {\"password\": \"$1aAAAAAAAAB\", \"backend_roles\": [\"vulcan\"] } }]", + new Header[0] + ); + Assert.assertEquals(response.getBody(), HttpStatus.SC_OK, response.getStatusCode()); response = rh.executePutRequest(PLUGINS_PREFIX + "/api/internalusers/ok1", "{\"backend_roles\":[\"my-backend-role\"],\"attributes\":{},\"password\":\"Admin_123\"}", new Header[0]); Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); response = rh.executePutRequest(PLUGINS_PREFIX + "/api/internalusers/ok1", "{\"backend_roles\":[\"my-backend-role\"],\"attributes\":{}}", new Header[0]); Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - response = rh.executePutRequest(PLUGINS_PREFIX + "/api/internalusers/ok1", "{\"backend_roles\":[\"my-backend-role\"],\"attributes\":{},\"password\":\"bla\"}", - new Header[0]); - Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode()); + } + + private void verifySimilarity(final String expectedMessage) throws Exception { + addUserWithPassword( + "some_user_name", "H3235,cc,some_User_Name", + HttpStatus.SC_BAD_REQUEST, + expectedMessage + ); } @Test - public void testUserApiWithDots() throws Exception { + public void testScoreBasedPasswordRules() throws Exception { + + Settings nodeSettings = + Settings.builder() + .put(ConfigConstants.SECURITY_RESTAPI_PASSWORD_MIN_LENGTH, 9) + .build(); + + setup(nodeSettings); + + rh.keystore = "restapi/kirk-keystore.jks"; + rh.sendAdminCertificate = true; + // initial configuration, 6 users + HttpResponse response = rh + .executeGetRequest("_plugins/_security/api/" + CType.INTERNALUSERS.toLCString()); + Assert.assertEquals(response.getBody(), HttpStatus.SC_OK, response.getStatusCode()); + Settings settings = Settings.builder().loadFromSource(response.getBody(), XContentType.JSON).build(); + Assert.assertEquals(USER_SETTING_SIZE, settings.size()); + + addUserWithPassword( + "admin", "password89", + HttpStatus.SC_BAD_REQUEST, + AbstractConfigurationValidator.ErrorType.WEAK_PASSWORD.getMessage() + ); + addUserWithPassword( + "admin", "A123456789", + HttpStatus.SC_BAD_REQUEST, + AbstractConfigurationValidator.ErrorType.WEAK_PASSWORD.getMessage() + ); + + addUserWithPassword( + "admin", "pas", + HttpStatus.SC_BAD_REQUEST, + "Password does not match minimum criteria" + ); + + verifySimilarity(AbstractConfigurationValidator.ErrorType.SIMILAR_PASSWORD.getMessage()); + + addUserWithPassword("some_user_name", "ASSDsadwe324wadaasdadqwe", HttpStatus.SC_CREATED); + } + + @Test + public void testUserApiWithDots() throws Exception { setup(); rh.keystore = "restapi/kirk-keystore.jks"; @@ -623,22 +688,22 @@ public void testUserApiWithDots() throws Exception { // initial configuration, 6 users HttpResponse response = rh - .executeGetRequest(ENDPOINT + "/" + CType.INTERNALUSERS.toLCString()); + .executeGetRequest(ENDPOINT + "/" + CType.INTERNALUSERS.toLCString()); Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); Settings settings = Settings.builder().loadFromSource(response.getBody(), XContentType.JSON).build(); Assert.assertEquals(USER_SETTING_SIZE, settings.size()); addUserWithPassword(".my.dotuser0", "$2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m", - HttpStatus.SC_CREATED); + HttpStatus.SC_CREATED); - addUserWithPassword(".my.dot.user0", "12345678", - HttpStatus.SC_CREATED); + addUserWithPassword(".my.dot.user0", "12345678Sd", + HttpStatus.SC_CREATED); addUserWithHash(".my.dotuser1", "$2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m", - HttpStatus.SC_CREATED); + HttpStatus.SC_CREATED); - addUserWithPassword(".my.dot.user2", "12345678", - HttpStatus.SC_CREATED); + addUserWithPassword(".my.dot.user2", "12345678Sd", + HttpStatus.SC_CREATED); } diff --git a/src/test/java/org/opensearch/security/dlic/rest/validation/PasswordValidatorTest.java b/src/test/java/org/opensearch/security/dlic/rest/validation/PasswordValidatorTest.java new file mode 100644 index 0000000000..b5d27827b8 --- /dev/null +++ b/src/test/java/org/opensearch/security/dlic/rest/validation/PasswordValidatorTest.java @@ -0,0 +1,197 @@ +/* + * 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. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.security.dlic.rest.validation; + +import java.util.List; + +import com.google.common.collect.ImmutableList; +import org.junit.Test; + +import org.opensearch.common.settings.Settings; + +import static org.junit.Assert.assertEquals; +import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_PASSWORD_MIN_LENGTH; +import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_PASSWORD_SCORE_BASED_VALIDATION_STRENGTH; +import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_REGEX; + +public class PasswordValidatorTest { + + static final List WEAK_PASSWORDS = ImmutableList.of( + "q", "5", "&", "admin", "123456", "password" + ); + + static final List FAIR_PASSWORDS = ImmutableList.of( + "p@$$word@dmin", "qwertyuiop@[", + "zxcvbnm,./_", "asdfghjkl;:]", "20300101", + "pandapandapandapandapandapandapandapandapandaa", + "appleappleappleappleappleappleappleappleapplea", + "aelppaaelppaaelppaaelppaaelppaaelppaaelppaaelppa" + ); + + static final List GOOD_PASSWORDS = ImmutableList.of( + "xsw234rfvb", "yaq123edc", "cde345tgbn", "yaqwedcvb", + "Tr0ub4dour&3", "qwER43@!" + ); + + static final List STRONG_PASSWORDS = ImmutableList.of( + "YWert,H90", "Admincc,H90", "Hadmin,120" + ); + + static final List VERY_STRONG_PASSWORDS = ImmutableList.of( + "AeTq($%u-44c_j9NJB45a#2#JP7sH", "IB7~EOw!51gug+7s#+%A9P1O/w8f", + "1v_f%7JvS8w!_t398+ON-CObI#v0", "8lFmfc0!w)&iU9DM6~4_w)D)Y44J" + ); + + static final List SIMILAR_PASSWORDS = ImmutableList.of( + "some_user_name,H2344cc", "H3235,Some_User_Name,cc", + "H3235,cc,some_User_Name", "H3235,SOME_User_Name,cc", + "H3235,eman_resu_emos,cc" + ); + + public void verifyWeakPasswords(final PasswordValidator passwordValidator, + final AbstractConfigurationValidator.ErrorType expectedValidationResult) { + for (final String password : WEAK_PASSWORDS) + assertEquals( + password, + expectedValidationResult, + passwordValidator.validate("some_user_name", password) + ); + + } + + public void verifyFairPasswords(final PasswordValidator passwordValidator, + final AbstractConfigurationValidator.ErrorType expectedValidationResult) { + for (final String password : FAIR_PASSWORDS) + assertEquals( + password, + expectedValidationResult, + passwordValidator.validate("some_user_name", password) + ); + + } + + public void verifyGoodPasswords(final PasswordValidator passwordValidator, + final AbstractConfigurationValidator.ErrorType expectedValidationResult) { + for (final String password : GOOD_PASSWORDS) + assertEquals( + password, + expectedValidationResult, + passwordValidator.validate("some_user_name", password) + ); + + } + + public void verifyStrongPasswords(final PasswordValidator passwordValidator, + final AbstractConfigurationValidator.ErrorType expectedValidationResult) { + for (final String password : STRONG_PASSWORDS) + assertEquals( + password, + expectedValidationResult, + passwordValidator.validate("some_user_name", password) + ); + + } + + public void verifyVeryStrongPasswords(final PasswordValidator passwordValidator, + final AbstractConfigurationValidator.ErrorType expectedValidationResult) { + for (final String password : VERY_STRONG_PASSWORDS) + assertEquals( + password, + expectedValidationResult, + passwordValidator.validate("some_user_name", password) + ); + + } + + public void verifySimilarPasswords(final PasswordValidator passwordValidator) { + for (final String password : SIMILAR_PASSWORDS) + assertEquals( + password, + AbstractConfigurationValidator.ErrorType.SIMILAR_PASSWORD, + passwordValidator.validate("some_user_name", password) + ); + + } + + @Test + public void testRegExpBasedValidation() { + final PasswordValidator passwordValidator = + PasswordValidator.of( + Settings.builder() + .put( + SECURITY_RESTAPI_PASSWORD_VALIDATION_REGEX, + "(?=.*[A-Z])(?=.*[^a-zA-Z\\\\d])(?=.*[0-9])(?=.*[a-z]).{8,}") + .build() + ); + verifyWeakPasswords(passwordValidator, AbstractConfigurationValidator.ErrorType.INVALID_PASSWORD); + verifyFairPasswords(passwordValidator, AbstractConfigurationValidator.ErrorType.INVALID_PASSWORD); + for (final String password : GOOD_PASSWORDS.subList(0, GOOD_PASSWORDS.size() - 2)) + assertEquals( + password, + AbstractConfigurationValidator.ErrorType.INVALID_PASSWORD, + passwordValidator.validate("some_user_name", password) + ); + for (final String password: GOOD_PASSWORDS.subList(GOOD_PASSWORDS.size() - 2, GOOD_PASSWORDS.size())) + assertEquals( + password, + AbstractConfigurationValidator.ErrorType.WEAK_PASSWORD, + passwordValidator.validate("some_user_name", password) + ); + verifyStrongPasswords(passwordValidator, AbstractConfigurationValidator.ErrorType.NONE); + verifyVeryStrongPasswords(passwordValidator, AbstractConfigurationValidator.ErrorType.NONE); + verifySimilarPasswords(passwordValidator); + } + + @Test + public void testMinLength() { + final PasswordValidator passwordValidator = + PasswordValidator.of( + Settings.builder() + .put(SECURITY_RESTAPI_PASSWORD_MIN_LENGTH, 15) + .build() + ); + for (final String password: STRONG_PASSWORDS) { + assertEquals( + AbstractConfigurationValidator.ErrorType.INVALID_PASSWORD, + passwordValidator.validate(password, "some_user_name") + ); + } + + } + + @Test + public void testScoreBasedValidation() { + PasswordValidator passwordValidator = PasswordValidator.of(Settings.EMPTY); + verifyWeakPasswords(passwordValidator, AbstractConfigurationValidator.ErrorType.WEAK_PASSWORD); + verifyFairPasswords(passwordValidator, AbstractConfigurationValidator.ErrorType.WEAK_PASSWORD); + verifyGoodPasswords(passwordValidator, AbstractConfigurationValidator.ErrorType.WEAK_PASSWORD); + verifyStrongPasswords(passwordValidator, AbstractConfigurationValidator.ErrorType.NONE); + verifyVeryStrongPasswords(passwordValidator, AbstractConfigurationValidator.ErrorType.NONE); + verifySimilarPasswords(passwordValidator); + + passwordValidator = + PasswordValidator.of( + Settings.builder() + .put( + SECURITY_RESTAPI_PASSWORD_SCORE_BASED_VALIDATION_STRENGTH, + PasswordValidator.ScoreStrength.FAIR.name() + ).build()); + + verifyWeakPasswords(passwordValidator, AbstractConfigurationValidator.ErrorType.WEAK_PASSWORD); + verifyFairPasswords(passwordValidator, AbstractConfigurationValidator.ErrorType.NONE); + verifyGoodPasswords(passwordValidator, AbstractConfigurationValidator.ErrorType.NONE); + verifyStrongPasswords(passwordValidator, AbstractConfigurationValidator.ErrorType.NONE); + verifyVeryStrongPasswords(passwordValidator, AbstractConfigurationValidator.ErrorType.NONE); + verifySimilarPasswords(passwordValidator); + } + +}