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

Migrate custom role providers to licensed feature #79127

Merged
merged 4 commits into from
Oct 18, 2021
Merged

Migrate custom role providers to licensed feature #79127

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f435488
Migrate custom role providers to licensed feature
tvernum Oct 14, 2021
a599361
Merge branch 'master' into licensed-feature-role-providers
tvernum Oct 15, 2021
842f0de
Add test & other feedback
tvernum Oct 15, 2021
18ea781
Merge branch 'master' into licensed-feature-role-providers
tvernum Oct 18, 2021
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
1 change: 0 additions & 1 deletion x-pack/plugin/core/src/main/java/org/elasticsearch/license/XPackLicenseState.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ public class XPackLicenseState {
*/
public enum Feature {
SECURITY_AUDITING(OperationMode.GOLD, false),
SECURITY_CUSTOM_ROLE_PROVIDERS(OperationMode.PLATINUM, true),
SECURITY_TOKEN_SERVICE(OperationMode.STANDARD, false),
SECURITY_AUTHORIZATION_REALM(OperationMode.PLATINUM, true),
SECURITY_AUTHORIZATION_ENGINE(OperationMode.PLATINUM, true),
Expand Down
19 changes: 19 additions & 0 deletions x-pack/plugin/core/src/test/java/org/elasticsearch/license/MockLicenseState.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,13 @@

package org.elasticsearch.license;

import org.mockito.Mockito;

import java.util.function.Consumer;
import java.util.function.LongSupplier;

import static org.mockito.Mockito.doAnswer;

/** A license state that may be mocked by testing because the internal methods are made public */
public class MockLicenseState extends XPackLicenseState {

Expand All @@ -30,4 +35,18 @@ public void enableUsageTracking(LicensedFeature feature, String contextName) {
public void disableUsageTracking(LicensedFeature feature, String contextName) {
super.disableUsageTracking(feature, contextName);
}

public static MockLicenseState createMock() {
MockLicenseState mock = Mockito.mock(MockLicenseState.class);
Mockito.when(mock.copyCurrentLicenseState()).thenReturn(mock);
return mock;
}

public static void acceptListeners(MockLicenseState licenseState, Consumer<LicenseStateListener> addListener) {
doAnswer(inv -> {
final LicenseStateListener listener = (LicenseStateListener) inv.getArguments()[0];
addListener.accept(listener);
return null;
}).when(licenseState).addListener(Mockito.any());
}
}
8 changes: 0 additions & 8 deletions x-pack/plugin/core/src/test/java/org/elasticsearch/license/XPackLicenseStateTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,15 +89,13 @@ public static OperationMode randomBasicStandardOrGold() {
public void testSecurityDefaults() {
XPackLicenseState licenseState = new XPackLicenseState(() -> 0);
assertThat(licenseState.checkFeature(Feature.SECURITY_AUDITING), is(true));
assertThat(licenseState.checkFeature(Feature.SECURITY_CUSTOM_ROLE_PROVIDERS), is(true));
}

public void testSecurityStandard() {
XPackLicenseState licenseState = new XPackLicenseState(() -> 0);
licenseState.update(STANDARD, true, null);

assertThat(licenseState.checkFeature(Feature.SECURITY_AUDITING), is(false));
assertThat(licenseState.checkFeature(Feature.SECURITY_CUSTOM_ROLE_PROVIDERS), is(false));
assertThat(licenseState.checkFeature(Feature.SECURITY_TOKEN_SERVICE), is(true));
}

Expand All @@ -106,7 +104,6 @@ public void testSecurityStandardExpired() {
licenseState.update(STANDARD, false, null);

assertThat(licenseState.checkFeature(Feature.SECURITY_AUDITING), is(false));
assertThat(licenseState.checkFeature(Feature.SECURITY_CUSTOM_ROLE_PROVIDERS), is(false));
assertThat(licenseState.checkFeature(Feature.SECURITY_TOKEN_SERVICE), is(true));
}

Expand All @@ -115,7 +112,6 @@ public void testSecurityBasic() {
licenseState.update(BASIC, true, null);

assertThat(licenseState.checkFeature(Feature.SECURITY_AUDITING), is(false));
assertThat(licenseState.checkFeature(Feature.SECURITY_CUSTOM_ROLE_PROVIDERS), is(false));
assertThat(licenseState.checkFeature(Feature.SECURITY_TOKEN_SERVICE), is(false));
}

Expand All @@ -124,7 +120,6 @@ public void testSecurityGold() {
licenseState.update(GOLD, true, null);

assertThat(licenseState.checkFeature(Feature.SECURITY_AUDITING), is(true));
assertThat(licenseState.checkFeature(Feature.SECURITY_CUSTOM_ROLE_PROVIDERS), is(false));
assertThat(licenseState.checkFeature(Feature.SECURITY_TOKEN_SERVICE), is(true));
}

Expand All @@ -133,7 +128,6 @@ public void testSecurityGoldExpired() {
licenseState.update(GOLD, false, null);

assertThat(licenseState.checkFeature(Feature.SECURITY_AUDITING), is(true));
assertThat(licenseState.checkFeature(Feature.SECURITY_CUSTOM_ROLE_PROVIDERS), is(false));
assertThat(licenseState.checkFeature(Feature.SECURITY_TOKEN_SERVICE), is(true));
}

Expand All @@ -142,7 +136,6 @@ public void testSecurityPlatinum() {
licenseState.update(PLATINUM, true, null);

assertThat(licenseState.checkFeature(Feature.SECURITY_AUDITING), is(true));
assertThat(licenseState.checkFeature(Feature.SECURITY_CUSTOM_ROLE_PROVIDERS), is(true));
assertThat(licenseState.checkFeature(Feature.SECURITY_TOKEN_SERVICE), is(true));
}

Expand All @@ -151,7 +144,6 @@ public void testSecurityPlatinumExpired() {
licenseState.update(PLATINUM, false, null);

assertThat(licenseState.checkFeature(Feature.SECURITY_AUDITING), is(true));
assertThat(licenseState.checkFeature(Feature.SECURITY_CUSTOM_ROLE_PROVIDERS), is(false));
assertThat(licenseState.checkFeature(Feature.SECURITY_TOKEN_SERVICE), is(true));
}

Expand Down
28 changes: 23 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 @@ -245,6 +245,7 @@
import org.elasticsearch.xpack.security.authz.store.FileRolesStore;
import org.elasticsearch.xpack.security.authz.store.NativePrivilegeStore;
import org.elasticsearch.xpack.security.authz.store.NativeRolesStore;
import org.elasticsearch.xpack.security.authz.store.RoleProviders;
import org.elasticsearch.xpack.security.ingest.SetSecurityUserProcessor;
import org.elasticsearch.xpack.security.operator.FileOperatorUsersStore;
import org.elasticsearch.xpack.security.operator.OperatorOnlyRegistry;
Expand Down Expand Up @@ -318,6 +319,7 @@
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
Expand Down Expand Up @@ -376,6 +378,10 @@ public class Security extends Plugin implements SystemIndexPlugin, IngestPlugin,
public static final LicensedFeature.Persistent CUSTOM_REALMS_FEATURE =
LicensedFeature.persistentLenient(REALMS_FEATURE_FAMILY, "custom", License.OperationMode.PLATINUM);

// Custom role providers are Platinum+
public static final LicensedFeature.Persistent CUSTOM_ROLE_PROVIDERS_FEATURE =
LicensedFeature.persistent(null, "security-roles-provider", License.OperationMode.PLATINUM);

private static final Logger logger = LogManager.getLogger(Security.class);

public static final SystemIndexDescriptor SECURITY_MAIN_INDEX_DESCRIPTOR = getSecurityMainIndexDescriptor();
Expand Down Expand Up @@ -423,7 +429,6 @@ public Security(Settings settings, final Path configPath) {
this.bootstrapChecks.set(Collections.emptyList());
}
this.securityExtensions.addAll(extensions);

}

private static void runStartupChecks(Settings settings) {
Expand Down Expand Up @@ -548,9 +553,15 @@ Collection<Object> createComponents(Client client, ThreadPool threadPool, Cluste
xContentRegistry);
final NativeRolesStore nativeRolesStore = new NativeRolesStore(settings, client, getLicenseState(), securityIndex.get());
final ReservedRolesStore reservedRolesStore = new ReservedRolesStore();
List<BiConsumer<Set<String>, ActionListener<RoleRetrievalResult>>> rolesProviders = new ArrayList<>();

final Map<String, List<BiConsumer<Set<String>, ActionListener<RoleRetrievalResult>>>> customRoleProviders = new LinkedHashMap<>();
for (SecurityExtension extension : securityExtensions) {
rolesProviders.addAll(extension.getRolesProviders(extensionComponents));
final List<BiConsumer<Set<String>, ActionListener<RoleRetrievalResult>>> providers = extension.getRolesProviders(
extensionComponents
);
if (providers != null && providers.isEmpty() == false) {
customRoleProviders.put(extension.toString(), providers);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The generic toString include the hashCode of the object. Is that really an useful information to include? It is not stable. I'd think the class name is sufficient and easier to read.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree in principle, however

elasticsearch/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java

Lines 677 to 683 in f435488

if (extensionEngine != null && authorizationEngine != null) {
throw new IllegalStateException("Extensions [" + extensionName + "] and [" + extension.toString() + "] "
+ "both set an authorization engine");
}
authorizationEngine = extensionEngine;
extensionName = extension.toString();
}

we already rely on extension.toString().

I want to follow up by adding a name() method on extension and replace all of these uses with that instead.

}
}

final ApiKeyService apiKeyService = new ApiKeyService(settings, Clock.systemUTC(), client, securityIndex.get(),
Expand All @@ -572,8 +583,15 @@ Collection<Object> createComponents(Client client, ThreadPool threadPool, Cluste
);
components.add(serviceAccountService);

final CompositeRolesStore allRolesStore = new CompositeRolesStore(settings, fileRolesStore, nativeRolesStore, reservedRolesStore,
privilegeStore, rolesProviders, threadPool.getThreadContext(), getLicenseState(), fieldPermissionsCache, apiKeyService,
final RoleProviders roleProviders = new RoleProviders(
reservedRolesStore,
fileRolesStore,
nativeRolesStore,
customRoleProviders,
getLicenseState()
);
final CompositeRolesStore allRolesStore = new CompositeRolesStore(settings, roleProviders,
privilegeStore, threadPool.getThreadContext(), getLicenseState(), fieldPermissionsCache, apiKeyService,
serviceAccountService, dlsBitsetCache.get(), expressionResolver,
new DeprecationRoleDescriptorConsumer(clusterService, threadPool));
securityIndex.get().addStateListener(allRolesStore::onSecurityIndexStateChange);
Expand Down
57 changes: 24 additions & 33 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 @@ -30,7 +30,6 @@
import org.elasticsearch.common.util.concurrent.ThreadContext;
import org.elasticsearch.common.util.set.Sets;
import org.elasticsearch.license.XPackLicenseState;
import org.elasticsearch.license.XPackLicenseState.Feature;
import org.elasticsearch.xpack.core.common.IteratingActionListener;
import org.elasticsearch.xpack.core.security.authc.Authentication;
import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
Expand Down Expand Up @@ -84,8 +83,8 @@
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
* file roles, and finally the index roles.
* A composite roles store that can retrieve roles from multiple sources.
* @see RoleProviders
*/
public class CompositeRolesStore {

Expand All @@ -98,8 +97,7 @@ public class CompositeRolesStore {

private final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(CompositeRolesStore.class);

private final FileRolesStore fileRolesStore;
private final NativeRolesStore nativeRolesStore;
private final RoleProviders roleProviders;
private final NativePrivilegeStore privilegeStore;
private final XPackLicenseState licenseState;
private final Consumer<Collection<RoleDescriptor>> effectiveRoleDescriptorsConsumer;
Expand All @@ -114,25 +112,31 @@ public class CompositeRolesStore {
private final ApiKeyService apiKeyService;
private final ServiceAccountService serviceAccountService;
private final boolean isAnonymousEnabled;
private final List<BiConsumer<Set<String>, ActionListener<RoleRetrievalResult>>> builtInRoleProviders;
private final List<BiConsumer<Set<String>, ActionListener<RoleRetrievalResult>>> allRoleProviders;
private final Role superuserRole;
private final Role xpackUserRole;
private final Role asyncSearchUserRole;
private final Automaton restrictedIndicesAutomaton;

public CompositeRolesStore(Settings settings, FileRolesStore fileRolesStore, NativeRolesStore nativeRolesStore,
ReservedRolesStore reservedRolesStore, NativePrivilegeStore privilegeStore,
List<BiConsumer<Set<String>, ActionListener<RoleRetrievalResult>>> rolesProviders,
public CompositeRolesStore(Settings settings, RoleProviders roleProviders, NativePrivilegeStore privilegeStore,
ThreadContext threadContext, XPackLicenseState licenseState, FieldPermissionsCache fieldPermissionsCache,
ApiKeyService apiKeyService, ServiceAccountService serviceAccountService,
DocumentSubsetBitsetCache dlsBitsetCache, IndexNameExpressionResolver resolver,
Consumer<Collection<RoleDescriptor>> effectiveRoleDescriptorsConsumer) {
this.fileRolesStore = Objects.requireNonNull(fileRolesStore);
this.dlsBitsetCache = Objects.requireNonNull(dlsBitsetCache);
fileRolesStore.addListener(this::invalidate);
this.nativeRolesStore = Objects.requireNonNull(nativeRolesStore);
this.roleProviders = roleProviders;
roleProviders.addChangeListener(new RoleProviders.ChangeListener() {
@Override
public void rolesChanged(Set<String> roles) {
CompositeRolesStore.this.invalidate(roles);
}

@Override
public void providersChanged() {
CompositeRolesStore.this.invalidateAll();
}
});

this.privilegeStore = Objects.requireNonNull(privilegeStore);
this.dlsBitsetCache = Objects.requireNonNull(dlsBitsetCache);
this.licenseState = Objects.requireNonNull(licenseState);
this.fieldPermissionsCache = Objects.requireNonNull(fieldPermissionsCache);
this.apiKeyService = Objects.requireNonNull(apiKeyService);
Expand All @@ -152,16 +156,6 @@ public CompositeRolesStore(Settings settings, FileRolesStore fileRolesStore, Nat
nlcBuilder.setMaximumWeight(nlcCacheSize);
}
this.negativeLookupCache = nlcBuilder.build();
this.builtInRoleProviders = List.of(reservedRolesStore, fileRolesStore, nativeRolesStore);
if (rolesProviders.isEmpty()) {
this.allRoleProviders = this.builtInRoleProviders;
} else {
List<BiConsumer<Set<String>, ActionListener<RoleRetrievalResult>>> allList =
new ArrayList<>(builtInRoleProviders.size() + rolesProviders.size());
allList.addAll(builtInRoleProviders);
allList.addAll(rolesProviders);
this.allRoleProviders = Collections.unmodifiableList(allList);
}
this.anonymousUser = new AnonymousUser(settings);
this.isAnonymousEnabled = AnonymousUser.isAnonymousEnabled(settings);
this.restrictedIndicesAutomaton = resolver.getSystemNameAutomaton();
Expand Down Expand Up @@ -411,9 +405,7 @@ private void roleDescriptors(Set<String> roleNames, ActionListener<RolesRetrieva

private void loadRoleDescriptorsAsync(Set<String> roleNames, ActionListener<RolesRetrievalResult> listener) {
final RolesRetrievalResult rolesResult = new RolesRetrievalResult();
final List<BiConsumer<Set<String>, ActionListener<RoleRetrievalResult>>> asyncRoleProviders =
licenseState.checkFeature(Feature.SECURITY_CUSTOM_ROLE_PROVIDERS) ? allRoleProviders : builtInRoleProviders;

final List<BiConsumer<Set<String>, ActionListener<RoleRetrievalResult>>> asyncRoleProviders = roleProviders.getProviders();
final ActionListener<RoleRetrievalResult> descriptorsListener =
ContextPreservingActionListener.wrapPreservingContext(ActionListener.wrap(ignore -> {
rolesResult.setMissingRoles(roleNames);
Expand Down Expand Up @@ -553,13 +545,12 @@ public void invalidate(Set<String> roles) {
}

public void usageStats(ActionListener<Map<String, Object>> listener) {
final Map<String, Object> usage = new HashMap<>(2);
usage.put("file", fileRolesStore.usageStats());
final Map<String, Object> usage = new HashMap<>();
usage.put("dls", Map.of("bit_set_cache", dlsBitsetCache.usageStats()));
nativeRolesStore.usageStats(ActionListener.wrap(map -> {
usage.put("native", map);
listener.onResponse(usage);
}, listener::onFailure));
roleProviders.usageStats(listener.map(roleUsage -> {
usage.putAll(roleUsage);
return usage;
}));
}

public void onSecurityIndexStateChange(SecurityIndexManager.State previousState, SecurityIndexManager.State currentState) {
Expand Down
Loading