Skip to content

Commit 7383ea8

Browse files
committed
fix error messages for parsing types
1 parent f0b4136 commit 7383ea8

File tree

2 files changed

+52
-21
lines changed

2 files changed

+52
-21
lines changed

libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/entitlements/FilesEntitlement.java

Lines changed: 50 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99

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

12-
import org.elasticsearch.core.Booleans;
1312
import org.elasticsearch.entitlement.runtime.policy.ExternalEntitlement;
1413
import org.elasticsearch.entitlement.runtime.policy.PathLookup;
1514
import org.elasticsearch.entitlement.runtime.policy.PolicyValidationException;
@@ -21,6 +20,7 @@
2120
import java.util.Locale;
2221
import java.util.Map;
2322
import java.util.Objects;
23+
import java.util.function.BiFunction;
2424
import java.util.stream.Stream;
2525

2626
import static java.lang.Character.isLetter;
@@ -330,18 +330,53 @@ public static FilesEntitlement build(List<Object> paths) {
330330
if (paths == null || paths.isEmpty()) {
331331
throw new PolicyValidationException("must specify at least one path");
332332
}
333+
BiFunction<Map<String, Object>, String, String> checkString = (values, key) -> {
334+
Object value = values.remove(key);
335+
if (value == null) {
336+
return null;
337+
} else if (value instanceof String str) {
338+
return str;
339+
}
340+
throw new PolicyValidationException(
341+
"expected ["
342+
+ key
343+
+ "] to be type ["
344+
+ String.class.getSimpleName()
345+
+ "] but found type ["
346+
+ value.getClass().getSimpleName()
347+
+ "]"
348+
);
349+
};
350+
BiFunction<Map<String, Object>, String, Boolean> checkBoolean = (values, key) -> {
351+
Object value = values.remove(key);
352+
if (value == null) {
353+
return null;
354+
} else if (value instanceof Boolean bool) {
355+
return bool;
356+
}
357+
throw new PolicyValidationException(
358+
"expected ["
359+
+ key
360+
+ "] to be type ["
361+
+ boolean.class.getSimpleName()
362+
+ "] but found type ["
363+
+ value.getClass().getSimpleName()
364+
+ "]"
365+
);
366+
};
333367
List<FileData> filesData = new ArrayList<>();
334368
for (Object object : paths) {
335-
Map<String, Object> file = new HashMap<>((Map<String, String>) object);
336-
String pathAsString = (String) file.remove("path");
337-
String relativePathAsString = (String) file.remove("relative_path");
338-
String relativeTo = (String) file.remove("relative_to");
339-
String pathSetting = (String) file.remove("path_setting");
340-
String relativePathSetting = (String) file.remove("relative_path_setting");
341-
String modeAsString = (String) file.remove("mode");
342-
String platformAsString = (String) file.remove("platform");
343-
String ignoreUrlAsString = (String) file.remove("ignore_url");
344-
Boolean exclusiveBoolean = (Boolean) file.remove("exclusive");
369+
Map<String, Object> file = new HashMap<>((Map<String, Object>) object);
370+
String pathAsString = checkString.apply(file, "path");
371+
String relativePathAsString = checkString.apply(file, "relative_path");
372+
String relativeTo = checkString.apply(file, "relative_to");
373+
String pathSetting = checkString.apply(file, "path_setting");
374+
String relativePathSetting = checkString.apply(file, "relative_path_setting");
375+
String modeAsString = checkString.apply(file, "mode");
376+
String platformAsString = checkString.apply(file, "platform");
377+
Boolean ignoreUrlAsStringBoolean = checkBoolean.apply(file, "ignore_url");
378+
boolean ignoreUrlAsString = ignoreUrlAsStringBoolean != null && ignoreUrlAsStringBoolean;
379+
Boolean exclusiveBoolean = checkBoolean.apply(file, "exclusive");
345380
boolean exclusive = exclusiveBoolean != null && exclusiveBoolean;
346381

347382
if (file.isEmpty() == false) {
@@ -369,12 +404,8 @@ public static FilesEntitlement build(List<Object> paths) {
369404
baseDir = parseBaseDir(relativeTo);
370405
}
371406

372-
boolean ignoreUrl = false;
373-
if (ignoreUrlAsString != null) {
374-
if (relativePathAsString != null || pathAsString != null) {
375-
throw new PolicyValidationException("'ignore_url' may only be used with `path_setting` or `relative_path_setting`");
376-
}
377-
ignoreUrl = Booleans.parseBoolean(ignoreUrlAsString);
407+
if (ignoreUrlAsStringBoolean != null && (relativePathAsString != null || pathAsString != null)) {
408+
throw new PolicyValidationException("'ignore_url' may only be used with `path_setting` or `relative_path_setting`");
378409
}
379410

380411
final FileData fileData;
@@ -394,12 +425,12 @@ public static FilesEntitlement build(List<Object> paths) {
394425
}
395426
fileData = FileData.ofPath(path, mode);
396427
} else if (pathSetting != null) {
397-
fileData = FileData.ofPathSetting(pathSetting, mode, ignoreUrl);
428+
fileData = FileData.ofPathSetting(pathSetting, mode, ignoreUrlAsString);
398429
} else if (relativePathSetting != null) {
399430
if (baseDir == null) {
400431
throw new PolicyValidationException("files entitlement with a 'relative_path_setting' must specify 'relative_to'");
401432
}
402-
fileData = FileData.ofRelativePathSetting(relativePathSetting, baseDir, mode, ignoreUrl);
433+
fileData = FileData.ofRelativePathSetting(relativePathSetting, baseDir, mode, ignoreUrlAsString);
403434
} else {
404435
throw new AssertionError("File entry validation error");
405436
}

libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/entitlements/FilesEntitlementTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,14 +154,14 @@ public void testRelativePathSettingIgnoreUrl() {
154154
public void testIgnoreUrlValidation() {
155155
var e = expectThrows(
156156
PolicyValidationException.class,
157-
() -> FilesEntitlement.build(List.of(Map.of("path", "/foo", "mode", "read", "ignore_url", "true")))
157+
() -> FilesEntitlement.build(List.of(Map.of("path", "/foo", "mode", "read", "ignore_url", true)))
158158
);
159159
assertThat(e.getMessage(), is("'ignore_url' may only be used with `path_setting` or `relative_path_setting`"));
160160

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

0 commit comments

Comments
 (0)