From e5cdee70dcc14c2d1b69e7549f1790dd26374867 Mon Sep 17 00:00:00 2001 From: Nikolaj Volgushev Date: Mon, 29 Jan 2024 15:01:27 +0100 Subject: [PATCH 1/6] Test unknown named application privileges --- .../security/HasApplicationPrivilegesIT.java | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/HasApplicationPrivilegesIT.java b/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/HasApplicationPrivilegesIT.java index 9e9f42b5c68bf..cf2568fd77cc3 100644 --- a/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/HasApplicationPrivilegesIT.java +++ b/x-pack/plugin/security/qa/security-basic/src/javaRestTest/java/org/elasticsearch/xpack/security/HasApplicationPrivilegesIT.java @@ -151,6 +151,32 @@ public void testUserWithWildcardPrivileges() throws Exception { } } + public void testNamed() throws IOException { + createApplicationPrivilege("app", "write", new String[] { "action:write/*" }); + createRole("correct", "app", new String[] { "read" }, new String[] { "*" }); + createRole("wrong", "app", new String[] { "read", randomFrom("create", "write") }, new String[] { "*" }); + + var password = new SecureString("password-123"); + createUser("correct", password, Set.of("correct")); + createUser("wrong", password, Set.of("wrong")); + + { + var reqOptions = RequestOptions.DEFAULT.toBuilder() + .addHeader("Authorization", UsernamePasswordToken.basicAuthHeaderValue("correct", password)) + .build(); + var actual = hasPrivilege(reqOptions, "app", new String[] { "read" }, new String[] { "resource" }); + assertSinglePrivilege(actual, "resource", "read", true); + } + + { + var reqOptions = RequestOptions.DEFAULT.toBuilder() + .addHeader("Authorization", UsernamePasswordToken.basicAuthHeaderValue("wrong", password)) + .build(); + var actual = hasPrivilege(reqOptions, "app", new String[] { "read" }, new String[] { "resource" }); + assertSinglePrivilege(actual, "resource", "read", true); + } + } + private void assertSinglePrivilege( List hasPrivilegesResult, String expectedResource, From 03f6e09a16c38111497bd0f14e008beac6b3d4a6 Mon Sep 17 00:00:00 2001 From: Nikolaj Volgushev Date: Thu, 1 Feb 2024 12:25:40 +0100 Subject: [PATCH 2/6] WIP fix --- .../authz/permission/ApplicationPermission.java | 14 +++++++------- .../authz/privilege/ApplicationPrivilege.java | 1 - 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ApplicationPermission.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ApplicationPermission.java index c85a648761ca7..40d9744ae957b 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ApplicationPermission.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ApplicationPermission.java @@ -191,18 +191,18 @@ private boolean grants(ApplicationPrivilege other, Automaton resource) { } private boolean matchesPrivilege(ApplicationPrivilege other) { - if (this.privilege.equals(other)) { - return true; - } - if (this.application.test(other.getApplication()) == false) { + assert other.name().size() <= 1 : "can only compare to singletons"; + if (application.test(other.getApplication()) == false) { return false; } if (Operations.isTotal(privilege.getAutomaton())) { return true; } - return Operations.isEmpty(privilege.getAutomaton()) == false - && Operations.isEmpty(other.getAutomaton()) == false - && Operations.subsetOf(other.getAutomaton(), privilege.getAutomaton()); + // If both empty, privileges are not stored, so we need to compare based on privilege names + if (Operations.isEmpty(privilege.getAutomaton()) && Operations.isEmpty(other.getAutomaton())) { + return privilege.name().containsAll(other.name()); + } + return Operations.subsetOf(other.getAutomaton(), privilege.getAutomaton()); } @Override 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 8dbab86210f56..0e09236c27325 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 @@ -272,5 +272,4 @@ public boolean equals(Object o) { && Objects.equals(this.application, ((ApplicationPrivilege) o).application) && Arrays.equals(this.patterns, ((ApplicationPrivilege) o).patterns); } - } From f3f8076f0af7cb202a0081226fde75b2dd64d38c Mon Sep 17 00:00:00 2001 From: Nikolaj Volgushev Date: Thu, 1 Feb 2024 12:26:26 +0100 Subject: [PATCH 3/6] Smaller diff --- .../core/security/authz/privilege/ApplicationPrivilege.java | 1 + 1 file changed, 1 insertion(+) 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 0e09236c27325..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 @@ -272,4 +272,5 @@ public boolean equals(Object o) { && Objects.equals(this.application, ((ApplicationPrivilege) o).application) && Arrays.equals(this.patterns, ((ApplicationPrivilege) o).patterns); } + } From b4be2e263d04cf28e730f8c4889110d886c720dc Mon Sep 17 00:00:00 2001 From: Nikolaj Volgushev Date: Thu, 1 Feb 2024 13:47:54 +0100 Subject: [PATCH 4/6] im holding this wrong --- .../security/authz/permission/ApplicationPermission.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ApplicationPermission.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ApplicationPermission.java index 40d9744ae957b..90d55575572e9 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ApplicationPermission.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ApplicationPermission.java @@ -191,15 +191,15 @@ private boolean grants(ApplicationPrivilege other, Automaton resource) { } private boolean matchesPrivilege(ApplicationPrivilege other) { - assert other.name().size() <= 1 : "can only compare to singletons"; + assert other.name().size() <= 1 : "can only compare to singletons but got [" + other + "]"; if (application.test(other.getApplication()) == false) { return false; } if (Operations.isTotal(privilege.getAutomaton())) { return true; } - // If both empty, privileges are not stored, so we need to compare based on privilege names - if (Operations.isEmpty(privilege.getAutomaton()) && Operations.isEmpty(other.getAutomaton())) { + // If the privilege we are checking is not registered (i.e., maps to no action patterns) check by name instead + if (Operations.isEmpty(other.getAutomaton())) { return privilege.name().containsAll(other.name()); } return Operations.subsetOf(other.getAutomaton(), privilege.getAutomaton()); From 56c5f4ba5abcf88af28ff6c9b617ece422014398 Mon Sep 17 00:00:00 2001 From: Nikolaj Volgushev Date: Thu, 1 Feb 2024 13:49:54 +0100 Subject: [PATCH 5/6] Comment --- .../core/security/authz/permission/ApplicationPermission.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/permission/ApplicationPermission.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ApplicationPermission.java index 90d55575572e9..596518f72a24a 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ApplicationPermission.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ApplicationPermission.java @@ -198,7 +198,7 @@ private boolean matchesPrivilege(ApplicationPrivilege other) { if (Operations.isTotal(privilege.getAutomaton())) { return true; } - // If the privilege we are checking is not registered (i.e., maps to no action patterns) check by name instead + // If the privilege we are checking is not stored (i.e., maps to no action patterns) check by name instead if (Operations.isEmpty(other.getAutomaton())) { return privilege.name().containsAll(other.name()); } From 0d8035d7aef9a0322b92f67775a6c91c2a081a20 Mon Sep 17 00:00:00 2001 From: Nikolaj Volgushev Date: Fri, 2 Feb 2024 16:39:39 +0100 Subject: [PATCH 6/6] Hack around get user privs --- .../security/authz/permission/ApplicationPermission.java | 8 ++++++++ .../elasticsearch/xpack/security/authz/RBACEngine.java | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ApplicationPermission.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ApplicationPermission.java index 596518f72a24a..eb7a6dd6fe774 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ApplicationPermission.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ApplicationPermission.java @@ -173,6 +173,14 @@ public Set getResourcePatterns(ApplicationPrivilege privilege) { .collect(Collectors.toSet()); } + public Set getResourcePatternsByName(ApplicationPrivilege privilege) { + return permissions.stream() + .filter(e -> e.privilege.getApplication().equals(privilege.getApplication()) && e.privilege.name().equals(privilege.name())) + .map(e -> e.resourceNames) + .flatMap(Set::stream) + .collect(Collectors.toSet()); + } + private static class PermissionEntry { private final ApplicationPrivilege privilege; private final Predicate application; diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java index 1b1d7c789b21d..e4902e7d5bcc2 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java @@ -734,7 +734,7 @@ static GetUserPrivilegesResponse buildUserPrivilegesResponseObject(Role userRole final Set application = new LinkedHashSet<>(); for (String applicationName : userRole.application().getApplicationNames()) { for (ApplicationPrivilege privilege : userRole.application().getPrivileges(applicationName)) { - final Set resources = userRole.application().getResourcePatterns(privilege); + final Set resources = userRole.application().getResourcePatternsByName(privilege); if (resources.isEmpty()) { logger.trace("No resources defined in application privilege {}", privilege); } else {