Skip to content

Commit

Permalink
Security: remove put privilege API (#32879)
Browse files Browse the repository at this point in the history
This commit removes the put privilege API in favor of having a single API to
create and update privileges. If we see the need to have an API like this in
the future we can always add it back.
  • Loading branch information
jaymode committed Aug 17, 2018
1 parent 813b189 commit a13174f
Show file tree
Hide file tree
Showing 10 changed files with 49 additions and 184 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import java.io.IOException;
import java.io.InputStream;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Objects;

Expand All @@ -35,32 +34,6 @@ public PutPrivilegesRequestBuilder(ElasticsearchClient client, PutPrivilegesActi
super(client, action, new PutPrivilegesRequest());
}

/**
* Populate the put privileges request using the given source, application name and privilege name
* The source must contain a single privilege object which matches the application and privilege names.
*/
public PutPrivilegesRequestBuilder source(String applicationName, String expectedName,
BytesReference source, XContentType xContentType)
throws IOException {
Objects.requireNonNull(xContentType);
// EMPTY is ok here because we never call namedObject
try (InputStream stream = source.streamInput();
XContentParser parser = xContentType.xContent()
.createParser(NamedXContentRegistry.EMPTY, LoggingDeprecationHandler.INSTANCE, stream)) {
XContentParser.Token token = parser.currentToken();
if (token == null) {
token = parser.nextToken();
}
if (token == XContentParser.Token.START_OBJECT) {
final ApplicationPrivilegeDescriptor privilege = parsePrivilege(parser, applicationName, expectedName);
this.request.setPrivileges(Collections.singleton(privilege));
} else {
throw new ElasticsearchParseException("expected an object but found {} instead", token);
}
}
return this;
}

ApplicationPrivilegeDescriptor parsePrivilege(XContentParser parser, String applicationName, String privilegeName) throws IOException {
ApplicationPrivilegeDescriptor privilege = ApplicationPrivilegeDescriptor.parse(parser, applicationName, privilegeName, false);
checkPrivilegeName(privilege, applicationName, privilegeName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -295,12 +295,6 @@ public GetPrivilegesRequestBuilder prepareGetPrivileges(String applicationName,
return new GetPrivilegesRequestBuilder(client, GetPrivilegesAction.INSTANCE).application(applicationName).privileges(privileges);
}

public PutPrivilegesRequestBuilder preparePutPrivilege(String applicationName, String privilegeName,
BytesReference bytesReference, XContentType xContentType) throws IOException {
return new PutPrivilegesRequestBuilder(client, PutPrivilegesAction.INSTANCE)
.source(applicationName, privilegeName, bytesReference, xContentType);
}

public PutPrivilegesRequestBuilder preparePutPrivileges(BytesReference bytesReference, XContentType xContentType) throws IOException {
return new PutPrivilegesRequestBuilder(client, PutPrivilegesAction.INSTANCE).source(bytesReference, xContentType);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,6 @@
import org.elasticsearch.xpack.security.rest.action.oauth2.RestInvalidateTokenAction;
import org.elasticsearch.xpack.security.rest.action.privilege.RestDeletePrivilegesAction;
import org.elasticsearch.xpack.security.rest.action.privilege.RestGetPrivilegesAction;
import org.elasticsearch.xpack.security.rest.action.privilege.RestPutPrivilegeAction;
import org.elasticsearch.xpack.security.rest.action.privilege.RestPutPrivilegesAction;
import org.elasticsearch.xpack.security.rest.action.realm.RestClearRealmCacheAction;
import org.elasticsearch.xpack.security.rest.action.role.RestClearRolesCacheAction;
Expand Down Expand Up @@ -788,7 +787,6 @@ public List<RestHandler> getRestHandlers(Settings settings, RestController restC
new RestSamlInvalidateSessionAction(settings, restController, getLicenseState()),
new RestGetPrivilegesAction(settings, restController, getLicenseState()),
new RestPutPrivilegesAction(settings, restController, getLicenseState()),
new RestPutPrivilegeAction(settings, restController, getLicenseState()),
new RestDeletePrivilegesAction(settings, restController, getLicenseState())
);
}
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import java.util.Map;

import static org.elasticsearch.rest.RestRequest.Method.POST;
import static org.elasticsearch.rest.RestRequest.Method.PUT;

/**
* Rest endpoint to add one or more {@link ApplicationPrivilege} objects to the security index
Expand All @@ -37,6 +38,7 @@ public class RestPutPrivilegesAction extends SecurityBaseRestHandler {

public RestPutPrivilegesAction(Settings settings, RestController controller, XPackLicenseState licenseState) {
super(settings, licenseState);
controller.registerHandler(PUT, "/_xpack/security/privilege/", this);
controller.registerHandler(POST, "/_xpack/security/privilege/", this);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,36 +52,6 @@ private ApplicationPrivilegeDescriptor descriptor(String app, String name, Strin
return new ApplicationPrivilegeDescriptor(app, name, Sets.newHashSet(actions), Collections.emptyMap());
}

public void testBuildRequestFromJsonObject() throws Exception {
final PutPrivilegesRequestBuilder builder = new PutPrivilegesRequestBuilder(null, PutPrivilegesAction.INSTANCE);
builder.source("foo", "read", new BytesArray(
"{ \"application\":\"foo\", \"name\":\"read\", \"actions\":[ \"data:/read/*\", \"admin:/read/*\" ] }"
), XContentType.JSON);
final List<ApplicationPrivilegeDescriptor> privileges = builder.request().getPrivileges();
assertThat(privileges, iterableWithSize(1));
assertThat(privileges, contains(descriptor("foo", "read", "data:/read/*", "admin:/read/*")));
}

public void testPrivilegeNameValidationOfSingleElement() throws Exception {
final PutPrivilegesRequestBuilder builder = new PutPrivilegesRequestBuilder(null, PutPrivilegesAction.INSTANCE);
final IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () ->
builder.source("foo", "write", new BytesArray(
"{ \"application\":\"foo\", \"name\":\"read\", \"actions\":[ \"data:/read/*\", \"admin:/read/*\" ] }"
), XContentType.JSON));
assertThat(exception.getMessage(), containsString("write"));
assertThat(exception.getMessage(), containsString("read"));
}

public void testApplicationNameValidationOfSingleElement() throws Exception {
final PutPrivilegesRequestBuilder builder = new PutPrivilegesRequestBuilder(null, PutPrivilegesAction.INSTANCE);
final IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () ->
builder.source("bar", "read", new BytesArray(
"{ \"application\":\"foo\", \"name\":\"read\", \"actions\":[ \"data:/read/*\", \"admin:/read/*\" ] }"
), XContentType.JSON));
assertThat(exception.getMessage(), containsString("foo"));
assertThat(exception.getMessage(), containsString("bar"));
}

public void testPrivilegeNameValidationOfMultipleElement() throws Exception {
final PutPrivilegesRequestBuilder builder = new PutPrivilegesRequestBuilder(null, PutPrivilegesAction.INSTANCE);
final IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () ->
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"xpack.security.put_privileges": {
"documentation": "TODO",
"methods": [ "POST" ],
"methods": [ "PUT", "POST" ],
"url": {
"path": "/_xpack/security/privilege/",
"paths": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,24 +30,26 @@ teardown:
ignore: 404
---
"Test put and get privileges":
# Single privilege, with names in URL
# Single privilege
- do:
xpack.security.put_privilege:
application: app
name: p1
xpack.security.put_privileges:
body: >
{
"application": "app",
"name": "p1",
"actions": [ "data:read/*" , "action:login" ],
"metadata": {
"key1" : "val1a",
"key2" : "val2a"
"app": {
"p1": {
"application": "app",
"name": "p1",
"actions": [ "data:read/*" , "action:login" ],
"metadata": {
"key1" : "val1a",
"key2" : "val2a"
}
}
}
}
- match: { "app.p1" : { created: true } }

# Multiple privileges, no names in URL
# Multiple privileges
- do:
xpack.security.put_privileges:
body: >
Expand Down Expand Up @@ -84,18 +86,18 @@ teardown:
- match: { "app.p3" : { created: true } }
- match: { "app2.p1" : { created: true } }

# Update existing privilege, with names in URL
# Update existing privilege
- do:
xpack.security.put_privilege:
application: app
name: p1
xpack.security.put_privileges:
body: >
{
"application": "app",
"name": "p1",
"actions": [ "data:read/*" , "action:login" ],
"metadata": {
"key3" : "val3"
"app": {
"p1": {
"actions": [ "data:read/*" , "action:login" ],
"metadata": {
"key3" : "val3"
}
}
}
}
- match: { "app.p1" : { created: false } }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,21 +31,25 @@ setup:
}
- do:
xpack.security.put_privilege:
application: app-allow
name: read
xpack.security.put_privileges:
body: >
{
"actions": [ "data:read/*" ]
"app-allow": {
"read": {
"actions": [ "data:read/*" ]
}
}
}
- do:
xpack.security.put_privilege:
application: app_deny
name: read
xpack.security.put_privileges:
body: >
{
"actions": [ "data:read/*" ]
"app-deny": {
"read": {
"actions": [ "data:read/*" ]
}
}
}
---
Expand Down Expand Up @@ -82,12 +86,14 @@ teardown:

- do:
headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user
xpack.security.put_privilege:
application: app
name: read
xpack.security.put_privileges:
body: >
{
"actions": [ "data:read/*" ]
"app": {
"read": {
"actions": [ "data:read/*" ]
}
}
}
- match: { "app.read" : { created: true } }

Expand All @@ -112,12 +118,14 @@ teardown:
"Test put application privileges when not allowed":
- do:
headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user
xpack.security.put_privilege:
application: app_deny
name: write
xpack.security.put_privileges:
body: >
{
"actions": [ "data:write/*" ]
"app_deny": {
"write": {
"actions": [ "data:write/*" ]
}
}
}
catch: forbidden

Expand Down

0 comments on commit a13174f

Please sign in to comment.