Skip to content

Commit

Permalink
Support roles with application privileges against wildcard applications
Browse files Browse the repository at this point in the history
This commit introduces 2 changes to application privileges:

- The validation rules now accept a wildcard in the "suffix" of an application name.
  Wildcards were always accepted in the application name, but the "valid filename" check
  for the suffix incorrectly prevented the use of wildcards there.

- A role may now be defined against a wildcard application (e.g. kibana-*) and this will
  be correctly treated as granting the named privileges against all named applications.
  This does not allow wildcard application names in the body of a "has-privileges" check, but the
  "has-privileges" check can test concrete application names against roles with wildcards.

Backport of: elastic#40398
  • Loading branch information
tvernum committed Apr 1, 2019
1 parent e596b33 commit 0fbd63a
Show file tree
Hide file tree
Showing 9 changed files with 302 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -104,15 +104,18 @@ public ResourcePrivilegesMap checkResourcePrivileges(final String applicationNam
for (String checkResource : checkForResources) {
for (String checkPrivilegeName : checkForPrivilegeNames) {
final Set<String> nameSet = Collections.singleton(checkPrivilegeName);
final ApplicationPrivilege checkPrivilege = ApplicationPrivilege.get(applicationName, nameSet, storedPrivileges);
assert checkPrivilege.getApplication().equals(applicationName) : "Privilege " + checkPrivilege + " should have application "
+ applicationName;
assert checkPrivilege.name().equals(nameSet) : "Privilege " + checkPrivilege + " should have name " + nameSet;

if (grants(checkPrivilege, checkResource)) {
resourcePrivilegesMapBuilder.addResourcePrivilege(checkResource, checkPrivilegeName, Boolean.TRUE);
} else {
resourcePrivilegesMapBuilder.addResourcePrivilege(checkResource, checkPrivilegeName, Boolean.FALSE);
final Set<ApplicationPrivilege> checkPrivileges = ApplicationPrivilege.get(applicationName, nameSet, storedPrivileges);
logger.trace("Resolved privileges [{}] for [{},{}]", checkPrivileges, applicationName, nameSet);
for (ApplicationPrivilege checkPrivilege : checkPrivileges) {
assert Automatons.predicate(applicationName).test(checkPrivilege.getApplication()) : "Privilege " + checkPrivilege +
" should have application " + applicationName;
assert checkPrivilege.name().equals(nameSet) : "Privilege " + checkPrivilege + " should have name " + nameSet;

if (grants(checkPrivilege, checkResource)) {
resourcePrivilegesMapBuilder.addResourcePrivilege(checkResource, checkPrivilegeName, Boolean.TRUE);
} else {
resourcePrivilegesMapBuilder.addResourcePrivilege(checkResource, checkPrivilegeName, Boolean.FALSE);
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package org.elasticsearch.xpack.core.security.authz.privilege;

import org.elasticsearch.common.Strings;
import org.elasticsearch.xpack.core.security.support.Automatons;

import java.util.Arrays;
import java.util.Collection;
Expand All @@ -15,6 +16,7 @@
import java.util.Objects;
import java.util.Set;
import java.util.function.Function;
import java.util.function.Predicate;
import java.util.regex.Pattern;
import java.util.stream.Collectors;

Expand Down Expand Up @@ -101,7 +103,7 @@ private static void validateApplicationName(String application, boolean allowWil
if (allowWildcard == false) {
throw new IllegalArgumentException("Application names may not contain '*' (found '" + application + "')");
}
if(application.equals("*")) {
if (application.equals("*")) {
// this is allowed and short-circuiting here makes the later validation simpler
return;
}
Expand All @@ -128,7 +130,10 @@ private static void validateApplicationName(String application, boolean allowWil
}

if (parts.length > 1) {
final String suffix = parts[1];
String suffix = parts[1];
if (allowWildcard && suffix.endsWith("*")) {
suffix = suffix.substring(0, suffix.length() - 1);
}
if (Strings.validFileName(suffix) == false) {
throw new IllegalArgumentException("An application name suffix may not contain any of the characters '" +
Strings.collectionToDelimitedString(Strings.INVALID_FILENAME_CHARS, "") + "' (found '" + suffix + "')");
Expand Down Expand Up @@ -165,20 +170,38 @@ public static void validatePrivilegeOrActionName(String name) {
}

/**
* Finds or creates an application privileges with the provided names.
* Finds or creates a collection of application privileges with the provided names.
* If application is a wildcard, it will be expanded to all matching application names in {@code stored}
* Each element in {@code name} may be the name of a stored privilege (to be resolved from {@code stored}, or a bespoke action pattern.
*/
public static ApplicationPrivilege get(String application, Set<String> name, Collection<ApplicationPrivilegeDescriptor> stored) {
public static Set<ApplicationPrivilege> get(String application, Set<String> name, Collection<ApplicationPrivilegeDescriptor> stored) {
if (name.isEmpty()) {
return NONE.apply(application);
return Collections.singleton(NONE.apply(application));
} else if (application.contains("*")) {
Predicate<String> predicate = Automatons.predicate(application);
final Set<ApplicationPrivilege> result = stored.stream()
.map(ApplicationPrivilegeDescriptor::getApplication)
.filter(predicate)
.distinct()
.map(appName -> resolve(appName, name, stored))
.collect(Collectors.toSet());
if (result.isEmpty()) {
return Collections.singleton(resolve(application, name, Collections.emptyMap()));
} else {
return result;
}
} else {
Map<String, ApplicationPrivilegeDescriptor> lookup = stored.stream()
.filter(apd -> apd.getApplication().equals(application))
.collect(Collectors.toMap(ApplicationPrivilegeDescriptor::getName, Function.identity()));
return resolve(application, name, lookup);
return Collections.singleton(resolve(application, name, stored));
}
}

private static ApplicationPrivilege resolve(String application, Set<String> name, Collection<ApplicationPrivilegeDescriptor> stored) {
final Map<String, ApplicationPrivilegeDescriptor> lookup = stored.stream()
.filter(apd -> apd.getApplication().equals(application))
.collect(Collectors.toMap(ApplicationPrivilegeDescriptor::getName, Function.identity()));
return resolve(application, name, lookup);
}

private static ApplicationPrivilege resolve(String application, Set<String> names, Map<String, ApplicationPrivilegeDescriptor> lookup) {
final int size = names.size();
if (size == 0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
import java.util.Objects;
import java.util.Set;

import static org.elasticsearch.common.Strings.collectionToCommaDelimitedString;

/**
* An {@code ApplicationPrivilegeDescriptor} is a representation of a <em>stored</em> {@link ApplicationPrivilege}.
* A user (via a role) can be granted an application privilege by name (e.g. ("myapp", "read").
Expand Down Expand Up @@ -104,6 +106,11 @@ public XContentBuilder toXContent(XContentBuilder builder, boolean includeTypeFi
return builder.endObject();
}

@Override
public String toString() {
return getClass().getSimpleName() + "{[" + application + "],[" + name + "],[" + collectionToCommaDelimitedString(actions) + "]}";
}

/**
* Construct a new {@link ApplicationPrivilegeDescriptor} from XContent.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,16 @@

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import static java.util.Collections.singletonList;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasSize;

public class ApplicationPermissionTests extends ESTestCase {

Expand All @@ -34,6 +35,7 @@ public class ApplicationPermissionTests extends ESTestCase {
private ApplicationPrivilege app1Delete = storePrivilege("app1", "delete", "write/delete");
private ApplicationPrivilege app1Create = storePrivilege("app1", "create", "write/create");
private ApplicationPrivilege app2Read = storePrivilege("app2", "read", "read/*");
private ApplicationPrivilege otherAppRead = storePrivilege("other-app", "read", "read/*");

private ApplicationPrivilege storePrivilege(String app, String name, String... patterns) {
store.add(new ApplicationPrivilegeDescriptor(app, name, Sets.newHashSet(patterns), Collections.emptyMap()));
Expand Down Expand Up @@ -104,6 +106,16 @@ public void testDoesNotMatchAcrossApplications() {
assertThat(buildPermission(app1All, "*").grants(app2Read, "123"), equalTo(false));
}

public void testMatchingWithWildcardApplicationNames() {
final Set<ApplicationPrivilege> readAllApp = ApplicationPrivilege.get("app*", Collections.singleton("read"), store);
assertThat(buildPermission(readAllApp, "*").grants(app1Read, "123"), equalTo(true));
assertThat(buildPermission(readAllApp, "foo/*").grants(app2Read, "foo/bar"), equalTo(true));

assertThat(buildPermission(readAllApp, "*").grants(app1Write, "123"), equalTo(false));
assertThat(buildPermission(readAllApp, "foo/*").grants(app2Read, "bar/baz"), equalTo(false));
assertThat(buildPermission(readAllApp, "*").grants(otherAppRead, "abc"), equalTo(false));
}

public void testMergedPermissionChecking() {
final ApplicationPrivilege app1ReadWrite = compositePrivilege("app1", app1Read, app1Write);
final ApplicationPermission hasPermission = buildPermission(app1ReadWrite, "allow/*");
Expand Down Expand Up @@ -138,16 +150,27 @@ public void testInspectPermissionContents() {
}

private ApplicationPrivilege actionPrivilege(String appName, String... actions) {
return ApplicationPrivilege.get(appName, Sets.newHashSet(actions), Collections.emptyList());
final Set<ApplicationPrivilege> privileges = ApplicationPrivilege.get(appName, Sets.newHashSet(actions), Collections.emptyList());
assertThat(privileges, hasSize(1));
return privileges.iterator().next();
}

private ApplicationPrivilege compositePrivilege(String application, ApplicationPrivilege... children) {
Set<String> names = Stream.of(children).map(ApplicationPrivilege::name).flatMap(Set::stream).collect(Collectors.toSet());
return ApplicationPrivilege.get(application, names, store);
final Set<ApplicationPrivilege> privileges = ApplicationPrivilege.get(application, names, store);
assertThat(privileges, hasSize(1));
return privileges.iterator().next();
}


private ApplicationPermission buildPermission(ApplicationPrivilege privilege, String... resources) {
return new ApplicationPermission(singletonList(new Tuple<>(privilege, Sets.newHashSet(resources))));
return buildPermission(Collections.singleton(privilege), resources);
}

private ApplicationPermission buildPermission(Collection<ApplicationPrivilege> privileges, String... resources) {
final Set<String> resourceSet = Sets.newHashSet(resources);
final List<Tuple<ApplicationPrivilege, Set<String>>> privilegesAndResources = privileges.stream()
.map(p -> new Tuple<>(p, resourceSet))
.collect(Collectors.toList());
return new ApplicationPermission(privilegesAndResources);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import org.elasticsearch.common.util.set.Sets;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.EqualsHashCodeTestUtils;
import org.hamcrest.Matchers;
import org.junit.Assert;

import java.util.Arrays;
Expand All @@ -22,9 +23,11 @@

import static org.elasticsearch.common.Strings.collectionToCommaDelimitedString;
import static org.hamcrest.Matchers.arrayContainingInAnyOrder;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.iterableWithSize;

public class ApplicationPrivilegeTests extends ESTestCase {

Expand Down Expand Up @@ -59,6 +62,12 @@ public void testValidationOfApplicationName() {
assertNoException(app, () -> ApplicationPrivilege.validateApplicationName(app));
assertNoException(app, () -> ApplicationPrivilege.validateApplicationNameOrWildcard(app));
}

// wildcards in the suffix
for (String app : Arrays.asList("app1-*", "app1-foo*", "app1-.*", "app1-.foo.*", appNameWithSpecialChars + "*")) {
assertValidationFailure(app, "application name", () -> ApplicationPrivilege.validateApplicationName(app));
assertNoException(app, () -> ApplicationPrivilege.validateApplicationNameOrWildcard(app));
}
}

public void testValidationOfPrivilegeName() {
Expand Down Expand Up @@ -101,16 +110,23 @@ public void testNonePrivilege() {
}

public void testGetPrivilegeByName() {
final ApplicationPrivilegeDescriptor descriptor = descriptor("my-app", "read", "data:read/*", "action:login");
final ApplicationPrivilegeDescriptor myRead = descriptor("my-app", "read", "data:read/*", "action:login");
final ApplicationPrivilegeDescriptor myWrite = descriptor("my-app", "write", "data:write/*", "action:login");
final ApplicationPrivilegeDescriptor myAdmin = descriptor("my-app", "admin", "data:read/*", "action:*");
final ApplicationPrivilegeDescriptor yourRead = descriptor("your-app", "read", "data:read/*", "action:login");
final Set<ApplicationPrivilegeDescriptor> stored = Sets.newHashSet(descriptor, myWrite, myAdmin, yourRead);
final Set<ApplicationPrivilegeDescriptor> stored = Sets.newHashSet(myRead, myWrite, myAdmin, yourRead);

final Set<ApplicationPrivilege> myAppRead = ApplicationPrivilege.get("my-app", Collections.singleton("read"), stored);
assertThat(myAppRead, iterableWithSize(1));
assertPrivilegeEquals(myAppRead.iterator().next(), myRead);

assertEqual(ApplicationPrivilege.get("my-app", Collections.singleton("read"), stored), descriptor);
assertEqual(ApplicationPrivilege.get("my-app", Collections.singleton("write"), stored), myWrite);
final Set<ApplicationPrivilege> myAppWrite = ApplicationPrivilege.get("my-app", Collections.singleton("write"), stored);
assertThat(myAppWrite, iterableWithSize(1));
assertPrivilegeEquals(myAppWrite.iterator().next(), myWrite);

final ApplicationPrivilege readWrite = ApplicationPrivilege.get("my-app", Sets.newHashSet("read", "write"), stored);
final Set<ApplicationPrivilege> myReadWrite = ApplicationPrivilege.get("my-app", Sets.newHashSet("read", "write"), stored);
assertThat(myReadWrite, Matchers.hasSize(1));
final ApplicationPrivilege readWrite = myReadWrite.iterator().next();
assertThat(readWrite.getApplication(), equalTo("my-app"));
assertThat(readWrite.name(), containsInAnyOrder("read", "write"));
assertThat(readWrite.getPatterns(), arrayContainingInAnyOrder("data:read/*", "data:write/*", "action:login"));
Expand All @@ -124,10 +140,10 @@ public void testGetPrivilegeByName() {
}
}

private void assertEqual(ApplicationPrivilege myReadPriv, ApplicationPrivilegeDescriptor myRead) {
assertThat(myReadPriv.getApplication(), equalTo(myRead.getApplication()));
assertThat(getPrivilegeName(myReadPriv), equalTo(myRead.getName()));
assertThat(Sets.newHashSet(myReadPriv.getPatterns()), equalTo(myRead.getActions()));
private void assertPrivilegeEquals(ApplicationPrivilege privilege, ApplicationPrivilegeDescriptor descriptor) {
assertThat(privilege.getApplication(), equalTo(descriptor.getApplication()));
assertThat(privilege.name(), contains(descriptor.getName()));
assertThat(Sets.newHashSet(privilege.getPatterns()), equalTo(descriptor.getActions()));
}

private ApplicationPrivilegeDescriptor descriptor(String application, String name, String... actions) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -402,8 +402,8 @@ public static void buildRoleFromDescriptors(Collection<RoleDescriptor> roleDescr
.flatMap(Collection::stream)
.collect(Collectors.toSet());
privilegeStore.getPrivileges(applicationNames, applicationPrivilegeNames, ActionListener.wrap(appPrivileges -> {
applicationPrivilegesMap.forEach((key, names) ->
builder.addApplicationPrivilege(ApplicationPrivilege.get(key.v1(), names, appPrivileges), key.v2()));
applicationPrivilegesMap.forEach((key, names) -> ApplicationPrivilege.get(key.v1(), names, appPrivileges)
.forEach(priv -> builder.addApplicationPrivilege(priv, key.v2())));
listener.onResponse(builder.build());
}, listener::onFailure));
}
Expand Down
Loading

0 comments on commit 0fbd63a

Please sign in to comment.