Skip to content

Commit 4c51737

Browse files
n1v0lgn1v0lg
committed
Tweaks
1 parent 56a8c76 commit 4c51737

File tree

2 files changed

+143
-39
lines changed

2 files changed

+143
-39
lines changed

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/IndicesPermission.java‎

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -818,6 +818,9 @@ private Map<Automaton, Automaton> indexGroupAutomatons(boolean combine) {
818818

819819
public static class Group {
820820
public static final Group[] EMPTY_ARRAY = new Group[0];
821+
// TODO this is just a hack to avoid implementing a new field in this POC; this would be set via allow_failure_store_access on
822+
// the role descriptor
823+
private static final String FAILURE_STORE_ACCESS_MARKER = ".failure_store_access_marker";
821824

822825
private final IndexPrivilege privilege;
823826
private final Predicate<String> actionMatcher;
@@ -831,42 +834,56 @@ public static class Group {
831834
// users. Setting this flag true eliminates the special status for the purpose of this permission - restricted indices still have
832835
// to be covered by the "indices"
833836
private final boolean allowRestrictedIndices;
837+
private final boolean allowFailureStoreAccess;
834838

835839
public Group(
836840
IndexPrivilege privilege,
837841
FieldPermissions fieldPermissions,
838842
@Nullable Set<BytesReference> query,
839843
boolean allowRestrictedIndices,
840844
RestrictedIndices restrictedIndices,
841-
final String... indices
845+
String... indices
842846
) {
843847
assert indices.length != 0;
844848
this.privilege = privilege;
845849
this.actionMatcher = privilege.predicate();
846850
this.indices = indices;
851+
boolean allowFailureStoreAccess = allowFailureStoreAccess(indices);
852+
this.allowFailureStoreAccess = allowFailureStoreAccess;
847853
// TODO: [Jake] how to support selectors for hasPrivileges ? (are reg-ex's just broken for hasPrivilege index checks ?)
848854
// TODO: [Jake] ensure that only ::failure selectors can be added the role (i.e. error on name::* or name::data)
849855
// TODO: [Jake] ensure that no selectors can be added to remote_indices (or gate usage with a feature flag, or just test)
850-
String[] patternsReWritten = maybeAddFailureExclusions(indices);
851856
this.allowRestrictedIndices = allowRestrictedIndices;
852857
ConcurrentHashMap<String[], Automaton> indexNameAutomatonMemo = new ConcurrentHashMap<>(1);
853858
if (allowRestrictedIndices) {
854-
this.indexNameMatcher = StringMatcher.of(patternsReWritten);
855-
this.indexNameAutomaton = () -> indexNameAutomatonMemo.computeIfAbsent(
856-
patternsReWritten,
857-
k -> Automatons.patterns(patternsReWritten)
858-
);
859+
this.indexNameMatcher = getIndexNameMatcher(allowFailureStoreAccess, indices);
860+
// TODO handle this, too
861+
this.indexNameAutomaton = () -> indexNameAutomatonMemo.computeIfAbsent(indices, k -> Automatons.patterns(indices));
859862
} else {
860-
this.indexNameMatcher = StringMatcher.of(patternsReWritten).and(name -> restrictedIndices.isRestricted(name) == false);
863+
this.indexNameMatcher = getIndexNameMatcher(allowFailureStoreAccess, indices).and(
864+
name -> restrictedIndices.isRestricted(name) == false
865+
);
866+
// TODO handle this, too
861867
this.indexNameAutomaton = () -> indexNameAutomatonMemo.computeIfAbsent(
862-
patternsReWritten,
863-
k -> Automatons.minusAndMinimize(Automatons.patterns(patternsReWritten), restrictedIndices.getAutomaton())
868+
indices,
869+
k -> Automatons.minusAndMinimize(Automatons.patterns(indices), restrictedIndices.getAutomaton())
864870
);
865871
}
866872
this.fieldPermissions = Objects.requireNonNull(fieldPermissions);
867873
this.query = query;
868874
}
869875

876+
private static StringMatcher getIndexNameMatcher(boolean allowFailureStoreAccess, String[] indices) {
877+
if (false == allowFailureStoreAccess) {
878+
return StringMatcher.of(indices).and(name -> name.endsWith("::failures") == false);
879+
}
880+
return StringMatcher.of(indices);
881+
}
882+
883+
private static boolean allowFailureStoreAccess(String... indices) {
884+
return Arrays.stream(indices).anyMatch(index -> index.equals("*") || index.equals(FAILURE_STORE_ACCESS_MARKER));
885+
}
886+
870887
// TODO: [Jake] ensure this javadoc is still correct before merging (some minor details are wrong, but the gist is correct)
871888
/**
872889
* This method looks for any index patterns in this group that have all the following characteristics:

‎x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/FailureStoreSecurityRestIT.java‎

Lines changed: 116 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,15 @@
1313
import org.elasticsearch.client.Response;
1414
import org.elasticsearch.client.ResponseException;
1515
import org.elasticsearch.common.settings.SecureString;
16+
import org.elasticsearch.common.xcontent.support.XContentMapValues;
1617
import org.elasticsearch.core.Strings;
1718
import org.elasticsearch.search.SearchHit;
1819
import org.elasticsearch.search.SearchResponseUtils;
1920

2021
import java.io.IOException;
2122
import java.util.ArrayList;
2223
import java.util.Arrays;
24+
import java.util.Collections;
2325
import java.util.List;
2426
import java.util.Map;
2527

@@ -29,36 +31,115 @@
2931

3032
public class FailureStoreSecurityRestIT extends SecurityOnTrialLicenseRestTestCase {
3133

32-
private static final String USER = "user";
34+
private static final String DATA_ACCESS_USER = "data_access_user";
35+
private static final String FAILURE_STORE_ACCESS_USER = "failure_store_access_user";
3336
private static final SecureString PASSWORD = new SecureString("elastic-password");
3437

38+
@SuppressWarnings("unchecked")
3539
public void testFailureStoreAccess() throws IOException {
40+
String dataAccessRole = "data_access";
3641
String failureStoreAccessRole = "failure_store_access";
37-
createUser(USER, PASSWORD, List.of(failureStoreAccessRole));
42+
43+
createUser(DATA_ACCESS_USER, PASSWORD, List.of(dataAccessRole));
44+
createUser(FAILURE_STORE_ACCESS_USER, PASSWORD, List.of(failureStoreAccessRole));
3845

46+
upsertRole(Strings.format("""
47+
{
48+
"description": "Role with data access",
49+
"cluster": ["all"],
50+
"indices": [{"names": ["test*"], "privileges": ["read"]}]
51+
}"""), dataAccessRole);
3952
upsertRole(Strings.format("""
4053
{
4154
"description": "Role with failure store access",
4255
"cluster": ["all"],
43-
"indices": [{"names": ["test*::failures"], "privileges": ["read"]}]
56+
"indices": [{"names": ["test*::failures", ".failure_store_access_marker"], "privileges": ["read"]}]
4457
}"""), failureStoreAccessRole);
4558

4659
createTemplates();
47-
List<String> ids = populateDataStreamWithBulkRequest();
48-
assertThat(ids.size(), equalTo(2));
49-
assertThat(ids, hasItem("1"));
60+
List<String> docIds = populateDataStreamWithBulkRequest();
61+
assertThat(docIds.size(), equalTo(2));
62+
assertThat(docIds, hasItem("1"));
5063
String successDocId = "1";
51-
String failedDocId = ids.stream().filter(id -> false == id.equals(successDocId)).findFirst().get();
64+
String failedDocId = docIds.stream().filter(id -> false == id.equals(successDocId)).findFirst().get();
65+
66+
Request dataStream = new Request("GET", "/_data_stream/test1");
67+
Response response = adminClient().performRequest(dataStream);
68+
Map<String, Object> dataStreams = entityAsMap(response);
69+
assertEquals(Collections.singletonList("test1"), XContentMapValues.extractValue("data_streams.name", dataStreams));
70+
List<String> dataIndexNames = (List<String>) XContentMapValues.extractValue("data_streams.indices.index_name", dataStreams);
71+
assertThat(dataIndexNames.size(), equalTo(1));
72+
List<String> failureIndexNames = (List<String>) XContentMapValues.extractValue(
73+
"data_streams.failure_store.indices.index_name",
74+
dataStreams
75+
);
76+
assertThat(failureIndexNames.size(), equalTo(1));
77+
78+
String dataIndexName = dataIndexNames.get(0);
79+
String failureIndexName = failureIndexNames.get(0);
5280

5381
// user with access to failures index
54-
assertContainsDocIds(performRequestAsUser1(new Request("GET", "/test1::failures/_search")), failedDocId);
55-
assertContainsDocIds(performRequestAsUser1(new Request("GET", "/test*::failures/_search")), failedDocId);
56-
assertContainsDocIds(performRequestAsUser1(new Request("GET", "/*1::failures/_search")), failedDocId);
57-
assertContainsDocIds(performRequestAsUser1(new Request("GET", "/*::failures/_search")), failedDocId);
58-
assertContainsDocIds(performRequestAsUser1(new Request("GET", "/.fs*/_search")), failedDocId);
82+
assertContainsDocIds(performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/test1::failures/_search")), failedDocId);
83+
assertContainsDocIds(performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/test*::failures/_search")), failedDocId);
84+
assertContainsDocIds(performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/*1::failures/_search")), failedDocId);
85+
assertContainsDocIds(performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/*::failures/_search")), failedDocId);
86+
assertContainsDocIds(performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/.fs*/_search")), failedDocId);
87+
assertContainsDocIds(
88+
performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/" + failureIndexName + "/_search")),
89+
failedDocId
90+
);
91+
// TODO fix this
92+
// assertContainsDocIds(
93+
// performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/" + failureIndexName + "/_search?ignore_unavailable=true")),
94+
// failedDocId
95+
// );
5996

60-
expectThrows404(() -> performRequestAsUser1(new Request("GET", "/test12::failures/_search")));
61-
expectThrows404(() -> performRequestAsUser1(new Request("GET", "/test2::failures/_search")));
97+
expectThrows404(() -> performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/test12::failures/_search")));
98+
expectThrows404(() -> performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/test2::failures/_search")));
99+
expectThrows403(() -> performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/test12::*/_search")));
100+
101+
expectThrows403(() -> performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/test1::data/_search")));
102+
expectThrows403(() -> performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/test1/_search")));
103+
expectThrows403(() -> performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/test2::data/_search")));
104+
expectThrows403(() -> performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/test2/_search")));
105+
expectThrows403(() -> performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/" + dataIndexName + "/_search")));
106+
107+
assertEmpty(performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/test1::data/_search?ignore_unavailable=true")));
108+
assertEmpty(performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/test1/_search?ignore_unavailable=true")));
109+
assertEmpty(performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/test2::data/_search?ignore_unavailable=true")));
110+
assertEmpty(performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/test2/_search?ignore_unavailable=true")));
111+
// TODO fix this
112+
// assertEmpty(
113+
// performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/" + dataIndexName + "/_search?ignore_unavailable=true"))
114+
// );
115+
116+
// assertEmpty(performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/*1::data/_search")));
117+
// assertEmpty(performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/*1/_search")));
118+
// TODO is this correct?
119+
assertEmpty(performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/.ds*/_search")));
120+
121+
// user with access to data index
122+
assertContainsDocIds(performRequest(DATA_ACCESS_USER, new Request("GET", "/test1/_search")), successDocId);
123+
assertContainsDocIds(performRequest(DATA_ACCESS_USER, new Request("GET", "/test*/_search")), successDocId);
124+
assertContainsDocIds(performRequest(DATA_ACCESS_USER, new Request("GET", "/*1/_search")), successDocId);
125+
assertContainsDocIds(performRequest(DATA_ACCESS_USER, new Request("GET", "/*/_search")), successDocId);
126+
assertContainsDocIds(performRequest(DATA_ACCESS_USER, new Request("GET", "/.ds*/_search")), successDocId);
127+
assertContainsDocIds(performRequest(DATA_ACCESS_USER, new Request("GET", "/" + dataIndexName + "/_search")), successDocId);
128+
assertContainsDocIds(
129+
performRequest(DATA_ACCESS_USER, new Request("GET", "/" + dataIndexName + "/_search?ignore_unavailable=true")),
130+
successDocId
131+
);
132+
133+
expectThrows404(() -> performRequest(DATA_ACCESS_USER, new Request("GET", "/test12/_search")));
134+
expectThrows404(() -> performRequest(DATA_ACCESS_USER, new Request("GET", "/test2/_search")));
135+
expectThrows403(() -> performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/test12::*/_search")));
136+
137+
expectThrows403(() -> performRequest(DATA_ACCESS_USER, new Request("GET", "/test1::failures/_search")));
138+
expectThrows403(() -> performRequest(DATA_ACCESS_USER, new Request("GET", "/test2::failures/_search")));
139+
expectThrows403(() -> performRequest(DATA_ACCESS_USER, new Request("GET", "/" + failureIndexName + "/_search")));
140+
// TODO is this correct?
141+
assertEmpty(performRequest(DATA_ACCESS_USER, new Request("GET", "/.fs*/_search")));
142+
// assertEmpty(performRequest(DATA_ACCESS_USER, new Request("GET", "/*1::failures/_search")));
62143

63144
// user with access to everything
64145
assertContainsDocIds(adminClient().performRequest(new Request("GET", "/test1::failures/_search")), failedDocId);
@@ -76,6 +157,11 @@ private static void expectThrows404(ThrowingRunnable get) {
76157
assertThat(ex.getResponse().getStatusLine().getStatusCode(), equalTo(404));
77158
}
78159

160+
private static void expectThrows403(ThrowingRunnable get) {
161+
var ex = expectThrows(ResponseException.class, get);
162+
assertThat(ex.getResponse().getStatusLine().getStatusCode(), equalTo(403));
163+
}
164+
79165
@SuppressWarnings("unchecked")
80166
private static void assertContainsDocIds(Response response, String... docIds) throws IOException {
81167
assertOK(response);
@@ -90,12 +176,15 @@ private static void assertContainsDocIds(Response response, String... docIds) th
90176
}
91177
}
92178

93-
private static void assert404(Response response) {
94-
assertThat(response.getStatusLine().getStatusCode(), equalTo(404));
95-
}
96-
97-
private static void assert403(Response response) {
98-
assertThat(response.getStatusLine().getStatusCode(), equalTo(403));
179+
private static void assertEmpty(Response response) throws IOException {
180+
assertOK(response);
181+
final SearchResponse searchResponse = SearchResponseUtils.parseSearchResponse(responseAsParser(response));
182+
try {
183+
SearchHit[] hits = searchResponse.getHits().getHits();
184+
assertThat(hits.length, equalTo(0));
185+
} finally {
186+
searchResponse.decRef();
187+
}
99188
}
100189

101190
private void createTemplates() throws IOException {
@@ -120,10 +209,10 @@ private void createTemplates() throws IOException {
120209
}
121210
},
122211
"data_stream_options": {
123-
"failure_store": {
124-
"enabled": true
125-
}
126-
}
212+
"failure_store": {
213+
"enabled": true
214+
}
215+
}
127216
}
128217
}
129218
""");
@@ -165,10 +254,8 @@ private List<String> populateDataStreamWithBulkRequest() throws IOException {
165254
return ids;
166255
}
167256

168-
private Response performRequestAsUser1(Request request) throws IOException {
169-
request.setOptions(RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", basicAuthHeaderValue(USER, PASSWORD)).build());
170-
var response = client().performRequest(request);
171-
return response;
257+
private Response performRequest(String user, Request request) throws IOException {
258+
request.setOptions(RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", basicAuthHeaderValue(user, PASSWORD)).build());
259+
return client().performRequest(request);
172260
}
173-
174261
}

0 commit comments

Comments
 (0)