Skip to content

Commit 6ae9f84

Browse files
n1v0lgn1v0lg
committed
Javadoc and test fixes
1 parent 79433cb commit 6ae9f84

File tree

12 files changed

+71
-39
lines changed

12 files changed

+71
-39
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,7 @@ public boolean checkResourcePrivileges(
302302
}
303303
}
304304
for (String privilege : checkForPrivileges) {
305-
IndexPrivilege indexPrivilege = IndexPrivilege.getSingle(Collections.singleton(privilege));
305+
IndexPrivilege indexPrivilege = IndexPrivilege.getSingleSelector(Collections.singleton(privilege));
306306
if (allowedIndexPrivilegesAutomaton != null
307307
&& Automatons.subsetOf(indexPrivilege.getAutomaton(), allowedIndexPrivilegesAutomaton)) {
308308
if (resourcePrivilegesMapBuilder != null) {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -416,7 +416,7 @@ public ManageRolesPrivilege(List<ManageRolesIndexPermissionGroup> manageRolesInd
416416
for (ManageRolesIndexPermissionGroup indexPatternPrivilege : manageRolesIndexPermissionGroups) {
417417
// TODO handle selectors
418418
indicesPermissionBuilder.addGroup(
419-
IndexPrivilege.getSingle(Set.of(indexPatternPrivilege.privileges())),
419+
IndexPrivilege.getSingleSelector(Set.of(indexPatternPrivilege.privileges())),
420420
FieldPermissions.DEFAULT,
421421
null,
422422
false,

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

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -271,16 +271,26 @@ private IndexPrivilege(Set<String> name, Automaton automaton, IndexComponentSele
271271
this.selectorPrivilege = selectorPrivilege;
272272
}
273273

274-
// TODO javadoc
275-
public static IndexPrivilege getSingle(Set<String> name) {
274+
/**
275+
* TODO more detail
276+
* Returns an index privilege for a single selector. Delegates to {@link #getSplitBySelector(Set)} but throws if the result has more
277+
* than one selector. The caller must ensure that the names only map to privileges with the same selector.
278+
*/
279+
public static IndexPrivilege getSingleSelector(Set<String> name) {
276280
var single = getSplitBySelector(name);
277281
if (single.size() != 1) {
278282
throw new IllegalArgumentException("expected singleton");
279283
}
280284
return single.iterator().next();
281285
}
282286

283-
// TODO javadoc
287+
/**
288+
* TODO more detail
289+
* Returns a set of index privileges, each privilege responsible for a separate selector.
290+
* For instance, `getSplitBySelector(Set.of("view_index_metadata", "write", "read_failure_store"))` will return two index privileges
291+
* one covering "view_index_metadata" and "write" for a {@link IndexComponentSelectorPrivilege#DATA}, the other covering
292+
* "read_failure_store" for a {@link IndexComponentSelectorPrivilege#FAILURES} selector.
293+
*/
284294
public static Set<IndexPrivilege> getSplitBySelector(Set<String> name) {
285295
return CACHE.computeIfAbsent(name, (theName) -> {
286296
if (theName.isEmpty()) {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ private static FieldPermissions randomFlsPermissions(String... grantedFields) {
240240

241241
private static IndexPrivilege randomIndexPrivilege() {
242242
// TODO handle failure store
243-
return IndexPrivilege.getSingle(Set.of(randomFrom(IndexPrivilege.names())));
243+
return IndexPrivilege.getSingleSelector(Set.of(randomFrom(IndexPrivilege.names())));
244244
}
245245

246246
public void testGetRoleDescriptorsIntersectionForRemoteClusterReturnsEmpty() {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ public void testGetRoleDescriptorsIntersectionForRemoteCluster() {
177177
new FieldPermissions(new FieldPermissionsDefinition(new String[] { randomAlphaOfLength(5) }, null)),
178178
null,
179179
// TODO handle failure store
180-
IndexPrivilege.getSingle(Set.of(randomFrom(IndexPrivilege.names()))),
180+
IndexPrivilege.getSingleSelector(Set.of(randomFrom(IndexPrivilege.names()))),
181181
randomBoolean(),
182182
randomAlphaOfLength(9)
183183
)

‎x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/IndexPrivilegeTests.java‎

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -84,39 +84,39 @@ public void testPrivilegesForGetCheckPointAction() {
8484
public void testRelationshipBetweenPrivileges() {
8585
assertThat(
8686
Automatons.subsetOf(
87-
IndexPrivilege.getSingle(Set.of("view_index_metadata")).automaton,
88-
IndexPrivilege.getSingle(Set.of("manage")).automaton
87+
IndexPrivilege.getSingleSelector(Set.of("view_index_metadata")).automaton,
88+
IndexPrivilege.getSingleSelector(Set.of("manage")).automaton
8989
),
9090
is(true)
9191
);
9292

9393
assertThat(
9494
Automatons.subsetOf(
95-
IndexPrivilege.getSingle(Set.of("monitor")).automaton,
96-
IndexPrivilege.getSingle(Set.of("manage")).automaton
95+
IndexPrivilege.getSingleSelector(Set.of("monitor")).automaton,
96+
IndexPrivilege.getSingleSelector(Set.of("manage")).automaton
9797
),
9898
is(true)
9999
);
100100

101101
assertThat(
102102
Automatons.subsetOf(
103-
IndexPrivilege.getSingle(Set.of("create", "create_doc", "index", "delete")).automaton,
104-
IndexPrivilege.getSingle(Set.of("write")).automaton
103+
IndexPrivilege.getSingleSelector(Set.of("create", "create_doc", "index", "delete")).automaton,
104+
IndexPrivilege.getSingleSelector(Set.of("write")).automaton
105105
),
106106
is(true)
107107
);
108108

109109
assertThat(
110110
Automatons.subsetOf(
111-
IndexPrivilege.getSingle(Set.of("create_index", "delete_index")).automaton,
112-
IndexPrivilege.getSingle(Set.of("manage")).automaton
111+
IndexPrivilege.getSingleSelector(Set.of("create_index", "delete_index")).automaton,
112+
IndexPrivilege.getSingleSelector(Set.of("manage")).automaton
113113
),
114114
is(true)
115115
);
116116
}
117117

118118
public void testCrossClusterReplicationPrivileges() {
119-
final IndexPrivilege crossClusterReplication = IndexPrivilege.getSingle(Set.of("cross_cluster_replication"));
119+
final IndexPrivilege crossClusterReplication = IndexPrivilege.getSingleSelector(Set.of("cross_cluster_replication"));
120120
List.of(
121121
"indices:data/read/xpack/ccr/shard_changes",
122122
"indices:monitor/stats",
@@ -125,11 +125,16 @@ public void testCrossClusterReplicationPrivileges() {
125125
"indices:admin/seq_no/renew_retention_lease"
126126
).forEach(action -> assertThat(crossClusterReplication.predicate.test(action + randomAlphaOfLengthBetween(0, 8)), is(true)));
127127
assertThat(
128-
Automatons.subsetOf(crossClusterReplication.automaton, IndexPrivilege.getSingle(Set.of("manage", "read", "monitor")).automaton),
128+
Automatons.subsetOf(
129+
crossClusterReplication.automaton,
130+
IndexPrivilege.getSingleSelector(Set.of("manage", "read", "monitor")).automaton
131+
),
129132
is(true)
130133
);
131134

132-
final IndexPrivilege crossClusterReplicationInternal = IndexPrivilege.getSingle(Set.of("cross_cluster_replication_internal"));
135+
final IndexPrivilege crossClusterReplicationInternal = IndexPrivilege.getSingleSelector(
136+
Set.of("cross_cluster_replication_internal")
137+
);
133138
List.of(
134139
"indices:internal/admin/ccr/restore/session/clear",
135140
"indices:internal/admin/ccr/restore/file_chunk/get",
@@ -142,11 +147,11 @@ public void testCrossClusterReplicationPrivileges() {
142147
);
143148

144149
assertThat(
145-
Automatons.subsetOf(crossClusterReplicationInternal.automaton, IndexPrivilege.getSingle(Set.of("manage")).automaton),
150+
Automatons.subsetOf(crossClusterReplicationInternal.automaton, IndexPrivilege.getSingleSelector(Set.of("manage")).automaton),
146151
is(false)
147152
);
148153
assertThat(
149-
Automatons.subsetOf(crossClusterReplicationInternal.automaton, IndexPrivilege.getSingle(Set.of("all")).automaton),
154+
Automatons.subsetOf(crossClusterReplicationInternal.automaton, IndexPrivilege.getSingleSelector(Set.of("all")).automaton),
150155
is(true)
151156
);
152157
}

‎x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/PrivilegeTests.java‎

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
package org.elasticsearch.xpack.core.security.authz.privilege;
88

99
import org.apache.lucene.tests.util.automaton.AutomatonTestUtil;
10+
import org.apache.lucene.util.automaton.Automaton;
1011
import org.elasticsearch.action.admin.cluster.health.TransportClusterHealthAction;
1112
import org.elasticsearch.action.admin.cluster.node.tasks.cancel.TransportCancelTasksAction;
1213
import org.elasticsearch.action.admin.cluster.reroute.TransportClusterRerouteAction;
@@ -66,6 +67,7 @@
6667
import org.junit.Rule;
6768
import org.junit.rules.ExpectedException;
6869

70+
import java.util.Iterator;
6971
import java.util.Set;
7072
import java.util.function.Predicate;
7173

@@ -203,29 +205,44 @@ public void testClusterAction() throws Exception {
203205

204206
public void testIndexAction() throws Exception {
205207
Set<String> actionName = Sets.newHashSet("indices:admin/mapping/delete");
206-
IndexPrivilege index = IndexPrivilege.getSingle(actionName);
208+
IndexPrivilege index = IndexPrivilege.getSingleSelector(actionName);
207209
assertThat(index, notNullValue());
208210
assertThat(index.predicate().test("indices:admin/mapping/delete"), is(true));
209211
assertThat(index.predicate().test("indices:admin/mapping/dele"), is(false));
210212
assertThat(IndexPrivilege.READ_CROSS_CLUSTER.predicate().test("internal:transport/proxy/indices:data/read/query"), is(true));
211213
}
212214

213-
public void testIndexCollapse() throws Exception {
214-
IndexPrivilege[] values = IndexPrivilege.values().values().toArray(new IndexPrivilege[IndexPrivilege.values().size()]);
215+
public void testIndexCollapse() {
216+
IndexPrivilege[] values = IndexPrivilege.values().values().toArray(new IndexPrivilege[0]);
215217
IndexPrivilege first = values[randomIntBetween(0, values.length - 1)];
216218
IndexPrivilege second = values[randomIntBetween(0, values.length - 1)];
217219

218220
Set<String> name = Sets.newHashSet(first.name().iterator().next(), second.name().iterator().next());
219-
IndexPrivilege index = IndexPrivilege.getSingle(name);
221+
Set<IndexPrivilege> indices = IndexPrivilege.getSplitBySelector(name);
222+
223+
Automaton automaton = null;
224+
if (indices.size() == 1) {
225+
IndexPrivilege index = indices.iterator().next();
226+
automaton = index.getAutomaton();
227+
} else if (indices.size() == 2) {
228+
Iterator<IndexPrivilege> it = indices.iterator();
229+
IndexPrivilege index1 = it.next();
230+
IndexPrivilege index2 = it.next();
231+
automaton = Automatons.unionAndMinimize(Set.of(index1.getAutomaton(), index2.getAutomaton()));
232+
} else {
233+
fail("indices.size() must be 1 or 2");
234+
assert false;
235+
}
220236

221237
if (Automatons.subsetOf(second.getAutomaton(), first.getAutomaton())) {
222-
assertTrue(AutomatonTestUtil.sameLanguage(index.getAutomaton(), first.getAutomaton()));
238+
assertTrue(AutomatonTestUtil.sameLanguage(automaton, first.getAutomaton()));
223239
} else if (Automatons.subsetOf(first.getAutomaton(), second.getAutomaton())) {
224-
assertTrue(AutomatonTestUtil.sameLanguage(index.getAutomaton(), second.getAutomaton()));
240+
assertTrue(AutomatonTestUtil.sameLanguage(automaton, second.getAutomaton()));
225241
} else {
226-
assertFalse(AutomatonTestUtil.sameLanguage(index.getAutomaton(), first.getAutomaton()));
227-
assertFalse(AutomatonTestUtil.sameLanguage(index.getAutomaton(), second.getAutomaton()));
242+
assertFalse(AutomatonTestUtil.sameLanguage(automaton, first.getAutomaton()));
243+
assertFalse(AutomatonTestUtil.sameLanguage(automaton, second.getAutomaton()));
228244
}
245+
229246
}
230247

231248
public void testSystem() throws Exception {

‎x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/support/AutomatonPatternsTests.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ public void testRemoteClusterPrivsDoNotOverlap() {
3737

3838
// check that the action patterns for remote CCS are not allowed by remote CCR privileges
3939
Arrays.stream(CCS_INDICES_PRIVILEGE_NAMES).forEach(ccsPrivilege -> {
40-
Automaton ccsAutomaton = IndexPrivilege.getSingle(Set.of(ccsPrivilege)).getAutomaton();
40+
Automaton ccsAutomaton = IndexPrivilege.getSingleSelector(Set.of(ccsPrivilege)).getAutomaton();
4141
Automatons.getPatterns(ccsAutomaton).forEach(ccsPattern -> {
4242
// emulate an action name that could be allowed by a CCS privilege
4343
String actionName = ccsPattern.replaceAll("\\*", randomAlphaOfLengthBetween(1, 8));
@@ -49,14 +49,14 @@ public void testRemoteClusterPrivsDoNotOverlap() {
4949
ccrPrivileges,
5050
ccsPattern
5151
);
52-
assertFalse(errorMessage, IndexPrivilege.getSingle(Set.of(ccrPrivileges)).predicate().test(actionName));
52+
assertFalse(errorMessage, IndexPrivilege.getSingleSelector(Set.of(ccrPrivileges)).predicate().test(actionName));
5353
});
5454
});
5555
});
5656

5757
// check that the action patterns for remote CCR are not allowed by remote CCS privileges
5858
Arrays.stream(CCR_INDICES_PRIVILEGE_NAMES).forEach(ccrPrivilege -> {
59-
Automaton ccrAutomaton = IndexPrivilege.getSingle(Set.of(ccrPrivilege)).getAutomaton();
59+
Automaton ccrAutomaton = IndexPrivilege.getSingleSelector(Set.of(ccrPrivilege)).getAutomaton();
6060
Automatons.getPatterns(ccrAutomaton).forEach(ccrPattern -> {
6161
// emulate an action name that could be allowed by a CCR privilege
6262
String actionName = ccrPattern.replaceAll("\\*", randomAlphaOfLengthBetween(1, 8));
@@ -72,7 +72,7 @@ public void testRemoteClusterPrivsDoNotOverlap() {
7272
ccsPrivileges,
7373
ccrPattern
7474
);
75-
assertFalse(errorMessage, IndexPrivilege.getSingle(Set.of(ccsPrivileges)).predicate().test(actionName));
75+
assertFalse(errorMessage, IndexPrivilege.getSingleSelector(Set.of(ccsPrivileges)).predicate().test(actionName));
7676
}
7777
});
7878
});

‎x-pack/plugin/security/qa/consistency-checks/src/test/java/org/elasticsearch/xpack/security/CrossClusterShardTests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ public void testCheckForNewShardLevelTransportActions() throws Exception {
112112

113113
List<String> shardActions = transportActionBindings.stream()
114114
.map(binding -> binding.getProvider().get())
115-
.filter(action -> IndexPrivilege.getSingle(crossClusterPrivilegeNames).predicate().test(action.actionName))
115+
.filter(action -> IndexPrivilege.getSingleSelector(crossClusterPrivilegeNames).predicate().test(action.actionName))
116116
.filter(this::actionIsLikelyShardAction)
117117
.map(action -> action.actionName)
118118
.toList();

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/DeprecationRoleDescriptorConsumer.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ private void logDeprecatedPermission(RoleDescriptor roleDescriptor) {
183183
for (Map.Entry<String, Set<String>> privilegesByAlias : privilegesByAliasMap.entrySet()) {
184184
final String aliasName = privilegesByAlias.getKey();
185185
final Set<String> aliasPrivilegeNames = privilegesByAlias.getValue();
186-
final Automaton aliasPrivilegeAutomaton = IndexPrivilege.getSingle(aliasPrivilegeNames).getAutomaton();
186+
final Automaton aliasPrivilegeAutomaton = IndexPrivilege.getSingleSelector(aliasPrivilegeNames).getAutomaton();
187187
final SortedSet<String> inferiorIndexNames = new TreeSet<>();
188188
// check if the alias grants superiors privileges than the indices it points to
189189
for (Index index : aliasOrIndexMap.get(aliasName).getIndices()) {
@@ -193,7 +193,7 @@ private void logDeprecatedPermission(RoleDescriptor roleDescriptor) {
193193
// compute automaton once per index no matter how many times it is pointed to
194194
final Automaton indexPrivilegeAutomaton = indexAutomatonMap.computeIfAbsent(
195195
index.getName(),
196-
i -> IndexPrivilege.getSingle(indexPrivileges).getAutomaton()
196+
i -> IndexPrivilege.getSingleSelector(indexPrivileges).getAutomaton()
197197
);
198198
if (false == Automatons.subsetOf(indexPrivilegeAutomaton, aliasPrivilegeAutomaton)) {
199199
inferiorIndexNames.add(index.getName());

0 commit comments

Comments
 (0)