Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Signed-off-by: Ryan Liang <jiallian@amazon.com>
  • Loading branch information
RyanL1997 committed Nov 2, 2023
1 parent fcbd5ed commit 55bc688
Show file tree
Hide file tree
Showing 6 changed files with 176 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@
import org.opensearch.security.dlic.rest.validation.ValidationResult;
import org.opensearch.security.securityconf.Hashed;
import org.opensearch.security.securityconf.impl.CType;
import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration;
import org.opensearch.security.support.SecurityJsonNode;
import org.opensearch.security.user.UserFilterType;
import org.opensearch.security.user.UserService;
import org.opensearch.security.user.UserServiceException;
import org.opensearch.threadpool.ThreadPool;
Expand All @@ -48,6 +50,12 @@

public class InternalUsersApiAction extends AbstractApiAction {

@Override
protected void consumeParameters(final RestRequest request) {
request.param("name");
request.param("filterBy");
}

static final List<String> RESTRICTED_FROM_USERNAME = ImmutableList.of(
":" // Not allowed in basic auth, see https://stackoverflow.com/a/33391003/533057
);
Expand Down Expand Up @@ -96,7 +104,13 @@ protected CType getConfigType() {
}

private void internalUsersApiRequestHandlers(RequestHandler.RequestHandlersBuilder requestHandlersBuilder) {
requestHandlersBuilder
requestHandlersBuilder.onGetRequest(
request -> ValidationResult.success(request).map(this::processGetRequest).map(securityConfiguration -> {
final var configuration = securityConfiguration.configuration();
filterUsers(configuration, filterParam(request));
return ValidationResult.success(securityConfiguration);
})
)
// Overrides the GET request functionality to allow for the special case of requesting an auth token.
.override(
Method.POST,
Expand All @@ -121,6 +135,21 @@ private void internalUsersApiRequestHandlers(RequestHandler.RequestHandlersBuild
.map(this::validateAndUpdatePassword)
.map(this::addEntityToConfig)
);

}

protected final ValidationResult<SecurityConfiguration> filterUsers(SecurityDynamicConfiguration users, UserFilterType userType) {
userService.includeAccountsIfType(users, userType);
return ValidationResult.success(SecurityConfiguration.of(users.getCType().toString(), users));

}

protected final UserFilterType filterParam(final RestRequest request) {
final String filter = request.param("filterBy");
if (Strings.isNullOrEmpty(filter)) {
return UserFilterType.ANY;
}
return UserFilterType.fromString(filter);
}

ValidationResult<String> withAuthTokenPath(final RestRequest request) throws IOException {
Expand Down Expand Up @@ -179,8 +208,8 @@ ValidationResult<SecurityConfiguration> createOrUpdateAccount(
try {
final var username = securityConfiguration.entityName();
final var content = (ObjectNode) securityConfiguration.requestContent();
if (request.hasParam("service")) {
content.put("service", request.param("service"));
if (request.hasParam("attributes")) {
content.put("attributes", request.param("attributes"));
}
if (request.hasParam("enabled")) {
content.put("enabled", request.param("enabled"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.io.IOException;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;

Expand Down Expand Up @@ -286,7 +287,11 @@ public SecurityDynamicConfiguration<T> deepClone() {
@JsonIgnore
public void remove(String key) {
centries.remove(key);
}

@JsonIgnore
public void remove(List<String> keySet) {
keySet.stream().forEach(this::remove);
}

@SuppressWarnings({ "rawtypes", "unchecked" })
Expand Down
41 changes: 41 additions & 0 deletions src/main/java/org/opensearch/security/user/UserFilterType.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* 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.user;

/**
* Filter types to be used when requesting the list of users.
* 'Service' refers to accounts used by other services like Dashboards
* 'Internal' refers the standard user accounts
* 'Any' refers to both types of accounts
*/
public enum UserFilterType {

ANY("any"),
INTERNAL("internal"),
SERVICE("service");

private String name;

UserFilterType(String name) {
this.name = name;
}

public static UserFilterType fromString(String name) {
for (UserFilterType b : UserFilterType.values()) {
if (b.name.equalsIgnoreCase(name)) {
return b;
}
}
return UserFilterType.ANY;
}

}
44 changes: 44 additions & 0 deletions src/main/java/org/opensearch/security/user/UserService.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,12 @@

import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Base64;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Random;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -45,6 +47,7 @@
import org.opensearch.security.securityconf.Hashed;
import org.opensearch.security.securityconf.impl.CType;
import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration;
import org.opensearch.security.securityconf.impl.v7.InternalUserV7;
import org.opensearch.security.support.ConfigConstants;
import org.opensearch.security.support.SecurityJsonNode;
import org.passay.CharacterRule;
Expand Down Expand Up @@ -315,4 +318,45 @@ public static void saveAndUpdateConfigs(
throw ExceptionsHelper.convertToOpenSearchException(e);
}
}

/**
* Removes accounts that are not of the requested type from the SecurityDynamicConfiguration object passed.
*
* Accounts with the 'service' attribute set to true, are considered of type 'service'.
* Accounts with the 'service' attribute set to false or without the 'service' attribute, are considered of type 'internal'.
*
* @param configuration SecurityDynamicConfiguration object containing all accounts
* @param requestedAccountType The type of account to be kept. Should be "service" or "internal"
*
*/
public void includeAccountsIfType(SecurityDynamicConfiguration<?> configuration, UserFilterType requestedAccountType) {
if (requestedAccountType != UserFilterType.INTERNAL && requestedAccountType != UserFilterType.SERVICE) {
return;
}
List<String> toBeRemoved = new ArrayList<>();

if (requestedAccountType == UserFilterType.SERVICE) {
accountsToRemoveFromConfiguration(configuration, toBeRemoved, false);
} else if (requestedAccountType == UserFilterType.INTERNAL) {
accountsToRemoveFromConfiguration(configuration, toBeRemoved, true);
}
configuration.remove(toBeRemoved);
}

private void accountsToRemoveFromConfiguration(
SecurityDynamicConfiguration<?> configuration,
List<String> toBeRemoved,
boolean isServiceAccountRequested
) {
for (Map.Entry<String, ?> entry : configuration.getCEntries().entrySet()) {
final InternalUserV7 internalUserEntry = (InternalUserV7) entry.getValue();
final Map accountAttributes = internalUserEntry.getAttributes();
final String accountName = entry.getKey();
final boolean isServiceAccount = Boolean.parseBoolean(accountAttributes.getOrDefault("service", "false").toString());

if (isServiceAccount == isServiceAccountRequested) {
toBeRemoved.add(accountName);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,17 @@
import java.util.concurrent.Executors;
import java.util.concurrent.Future;

import com.fasterxml.jackson.databind.JsonNode;
import org.apache.http.Header;
import org.apache.http.HttpStatus;
import org.apache.http.message.BasicHeader;
import org.hamcrest.Matchers;
import org.junit.Assert;
import org.junit.Test;

import org.opensearch.common.settings.Settings;
import org.opensearch.common.xcontent.XContentType;
import org.opensearch.security.DefaultObjectMapper;
import org.opensearch.security.dlic.rest.validation.PasswordValidator;
import org.opensearch.security.dlic.rest.validation.RequestContentValidator;
import org.opensearch.security.securityconf.impl.CType;
Expand Down Expand Up @@ -59,7 +62,7 @@ protected String getEndpointPrefix() {

private static final String ENABLED_SERVICE_ACCOUNT_BODY = "{"
+ " \"attributes\": { \"service\": \"true\", "
+ "\"enabled\": \"true\"}"
+ " \"enabled \": \"true\"}"
+ " }\n";

private static final String DISABLED_SERVICE_ACCOUNT_BODY = "{"
Expand Down Expand Up @@ -166,6 +169,55 @@ private HttpResponse from(Future<HttpResponse> future) {
}
}

@Test
public void testUserFilters() throws Exception {
setup();
rh.keystore = "restapi/kirk-keystore.jks";
rh.sendAdminCertificate = true;
final int SERVICE_ACCOUNTS_IN_SETTINGS = 1;
final int INTERNAL_ACCOUNTS_IN_SETTINGS = 19;
final String serviceAccountName = "JohnDoeService";
HttpResponse response;

response = rh.executeGetRequest(ENDPOINT + "/internalusers?filterBy=internal");

Assert.assertEquals(response.getBody(), HttpStatus.SC_OK, response.getStatusCode());
JsonNode list = DefaultObjectMapper.readTree(response.getBody());
Assert.assertEquals(INTERNAL_ACCOUNTS_IN_SETTINGS, list.size());

response = rh.executeGetRequest(ENDPOINT + "/internalusers?filterBy=service");
Assert.assertEquals(response.getBody(), HttpStatus.SC_OK, response.getStatusCode());
list = DefaultObjectMapper.readTree(response.getBody());
assertThat(list, Matchers.emptyIterable());

response = rh.executePutRequest(ENDPOINT + "/internalusers/" + serviceAccountName, ENABLED_SERVICE_ACCOUNT_BODY);

// repeat assertions after adding the service account

response = rh.executeGetRequest(ENDPOINT + "/internalusers?filterBy=internal");

Assert.assertEquals(response.getBody(), HttpStatus.SC_OK, response.getStatusCode());
list = DefaultObjectMapper.readTree(response.getBody());
Assert.assertEquals(INTERNAL_ACCOUNTS_IN_SETTINGS, list.size());

response = rh.executeGetRequest(ENDPOINT + "/internalusers?filterBy=service");
Assert.assertEquals(response.getBody(), HttpStatus.SC_OK, response.getStatusCode());
list = DefaultObjectMapper.readTree(response.getBody());
Assert.assertEquals(SERVICE_ACCOUNTS_IN_SETTINGS, list.size());
assertThat(response.findValueInJson(serviceAccountName + ".attributes.service"), containsString("true"));

response = rh.executeGetRequest(ENDPOINT + "/internalusers?filterBy=ssas");
Assert.assertEquals(response.getBody(), HttpStatus.SC_OK, response.getStatusCode());
list = DefaultObjectMapper.readTree(response.getBody());
Assert.assertEquals(SERVICE_ACCOUNTS_IN_SETTINGS + INTERNAL_ACCOUNTS_IN_SETTINGS, list.size());

response = rh.executeGetRequest(ENDPOINT + "/internalusers?wrongparameter=jhondoe");
Assert.assertEquals(response.getBody(), HttpStatus.SC_BAD_REQUEST, response.getStatusCode());

response = rh.executePutRequest(ENDPOINT + "/internalusers", "{sample:value");
Assert.assertEquals(response.getBody(), HttpStatus.SC_METHOD_NOT_ALLOWED, response.getStatusCode());
}

@Test
public void testUserApi() throws Exception {

Expand Down
2 changes: 1 addition & 1 deletion src/test/resources/internal_users.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ bug.99:
reserved: false
hidden: false
backend_roles: []
attributes: {}
attributes: {service : "true"}
description: "Migrated from v6"
user_c:
hash: "$2a$04$jQcEXpODnTFoGDuA7DPdSevA84CuH/7MOYkb80M3XZIrH76YMWS9G"
Expand Down

0 comments on commit 55bc688

Please sign in to comment.