Skip to content

Commit

Permalink
Merge branch 'main' into extensions_config
Browse files Browse the repository at this point in the history
  • Loading branch information
@MaciejMierzwa
MaciejMierzwa committed Apr 27, 2023
2 parents 29896af + 6997f97 commit 454c4c6
Show file tree
Hide file tree
Showing 14 changed files with 328 additions and 85 deletions.
11 changes: 6 additions & 5 deletions src/integrationTest/java/org/opensearch/test/framework/matcher/AliasExistsMatcher.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ExecutionException;
import java.util.stream.Collectors;
Expand All @@ -23,7 +24,6 @@
import org.opensearch.action.admin.indices.alias.get.GetAliasesResponse;
import org.opensearch.client.Client;
import org.opensearch.cluster.metadata.AliasMetadata;
import org.opensearch.common.collect.ImmutableOpenMap;

import static java.util.Objects.requireNonNull;
import static java.util.Spliterator.IMMUTABLE;
Expand All @@ -41,8 +41,9 @@ public AliasExistsMatcher(String aliasName) {
protected boolean matchesSafely(Client client, Description mismatchDescription) {
try {
GetAliasesResponse response = client.admin().indices().getAliases(new GetAliasesRequest(aliasName)).get();
ImmutableOpenMap<String, List<AliasMetadata>> aliases = response.getAliases();
Set<String> actualAliasNames = StreamSupport.stream(spliteratorUnknownSize(aliases.valuesIt(), IMMUTABLE), false)

Map<String, List<AliasMetadata>> aliases = response.getAliases();
Set<String> actualAliasNames = StreamSupport.stream(spliteratorUnknownSize(aliases.values().iterator(), IMMUTABLE), false)
.flatMap(Collection::stream)
.map(AliasMetadata::getAlias)
.collect(Collectors.toSet());
Expand All @@ -53,8 +54,8 @@ protected boolean matchesSafely(Client client, Description mismatchDescription)
}
return true;
} catch (InterruptedException | ExecutionException e) {
mismatchDescription.appendText("Error occured during checking if cluster contains alias ")
.appendValue(e);
mismatchDescription.appendText("Error occurred during checking if cluster contains alias ")
.appendValue(e);
return false;
}
}
Expand Down
9 changes: 6 additions & 3 deletions ...st/java/org/opensearch/test/framework/matcher/ClusterContainTemplateWithAliasMatcher.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
*/
package org.opensearch.test.framework.matcher;

import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
Expand All @@ -21,7 +22,6 @@
import org.opensearch.action.admin.indices.template.get.GetIndexTemplatesResponse;
import org.opensearch.client.Client;
import org.opensearch.cluster.metadata.AliasMetadata;
import org.opensearch.common.collect.ImmutableOpenMap;

import static java.util.Objects.requireNonNull;

Expand Down Expand Up @@ -60,10 +60,13 @@ private Set<String> getAliases(GetIndexTemplatesResponse response) {
.collect(Collectors.toSet());
}

private Stream<String> aliasNames(ImmutableOpenMap<String, AliasMetadata> aliasMap) {
return StreamSupport.stream(aliasMap.keys().spliterator(), false).map(objectCursor -> objectCursor.value);
private Stream<String> aliasNames(Map<String, AliasMetadata> aliasMap) {
Iterable<Map.Entry<String, AliasMetadata>> iterable = () -> aliasMap.entrySet().iterator();
return StreamSupport.stream(iterable.spliterator(), false)
.map(entry -> entry.getValue().getAlias());
}


@Override
public void describeTo(Description description) {
description.appendText("template ").appendValue(templateName).appendText(" exists and ");
Expand Down
3 changes: 2 additions & 1 deletion ...java/org/opensearch/test/framework/matcher/GetSettingsResponseContainsIndicesMatcher.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ class GetSettingsResponseContainsIndicesMatcher extends TypeSafeDiagnosingMatche

@Override
protected boolean matchesSafely(GetSettingsResponse response, Description mismatchDescription) {
ImmutableOpenMap<String, Settings> indexToSettings = response.getIndexToSettings();

final ImmutableOpenMap<String, Settings> indexToSettings = response.getIndexToSettings();
for (String index : expectedIndices) {
if (!indexToSettings.containsKey(index)) {
mismatchDescription
Expand Down
5 changes: 1 addition & 4 deletions ...egrationTest/java/org/opensearch/test/framework/matcher/IndexMappingIsEqualToMatcher.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@
import org.opensearch.action.admin.indices.mapping.get.GetMappingsRequest;
import org.opensearch.action.admin.indices.mapping.get.GetMappingsResponse;
import org.opensearch.client.Client;
import org.opensearch.cluster.metadata.MappingMetadata;
import org.opensearch.common.collect.ImmutableOpenMap;
import org.opensearch.index.IndexNotFoundException;
import org.opensearch.test.framework.cluster.LocalCluster;

Expand All @@ -44,8 +42,7 @@ protected boolean matchesSafely(LocalCluster cluster, Description mismatchDescri
GetMappingsResponse response = client.admin().indices()
.getMappings(new GetMappingsRequest().indices(expectedIndexName)).actionGet();

ImmutableOpenMap<String, MappingMetadata> actualMappings = response.mappings();
Map<String, Object> actualIndexMapping = actualMappings.get(expectedIndexName).getSourceAsMap();
Map<String, Object> actualIndexMapping = response.getMappings().get(expectedIndexName).sourceAsMap();

if (!expectedMapping.equals(actualIndexMapping)) {
mismatchDescription.appendText("Actual mapping ").appendValue(actualIndexMapping).appendText(" does not match expected");
Expand Down
5 changes: 3 additions & 2 deletions ...ntegrationTest/java/org/opensearch/test/framework/matcher/IndexStateIsEqualToMatcher.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,15 @@
*/
package org.opensearch.test.framework.matcher;

import java.util.Map;

import org.hamcrest.Description;
import org.hamcrest.TypeSafeDiagnosingMatcher;

import org.opensearch.action.admin.cluster.state.ClusterStateRequest;
import org.opensearch.action.admin.cluster.state.ClusterStateResponse;
import org.opensearch.client.Client;
import org.opensearch.cluster.metadata.IndexMetadata;
import org.opensearch.common.collect.ImmutableOpenMap;
import org.opensearch.test.framework.cluster.LocalCluster;

import static java.util.Objects.requireNonNull;
Expand All @@ -37,7 +38,7 @@ protected boolean matchesSafely(LocalCluster cluster, Description mismatchDescri
ClusterStateRequest clusterStateRequest = new ClusterStateRequest().indices(expectedIndexName);
ClusterStateResponse clusterStateResponse = client.admin().cluster().state(clusterStateRequest).actionGet();

ImmutableOpenMap<String, IndexMetadata> indicesMetadata = clusterStateResponse.getState().getMetadata().indices();
Map<String, IndexMetadata> indicesMetadata = clusterStateResponse.getState().getMetadata().indices();
if (!indicesMetadata.containsKey(expectedIndexName)) {
mismatchDescription.appendValue("Index does not exist");
}
Expand Down
12 changes: 9 additions & 3 deletions src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@
import org.opensearch.security.transport.InterClusterRequestEvaluator;
import org.opensearch.security.transport.SecurityInterceptor;
import org.opensearch.security.user.User;
import org.opensearch.security.user.UserService;
import org.opensearch.tasks.Task;
import org.opensearch.threadpool.ThreadPool;
import org.opensearch.transport.RemoteClusterService;
Expand Down Expand Up @@ -204,6 +205,7 @@ public final class OpenSearchSecurityPlugin extends OpenSearchSecuritySSLPlugin
private volatile SecurityRestFilter securityRestHandler;
private volatile SecurityInterceptor si;
private volatile PrivilegesEvaluator evaluator;
private volatile UserService userService;
private volatile ThreadPool threadPool;
private volatile ConfigurationRepository cr;
private volatile AdminDNs adminDns;
Expand Down Expand Up @@ -487,6 +489,7 @@ public List<RestHandler> getRestHandlers(Settings settings, RestController restC
evaluator,
threadPool,
Objects.requireNonNull(auditLog), sks,
Objects.requireNonNull(userService),
sslCertReloadEnabled)
);
log.debug("Added {} rest handler(s)", handlers.size());
Expand Down Expand Up @@ -813,9 +816,11 @@ public Collection<Object> createComponents(Client localClient, ClusterService cl
sslExceptionHandler = new AuditLogSslExceptionHandler(auditLog);

adminDns = new AdminDNs(settings);

cr = ConfigurationRepository.create(settings, this.configPath, threadPool, localClient, clusterService, auditLog);

userService = new UserService(cs, cr, settings, localClient);

final XFFResolver xffResolver = new XFFResolver(threadPool);
backendRegistry = new BackendRegistry(settings, adminDns, xffResolver, auditLog, threadPool);

Expand Down Expand Up @@ -872,6 +877,7 @@ public Collection<Object> createComponents(Client localClient, ClusterService cl
components.add(evaluator);
components.add(si);
components.add(dcf);
components.add(userService);


return components;
Expand Down Expand Up @@ -1179,7 +1185,6 @@ public static class GuiceHolder implements LifecycleComponent {
private static RepositoriesService repositoriesService;
private static RemoteClusterService remoteClusterService;
private static IndicesService indicesService;

private static PitService pitService;

@Inject
Expand All @@ -1204,7 +1209,8 @@ public static IndicesService getIndicesService() {
}

public static PitService getPitService() { return pitService; }



@Override
public void close() {
}
Expand Down
76 changes: 24 additions & 52 deletions src/main/java/org/opensearch/security/dlic/rest/api/InternalUsersApiAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import java.io.IOException;
import java.nio.file.Path;
import java.util.List;
import java.util.stream.Collectors;

import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.node.ObjectNode;
Expand All @@ -32,18 +31,18 @@
import org.opensearch.rest.RestController;
import org.opensearch.rest.RestRequest;
import org.opensearch.rest.RestRequest.Method;
import org.opensearch.security.DefaultObjectMapper;
import org.opensearch.security.auditlog.AuditLog;
import org.opensearch.security.configuration.AdminDNs;
import org.opensearch.security.configuration.ConfigurationRepository;
import org.opensearch.security.dlic.rest.validation.AbstractConfigurationValidator;
import org.opensearch.security.dlic.rest.validation.InternalUsersValidator;
import org.opensearch.security.privileges.PrivilegesEvaluator;
import org.opensearch.security.securityconf.Hashed;
import org.opensearch.security.securityconf.impl.CType;
import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration;
import org.opensearch.security.ssl.transport.PrincipalExtractor;
import org.opensearch.security.support.SecurityJsonNode;
import org.opensearch.security.user.UserService;
import org.opensearch.security.user.UserServiceException;
import org.opensearch.threadpool.ThreadPool;

import static org.opensearch.security.dlic.rest.support.Utils.addRoutesPrefix;
Expand All @@ -69,13 +68,16 @@ public class InternalUsersApiAction extends PatchableResourceApiAction {
new Route(Method.PATCH, "/internalusers/{name}")
));

UserService userService;

@Inject
public InternalUsersApiAction(final Settings settings, final Path configPath, final RestController controller,
final Client client, final AdminDNs adminDNs, final ConfigurationRepository cl,
final ClusterService cs, final PrincipalExtractor principalExtractor, final PrivilegesEvaluator evaluator,
ThreadPool threadPool, AuditLog auditLog) {
ThreadPool threadPool, UserService userService, AuditLog auditLog) {
super(settings, configPath, controller, client, adminDNs, cl, cs, principalExtractor, evaluator, threadPool,
auditLog);
this.userService = userService;
}

@Override
Expand All @@ -100,22 +102,7 @@ protected void handlePut(RestChannel channel, final RestRequest request, final C

final String username = request.param("name");

if (username == null || username.length() == 0) {
badRequestResponse(channel, "No " + getResourceName() + " specified.");
return;
}

final List<String> foundRestrictedContents = RESTRICTED_FROM_USERNAME.stream().filter(username::contains).collect(Collectors.toList());
if (!foundRestrictedContents.isEmpty()) {
final String restrictedContents = foundRestrictedContents.stream().map(s -> "'" + s + "'").collect(Collectors.joining(","));
badRequestResponse(channel, "Username has restricted characters " + restrictedContents + " that are not permitted.");
return;
}

// TODO it might be sensible to consolidate this with the overridden method in
// order to minimize duplicated logic

final SecurityDynamicConfiguration<?> internalUsersConfiguration = load(getConfigName(), false);
SecurityDynamicConfiguration<?> internalUsersConfiguration = load(getConfigName(), false);

if (!isWriteable(channel, internalUsersConfiguration, username)) {
return;
Expand All @@ -128,50 +115,35 @@ protected void handlePut(RestChannel channel, final RestRequest request, final C
final List<String> securityRoles = securityJsonNode.get("opendistro_security_roles").asList();
if (securityRoles != null) {
for (final String role: securityRoles) {
if (!isValidRolesMapping(channel, role)) return;
if (!isValidRolesMapping(channel, role)) {
return;
}
}
}

// if password is set, it takes precedence over hash
final String plainTextPassword = securityJsonNode.get("password").asString();
final String origHash = securityJsonNode.get("hash").asString();
if (plainTextPassword != null && plainTextPassword.length() > 0) {
contentAsNode.remove("password");
contentAsNode.put("hash", hash(plainTextPassword.toCharArray()));
} else if (origHash != null && origHash.length() > 0) {
contentAsNode.remove("password");
} else if (plainTextPassword != null && plainTextPassword.isEmpty() && origHash == null) {
contentAsNode.remove("password");
}

final boolean userExisted = internalUsersConfiguration.exists(username);

// when updating an existing user password hash can be blank, which means no
// changes

// sanity checks, hash is mandatory for newly created users
if (!userExisted && securityJsonNode.get("hash").asString() == null) {
badRequestResponse(channel, "Please specify either 'hash' or 'password' when creating a new internal user.");
try {
if (request.hasParam("owner")) {
((ObjectNode) content).put("owner", request.param("owner"));
}
if (request.hasParam("isEnabled")) {
((ObjectNode) content).put("isEnabled", request.param("isEnabled"));
}
((ObjectNode) content).put("name", username);
internalUsersConfiguration = userService.createOrUpdateAccount((ObjectNode) content);
}
catch (UserServiceException ex) {
badRequestResponse(channel, ex.getMessage());
return;
}

// for existing users, hash is optional
if (userExisted && securityJsonNode.get("hash").asString() == null) {
// sanity check, this should usually not happen
final String hash = ((Hashed) internalUsersConfiguration.getCEntry(username)).getHash();
if (hash == null || hash.length() == 0) {
internalErrorResponse(channel,
"Existing user " + username + " has no password, and no new password or hash was specified.");
return;
}
contentAsNode.put("hash", hash);
catch (IOException ex) {
throw new IOException(ex);
}

internalUsersConfiguration.remove(username);

// checks complete, create or update the user
internalUsersConfiguration.putCObject(username, DefaultObjectMapper.readTree(contentAsNode, internalUsersConfiguration.getImplementingClass()));

saveAndUpdateConfigs(this.securityIndexName,client, CType.INTERNALUSERS, internalUsersConfiguration, new OnSucessActionListener<IndexResponse>(channel) {

@Override
Expand Down
4 changes: 3 additions & 1 deletion src/main/java/org/opensearch/security/dlic/rest/api/SecurityRestApiActions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.opensearch.security.privileges.PrivilegesEvaluator;
import org.opensearch.security.ssl.SecurityKeyStore;
import org.opensearch.security.ssl.transport.PrincipalExtractor;
import org.opensearch.security.user.UserService;
import org.opensearch.threadpool.ThreadPool;

public class SecurityRestApiActions {
Expand All @@ -44,9 +45,10 @@ public static Collection<RestHandler> getHandler(final Settings settings,
final ThreadPool threadPool,
final AuditLog auditLog,
final SecurityKeyStore securityKeyStore,
final UserService userService,
final boolean certificatesReloadEnabled) {
final List<RestHandler> handlers = new ArrayList<RestHandler>(16);
handlers.add(new InternalUsersApiAction(settings, configPath, controller, client, adminDns, cr, cs, principalExtractor, evaluator, threadPool, auditLog));
handlers.add(new InternalUsersApiAction(settings, configPath, controller, client, adminDns, cr, cs, principalExtractor, evaluator, threadPool, userService, auditLog));
handlers.add(new RolesMappingApiAction(settings, configPath, controller, client, adminDns, cr, cs, principalExtractor, evaluator, threadPool, auditLog));
handlers.add(new RolesApiAction(settings, configPath, controller, client, adminDns, cr, cs, principalExtractor, evaluator, threadPool, auditLog));
handlers.add(new ActionGroupsApiAction(settings, configPath, controller, client, adminDns, cr, cs, principalExtractor, evaluator, threadPool, auditLog));
Expand Down
10 changes: 4 additions & 6 deletions src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@
import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
import org.opensearch.cluster.service.ClusterService;
import org.opensearch.common.Strings;
import org.opensearch.common.collect.ImmutableOpenMap;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.transport.TransportAddress;
import org.opensearch.common.util.concurrent.ThreadContext;
Expand Down Expand Up @@ -378,8 +377,6 @@ public PrivilegesEvaluatorResponse evaluate(final User user, String action0, fin
presponse.allowed = true;
return presponse;
}


}
}

Expand Down Expand Up @@ -632,6 +629,7 @@ public static boolean isClusterPerm(String action0) {
) ;
}

@SuppressWarnings("unchecked")
private boolean checkFilteredAliases(Resolved requestedResolved, String action, boolean isDebugEnabled) {
final String faMode = dcm.getFilteredAliasMode();// getConfigSettings().dynamic.filtered_alias_mode;

Expand All @@ -649,7 +647,7 @@ private boolean checkFilteredAliases(Resolved requestedResolved, String action,
indexMetaDataCollection = new Iterable<IndexMetadata>() {
@Override
public Iterator<IndexMetadata> iterator() {
return clusterService.state().getMetadata().getIndices().valuesIt();
return clusterService.state().getMetadata().getIndices().values().iterator();
}
};
} else {
Expand All @@ -674,14 +672,14 @@ public Iterator<IndexMetadata> iterator() {

final List<AliasMetadata> filteredAliases = new ArrayList<AliasMetadata>();

final ImmutableOpenMap<String, AliasMetadata> aliases = indexMetaData.getAliases();
final Map<String, AliasMetadata> aliases = indexMetaData.getAliases();

if(aliases != null && aliases.size() > 0) {
if (isDebugEnabled) {
log.debug("Aliases for {}: {}", indexMetaData.getIndex().getName(), aliases);
}

final Iterator<String> it = aliases.keysIt();
final Iterator<String> it = aliases.keySet().iterator();
while(it.hasNext()) {
final String alias = it.next();
final AliasMetadata aliasMetadata = aliases.get(alias);
Expand Down
Loading

0 comments on commit 454c4c6

Please sign in to comment.