Skip to content

[Failure Store] Manage-style privileges grant both data and failures access #125900

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 8 commits into from
Apr 1, 2025
Merged

[Failure Store] Manage-style privileges grant both data and failures access #125900

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
20061fc
[Failure Store] manage privileges grant data and failures access
n1v0lg Mar 30, 2025
35b214b
More tests
n1v0lg Mar 30, 2025
65dd807
ff
n1v0lg Mar 30, 2025
7f195f1
Tests
n1v0lg Mar 30, 2025
f881a48
Test
n1v0lg Mar 30, 2025
c1b4903
Address review comments
n1v0lg Apr 1, 2025
2b608c2
Merge branch 'main' into failure-store-data-and-failures-privileges
n1v0lg Apr 1, 2025
6514b39
Merge branch 'main' into failure-store-data-and-failures-privileges
n1v0lg Apr 1, 2025
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
4 changes: 4 additions & 0 deletions ...rg/elasticsearch/xpack/core/security/authz/privilege/IndexComponentSelectorPredicate.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ public record IndexComponentSelectorPredicate(Set<String> names, Predicate<Index
"failures",
IndexComponentSelector.FAILURES::equals
);
public static final IndexComponentSelectorPredicate DATA_AND_FAILURES = new IndexComponentSelectorPredicate(
Set.of("data", "failures"),
DATA.predicate.or(FAILURES.predicate)
);

@Override
public boolean test(IndexComponentSelector selector) {
Expand Down
29 changes: 23 additions & 6 deletions ...e/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/IndexPrivilege.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@
import java.util.Collections;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
Expand Down Expand Up @@ -195,7 +194,11 @@ public final class IndexPrivilege extends Privilege {
public static final IndexPrivilege WRITE = new IndexPrivilege("write", WRITE_AUTOMATON);
public static final IndexPrivilege CREATE_DOC = new IndexPrivilege("create_doc", CREATE_DOC_AUTOMATON);
public static final IndexPrivilege MONITOR = new IndexPrivilege("monitor", MONITOR_AUTOMATON);
public static final IndexPrivilege MANAGE = new IndexPrivilege("manage", MANAGE_AUTOMATON);
public static final IndexPrivilege MANAGE = new IndexPrivilege(
"manage",
MANAGE_AUTOMATON,
IndexComponentSelectorPredicate.DATA_AND_FAILURES
);
public static final IndexPrivilege DELETE_INDEX = new IndexPrivilege("delete_index", DELETE_INDEX_AUTOMATON);
public static final IndexPrivilege CREATE_INDEX = new IndexPrivilege("create_index", CREATE_INDEX_AUTOMATON);
public static final IndexPrivilege VIEW_METADATA = new IndexPrivilege("view_index_metadata", VIEW_METADATA_AUTOMATON);
Expand All @@ -204,7 +207,8 @@ public final class IndexPrivilege extends Privilege {
public static final IndexPrivilege MANAGE_ILM = new IndexPrivilege("manage_ilm", MANAGE_ILM_AUTOMATON);
public static final IndexPrivilege MANAGE_DATA_STREAM_LIFECYCLE = new IndexPrivilege(
"manage_data_stream_lifecycle",
MANAGE_DATA_STREAM_LIFECYCLE_AUTOMATON
MANAGE_DATA_STREAM_LIFECYCLE_AUTOMATON,
IndexComponentSelectorPredicate.DATA_AND_FAILURES
);
public static final IndexPrivilege MAINTENANCE = new IndexPrivilege("maintenance", MAINTENANCE_AUTOMATON);
public static final IndexPrivilege AUTO_CONFIGURE = new IndexPrivilege("auto_configure", AUTO_CONFIGURE_AUTOMATON);
Expand Down Expand Up @@ -364,6 +368,7 @@ private static Set<IndexPrivilege> resolve(Set<String> name) {
final Set<IndexPrivilege> allSelectorAccessPrivileges = new HashSet<>();
final Set<IndexPrivilege> dataSelectorAccessPrivileges = new HashSet<>();
final Set<IndexPrivilege> failuresSelectorAccessPrivileges = new HashSet<>();
final Set<IndexPrivilege> dataAndFailuresSelectorAccessPrivileges = new HashSet<>();

boolean containsAllAccessPrivilege = name.stream().anyMatch(n -> getNamedOrNull(n) == ALL);
for (String part : name) {
Expand All @@ -383,6 +388,8 @@ private static Set<IndexPrivilege> resolve(Set<String> name) {
dataSelectorAccessPrivileges.add(indexPrivilege);
} else if (indexPrivilege.selectorPredicate == IndexComponentSelectorPredicate.FAILURES) {
failuresSelectorAccessPrivileges.add(indexPrivilege);
} else if (indexPrivilege.selectorPredicate == IndexComponentSelectorPredicate.DATA_AND_FAILURES) {
dataAndFailuresSelectorAccessPrivileges.add(indexPrivilege);
} else {
String errorMessage = "unexpected selector [" + indexPrivilege.selectorPredicate + "]";
assert false : errorMessage;
Expand All @@ -406,6 +413,7 @@ private static Set<IndexPrivilege> resolve(Set<String> name) {
allSelectorAccessPrivileges,
dataSelectorAccessPrivileges,
failuresSelectorAccessPrivileges,
dataAndFailuresSelectorAccessPrivileges,
actions
);
assertNamesMatch(name, combined);
Expand All @@ -416,24 +424,33 @@ private static Set<IndexPrivilege> combineIndexPrivileges(
Set<IndexPrivilege> allSelectorAccessPrivileges,
Set<IndexPrivilege> dataSelectorAccessPrivileges,
Set<IndexPrivilege> failuresSelectorAccessPrivileges,
Set<IndexPrivilege> dataAndFailuresSelectorAccessPrivileges,
Set<String> actions
) {
assert false == allSelectorAccessPrivileges.isEmpty()
|| false == dataSelectorAccessPrivileges.isEmpty()
|| false == failuresSelectorAccessPrivileges.isEmpty()
|| false == dataAndFailuresSelectorAccessPrivileges.isEmpty()
|| false == actions.isEmpty() : "at least one of the privilege sets or actions must be non-empty";

if (false == allSelectorAccessPrivileges.isEmpty()) {
assert failuresSelectorAccessPrivileges.isEmpty() && dataSelectorAccessPrivileges.isEmpty()
: "data and failure access must be empty when all access is present";
assert failuresSelectorAccessPrivileges.isEmpty()
&& dataSelectorAccessPrivileges.isEmpty()
&& dataAndFailuresSelectorAccessPrivileges.isEmpty() : "data and failure access must be empty when all access is present";
return Set.of(union(allSelectorAccessPrivileges, actions, IndexComponentSelectorPredicate.ALL));
}

// linked hash set to preserve order across selectors
final Set<IndexPrivilege> combined = new LinkedHashSet<>();
final Set<IndexPrivilege> combined = Sets.newLinkedHashSetWithExpectedSize(
dataAndFailuresSelectorAccessPrivileges.size() + failuresSelectorAccessPrivileges.size() + dataSelectorAccessPrivileges.size()
+ actions.size()
);
if (false == dataSelectorAccessPrivileges.isEmpty() || false == actions.isEmpty()) {
combined.add(union(dataSelectorAccessPrivileges, actions, IndexComponentSelectorPredicate.DATA));
}
if (false == dataAndFailuresSelectorAccessPrivileges.isEmpty()) {
combined.add(union(dataAndFailuresSelectorAccessPrivileges, Set.of(), IndexComponentSelectorPredicate.DATA_AND_FAILURES));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: consider pre-sizing the combined hash set

}
if (false == failuresSelectorAccessPrivileges.isEmpty()) {
combined.add(union(failuresSelectorAccessPrivileges, Set.of(), IndexComponentSelectorPredicate.FAILURES));
}
Expand Down
70 changes: 69 additions & 1 deletion .../test/java/org/elasticsearch/xpack/core/security/authz/privilege/IndexPrivilegeTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,70 @@ public void testResolveBySelectorAccess() {
List<IndexComponentSelectorPredicate> actualPredicates = actual.stream().map(IndexPrivilege::getSelectorPredicate).toList();
assertThat(actualPredicates, containsInAnyOrder(IndexComponentSelectorPredicate.ALL));
}
{
Set<IndexPrivilege> actual = IndexPrivilege.resolveBySelectorAccess(
Set.of("manage", "all", "read", "indices:data/read/search", "view_index_metadata")
);
assertThat(
actual,
containsInAnyOrder(
resolvePrivilegeAndAssertSingleton(Set.of("manage", "all", "read", "indices:data/read/search", "view_index_metadata"))
)
);
List<IndexComponentSelectorPredicate> actualPredicates = actual.stream().map(IndexPrivilege::getSelectorPredicate).toList();
assertThat(actualPredicates, containsInAnyOrder(IndexComponentSelectorPredicate.ALL));
}
{
Set<IndexPrivilege> actual = IndexPrivilege.resolveBySelectorAccess(
Set.of("manage", "read", "indices:data/read/search", "read_failure_store")
);
assertThat(
actual,
containsInAnyOrder(
IndexPrivilege.MANAGE,
IndexPrivilege.READ_FAILURE_STORE,
resolvePrivilegeAndAssertSingleton(Set.of("read", "indices:data/read/search"))
)
);
List<IndexComponentSelectorPredicate> actualPredicates = actual.stream().map(IndexPrivilege::getSelectorPredicate).toList();
assertThat(
actualPredicates,
containsInAnyOrder(
IndexComponentSelectorPredicate.DATA,
IndexComponentSelectorPredicate.FAILURES,
IndexComponentSelectorPredicate.DATA_AND_FAILURES
)
);
}
{
Set<IndexPrivilege> actual = IndexPrivilege.resolveBySelectorAccess(Set.of("manage", "read", "indices:data/read/search"));
assertThat(
actual,
containsInAnyOrder(IndexPrivilege.MANAGE, resolvePrivilegeAndAssertSingleton(Set.of("read", "indices:data/read/search")))
);
List<IndexComponentSelectorPredicate> actualPredicates = actual.stream().map(IndexPrivilege::getSelectorPredicate).toList();
assertThat(
actualPredicates,
containsInAnyOrder(IndexComponentSelectorPredicate.DATA, IndexComponentSelectorPredicate.DATA_AND_FAILURES)
);
}
{
Set<IndexPrivilege> actual = IndexPrivilege.resolveBySelectorAccess(
Set.of("manage", "read", "manage_data_stream_lifecycle", "indices:admin/*")
);
assertThat(
actual,
containsInAnyOrder(
resolvePrivilegeAndAssertSingleton(Set.of("manage_data_stream_lifecycle", "manage")),
resolvePrivilegeAndAssertSingleton(Set.of("read", "indices:admin/*"))
)
);
List<IndexComponentSelectorPredicate> actualPredicates = actual.stream().map(IndexPrivilege::getSelectorPredicate).toList();
assertThat(
actualPredicates,
containsInAnyOrder(IndexComponentSelectorPredicate.DATA, IndexComponentSelectorPredicate.DATA_AND_FAILURES)
);
}
}

public void testPrivilegesForRollupFieldCapsAction() {
Expand Down Expand Up @@ -300,7 +364,11 @@ public void testCrossClusterReplicationPrivileges() {
assertThat(
Automatons.subsetOf(
crossClusterReplication.automaton,
resolvePrivilegeAndAssertSingleton(Set.of("manage", "read", "monitor")).automaton
IndexPrivilege.resolveBySelectorAccess(Set.of("manage", "read", "monitor"))
.stream()
.map(p -> p.automaton)
.reduce((a1, a2) -> Automatons.unionAndMinimize(List.of(a1, a2)))
.get()
),
is(true)
);
Expand Down
106 changes: 90 additions & 16 deletions ...stTest/java/org/elasticsearch/xpack/security/failurestore/FailureStoreSecurityRestIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ protected Settings restAdminSettings() {
private static final String WRITE_ACCESS = "write_access";
private static final String MANAGE_ACCESS = "manage_access";
private static final String MANAGE_FAILURE_STORE_ACCESS = "manage_failure_store_access";
private static final String MANAGE_DATA_STREAM_LIFECYCLE = "manage_data_stream_lifecycle";
private static final SecureString PASSWORD = new SecureString("admin-password");

@Before
Expand Down Expand Up @@ -126,14 +127,10 @@ public void testGetUserPrivileges() throws IOException {
"global": [],
"indices": [{
"names": ["*"],
"privileges": ["read"],
"privileges": ["read", "read_failure_store"],
"allow_restricted_indices": false
},
{
"names": ["*"],
"privileges": ["read_failure_store"],
"allow_restricted_indices": false
}],
}
],
"applications": [],
"run_as": []
}""");
Expand Down Expand Up @@ -210,14 +207,57 @@ public void testGetUserPrivileges() throws IOException {
"indices": [
{
"names": ["*"],
"privileges": ["read", "write"],
"privileges": ["manage_failure_store", "read", "read_failure_store", "write"],
"allow_restricted_indices": false
}
],
"applications": [],
"run_as": []
}""");

upsertRole("""
{
"cluster": ["all"],
"indices": [
{
"names": ["*", "idx"],
"privileges": ["read", "manage"],
"allow_restricted_indices": false
},
{
"names": ["*"],
"privileges": ["manage_failure_store", "read_failure_store"],
"names": ["idx", "*"],
"privileges": ["manage_data_stream_lifecycle"],
"allow_restricted_indices": false
}],
},
{
"names": ["*", "idx"],
"privileges": ["write"],
"allow_restricted_indices": true
},
{
"names": ["idx", "*"],
"privileges": ["manage"],
"allow_restricted_indices": true
}
]
}
""", "role");
expectUserPrivilegesResponse("""
{
"cluster": ["all"],
"global": [],
"indices": [
{
"names": ["*", "idx"],
"privileges": ["manage", "manage_data_stream_lifecycle", "read"],
"allow_restricted_indices": false
},
{
"names": ["*", "idx"],
"privileges": ["manage", "write"],
"allow_restricted_indices": true
}
],
"applications": [],
"run_as": []
}""");
Expand Down Expand Up @@ -1772,7 +1812,7 @@ public void testFailureStoreAccess() throws Exception {
}
}

public void testWriteOperations() throws IOException {
public void testWriteAndManageOperations() throws IOException {
setupDataStream();
Tuple<String, String> backingIndices = getSingleDataAndFailureIndices("test1");
String dataIndexName = backingIndices.v1();
Expand Down Expand Up @@ -1808,15 +1848,49 @@ public void testWriteOperations() throws IOException {
}
""");

// user with manage access to data stream does NOT get direct access to failure index
expectThrows(() -> deleteIndex(MANAGE_ACCESS, failureIndexName), 403);
createUser(MANAGE_DATA_STREAM_LIFECYCLE, PASSWORD, MANAGE_DATA_STREAM_LIFECYCLE);
upsertRole(Strings.format("""
{
"cluster": ["all"],
"indices": [{"names": ["test*"], "privileges": ["manage_data_stream_lifecycle"]}]
}"""), MANAGE_DATA_STREAM_LIFECYCLE);
createAndStoreApiKey(MANAGE_DATA_STREAM_LIFECYCLE, randomBoolean() ? null : """
{
"role": {
"cluster": ["all"],
"indices": [{"names": ["test*"], "privileges": ["manage_data_stream_lifecycle"]}]
}
}
""");

// explain lifecycle API with and without failures selector is granted by manage
assertOK(performRequest(MANAGE_ACCESS, new Request("GET", "test1/_lifecycle/explain")));
assertOK(performRequest(MANAGE_ACCESS, new Request("GET", "test1::failures/_lifecycle/explain")));
assertOK(performRequest(MANAGE_ACCESS, new Request("GET", failureIndexName + "/_lifecycle/explain")));
assertOK(performRequest(MANAGE_ACCESS, new Request("GET", dataIndexName + "/_lifecycle/explain")));

assertOK(performRequest(MANAGE_DATA_STREAM_LIFECYCLE, new Request("GET", "test1/_lifecycle/explain")));
assertOK(performRequest(MANAGE_DATA_STREAM_LIFECYCLE, new Request("GET", "test1::failures/_lifecycle/explain")));
assertOK(performRequest(MANAGE_DATA_STREAM_LIFECYCLE, new Request("GET", failureIndexName + "/_lifecycle/explain")));
assertOK(performRequest(MANAGE_DATA_STREAM_LIFECYCLE, new Request("GET", dataIndexName + "/_lifecycle/explain")));

// explain lifecycle API is granted by manage_failure_store only for failures selector
expectThrows(() -> performRequest(MANAGE_FAILURE_STORE_ACCESS, new Request("GET", "test1/_lifecycle/explain")), 403);
assertOK(performRequest(MANAGE_FAILURE_STORE_ACCESS, new Request("GET", "test1::failures/_lifecycle/explain")));
assertOK(performRequest(MANAGE_FAILURE_STORE_ACCESS, new Request("GET", failureIndexName + "/_lifecycle/explain")));
expectThrows(() -> performRequest(MANAGE_FAILURE_STORE_ACCESS, new Request("GET", dataIndexName + "/_lifecycle/explain")), 403);

// user with manage access to data stream can delete failure index because manage grants access to both data and failures
expectThrows(() -> deleteIndex(MANAGE_ACCESS, failureIndexName), 400);
expectThrows(() -> deleteIndex(MANAGE_ACCESS, dataIndexName), 400);
// manage_failure_store user COULD delete failure index (not valid because it's a write index, but allow security-wise)
expectThrows(() -> deleteIndex(MANAGE_FAILURE_STORE_ACCESS, dataIndexName), 403);

// manage_failure_store user COULD delete failure index (not valid because it's a write index, but allowed security-wise)
expectThrows(() -> deleteIndex(MANAGE_FAILURE_STORE_ACCESS, failureIndexName), 400);
expectThrows(() -> deleteIndex(MANAGE_FAILURE_STORE_ACCESS, dataIndexName), 403);
expectThrows(() -> deleteDataStream(MANAGE_FAILURE_STORE_ACCESS, dataIndexName), 403);

expectThrows(() -> deleteDataStream(MANAGE_FAILURE_STORE_ACCESS, "test1"), 403);
// selectors aren't supported for deletes so we get a 403
expectThrows(() -> deleteDataStream(MANAGE_FAILURE_STORE_ACCESS, "test1::failures"), 403);

// manage user can delete data stream
Expand Down
Loading