Skip to content

Commit b20164c

Browse files
jaymodejaymode
committed
Security: remove put privilege API (#32879)
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.
1 parent a8cadfc commit b20164c

File tree

10 files changed

+49
-184
lines changed

10 files changed

+49
-184
lines changed

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/privilege/PutPrivilegesRequestBuilder.java‎

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
import java.io.IOException;
2121
import java.io.InputStream;
2222
import java.util.ArrayList;
23-
import java.util.Collections;
2423
import java.util.List;
2524
import java.util.Objects;
2625

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

38-
/**
39-
* Populate the put privileges request using the given source, application name and privilege name
40-
* The source must contain a single privilege object which matches the application and privilege names.
41-
*/
42-
public PutPrivilegesRequestBuilder source(String applicationName, String expectedName,
43-
BytesReference source, XContentType xContentType)
44-
throws IOException {
45-
Objects.requireNonNull(xContentType);
46-
// EMPTY is ok here because we never call namedObject
47-
try (InputStream stream = source.streamInput();
48-
XContentParser parser = xContentType.xContent()
49-
.createParser(NamedXContentRegistry.EMPTY, LoggingDeprecationHandler.INSTANCE, stream)) {
50-
XContentParser.Token token = parser.currentToken();
51-
if (token == null) {
52-
token = parser.nextToken();
53-
}
54-
if (token == XContentParser.Token.START_OBJECT) {
55-
final ApplicationPrivilegeDescriptor privilege = parsePrivilege(parser, applicationName, expectedName);
56-
this.request.setPrivileges(Collections.singleton(privilege));
57-
} else {
58-
throw new ElasticsearchParseException("expected an object but found {} instead", token);
59-
}
60-
}
61-
return this;
62-
}
63-
6437
ApplicationPrivilegeDescriptor parsePrivilege(XContentParser parser, String applicationName, String privilegeName) throws IOException {
6538
ApplicationPrivilegeDescriptor privilege = ApplicationPrivilegeDescriptor.parse(parser, applicationName, privilegeName, false);
6639
checkPrivilegeName(privilege, applicationName, privilegeName);

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/client/SecurityClient.java‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -295,12 +295,6 @@ public GetPrivilegesRequestBuilder prepareGetPrivileges(String applicationName,
295295
return new GetPrivilegesRequestBuilder(client, GetPrivilegesAction.INSTANCE).application(applicationName).privileges(privileges);
296296
}
297297

298-
public PutPrivilegesRequestBuilder preparePutPrivilege(String applicationName, String privilegeName,
299-
BytesReference bytesReference, XContentType xContentType) throws IOException {
300-
return new PutPrivilegesRequestBuilder(client, PutPrivilegesAction.INSTANCE)
301-
.source(applicationName, privilegeName, bytesReference, xContentType);
302-
}
303-
304298
public PutPrivilegesRequestBuilder preparePutPrivileges(BytesReference bytesReference, XContentType xContentType) throws IOException {
305299
return new PutPrivilegesRequestBuilder(client, PutPrivilegesAction.INSTANCE).source(bytesReference, xContentType);
306300
}

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,6 @@
193193
import org.elasticsearch.xpack.security.rest.action.oauth2.RestInvalidateTokenAction;
194194
import org.elasticsearch.xpack.security.rest.action.privilege.RestDeletePrivilegesAction;
195195
import org.elasticsearch.xpack.security.rest.action.privilege.RestGetPrivilegesAction;
196-
import org.elasticsearch.xpack.security.rest.action.privilege.RestPutPrivilegeAction;
197196
import org.elasticsearch.xpack.security.rest.action.privilege.RestPutPrivilegesAction;
198197
import org.elasticsearch.xpack.security.rest.action.realm.RestClearRealmCacheAction;
199198
import org.elasticsearch.xpack.security.rest.action.role.RestClearRolesCacheAction;
@@ -789,7 +788,6 @@ public List<RestHandler> getRestHandlers(Settings settings, RestController restC
789788
new RestSamlInvalidateSessionAction(settings, restController, getLicenseState()),
790789
new RestGetPrivilegesAction(settings, restController, getLicenseState()),
791790
new RestPutPrivilegesAction(settings, restController, getLicenseState()),
792-
new RestPutPrivilegeAction(settings, restController, getLicenseState()),
793791
new RestDeletePrivilegesAction(settings, restController, getLicenseState())
794792
);
795793
}

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/privilege/RestPutPrivilegeAction.java‎

Lines changed: 0 additions & 49 deletions
This file was deleted.

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/privilege/RestPutPrivilegesAction.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import java.util.Map;
3030

3131
import static org.elasticsearch.rest.RestRequest.Method.POST;
32+
import static org.elasticsearch.rest.RestRequest.Method.PUT;
3233

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

3839
public RestPutPrivilegesAction(Settings settings, RestController controller, XPackLicenseState licenseState) {
3940
super(settings, licenseState);
41+
controller.registerHandler(PUT, "/_xpack/security/privilege/", this);
4042
controller.registerHandler(POST, "/_xpack/security/privilege/", this);
4143
}
4244

‎x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/core/security/action/privilege/PutPrivilegesRequestBuilderTests.java‎

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -52,36 +52,6 @@ private ApplicationPrivilegeDescriptor descriptor(String app, String name, Strin
5252
return new ApplicationPrivilegeDescriptor(app, name, Sets.newHashSet(actions), Collections.emptyMap());
5353
}
5454

55-
public void testBuildRequestFromJsonObject() throws Exception {
56-
final PutPrivilegesRequestBuilder builder = new PutPrivilegesRequestBuilder(null, PutPrivilegesAction.INSTANCE);
57-
builder.source("foo", "read", new BytesArray(
58-
"{ \"application\":\"foo\", \"name\":\"read\", \"actions\":[ \"data:/read/*\", \"admin:/read/*\" ] }"
59-
), XContentType.JSON);
60-
final List<ApplicationPrivilegeDescriptor> privileges = builder.request().getPrivileges();
61-
assertThat(privileges, iterableWithSize(1));
62-
assertThat(privileges, contains(descriptor("foo", "read", "data:/read/*", "admin:/read/*")));
63-
}
64-
65-
public void testPrivilegeNameValidationOfSingleElement() throws Exception {
66-
final PutPrivilegesRequestBuilder builder = new PutPrivilegesRequestBuilder(null, PutPrivilegesAction.INSTANCE);
67-
final IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () ->
68-
builder.source("foo", "write", new BytesArray(
69-
"{ \"application\":\"foo\", \"name\":\"read\", \"actions\":[ \"data:/read/*\", \"admin:/read/*\" ] }"
70-
), XContentType.JSON));
71-
assertThat(exception.getMessage(), containsString("write"));
72-
assertThat(exception.getMessage(), containsString("read"));
73-
}
74-
75-
public void testApplicationNameValidationOfSingleElement() throws Exception {
76-
final PutPrivilegesRequestBuilder builder = new PutPrivilegesRequestBuilder(null, PutPrivilegesAction.INSTANCE);
77-
final IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () ->
78-
builder.source("bar", "read", new BytesArray(
79-
"{ \"application\":\"foo\", \"name\":\"read\", \"actions\":[ \"data:/read/*\", \"admin:/read/*\" ] }"
80-
), XContentType.JSON));
81-
assertThat(exception.getMessage(), containsString("foo"));
82-
assertThat(exception.getMessage(), containsString("bar"));
83-
}
84-
8555
public void testPrivilegeNameValidationOfMultipleElement() throws Exception {
8656
final PutPrivilegesRequestBuilder builder = new PutPrivilegesRequestBuilder(null, PutPrivilegesAction.INSTANCE);
8757
final IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () ->

‎x-pack/plugin/src/test/resources/rest-api-spec/api/xpack.security.put_privilege.json‎

Lines changed: 0 additions & 33 deletions
This file was deleted.

‎x-pack/plugin/src/test/resources/rest-api-spec/api/xpack.security.put_privileges.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"xpack.security.put_privileges": {
33
"documentation": "TODO",
4-
"methods": [ "POST" ],
4+
"methods": [ "PUT", "POST" ],
55
"url": {
66
"path": "/_xpack/security/privilege/",
77
"paths": [

‎x-pack/plugin/src/test/resources/rest-api-spec/test/privileges/10_basic.yml‎

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -30,24 +30,26 @@ teardown:
3030
ignore: 404
3131
---
3232
"Test put and get privileges":
33-
# Single privilege, with names in URL
33+
# Single privilege
3434
- do:
35-
xpack.security.put_privilege:
36-
application: app
37-
name: p1
35+
xpack.security.put_privileges:
3836
body: >
3937
{
40-
"application": "app",
41-
"name": "p1",
42-
"actions": [ "data:read/*" , "action:login" ],
43-
"metadata": {
44-
"key1" : "val1a",
45-
"key2" : "val2a"
38+
"app": {
39+
"p1": {
40+
"application": "app",
41+
"name": "p1",
42+
"actions": [ "data:read/*" , "action:login" ],
43+
"metadata": {
44+
"key1" : "val1a",
45+
"key2" : "val2a"
46+
}
47+
}
4648
}
4749
}
4850
- match: { "app.p1" : { created: true } }
4951

50-
# Multiple privileges, no names in URL
52+
# Multiple privileges
5153
- do:
5254
xpack.security.put_privileges:
5355
body: >
@@ -84,18 +86,18 @@ teardown:
8486
- match: { "app.p3" : { created: true } }
8587
- match: { "app2.p1" : { created: true } }
8688

87-
# Update existing privilege, with names in URL
89+
# Update existing privilege
8890
- do:
89-
xpack.security.put_privilege:
90-
application: app
91-
name: p1
91+
xpack.security.put_privileges:
9292
body: >
9393
{
94-
"application": "app",
95-
"name": "p1",
96-
"actions": [ "data:read/*" , "action:login" ],
97-
"metadata": {
98-
"key3" : "val3"
94+
"app": {
95+
"p1": {
96+
"actions": [ "data:read/*" , "action:login" ],
97+
"metadata": {
98+
"key3" : "val3"
99+
}
100+
}
99101
}
100102
}
101103
- match: { "app.p1" : { created: false } }

‎x-pack/plugin/src/test/resources/rest-api-spec/test/security/authz/40_condtional_cluster_priv.yml‎

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -31,21 +31,25 @@ setup:
3131
}
3232
3333
- do:
34-
xpack.security.put_privilege:
35-
application: app-allow
36-
name: read
34+
xpack.security.put_privileges:
3735
body: >
3836
{
39-
"actions": [ "data:read/*" ]
37+
"app-allow": {
38+
"read": {
39+
"actions": [ "data:read/*" ]
40+
}
41+
}
4042
}
4143
4244
- do:
43-
xpack.security.put_privilege:
44-
application: app_deny
45-
name: read
45+
xpack.security.put_privileges:
4646
body: >
4747
{
48-
"actions": [ "data:read/*" ]
48+
"app-deny": {
49+
"read": {
50+
"actions": [ "data:read/*" ]
51+
}
52+
}
4953
}
5054
5155
---
@@ -82,12 +86,14 @@ teardown:
8286

8387
- do:
8488
headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user
85-
xpack.security.put_privilege:
86-
application: app
87-
name: read
89+
xpack.security.put_privileges:
8890
body: >
8991
{
90-
"actions": [ "data:read/*" ]
92+
"app": {
93+
"read": {
94+
"actions": [ "data:read/*" ]
95+
}
96+
}
9197
}
9298
- match: { "app.read" : { created: true } }
9399

@@ -112,12 +118,14 @@ teardown:
112118
"Test put application privileges when not allowed":
113119
- do:
114120
headers: { Authorization: "Basic dGVzdF91c2VyOngtcGFjay10ZXN0LXBhc3N3b3Jk" } # test_user
115-
xpack.security.put_privilege:
116-
application: app_deny
117-
name: write
121+
xpack.security.put_privileges:
118122
body: >
119123
{
120-
"actions": [ "data:write/*" ]
124+
"app_deny": {
125+
"write": {
126+
"actions": [ "data:write/*" ]
127+
}
128+
}
121129
}
122130
catch: forbidden
123131

0 commit comments

Comments
 (0)