From 53dbe654df7fe098c02422fd61cd2702d51a8c3d Mon Sep 17 00:00:00 2001 From: Nikolaj Volgushev Date: Fri, 28 Oct 2022 14:17:54 +0200 Subject: [PATCH 1/6] Lock down application privilege constructor --- .../authz/privilege/ApplicationPrivilege.java | 13 +- .../ApplicationPermissionTests.java | 3 +- .../authz/permission/LimitedRoleTests.java | 3 +- .../authz/permission/SimpleRoleTests.java | 6 +- .../privilege/ApplicationPrivilegeTests.java | 14 +- .../authz/store/ReservedRolesStoreTests.java | 174 ++++++++++++++---- .../service/ElasticServiceAccountsTests.java | 10 +- .../xpack/security/authz/RBACEngineTests.java | 5 +- .../authz/store/CompositeRolesStoreTests.java | 12 +- 9 files changed, 175 insertions(+), 65 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilege.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilege.java index c5ac48492c0dd..4a1e13ccd925d 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilege.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilege.java @@ -43,16 +43,15 @@ public final class ApplicationPrivilege extends Privilege { */ private static final Pattern VALID_NAME_OR_ACTION = Pattern.compile("^\\p{Graph}*$"); - public static final Function NONE = app -> new ApplicationPrivilege(app, "none", new String[0]); + public static final Function NONE = app -> new ApplicationPrivilege(app, Collections.singleton("none")); private final String application; private final String[] patterns; - public ApplicationPrivilege(String application, String privilegeName, String... patterns) { - this(application, Collections.singleton(privilegeName), patterns); - } - - public ApplicationPrivilege(String application, Set name, String... patterns) { + /** + * Package-private for tests. Use {@link #get(String, Set, Collection)} to construct application privileges. + */ + ApplicationPrivilege(String application, Set name, String... patterns) { super(name, patterns); this.application = application; this.patterns = patterns; @@ -253,7 +252,7 @@ private static ApplicationPrivilege resolve(String application, Set name } patterns.addAll(actions); - return new ApplicationPrivilege(application, names, patterns.toArray(new String[patterns.size()])); + return new ApplicationPrivilege(application, names, patterns.toArray(new String[0])); } @Override diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/ApplicationPermissionTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/ApplicationPermissionTests.java index 729f2dc2e148e..997c2274b1261 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/ApplicationPermissionTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/ApplicationPermissionTests.java @@ -11,6 +11,7 @@ import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilege; import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilegeDescriptor; +import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilegeTests; import java.util.ArrayList; import java.util.Arrays; @@ -40,7 +41,7 @@ public class ApplicationPermissionTests extends ESTestCase { private ApplicationPrivilege storePrivilege(String app, String name, String... patterns) { store.add(new ApplicationPrivilegeDescriptor(app, name, Sets.newHashSet(patterns), Collections.emptyMap())); - return new ApplicationPrivilege(app, name, patterns); + return ApplicationPrivilegeTests.createPrivilege(app, name, patterns); } public void testCheckSimplePermission() { diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/LimitedRoleTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/LimitedRoleTests.java index b6009da014b4c..1bf6ef61b3a1e 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/LimitedRoleTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/LimitedRoleTests.java @@ -28,6 +28,7 @@ import org.elasticsearch.xpack.core.security.authz.accesscontrol.IndicesAccessControl; import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilege; import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilegeDescriptor; +import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilegeTests; import org.elasticsearch.xpack.core.security.authz.privilege.ClusterPrivilegeResolver; import org.elasticsearch.xpack.core.security.authz.privilege.IndexPrivilege; import org.elasticsearch.xpack.core.security.support.Automatons; @@ -873,7 +874,7 @@ private ApplicationPrivilege defineApplicationPrivilege(String app, String name, applicationPrivilegeDescriptors.add( new ApplicationPrivilegeDescriptor(app, name, Sets.newHashSet(actions), Collections.emptyMap()) ); - return new ApplicationPrivilege(app, name, actions); + return ApplicationPrivilegeTests.createPrivilege(app, name, actions); } private static MapBuilder mapBuilder() { diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/SimpleRoleTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/SimpleRoleTests.java index b6c9ffd8fb4bf..fdf0af7916d2b 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/SimpleRoleTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/SimpleRoleTests.java @@ -11,8 +11,8 @@ import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.core.security.authz.AuthorizationEngine; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; -import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilege; import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilegeDescriptor; +import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilegeTests; import java.util.Arrays; import java.util.List; @@ -102,7 +102,7 @@ public void testBuildFromRoleDescriptorWithApplicationPrivileges() { "expected grant for role with application privilege to be: " + applicationPrivilege, role.application() .grants( - new ApplicationPrivilege( + ApplicationPrivilegeTests.createPrivilege( wildcardApplication ? randomAlphaOfLengthBetween(1, 10) : applicationPrivilege.getApplication(), wildcardPrivileges ? Set.of(randomAlphaOfLengthBetween(1, 10)) : Set.of(applicationPrivilege.getPrivileges()), wildcardPrivileges ? randomAlphaOfLengthBetween(1, 10) : allowedApplicationActionPattern @@ -117,7 +117,7 @@ public void testBuildFromRoleDescriptorWithApplicationPrivileges() { "expected grant for role with application privilege to be: " + applicationPrivilege, role.application() .grants( - new ApplicationPrivilege( + ApplicationPrivilegeTests.createPrivilege( false == wildcardApplication ? randomValueOtherThan(applicationPrivilege.getApplication(), () -> randomAlphaOfLengthBetween(1, 10)) : randomAlphaOfLengthBetween(1, 10), diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilegeTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilegeTests.java index f091230dc71c7..e60bd2afdcd7a 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilegeTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilegeTests.java @@ -35,6 +35,14 @@ public class ApplicationPrivilegeTests extends ESTestCase { + public static ApplicationPrivilege createPrivilege(String applicationName, String privilegeName, String... patterns) { + return createPrivilege(applicationName, Collections.singleton(privilegeName), patterns); + } + + public static ApplicationPrivilege createPrivilege(String application, Set name, String... patterns) { + return new ApplicationPrivilege(application, name, patterns); + } + public void testValidationOfApplicationName() { final String specialCharacters = ":;$#%()+='.{}[]!@^&'"; final Supplier specialCharacter = () -> specialCharacters.charAt(randomInt(specialCharacters.length() - 1)); @@ -203,7 +211,7 @@ public void testEqualsAndHashCode() { final EqualsHashCodeTestUtils.MutateFunction mutate = randomFrom( orig -> createPrivilege("x" + orig.getApplication(), getPrivilegeName(orig), orig.getPatterns()), orig -> createPrivilege(orig.getApplication(), "x" + getPrivilegeName(orig), orig.getPatterns()), - orig -> new ApplicationPrivilege(orig.getApplication(), getPrivilegeName(orig), "*") + orig -> createPrivilege(orig.getApplication(), getPrivilegeName(orig), "*") ); EqualsHashCodeTestUtils.checkEqualsAndHashCode( privilege, @@ -212,10 +220,6 @@ public void testEqualsAndHashCode() { ); } - private ApplicationPrivilege createPrivilege(String applicationName, String privilegeName, String... patterns) { - return new ApplicationPrivilege(applicationName, privilegeName, patterns); - } - private String getPrivilegeName(ApplicationPrivilege privilege) { if (privilege.name.size() == 1) { return privilege.name.iterator().next(); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStoreTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStoreTests.java index 7ba4281d0994b..8c562e267a62c 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStoreTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStoreTests.java @@ -181,8 +181,8 @@ import org.elasticsearch.xpack.core.security.authz.accesscontrol.IndicesAccessControl; import org.elasticsearch.xpack.core.security.authz.permission.FieldPermissionsCache; import org.elasticsearch.xpack.core.security.authz.permission.Role; -import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilege; import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilegeDescriptor; +import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilegeTests; import org.elasticsearch.xpack.core.security.test.TestRestrictedIndices; import org.elasticsearch.xpack.core.security.user.APMSystemUser; import org.elasticsearch.xpack.core.security.user.BeatsSystemUser; @@ -1169,18 +1169,26 @@ public void testKibanaAdminRole() { ); final String randomApplication = "kibana-" + randomAlphaOfLengthBetween(8, 24); - assertThat(kibanaAdminRole.application().grants(new ApplicationPrivilege(randomApplication, "app-random", "all"), "*"), is(false)); + assertThat( + kibanaAdminRole.application().grants(ApplicationPrivilegeTests.createPrivilege(randomApplication, "app-random", "all"), "*"), + is(false) + ); final String application = "kibana-.kibana"; - assertThat(kibanaAdminRole.application().grants(new ApplicationPrivilege(application, "app-foo", "foo"), "*"), is(false)); assertThat( - kibanaAdminRole.application().grants(new ApplicationPrivilege(application, "app-all", allowedApplicationActionPattern), "*"), + kibanaAdminRole.application().grants(ApplicationPrivilegeTests.createPrivilege(application, "app-foo", "foo"), "*"), + is(false) + ); + assertThat( + kibanaAdminRole.application() + .grants(ApplicationPrivilegeTests.createPrivilege(application, "app-all", allowedApplicationActionPattern), "*"), is(true) ); final String applicationWithRandomIndex = "kibana-.kibana_" + randomAlphaOfLengthBetween(8, 24); assertThat( - kibanaAdminRole.application().grants(new ApplicationPrivilege(applicationWithRandomIndex, "app-random-index", "all"), "*"), + kibanaAdminRole.application() + .grants(ApplicationPrivilegeTests.createPrivilege(applicationWithRandomIndex, "app-random-index", "all"), "*"), is(false) ); @@ -1224,21 +1232,32 @@ public void testKibanaUserRole() { final String randomApplication = "kibana-" + randomAlphaOfLengthBetween(8, 24); assertThat( kibanaUserRole.application() - .grants(new ApplicationPrivilege(randomApplication, "app-random", allowedApplicationActionPattern), "*"), + .grants(ApplicationPrivilegeTests.createPrivilege(randomApplication, "app-random", allowedApplicationActionPattern), "*"), is(false) ); final String application = "kibana-.kibana"; - assertThat(kibanaUserRole.application().grants(new ApplicationPrivilege(application, "app-foo", "foo"), "*"), is(false)); assertThat( - kibanaUserRole.application().grants(new ApplicationPrivilege(application, "app-all", allowedApplicationActionPattern), "*"), + kibanaUserRole.application().grants(ApplicationPrivilegeTests.createPrivilege(application, "app-foo", "foo"), "*"), + is(false) + ); + assertThat( + kibanaUserRole.application() + .grants(ApplicationPrivilegeTests.createPrivilege(application, "app-all", allowedApplicationActionPattern), "*"), is(true) ); final String applicationWithRandomIndex = "kibana-.kibana_" + randomAlphaOfLengthBetween(8, 24); assertThat( kibanaUserRole.application() - .grants(new ApplicationPrivilege(applicationWithRandomIndex, "app-random-index", allowedApplicationActionPattern), "*"), + .grants( + ApplicationPrivilegeTests.createPrivilege( + applicationWithRandomIndex, + "app-random-index", + allowedApplicationActionPattern + ), + "*" + ), is(false) ); @@ -1330,23 +1349,34 @@ public void testMonitoringUserRole() { assertNoAccessAllowed(monitoringUserRole, XPackPlugin.ASYNC_RESULTS_INDEX + randomAlphaOfLengthBetween(0, 2)); assertThat( - monitoringUserRole.application().grants(new ApplicationPrivilege(kibanaApplicationWithRandomIndex, "app-foo", "foo"), "*"), + monitoringUserRole.application() + .grants(ApplicationPrivilegeTests.createPrivilege(kibanaApplicationWithRandomIndex, "app-foo", "foo"), "*"), is(false) ); assertThat( monitoringUserRole.application() .grants( - new ApplicationPrivilege(kibanaApplicationWithRandomIndex, "app-reserved_monitoring", allowedApplicationActionPattern), + ApplicationPrivilegeTests.createPrivilege( + kibanaApplicationWithRandomIndex, + "app-reserved_monitoring", + allowedApplicationActionPattern + ), "*" ), is(true) ); final String otherApplication = "logstash-" + randomAlphaOfLengthBetween(8, 24); - assertThat(monitoringUserRole.application().grants(new ApplicationPrivilege(otherApplication, "app-foo", "foo"), "*"), is(false)); + assertThat( + monitoringUserRole.application().grants(ApplicationPrivilegeTests.createPrivilege(otherApplication, "app-foo", "foo"), "*"), + is(false) + ); assertThat( monitoringUserRole.application() - .grants(new ApplicationPrivilege(otherApplication, "app-reserved_monitoring", allowedApplicationActionPattern), "*"), + .grants( + ApplicationPrivilegeTests.createPrivilege(otherApplication, "app-reserved_monitoring", allowedApplicationActionPattern), + "*" + ), is(false) ); } @@ -2213,21 +2243,38 @@ public void testAPMUserRole() { assertOnlyReadAllowed(role, "observability-annotations"); - assertThat(role.application().grants(new ApplicationPrivilege(kibanaApplicationWithRandomIndex, "app-foo", "foo"), "*"), is(false)); + assertThat( + role.application().grants(ApplicationPrivilegeTests.createPrivilege(kibanaApplicationWithRandomIndex, "app-foo", "foo"), "*"), + is(false) + ); assertThat( role.application() .grants( - new ApplicationPrivilege(kibanaApplicationWithRandomIndex, "app-reserved_ml_apm_user", allowedApplicationActionPattern), + ApplicationPrivilegeTests.createPrivilege( + kibanaApplicationWithRandomIndex, + "app-reserved_ml_apm_user", + allowedApplicationActionPattern + ), "*" ), is(true) ); final String otherApplication = "logstash-" + randomAlphaOfLengthBetween(8, 24); - assertThat(role.application().grants(new ApplicationPrivilege(otherApplication, "app-foo", "foo"), "*"), is(false)); + assertThat( + role.application().grants(ApplicationPrivilegeTests.createPrivilege(otherApplication, "app-foo", "foo"), "*"), + is(false) + ); assertThat( role.application() - .grants(new ApplicationPrivilege(otherApplication, "app-reserved_ml_apm_user", allowedApplicationActionPattern), "*"), + .grants( + ApplicationPrivilegeTests.createPrivilege( + otherApplication, + "app-reserved_ml_apm_user", + allowedApplicationActionPattern + ), + "*" + ), is(false) ); } @@ -2271,20 +2318,34 @@ public void testMachineLearningAdminRole() { assertNoAccessAllowed(role, TestRestrictedIndices.SAMPLE_RESTRICTED_NAMES); assertNoAccessAllowed(role, XPackPlugin.ASYNC_RESULTS_INDEX + randomAlphaOfLengthBetween(0, 2)); - assertThat(role.application().grants(new ApplicationPrivilege(kibanaApplicationWithRandomIndex, "app-foo", "foo"), "*"), is(false)); + assertThat( + role.application().grants(ApplicationPrivilegeTests.createPrivilege(kibanaApplicationWithRandomIndex, "app-foo", "foo"), "*"), + is(false) + ); assertThat( role.application() .grants( - new ApplicationPrivilege(kibanaApplicationWithRandomIndex, "app-reserved_ml", allowedApplicationActionPattern), + ApplicationPrivilegeTests.createPrivilege( + kibanaApplicationWithRandomIndex, + "app-reserved_ml", + allowedApplicationActionPattern + ), "*" ), is(true) ); final String otherApplication = "logstash-" + randomAlphaOfLengthBetween(8, 24); - assertThat(role.application().grants(new ApplicationPrivilege(otherApplication, "app-foo", "foo"), "*"), is(false)); assertThat( - role.application().grants(new ApplicationPrivilege(otherApplication, "app-reserved_ml", allowedApplicationActionPattern), "*"), + role.application().grants(ApplicationPrivilegeTests.createPrivilege(otherApplication, "app-foo", "foo"), "*"), + is(false) + ); + assertThat( + role.application() + .grants( + ApplicationPrivilegeTests.createPrivilege(otherApplication, "app-reserved_ml", allowedApplicationActionPattern), + "*" + ), is(false) ); } @@ -2450,20 +2511,34 @@ public void testMachineLearningUserRole() { assertNoAccessAllowed(role, TestRestrictedIndices.SAMPLE_RESTRICTED_NAMES); assertNoAccessAllowed(role, XPackPlugin.ASYNC_RESULTS_INDEX + randomAlphaOfLengthBetween(0, 2)); - assertThat(role.application().grants(new ApplicationPrivilege(kibanaApplicationWithRandomIndex, "app-foo", "foo"), "*"), is(false)); + assertThat( + role.application().grants(ApplicationPrivilegeTests.createPrivilege(kibanaApplicationWithRandomIndex, "app-foo", "foo"), "*"), + is(false) + ); assertThat( role.application() .grants( - new ApplicationPrivilege(kibanaApplicationWithRandomIndex, "app-reserved_ml", allowedApplicationActionPattern), + ApplicationPrivilegeTests.createPrivilege( + kibanaApplicationWithRandomIndex, + "app-reserved_ml", + allowedApplicationActionPattern + ), "*" ), is(true) ); final String otherApplication = "logstash-" + randomAlphaOfLengthBetween(8, 24); - assertThat(role.application().grants(new ApplicationPrivilege(otherApplication, "app-foo", "foo"), "*"), is(false)); assertThat( - role.application().grants(new ApplicationPrivilege(otherApplication, "app-reserved_ml", allowedApplicationActionPattern), "*"), + role.application().grants(ApplicationPrivilegeTests.createPrivilege(otherApplication, "app-foo", "foo"), "*"), + is(false) + ); + assertThat( + role.application() + .grants( + ApplicationPrivilegeTests.createPrivilege(otherApplication, "app-reserved_ml", allowedApplicationActionPattern), + "*" + ), is(false) ); } @@ -2519,7 +2594,8 @@ public void testTransformAdminRole() { assertNoAccessAllowed(role, XPackPlugin.ASYNC_RESULTS_INDEX + randomAlphaOfLengthBetween(0, 2)); assertThat( - role.application().grants(new ApplicationPrivilege(kibanaApplicationWithRandomIndex, "app-foo", "foo"), "*"), + role.application() + .grants(ApplicationPrivilegeTests.createPrivilege(kibanaApplicationWithRandomIndex, "app-foo", "foo"), "*"), is(false) ); @@ -2527,7 +2603,11 @@ public void testTransformAdminRole() { assertThat( role.application() .grants( - new ApplicationPrivilege(kibanaApplicationWithRandomIndex, "app-reserved_ml", allowedApplicationActionPattern), + ApplicationPrivilegeTests.createPrivilege( + kibanaApplicationWithRandomIndex, + "app-reserved_ml", + allowedApplicationActionPattern + ), "*" ), is(true) @@ -2535,11 +2615,17 @@ public void testTransformAdminRole() { } final String otherApplication = "logstash-" + randomAlphaOfLengthBetween(8, 24); - assertThat(role.application().grants(new ApplicationPrivilege(otherApplication, "app-foo", "foo"), "*"), is(false)); + assertThat( + role.application().grants(ApplicationPrivilegeTests.createPrivilege(otherApplication, "app-foo", "foo"), "*"), + is(false) + ); if (roleDescriptor.getName().equals("data_frame_transforms_admin")) { assertThat( role.application() - .grants(new ApplicationPrivilege(otherApplication, "app-reserved_ml", allowedApplicationActionPattern), "*"), + .grants( + ApplicationPrivilegeTests.createPrivilege(otherApplication, "app-reserved_ml", allowedApplicationActionPattern), + "*" + ), is(false) ); } @@ -2602,7 +2688,8 @@ public void testTransformUserRole() { assertNoAccessAllowed(role, XPackPlugin.ASYNC_RESULTS_INDEX + randomAlphaOfLengthBetween(0, 2)); assertThat( - role.application().grants(new ApplicationPrivilege(kibanaApplicationWithRandomIndex, "app-foo", "foo"), "*"), + role.application() + .grants(ApplicationPrivilegeTests.createPrivilege(kibanaApplicationWithRandomIndex, "app-foo", "foo"), "*"), is(false) ); @@ -2610,7 +2697,11 @@ public void testTransformUserRole() { assertThat( role.application() .grants( - new ApplicationPrivilege(kibanaApplicationWithRandomIndex, "app-reserved_ml", allowedApplicationActionPattern), + ApplicationPrivilegeTests.createPrivilege( + kibanaApplicationWithRandomIndex, + "app-reserved_ml", + allowedApplicationActionPattern + ), "*" ), is(true) @@ -2618,11 +2709,17 @@ public void testTransformUserRole() { } final String otherApplication = "logstash-" + randomAlphaOfLengthBetween(8, 24); - assertThat(role.application().grants(new ApplicationPrivilege(otherApplication, "app-foo", "foo"), "*"), is(false)); + assertThat( + role.application().grants(ApplicationPrivilegeTests.createPrivilege(otherApplication, "app-foo", "foo"), "*"), + is(false) + ); if (roleDescriptor.getName().equals("data_frame_transforms_user")) { assertThat( role.application() - .grants(new ApplicationPrivilege(otherApplication, "app-reserved_ml", allowedApplicationActionPattern), "*"), + .grants( + ApplicationPrivilegeTests.createPrivilege(otherApplication, "app-reserved_ml", allowedApplicationActionPattern), + "*" + ), is(false) ); } @@ -2754,10 +2851,14 @@ public void testPredefinedViewerRole() { assertNoAccessAllowed(role, "ilm-history-" + randomIntBetween(0, 5)); // Check application privileges assertThat( - role.application().grants(new ApplicationPrivilege("kibana-.kibana", "kibana-read", allowedApplicationActionPattern), "*"), + role.application() + .grants(ApplicationPrivilegeTests.createPrivilege("kibana-.kibana", "kibana-read", allowedApplicationActionPattern), "*"), is(true) ); - assertThat(role.application().grants(new ApplicationPrivilege("kibana-.kibana", "kibana-all", "all"), "*"), is(false)); + assertThat( + role.application().grants(ApplicationPrivilegeTests.createPrivilege("kibana-.kibana", "kibana-all", "all"), "*"), + is(false) + ); assertThat(role.runAs().check(randomAlphaOfLengthBetween(1, 20)), is(false)); } @@ -2827,7 +2928,8 @@ public void testPredefinedEditorRole() { // Check application privileges assertThat( - role.application().grants(new ApplicationPrivilege("kibana-.kibana", "kibana-all", allowedApplicationActionPattern), "*"), + role.application() + .grants(ApplicationPrivilegeTests.createPrivilege("kibana-.kibana", "kibana-all", allowedApplicationActionPattern), "*"), is(true) ); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/service/ElasticServiceAccountsTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/service/ElasticServiceAccountsTests.java index 70906a7f8683d..27a8d383a4203 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/service/ElasticServiceAccountsTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/service/ElasticServiceAccountsTests.java @@ -110,8 +110,8 @@ import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; import org.elasticsearch.xpack.core.security.authz.permission.FieldPermissionsCache; import org.elasticsearch.xpack.core.security.authz.permission.Role; -import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilege; import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilegeDescriptor; +import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilegeTests; import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore; import org.elasticsearch.xpack.core.security.user.KibanaSystemUser; import org.elasticsearch.xpack.core.security.user.User; @@ -250,7 +250,8 @@ public void testElasticFleetServerPrivileges() { final String privilegeName = randomAlphaOfLengthBetween(3, 16); assertThat( - role.application().grants(new ApplicationPrivilege(kibanaApplication, privilegeName, allowedApplicationActionPattern), "*"), + role.application() + .grants(ApplicationPrivilegeTests.createPrivilege(kibanaApplication, privilegeName, allowedApplicationActionPattern), "*"), is(true) ); @@ -258,14 +259,15 @@ public void testElasticFleetServerPrivileges() { + "-" + randomAlphaOfLengthBetween(8, 24); assertThat( - role.application().grants(new ApplicationPrivilege(otherApplication, privilegeName, allowedApplicationActionPattern), "*"), + role.application() + .grants(ApplicationPrivilegeTests.createPrivilege(otherApplication, privilegeName, allowedApplicationActionPattern), "*"), is(false) ); assertThat( role.application() .grants( - new ApplicationPrivilege( + ApplicationPrivilegeTests.createPrivilege( kibanaApplication, privilegeName, randomArray(1, 5, String[]::new, () -> randomAlphaOfLengthBetween(3, 16)) diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java index 1d7212e4eff33..d8e8b558db928 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java @@ -72,6 +72,7 @@ import org.elasticsearch.xpack.core.security.authz.permission.SimpleRole; import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilege; import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilegeDescriptor; +import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilegeTests; import org.elasticsearch.xpack.core.security.authz.privilege.ConfigurableClusterPrivileges.ManageApplicationPrivileges; import org.elasticsearch.xpack.core.security.authz.privilege.IndexPrivilege; import org.elasticsearch.xpack.core.security.authz.privilege.Privilege; @@ -1387,7 +1388,7 @@ public void testBuildUserPrivilegeResponse() { "index-4", "index-5" ) - .addApplicationPrivilege(new ApplicationPrivilege("app01", "read", "data:read"), Collections.singleton("*")) + .addApplicationPrivilege(ApplicationPrivilegeTests.createPrivilege("app01", "read", "data:read"), Collections.singleton("*")) .runAs(new Privilege(Sets.newHashSet("user01", "user02"), "user01", "user02")) .build(); @@ -1605,7 +1606,7 @@ private ApplicationPrivilege defineApplicationPrivilege( String... actions ) { privs.add(new ApplicationPrivilegeDescriptor(app, name, newHashSet(actions), emptyMap())); - return new ApplicationPrivilege(app, name, actions); + return ApplicationPrivilegeTests.createPrivilege(app, name, actions); } private PrivilegesCheckResult hasPrivileges( diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java index 60d83aa001bee..69ca758a924d6 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java @@ -70,8 +70,8 @@ import org.elasticsearch.xpack.core.security.authz.permission.RemoteIndicesPermission; import org.elasticsearch.xpack.core.security.authz.permission.Role; import org.elasticsearch.xpack.core.security.authz.privilege.ActionClusterPrivilege; -import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilege; import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilegeDescriptor; +import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilegeTests; import org.elasticsearch.xpack.core.security.authz.privilege.ClusterPrivilegeResolver; import org.elasticsearch.xpack.core.security.authz.privilege.ConfigurableClusterPrivilege; import org.elasticsearch.xpack.core.security.authz.privilege.IndexPrivilege; @@ -494,7 +494,7 @@ private void trySuccessfullyLoadSuperuserRole(CompositeRolesStore compositeRoles assertThat( role.application() .grants( - new ApplicationPrivilege( + ApplicationPrivilegeTests.createPrivilege( randomAlphaOfLengthBetween(2, 10), randomAlphaOfLengthBetween(2, 10), randomAlphaOfLengthBetween(2, 10) @@ -1020,10 +1020,10 @@ public ClusterPermission.Builder buildPermission(ClusterPermission.Builder build assertThat(allowedWrite.test(mockIndexAbstraction("xyz")), equalTo(false)); assertThat(allowedWrite.test(mockIndexAbstraction("ind-3-a")), equalTo(false)); - role.application().grants(new ApplicationPrivilege("app1", "app1-read", "write"), "user/joe"); - role.application().grants(new ApplicationPrivilege("app1", "app1-read", "read"), "settings/hostname"); - role.application().grants(new ApplicationPrivilege("app2a", "app2a-all", "all"), "user/joe"); - role.application().grants(new ApplicationPrivilege("app2b", "app2b-read", "read"), "settings/hostname"); + role.application().grants(ApplicationPrivilegeTests.createPrivilege("app1", "app1-read", "write"), "user/joe"); + role.application().grants(ApplicationPrivilegeTests.createPrivilege("app1", "app1-read", "read"), "settings/hostname"); + role.application().grants(ApplicationPrivilegeTests.createPrivilege("app2a", "app2a-all", "all"), "user/joe"); + role.application().grants(ApplicationPrivilegeTests.createPrivilege("app2b", "app2b-read", "read"), "settings/hostname"); assertHasRemoteGroupsForClusters(role.remoteIndices(), Set.of("remote-*", "remote"), Set.of("*"), Set.of("remote-*")); assertHasIndexGroupsForClusters( From 3b6372c39135771e13e04e7d2821c943f1301873 Mon Sep 17 00:00:00 2001 From: Nikolaj Volgushev Date: Fri, 28 Oct 2022 15:56:03 +0200 Subject: [PATCH 2/6] TODOs --- .../core/security/authz/privilege/ApplicationPrivilege.java | 4 +--- .../security/authz/privilege/ApplicationPrivilegeTests.java | 1 + 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilege.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilege.java index 4a1e13ccd925d..a2c5a90023e6f 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilege.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilege.java @@ -48,9 +48,7 @@ public final class ApplicationPrivilege extends Privilege { private final String application; private final String[] patterns; - /** - * Package-private for tests. Use {@link #get(String, Set, Collection)} to construct application privileges. - */ + // TODO make this private once ApplicationPrivilegeTests.createPrivilege use ApplicationPrivilege.get() ApplicationPrivilege(String application, Set name, String... patterns) { super(name, patterns); this.application = application; diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilegeTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilegeTests.java index e60bd2afdcd7a..addf82e07a3e8 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilegeTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilegeTests.java @@ -40,6 +40,7 @@ public static ApplicationPrivilege createPrivilege(String applicationName, Strin } public static ApplicationPrivilege createPrivilege(String application, Set name, String... patterns) { + // TODO rewrite this to use `ApplicationPrivilege.get()` return new ApplicationPrivilege(application, name, patterns); } From 10812c28ffe89055b92614744ff36a85b7c4617f Mon Sep 17 00:00:00 2001 From: Nikolaj Volgushev Date: Fri, 28 Oct 2022 16:34:56 +0200 Subject: [PATCH 3/6] Clean up comment --- .../core/security/authz/privilege/ApplicationPrivilege.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilege.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilege.java index a2c5a90023e6f..cba67fc4db82b 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilege.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilege.java @@ -48,7 +48,7 @@ public final class ApplicationPrivilege extends Privilege { private final String application; private final String[] patterns; - // TODO make this private once ApplicationPrivilegeTests.createPrivilege use ApplicationPrivilege.get() + // TODO make this private once ApplicationPrivilegeTests::createPrivilege uses ApplicationPrivilege::get ApplicationPrivilege(String application, Set name, String... patterns) { super(name, patterns); this.application = application; From 3bd987c2dfd841ca1260b4e6adbdda89c7ed4469 Mon Sep 17 00:00:00 2001 From: Nikolaj Volgushev Date: Fri, 28 Oct 2022 17:16:08 +0200 Subject: [PATCH 4/6] Nits --- .../security/authz/privilege/ApplicationPrivilege.java | 2 +- .../authz/privilege/ApplicationPrivilegeTests.java | 10 +++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilege.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilege.java index cba67fc4db82b..8dbab86210f56 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilege.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilege.java @@ -250,7 +250,7 @@ private static ApplicationPrivilege resolve(String application, Set name } patterns.addAll(actions); - return new ApplicationPrivilege(application, names, patterns.toArray(new String[0])); + return new ApplicationPrivilege(application, names, patterns.toArray(new String[patterns.size()])); } @Override diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilegeTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilegeTests.java index addf82e07a3e8..c0695a81e39fd 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilegeTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilegeTests.java @@ -35,13 +35,17 @@ public class ApplicationPrivilegeTests extends ESTestCase { - public static ApplicationPrivilege createPrivilege(String applicationName, String privilegeName, String... patterns) { + public static ApplicationPrivilege createPrivilege(final String applicationName, final String privilegeName, final String... patterns) { return createPrivilege(applicationName, Collections.singleton(privilegeName), patterns); } - public static ApplicationPrivilege createPrivilege(String application, Set name, String... patterns) { + public static ApplicationPrivilege createPrivilege( + final String applicationName, + final Set privilegeNames, + final String... patterns + ) { // TODO rewrite this to use `ApplicationPrivilege.get()` - return new ApplicationPrivilege(application, name, patterns); + return new ApplicationPrivilege(applicationName, privilegeNames, patterns); } public void testValidationOfApplicationName() { From 47dd44e2e277b04688b2be6fe3eaac04db1aeb89 Mon Sep 17 00:00:00 2001 From: Nikolaj Volgushev Date: Fri, 28 Oct 2022 17:21:14 +0200 Subject: [PATCH 5/6] Rm metadata setup --- .../security/authz/privilege/ApplicationPrivilegeTests.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilegeTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilegeTests.java index c0695a81e39fd..77852839b9df1 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilegeTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilegeTests.java @@ -266,10 +266,6 @@ private ApplicationPrivilege randomPrivilege() { patterns[i] = randomAlphaOfLengthBetween(2, 5) + "/" + suffix; } - final Map metadata = new HashMap<>(); - for (int i = randomInt(3); i > 0; i--) { - metadata.put(randomAlphaOfLengthBetween(2, 5), randomFrom(randomBoolean(), randomInt(10), randomAlphaOfLength(5))); - } return createPrivilege(applicationName, privilegeName, patterns); } From 02cffd4ddc6b6393e5640bd795f79ce36044c3ba Mon Sep 17 00:00:00 2001 From: Nikolaj Volgushev Date: Fri, 28 Oct 2022 17:21:31 +0200 Subject: [PATCH 6/6] Imports --- .../security/authz/privilege/ApplicationPrivilegeTests.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilegeTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilegeTests.java index 77852839b9df1..c4e1f68ce694f 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilegeTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ApplicationPrivilegeTests.java @@ -19,9 +19,7 @@ import java.util.Arrays; import java.util.Collections; -import java.util.HashMap; import java.util.Locale; -import java.util.Map; import java.util.Set; import java.util.function.Supplier;