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

Add extension point for file-based role validation #107177

Merged
merged 6 commits into from
Apr 12, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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 @@ -296,6 +296,7 @@
import org.elasticsearch.xpack.security.authz.AuthorizationDenialMessages;
import org.elasticsearch.xpack.security.authz.AuthorizationService;
import org.elasticsearch.xpack.security.authz.DlsFlsRequestCacheDifferentiator;
import org.elasticsearch.xpack.security.authz.FileRoleValidator;
import org.elasticsearch.xpack.security.authz.ReservedRoleNameChecker;
import org.elasticsearch.xpack.security.authz.SecuritySearchOperationListener;
import org.elasticsearch.xpack.security.authz.accesscontrol.OptOutQueryCache;
Expand Down Expand Up @@ -588,6 +589,7 @@ public class Security extends Plugin
private final SetOnce<List<ReloadableSecurityComponent>> reloadableComponents = new SetOnce<>();
private final SetOnce<AuthorizationDenialMessages> authorizationDenialMessages = new SetOnce<>();
private final SetOnce<ReservedRoleNameChecker.Factory> reservedRoleNameCheckerFactory = new SetOnce<>();
private final SetOnce<FileRoleValidator> fileRoleValidator = new SetOnce<>();
private final SetOnce<SecondaryAuthActions> secondaryAuthActions = new SetOnce<>();

public Security(Settings settings) {
Expand Down Expand Up @@ -828,7 +830,6 @@ Collection<Object> createComponents(
dlsBitsetCache.set(new DocumentSubsetBitsetCache(settings, threadPool));
final FieldPermissionsCache fieldPermissionsCache = new FieldPermissionsCache(settings);

this.fileRolesStore.set(new FileRolesStore(settings, environment, resourceWatcherService, getLicenseState(), xContentRegistry));
final NativeRolesStore nativeRolesStore = new NativeRolesStore(
settings,
client,
Expand Down Expand Up @@ -859,6 +860,12 @@ Collection<Object> createComponents(
if (reservedRoleNameCheckerFactory.get() == null) {
reservedRoleNameCheckerFactory.set(new ReservedRoleNameChecker.Factory.Default());
}
if (fileRoleValidator.get() == null) {
fileRoleValidator.set(new FileRoleValidator.Default());
}
this.fileRolesStore.set(
new FileRolesStore(settings, environment, resourceWatcherService, getLicenseState(), xContentRegistry, fileRoleValidator.get())
);
final ReservedRoleNameChecker reservedRoleNameChecker = reservedRoleNameCheckerFactory.get().create(fileRolesStore.get()::exists);
components.add(new PluginComponentBinding<>(ReservedRoleNameChecker.class, reservedRoleNameChecker));

Expand Down Expand Up @@ -2118,6 +2125,7 @@ public void loadExtensions(ExtensionLoader loader) {
loadSingletonExtensionAndSetOnce(loader, hasPrivilegesRequestBuilderFactory, HasPrivilegesRequestBuilderFactory.class);
loadSingletonExtensionAndSetOnce(loader, authorizationDenialMessages, AuthorizationDenialMessages.class);
loadSingletonExtensionAndSetOnce(loader, reservedRoleNameCheckerFactory, ReservedRoleNameChecker.Factory.class);
loadSingletonExtensionAndSetOnce(loader, fileRoleValidator, FileRoleValidator.class);
loadSingletonExtensionAndSetOnce(loader, secondaryAuthActions, SecondaryAuthActions.class);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

package org.elasticsearch.xpack.security.authz;

import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;

/**
* Provides a check which will be applied to roles in the file-based roles store.
*/
@FunctionalInterface
public interface FileRoleValidator {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, I think this came out quite clean!

ActionRequestValidationException validatePredefinedRole(RoleDescriptor roleDescriptor);

/**
* The default file role validator used in stateful Elasticsearch, a no-op.
*/
class Default implements FileRoleValidator {
@Override
public ActionRequestValidationException validatePredefinedRole(RoleDescriptor roleDescriptor) {
return null;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.Maps;
import org.elasticsearch.common.util.set.Sets;
Expand All @@ -36,6 +37,7 @@
import org.elasticsearch.xpack.core.security.authz.support.DLSRoleQueryValidator;
import org.elasticsearch.xpack.core.security.support.NoOpLogger;
import org.elasticsearch.xpack.core.security.support.Validation;
import org.elasticsearch.xpack.security.authz.FileRoleValidator;

import java.io.IOException;
import java.nio.charset.StandardCharsets;
Expand Down Expand Up @@ -67,6 +69,7 @@ public class FileRolesStore implements BiConsumer<Set<String>, ActionListener<Ro

private final Settings settings;
private final Path file;
private final FileRoleValidator roleValidator;
private final XPackLicenseState licenseState;
private final NamedXContentRegistry xContentRegistry;
private final List<Consumer<Set<String>>> listeners = new ArrayList<>();
Expand All @@ -78,21 +81,24 @@ public FileRolesStore(
Environment env,
ResourceWatcherService watcherService,
XPackLicenseState licenseState,
NamedXContentRegistry xContentRegistry
NamedXContentRegistry xContentRegistry,
FileRoleValidator roleValidator
) throws IOException {
this(settings, env, watcherService, null, licenseState, xContentRegistry);
this(settings, env, watcherService, null, roleValidator, licenseState, xContentRegistry);
}

FileRolesStore(
Settings settings,
Environment env,
ResourceWatcherService watcherService,
Consumer<Set<String>> listener,
FileRoleValidator roleValidator,
XPackLicenseState licenseState,
NamedXContentRegistry xContentRegistry
) throws IOException {
this.settings = settings;
this.file = resolveFile(env);
this.roleValidator = roleValidator;
if (listener != null) {
listeners.add(listener);
}
Expand All @@ -101,7 +107,7 @@ public FileRolesStore(
FileWatcher watcher = new FileWatcher(file.getParent());
watcher.addListener(new FileListener());
watcherService.add(watcher, ResourceWatcherService.Frequency.HIGH);
permissions = parseFile(file, logger, settings, licenseState, xContentRegistry);
permissions = parseFile(file, logger, settings, licenseState, xContentRegistry, roleValidator);
}

@Override
Expand Down Expand Up @@ -176,27 +182,18 @@ public static Path resolveFile(Environment env) {
}

public static Set<String> parseFileForRoleNames(Path path, Logger logger) {
// EMPTY is safe here because we never use namedObject as we are just parsing role names
return parseRoleDescriptors(path, logger, false, Settings.EMPTY, NamedXContentRegistry.EMPTY).keySet();
}

public static Map<String, RoleDescriptor> parseFile(
Path path,
Logger logger,
Settings settings,
XPackLicenseState licenseState,
NamedXContentRegistry xContentRegistry
) {
return parseFile(path, logger, true, settings, licenseState, xContentRegistry);
// EMPTY & default role validator are safe here because we never use namedObject as we are just parsing role names
return parseRoleDescriptors(path, logger, false, Settings.EMPTY, new FileRoleValidator.Default(), NamedXContentRegistry.EMPTY)
Copy link
Contributor

Choose a reason for hiding this comment

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

Optional nit: parseRoleDescriptors with the above invocation is only used in one place now, so might as well inline it.

.keySet();
}

public static Map<String, RoleDescriptor> parseFile(
Path path,
Logger logger,
boolean resolvePermission,
Settings settings,
XPackLicenseState licenseState,
NamedXContentRegistry xContentRegistry
NamedXContentRegistry xContentRegistry,
FileRoleValidator roleValidator
) {
if (logger == null) {
logger = NoOpLogger.INSTANCE;
Expand All @@ -210,7 +207,7 @@ public static Map<String, RoleDescriptor> parseFile(

final boolean isDlsLicensed = DOCUMENT_LEVEL_SECURITY_FEATURE.checkWithoutTracking(licenseState);
for (String segment : roleSegments) {
RoleDescriptor descriptor = parseRoleDescriptor(segment, path, logger, resolvePermission, settings, xContentRegistry);
RoleDescriptor descriptor = parseRoleDescriptor(segment, path, logger, true, settings, xContentRegistry, roleValidator);
if (descriptor != null) {
if (ReservedRolesStore.isReserved(descriptor.getName())) {
logger.warn(
Expand Down Expand Up @@ -243,11 +240,12 @@ public static Map<String, RoleDescriptor> parseFile(
return unmodifiableMap(roles);
}

public static Map<String, RoleDescriptor> parseRoleDescriptors(
private static Map<String, RoleDescriptor> parseRoleDescriptors(
Path path,
Logger logger,
boolean resolvePermission,
Settings settings,
FileRoleValidator roleValidator,
NamedXContentRegistry xContentRegistry
) {
if (logger == null) {
Expand All @@ -260,7 +258,15 @@ public static Map<String, RoleDescriptor> parseRoleDescriptors(
try {
List<String> roleSegments = roleSegments(path);
for (String segment : roleSegments) {
RoleDescriptor rd = parseRoleDescriptor(segment, path, logger, resolvePermission, settings, xContentRegistry);
RoleDescriptor rd = parseRoleDescriptor(
segment,
path,
logger,
resolvePermission,
settings,
xContentRegistry,
roleValidator
);
if (rd != null) {
roles.put(rd.getName(), rd);
}
Expand All @@ -280,7 +286,8 @@ static RoleDescriptor parseRoleDescriptor(
Logger logger,
boolean resolvePermissions,
Settings settings,
NamedXContentRegistry xContentRegistry
NamedXContentRegistry xContentRegistry,
FileRoleValidator roleValidator
) {
String roleName = null;
XContentParserConfiguration parserConfig = XContentParserConfiguration.EMPTY.withRegistry(xContentRegistry)
Expand Down Expand Up @@ -311,7 +318,7 @@ static RoleDescriptor parseRoleDescriptor(
// we pass true as last parameter because we do not want to reject files if field permissions
// are given in 2.x syntax
RoleDescriptor descriptor = RoleDescriptor.parse(roleName, parser, true, false);
return checkDescriptor(descriptor, path, logger, settings, xContentRegistry);
return checkDescriptor(descriptor, path, logger, settings, xContentRegistry, roleValidator);
} else {
logger.error("invalid role definition [{}] in roles file [{}]. skipping role...", roleName, path.toAbsolutePath());
return null;
Expand Down Expand Up @@ -344,7 +351,8 @@ private static RoleDescriptor checkDescriptor(
Path path,
Logger logger,
Settings settings,
NamedXContentRegistry xContentRegistry
NamedXContentRegistry xContentRegistry,
FileRoleValidator roleValidator
) {
String roleName = descriptor.getName();
// first check if FLS/DLS is enabled on the role...
Expand Down Expand Up @@ -374,6 +382,10 @@ private static RoleDescriptor checkDescriptor(
}
}
}
ActionRequestValidationException ex = roleValidator.validatePredefinedRole(descriptor);
Copy link
Contributor

Choose a reason for hiding this comment

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

Might add a test to FileRoleStoreTests with a mocked validator that throws

if (ex != null) {
throw ex;
}
return descriptor;
}

Expand Down Expand Up @@ -417,7 +429,7 @@ public synchronized void onFileChanged(Path file) {
if (file.equals(FileRolesStore.this.file)) {
final Map<String, RoleDescriptor> previousPermissions = permissions;
try {
permissions = parseFile(file, logger, settings, licenseState, xContentRegistry);
permissions = parseFile(file, logger, settings, licenseState, xContentRegistry, roleValidator);
} catch (Exception e) {
logger.error(
() -> format("could not reload roles file [%s]. Current roles remain unmodified", file.toAbsolutePath()),
Expand Down
Loading