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

Support roles with application privileges against wildcard applications #40398

Merged
merged 6 commits into from
Mar 29, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@ -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