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.

Sign up for GitHub

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

Jump to bottom

Security: Simplify security index listeners #30466

Merged
merged 4 commits into from
May 10, 2018
Merged

Security: Simplify security index listeners #30466

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1b61e1e
Security: Simplify security index listeners
rjernst May 7, 2018
4f8eb63
Merge branch 'master' into node_started2
rjernst May 9, 2018
857f749
fix inaccessible constant in test
rjernst May 9, 2018
44ffab7
cleanup
rjernst May 10, 2018
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
12 changes: 0 additions & 12 deletions ...re/src/main/java/org/elasticsearch/xpack/core/security/SecurityLifecycleServiceField.java
View file
Open in desktop

This file was deleted.

8 changes: 3 additions & 5 deletions x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@
import static java.util.Collections.singletonList;
import static org.elasticsearch.cluster.metadata.IndexMetaData.INDEX_FORMAT_SETTING;
import static org.elasticsearch.xpack.core.XPackSettings.HTTP_SSL_ENABLED;
import static org.elasticsearch.xpack.core.security.SecurityLifecycleServiceField.SECURITY_TEMPLATE_NAME;
import static org.elasticsearch.xpack.security.support.SecurityIndexManager.SECURITY_TEMPLATE_NAME;
import static org.elasticsearch.xpack.security.SecurityLifecycleService.SECURITY_INDEX_NAME;
import static org.elasticsearch.xpack.security.support.SecurityIndexManager.INTERNAL_INDEX_FORMAT;

Expand Down Expand Up @@ -424,8 +424,7 @@ Collection<Object> createComponents(Client client, ThreadPool threadPool, Cluste
components.add(realms);
components.add(reservedRealm);

securityLifecycleService.securityIndex().addIndexHealthChangeListener(nativeRoleMappingStore::onSecurityIndexHealthChange);
securityLifecycleService.securityIndex().addIndexOutOfDateListener(nativeRoleMappingStore::onSecurityIndexOutOfDateChange);
securityLifecycleService.securityIndex().addIndexStateListener(nativeRoleMappingStore::onSecurityIndexStateChange);

AuthenticationFailureHandler failureHandler = null;
String extensionName = null;
Expand Down Expand Up @@ -458,8 +457,7 @@ Collection<Object> createComponents(Client client, ThreadPool threadPool, Cluste
}
final CompositeRolesStore allRolesStore = new CompositeRolesStore(settings, fileRolesStore, nativeRolesStore,
reservedRolesStore, rolesProviders, threadPool.getThreadContext(), getLicenseState());
securityLifecycleService.securityIndex().addIndexHealthChangeListener(allRolesStore::onSecurityIndexHealthChange);
securityLifecycleService.securityIndex().addIndexOutOfDateListener(allRolesStore::onSecurityIndexOutOfDateChange);
securityLifecycleService.securityIndex().addIndexStateListener(allRolesStore::onSecurityIndexStateChange);
// to keep things simple, just invalidate all cached entries on license change. this happens so rarely that the impact should be
// minimal
getLicenseState().addListener(allRolesStore::invalidateAll);
Expand Down
31 changes: 0 additions & 31 deletions ...gin/security/src/main/java/org/elasticsearch/xpack/security/SecurityLifecycleService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,6 @@ public class SecurityLifecycleService extends AbstractComponent implements Clust
public static final String INTERNAL_SECURITY_INDEX = SecurityIndexManager.INTERNAL_SECURITY_INDEX;
public static final String SECURITY_INDEX_NAME = ".security";

private static final Version MIN_READ_VERSION = Version.V_5_0_0;

private final Settings settings;
private final ThreadPool threadPool;
private final IndexAuditTrail indexAuditTrail;
Expand Down Expand Up @@ -125,36 +123,7 @@ private void close() {
}
}

public static boolean securityIndexMappingSufficientToRead(ClusterState clusterState, Logger logger) {
return checkMappingVersions(clusterState, logger, MIN_READ_VERSION::onOrBefore);
}

static boolean securityIndexMappingUpToDate(ClusterState clusterState, Logger logger) {
return checkMappingVersions(clusterState, logger, Version.CURRENT::equals);
}

private static boolean checkMappingVersions(ClusterState clusterState, Logger logger, Predicate<Version> versionPredicate) {
return SecurityIndexManager.checkIndexMappingVersionMatches(SECURITY_INDEX_NAME, clusterState, logger, versionPredicate);
}

public static List<String> indexNames() {
return Collections.unmodifiableList(Arrays.asList(SECURITY_INDEX_NAME, INTERNAL_SECURITY_INDEX));
}

/**
* Is the move from {@code previousHealth} to {@code currentHealth} a move from an unhealthy ("RED") index state to a healthy
* ("non-RED") state.
*/
public static boolean isMoveFromRedToNonRed(ClusterIndexHealth previousHealth, ClusterIndexHealth currentHealth) {
return (previousHealth == null || previousHealth.getStatus() == ClusterHealthStatus.RED)
&& currentHealth != null && currentHealth.getStatus() != ClusterHealthStatus.RED;
}

/**
* Is the move from {@code previousHealth} to {@code currentHealth} a move from index-exists to index-deleted
*/
public static boolean isIndexDeleted(ClusterIndexHealth previousHealth, ClusterIndexHealth currentHealth) {
return previousHealth != null && currentHealth == null;
}

}
2 changes: 1 addition & 1 deletion .../plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/InternalRealms.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public static Map<String, Realm.Factory> getFactories(ThreadPool threadPool, Res
map.put(FileRealmSettings.TYPE, config -> new FileRealm(config, resourceWatcherService));
map.put(NativeRealmSettings.TYPE, config -> {
final NativeRealm nativeRealm = new NativeRealm(config, nativeUsersStore);
securityLifecycleService.securityIndex().addIndexHealthChangeListener(nativeRealm::onSecurityIndexHealthChange);
securityLifecycleService.securityIndex().addIndexStateListener(nativeRealm::onSecurityIndexStateChange);
return nativeRealm;
});
map.put(LdapRealmSettings.AD_TYPE, config -> new LdapRealm(LdapRealmSettings.AD_TYPE, config, sslService,
Expand Down
14 changes: 6 additions & 8 deletions ...n/security/src/main/java/org/elasticsearch/xpack/security/authc/esnative/NativeRealm.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,16 @@
package org.elasticsearch.xpack.security.authc.esnative;

import org.elasticsearch.action.ActionListener;
import org.elasticsearch.cluster.health.ClusterHealthStatus;
import org.elasticsearch.cluster.health.ClusterIndexHealth;
import org.elasticsearch.xpack.core.security.authc.AuthenticationResult;
import org.elasticsearch.xpack.core.security.authc.RealmConfig;
import org.elasticsearch.xpack.core.security.authc.esnative.NativeRealmSettings;
import org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken;
import org.elasticsearch.xpack.core.security.user.User;
import org.elasticsearch.xpack.security.authc.support.CachingUsernamePasswordRealm;
import org.elasticsearch.xpack.security.support.SecurityIndexManager;

import static org.elasticsearch.xpack.security.support.SecurityIndexManager.isIndexDeleted;
import static org.elasticsearch.xpack.security.support.SecurityIndexManager.isMoveFromRedToNonRed;

/**
* User/password realm that is backed by an Elasticsearch index
Expand All @@ -37,12 +39,8 @@ protected void doAuthenticate(UsernamePasswordToken token, ActionListener<Authen
userStore.verifyPassword(token.principal(), token.credentials(), listener);
}

public void onSecurityIndexHealthChange(ClusterIndexHealth previousHealth, ClusterIndexHealth currentHealth) {
final boolean movedFromRedToNonRed = (previousHealth == null || previousHealth.getStatus() == ClusterHealthStatus.RED)
&& currentHealth != null && currentHealth.getStatus() != ClusterHealthStatus.RED;
final boolean indexDeleted = previousHealth != null && currentHealth == null;

if (movedFromRedToNonRed || indexDeleted) {
public void onSecurityIndexStateChange(SecurityIndexManager.State previousState, SecurityIndexManager.State currentState) {
if (isMoveFromRedToNonRed(previousState, currentState) || isIndexDeleted(previousState, currentState)) {
clearCache();
}
}
Expand Down
16 changes: 7 additions & 9 deletions ...in/java/org/elasticsearch/xpack/security/authc/support/mapper/NativeRoleMappingStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.elasticsearch.action.search.SearchRequest;
import org.elasticsearch.action.support.ContextPreservingActionListener;
import org.elasticsearch.client.Client;
import org.elasticsearch.cluster.health.ClusterHealthStatus;
import org.elasticsearch.cluster.health.ClusterIndexHealth;
import org.elasticsearch.common.CheckedBiConsumer;
import org.elasticsearch.common.bytes.BytesReference;
Expand All @@ -38,6 +39,7 @@
import org.elasticsearch.xpack.security.SecurityLifecycleService;
import org.elasticsearch.xpack.security.authc.support.CachingUsernamePasswordRealm;
import org.elasticsearch.xpack.security.authc.support.UserRoleMapper;
import org.elasticsearch.xpack.security.support.SecurityIndexManager;

import java.io.IOException;
import java.io.InputStream;
Expand All @@ -61,8 +63,8 @@
import static org.elasticsearch.xpack.core.ClientHelper.executeAsyncWithOrigin;
import static org.elasticsearch.xpack.core.ClientHelper.stashWithOrigin;
import static org.elasticsearch.xpack.security.SecurityLifecycleService.SECURITY_INDEX_NAME;
import static org.elasticsearch.xpack.security.SecurityLifecycleService.isIndexDeleted;
import static org.elasticsearch.xpack.security.SecurityLifecycleService.isMoveFromRedToNonRed;
import static org.elasticsearch.xpack.security.support.SecurityIndexManager.isIndexDeleted;
import static org.elasticsearch.xpack.security.support.SecurityIndexManager.isMoveFromRedToNonRed;

/**
* This store reads + writes {@link ExpressionRoleMapping role mappings} in an Elasticsearch
Expand Down Expand Up @@ -316,17 +318,13 @@ private void reportStats(ActionListener<Map<String, Object>> listener, List<Expr
listener.onResponse(usageStats);
}

public void onSecurityIndexHealthChange(ClusterIndexHealth previousHealth, ClusterIndexHealth currentHealth) {
if (isMoveFromRedToNonRed(previousHealth, currentHealth) || isIndexDeleted(previousHealth, currentHealth)) {
public void onSecurityIndexStateChange(SecurityIndexManager.State previousState, SecurityIndexManager.State currentState) {
if (isMoveFromRedToNonRed(previousState, currentState) || isIndexDeleted(previousState, currentState) ||
previousState.isIndexUpToDate != currentState.isIndexUpToDate) {
refreshRealms(NO_OP_ACTION_LISTENER, null);
}
}

public void onSecurityIndexOutOfDateChange(boolean prevOutOfDate, boolean outOfDate) {
assert prevOutOfDate != outOfDate : "this method should only be called if the two values are different";
refreshRealms(NO_OP_ACTION_LISTENER, null);
}

private <Result> void refreshRealms(ActionListener<Result> listener, Result result) {
String[] realmNames = this.realmsToRefresh.toArray(new String[realmsToRefresh.size()]);
final SecurityClient securityClient = new SecurityClient(client);
Expand Down
16 changes: 7 additions & 9 deletions ...urity/src/main/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import org.apache.logging.log4j.message.ParameterizedMessage;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.cluster.health.ClusterHealthStatus;
import org.elasticsearch.cluster.health.ClusterIndexHealth;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.Strings;
Expand Down Expand Up @@ -34,6 +35,7 @@
import org.elasticsearch.xpack.core.security.authz.privilege.Privilege;
import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore;
import org.elasticsearch.xpack.security.SecurityLifecycleService;
import org.elasticsearch.xpack.security.support.SecurityIndexManager;

import java.util.ArrayList;
import java.util.Arrays;
Expand All @@ -53,8 +55,8 @@
import java.util.stream.Collectors;

import static org.elasticsearch.xpack.core.security.SecurityField.setting;
import static org.elasticsearch.xpack.security.SecurityLifecycleService.isIndexDeleted;
import static org.elasticsearch.xpack.security.SecurityLifecycleService.isMoveFromRedToNonRed;
import static org.elasticsearch.xpack.security.support.SecurityIndexManager.isIndexDeleted;
import static org.elasticsearch.xpack.security.support.SecurityIndexManager.isMoveFromRedToNonRed;

/**
* A composite roles store that combines built in roles, file-based roles, and index-based roles. Checks the built in roles first, then the
Expand Down Expand Up @@ -323,17 +325,13 @@ public void usageStats(ActionListener<Map<String, Object>> listener) {
}, listener::onFailure));
}

public void onSecurityIndexHealthChange(ClusterIndexHealth previousHealth, ClusterIndexHealth currentHealth) {
if (isMoveFromRedToNonRed(previousHealth, currentHealth) || isIndexDeleted(previousHealth, currentHealth)) {
public void onSecurityIndexStateChange(SecurityIndexManager.State previousState, SecurityIndexManager.State currentState) {
if (isMoveFromRedToNonRed(previousState, currentState) || isIndexDeleted(previousState, currentState) ||
previousState.isIndexUpToDate != currentState.isIndexUpToDate) {
invalidateAll();
}
}

public void onSecurityIndexOutOfDateChange(boolean prevOutOfDate, boolean outOfDate) {
assert prevOutOfDate != outOfDate : "this method should only be called if the two values are different";
invalidateAll();
}

/**
* A mutable class that can be used to represent the combination of one or more {@link IndicesPrivileges}
*/
Expand Down
Loading