Skip to content

Commit

Permalink
buildFromRoleDescriptor uses application privilege look up (#91152)
Browse files Browse the repository at this point in the history
Building a role from a single role descriptor previously only worked
correctly for application privileges with wildcard patterns. This PR
adds support for concrete name look up and ports the relevant tests.

Relates: #91107 (comment)
  • Loading branch information
n1v0lg authored Oct 28, 2022
1 parent 98aee60 commit 67204a9
Show file tree
Hide file tree
Showing 5 changed files with 292 additions and 48 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,15 @@ static SimpleRole buildFromRoleDescriptor(
final RoleDescriptor roleDescriptor,
final FieldPermissionsCache fieldPermissionsCache,
final RestrictedIndices restrictedIndices
) {
return buildFromRoleDescriptor(roleDescriptor, fieldPermissionsCache, restrictedIndices, List.of());
}

static SimpleRole buildFromRoleDescriptor(
final RoleDescriptor roleDescriptor,
final FieldPermissionsCache fieldPermissionsCache,
final RestrictedIndices restrictedIndices,
final Collection<ApplicationPrivilegeDescriptor> storedApplicationPrivilegeDescriptors
) {
// TODO handle this when we introduce remote index privileges for built-in users and roles. That's the only production code
// using this builder
Expand All @@ -344,14 +353,11 @@ static SimpleRole buildFromRoleDescriptor(
}

for (RoleDescriptor.ApplicationResourcePrivileges applicationPrivilege : roleDescriptor.getApplicationPrivileges()) {
builder.addApplicationPrivilege(
new ApplicationPrivilege(
applicationPrivilege.getApplication(),
Sets.newHashSet(applicationPrivilege.getPrivileges()),
applicationPrivilege.getPrivileges()
),
Sets.newHashSet(applicationPrivilege.getResources())
);
ApplicationPrivilege.get(
applicationPrivilege.getApplication(),
Sets.newHashSet(applicationPrivilege.getPrivileges()),
storedApplicationPrivilegeDescriptors
).forEach(priv -> builder.addApplicationPrivilege(priv, Sets.newHashSet(applicationPrivilege.getResources())));
}

final String[] rdRunAs = roleDescriptor.getRunAs();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,19 @@
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xpack.core.security.authz.AuthorizationEngine;
import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilege;
import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilegeDescriptor;

import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ExecutionException;

import static org.elasticsearch.xpack.core.security.test.TestRestrictedIndices.RESTRICTED_INDICES;
import static org.hamcrest.Matchers.emptyArray;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;

Expand Down Expand Up @@ -56,4 +62,84 @@ public void testHasPrivilegesCache() throws ExecutionException {
assertThat(role.checkPrivilegesWithCache(privilegesToCheck), equalTo(privilegesCheckResult));
}

public void testBuildFromRoleDescriptorWithApplicationPrivileges() {
final boolean wildcardApplication = randomBoolean();
final boolean wildcardPrivileges = randomBoolean();
final boolean wildcardResources = randomBoolean();
final RoleDescriptor.ApplicationResourcePrivileges applicationPrivilege = RoleDescriptor.ApplicationResourcePrivileges.builder()
.application(wildcardApplication ? "*" : randomAlphaOfLengthBetween(5, 12))
// concrete privileges need to be prefixed with lower case letters to be considered valid, so use "app"
.privileges(wildcardPrivileges ? "*" : "app" + randomAlphaOfLengthBetween(5, 12))
.resources(wildcardResources ? new String[] { "*" } : generateRandomStringArray(6, randomIntBetween(4, 8), false, false))
.build();

final String allowedApplicationActionPattern = randomAlphaOfLengthBetween(5, 12);
final SimpleRole role = Role.buildFromRoleDescriptor(
new RoleDescriptor(
"r1",
null,
null,
new RoleDescriptor.ApplicationResourcePrivileges[] { applicationPrivilege },
null,
null,
null,
null
),
new FieldPermissionsCache(Settings.EMPTY),
RESTRICTED_INDICES,
wildcardPrivileges
? List.of()
: List.of(
new ApplicationPrivilegeDescriptor(
applicationPrivilege.getApplication(),
Arrays.stream(applicationPrivilege.getPrivileges()).iterator().next(),
Set.of(allowedApplicationActionPattern),
Map.of()
)
)
);
assertThat(
"expected grant for role with application privilege to be: " + applicationPrivilege,
role.application()
.grants(
new ApplicationPrivilege(
wildcardApplication ? randomAlphaOfLengthBetween(1, 10) : applicationPrivilege.getApplication(),
wildcardPrivileges ? Set.of(randomAlphaOfLengthBetween(1, 10)) : Set.of(applicationPrivilege.getPrivileges()),
wildcardPrivileges ? randomAlphaOfLengthBetween(1, 10) : allowedApplicationActionPattern
),
wildcardResources ? randomAlphaOfLengthBetween(1, 10) : randomFrom(applicationPrivilege.getResources())
),
is(true)
);
// This gives decent but not complete coverage of denial cases; for any non-wildcard field we pick a mismatched value to force a
// denial
assertThat(
"expected grant for role with application privilege to be: " + applicationPrivilege,
role.application()
.grants(
new ApplicationPrivilege(
false == wildcardApplication
? randomValueOtherThan(applicationPrivilege.getApplication(), () -> randomAlphaOfLengthBetween(1, 10))
: randomAlphaOfLengthBetween(1, 10),
false == wildcardPrivileges
? randomValueOtherThan(
Set.of(applicationPrivilege.getPrivileges()),
() -> Set.of(randomAlphaOfLengthBetween(1, 10))
)
: Set.of(randomAlphaOfLengthBetween(1, 10)),
false == wildcardPrivileges
? randomValueOtherThan(allowedApplicationActionPattern, () -> randomAlphaOfLengthBetween(1, 10))
: randomAlphaOfLengthBetween(1, 10)
),
false == wildcardResources
? randomValueOtherThanMany(
it -> List.of(applicationPrivilege.getResources()).contains(it),
() -> randomAlphaOfLengthBetween(1, 10)
)
: randomAlphaOfLengthBetween(1, 10)
),
// If all are wildcards, then we necessarily get a grant, otherwise expect a denial
is(wildcardApplication && wildcardPrivileges && wildcardResources)
);
}
}
Loading

0 comments on commit 67204a9

Please sign in to comment.