Empty list of privileges should be skipped

When a role is configured with an entry with an empty list of privileges (cluster or indices), the entry should be skipped.

Fixes elastic/elasticsearch#339

Original commit: elastic/x-pack-elasticsearch@44d957529e
This commit is contained in:
uboness 2014-11-19 12:35:02 +01:00
parent f004275641
commit 8783abcc6f
3 changed files with 44 additions and 18 deletions

View File

@ -8,7 +8,6 @@ package org.elasticsearch.shield.authz.store;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.collect.ImmutableMap;
import org.elasticsearch.common.collect.ImmutableSet;
import org.elasticsearch.common.component.AbstractComponent;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.jackson.dataformat.yaml.snakeyaml.error.YAMLException;
@ -105,34 +104,44 @@ public class FileRolesStore extends AbstractComponent implements RolesStore {
if (token == XContentParser.Token.FIELD_NAME) {
currentFieldName = parser.currentName();
} else if ("cluster".equals(currentFieldName)) {
Privilege.Name name;
Privilege.Name name = null;
if (token == XContentParser.Token.VALUE_STRING) {
String[] names = COMMA_DELIM.split(parser.text().trim());
name = new Privilege.Name(names);
String namesStr = parser.text().trim();
if (Strings.hasLength(namesStr)) {
String[] names = COMMA_DELIM.split(namesStr);
name = new Privilege.Name(names);
}
} else if (token == XContentParser.Token.START_ARRAY) {
ImmutableSet.Builder<String> names = ImmutableSet.builder();
Set<String> names = new HashSet<>();
while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) {
if (token == XContentParser.Token.VALUE_STRING) {
names.add(parser.text());
}
}
name = new Privilege.Name(names.build());
if (!names.isEmpty()) {
name = new Privilege.Name(names);
}
} else {
throw new ElasticsearchException("Invalid roles file format [" + path.toAbsolutePath() +
"]. [cluster] field value can either be a string or a list of strings, but [" + token + "] was found instead in role [" + roleName + "]");
}
permission.set(Privilege.Cluster.get(name));
if (name != null) {
permission.set(Privilege.Cluster.get(name));
}
} else if ("indices".equals(currentFieldName)) {
if (token == XContentParser.Token.START_OBJECT) {
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
if (token == XContentParser.Token.FIELD_NAME) {
currentFieldName = parser.currentName();
} else {
} else if (Strings.hasLength(currentFieldName)) {
String[] indices = COMMA_DELIM.split(currentFieldName);
Privilege.Name name;
Privilege.Name name = null;
if (token == XContentParser.Token.VALUE_STRING) {
String[] names = COMMA_DELIM.split(parser.text());
name = new Privilege.Name(names);
String namesStr = parser.text().trim();
if (Strings.hasLength(namesStr)) {
String[] names = COMMA_DELIM.split(parser.text());
name = new Privilege.Name(names);
}
} else if (token == XContentParser.Token.START_ARRAY) {
Set<String> names = new HashSet<>();
while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) {
@ -143,13 +152,17 @@ public class FileRolesStore extends AbstractComponent implements RolesStore {
"]. Could not parse [" + token + "] as index privilege in role[" + roleName + "]. Privilege names must be strings");
}
}
name = new Privilege.Name(names);
if (!names.isEmpty()) {
name = new Privilege.Name(names);
}
} else {
throw new ElasticsearchException("Invalid roles file format [" + path.toAbsolutePath() +
"]. Could not parse [" + token + "] as index privileges list in role [" + roleName + "]. Privilege lists must either " +
"be a comma delimited string or an array of strings");
}
permission.add(Privilege.Index.get(name), indices);
if (name != null) {
permission.add(Privilege.Index.get(name), indices);
}
}
}
} else {

View File

@ -41,7 +41,7 @@ public class FileRolesStoreTests extends ElasticsearchTestCase {
Path path = Paths.get(getClass().getResource("roles.yml").toURI());
Map<String, Permission.Global.Role> roles = FileRolesStore.parseFile(path, logger, mock(AuthorizationService.class));
assertThat(roles, notNullValue());
assertThat(roles.size(), is(3));
assertThat(roles.size(), is(4));
Permission.Global.Role role = roles.get("role1");
assertThat(role, notNullValue());
@ -90,6 +90,13 @@ public class FileRolesStoreTests extends ElasticsearchTestCase {
assertThat(group.indices()[0], equalTo("/.*_.*/"));
assertThat(group.privilege(), notNullValue());
assertThat(group.privilege().isAlias(Privilege.Index.union(Privilege.Index.READ, Privilege.Index.WRITE)), is(true));
role = roles.get("role4");
assertThat(role, notNullValue());
assertThat(role.name(), equalTo("role4"));
assertThat(role.cluster(), notNullValue());
assertThat(role.cluster(), is(Permission.Cluster.Core.NONE));
assertThat(role.indices(), is(Permission.Indices.Core.NONE));
}
/**
@ -139,7 +146,7 @@ public class FileRolesStoreTests extends ElasticsearchTestCase {
Permission.Global.Role role = store.role("role1");
assertThat(role, notNullValue());
role = store.role("role4");
role = store.role("role5");
assertThat(role, nullValue());
watcherService.start();
@ -148,7 +155,7 @@ public class FileRolesStoreTests extends ElasticsearchTestCase {
writer.newLine();
writer.newLine();
writer.newLine();
writer.append("role4:").append(System.lineSeparator());
writer.append("role5:").append(System.lineSeparator());
writer.append(" cluster: 'MONITOR'");
}
@ -156,9 +163,9 @@ public class FileRolesStoreTests extends ElasticsearchTestCase {
fail("Waited too long for the updated file to be picked up");
}
role = store.role("role4");
role = store.role("role5");
assertThat(role, notNullValue());
assertThat(role.name(), equalTo("role4"));
assertThat(role.name(), equalTo("role5"));
assertThat(role.cluster().check("cluster:monitor/foo/bar"), is(true));
assertThat(role.cluster().check("cluster:admin/foo/bar"), is(false));

View File

@ -11,3 +11,9 @@ role3:
indices:
'/.*_.*/': READ, WRITE
role4:
cluster:
indices:
'idx2':
'': READ
'idx1': []