From 51e492cdfd2da5dd7aa5aa3c6c4ad3526fb80b3a Mon Sep 17 00:00:00 2001 From: CaitlinH <89764428+ch-govau@users.noreply.github.com> Date: Tue, 29 Mar 2022 09:54:02 +1100 Subject: [PATCH] Set the mapped security roles of the user so these can be used by the DLS privileges evaluator. Allow security roles to be used for DLS parameter substitution. Fixes opensearch-project/security/#1568 (#1588) Signed-off-by: Caitlin Harper --- .../security/privileges/PrivilegesEvaluator.java | 2 ++ .../security/securityconf/ConfigModelV7.java | 10 ++++++++++ .../security/dlic/dlsfls/DlsPropsReplaceTest.java | 10 +++++++++- src/test/resources/dlsfls/internal_users.yml | 1 + src/test/resources/dlsfls/roles.yml | 7 +++++++ src/test/resources/dlsfls/roles_mapping.yml | 9 +++++++++ 6 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java index 8ac47f3fa4..b086847433 100644 --- a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java +++ b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java @@ -236,6 +236,8 @@ public PrivilegesEvaluatorResponse evaluate(final User user, String action0, fin final SecurityRoles securityRoles = getSecurityRoles(mappedRoles); setUserInfoInThreadContext(user, mappedRoles); + // Add the security roles for this user so that they can be used for DLS parameter substitution. + user.addSecurityRoles(mappedRoles); final boolean isDebugEnabled = log.isDebugEnabled(); if (isDebugEnabled) { diff --git a/src/main/java/org/opensearch/security/securityconf/ConfigModelV7.java b/src/main/java/org/opensearch/security/securityconf/ConfigModelV7.java index aec7d881e3..9481173959 100644 --- a/src/main/java/org/opensearch/security/securityconf/ConfigModelV7.java +++ b/src/main/java/org/opensearch/security/securityconf/ConfigModelV7.java @@ -907,6 +907,7 @@ private static String replaceProperties(String orig, User user) { orig = orig.replace("${user.name}", user.getName()).replace("${user_name}", user.getName()); orig = replaceRoles(orig, user); + orig = replaceSecurityRoles(orig, user); for (Entry entry : user.getCustomAttributesMap().entrySet()) { if (entry == null || entry.getKey() == null || entry.getValue() == null) { continue; @@ -926,6 +927,15 @@ private static String replaceRoles(final String orig, final User user) { return retVal; } + private static String replaceSecurityRoles(final String orig, final User user) { + String retVal = orig; + if (orig.contains("${user.securityRoles}") || orig.contains("${user_securityRoles}")) { + final String commaSeparatedRoles = toQuotedCommaSeparatedString(user.getSecurityRoles()); + retVal = orig.replace("${user.securityRoles}", commaSeparatedRoles).replace("${user_securityRoles}", commaSeparatedRoles); + } + return retVal; + } + private static String toQuotedCommaSeparatedString(final Set roles) { return Joiner.on(',').join(Iterables.transform(roles, s -> { return new StringBuilder(s.length() + 2).append('"').append(s).append('"').toString(); diff --git a/src/test/java/org/opensearch/security/dlic/dlsfls/DlsPropsReplaceTest.java b/src/test/java/org/opensearch/security/dlic/dlsfls/DlsPropsReplaceTest.java index 7ec28a345e..356de7087a 100644 --- a/src/test/java/org/opensearch/security/dlic/dlsfls/DlsPropsReplaceTest.java +++ b/src/test/java/org/opensearch/security/dlic/dlsfls/DlsPropsReplaceTest.java @@ -40,7 +40,10 @@ protected void populateData(Client tc) { .source("{\"role\": \"prole2\", \"amount\": 4040}", XContentType.JSON)).actionGet(); tc.index(new IndexRequest("prop2").type("_doc").setRefreshPolicy(RefreshPolicy.IMMEDIATE) .source("{\"role\": \"prole3\", \"amount\": 5050}", XContentType.JSON)).actionGet(); - + tc.index(new IndexRequest("prop-mapped").type("_doc").setRefreshPolicy(RefreshPolicy.IMMEDIATE) + .source("{\"securityRole\": \"opendistro_security_mapped\", \"amount\": 6060}", XContentType.JSON)).actionGet(); + tc.index(new IndexRequest("prop-mapped").type("_doc").setRefreshPolicy(RefreshPolicy.IMMEDIATE) + .source("{\"securityRole\": \"not_assigned\", \"amount\": 7070}", XContentType.JSON)).actionGet(); } @@ -58,5 +61,10 @@ public void testDlsProps() throws Exception { Assert.assertEquals(HttpStatus.SC_OK, (res = rh.executeGetRequest("/prop1,prop2/_search?pretty&size=100", encodeBasicHeader("prop_replace", "password"))).getStatusCode()); System.out.println(res.getBody()); Assert.assertTrue(res.getBody().contains("\"value\" : 3,\n \"relation")); + + Assert.assertEquals(HttpStatus.SC_OK, (res = rh.executeGetRequest("/prop-mapped/_search?pretty&size=100", encodeBasicHeader("prop_replace", "password"))).getStatusCode()); + System.out.println(res.getBody()); + Assert.assertTrue(res.getBody().contains("\"value\" : 1,\n \"relation")); + Assert.assertTrue(res.getBody().contains("\"amount\" : 6060")); } } diff --git a/src/test/resources/dlsfls/internal_users.yml b/src/test/resources/dlsfls/internal_users.yml index 33c85a4c4e..acda68c42a 100644 --- a/src/test/resources/dlsfls/internal_users.yml +++ b/src/test/resources/dlsfls/internal_users.yml @@ -156,6 +156,7 @@ prop_replace: backend_roles: - "prole1" - "prole2" + - "prolemapped" attributes: {} description: "Migrated from v6" user_masked_custom: diff --git a/src/test/resources/dlsfls/roles.yml b/src/test/resources/dlsfls/roles.yml index d9b7e76b21..df93cb2318 100644 --- a/src/test/resources/dlsfls/roles.yml +++ b/src/test/resources/dlsfls/roles.yml @@ -1385,6 +1385,13 @@ opendistro_security_prop_replace: masked_fields: null allowed_actions: - "OPENDISTRO_SECURITY_READ" + - index_patterns: + - "prop-mapped" + dls: "{\"terms\" : { \"securityRole\" : [${user.securityRoles}]}}" + fls: null + masked_fields: null + allowed_actions: + - "OPENDISTRO_SECURITY_READ" tenant_permissions: [] opendistro_security_logstash: reserved: false diff --git a/src/test/resources/dlsfls/roles_mapping.yml b/src/test/resources/dlsfls/roles_mapping.yml index ebe633d0d6..092770014c 100644 --- a/src/test/resources/dlsfls/roles_mapping.yml +++ b/src/test/resources/dlsfls/roles_mapping.yml @@ -234,3 +234,12 @@ opendistro_security_date_math: opendistro_security_fls_exists: users: - fls_exists +opendistro_security_mapped: + reserved: false + hidden: false + backend_roles: + - prolemapped + hosts: [] + users: [] + and_backend_roles: [] + description: "Security role mapping to backend role prolemapped"