Skip to content

Add ignore_url for setting based files entitlement #123541

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

Merged
rjernst merged 2 commits into from
Feb 26, 2025
Merged

Add ignore_url for setting based files entitlement #123541

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
597f5f9
Add ignore_url for setting based files entitlement
rjernst Feb 26, 2025
45eb0f2
[CI] Auto commit changes from spotless
Feb 26, 2025
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
49 changes: 33 additions & 16 deletions ...main/java/org/elasticsearch/entitlement/runtime/policy/entitlements/FilesEntitlement.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

package org.elasticsearch.entitlement.runtime.policy.entitlements;

import org.elasticsearch.core.Booleans;
import org.elasticsearch.entitlement.runtime.policy.ExternalEntitlement;
import org.elasticsearch.entitlement.runtime.policy.PathLookup;
import org.elasticsearch.entitlement.runtime.policy.PolicyValidationException;
Expand All @@ -17,6 +18,7 @@
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.stream.Stream;
Expand Down Expand Up @@ -85,12 +87,12 @@ static FileData ofRelativePath(Path relativePath, BaseDir baseDir, Mode mode) {
return new RelativePathFileData(relativePath, baseDir, mode, null);
}

static FileData ofPathSetting(String setting, Mode mode) {
return new PathSettingFileData(setting, mode, null);
static FileData ofPathSetting(String setting, Mode mode, boolean ignoreUrl) {
return new PathSettingFileData(setting, mode, ignoreUrl, null);
}

static FileData ofRelativePathSetting(String setting, BaseDir baseDir, Mode mode) {
return new RelativePathSettingFileData(setting, baseDir, mode, null);
static FileData ofRelativePathSetting(String setting, BaseDir baseDir, Mode mode, boolean ignoreUrl) {
return new RelativePathSettingFileData(setting, baseDir, mode, ignoreUrl, null);
}

/**
Expand Down Expand Up @@ -207,45 +209,51 @@ public FileData withPlatform(Platform platform) {
}
}

private record PathSettingFileData(String setting, Mode mode, Platform platform) implements FileData {
private record PathSettingFileData(String setting, Mode mode, boolean ignoreUrl, Platform platform) implements FileData {
@Override
public Stream<Path> resolvePaths(PathLookup pathLookup) {
return resolvePathSettings(pathLookup, setting);
return resolvePathSettings(pathLookup, setting, ignoreUrl);
}

@Override
public FileData withPlatform(Platform platform) {
if (platform == platform()) {
return this;
}
return new PathSettingFileData(setting, mode, platform);
return new PathSettingFileData(setting, mode, ignoreUrl, platform);
}
}

private record RelativePathSettingFileData(String setting, BaseDir baseDir, Mode mode, Platform platform)
private record RelativePathSettingFileData(String setting, BaseDir baseDir, Mode mode, boolean ignoreUrl, Platform platform)
implements
FileData,
RelativeFileData {
@Override
public Stream<Path> resolveRelativePaths(PathLookup pathLookup) {
return resolvePathSettings(pathLookup, setting);
return resolvePathSettings(pathLookup, setting, ignoreUrl);
}

@Override
public FileData withPlatform(Platform platform) {
if (platform == platform()) {
return this;
}
return new RelativePathSettingFileData(setting, baseDir, mode, platform);
return new RelativePathSettingFileData(setting, baseDir, mode, ignoreUrl, platform);
}
}

private static Stream<Path> resolvePathSettings(PathLookup pathLookup, String setting) {
private static Stream<Path> resolvePathSettings(PathLookup pathLookup, String setting, boolean ignoreUrl) {
Stream<String> result;
if (setting.contains("*")) {
return pathLookup.settingGlobResolver().apply(setting).map(Path::of);
result = pathLookup.settingGlobResolver().apply(setting);
} else {
String path = pathLookup.settingResolver().apply(setting);
result = path == null ? Stream.of() : Stream.of(path);
}
if (ignoreUrl) {
result = result.filter(s -> s.toLowerCase(Locale.ROOT).startsWith("https://") == false);
}
String path = pathLookup.settingResolver().apply(setting);
return path == null ? Stream.of() : Stream.of(Path.of(path));
return result.map(Path::of);
Copy link
Contributor

@mark-vieira mark-vieira Feb 26, 2025

Choose a reason for hiding this comment

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

In what scenario would it make sense to not use ignore_url in a file entitlement? Wouldn't calling Path.of() explode on such a value if it weren't filtered out? Should this behavior be the default?

Copy link
Member Author

@rjernst rjernst Feb 26, 2025

Choose a reason for hiding this comment

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

Yes it would explode. But we don't support urls in all settings? These are very specific settings that allow a url to be present. I don't want to add leniency generally to all file settings.

}

private static Mode parseMode(String mode) {
Expand Down Expand Up @@ -298,6 +306,7 @@ public static FilesEntitlement build(List<Object> paths) {
String relativePathSetting = file.remove("relative_path_setting");
String modeAsString = file.remove("mode");
String platformAsString = file.remove("platform");
String ignoreUrlAsString = file.remove("ignore_url");

if (file.isEmpty() == false) {
throw new PolicyValidationException("unknown key(s) [" + file + "] in a listed file for files entitlement");
Expand All @@ -324,6 +333,14 @@ public static FilesEntitlement build(List<Object> paths) {
baseDir = parseBaseDir(relativeTo);
}

boolean ignoreUrl = false;
if (ignoreUrlAsString != null) {
if (relativePathAsString != null || pathAsString != null) {
throw new PolicyValidationException("'ignore_url' may only be used with `path_setting` or `relative_path_setting`");
}
ignoreUrl = Booleans.parseBoolean(ignoreUrlAsString);
}

final FileData fileData;
if (relativePathAsString != null) {
if (baseDir == null) {
Expand All @@ -342,12 +359,12 @@ public static FilesEntitlement build(List<Object> paths) {
}
fileData = FileData.ofPath(path, mode);
} else if (pathSetting != null) {
fileData = FileData.ofPathSetting(pathSetting, mode);
fileData = FileData.ofPathSetting(pathSetting, mode, ignoreUrl);
} else if (relativePathSetting != null) {
if (baseDir == null) {
throw new PolicyValidationException("files entitlement with a 'relative_path_setting' must specify 'relative_to'");
}
fileData = FileData.ofRelativePathSetting(relativePathSetting, baseDir, mode);
fileData = FileData.ofRelativePathSetting(relativePathSetting, baseDir, mode, ignoreUrl);
} else {
throw new AssertionError("File entry validation error");
}
Expand Down
35 changes: 30 additions & 5 deletions ...java/org/elasticsearch/entitlement/runtime/policy/entitlements/FilesEntitlementTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.util.List;
import java.util.Map;

import static org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement.BaseDir.CONFIG;
import static org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement.Mode.READ;
import static org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement.Mode.READ_WRITE;
import static org.hamcrest.Matchers.contains;
Expand Down Expand Up @@ -94,22 +95,46 @@ public void testFileDataRelativeWithEmptyDirectory() {
public void testPathSettingResolve() {
var entitlement = FilesEntitlement.build(List.of(Map.of("path_setting", "foo.bar", "mode", "read")));
var filesData = entitlement.filesData();
assertThat(filesData, contains(FileData.ofPathSetting("foo.bar", READ)));
assertThat(filesData, contains(FileData.ofPathSetting("foo.bar", READ, false)));

var fileData = FileData.ofPathSetting("foo.bar", READ);
var fileData = FileData.ofPathSetting("foo.bar", READ, false);
// empty settings
assertThat(fileData.resolvePaths(TEST_PATH_LOOKUP).toList(), empty());

fileData = FileData.ofPathSetting("foo.bar", READ);
fileData = FileData.ofPathSetting("foo.bar", READ, false);
settings = Settings.builder().put("foo.bar", "/setting/path").build();
assertThat(fileData.resolvePaths(TEST_PATH_LOOKUP).toList(), contains(Path.of("/setting/path")));

fileData = FileData.ofPathSetting("foo.*.bar", READ);
fileData = FileData.ofPathSetting("foo.*.bar", READ, false);
settings = Settings.builder().put("foo.baz.bar", "/setting/path").build();
assertThat(fileData.resolvePaths(TEST_PATH_LOOKUP).toList(), contains(Path.of("/setting/path")));

fileData = FileData.ofPathSetting("foo.*.bar", READ);
fileData = FileData.ofPathSetting("foo.*.bar", READ, false);
settings = Settings.builder().put("foo.baz.bar", "/setting/path").put("foo.baz2.bar", "/other/path").build();
assertThat(fileData.resolvePaths(TEST_PATH_LOOKUP).toList(), containsInAnyOrder(Path.of("/setting/path"), Path.of("/other/path")));
}

public void testPathSettingIgnoreUrl() {
var fileData = FileData.ofPathSetting("foo.*.bar", READ, true);
settings = Settings.builder()
.put("foo.nonurl.bar", "/setting/path")
.put("foo.url.bar", "https://mysite").build();
assertThat(fileData.resolvePaths(TEST_PATH_LOOKUP).toList(), contains(Path.of("/setting/path")));
}

public void testRelativePathSettingIgnoreUrl() {
var fileData = FileData.ofRelativePathSetting("foo.*.bar", CONFIG, READ, true);
settings = Settings.builder()
.put("foo.nonurl.bar", "path")
.put("foo.url.bar", "https://mysite").build();
assertThat(fileData.resolvePaths(TEST_PATH_LOOKUP).toList(), contains(Path.of("/config/path")));
}

public void testIgnoreUrlValidation() {
var e = expectThrows(PolicyValidationException.class, () -> FilesEntitlement.build(List.of(Map.of("path", "/foo", "mode", "read", "ignore_url", "true"))));
assertThat(e.getMessage(), is("'ignore_url' may only be used with `path_setting` or `relative_path_setting`"));

e = expectThrows(PolicyValidationException.class, () -> FilesEntitlement.build(List.of(Map.of("relative_path", "foo", "relative_to", "config", "mode", "read", "ignore_url", "true"))));
assertThat(e.getMessage(), is("'ignore_url' may only be used with `path_setting` or `relative_path_setting`"));
}
}