Skip to content

Commit

Permalink
Change password security message (opensearch-project#3057)
Browse files Browse the repository at this point in the history
Returns the error message instead of the validation message because
validation message should already have been shown in above case
statements.

Fix: opensearch-project#3055

Is this a backport? If so, please add backport PR # and/or commits #

[Please provide details of testing done: unit testing, integration
testing and manual testing]

- [ ] New functionality includes testing
- [ ] New functionality has been documented
- [ ] Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and
signing off your commits, please check
[here](https://github.com/opensearch-project/OpenSearch/blob/main/CONTRIBUTING.md#developer-certificate-of-origin).

---------

Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Derek Ho <dxho@amazon.com>
Co-authored-by: Craig Perkins <cwperx@amazon.com>
(cherry picked from commit 847f911)
  • Loading branch information
derek-ho committed Aug 17, 2023
1 parent 4ee078b commit 648d737
Show file tree
Hide file tree
Showing 5 changed files with 519 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*
*/
package org.opensearch.security.api;

import java.util.List;
import java.util.Map;

import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
import org.junit.ClassRule;
import org.junit.Test;
import org.junit.runner.RunWith;

import org.opensearch.security.dlic.rest.validation.RequestContentValidator;
import org.opensearch.security.support.ConfigConstants;
import org.opensearch.test.framework.TestSecurityConfig.User;
import org.opensearch.test.framework.cluster.ClusterManager;
import org.opensearch.test.framework.cluster.LocalCluster;
import org.opensearch.test.framework.cluster.TestRestClient;
import org.opensearch.test.framework.cluster.TestRestClient.HttpResponse;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.opensearch.security.SecurityConfigurationTests.ADDITIONAL_USER_1;
import static org.opensearch.security.SecurityConfigurationTests.CREATE_USER_BODY;
import static org.opensearch.security.SecurityConfigurationTests.INTERNAL_USERS_RESOURCE;
import static org.opensearch.security.support.ConfigConstants.SECURITY_BACKGROUND_INIT_IF_SECURITYINDEX_NOT_EXIST;
import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_ROLES_ENABLED;
import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL;
import static org.opensearch.test.framework.TestSecurityConfig.Role.ALL_ACCESS;

@RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class)
@ThreadLeakScope(ThreadLeakScope.Scope.NONE)
public class CreateResetPasswordTest {

private static final User USER_ADMIN = new User("admin").roles(ALL_ACCESS);

public static final String INVALID_PASSWORD_REGEX = "user 1 fair password";

public static final String VALID_WEAK_PASSWORD = "Asdfghjkl1!";

public static final String VALID_SIMILAR_PASSWORD = "456Additional00001_1234!";

private static final String CUSTOM_PASSWORD_MESSAGE =
"Password must be minimum 5 characters long and must contain at least one uppercase letter, one lowercase letter, one digit, and one special character.";

private static final String CUSTOM_PASSWORD_REGEX = "(?=.*[A-Z])(?=.*[^a-zA-Z\\d])(?=.*[0-9])(?=.*[a-z]).{5,}";

@ClassRule
public static LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.THREE_CLUSTER_MANAGERS)
.authc(AUTHC_HTTPBASIC_INTERNAL)
.users(USER_ADMIN)
.anonymousAuth(false)
.nodeSettings(
Map.of(
SECURITY_RESTAPI_ROLES_ENABLED,
List.of("user_" + USER_ADMIN.getName() + "__" + ALL_ACCESS.getName()),
SECURITY_BACKGROUND_INIT_IF_SECURITYINDEX_NOT_EXIST,
true,
ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_REGEX,
CUSTOM_PASSWORD_REGEX,
ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_ERROR_MESSAGE,
CUSTOM_PASSWORD_MESSAGE
)
)
.build();

@Test
public void shouldValidateCreateUserAPIErrorMessages() {
try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
HttpResponse httpResponse = client.putJson(
INTERNAL_USERS_RESOURCE + ADDITIONAL_USER_1,
String.format(CREATE_USER_BODY, INVALID_PASSWORD_REGEX)
);

assertThat(httpResponse.getStatusCode(), equalTo(400));
assertThat(httpResponse.getBody(), containsString(CUSTOM_PASSWORD_MESSAGE));
}

try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
HttpResponse httpResponse = client.putJson(
INTERNAL_USERS_RESOURCE + ADDITIONAL_USER_1,
String.format(CREATE_USER_BODY, VALID_WEAK_PASSWORD)
);

assertThat(httpResponse.getStatusCode(), equalTo(400));
assertThat(httpResponse.getBody(), containsString(RequestContentValidator.ValidationError.WEAK_PASSWORD.message()));
}

try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
HttpResponse httpResponse = client.putJson(
INTERNAL_USERS_RESOURCE + ADDITIONAL_USER_1,
String.format(CREATE_USER_BODY, VALID_SIMILAR_PASSWORD)
);

assertThat(httpResponse.getStatusCode(), equalTo(400));
assertThat(httpResponse.getBody(), containsString(RequestContentValidator.ValidationError.SIMILAR_PASSWORD.message()));
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -86,19 +86,31 @@ ErrorType validate(final String username, final String password) {
minPasswordLength,
password.length()
);
<<<<<<< HEAD
return ErrorType.INVALID_PASSWORD;
=======
return RequestContentValidator.ValidationError.INVALID_PASSWORD_TOO_SHORT;
>>>>>>> 847f9110 (Change password security message (#3057))
}
if (password.length() > MAX_LENGTH) {
logger.debug(
"Password is too long, the maximum required length is {}, but current length is {}",
MAX_LENGTH,
password.length()
);
<<<<<<< HEAD
return ErrorType.INVALID_PASSWORD;
}
if (Objects.nonNull(passwordRegexpPattern) && !passwordRegexpPattern.matcher(password).matches()) {
logger.debug("Regex does not match password");
return ErrorType.INVALID_PASSWORD;
=======
return RequestContentValidator.ValidationError.INVALID_PASSWORD_TOO_LONG;
}
if (Objects.nonNull(passwordRegexpPattern) && !passwordRegexpPattern.matcher(password).matches()) {
logger.debug("Regex does not match password");
return RequestContentValidator.ValidationError.INVALID_PASSWORD_INVALID_REGEX;
>>>>>>> 847f9110 (Change password security message (#3057))
}
final Strength strength = zxcvbn.measure(password, ImmutableList.of(username));
if (strength.getScore() < scoreStrength.score()) {
Expand Down
Loading

0 comments on commit 648d737

Please sign in to comment.