Skip to content

[Backport 3.0] Corrections in DlsFlsFilterLeafReader regarding PointVales and object valued attributes #5304

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 29, 2025
Merged

[Backport 3.0] Corrections in DlsFlsFilterLeafReader regarding PointVales and object valued attributes #5304

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 73 additions & 35 deletions src/integrationTest/java/org/opensearch/security/privileges/dlsfls/FieldPrivilegesTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ public void indexPattern_simple_inclusive() throws Exception {
FieldPrivileges subject = createSubject(roleConfig);

FieldPrivileges.FlsRule rule = subject.getRestriction(ctx("fls_role_1"), "index_a1");
assertTrue("included_field_a should be allowed", rule.isAllowed("included_field_a"));
assertFalse("Fields other than included_field_a should be not allowed", rule.isAllowed("other_field"));
assertTrue("included_field_a should be allowed", rule.isAllowedAssumingParentsAreAllowed("included_field_a"));
assertFalse("Fields other than included_field_a should be not allowed", rule.isAllowedAssumingParentsAreAllowed("other_field"));
}

@Test
Expand All @@ -72,8 +72,8 @@ public void indexPattern_simple_exclusive() throws Exception {
FieldPrivileges subject = createSubject(roleConfig);

FieldPrivileges.FlsRule rule = subject.getRestriction(ctx("fls_role_1"), "index_a1");
assertFalse("excluded_field_a should be not allowed", rule.isAllowed("excluded_field_a"));
assertTrue("Fields other than included_field_a should be allowed", rule.isAllowed("other_field"));
assertFalse("excluded_field_a should be not allowed", rule.isAllowedAssumingParentsAreAllowed("excluded_field_a"));
assertTrue("Fields other than included_field_a should be allowed", rule.isAllowedAssumingParentsAreAllowed("other_field"));
}

@Test
Expand All @@ -86,11 +86,11 @@ public void indexPattern_joined_inclusive() throws Exception {
FieldPrivileges subject = createSubject(roleConfig);

FieldPrivileges.FlsRule rule = subject.getRestriction(ctx("fls_role_1", "fls_role_2"), "index_a1");
assertTrue("included_field_a should be allowed", rule.isAllowed("included_field_a"));
assertTrue("included_field_a1_foo should be allowed", rule.isAllowed("included_field_a1_foo"));
assertTrue("included_field_a should be allowed", rule.isAllowedAssumingParentsAreAllowed("included_field_a"));
assertTrue("included_field_a1_foo should be allowed", rule.isAllowedAssumingParentsAreAllowed("included_field_a1_foo"));
assertFalse(
"Fields other than included_field_a and included_field_a1_foo should be not allowed",
rule.isAllowed("other_field")
rule.isAllowedAssumingParentsAreAllowed("other_field")
);
}

Expand All @@ -104,9 +104,12 @@ public void indexPattern_joined_exclusive() throws Exception {
FieldPrivileges subject = createSubject(roleConfig);

FieldPrivileges.FlsRule rule = subject.getRestriction(ctx("fls_role_1", "fls_role_2"), "index_a1");
assertFalse("excluded_field_a should be not allowed", rule.isAllowed("excluded_field_a"));
assertFalse("excluded_field_a1_foo should be not allowed", rule.isAllowed("excluded_field_a1_foo"));
assertTrue("Fields other than included_field_a and included_field_a1_foo should be allowed", rule.isAllowed("other_field"));
assertFalse("excluded_field_a should be not allowed", rule.isAllowedAssumingParentsAreAllowed("excluded_field_a"));
assertFalse("excluded_field_a1_foo should be not allowed", rule.isAllowedAssumingParentsAreAllowed("excluded_field_a1_foo"));
assertTrue(
"Fields other than included_field_a and included_field_a1_foo should be allowed",
rule.isAllowedAssumingParentsAreAllowed("other_field")
);
}

@Test
Expand All @@ -119,8 +122,8 @@ public void indexPattern_unrestricted_inclusive() throws Exception {
FieldPrivileges subject = createSubject(roleConfig);

FieldPrivileges.FlsRule rule = subject.getRestriction(ctx("fls_role_1", "non_fls_role"), "index_a1");
assertTrue("included_field_a should be allowed", rule.isAllowed("included_field_a"));
assertTrue("other_field should be allowed", rule.isAllowed("other_field"));
assertTrue("included_field_a should be allowed", rule.isAllowedAssumingParentsAreAllowed("included_field_a"));
assertTrue("other_field should be allowed", rule.isAllowedAssumingParentsAreAllowed("other_field"));
}

@Test
Expand All @@ -133,8 +136,8 @@ public void indexPattern_unrestricted_exclusive() throws Exception {
FieldPrivileges subject = createSubject(roleConfig);

FieldPrivileges.FlsRule rule = subject.getRestriction(ctx("fls_role_1", "non_fls_role"), "index_a1");
assertTrue("excluded_field_a should be allowed", rule.isAllowed("excluded_field_a"));
assertTrue("other_field should be allowed", rule.isAllowed("other_field"));
assertTrue("excluded_field_a should be allowed", rule.isAllowedAssumingParentsAreAllowed("excluded_field_a"));
assertTrue("other_field should be allowed", rule.isAllowedAssumingParentsAreAllowed("other_field"));
}

static SecurityDynamicConfiguration<RoleV7> roleConfig(TestSecurityConfig.Role... roles) {
Expand Down Expand Up @@ -168,8 +171,13 @@ public static class FlsRule {
public void simple_inclusive() throws Exception {
FieldPrivileges.FlsRule flsRule = FieldPrivileges.FlsRule.of("field_inclusive");
assertFalse("FLS rule field_inclusive should be restricted", flsRule.isUnrestricted());
assertTrue("field_inclusive is allowed", flsRule.isAllowed("field_inclusive"));
assertFalse("other_field is not allowed", flsRule.isAllowed("other_field"));

assertTrue("field_inclusive is allowed", flsRule.isAllowedAssumingParentsAreAllowed("field_inclusive"));
assertTrue("field_inclusive is allowed", flsRule.isAllowedRecursive("field_inclusive"));

assertFalse("other_field is not allowed", flsRule.isAllowedAssumingParentsAreAllowed("other_field"));
assertFalse("other_field is not allowed", flsRule.isAllowedRecursive("other_field"));

assertEquals("FLS:[field_inclusive]", flsRule.toString());
assertEquals(Arrays.asList("field_inclusive"), flsRule.getSource());
}
Expand All @@ -178,26 +186,40 @@ public void simple_inclusive() throws Exception {
public void simple_exclusive() throws Exception {
FieldPrivileges.FlsRule flsRule = FieldPrivileges.FlsRule.of("~field_exclusive");
assertFalse("FLS rule field_exclusive should be restricted", flsRule.isUnrestricted());
assertFalse("field_exclusive is not allowed", flsRule.isAllowed("field_exclusive"));
assertTrue("other_field is allowed", flsRule.isAllowed("other_field"));

assertFalse("field_exclusive is not allowed", flsRule.isAllowedAssumingParentsAreAllowed("field_exclusive"));
assertFalse("field_exclusive is not allowed", flsRule.isAllowedRecursive("field_exclusive"));

assertTrue("other_field is allowed", flsRule.isAllowedAssumingParentsAreAllowed("other_field"));
assertTrue("other_field is allowed", flsRule.isAllowedRecursive("other_field"));
}

@Test
public void multi_inclusive() throws Exception {
FieldPrivileges.FlsRule flsRule = FieldPrivileges.FlsRule.of("field_inclusive_1", "field_inclusive_2");
assertFalse("FLS rule should be restricted", flsRule.isUnrestricted());
assertTrue("field_inclusive_1 is allowed", flsRule.isAllowed("field_inclusive_1"));
assertTrue("field_inclusive_2 is allowed", flsRule.isAllowed("field_inclusive_2"));
assertFalse("other_field is not allowed", flsRule.isAllowed("other_field"));

assertTrue("field_inclusive_1 is allowed", flsRule.isAllowedAssumingParentsAreAllowed("field_inclusive_1"));
assertTrue("field_inclusive_2 is allowed", flsRule.isAllowedAssumingParentsAreAllowed("field_inclusive_2"));
assertTrue("field_inclusive_1 is allowed", flsRule.isAllowedRecursive("field_inclusive_1"));
assertTrue("field_inclusive_2 is allowed", flsRule.isAllowedRecursive("field_inclusive_2"));

assertFalse("other_field is not allowed", flsRule.isAllowedAssumingParentsAreAllowed("other_field"));
assertFalse("other_field is not allowed", flsRule.isAllowedRecursive("other_field"));
}

@Test
public void multi_exclusive() throws Exception {
FieldPrivileges.FlsRule flsRule = FieldPrivileges.FlsRule.of("~field_exclusive_1", "~field_exclusive_2");
assertFalse("FLS rule should be restricted", flsRule.isUnrestricted());
assertFalse("field_exclusive_1 is not allowed", flsRule.isAllowed("field_exclusive_1"));
assertFalse("field_exclusive_1 is not allowed", flsRule.isAllowed("field_exclusive_2"));
assertTrue("other_field is allowed", flsRule.isAllowed("other_field"));

assertFalse("field_exclusive_1 is not allowed", flsRule.isAllowedAssumingParentsAreAllowed("field_exclusive_1"));
assertFalse("field_exclusive_2 is not allowed", flsRule.isAllowedAssumingParentsAreAllowed("field_exclusive_2"));
assertFalse("field_exclusive_1 is not allowed", flsRule.isAllowedRecursive("field_exclusive_1"));
assertFalse("field_exclusive_2 is not allowed", flsRule.isAllowedRecursive("field_exclusive_2"));

assertTrue("other_field is allowed", flsRule.isAllowedAssumingParentsAreAllowed("other_field"));
assertTrue("other_field is allowed", flsRule.isAllowedRecursive("other_field"));
}

@Test
Expand All @@ -207,35 +229,51 @@ public void multi_mixed() throws Exception {
// The behavior is undocumented - if there are exclusions and inclusions, only exclusions are regarded.
// It might make sense to re-think this behavior.
assertFalse("FLS rule should be restricted", flsRule.isUnrestricted());
assertFalse("field_exclusive_1 is not allowed", flsRule.isAllowed("field_exclusive_1"));
assertTrue("other_field is allowed", flsRule.isAllowed("other_field"));
assertFalse("field_exclusive_1 is not allowed", flsRule.isAllowedAssumingParentsAreAllowed("field_exclusive_1"));
assertTrue("other_field is allowed", flsRule.isAllowedAssumingParentsAreAllowed("other_field"));
}

@Test
public void nested_inclusive() throws Exception {
FieldPrivileges.FlsRule flsRule = FieldPrivileges.FlsRule.of("a.b.c");
assertFalse("FLS rule should be restricted", flsRule.isUnrestricted());
assertTrue("a.b.c is allowed", flsRule.isAllowed("a.b.c"));
assertFalse("a.b is not allowed for non-objects", flsRule.isAllowed("a.b"));
assertTrue("a.b is not allowed for objects", flsRule.isObjectAllowed("a.b"));
assertFalse("other_field is not allowed", flsRule.isAllowed("other_field"));
assertFalse("a.b.other_field is not allowed", flsRule.isAllowed("a.b.other_field"));

assertTrue("a.b.c is allowed", flsRule.isAllowedAssumingParentsAreAllowed("a.b.c"));
assertTrue("a.b.c is allowed", flsRule.isAllowedRecursive("a.b.c"));

assertFalse("a.b is not allowed for non-objects", flsRule.isAllowedAssumingParentsAreAllowed("a.b"));
assertTrue("a.b is not allowed for objects", flsRule.isObjectAllowedAssumingParentsAreAllowed("a.b"));
assertFalse("a.b is not allowed recursively", flsRule.isAllowedRecursive("a.b"));

assertFalse("other_field is not allowed", flsRule.isAllowedAssumingParentsAreAllowed("other_field"));
assertFalse("other_field is not allowed", flsRule.isAllowedRecursive("other_field"));

assertFalse("a.b.other_field is not allowed", flsRule.isAllowedAssumingParentsAreAllowed("a.b.other_field"));
assertFalse("a.b.other_field is not allowed", flsRule.isAllowedRecursive("a.b.other_field"));

assertTrue("a.b.c.d is allowed", flsRule.isAllowedRecursive("a.b.c.d"));
}

@Test
public void nested_exclusive() throws Exception {
FieldPrivileges.FlsRule flsRule = FieldPrivileges.FlsRule.of("~a.b.c");
assertFalse("FLS rule should be restricted", flsRule.isUnrestricted());
assertFalse("a.b.c is not allowed", flsRule.isAllowed("a.b.c"));
assertTrue("a.b is allowed", flsRule.isAllowed("a.b"));
assertTrue("a.b is allowed for objects", flsRule.isObjectAllowed("a.b"));

assertFalse("a.b.c is not allowed", flsRule.isAllowedAssumingParentsAreAllowed("a.b.c"));
assertFalse("a.b.c is not allowed", flsRule.isAllowedRecursive("a.b.c"));

assertTrue("a.b is allowed", flsRule.isAllowedAssumingParentsAreAllowed("a.b"));
assertTrue("a.b is allowed for objects", flsRule.isObjectAllowedAssumingParentsAreAllowed("a.b"));
assertTrue("a.b is allowed recursively", flsRule.isAllowedAssumingParentsAreAllowed("a.b"));

assertFalse("a.b.c.d is not allowed", flsRule.isAllowedRecursive("a.b.c.d"));
}

@Test
public void wildcard_inclusive() throws Exception {
FieldPrivileges.FlsRule flsRule = FieldPrivileges.FlsRule.of("*");
assertTrue("FLS rule * is unrestricted", flsRule.isUnrestricted());
assertTrue("anything is allowed", flsRule.isAllowed("anything"));
assertTrue("anything is allowed", flsRule.isAllowedAssumingParentsAreAllowed("anything"));
assertEquals("FLS:*", flsRule.toString());
}

Expand Down
Loading
Loading