From 54786a21193d786d4ef79d1fd7358e53015c12ad Mon Sep 17 00:00:00 2001 From: Andrey Pleskach Date: Mon, 15 May 2023 20:23:41 +0200 Subject: [PATCH] Add score based password verification (#2557) Signed-off-by: Andrey Pleskach Signed-off-by: Maciej Mierzwa --- build.gradle | 1 + .../security/SecurityConfigurationTests.java | 4 +- .../security/OpenSearchSecurityPlugin.java | 14 ++ .../AbstractConfigurationValidator.java | 22 +- .../rest/validation/CredentialsValidator.java | 52 ++--- .../rest/validation/PasswordValidator.java | 185 ++++++++++++++++ .../security/support/ConfigConstants.java | 4 +- .../RestApiComplianceAuditlogTest.java | 13 +- .../rest/api/AbstractRestApiUnitTest.java | 15 +- .../dlic/rest/api/AccountApiTest.java | 2 +- .../dlic/rest/api/ActionGroupsApiTest.java | 38 ++-- .../security/dlic/rest/api/RolesApiTest.java | 39 ++-- .../dlic/rest/api/RolesMappingApiTest.java | 52 ++--- .../security/dlic/rest/api/UserApiTest.java | 201 ++++++++++++------ .../validation/PasswordValidatorTest.java | 197 +++++++++++++++++ 15 files changed, 661 insertions(+), 178 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 8cfe97ea80..ef187e290f 100644 --- a/build.gradle +++ b/build.gradle @@ -395,6 +395,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/integrationTest/java/org/opensearch/security/SecurityConfigurationTests.java b/src/integrationTest/java/org/opensearch/security/SecurityConfigurationTests.java index 9409809bc8..920fd04e00 100644 --- a/src/integrationTest/java/org/opensearch/security/SecurityConfigurationTests.java +++ b/src/integrationTest/java/org/opensearch/security/SecurityConfigurationTests.java @@ -50,10 +50,10 @@ public class SecurityConfigurationTests { .roles(new Role("limited-role").indexPermissions("indices:data/read/search", "indices:data/read/get").on("user-${user.name}")); public static final String LIMITED_USER_INDEX = "user-" + LIMITED_USER.getName(); public static final String ADDITIONAL_USER_1 = "additional00001"; - public static final String ADDITIONAL_PASSWORD_1 = ADDITIONAL_USER_1; + public static final String ADDITIONAL_PASSWORD_1 = "user 1 fair password"; public static final String ADDITIONAL_USER_2 = "additional2"; - public static final String ADDITIONAL_PASSWORD_2 = ADDITIONAL_USER_2; + public static final String ADDITIONAL_PASSWORD_2 = "user 2 fair password"; public static final String CREATE_USER_BODY = "{\"password\": \"%s\",\"opendistro_security_roles\": []}"; public static final String INTERNAL_USERS_RESOURCE = "_plugins/_security/api/internalusers/"; public static final String ID_1 = "one"; diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index c2fb9f281a..99cbb0e30b 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -138,6 +138,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.HTTPOnBehalfOfJwtAuthenticator; @@ -1047,6 +1048,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 a58639bc0a..83281c9d0b 100644 --- a/src/main/java/org/opensearch/security/support/ConfigConstants.java +++ b/src/main/java/org/opensearch/security/support/ConfigConstants.java @@ -255,7 +255,9 @@ 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"; public static final String SECURITY_UNSUPPORTED_DISABLE_INTERTRANSPORT_AUTH_INITIALLY = "plugins.security.unsupported.disable_intertransport_auth_initially"; public static final String SECURITY_UNSUPPORTED_PASSIVE_INTERTRANSPORT_AUTH_INITIALLY = "plugins.security.unsupported.passive_intertransport_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 0a90f2f396..37ef283ccb 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 a1cb20d99c..7b590f1d46 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 java.util.stream.Stream; import com.fasterxml.jackson.core.JsonParseException; @@ -32,12 +33,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; @@ -53,6 +57,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", @@ -89,6 +94,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", @@ -130,12 +136,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 { @@ -220,7 +233,7 @@ protected String checkWriteAccess(int status, String username, String password, String payload = "{\"value\" : \"true\"}"; HttpResponse response = rh.executePutRequest(action, payload, encodeBasicHeader(username, password)); int returnedStatus = response.getStatusCode(); - Assert.assertEquals(status, returnedStatus); + Assert.assertEquals(response.getBody(), status, returnedStatus); return response.getBody(); } 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 0b91aa35af..9f6bcb65c9 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 abb45df268..926ee23f28 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 @@ -53,9 +53,9 @@ 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); - checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picard", "sf", "_doc", 0); + addUserWithPassword("picard", "picardpicardpicard", new String[] { "starfleet" }, HttpStatus.SC_CREATED); + checkReadAccess(HttpStatus.SC_OK, "picard", "picardpicardpicard", "sf", "_doc", 0); + checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicard", "sf", "_doc", 0); rh.sendAdminCertificate = true; verifyGetForSuperAdmin(new Header[0]); rh.sendAdminCertificate = true; @@ -122,21 +122,21 @@ void verifyDeleteForSuperAdmin(final Header[] header, final boolean userAdminCer 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); } void verifyPutForSuperAdmin(final Header[] header, final boolean userAdminCert) throws Exception { @@ -161,8 +161,8 @@ void verifyPutForSuperAdmin(final Header[] header, final boolean userAdminCert) 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 = userAdminCert; @@ -171,8 +171,8 @@ void verifyPutForSuperAdmin(final Header[] header, final boolean userAdminCert) 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 = userAdminCert; @@ -370,9 +370,9 @@ public void testActionGroupsApiForRestAdmin() throws Exception { final Header restApiAdminHeader = encodeBasicHeader("rest_api_admin_user", "rest_api_admin_user"); // 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); - checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picard", "sf", "_doc", 0); + addUserWithPassword("picard", "picardpicardpicard", new String[] { "starfleet" }, HttpStatus.SC_CREATED); + checkReadAccess(HttpStatus.SC_OK, "picard", "picardpicardpicard", "sf", "_doc", 0); + checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicard", "sf", "_doc", 0); verifyGetForSuperAdmin(new Header[] {restApiAdminHeader}); verifyDeleteForSuperAdmin(new Header[]{restApiAdminHeader}, false); verifyPutForSuperAdmin(new Header[]{restApiAdminHeader}, false); @@ -388,9 +388,9 @@ public void testActionGroupsApiForActionGroupsRestApiAdmin() throws Exception { final Header restApiAdminActionGroupsHeader = encodeBasicHeader("rest_api_admin_actiongroups", "rest_api_admin_actiongroups"); // 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); - checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picard", "sf", "_doc", 0); + addUserWithPassword("picard", "picardpicardpicard", new String[] { "starfleet" }, HttpStatus.SC_CREATED); + checkReadAccess(HttpStatus.SC_OK, "picard", "picardpicardpicard", "sf", "_doc", 0); + checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicard", "sf", "_doc", 0); verifyGetForSuperAdmin(new Header[] {restApiAdminActionGroupsHeader}); verifyDeleteForSuperAdmin(new Header[]{restApiAdminActionGroupsHeader}, false); verifyPutForSuperAdmin(new Header[]{restApiAdminActionGroupsHeader}, false); 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 5a6d1f297c..a85566dc91 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 @@ -151,9 +151,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); rh.sendAdminCertificate = true; verifyGetForSuperAdmin(new Header[0]); @@ -220,17 +220,17 @@ void verifyDeleteForSuperAdmin(final Header[] header, final boolean sendAdminCer Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); 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 = sendAdminCert; // remove also starfleet role, nothing is allowed anymore response = rh.executeDeleteRequest(ENDPOINT + "/roles/opendistro_security_role_starfleet", header); 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); } void verifyPutForSuperAdmin(final Header[] header, final boolean sendAdminCert) throws Exception { @@ -282,14 +282,14 @@ void verifyPutForSuperAdmin(final Header[] header, final boolean sendAdminCert) FileHelper.loadFile("restapi/roles_starfleet.json"), header); 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 = sendAdminCert; @@ -298,19 +298,14 @@ void verifyPutForSuperAdmin(final Header[] header, final boolean sendAdminCert) FileHelper.loadFile("restapi/roles_captains.json"), header); 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 = sendAdminCert; response = rh.executePutRequest(ENDPOINT + "/roles/opendistro_security_role_starfleet_captains", FileHelper.loadFile("restapi/roles_complete_invalid.json"), header); Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode()); -// rh.sendAdminCertificate = sendAdminCert; -// response = rh.executePutRequest(ENDPOINT + "/roles/opendistro_security_role_starfleet_captains", -// FileHelper.loadFile("restapi/roles_multiple.json"), header); -// Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode()); - response = rh.executePutRequest(ENDPOINT + "/roles/opendistro_security_role_starfleet_captains", FileHelper.loadFile("restapi/roles_multiple_2.json"), header); Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode()); @@ -530,9 +525,9 @@ public void testRolesApiWithAllRestApiPermissions() 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); verifyGetForSuperAdmin(new Header[]{restApiAdminHeader}); verifyDeleteForSuperAdmin(new Header[]{restApiAdminHeader}, false); @@ -550,9 +545,9 @@ public void testRolesApiWithRestApiRolePermission() 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); verifyGetForSuperAdmin(new Header[]{restApiRolesHeader}); 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 44e160e151..3bc647bf12 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 @@ -53,8 +53,8 @@ public void testRolesMappingApi() throws Exception { setupStarfleetIndex(); // 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", "picardpicardpicard", new String[] { "captains" }, HttpStatus.SC_CREATED); + checkWriteAccess(HttpStatus.SC_CREATED, "picard", "picardpicardpicard", "sf", "_doc", 1); // TODO: only one doctype allowed for ES6 //checkWriteAccess(HttpStatus.SC_CREATED, "picard", "picard", "sf", "_doc", 1); rh.sendAdminCertificate = true; @@ -65,35 +65,35 @@ public void testRolesMappingApi() throws Exception { verifyPutForSuperAdmin(new Header[0]); verifyPatchForSuperAdmin(new Header[0]); // mapping with several backend roles, one of the is captain - deleteAndputNewMapping(new Header[0],"rolesmapping_backendroles_captains_list.json", true); + deleteAndPutNewMapping(new Header[0],"rolesmapping_backendroles_captains_list.json", true); checkAllSfAllowed(); // mapping with one backend role, captain - deleteAndputNewMapping(new Header[0],"rolesmapping_backendroles_captains_single.json", true); + deleteAndPutNewMapping(new Header[0],"rolesmapping_backendroles_captains_single.json", true); checkAllSfAllowed(); // mapping with several users, one is picard - deleteAndputNewMapping(new Header[0],"rolesmapping_users_picard_list.json", true); + deleteAndPutNewMapping(new Header[0],"rolesmapping_users_picard_list.json", true); checkAllSfAllowed(); // just user picard - deleteAndputNewMapping(new Header[0],"rolesmapping_users_picard_single.json", true); + deleteAndPutNewMapping(new Header[0],"rolesmapping_users_picard_single.json", true); checkAllSfAllowed(); // hosts - deleteAndputNewMapping(new Header[0],"rolesmapping_hosts_list.json", true); + deleteAndPutNewMapping(new Header[0],"rolesmapping_hosts_list.json", true); checkAllSfAllowed(); // hosts - deleteAndputNewMapping(new Header[0],"rolesmapping_hosts_single.json", true); + deleteAndPutNewMapping(new Header[0],"rolesmapping_hosts_single.json", true); checkAllSfAllowed(); // full settings, access - deleteAndputNewMapping(new Header[0],"rolesmapping_all_access.json", true); + deleteAndPutNewMapping(new Header[0],"rolesmapping_all_access.json", true); checkAllSfAllowed(); // full settings, no access - deleteAndputNewMapping(new Header[0],"rolesmapping_all_noaccess.json", true); + deleteAndPutNewMapping(new Header[0],"rolesmapping_all_noaccess.json", true); checkAllSfForbidden(); } @@ -107,8 +107,8 @@ public void testRolesMappingApiWithFullPermissions() throws Exception { setupStarfleetIndex(); // 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", "picardpicardpicard", new String[] { "captains" }, HttpStatus.SC_CREATED); + checkWriteAccess(HttpStatus.SC_CREATED, "picard", "picardpicardpicard", "sf", "_doc", 1); // TODO: only one doctype allowed for ES6 //checkWriteAccess(HttpStatus.SC_CREATED, "picard", "picard", "sf", "_doc", 1); @@ -117,35 +117,35 @@ public void testRolesMappingApiWithFullPermissions() throws Exception { verifyPutForSuperAdmin(new Header[]{restApiAdminHeader}); verifyPatchForSuperAdmin(new Header[]{restApiAdminHeader}); // mapping with several backend roles, one of the is captain - deleteAndputNewMapping(new Header[]{restApiAdminHeader}, "rolesmapping_backendroles_captains_list.json", false); + deleteAndPutNewMapping(new Header[]{restApiAdminHeader}, "rolesmapping_backendroles_captains_list.json", false); checkAllSfAllowed(); // mapping with one backend role, captain - deleteAndputNewMapping(new Header[]{restApiAdminHeader},"rolesmapping_backendroles_captains_single.json", true); + deleteAndPutNewMapping(new Header[]{restApiAdminHeader},"rolesmapping_backendroles_captains_single.json", true); checkAllSfAllowed(); // mapping with several users, one is picard - deleteAndputNewMapping(new Header[]{restApiAdminHeader},"rolesmapping_users_picard_list.json", true); + deleteAndPutNewMapping(new Header[]{restApiAdminHeader},"rolesmapping_users_picard_list.json", true); checkAllSfAllowed(); // just user picard - deleteAndputNewMapping(new Header[]{restApiAdminHeader},"rolesmapping_users_picard_single.json", true); + deleteAndPutNewMapping(new Header[]{restApiAdminHeader},"rolesmapping_users_picard_single.json", true); checkAllSfAllowed(); // hosts - deleteAndputNewMapping(new Header[]{restApiAdminHeader},"rolesmapping_hosts_list.json", true); + deleteAndPutNewMapping(new Header[]{restApiAdminHeader},"rolesmapping_hosts_list.json", true); checkAllSfAllowed(); // hosts - deleteAndputNewMapping(new Header[]{restApiAdminHeader},"rolesmapping_hosts_single.json", true); + deleteAndPutNewMapping(new Header[]{restApiAdminHeader},"rolesmapping_hosts_single.json", true); checkAllSfAllowed(); // full settings, access - deleteAndputNewMapping(new Header[]{restApiAdminHeader},"rolesmapping_all_access.json", true); + deleteAndPutNewMapping(new Header[]{restApiAdminHeader},"rolesmapping_all_access.json", true); checkAllSfAllowed(); // full settings, no access - deleteAndputNewMapping(new Header[]{restApiAdminHeader},"rolesmapping_all_noaccess.json", true); + deleteAndPutNewMapping(new Header[]{restApiAdminHeader},"rolesmapping_all_noaccess.json", true); checkAllSfForbidden(); } @@ -221,7 +221,7 @@ void verifyDeleteForSuperAdmin(final Header[] header, final boolean useAdminCert // 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", "picardpicardpicard", "sf", "_doc", 1); // TODO: only one doctype allowed for ES6 // checkWriteAccess(HttpStatus.SC_OK, "picard", "picard", "sf", "_doc", 1); @@ -382,17 +382,17 @@ void verifyPatchForSuperAdmin(final Header[] header) 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", "picardpicardpicard", "sf", "_doc", 1); + checkWriteAccess(HttpStatus.SC_OK, "picard", "picardpicardpicard", "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", "picardpicardpicard", "sf", "_doc", 1); + checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicard", "sf", "_doc", 1); } - private HttpResponse deleteAndputNewMapping(final Header[] header, final String fileName, final boolean useAdminCert) throws Exception { + private HttpResponse deleteAndPutNewMapping(final Header[] header, final String fileName, final boolean useAdminCert) throws Exception { rh.sendAdminCertificate = useAdminCert; HttpResponse response = rh.executeDeleteRequest(ENDPOINT + "/rolesmapping/opendistro_security_role_starfleet_captains", header); 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 60c2ccde6f..abcda9a69c 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; @@ -89,15 +90,22 @@ public void testSecurityRoles() throws Exception { .executeGetRequest(ENDPOINT + "/" + 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()); - response = rh.executePatchRequest(ENDPOINT + "/internalusers", "[{ \"op\": \"add\", \"path\": \"/newuser\", \"value\": {\"password\": \"newuser\", \"opendistro_security_roles\": [\"opendistro_security_all_access\"] } }]", new Header[0]); + Assert.assertEquals(133, settings.size()); + 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 @@ -108,7 +116,8 @@ 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(); @@ -254,7 +263,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()); @@ -292,7 +301,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()); @@ -382,11 +391,11 @@ private void verifyPatch(final boolean sendAdminCert, Header... restAdminHeader) // use password instead of hash rh.sendAdminCertificate = sendAdminCert; - addUserWithPassword("nagilum", "correctpassword", HttpStatus.SC_CREATED); + addUserWithPassword("nagilum", "correct password 42", HttpStatus.SC_CREATED); rh.sendAdminCertificate = false; checkGeneralAccess(HttpStatus.SC_UNAUTHORIZED, "nagilum", "wrongpassword"); - checkGeneralAccess(HttpStatus.SC_OK, "nagilum", "correctpassword"); + checkGeneralAccess(HttpStatus.SC_OK, "nagilum", "correct password 42"); deleteUser("nagilum"); @@ -490,24 +499,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); @@ -520,8 +529,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); @@ -565,13 +574,15 @@ public void testUserApiWithRestInternalUsersAdminPermissions() throws Exception } @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,}") + .put(ConfigConstants.SECURITY_RESTAPI_PASSWORD_SCORE_BASED_VALIDATION_STRENGTH, + PasswordValidator.ScoreStrength.FAIR.name()) .build(); setup(nodeSettings); @@ -582,73 +593,137 @@ 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()); + 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.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")); + } + 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(HttpStatus.SC_BAD_REQUEST, response.getStatusCode()); - - addUserWithPassword("ok4", "123", HttpStatus.SC_BAD_REQUEST); - - 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); + 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\"] } }" + "]"; 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 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(133, 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 @@ -669,13 +744,13 @@ public void testUserApiWithDots() throws Exception { addUserWithPassword(".my.dotuser0", "$2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m", HttpStatus.SC_CREATED); - addUserWithPassword(".my.dot.user0", "12345678", + addUserWithPassword(".my.dot.user0", "12345678Sd", HttpStatus.SC_CREATED); addUserWithHash(".my.dotuser1", "$2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m", HttpStatus.SC_CREATED); - addUserWithPassword(".my.dot.user2", "12345678", + addUserWithPassword(".my.dot.user2", "12345678Sd", HttpStatus.SC_CREATED); } @@ -697,7 +772,7 @@ public void testUserApiNoPasswordChange() throws Exception { response = rh.executePutRequest(ENDPOINT + "/internalusers/user1", "{\"hash\":\"$2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m\",\"password\":\"\",\"backend_roles\":[\"admin\",\"rolea\"]}"); Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode()); - response = rh.executePutRequest(ENDPOINT + "/internalusers/user1", "{\"hash\":\"$2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m\",\"password\":\"Admin_123\",\"backend_roles\":[\"admin\",\"rolea\"]}"); + response = rh.executePutRequest(ENDPOINT + "/internalusers/user1", "{\"hash\":\"$2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m\",\"password\":\"Admin_123345Yq\",\"backend_roles\":[\"admin\",\"rolea\"]}"); Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); response = rh.executeGetRequest(ENDPOINT + "/internalusers/user1"); @@ -709,7 +784,7 @@ public void testUserApiNoPasswordChange() throws Exception { response = rh.executePutRequest(ENDPOINT + "/internalusers/user2", "{\"password\":\"\",\"backend_roles\":[\"admin\",\"rolex\"]}"); Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode()); - response = rh.executePutRequest(ENDPOINT + "/internalusers/user2", "{\"password\":\"Admin_123\",\"backend_roles\":[\"admin\",\"rolex\"]}"); + response = rh.executePutRequest(ENDPOINT + "/internalusers/user2", "{\"password\":\"Admin_123Qerty\",\"backend_roles\":[\"admin\",\"rolex\"]}"); Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); response = rh.executeGetRequest(ENDPOINT + "/internalusers/user2"); 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); + } + +}