Skip to content

Commit c8fc1f7

Browse files
n1v0lgn1v0lg
committed
Keep selector with index privilege
1 parent ffcf427 commit c8fc1f7

File tree

18 files changed

+163
-363
lines changed

18 files changed

+163
-363
lines changed

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

Lines changed: 3 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -81,13 +81,9 @@ public Builder addGroup(
8181
FieldPermissions fieldPermissions,
8282
@Nullable Set<BytesReference> query,
8383
boolean allowRestrictedIndices,
84-
IndexComponentSelectorPrivilege selectorPrivilege,
8584
String... indices
8685
) {
87-
assert privilege != IndexPrivilege.ALL || selectorPrivilege.isTotal() : "all privilege must be associated with all selector";
88-
groups.add(
89-
new Group(privilege, fieldPermissions, query, allowRestrictedIndices, restrictedIndices, selectorPrivilege, indices)
90-
);
86+
groups.add(new Group(privilege, fieldPermissions, query, allowRestrictedIndices, restrictedIndices, indices));
9187
return this;
9288
}
9389

@@ -808,34 +804,13 @@ public static class Group {
808804
// users. Setting this flag true eliminates the special status for the purpose of this permission - restricted indices still have
809805
// to be covered by the "indices"
810806
private final boolean allowRestrictedIndices;
811-
private final IndexComponentSelectorPrivilege selectorPrivilege;
812-
813-
public Group(
814-
IndexPrivilege privilege,
815-
FieldPermissions fieldPermissions,
816-
@Nullable Set<BytesReference> query,
817-
boolean allowRestrictedIndices,
818-
RestrictedIndices restrictedIndices,
819-
String... indices
820-
) {
821-
this(
822-
privilege,
823-
fieldPermissions,
824-
query,
825-
allowRestrictedIndices,
826-
restrictedIndices,
827-
IndexComponentSelectorPrivilege.DATA,
828-
indices
829-
);
830-
}
831807

832808
public Group(
833809
IndexPrivilege privilege,
834810
FieldPermissions fieldPermissions,
835811
@Nullable Set<BytesReference> query,
836812
boolean allowRestrictedIndices,
837813
RestrictedIndices restrictedIndices,
838-
IndexComponentSelectorPrivilege selectorPrivilege,
839814
String... indices
840815
) {
841816
assert indices.length != 0;
@@ -856,7 +831,6 @@ public Group(
856831
}
857832
this.fieldPermissions = Objects.requireNonNull(fieldPermissions);
858833
this.query = query;
859-
this.selectorPrivilege = selectorPrivilege;
860834
}
861835

862836
public IndexPrivilege privilege() {
@@ -890,7 +864,7 @@ boolean hasQuery() {
890864
}
891865

892866
public IndexComponentSelectorPrivilege getSelectorPrivilege() {
893-
return selectorPrivilege;
867+
return privilege.getSelectorPrivilege();
894868
}
895869

896870
public boolean allowRestrictedIndices() {
@@ -906,9 +880,7 @@ boolean isTotal() {
906880
&& indexNameMatcher.isTotal()
907881
&& privilege == IndexPrivilege.ALL
908882
&& query == null
909-
&& false == fieldPermissions.hasFieldLevelSecurity()
910-
// TODO ensure we want this now, not in a follow up instead
911-
&& selectorPrivilege.isTotal();
883+
&& false == fieldPermissions.hasFieldLevelSecurity();
912884
}
913885

914886
@Override
@@ -924,8 +896,6 @@ public String toString() {
924896
+ query
925897
+ ", allowRestrictedIndices="
926898
+ allowRestrictedIndices
927-
+ ", selectorPrivilege="
928-
+ selectorPrivilege
929899
+ '}';
930900
}
931901
}

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
import org.elasticsearch.common.bytes.BytesReference;
1111
import org.elasticsearch.core.Nullable;
1212
import org.elasticsearch.xpack.core.security.authz.RestrictedIndices;
13-
import org.elasticsearch.xpack.core.security.authz.privilege.IndexComponentSelectorPrivilege;
1413
import org.elasticsearch.xpack.core.security.authz.privilege.IndexPrivilege;
1514
import org.elasticsearch.xpack.core.security.support.Automatons;
1615
import org.elasticsearch.xpack.core.security.support.StringMatcher;
@@ -67,8 +66,6 @@ public Builder addGroup(
6766
// Deliberately passing EMPTY here since *which* indices are restricted is determined not on the querying cluster
6867
// but rather on the fulfilling cluster
6968
new RestrictedIndices(Automatons.EMPTY),
70-
// TODO
71-
IndexComponentSelectorPrivilege.DATA,
7269
indices
7370
)
7471
);

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

Lines changed: 8 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -253,20 +253,17 @@ public Builder runAs(Privilege privilege) {
253253
}
254254

255255
public Builder add(IndexPrivilege privilege, String... indices) {
256-
return add(FieldPermissions.DEFAULT, null, privilege, false, IndexComponentSelectorPrivilege.DATA, indices);
256+
return add(FieldPermissions.DEFAULT, null, privilege, false, indices);
257257
}
258258

259259
public Builder add(
260260
FieldPermissions fieldPermissions,
261261
Set<BytesReference> query,
262262
IndexPrivilege privilege,
263263
boolean allowRestrictedIndices,
264-
IndexComponentSelectorPrivilege selectorPrivilege,
265264
String... indices
266265
) {
267-
groups.add(
268-
new IndicesPermissionGroupDefinition(privilege, fieldPermissions, query, allowRestrictedIndices, selectorPrivilege, indices)
269-
);
266+
groups.add(new IndicesPermissionGroupDefinition(privilege, fieldPermissions, query, allowRestrictedIndices, indices));
270267
return this;
271268
}
272269

@@ -279,17 +276,7 @@ public Builder addRemoteIndicesGroup(
279276
final String... indices
280277
) {
281278
remoteIndicesGroups.computeIfAbsent(remoteClusterAliases, k -> new ArrayList<>())
282-
.add(
283-
new IndicesPermissionGroupDefinition(
284-
privilege,
285-
fieldPermissions,
286-
query,
287-
allowRestrictedIndices,
288-
// TODO
289-
IndexComponentSelectorPrivilege.DATA,
290-
indices
291-
)
292-
);
279+
.add(new IndicesPermissionGroupDefinition(privilege, fieldPermissions, query, allowRestrictedIndices, indices));
293280
return this;
294281
}
295282

@@ -329,7 +316,6 @@ public SimpleRole build() {
329316
group.fieldPermissions,
330317
group.query,
331318
group.allowRestrictedIndices,
332-
group.selectorPrivilege,
333319
group.indices
334320
);
335321
}
@@ -378,22 +364,19 @@ private static class IndicesPermissionGroupDefinition {
378364
private final FieldPermissions fieldPermissions;
379365
private final @Nullable Set<BytesReference> query;
380366
private final boolean allowRestrictedIndices;
381-
private final IndexComponentSelectorPrivilege selectorPrivilege;
382367
private final String[] indices;
383368

384369
private IndicesPermissionGroupDefinition(
385370
IndexPrivilege privilege,
386371
FieldPermissions fieldPermissions,
387372
@Nullable Set<BytesReference> query,
388373
boolean allowRestrictedIndices,
389-
IndexComponentSelectorPrivilege selectorPrivilege,
390374
String... indices
391375
) {
392376
this.privilege = privilege;
393377
this.fieldPermissions = fieldPermissions;
394378
this.query = query;
395379
this.allowRestrictedIndices = allowRestrictedIndices;
396-
this.selectorPrivilege = selectorPrivilege;
397380
this.indices = indices;
398381
}
399382
}
@@ -428,18 +411,14 @@ static SimpleRole buildFromRoleDescriptor(
428411
);
429412
Set<BytesReference> query = indexPrivilege.getQuery() == null ? null : Collections.singleton(indexPrivilege.getQuery());
430413
boolean allowRestrictedIndices = indexPrivilege.allowRestrictedIndices();
431-
Map<IndexComponentSelectorPrivilege, Set<String>> split = IndexComponentSelectorPrivilege.groupBySelector(
414+
Map<IndexComponentSelectorPrivilege, Set<String>> split = IndexComponentSelectorPrivilege.partitionBySelectorPrivilege(
432415
indexPrivilege.getPrivileges()
433416
);
434417
for (var entry : split.entrySet()) {
435-
builder.add(
436-
fieldPermissions,
437-
query,
438-
IndexPrivilege.get(entry.getValue()),
439-
allowRestrictedIndices,
440-
entry.getKey(),
441-
indexPrivilege.getIndices()
442-
);
418+
IndexPrivilege privilege = IndexPrivilege.get(entry.getValue());
419+
assert privilege.getSelectorPrivilege() == entry.getKey()
420+
: "expected selector privilege to match the partitioned privilege";
421+
builder.add(fieldPermissions, query, privilege, allowRestrictedIndices, indexPrivilege.getIndices());
443422
}
444423
}
445424

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -420,7 +420,6 @@ public ManageRolesPrivilege(List<ManageRolesIndexPermissionGroup> manageRolesInd
420420
null,
421421
false,
422422
// TODO
423-
IndexComponentSelectorPrivilege.DATA,
424423
indexPatternPrivilege.indexPatterns()
425424
);
426425
}

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

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,6 @@ public record IndexComponentSelectorPrivilege(String name, Predicate<IndexCompon
2727
IndexComponentSelector.FAILURES::equals
2828
);
2929

30-
private static final Set<IndexPrivilege> FAILURE_STORE_PRIVILEGE_NAMES = Set.of(
31-
IndexPrivilege.READ_FAILURE_STORE,
32-
IndexPrivilege.MANAGE_FAILURE_STORE_INTERNAL
33-
);
34-
3530
public boolean grants(IndexComponentSelector selector) {
3631
return predicate.test(selector);
3732
}
@@ -44,11 +39,11 @@ public static Set<IndexComponentSelectorPrivilege> get(Set<String> indexPrivileg
4439
return indexPrivileges.stream().map(IndexComponentSelectorPrivilege::get).collect(Collectors.toSet());
4540
}
4641

47-
public static Map<IndexComponentSelectorPrivilege, Set<String>> groupBySelector(String... indexPrivileges) {
48-
return groupBySelector(Set.of(indexPrivileges));
42+
public static Map<IndexComponentSelectorPrivilege, Set<String>> partitionBySelectorPrivilege(String... indexPrivileges) {
43+
return partitionBySelectorPrivilege(Set.of(indexPrivileges));
4944
}
5045

51-
public static Map<IndexComponentSelectorPrivilege, Set<String>> groupBySelector(Set<String> indexPrivileges) {
46+
public static Map<IndexComponentSelectorPrivilege, Set<String>> partitionBySelectorPrivilege(Set<String> indexPrivileges) {
5247
final Set<String> dataAccessPrivileges = new HashSet<>();
5348
final Set<String> failuresAccessPrivileges = new HashSet<>();
5449

@@ -82,14 +77,6 @@ public static Map<IndexComponentSelectorPrivilege, Set<String>> groupBySelector(
8277
private static IndexComponentSelectorPrivilege get(String indexPrivilegeName) {
8378
final IndexPrivilege indexPrivilege = IndexPrivilege.getNamedOrNull(indexPrivilegeName);
8479
// `null` means we got a raw action instead of a named privilege; all raw actions are treated as data access
85-
if (indexPrivilege == null) {
86-
return DATA;
87-
} else if (indexPrivilege == IndexPrivilege.ALL) {
88-
return ALL;
89-
} else if (FAILURE_STORE_PRIVILEGE_NAMES.contains(indexPrivilege)) {
90-
return FAILURES;
91-
} else {
92-
return DATA;
93-
}
80+
return indexPrivilege == null ? DATA : indexPrivilege.getSelectorPrivilege();
9481
}
9582
}

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

Lines changed: 47 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -178,12 +178,16 @@ public final class IndexPrivilege extends Privilege {
178178
);
179179

180180
public static final IndexPrivilege NONE = new IndexPrivilege("none", Automatons.EMPTY);
181-
public static final IndexPrivilege ALL = new IndexPrivilege("all", ALL_AUTOMATON);
182-
// TODO explain
183-
public static final IndexPrivilege READ_FAILURE_STORE = new IndexPrivilege("read_failure_store", READ_AUTOMATON);
181+
public static final IndexPrivilege ALL = new IndexPrivilege("all", ALL_AUTOMATON, IndexComponentSelectorPrivilege.ALL);
182+
public static final IndexPrivilege READ_FAILURE_STORE = new IndexPrivilege(
183+
"read_failure_store",
184+
READ_AUTOMATON,
185+
IndexComponentSelectorPrivilege.FAILURES
186+
);
184187
public static final IndexPrivilege MANAGE_FAILURE_STORE_INTERNAL = new IndexPrivilege(
185188
"manage_failure_store_internal",
186-
MANAGE_AUTOMATON
189+
MANAGE_AUTOMATON,
190+
IndexComponentSelectorPrivilege.FAILURES
187191
);
188192
public static final IndexPrivilege READ = new IndexPrivilege("read", READ_AUTOMATON);
189193
public static final IndexPrivilege READ_CROSS_CLUSTER = new IndexPrivilege("read_cross_cluster", READ_CROSS_CLUSTER_AUTOMATON);
@@ -253,12 +257,23 @@ public final class IndexPrivilege extends Privilege {
253257

254258
private static final ConcurrentHashMap<Set<String>, IndexPrivilege> CACHE = new ConcurrentHashMap<>();
255259

260+
private final IndexComponentSelectorPrivilege selectorPrivilege;
261+
256262
private IndexPrivilege(String name, Automaton automaton) {
257-
super(Collections.singleton(name), automaton);
263+
this(Collections.singleton(name), automaton);
264+
}
265+
266+
private IndexPrivilege(String name, Automaton automaton, IndexComponentSelectorPrivilege selectorPrivilege) {
267+
this(Collections.singleton(name), automaton, selectorPrivilege);
258268
}
259269

260270
private IndexPrivilege(Set<String> name, Automaton automaton) {
271+
this(name, automaton, IndexComponentSelectorPrivilege.DATA);
272+
}
273+
274+
private IndexPrivilege(Set<String> name, Automaton automaton, IndexComponentSelectorPrivilege selectorPrivilege) {
261275
super(name, automaton);
276+
this.selectorPrivilege = selectorPrivilege;
262277
}
263278

264279
public static IndexPrivilege get(Set<String> name) {
@@ -271,6 +286,10 @@ public static IndexPrivilege get(Set<String> name) {
271286
});
272287
}
273288

289+
public IndexComponentSelectorPrivilege getSelectorPrivilege() {
290+
return selectorPrivilege;
291+
}
292+
274293
public String getSingleName() {
275294
if (name().size() != 1) {
276295
throw new IllegalStateException("Expected a single name, but got: " + name());
@@ -291,6 +310,7 @@ private static IndexPrivilege resolve(Set<String> name) {
291310

292311
Set<String> actions = new HashSet<>();
293312
Set<Automaton> automata = new HashSet<>();
313+
Set<IndexComponentSelectorPrivilege> selectorPrivileges = new HashSet<>();
294314
for (String part : name) {
295315
part = part.toLowerCase(Locale.ROOT);
296316
if (ACTION_MATCHER.test(part)) {
@@ -301,6 +321,7 @@ private static IndexPrivilege resolve(Set<String> name) {
301321
return indexPrivilege;
302322
} else if (indexPrivilege != null) {
303323
automata.add(indexPrivilege.automaton);
324+
selectorPrivileges.add(indexPrivilege.getSelectorPrivilege());
304325
} else {
305326
String errorMessage = "unknown index privilege ["
306327
+ part
@@ -317,8 +338,20 @@ private static IndexPrivilege resolve(Set<String> name) {
317338

318339
if (actions.isEmpty() == false) {
319340
automata.add(patterns(actions));
341+
selectorPrivileges.add(IndexComponentSelectorPrivilege.DATA);
342+
}
343+
344+
for (IndexComponentSelectorPrivilege selectorPrivilege : selectorPrivileges) {
345+
if (selectorPrivilege == IndexComponentSelectorPrivilege.ALL) {
346+
return new IndexPrivilege(name, unionAndMinimize(automata), IndexComponentSelectorPrivilege.ALL);
347+
}
348+
}
349+
if (selectorPrivileges.size() != 1) {
350+
// TODO assertion and make this clearer
351+
throw new IllegalArgumentException("Cannot mix different selector privileges in a single index privilege for [" + name + "]");
320352
}
321-
return new IndexPrivilege(name, unionAndMinimize(automata));
353+
354+
return new IndexPrivilege(name, unionAndMinimize(automata), selectorPrivileges.iterator().next());
322355
}
323356

324357
static Map<String, IndexPrivilege> values() {
@@ -336,6 +369,13 @@ public static Set<String> names() {
336369
* @see Privilege#sortByAccessLevel
337370
*/
338371
public static Collection<String> findPrivilegesThatGrant(String action) {
339-
return VALUES.entrySet().stream().filter(e -> e.getValue().predicate.test(action)).map(e -> e.getKey()).toList();
372+
return VALUES.entrySet()
373+
.stream()
374+
.filter(e -> e.getValue().predicate.test(action))
375+
// Filter out the failure store privileges since these are confusing w.r.t. authorization failure messages are a handled
376+
// separately
377+
.filter(e -> false == (e.getValue().getSelectorPrivilege() == IndexComponentSelectorPrivilege.FAILURES))
378+
.map(Map.Entry::getKey)
379+
.toList();
340380
}
341381
}

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import org.elasticsearch.action.admin.cluster.repositories.get.GetRepositoriesAction;
1212
import org.elasticsearch.action.admin.indices.alias.TransportIndicesAliasesAction;
1313
import org.elasticsearch.action.admin.indices.rollover.RolloverAction;
14+
import org.elasticsearch.cluster.metadata.DataStream;
1415
import org.elasticsearch.common.Strings;
1516
import org.elasticsearch.common.settings.Setting;
1617
import org.elasticsearch.common.util.set.Sets;
@@ -78,7 +79,12 @@ public class ReservedRolesStore implements BiConsumer<Set<String>, ActionListene
7879
RoleDescriptor.IndicesPrivileges.builder().indices("*").privileges("all").allowRestrictedIndices(false).build(),
7980
RoleDescriptor.IndicesPrivileges.builder()
8081
.indices("*")
81-
.privileges("monitor", "read", "view_index_metadata", "read_cross_cluster", "read_failure_store")
82+
.privileges(
83+
// TODO are there edge-cases where this is a bad idea?
84+
DataStream.isFailureStoreFeatureFlagEnabled()
85+
? new String[] { "monitor", "read", "view_index_metadata", "read_cross_cluster", "read_failure_store" }
86+
: new String[] { "monitor", "read", "view_index_metadata", "read_cross_cluster" }
87+
)
8288
.allowRestrictedIndices(true)
8389
.build() },
8490
new RoleDescriptor.ApplicationResourcePrivileges[] {
@@ -95,7 +101,7 @@ public class ReservedRolesStore implements BiConsumer<Set<String>, ActionListene
95101
new RoleDescriptor.RemoteIndicesPrivileges(
96102
RoleDescriptor.IndicesPrivileges.builder()
97103
.indices("*")
98-
.privileges("monitor", "read", "view_index_metadata", "read_cross_cluster", "read_failure_store")
104+
.privileges("monitor", "read", "view_index_metadata", "read_cross_cluster")
99105
.allowRestrictedIndices(true)
100106
.build(),
101107
"*"

0 commit comments

Comments
 (0)