Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make invalid password message clearer #3057

Merged
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
/*
* 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.*;
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 @@ -46,6 +46,7 @@
import org.apache.lucene.index.SortedNumericDocValues;
import org.apache.lucene.index.SortedSetDocValues;
import org.apache.lucene.index.StoredFieldVisitor;
import org.apache.lucene.index.StoredFields;
import org.apache.lucene.index.TermState;
import org.apache.lucene.index.Terms;
import org.apache.lucene.index.TermsEnum;
Expand Down Expand Up @@ -473,6 +474,24 @@ public void close() throws IOException {
}
}

private class DlsFlsStoredFields extends StoredFields {
private final StoredFields in;

public DlsFlsStoredFields(StoredFields storedFields) {
this.in = storedFields;
}

@Override
public void document(final int docID, StoredFieldVisitor visitor) throws IOException {
visitor = getDlsFlsVisitor(visitor);
try {
in.document(docID, visitor);
} finally {
finishVisitor(visitor);
}
}
}

@Override
protected StoredFieldsReader doGetSequentialStoredFieldsReader(final StoredFieldsReader reader) {
return new DlsFlsStoredFieldsReader(reader);
Expand Down Expand Up @@ -1284,6 +1303,12 @@ public TermState termState() throws IOException {

}

@Override
public StoredFields storedFields() throws IOException {
ensureOpen();
return new DlsFlsStoredFields(in.storedFields());
}

private String getRuntimeActionName() {
return (String) threadContext.getTransient(ConfigConstants.OPENDISTRO_SECURITY_ACTION_NAME);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -286,14 +286,6 @@ public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params par
.get(SECURITY_RESTAPI_PASSWORD_VALIDATION_ERROR_MESSAGE, "Password does not match minimum criteria")
);
break;
case WEAK_PASSWORD:
derek-ho marked this conversation as resolved.
Show resolved Hide resolved
case SIMILAR_PASSWORD:
builder.field("status", "error");
builder.field(
"reason",
validationContext.settings().get(SECURITY_RESTAPI_PASSWORD_VALIDATION_ERROR_MESSAGE, validationError.message())
peternied marked this conversation as resolved.
Show resolved Hide resolved
);
break;
case WRONG_DATATYPE:
builder.field("status", "error");
builder.field("reason", ValidationError.WRONG_DATATYPE.message());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ public static BytesReference readXContent(final Reader reader, final MediaType m
BytesReference retVal;
XContentParser parser = null;
try {
parser = XContentFactory.xContent(mediaType).createParser(NamedXContentRegistry.EMPTY, THROW_UNSUPPORTED_OPERATION, reader);
parser = mediaType.xContent().createParser(NamedXContentRegistry.EMPTY, THROW_UNSUPPORTED_OPERATION, reader);
parser.nextToken();
final XContentBuilder builder = XContentFactory.jsonBuilder();
builder.copyCurrentStructure(parser);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1202,7 +1202,7 @@ private static BytesReference readXContent(final String content, final MediaType
BytesReference retVal;
XContentParser parser = null;
try {
parser = XContentFactory.xContent(mediaType).createParser(NamedXContentRegistry.EMPTY, THROW_UNSUPPORTED_OPERATION, content);
parser = mediaType.xContent().createParser(NamedXContentRegistry.EMPTY, THROW_UNSUPPORTED_OPERATION, content);
parser.nextToken();
final XContentBuilder builder = XContentFactory.jsonBuilder();
builder.copyCurrentStructure(parser);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import org.opensearch.common.Strings;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.xcontent.XContentFactory;
import org.opensearch.common.xcontent.XContentType;
import org.opensearch.core.common.bytes.BytesArray;
import org.opensearch.core.xcontent.NamedXContentRegistry;
import org.opensearch.core.xcontent.ToXContent;
Expand Down Expand Up @@ -300,7 +299,7 @@ public Map<String, RequestContentValidator.DataType> allowedKeys() {
}

private JsonNode xContentToJsonNode(final ToXContent toXContent) throws IOException {
try (final var xContentBuilder = XContentFactory.contentBuilder(XContentType.JSON)) {
try (final var xContentBuilder = XContentFactory.jsonBuilder()) {
toXContent.toXContent(xContentBuilder, ToXContent.EMPTY_PARAMS);
return DefaultObjectMapper.readTree(Strings.toString(xContentBuilder));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ public static BytesReference readYamlContent(final String file) {

XContentParser parser = null;
try {
parser = XContentFactory.xContent(XContentType.YAML)
parser = XContentType.YAML.xContent()
.createParser(NamedXContentRegistry.EMPTY, THROW_UNSUPPORTED_OPERATION, new StringReader(loadFile(file)));
parser.nextToken();
final XContentBuilder builder = XContentFactory.jsonBuilder();
Expand All @@ -133,7 +133,7 @@ public static BytesReference readYamlContentFromString(final String yaml) {

XContentParser parser = null;
try {
parser = XContentFactory.xContent(XContentType.YAML)
parser = XContentType.YAML.xContent()
.createParser(NamedXContentRegistry.EMPTY, THROW_UNSUPPORTED_OPERATION, new StringReader(yaml));
parser.nextToken();
final XContentBuilder builder = XContentFactory.jsonBuilder();
Expand Down