Skip to content

Commit

Permalink
Feedback changes
Browse files Browse the repository at this point in the history
Signed-off-by: Stephen Crawford <steecraw@amazon.com>
  • Loading branch information
stephen-crawford committed Apr 20, 2023
1 parent 37ac417 commit b1a0373
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@

package org.opensearch.security;

import com.fasterxml.jackson.databind.node.JsonNodeFactory;
import com.fasterxml.jackson.databind.node.ObjectNode;
import java.io.IOException;
import java.security.AccessController;
import java.security.PrivilegedActionException;
Expand Down Expand Up @@ -96,6 +98,10 @@ public static <T> T getOrDefault(Map<String, Object> properties, String key, T d
return value != null ? value : defaultValue;
}

public static ObjectNode createObjectNode(){
return new ObjectNode(new JsonNodeFactory(false));
}

@SuppressWarnings("removal")
public static <T> T readTree(JsonNode node, Class<T> clazz) throws IOException {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,13 +128,13 @@ protected void handlePut(RestChannel channel, final RestRequest request, final C

try {
if (request.hasParam("owner")) {
((ObjectNode) content).put("owner", request.param("owner"));
securityJsonNode.put("owner", request.param("owner"));
}
if (request.hasParam("isEnabled")) {
((ObjectNode) content).put("isEnabled", request.param("isEnabled"));
securityJsonNode.put("isEnabled", request.param("isEnabled"));
}
((ObjectNode) content).put("name", username);
internalUsersConfiguration = userService.createOrUpdateAccount((ObjectNode) content);
internalUsersConfiguration = userService.createOrUpdateAccount(securityJsonNode);
}
catch (UserServiceException ex) {
badRequestResponse(channel, ex.getMessage());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

package org.opensearch.security.support;

import com.fasterxml.jackson.databind.node.ObjectNode;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
Expand All @@ -43,7 +44,25 @@ public SecurityJsonNode get(String name) {
JsonNode val = node.get(name);
return new SecurityJsonNode(val);
}


public SecurityJsonNode put(String key, String value) {
if (isNull(node)) {
return new SecurityJsonNode(DefaultObjectMapper.createObjectNode().put(key, value));
} else {
((ObjectNode) node).put(key, value);
return new SecurityJsonNode(node);
}
}

public SecurityJsonNode remove(String key) {
if (isNull(node) || node.getNodeType() != JsonNodeType.OBJECT) {
return new SecurityJsonNode(node);
} else {
((ObjectNode) node).remove(key);
return new SecurityJsonNode(node);
}
}

public String asString() {
if(isNull(node)) {
return null;
Expand Down Expand Up @@ -95,4 +114,12 @@ public List<String> asList() {
public static SecurityJsonNode fromJson(String json) throws IOException {
return new SecurityJsonNode(DefaultObjectMapper.readTree(json));
}

public JsonNode getCapture() {
if(isNull(node) || node.getNodeType() != JsonNodeType.ARRAY) {
return null;
}

return (JsonNode) node;
}
}
23 changes: 10 additions & 13 deletions src/main/java/org/opensearch/security/user/UserService.java
Original file line number Diff line number Diff line change
Expand Up @@ -89,16 +89,14 @@ protected static final SecurityDynamicConfiguration<?> load(final CType config,
/**
* This function will handle the creation or update of a user account.
*
* @param contentAsNode An object node of different account configurations.
* @param securityJsonNode A SecuirtyJsonNode node of different account configurations.
* @return InternalUserConfiguration with the new/updated user
* @throws UserServiceException
* @throws IOException
*/
public SecurityDynamicConfiguration<?> createOrUpdateAccount(ObjectNode contentAsNode) throws IOException {
public SecurityDynamicConfiguration<?> createOrUpdateAccount(SecurityJsonNode securityJsonNode) throws IOException {


SecurityJsonNode securityJsonNode = new SecurityJsonNode(contentAsNode);

final SecurityDynamicConfiguration<?> internalUsersConfiguration = load(getConfigName(), false);
String accountName = securityJsonNode.get("name").asString();

Expand All @@ -109,10 +107,9 @@ public SecurityDynamicConfiguration<?> createOrUpdateAccount(ObjectNode contentA
if (!securityJsonNode.get("attributes").get("owner").isNull() && !securityJsonNode.get("attributes").get("owner").equals(accountName)) { // If this is a service account
verifyServiceAccount(securityJsonNode, accountName);
String password = generatePassword();
contentAsNode.put("hash", hash(password.toCharArray()));
securityJsonNode.put("hash", hash(password.toCharArray()));
}

securityJsonNode = new SecurityJsonNode(contentAsNode);
final List<String> foundRestrictedContents = RESTRICTED_FROM_USERNAME.stream().filter(accountName::contains).collect(Collectors.toList());
if (!foundRestrictedContents.isEmpty()) {
final String restrictedContents = foundRestrictedContents.stream().map(s -> "'" + s + "'").collect(Collectors.joining(","));
Expand All @@ -123,12 +120,12 @@ public SecurityDynamicConfiguration<?> createOrUpdateAccount(ObjectNode contentA
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()));
securityJsonNode.remove("password");
securityJsonNode.put("hash", hash(plainTextPassword.toCharArray()));
} else if (origHash != null && origHash.length() > 0) {
contentAsNode.remove("password");
securityJsonNode.remove("password");
} else if (plainTextPassword != null && plainTextPassword.isEmpty() && origHash == null) {
contentAsNode.remove("password");
securityJsonNode.remove("password");
}

final boolean userExisted = internalUsersConfiguration.exists(accountName);
Expand All @@ -145,13 +142,13 @@ public SecurityDynamicConfiguration<?> createOrUpdateAccount(ObjectNode contentA
if (hash == null || hash.length() == 0) {
throw new UserServiceException("Existing user " + accountName + " has no password, and no new password or hash was specified.");
}
contentAsNode.put("hash", hash);
securityJsonNode.put("hash", hash);
}

internalUsersConfiguration.remove(accountName);
contentAsNode.remove("name");
securityJsonNode.remove("name");

internalUsersConfiguration.putCObject(accountName, DefaultObjectMapper.readTree(contentAsNode, internalUsersConfiguration.getImplementingClass()));
internalUsersConfiguration.putCObject(accountName, DefaultObjectMapper.readTree(securityJsonNode.getCapture(), internalUsersConfiguration.getImplementingClass()));
return internalUsersConfiguration;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -328,38 +328,30 @@ private void verifyPatch(final boolean sendAdminCert, Header... restAdminHeader)
response = rh.executePutRequest(ENDPOINT + "/internalusers/happyServiceLive",
ENABLED_SERVICE_ACCOUNT_BODY, restAdminHeader);
Assert.assertEquals(response.getBody(), HttpStatus.SC_CREATED, response.getStatusCode());
rh.sendAdminCertificate = sendAdminCert;
response = rh.executeGetRequest(ENDPOINT + "/internalusers/happyServiceLive", restAdminHeader);
Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode());
settings = Settings.builder().loadFromSource(response.getBody(), XContentType.JSON).build();


// Add disabled service account
rh.sendAdminCertificate = sendAdminCert;
response = rh.executePutRequest(ENDPOINT + "/internalusers/happyServiceDead",
DISABLED_SERVICE_ACCOUNT_BODY, restAdminHeader);
Assert.assertEquals(response.getBody(), HttpStatus.SC_CREATED, response.getStatusCode());

// Add enabled non-service account
rh.sendAdminCertificate = sendAdminCert;
response = rh.executePutRequest(ENDPOINT + "/internalusers/user_is_owner_1",
ENABLED_NOT_SERVICE_ACCOUNT_BODY, restAdminHeader);
Assert.assertEquals(HttpStatus.SC_CREATED, response.getStatusCode());

// Add service account with password -- Should Fail
rh.sendAdminCertificate = sendAdminCert;
response = rh.executePutRequest(ENDPOINT + "/internalusers/passwordService",
PASSWORD_SERVICE, restAdminHeader);
Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode());

//Add service with hash -- should fail
rh.sendAdminCertificate = sendAdminCert;
response = rh.executePutRequest(ENDPOINT + "/internalusers/hashService",
HASH_SERVICE, restAdminHeader);
Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode());

// Add Service account with password & Hash -- should fail
rh.sendAdminCertificate = sendAdminCert;
response = rh.executePutRequest(ENDPOINT + "/internalusers/passwordHashService",
PASSWORD_HASH_SERVICE, restAdminHeader);
Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode());
Expand Down

0 comments on commit b1a0373

Please sign in to comment.