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.

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
Show file tree
Hide file tree
Changes from 42 commits
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
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

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,14 @@
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;
Expand All @@ -50,18 +52,20 @@

public class InternalUsersApiAction extends PatchableResourceApiAction {
static final List<String> RESTRICTED_FROM_USERNAME = ImmutableList.of(
":" // Not allowed in basic auth, see https://stackoverflow.com/a/33391003/533057
":" // Not allowed in basic auth, see https://stackoverflow.com/a/33391003/533057
);

private static final List<Route> routes = addRoutesPrefix(ImmutableList.of(
new Route(Method.GET, "/user/{name}"),
new Route(Method.GET, "/user/"),
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/"),
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 @@ -127,8 +131,8 @@ protected void handlePut(RestChannel channel, final RestRequest request, final C
// changes

try {
if (request.hasParam("owner")) {
((ObjectNode) content).put("owner", request.param("owner"));
if (request.hasParam("isService")) {
((ObjectNode) content).put("isService", request.param("isService"));
}
if (request.hasParam("isEnabled")) {
((ObjectNode) content).put("isEnabled", request.param("isEnabled"));
Expand All @@ -144,8 +148,28 @@ protected void handlePut(RestChannel channel, final RestRequest request, final C
throw new IOException(ex);
}

// 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);
}

internalUsersConfiguration.remove(username);

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


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


@Override
public void onResponse(IndexResponse response) {
if (userExisted) {
Expand All @@ -158,6 +182,63 @@ public void onResponse(IndexResponse response) {
});
}

/**
* Overrides the GET request functionality to allow for the special case of requesting an auth token.
*
* @param channel The channel the request is coming through
* @param request The request itself
* @param client The client executing the request
* @param content The content of the request parsed into a node
* @throws IOException when parsing of configuration files fails (should not happen)
*/
@Override
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
if (username == null || username.length() == 0) {

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

final boolean userExisted = internalUsersConfiguration.exists(username);

if (!userExisted) {
notFound(channel, "Resource '" + username + "' not found.");
return;
}

String authToken = "";
try {
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

notImplemented(channel, Method.POST);
return;
}
} catch (UserServiceException ex) {
badRequestResponse(channel, ex.getMessage());
return;
}
catch (IOException ex) {
throw new IOException(ex);
}

if (!authToken.isEmpty()) {
createdResponse(channel, "'" + username + "' authtoken generated " + authToken);
} else {
badRequestResponse(channel, "'" + username + "' authtoken failed to be created.");
}
}


@Override
protected void filter(SecurityDynamicConfiguration<?> builder) {
super.filter(builder);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ public class InternalUserV7 implements Hideable, Hashed, StaticDefinable {
private String hash;
private boolean reserved;
private boolean hidden;
private boolean isService;
private boolean isEnabled;
@JsonProperty(value = "static")
private boolean _static;
private List<String> backend_roles = Collections.emptyList();
Expand All @@ -58,7 +60,20 @@ private InternalUserV7(String hash, boolean reserved, boolean hidden, List<Strin
this.hidden = hidden;
this.backend_roles = backend_roles;
this.attributes = attributes;
}
this.isEnabled = true;
this.isService = false;
}

private InternalUserV7(String hash, boolean reserved, boolean hidden, List<String> backend_roles, Map<String, String> attributes, Boolean isEnabled, Boolean isService) {
super();
this.hash = hash;
this.reserved = reserved;
this.hidden = hidden;
this.backend_roles = backend_roles;
this.attributes = attributes;
this.isEnabled = isEnabled;
this.isService = isService;
}

public InternalUserV7() {
super();
Expand All @@ -80,7 +95,6 @@ public String getHash() {
public void setHash(String hash) {
this.hash = hash;
}



public boolean isHidden() {
Expand Down Expand Up @@ -114,9 +128,17 @@ public void setAttributes(Map<String, String> attributes) {
this.attributes = attributes;
}

public boolean getIsEnabled() {
return this.isEnabled;
}

public boolean getIsService() {
Copy link
Member

Choose a reason for hiding this comment

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

Shorten this to isService() and isEnabled()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No problem

return this.isService;
}

@Override
public String toString() {
return "InternalUserV7 [hash=" + hash + ", reserved=" + reserved + ", hidden=" + hidden + ", _static=" + _static + ", backend_roles="
return "InternalUserV7 [hash=" + hash + ", isEnabled=" + isEnabled + ", isService=" + isService + ", reserved=" + reserved + ", hidden=" + hidden + ", _static=" + _static + ", backend_roles="
Copy link
Member

Choose a reason for hiding this comment

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

This is awkward with the mix of casing. Do you think the work is is important in these variable names?

Copy link
Contributor Author

@stephen-crawford stephen-crawford May 2, 2023

Choose a reason for hiding this comment

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

Swapped

+ backend_roles + ", attributes=" + attributes + ", description=" + description + "]";
}

Expand All @@ -134,6 +156,14 @@ public void setDescription(String description) {
this.description = description;
}

public void setIsEnabled(boolean isEnabled) {
this.isEnabled = isEnabled;
}

public void setIsService(boolean isService) {
this.isService = isService;
}

public boolean isReserved() {
return reserved;
}
Expand Down
Loading