Skip to content

Commit

Permalink
Ensure authz role for API key is named after owner role (#59041)
Browse files Browse the repository at this point in the history 
The composite role that is used for authz, following the authn with an API key,
is an intersection of the privileges from the owner role and the key privileges defined
when the key has been created.
This change ensures that the `#names` property of such a role equals the `#names`
property of the key owner role, thereby rectifying the value for the `user.roles`
audit event field.
  • Loading branch information
@albertzaharovits
albertzaharovits authored Jul 7, 2020
1 parent 2bf44f5 commit d40ab54
Show file tree
Hide file tree
Showing 6 changed files with 128 additions and 46 deletions.
16 changes: 5 additions & 11 deletions ...ore/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/LimitedRole.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,12 @@
public final class LimitedRole extends Role {
private final Role limitedBy;

LimitedRole(String[] names, ClusterPermission cluster, IndicesPermission indices, ApplicationPermission application,
RunAsPermission runAs, Role limitedBy) {
super(names, cluster, indices, application, runAs);
assert limitedBy != null : "limiting role is required";
LimitedRole(ClusterPermission cluster, IndicesPermission indices, ApplicationPermission application, RunAsPermission runAs,
Role limitedBy) {
super(Objects.requireNonNull(limitedBy, "limiting role is required").names(), cluster, indices, application, runAs);
this.limitedBy = limitedBy;
}

public Role limitedBy() {
return limitedBy;
}

@Override
public ClusterPermission cluster() {
throw new UnsupportedOperationException("cannot retrieve cluster permission on limited role");
Expand Down Expand Up @@ -86,7 +81,7 @@ public Predicate<String> allowedIndicesMatcher(String action) {
@Override
public Automaton allowedActionsMatcher(String index) {
final Automaton allowedMatcher = super.allowedActionsMatcher(index);
final Automaton limitedByMatcher = super.allowedActionsMatcher(index);
final Automaton limitedByMatcher = limitedBy.allowedActionsMatcher(index);
return Automatons.intersectAndMinimize(allowedMatcher, limitedByMatcher);
}

Expand Down Expand Up @@ -187,7 +182,6 @@ public boolean checkRunAs(String runAs) {
*/
public static LimitedRole createLimitedRole(Role fromRole, Role limitedByRole) {
Objects.requireNonNull(limitedByRole, "limited by role is required to create limited role");
return new LimitedRole(fromRole.names(), fromRole.cluster(), fromRole.indices(), fromRole.application(), fromRole.runAs(),
limitedByRole);
return new LimitedRole(fromRole.cluster(), fromRole.indices(), fromRole.application(), fromRole.runAs(), limitedByRole);
}
}
41 changes: 41 additions & 0 deletions ...rc/test/java/org/elasticsearch/xpack/core/security/authz/permission/LimitedRoleTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@

package org.elasticsearch.xpack.core.security.authz.permission;

import org.apache.lucene.util.automaton.Automaton;
import org.elasticsearch.Version;
import org.elasticsearch.action.admin.indices.create.CreateIndexAction;
import org.elasticsearch.action.admin.indices.delete.DeleteIndexAction;
import org.elasticsearch.action.bulk.BulkAction;
import org.elasticsearch.action.search.SearchAction;
import org.elasticsearch.cluster.metadata.AliasMetadata;
import org.elasticsearch.cluster.metadata.IndexMetadata;
Expand All @@ -24,12 +26,14 @@
import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilegeDescriptor;
import org.elasticsearch.xpack.core.security.authz.privilege.ClusterPrivilegeResolver;
import org.elasticsearch.xpack.core.security.authz.privilege.IndexPrivilege;
import org.elasticsearch.xpack.core.security.support.Automatons;
import org.junit.Before;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Set;
import java.util.function.Predicate;

import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
Expand All @@ -50,6 +54,7 @@ public void testRoleConstructorWithLimitedRole() {
Role limitedByRole = Role.builder("limited-role").build();
Role role = LimitedRole.createLimitedRole(fromRole, limitedByRole);
assertNotNull(role);
assertThat(role.names(), is(limitedByRole.names()));

NullPointerException npe = expectThrows(NullPointerException.class, () -> LimitedRole.createLimitedRole(fromRole, null));
assertThat(npe.getMessage(), containsString("limited by role is required to create limited role"));
Expand Down Expand Up @@ -200,6 +205,42 @@ public void testAllowedIndicesMatcher() {
}
}

public void testAllowedActionsMatcher() {
Role fromRole = Role.builder("fromRole")
.add(IndexPrivilege.WRITE, "ind*")
.add(IndexPrivilege.READ, "ind*")
.add(IndexPrivilege.READ, "other*")
.build();
Automaton fromRoleAutomaton = fromRole.allowedActionsMatcher("index1");
Predicate<String> fromRolePredicate = Automatons.predicate(fromRoleAutomaton);
assertThat(fromRolePredicate.test(SearchAction.NAME), is(true));
assertThat(fromRolePredicate.test(BulkAction.NAME), is(true));

Role limitedByRole = Role.builder("limitedRole")
.add(IndexPrivilege.READ, "index1", "index2")
.build();
Automaton limitedByRoleAutomaton = limitedByRole.allowedActionsMatcher("index1");
Predicate<String> limitedByRolePredicated = Automatons.predicate(limitedByRoleAutomaton);
assertThat(limitedByRolePredicated.test(SearchAction.NAME), is(true));
assertThat(limitedByRolePredicated.test(BulkAction.NAME), is(false));
Role role = LimitedRole.createLimitedRole(fromRole, limitedByRole);

Automaton roleAutomaton = role.allowedActionsMatcher("index1");
Predicate<String> rolePredicate = Automatons.predicate(roleAutomaton);
assertThat(rolePredicate.test(SearchAction.NAME), is(true));
assertThat(rolePredicate.test(BulkAction.NAME), is(false));

roleAutomaton = role.allowedActionsMatcher("index2");
rolePredicate = Automatons.predicate(roleAutomaton);
assertThat(rolePredicate.test(SearchAction.NAME), is(true));
assertThat(rolePredicate.test(BulkAction.NAME), is(false));

roleAutomaton = role.allowedActionsMatcher("other");
rolePredicate = Automatons.predicate(roleAutomaton);
assertThat(rolePredicate.test(SearchAction.NAME), is(false));
assertThat(rolePredicate.test(BulkAction.NAME), is(false));
}

public void testCheckClusterPrivilege() {
Role fromRole = Role.builder("a-role").cluster(Collections.singleton("manage_security"), Collections.emptyList())
.build();
Expand Down
22 changes: 17 additions & 5 deletions ...k/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@
import org.elasticsearch.xpack.core.security.action.GetApiKeyResponse;
import org.elasticsearch.xpack.core.security.action.InvalidateApiKeyResponse;
import org.elasticsearch.xpack.core.security.authc.Authentication;
import org.elasticsearch.xpack.core.security.authc.Authentication.RealmRef;
import org.elasticsearch.xpack.core.security.authc.AuthenticationResult;
import org.elasticsearch.xpack.core.security.authc.support.Hasher;
import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
Expand Down Expand Up @@ -240,7 +241,7 @@ private void createApiKeyAndIndexIt(Authentication authentication, CreateApiKeyR
}

/**
* package protected for testing
* package-private for testing
*/
XContentBuilder newDocument(SecureString apiKey, String name, Authentication authentication, Set<RoleDescriptor> userRoles,
Instant created, Instant expiration, List<RoleDescriptor> keyRoles,
Expand Down Expand Up @@ -329,6 +330,16 @@ void authenticateWithApiKeyIfPresent(ThreadContext ctx, ActionListener<Authentic
}
}

public Authentication createApiKeyAuthentication(AuthenticationResult authResult, String nodeName) {
if (false == authResult.isAuthenticated()) {
throw new IllegalArgumentException("API Key authn result must be successful");
}
final User user = authResult.getUser();
final RealmRef authenticatedBy = new RealmRef(ApiKeyService.API_KEY_REALM_NAME, ApiKeyService.API_KEY_REALM_TYPE, nodeName);
return new Authentication(user, authenticatedBy, null, Version.CURRENT, Authentication.AuthenticationType.API_KEY,
authResult.getMetadata());
}

private void loadApiKeyAndValidateCredentials(ThreadContext ctx, ApiKeyCredentials credentials,
ActionListener<AuthenticationResult> listener) {
final String docId = credentials.getId();
Expand Down Expand Up @@ -525,7 +536,8 @@ CachedApiKeyHashResult getFromCache(String id) {
return apiKeyAuthCache == null ? null : FutureUtils.get(apiKeyAuthCache.get(id), 0L, TimeUnit.MILLISECONDS);
}

private void validateApiKeyExpiration(Map<String, Object> source, ApiKeyCredentials credentials, Clock clock,
// package-private for testing
void validateApiKeyExpiration(Map<String, Object> source, ApiKeyCredentials credentials, Clock clock,
ActionListener<AuthenticationResult> listener) {
final Long expirationEpochMilli = (Long) source.get("expiration_time");
if (expirationEpochMilli == null || Instant.ofEpochMilli(expirationEpochMilli).isAfter(clock.instant())) {
Expand Down Expand Up @@ -618,12 +630,12 @@ public void ensureEnabled() {
}
}

// package private class for testing
static final class ApiKeyCredentials implements Closeable {
// public class for testing
public static final class ApiKeyCredentials implements Closeable {
private final String id;
private final SecureString key;

ApiKeyCredentials(String id, SecureString key) {
public ApiKeyCredentials(String id, SecureString key) {
this.id = id;
this.key = key;
}
Expand Down
7 changes: 3 additions & 4 deletions .../security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -346,10 +346,9 @@ private void authenticateAsync() {
private void checkForApiKey() {
apiKeyService.authenticateWithApiKeyIfPresent(threadContext, ActionListener.wrap(authResult -> {
if (authResult.isAuthenticated()) {
final User user = authResult.getUser();
authenticatedBy = new RealmRef(ApiKeyService.API_KEY_REALM_NAME, ApiKeyService.API_KEY_REALM_TYPE, nodeName);
writeAuthToContext(new Authentication(user, authenticatedBy, null, Version.CURRENT,
Authentication.AuthenticationType.API_KEY, authResult.getMetadata()));
final Authentication authentication = apiKeyService.createApiKeyAuthentication(authResult, nodeName);
this.authenticatedBy = authentication.getAuthenticatedBy();
writeAuthToContext(authentication);
} else if (authResult.getStatus() == AuthenticationResult.Status.TERMINATE) {
Exception e = (authResult.getException() != null) ? authResult.getException()
: Exceptions.authenticationError(authResult.getMessage());
Expand Down
18 changes: 18 additions & 0 deletions ...gin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ExecutionException;
Expand Down Expand Up @@ -720,6 +721,23 @@ public void testCachedApiKeyValidationWillNotBeBlockedByUnCachedApiKey() throws
assertEquals(AuthenticationResult.Status.SUCCESS, authenticationResult3.getStatus());
}

public static class Utils {

public static Authentication createApiKeyAuthentication(ApiKeyService apiKeyService,
Authentication authentication,
Set<RoleDescriptor> userRoles,
List<RoleDescriptor> keyRoles) throws Exception {
XContentBuilder keyDocSource = apiKeyService.newDocument(new SecureString("secret".toCharArray()), "test", authentication,
userRoles, Instant.now(), Instant.now().plus(Duration.ofSeconds(3600)), keyRoles, Version.CURRENT);
Map<String, Object> keyDocMap = XContentHelper.convertToMap(BytesReference.bytes(keyDocSource), true, XContentType.JSON).v2();
PlainActionFuture<AuthenticationResult> authenticationResultFuture = PlainActionFuture.newFuture();
apiKeyService.validateApiKeyExpiration(keyDocMap, new ApiKeyService.ApiKeyCredentials("id",
new SecureString("pass".toCharArray())),
Clock.systemUTC(), authenticationResultFuture);
return apiKeyService.createApiKeyAuthentication(authenticationResultFuture.get(), "node01");
}
}

private ApiKeyService createApiKeyService(Settings baseSettings) {
final Settings settings = Settings.builder()
.put(XPackSettings.API_KEY_SERVICE_ENABLED_SETTING.getKey(), true)
Expand Down
Loading

0 comments on commit d40ab54

Please sign in to comment.