shield: handle merging granted and non-granted indices acls

This commit changes the handling in the merge method of the IndexAccessControl class to
properly handle merging IndexAccessControl objects with differing values for the granted
flag. Prior to this commit, in a scenario where the flag differed, one IndexAccessControl granted
no access to an index, and the other granted access with DLS/FLS resulted in full access
being granted to the index.

Closes elastic/elasticsearch#1821

Original commit: elastic/x-pack-elasticsearch@e403e43689
This commit is contained in:
jaymode 2016-03-28 11:38:32 -04:00
parent 77e6622179
commit 2550548a44
7 changed files with 131 additions and 36 deletions

View File

@ -87,10 +87,18 @@ public class IndicesAccessControl {
}
public IndexAccessControl merge(IndexAccessControl other) {
boolean granted = this.granted;
if (!granted) {
granted = other.isGranted();
if (other.isGranted() == false) {
// nothing to merge
return this;
}
final boolean granted = this.granted;
if (granted == false) {
// we do not support negatives, so if the current isn't granted - just return other
assert other.isGranted();
return other;
}
// this code is a bit of a pita, but right now we can't just initialize an empty set,
// because an empty Set means no permissions on fields and
// <code>null</code> means no field level security

View File

@ -43,29 +43,36 @@ public class DocumentAndFieldLevelSecurityTests extends ShieldIntegTestCase {
@Override
protected String configUsersRoles() {
return super.configUsersRoles() +
"role1:user1,user4\n" +
"role2:user2,user4\n" +
"role3:user3,user4\n";
"role1:user1\n" +
"role2:user1,user4\n" +
"role3:user2,user4\n" +
"role4:user3,user4\n";
}
@Override
protected String configRoles() {
return super.configRoles() +
"\nrole1:\n" +
" cluster: [ all ]\n" +
" cluster: [ none ]\n" +
" indices:\n" +
" - names: '*'\n" +
" privileges: [ none ]\n" +
"role2:\n" +
" cluster:\n" +
" - all\n" +
" indices:\n" +
" - names: '*'\n" +
" privileges: [ ALL ]\n" +
" fields: [ field1 ]\n" +
" query: '{\"term\" : {\"field1\" : \"value1\"}}'\n" +
"role2:\n" +
"role3:\n" +
" cluster: [ all ]\n" +
" indices:\n" +
" - names: '*'\n" +
" privileges: [ ALL ]\n" +
" fields: [ field2 ]\n" +
" query: '{\"term\" : {\"field2\" : \"value2\"}}'\n" +
"role3:\n" +
"role4:\n" +
" cluster: [ all ]\n" +
" indices:\n" +
" - names: '*'\n" +

View File

@ -48,6 +48,14 @@ public class DocumentLevelSecurityRandomTests extends ShieldIntegTestCase {
@Override
protected String configUsersRoles() {
StringBuilder builder = new StringBuilder(super.configUsersRoles());
builder.append("role0:");
for (int i = 1; i <= numberOfRoles; i++) {
builder.append("user").append(i);
if (i != numberOfRoles) {
builder.append(",");
}
}
builder.append("\n");
for (int i = 1; i <= numberOfRoles; i++) {
builder.append("role").append(i).append(":user").append(i).append('\n');
}
@ -57,7 +65,11 @@ public class DocumentLevelSecurityRandomTests extends ShieldIntegTestCase {
@Override
protected String configRoles() {
StringBuilder builder = new StringBuilder(super.configRoles());
builder.append('\n');
builder.append("\nrole0:\n");
builder.append(" cluster: [ none ]\n");
builder.append(" indices:\n");
builder.append(" - names: '*'\n");
builder.append(" privileges: [ none ]\n");
for (int i = 1; i <= numberOfRoles; i++) {
builder.append("role").append(i).append(":\n");
builder.append(" cluster: [ all ]\n");

View File

@ -65,14 +65,20 @@ public class DocumentLevelSecurityTests extends ShieldIntegTestCase {
@Override
protected String configUsersRoles() {
return super.configUsersRoles() +
"role1:user1,user3\n" +
"role2:user2,user3\n";
"role1:user1,user2,user3\n" +
"role2:user1,user3\n" +
"role3:user2,user3\n";
}
@Override
protected String configRoles() {
return super.configRoles() +
"\nrole1:\n" +
" cluster: [ none ]\n" +
" indices:\n" +
" - names: '*'\n" +
" privileges: [ none ]\n" +
"\nrole2:\n" +
" cluster:\n" +
" - all\n" +
" indices:\n" +
@ -82,7 +88,7 @@ public class DocumentLevelSecurityTests extends ShieldIntegTestCase {
" query: \n" +
" term: \n" +
" field1: value1\n" +
"role2:\n" +
"role3:\n" +
" cluster: [ all ]\n" +
" indices:\n" +
" - names: '*'\n" +

View File

@ -51,10 +51,11 @@ public class FieldLevelSecurityRandomTests extends ShieldIntegTestCase {
@Override
protected String configUsersRoles() {
return super.configUsersRoles() +
"role1:user1\n" +
"role2:user2\n" +
"role3:user3\n" +
"role4:user4\n";
"role1:user1,user2,user3,user4\n" +
"role2:user1\n" +
"role3:user2\n" +
"role4:user3\n" +
"role5:user4\n";
}
@Override
@ -80,12 +81,17 @@ public class FieldLevelSecurityRandomTests extends ShieldIntegTestCase {
return super.configRoles() +
"\nrole1:\n" +
" cluster: [ none ]\n" +
" indices:\n" +
" - names: '*'\n" +
" privileges: [ none ]\n" +
"\nrole2:\n" +
" cluster: [ all ]\n" +
" indices:\n" +
" - names: '*'\n" +
" privileges: [ ALL ]\n" +
" fields:\n" +roleFields.toString() +
"role2:\n" +
"role3:\n" +
" cluster:\n" +
" - all\n" +
" indices:\n" +
@ -94,14 +100,14 @@ public class FieldLevelSecurityRandomTests extends ShieldIntegTestCase {
" - all\n" +
" fields:\n" +
" - field1\n" +
"role3:\n" +
"role4:\n" +
" cluster: [ all ]\n" +
" indices:\n" +
" - names: test\n" +
" privileges: [ ALL ]\n" +
" fields:\n" +
" - field2\n" +
"role4:\n" +
"role5:\n" +
" cluster: [ all ]\n" +
" indices:\n" +
" - names: test\n" +

View File

@ -69,48 +69,54 @@ public class FieldLevelSecurityTests extends ShieldIntegTestCase {
@Override
protected String configUsersRoles() {
return super.configUsersRoles() +
"role1:user1,user7,user8\n" +
"role2:user2,user7,user8\n" +
"role3:user3,user7\n" +
"role4:user4,user7\n" +
"role5:user5,user7\n" +
"role6:user6\n";
"role1:user1\n" +
"role2:user1,user7,user8\n" +
"role3:user2,user7,user8\n" +
"role4:user3,user7\n" +
"role5:user4,user7\n" +
"role6:user5,user7\n" +
"role7:user6";
}
@Override
protected String configRoles() {
return super.configRoles() +
"\nrole1:\n" +
" cluster: [ all ]\n" +
" cluster: [ none ]\n" +
" indices:\n" +
" - names: '*'\n" +
" privileges: [ ALL ]\n" +
" fields: [ field1 ]\n" +
" - names: '*'\n" +
" privileges: [ none ]\n" +
"role2:\n" +
" cluster: [ all ]\n" +
" indices:\n" +
" - names: '*'\n" +
" privileges: [ ALL ]\n" +
" fields: [ field2 ]\n" +
" fields: [ field1 ]\n" +
"role3:\n" +
" cluster: [ all ]\n" +
" indices:\n" +
" - names: '*'\n" +
" privileges: [ ALL ]\n" +
" fields: [ field2 ]\n" +
"role4:\n" +
" cluster: [ all ]\n" +
" indices:\n" +
" - names: '*'\n" +
" privileges: [ ALL ]\n" +
" fields:\n" +
" - field1\n" +
" - field2\n" +
"role4:\n" +
"role5:\n" +
" cluster: [ all ]\n" +
" indices:\n" +
" - names: '*'\n" +
" privileges: [ ALL ]\n" +
" fields: []\n" +
"role5:\n" +
"role6:\n" +
" cluster: [ all ]\n" +
" indices:\n" +
" - names: '*'\n" +
" privileges: [ALL]\n" +
"role6:\n" +
"role7:\n" +
" cluster: [ all ]\n" +
" indices:\n" +
" - names: '*'\n" +

View File

@ -9,11 +9,13 @@ import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.util.set.Sets;
import org.elasticsearch.test.ESTestCase;
import java.util.Collections;
import org.elasticsearch.shield.authz.accesscontrol.IndicesAccessControl.IndexAccessControl;
import java.util.Collections;
import java.util.Set;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.nullValue;
@ -105,4 +107,52 @@ public class IndicesAccessControlTests extends ESTestCase {
assertThat(merge2.isGranted(), is(true));
assertThat(merge1.getQueries(), nullValue());
}
public void testMergeNotGrantedAndGranted() {
final Set<String> notGrantedFields = randomFrom(null, Collections.emptySet(), Collections.singleton("baz"));
final Set<BytesReference> notGrantedQueries = randomFrom(null, Collections.emptySet(),
Collections.singleton(new BytesArray(new byte[] { randomByte() })));
final IndexAccessControl indexAccessControl = new IndexAccessControl(false, notGrantedFields, notGrantedQueries);
final BytesReference query1 = new BytesArray(new byte[] { 0x1 });
final Set<String> fields =
randomFrom(null, Collections.singleton("foo"), Sets.newHashSet("foo", "bar"), Collections.emptySet());
final Set<BytesReference> queries =
randomFrom(null, Collections.singleton(query1), Collections.emptySet());
final IndexAccessControl other = new IndexAccessControl(true, fields, queries);
IndexAccessControl merged = indexAccessControl.merge(other);
assertThat(merged.isGranted(), is(true));
assertThat(merged.getFields(), equalTo(fields));
assertThat(merged.getQueries(), equalTo(queries));
merged = other.merge(indexAccessControl);
assertThat(merged.isGranted(), is(true));
assertThat(merged.getFields(), equalTo(fields));
assertThat(merged.getQueries(), equalTo(queries));
}
public void testMergeNotGranted() {
final Set<String> notGrantedFields = randomFrom(null, Collections.emptySet(), Collections.singleton("baz"));
final Set<BytesReference> notGrantedQueries = randomFrom(null, Collections.emptySet(),
Collections.singleton(new BytesArray(new byte[] { randomByte() })));
final IndexAccessControl indexAccessControl = new IndexAccessControl(false, notGrantedFields, notGrantedQueries);
final BytesReference query1 = new BytesArray(new byte[] { 0x1 });
final Set<String> fields =
randomFrom(null, Collections.singleton("foo"), Sets.newHashSet("foo", "bar"), Collections.emptySet());
final Set<BytesReference> queries =
randomFrom(null, Collections.singleton(query1), Collections.emptySet());
final IndexAccessControl other = new IndexAccessControl(false, fields, queries);
IndexAccessControl merged = indexAccessControl.merge(other);
assertThat(merged.isGranted(), is(false));
assertThat(merged.getFields(), equalTo(notGrantedFields));
assertThat(merged.getQueries(), equalTo(notGrantedQueries));
merged = other.merge(indexAccessControl);
assertThat(merged.isGranted(), is(false));
assertThat(merged.getFields(), equalTo(fields));
assertThat(merged.getQueries(), equalTo(queries));
}
}