Skip to content

Commit d5fb604

Browse files
authored
Restrict direct use of ApplicationPrivilege constructor (#91176)
The canonical way to construct application privileges is to use ApplicationPrivilege::get which accounts for stored privilege look-up. This PR restricts and reduces the direct usage of the ApplicationPrivilege constructor, to enforce this. To ease testing, the PR introduces a new utility function that matches the constructor's signature.
1 parent defa765 commit d5fb604

File tree

9 files changed

+177
-70
lines changed

9 files changed

+177
-70
lines changed

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilege.java

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,16 +43,13 @@ public final class ApplicationPrivilege extends Privilege {
4343
*/
4444
private static final Pattern VALID_NAME_OR_ACTION = Pattern.compile("^\\p{Graph}*$");
4545

46-
public static final Function<String, ApplicationPrivilege> NONE = app -> new ApplicationPrivilege(app, "none", new String[0]);
46+
public static final Function<String, ApplicationPrivilege> NONE = app -> new ApplicationPrivilege(app, Collections.singleton("none"));
4747

4848
private final String application;
4949
private final String[] patterns;
5050

51-
public ApplicationPrivilege(String application, String privilegeName, String... patterns) {
52-
this(application, Collections.singleton(privilegeName), patterns);
53-
}
54-
55-
public ApplicationPrivilege(String application, Set<String> name, String... patterns) {
51+
// TODO make this private once ApplicationPrivilegeTests::createPrivilege uses ApplicationPrivilege::get
52+
ApplicationPrivilege(String application, Set<String> name, String... patterns) {
5653
super(name, patterns);
5754
this.application = application;
5855
this.patterns = patterns;

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/ApplicationPermissionTests.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import org.elasticsearch.test.ESTestCase;
1212
import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilege;
1313
import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilegeDescriptor;
14+
import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilegeTests;
1415

1516
import java.util.ArrayList;
1617
import java.util.Arrays;
@@ -40,7 +41,7 @@ public class ApplicationPermissionTests extends ESTestCase {
4041

4142
private ApplicationPrivilege storePrivilege(String app, String name, String... patterns) {
4243
store.add(new ApplicationPrivilegeDescriptor(app, name, Sets.newHashSet(patterns), Collections.emptyMap()));
43-
return new ApplicationPrivilege(app, name, patterns);
44+
return ApplicationPrivilegeTests.createPrivilege(app, name, patterns);
4445
}
4546

4647
public void testCheckSimplePermission() {

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/LimitedRoleTests.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import org.elasticsearch.xpack.core.security.authz.accesscontrol.IndicesAccessControl;
2929
import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilege;
3030
import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilegeDescriptor;
31+
import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilegeTests;
3132
import org.elasticsearch.xpack.core.security.authz.privilege.ClusterPrivilegeResolver;
3233
import org.elasticsearch.xpack.core.security.authz.privilege.IndexPrivilege;
3334
import org.elasticsearch.xpack.core.security.support.Automatons;
@@ -873,7 +874,7 @@ private ApplicationPrivilege defineApplicationPrivilege(String app, String name,
873874
applicationPrivilegeDescriptors.add(
874875
new ApplicationPrivilegeDescriptor(app, name, Sets.newHashSet(actions), Collections.emptyMap())
875876
);
876-
return new ApplicationPrivilege(app, name, actions);
877+
return ApplicationPrivilegeTests.createPrivilege(app, name, actions);
877878
}
878879

879880
private static MapBuilder<String, ResourcePrivileges> mapBuilder() {

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/SimpleRoleTests.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@
1111
import org.elasticsearch.test.ESTestCase;
1212
import org.elasticsearch.xpack.core.security.authz.AuthorizationEngine;
1313
import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
14-
import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilege;
1514
import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilegeDescriptor;
15+
import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilegeTests;
1616

1717
import java.util.Arrays;
1818
import java.util.List;
@@ -102,7 +102,7 @@ public void testBuildFromRoleDescriptorWithApplicationPrivileges() {
102102
"expected grant for role with application privilege to be: " + applicationPrivilege,
103103
role.application()
104104
.grants(
105-
new ApplicationPrivilege(
105+
ApplicationPrivilegeTests.createPrivilege(
106106
wildcardApplication ? randomAlphaOfLengthBetween(1, 10) : applicationPrivilege.getApplication(),
107107
wildcardPrivileges ? Set.of(randomAlphaOfLengthBetween(1, 10)) : Set.of(applicationPrivilege.getPrivileges()),
108108
wildcardPrivileges ? randomAlphaOfLengthBetween(1, 10) : allowedApplicationActionPattern
@@ -117,7 +117,7 @@ public void testBuildFromRoleDescriptorWithApplicationPrivileges() {
117117
"expected grant for role with application privilege to be: " + applicationPrivilege,
118118
role.application()
119119
.grants(
120-
new ApplicationPrivilege(
120+
ApplicationPrivilegeTests.createPrivilege(
121121
false == wildcardApplication
122122
? randomValueOtherThan(applicationPrivilege.getApplication(), () -> randomAlphaOfLengthBetween(1, 10))
123123
: randomAlphaOfLengthBetween(1, 10),

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilegeTests.java

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,7 @@
1919

2020
import java.util.Arrays;
2121
import java.util.Collections;
22-
import java.util.HashMap;
2322
import java.util.Locale;
24-
import java.util.Map;
2523
import java.util.Set;
2624
import java.util.function.Supplier;
2725

@@ -35,6 +33,19 @@
3533

3634
public class ApplicationPrivilegeTests extends ESTestCase {
3735

36+
public static ApplicationPrivilege createPrivilege(final String applicationName, final String privilegeName, final String... patterns) {
37+
return createPrivilege(applicationName, Collections.singleton(privilegeName), patterns);
38+
}
39+
40+
public static ApplicationPrivilege createPrivilege(
41+
final String applicationName,
42+
final Set<String> privilegeNames,
43+
final String... patterns
44+
) {
45+
// TODO rewrite this to use `ApplicationPrivilege.get()`
46+
return new ApplicationPrivilege(applicationName, privilegeNames, patterns);
47+
}
48+
3849
public void testValidationOfApplicationName() {
3950
final String specialCharacters = ":;$#%()+='.{}[]!@^&'";
4051
final Supplier<Character> specialCharacter = () -> specialCharacters.charAt(randomInt(specialCharacters.length() - 1));
@@ -203,7 +214,7 @@ public void testEqualsAndHashCode() {
203214
final EqualsHashCodeTestUtils.MutateFunction<ApplicationPrivilege> mutate = randomFrom(
204215
orig -> createPrivilege("x" + orig.getApplication(), getPrivilegeName(orig), orig.getPatterns()),
205216
orig -> createPrivilege(orig.getApplication(), "x" + getPrivilegeName(orig), orig.getPatterns()),
206-
orig -> new ApplicationPrivilege(orig.getApplication(), getPrivilegeName(orig), "*")
217+
orig -> createPrivilege(orig.getApplication(), getPrivilegeName(orig), "*")
207218
);
208219
EqualsHashCodeTestUtils.checkEqualsAndHashCode(
209220
privilege,
@@ -212,10 +223,6 @@ public void testEqualsAndHashCode() {
212223
);
213224
}
214225

215-
private ApplicationPrivilege createPrivilege(String applicationName, String privilegeName, String... patterns) {
216-
return new ApplicationPrivilege(applicationName, privilegeName, patterns);
217-
}
218-
219226
private String getPrivilegeName(ApplicationPrivilege privilege) {
220227
if (privilege.name.size() == 1) {
221228
return privilege.name.iterator().next();
@@ -257,10 +264,6 @@ private ApplicationPrivilege randomPrivilege() {
257264
patterns[i] = randomAlphaOfLengthBetween(2, 5) + "/" + suffix;
258265
}
259266

260-
final Map<String, Object> metadata = new HashMap<>();
261-
for (int i = randomInt(3); i > 0; i--) {
262-
metadata.put(randomAlphaOfLengthBetween(2, 5), randomFrom(randomBoolean(), randomInt(10), randomAlphaOfLength(5)));
263-
}
264267
return createPrivilege(applicationName, privilegeName, patterns);
265268
}
266269

0 commit comments

Comments
 (0)