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.

Sign up for GitHub

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

[Extensions] Generate auth tokens for service accounts #2716

Merged
merged 48 commits into from
May 3, 2023
Merged

[Extensions] Generate auth tokens for service accounts #2716

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
70907f4
Add extensionsManager into GuiceHolder
stephen-crawford Mar 27, 2023
773503a
Basic outline of functions for making service accounts
stephen-crawford Mar 27, 2023
66e8640
Outline of service account creation
stephen-crawford Mar 27, 2023
c0047ec
Extension Service Outline
stephen-crawford Mar 28, 2023
d641510
Add test method names and pass through
stephen-crawford Mar 28, 2023
3edb196
add licenses and spotless
stephen-crawford Mar 28, 2023
02315d9
Test notes
stephen-crawford Mar 29, 2023
f0be8eb
Update
stephen-crawford Mar 30, 2023
a6796cd
clear
stephen-crawford Mar 31, 2023
82a6cb8
Remove imports
stephen-crawford Mar 31, 2023
a157268
Checkstyle
stephen-crawford Mar 31, 2023
686c135
Comment out test
stephen-crawford Apr 3, 2023
b8370c4
spotless
stephen-crawford Apr 3, 2023
6a77b27
Remove extra tests
stephen-crawford Apr 3, 2023
94b65c9
Update imports
stephen-crawford Apr 3, 2023
5a9c7e6
Update imports
stephen-crawford Apr 3, 2023
a693eb9
Update imports
stephen-crawford Apr 3, 2023
e7046e7
Update contains
stephen-crawford Apr 4, 2023
8172aa6
fix service
stephen-crawford Apr 4, 2023
7c5346f
Fix tests
stephen-crawford Apr 5, 2023
858be53
remove prints
stephen-crawford Apr 5, 2023
8ee7dd5
Clean service
stephen-crawford Apr 6, 2023
2243526
Clean service
stephen-crawford Apr 6, 2023
e4cb718
remove unused import
stephen-crawford Apr 6, 2023
09d55c7
fix user service and add tests
stephen-crawford Apr 7, 2023
3de0249
Update imports
stephen-crawford Apr 7, 2023
3d7a7e5
Remove dangling test helper
stephen-crawford Apr 7, 2023
3cc9e55
fix tests
stephen-crawford Apr 10, 2023
4fefe82
remove messaging
stephen-crawford Apr 10, 2023
d618c22
reset auditlog file
stephen-crawford Apr 10, 2023
51988c3
reset auditlog file
stephen-crawford Apr 10, 2023
b64eaa8
reset auditlog file
stephen-crawford Apr 10, 2023
df5c364
Swap tenancy action names
stephen-crawford Apr 12, 2023
6f44ec0
reset tenancy action naem
stephen-crawford Apr 14, 2023
836ad55
Initial changes from feedback
stephen-crawford Apr 21, 2023
354227a
Add extra documentation
stephen-crawford Apr 25, 2023
9f94640
Merge branch 'opensearch-project:main' into genAuthTokens
stephen-crawford Apr 25, 2023
b21820f
Swap to new fields in internalUserV7
stephen-crawford Apr 26, 2023
671d9d6
Merge branch 'opensearch-project:main' into genAuthTokens
stephen-crawford Apr 27, 2023
16e3f0b
Merge branch 'opensearch-project:main' into genAuthTokens
stephen-crawford Apr 28, 2023
e826bf8
Swap to InternalUser Attributes
stephen-crawford Apr 28, 2023
9733b73
Feedback from Peter and DC
stephen-crawford May 2, 2023
d4f668a
remove get keyword
stephen-crawford May 2, 2023
9961f53
Fix throws
stephen-crawford May 2, 2023
dc33ffa
swap back to obj Mapper
stephen-crawford May 2, 2023
972998e
swap to enabled and service
stephen-crawford May 2, 2023
e3c1575
swap to def again...
stephen-crawford May 2, 2023
e5d92df
Merge branch 'main' into genAuthTokens
stephen-crawford May 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 32 additions & 32 deletions build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,16 +75,16 @@ licenseFile = rootProject.file('LICENSE.txt')
noticeFile = rootProject.file('NOTICE.txt')

spotless {
java {
// note: you can use an empty string for all the imports you didn't specify explicitly, and '\\#` prefix for static imports
importOrder('java', 'javax', '', 'com.amazon', 'org.opensearch', '\\#')
targetExclude('src/integrationTest/**')
}
format("integrationTest", JavaExtension) {
target('src/integrationTest/java/**/*.java')
importOrder('java', 'javax', '', 'com.amazon', 'org.opensearch', '\\#')
indentWithTabs(4)
}
java {
// note: you can use an empty string for all the imports you didn't specify explicitly, and '\\#` prefix for static imports
importOrder('java', 'javax', '', 'com.amazon', 'org.opensearch', '\\#')
targetExclude('src/integrationTest/**')
}
format("integrationTest", JavaExtension) {
target('src/integrationTest/java/**/*.java')
importOrder('java', 'javax', '', 'com.amazon', 'org.opensearch', '\\#')
indentWithTabs(4)
}
}

spotbugs {
Expand Down Expand Up @@ -135,17 +135,17 @@ test {
}
jacoco {
excludes = [
"com.sun.jndi.dns.*",
"com.sun.security.sasl.gsskerb.*",
"java.sql.*",
"javax.script.*",
"org.jcp.xml.dsig.internal.dom.*",
"sun.nio.cs.ext.*",
"sun.security.ec.*",
"sun.security.jgss.*",
"sun.security.pkcs11.*",
"sun.security.smartcardio.*",
"sun.util.resources.provider.*"
"com.sun.jndi.dns.*",
"com.sun.security.sasl.gsskerb.*",
"java.sql.*",
"javax.script.*",
"org.jcp.xml.dsig.internal.dom.*",
"sun.nio.cs.ext.*",
"sun.security.ec.*",
"sun.security.jgss.*",
"sun.security.pkcs11.*",
"sun.security.smartcardio.*",
"sun.util.resources.provider.*"
]
}
}
Expand All @@ -159,17 +159,17 @@ task opensslTest(type: Test) {
}
jacoco {
excludes = [
"com.sun.jndi.dns.*",
"com.sun.security.sasl.gsskerb.*",
"java.sql.*",
"javax.script.*",
"org.jcp.xml.dsig.internal.dom.*",
"sun.nio.cs.ext.*",
"sun.security.ec.*",
"sun.security.jgss.*",
"sun.security.pkcs11.*",
"sun.security.smartcardio.*",
"sun.util.resources.provider.*"
"com.sun.jndi.dns.*",
"com.sun.security.sasl.gsskerb.*",
"java.sql.*",
"javax.script.*",
"org.jcp.xml.dsig.internal.dom.*",
"sun.nio.cs.ext.*",
"sun.security.ec.*",
"sun.security.jgss.*",
"sun.security.pkcs11.*",
"sun.security.smartcardio.*",
"sun.util.resources.provider.*"
]
}
}
Expand Down
15 changes: 7 additions & 8 deletions src/main/java/org/opensearch/security/dlic/rest/api/InternalUsersApiAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,14 +58,14 @@ public class InternalUsersApiAction extends PatchableResourceApiAction {
private static final List<Route> routes = addRoutesPrefix(ImmutableList.of(
new Route(Method.GET, "/user/{name}"),
new Route(Method.GET, "/user/"),
new Route(Method.GET, "/user/{name}/authtoken"),
new Route(Method.POST, "/user/{name}/authtoken"),
new Route(Method.DELETE, "/user/{name}"),
new Route(Method.PUT, "/user/{name}"),

// corrected mapping, introduced in OpenSearch Security
new Route(Method.GET, "/internalusers/{name}"),
new Route(Method.GET, "/internalusers/{name}/authtoken"),
new Route(Method.GET, "/internalusers/"),
new Route(Method.POST, "/internalusers/{name}/authtoken"),
new Route(Method.DELETE, "/internalusers/{name}"),
new Route(Method.PUT, "/internalusers/{name}"),
new Route(Method.PATCH, "/internalusers/"),
Expand Down Expand Up @@ -192,17 +192,17 @@ public void onResponse(IndexResponse response) {
* @throws IOException when parsing of configuration files fails (should not happen)
*/
@Override
protected void handleGet(final RestChannel channel, RestRequest request, Client client, final JsonNode content) throws IOException{
protected void handlePost(final RestChannel channel, RestRequest request, Client client, final JsonNode content) throws IOException{

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

final SecurityDynamicConfiguration<?> internalUsersConfiguration = load(getConfigName(), true);
filter(internalUsersConfiguration); // Hides hashes

// no specific resource requested, return complete internal user store
// no specific resource requested
if (username == null || username.length() == 0) {

successResponse(channel, internalUsersConfiguration);
notImplemented(channel, Method.POST);
return;
}

Expand All @@ -218,10 +218,9 @@ protected void handleGet(final RestChannel channel, RestRequest request, Client
if (request.uri().contains("/internalusers/" + username + "/authtoken") && request.uri().endsWith("/authtoken")) { // Handle auth token fetching

authToken = userService.generateAuthToken(username);
} else { // Not an auth token request so just get the target user
} else { // Not an auth token request

internalUsersConfiguration.removeOthers(username);
successResponse(channel, internalUsersConfiguration);
notImplemented(channel, Method.POST);
return;
}
} catch (UserServiceException ex) {
Expand Down
25 changes: 10 additions & 15 deletions src/main/java/org/opensearch/security/user/UserService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,11 @@
import java.util.Base64;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.stream.Collectors;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.node.ObjectNode;
import com.google.common.collect.ImmutableList;
import org.apache.logging.log4j.LogManager;
Expand Down Expand Up @@ -121,7 +120,9 @@ public SecurityDynamicConfiguration<?> createOrUpdateAccount(ObjectNode contentA
throw new UserServiceException(NO_ACCOUNT_NAME_MESSAGE);
}

if (!securityJsonNode.get("attributes").get("isService").isNull() && securityJsonNode.get("attributes").get("isService").asString().equalsIgnoreCase("true"))
SecurityJsonNode attributeNode = securityJsonNode.get("attributes");

if (!attributeNode.get("isService").isNull() && attributeNode.get("isService").asString().equalsIgnoreCase("true"))
{ // If this is a service account
verifyServiceAccount(securityJsonNode, accountName);
String password = generatePassword();
Expand Down Expand Up @@ -150,7 +151,7 @@ public SecurityDynamicConfiguration<?> createOrUpdateAccount(ObjectNode contentA
contentAsNode.remove("password");
}

if (!securityJsonNode.get("attributes").get("isEnabled").isNull()) {
if (!attributeNode.get("isEnabled").isNull()) {
contentAsNode.put("isEnabled", securityJsonNode.get("isEnabled").asString());
}

Expand Down Expand Up @@ -220,22 +221,16 @@ public String generateAuthToken(String accountName) throws IOException {

String authToken = null;
try {
ObjectMapper mapper = new ObjectMapper();
DefaultObjectMapper mapper = new DefaultObjectMapper();
JsonNode accountDetails = mapper.readTree(internalUsersConfiguration.getCEntry(accountName).toString());
final ObjectNode contentAsNode = (ObjectNode) accountDetails;
SecurityJsonNode securityJsonNode = new SecurityJsonNode(contentAsNode);

if (securityJsonNode.get("isService").isNull()) { // If this is not a service account
throw new UserServiceException(AUTH_TOKEN_GENERATION_MESSAGE);
}
if (Objects.requireNonNull(securityJsonNode.get("isService").asString()).equalsIgnoreCase("false")) { // If this is not a service account
throw new UserServiceException(AUTH_TOKEN_GENERATION_MESSAGE);
}
if (securityJsonNode.get("isEnabled").isNull()) { // If there is no enabled flag
throw new UserServiceException(AUTH_TOKEN_GENERATION_MESSAGE);
if (Optional.of(securityJsonNode.get("isService").asString().equalsIgnoreCase("false")).orElseThrow(() -> {throw new UserServiceException(AUTH_TOKEN_GENERATION_MESSAGE);})) {
throw new UserServiceException(AUTH_TOKEN_GENERATION_MESSAGE); // If the account is not a service account
}
if (Objects.requireNonNull(securityJsonNode.get("isEnabled").asString()).equalsIgnoreCase("false")) { // If the service account is not active
throw new UserServiceException(AUTH_TOKEN_GENERATION_MESSAGE);
if (Optional.of(securityJsonNode.get("isEnabled").asString().equalsIgnoreCase("false")).orElseThrow(() -> {throw new UserServiceException(AUTH_TOKEN_GENERATION_MESSAGE);})) {
throw new UserServiceException(AUTH_TOKEN_GENERATION_MESSAGE); // If the service account is not active
Copy link
Member

@DarshitChanpura DarshitChanpura May 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

orElseThrow will not be utilized here as the value will either be a boolean or it will throw an NPE stating return value of isEnabled is null. In order to utilize orElseThrow to catch a missing isEnabled or isService property in SecurityJsonNode, you can make following changes:

        UserServiceException e = new UserServiceException(AUTH_TOKEN_GENERATION_MESSAGE);
        boolean isService = readBooleanFromJsonNode(securityJsonNode, "isService", e);
        boolean isEnabled = readBooleanFromJsonNode(securityJsonNode, "isEnabled", e);
        
        if(!isService || !isEnabled) {
            throw e;
        }


        private boolean readBooleanFromJsonNode(SecurityJsonNode node, String key, Exception e) {
	    return Boolean.parseBoolean(Optional.ofNullable(node.get(key)).orElseThrow(() -> e).asText());
	}

}

// Generate a new password for the account and store the hash of it
Expand Down
20 changes: 11 additions & 9 deletions src/test/java/org/opensearch/security/dlic/rest/api/UserApiTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ protected String getEndpointPrefix() {
}


final int USER_SETTING_SIZE = 9 * 19; // Lines per account entry * number of accounts

private static final String ENABLED_SERVICE_ACCOUNT_BODY = "{"
+ " \"attributes\": { \"isService\": \"true\", "
+ "\"isEnabled\": \"true\"}"
Expand Down Expand Up @@ -87,7 +89,7 @@ public void testSecurityRoles() throws Exception {
.executeGetRequest(ENDPOINT + "/" + CType.INTERNALUSERS.toLCString());
Assert.assertEquals(response.getBody(), HttpStatus.SC_OK, response.getStatusCode());
Settings settings = Settings.builder().loadFromSource(response.getBody(), XContentType.JSON).build();
Assert.assertEquals(171, settings.size());
Assert.assertEquals(USER_SETTING_SIZE, settings.size());
response = rh.executePatchRequest(ENDPOINT + "/internalusers", "[{ \"op\": \"add\", \"path\": \"/newuser\", \"value\": {\"password\": \"newuser\", \"opendistro_security_roles\": [\"opendistro_security_all_access\"] } }]", new Header[0]);
Assert.assertEquals(response.getBody(), HttpStatus.SC_OK, response.getStatusCode());

Expand Down Expand Up @@ -137,7 +139,7 @@ public void testUserApi() throws Exception {
HttpResponse response = rh.executeGetRequest(ENDPOINT + "/" + CType.INTERNALUSERS.toLCString());
Assert.assertEquals(response.getBody(), HttpStatus.SC_OK, response.getStatusCode());
Settings settings = Settings.builder().loadFromSource(response.getBody(), XContentType.JSON).build();
Assert.assertEquals(171, settings.size());
Assert.assertEquals(USER_SETTING_SIZE, settings.size());
verifyGet();
verifyPut();
verifyPatch(true);
Expand Down Expand Up @@ -417,7 +419,7 @@ private void verifyAuthToken(final boolean sendAdminCert, Header... restAdminHea
response = rh.executeGetRequest(ENDPOINT + "/internalusers/happyServiceLive", restAdminHeader);
Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode());

response = rh.executeGetRequest(ENDPOINT + "/internalusers/happyServiceLive/authtoken",
response = rh.executePostRequest(ENDPOINT + "/internalusers/happyServiceLive/authtoken",
ENABLED_SERVICE_ACCOUNT_BODY, restAdminHeader);
Assert.assertEquals(response.getBody(), HttpStatus.SC_CREATED, response.getStatusCode());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this response.getBody() the entire body of the request? Or just the status code?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So for asserts, you can provide 3 arguments in which case the first one is what will be output if the assertion fails. It would be the whole body though in this case.

String tokenFromResponse = response.getBody();
Expand All @@ -433,7 +435,7 @@ private void verifyAuthToken(final boolean sendAdminCert, Header... restAdminHea
DISABLED_SERVICE_ACCOUNT_BODY, restAdminHeader);
Assert.assertEquals(response.getBody(), HttpStatus.SC_CREATED, response.getStatusCode());

response = rh.executeGetRequest(ENDPOINT + "/internalusers/happyServiceDead/authtoken",
response = rh.executePostRequest(ENDPOINT + "/internalusers/happyServiceDead/authtoken",
ENABLED_SERVICE_ACCOUNT_BODY, restAdminHeader);
Assert.assertEquals(response.getBody(), HttpStatus.SC_BAD_REQUEST, response.getStatusCode());

Expand All @@ -444,7 +446,7 @@ private void verifyAuthToken(final boolean sendAdminCert, Header... restAdminHea
ENABLED_NOT_SERVICE_ACCOUNT_BODY, restAdminHeader);
Assert.assertEquals(HttpStatus.SC_CREATED, response.getStatusCode());

response = rh.executeGetRequest(ENDPOINT + "/internalusers/user_is_owner_1/authtoken",
response = rh.executePostRequest(ENDPOINT + "/internalusers/user_is_owner_1/authtoken",
ENABLED_SERVICE_ACCOUNT_BODY, restAdminHeader);
Assert.assertEquals(response.getBody(), HttpStatus.SC_BAD_REQUEST, response.getStatusCode());

Expand Down Expand Up @@ -535,7 +537,7 @@ public void testUserApiWithRestAdminPermissions() throws Exception {
HttpResponse response = rh.executeGetRequest(ENDPOINT + "/" + CType.INTERNALUSERS.toLCString(), restApiAdminHeader);
Assert.assertEquals(response.getBody(), HttpStatus.SC_OK, response.getStatusCode());
Settings settings = Settings.builder().loadFromSource(response.getBody(), XContentType.JSON).build();
Assert.assertEquals(171, settings.size());
Assert.assertEquals(USER_SETTING_SIZE, settings.size());
verifyGet(restApiAdminHeader);
verifyPut(restApiAdminHeader);
verifyPatch(false, restApiAdminHeader);
Expand All @@ -553,7 +555,7 @@ public void testUserApiWithRestInternalUsersAdminPermissions() throws Exception
HttpResponse response = rh.executeGetRequest(ENDPOINT + "/" + CType.INTERNALUSERS.toLCString(), restApiInternalUsersAdminHeader);
Assert.assertEquals(response.getBody(), HttpStatus.SC_OK, response.getStatusCode());
Settings settings = Settings.builder().loadFromSource(response.getBody(), XContentType.JSON).build();
Assert.assertEquals(171, settings.size());
Assert.assertEquals(USER_SETTING_SIZE, settings.size());
verifyGet(restApiInternalUsersAdminHeader);
verifyPut(restApiInternalUsersAdminHeader);
verifyPatch(false, restApiInternalUsersAdminHeader);
Expand Down Expand Up @@ -582,7 +584,7 @@ public void testPasswordRules() throws Exception {
.executeGetRequest("_plugins/_security/api/" + CType.INTERNALUSERS.toLCString());
Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode());
Settings settings = Settings.builder().loadFromSource(response.getBody(), XContentType.JSON).build();
Assert.assertEquals(171, settings.size());
Assert.assertEquals(USER_SETTING_SIZE, settings.size());

addUserWithPassword("tooshoort", "", HttpStatus.SC_BAD_REQUEST);
addUserWithPassword("tooshoort", "123", HttpStatus.SC_BAD_REQUEST);
Expand Down Expand Up @@ -662,7 +664,7 @@ public void testUserApiWithDots() throws Exception {
.executeGetRequest(ENDPOINT + "/" + CType.INTERNALUSERS.toLCString());
Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode());
Settings settings = Settings.builder().loadFromSource(response.getBody(), XContentType.JSON).build();
Assert.assertEquals(171, settings.size());
Assert.assertEquals(USER_SETTING_SIZE, settings.size());

addUserWithPassword(".my.dotuser0", "$2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m",
HttpStatus.SC_CREATED);
Expand Down