Skip to content

Commit

Permalink
Security User Refactor (opensearch-project#2594)
Browse files Browse the repository at this point in the history 
---------

Signed-off-by: Stephen Crawford <steecraw@amazon.com>
Signed-off-by: Stephen Crawford <65832608+scrawfor99@users.noreply.github.com>
Signed-off-by: Maciej Mierzwa <dev.maciej.mierzwa@gmail.com>
  • Loading branch information
@stephen-crawford @MaciejMierzwa
stephen-crawford authored and MaciejMierzwa committed Apr 27, 2023
1 parent cbc015d commit 3fb72e0
Show file tree
Hide file tree
Showing 8 changed files with 306 additions and 64 deletions.
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
Loading

0 comments on commit 3fb72e0

Please sign in to comment.