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

Introduce Application Privileges to Roles #30164

Merged
merged 39 commits into from
Jun 7, 2018
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
fdf628b
Support application (custom) privileges
tvernum Apr 26, 2018
f7463bc
Static caches are evil
tvernum Apr 26, 2018
1af402c
Merge branch 'master' into feature/2767-custom-priv
tvernum Apr 30, 2018
9b1ff6e
Grant all application privileges to superuser
tvernum Apr 30, 2018
662fd50
Address review feedback
tvernum May 1, 2018
ccbb6eb
Merge branch 'master' into feature/2767-custom-priv
tvernum May 1, 2018
80977b0
Merge branch 'master' into feature/2767-custom-priv
tvernum May 4, 2018
c1cff59
Merge branch 'master' into feature/2767-custom-priv
tvernum May 9, 2018
ef62161
Fix merge
tvernum May 9, 2018
66c0d46
Merge branch 'master' into feature/2767-custom-priv
tvernum May 10, 2018
39c1222
Clear role cache on app privilege changes
tvernum May 10, 2018
bf0e7b3
Merge branch 'master' into feature/2767-custom-priv
tvernum May 18, 2018
5b37049
Merge branch 'master' into feature/2767-custom-priv
tvernum May 18, 2018
4526db1
Address feedback
tvernum May 22, 2018
15cf7b1
Merge branch 'master' into feature/2767-custom-priv
tvernum May 22, 2018
a2bad11
Address feedback
tvernum May 25, 2018
b9a5c41
Merge branch 'master' into feature/2767-custom-priv
tvernum May 25, 2018
e0a2013
Fix checkstyle
tvernum May 25, 2018
22418cc
Address feedback
tvernum May 25, 2018
e201c91
Address feedback
tvernum May 25, 2018
48820a2
Infer app/privilege name in PutPrivileges request
tvernum May 25, 2018
99d4dd1
Merge branch 'master' into feature/2767-custom-priv
tvernum May 25, 2018
502dd4b
Fix fragile test
tvernum May 28, 2018
24fcff9
Merge branch 'master' into feature/2767-custom-priv
tvernum May 28, 2018
2bcdadb
Fix import
tvernum May 28, 2018
ffd3058
Merge branch 'master' into feature/2767-custom-priv
tvernum May 30, 2018
85a11e0
Merge branch 'master' into feature/2767-custom-priv
tvernum May 31, 2018
26629a2
Merge branch 'master' into feature/2767-custom-priv
tvernum Jun 4, 2018
f9a47a4
Address feedback
tvernum Jun 5, 2018
63512f1
Merge branch 'master' into feature/2767-custom-priv
tvernum Jun 5, 2018
8098b96
Update for #30966
tvernum Jun 5, 2018
d40da46
Merge branch 'master' into feature/2767-custom-priv
tvernum Jun 5, 2018
92e8307
Merge branch 'security-app-privs' into feature/2767-custom-priv
tvernum Jun 6, 2018
5d80b39
Fix ML use of HasPrivileges
tvernum Jun 6, 2018
6f6bf4c
Update doc snippets for API changes
tvernum Jun 6, 2018
e8191a0
Fix RoleDescriptor.toString
tvernum Jun 6, 2018
4ebba4a
Merge branch 'master' into feature/2767-custom-priv
tvernum Jun 6, 2018
ea5eeab
Merge branch 'security-app-privs' into feature/2767-custom-priv
tvernum Jun 7, 2018
f43f04c
Fix broken test
tvernum Jun 7, 2018
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
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
import java.time.ZoneId;
import java.time.ZonedDateTime;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Date;
import java.util.EnumSet;
Expand All @@ -70,6 +71,7 @@
import java.util.Map;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import java.util.function.Function;
import java.util.function.IntFunction;
import java.util.function.Supplier;

Expand Down Expand Up @@ -932,8 +934,23 @@ public <T extends Streamable> List<T> readStreamableList(Supplier<T> constructor
* Reads a list of objects
*/
public <T> List<T> readList(Writeable.Reader<T> reader) throws IOException {
return readCollection(reader, ArrayList::new);
}

/**
* Reads a set of objects
*/
public <T> Set<T> readSet(Writeable.Reader<T> reader) throws IOException {
return readCollection(reader, HashSet::new);
}

/**
* Reads a collection of objects
*/
private <T, C extends Collection<? super T>> C readCollection(Writeable.Reader<T> reader,
IntFunction<C> constructor) throws IOException {
int count = readArraySize();
List<T> builder = new ArrayList<>(count);
C builder = constructor.apply(count);
for (int i=0; i<count; i++) {
builder.add(reader.read(this));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
import java.nio.file.NoSuchFileException;
import java.nio.file.NotDirectoryException;
import java.time.ZonedDateTime;
import java.util.Collection;
import java.util.Collections;
import java.util.Date;
import java.util.EnumMap;
Expand Down Expand Up @@ -969,6 +970,16 @@ public void writeList(List<? extends Writeable> list) throws IOException {
}
}

/**
* Writes a list of generic objects via a {@link Writer}
Copy link
Member

Choose a reason for hiding this comment

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

s/list/collection

*/
public <T> void writeCollection(Collection<T> collection, Writer<T> writer) throws IOException {
writeVInt(collection.size());
for (T val: collection) {
writer.write(this, val);
}
}

/**
* Writes a list of strings
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Locale;
Expand All @@ -42,6 +43,7 @@
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasToString;
import static org.hamcrest.Matchers.iterableWithSize;

public class StreamTests extends ESTestCase {

Expand All @@ -65,7 +67,7 @@ public void testBooleanSerialization() throws IOException {
final Set<Byte> set = IntStream.range(Byte.MIN_VALUE, Byte.MAX_VALUE).mapToObj(v -> (byte) v).collect(Collectors.toSet());
set.remove((byte) 0);
set.remove((byte) 1);
final byte[] corruptBytes = new byte[] { randomFrom(set) };
final byte[] corruptBytes = new byte[]{randomFrom(set)};
final BytesReference corrupt = new BytesArray(corruptBytes);
final IllegalStateException e = expectThrows(IllegalStateException.class, () -> corrupt.streamInput().readBoolean());
final String message = String.format(Locale.ROOT, "unexpected byte [0x%02x]", corruptBytes[0]);
Expand Down Expand Up @@ -100,7 +102,7 @@ public void testOptionalBooleanSerialization() throws IOException {
set.remove((byte) 0);
set.remove((byte) 1);
set.remove((byte) 2);
final byte[] corruptBytes = new byte[] { randomFrom(set) };
final byte[] corruptBytes = new byte[]{randomFrom(set)};
final BytesReference corrupt = new BytesArray(corruptBytes);
final IllegalStateException e = expectThrows(IllegalStateException.class, () -> corrupt.streamInput().readOptionalBoolean());
final String message = String.format(Locale.ROOT, "unexpected byte [0x%02x]", corruptBytes[0]);
Expand All @@ -119,22 +121,22 @@ public void testRandomVLongSerialization() throws IOException {

public void testSpecificVLongSerialization() throws IOException {
List<Tuple<Long, byte[]>> values =
Arrays.asList(
new Tuple<>(0L, new byte[]{0}),
new Tuple<>(-1L, new byte[]{1}),
new Tuple<>(1L, new byte[]{2}),
new Tuple<>(-2L, new byte[]{3}),
new Tuple<>(2L, new byte[]{4}),
new Tuple<>(Long.MIN_VALUE, new byte[]{-1, -1, -1, -1, -1, -1, -1, -1, -1, 1}),
new Tuple<>(Long.MAX_VALUE, new byte[]{-2, -1, -1, -1, -1, -1, -1, -1, -1, 1})

);
Arrays.asList(
new Tuple<>(0L, new byte[]{0}),
new Tuple<>(-1L, new byte[]{1}),
new Tuple<>(1L, new byte[]{2}),
new Tuple<>(-2L, new byte[]{3}),
new Tuple<>(2L, new byte[]{4}),
new Tuple<>(Long.MIN_VALUE, new byte[]{-1, -1, -1, -1, -1, -1, -1, -1, -1, 1}),
new Tuple<>(Long.MAX_VALUE, new byte[]{-2, -1, -1, -1, -1, -1, -1, -1, -1, 1})

);
for (Tuple<Long, byte[]> value : values) {
BytesStreamOutput out = new BytesStreamOutput();
out.writeZLong(value.v1());
assertArrayEquals(Long.toString(value.v1()), value.v2(), BytesReference.toBytes(out.bytes()));
BytesReference bytes = new BytesArray(value.v2());
assertEquals(Arrays.toString(value.v2()), (long)value.v1(), bytes.streamInput().readZLong());
assertEquals(Arrays.toString(value.v2()), (long) value.v1(), bytes.streamInput().readZLong());
}
}

Expand All @@ -158,7 +160,7 @@ public void testLinkedHashMap() throws IOException {
}
BytesStreamOutput out = new BytesStreamOutput();
out.writeGenericValue(write);
LinkedHashMap<String, Integer> read = (LinkedHashMap<String, Integer>)out.bytes().streamInput().readGenericValue();
LinkedHashMap<String, Integer> read = (LinkedHashMap<String, Integer>) out.bytes().streamInput().readGenericValue();
assertEquals(size, read.size());
int index = 0;
for (Map.Entry<String, Integer> entry : read.entrySet()) {
Expand All @@ -172,7 +174,8 @@ public void testFilterStreamInputDelegatesAvailable() throws IOException {
final int length = randomIntBetween(1, 1024);
StreamInput delegate = StreamInput.wrap(new byte[length]);

FilterStreamInput filterInputStream = new FilterStreamInput(delegate) {};
FilterStreamInput filterInputStream = new FilterStreamInput(delegate) {
};
assertEquals(filterInputStream.available(), length);

// read some bytes
Expand Down Expand Up @@ -201,7 +204,7 @@ public void testReadArraySize() throws IOException {
}
stream.writeByteArray(array);
InputStreamStreamInput streamInput = new InputStreamStreamInput(StreamInput.wrap(BytesReference.toBytes(stream.bytes())), array
.length-1);
.length - 1);
expectThrows(EOFException.class, streamInput::readByteArray);
streamInput = new InputStreamStreamInput(StreamInput.wrap(BytesReference.toBytes(stream.bytes())), BytesReference.toBytes(stream
.bytes()).length);
Expand Down Expand Up @@ -230,6 +233,21 @@ public void testWritableArrays() throws IOException {
assertThat(targetArray, equalTo(sourceArray));
}

public void testSetOfLongs() throws IOException {
final int size = randomIntBetween(0, 6);
final Set<Long> sourceSet = new HashSet<>(size);
for (int i = 0; i < size; i++) {
sourceSet.add(randomLongBetween(i * 1000, (i + 1) * 1000 - 1));
}
assertThat(sourceSet, iterableWithSize(size));

final BytesStreamOutput out = new BytesStreamOutput();
out.writeCollection(sourceSet, StreamOutput::writeLong);

final Set<Long> targetSet = out.bytes().streamInput().readSet(StreamInput::readLong);
assertThat(targetSet, equalTo(sourceSet));
}

static final class WriteableString implements Writeable {
final String string;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
package org.elasticsearch.xpack.core.security.action.privilege;

import org.elasticsearch.action.Action;
import org.elasticsearch.client.ElasticsearchClient;

/**
* Action for deleting application privileges.
*/
public final class DeletePrivilegesAction
extends Action<DeletePrivilegesRequest, DeletePrivilegesResponse, DeletePrivilegesRequestBuilder> {

public static final DeletePrivilegesAction INSTANCE = new DeletePrivilegesAction();
public static final String NAME = "cluster:admin/xpack/security/privilege/delete";

private DeletePrivilegesAction() {
super(NAME);
}

@Override
public DeletePrivilegesRequestBuilder newRequestBuilder(ElasticsearchClient client) {
return new DeletePrivilegesRequestBuilder(client, this);
}

@Override
public DeletePrivilegesResponse newResponse() {
return new DeletePrivilegesResponse();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
package org.elasticsearch.xpack.core.security.action.privilege;

import org.elasticsearch.action.ActionRequest;
import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.action.support.WriteRequest;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;

import java.io.IOException;

import static org.elasticsearch.action.ValidateActions.addValidationError;

/**
* A request to delete an application privilege.
*/
public final class DeletePrivilegesRequest extends ActionRequest implements WriteRequest<DeletePrivilegesRequest> {

private String application;
private String[] privileges;
private RefreshPolicy refreshPolicy = RefreshPolicy.IMMEDIATE;

public DeletePrivilegesRequest() {
this(null, Strings.EMPTY_ARRAY);
}

public DeletePrivilegesRequest(String application, String[] privileges) {
this.application = application;
this.privileges = privileges;
}

@Override
public DeletePrivilegesRequest setRefreshPolicy(RefreshPolicy refreshPolicy) {
this.refreshPolicy = refreshPolicy;
return this;
}

@Override
public RefreshPolicy getRefreshPolicy() {
return refreshPolicy;
}

@Override
public ActionRequestValidationException validate() {
Copy link
Member

Choose a reason for hiding this comment

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

lets add tests around validation

ActionRequestValidationException validationException = null;
if (Strings.isNullOrEmpty(application)) {
validationException = addValidationError("application name is missing", validationException);
}
if (privileges == null || privileges.length == 0) {
validationException = addValidationError("privileges are missing", validationException);
}
return validationException;
}

public void application(String application) {
this.application = application;
}

public String application() {
return application;
}

public String[] privileges() {
return this.privileges;
}

public void privileges(String[] privileges) {
this.privileges = privileges;
}

@Override
public void readFrom(StreamInput in) throws IOException {
Copy link
Member

Choose a reason for hiding this comment

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

add tests for serialization

super.readFrom(in);
application = in.readString();
privileges = in.readStringArray();
refreshPolicy = RefreshPolicy.readFrom(in);
}

@Override
public void writeTo(StreamOutput out) throws IOException {
super.writeTo(out);
out.writeString(application);
out.writeStringArray(privileges);
refreshPolicy.writeTo(out);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
package org.elasticsearch.xpack.core.security.action.privilege;

import org.elasticsearch.action.ActionRequestBuilder;
import org.elasticsearch.action.support.WriteRequestBuilder;
import org.elasticsearch.client.ElasticsearchClient;

import java.util.Collection;

/**
* Builder for {@link DeletePrivilegesRequest}
*/
public class DeletePrivilegesRequestBuilder
extends ActionRequestBuilder<DeletePrivilegesRequest, DeletePrivilegesResponse, DeletePrivilegesRequestBuilder>
implements WriteRequestBuilder<DeletePrivilegesRequestBuilder> {

public DeletePrivilegesRequestBuilder(ElasticsearchClient client, DeletePrivilegesAction action) {
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need this constructor and can just use the one above and call super with a instance of the request

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately need it in order to implement DeletePrivilegesAction.newRequestBuilder, but I've made it package protected.

super(client, action, new DeletePrivilegesRequest());
}

public DeletePrivilegesRequestBuilder privileges(String[] privileges) {
request.privileges(privileges);
return this;
}

public DeletePrivilegesRequestBuilder application(String applicationName) {
request.application(applicationName);
return this;
}
}
Loading