Skip to content

Commit 9275a69

Browse files
n1v0lgn1v0lg
committed
Handle wildcard and clean up
1 parent 24567ad commit 9275a69

File tree

10 files changed

+184
-111
lines changed

10 files changed

+184
-111
lines changed

‎build-tools/src/main/resources/roles.yml‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,6 @@ _es_test_root:
66
- names: [ "*" ]
77
allow_restricted_indices: true
88
privileges: [ "ALL" ]
9-
- names: [ "*::failures" ]
10-
allow_restricted_indices: true
11-
privileges: [ "ALL" ]
129
run_as: [ "*" ]
1310
applications:
1411
- application: "*"

‎test/test-clusters/src/main/resources/default_test_roles.yml‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,6 @@ _es_test_root:
66
- names: [ "*" ]
77
allow_restricted_indices: true
88
privileges: [ "ALL" ]
9-
- names: [ "*::failures" ]
10-
allow_restricted_indices: true
11-
privileges: [ "ALL" ]
129
run_as: [ "*" ]
1310
applications:
1411
- application: "*"

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

Lines changed: 36 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -80,12 +80,10 @@ public Builder addGroup(
8080
FieldPermissions fieldPermissions,
8181
@Nullable Set<BytesReference> query,
8282
boolean allowRestrictedIndices,
83-
boolean allowFailureStoreAccess,
83+
IndexComponentSelector selector,
8484
String... indices
8585
) {
86-
groups.add(
87-
new Group(privilege, fieldPermissions, query, allowRestrictedIndices, restrictedIndices, allowFailureStoreAccess, indices)
88-
);
86+
groups.add(new Group(privilege, fieldPermissions, query, allowRestrictedIndices, restrictedIndices, selector, indices));
8987
return this;
9088
}
9189

@@ -157,16 +155,24 @@ private IsResourceAuthorizedPredicate buildIndexMatcherPredicateForAction(String
157155
for (final Group group : groups) {
158156
if (group.actionMatcher.test(action)) {
159157
if (group.allowRestrictedIndices) {
160-
if (group.failureStoreOnly) {
161-
failureAccessRestrictedIndices.addAll(Arrays.asList(group.indices()));
162-
} else {
163-
dataAccessRestrictedIndices.addAll(Arrays.asList(group.indices()));
158+
switch (group.selector) {
159+
case DATA -> dataAccessRestrictedIndices.addAll(Arrays.asList(group.indices()));
160+
case FAILURES -> failureAccessRestrictedIndices.addAll(Arrays.asList(group.indices()));
161+
case ALL_APPLICABLE -> {
162+
dataAccessRestrictedIndices.addAll(Arrays.asList(group.indices()));
163+
failureAccessRestrictedIndices.addAll(Arrays.asList(group.indices()));
164+
}
165+
default -> throw new IllegalStateException("unexpected selector [" + group.selector + "]");
164166
}
165167
} else {
166-
if (group.failureStoreOnly) {
167-
failureAccessOrdinaryIndices.addAll(Arrays.asList(group.indices()));
168-
} else {
169-
dataAccessOrdinaryIndices.addAll(Arrays.asList(group.indices()));
168+
switch (group.selector) {
169+
case DATA -> dataAccessOrdinaryIndices.addAll(Arrays.asList(group.indices()));
170+
case FAILURES -> failureAccessOrdinaryIndices.addAll(Arrays.asList(group.indices()));
171+
case ALL_APPLICABLE -> {
172+
dataAccessOrdinaryIndices.addAll(Arrays.asList(group.indices()));
173+
failureAccessOrdinaryIndices.addAll(Arrays.asList(group.indices()));
174+
}
175+
default -> throw new IllegalStateException("unexpected selector [" + group.selector + "]");
170176
}
171177
}
172178
} else if (isMappingUpdateAction && containsPrivilegeThatGrantsMappingUpdatesForBwc(group)) {
@@ -456,30 +462,17 @@ public boolean checkIndex(Group group) {
456462
final DataStream ds = indexAbstraction == null ? null : indexAbstraction.getParentDataStream();
457463
if (ds != null) {
458464
if (group.checkIndex(ds.getName())) {
459-
// no selector implicitly means include data (?)
460-
if (selector == null || selector.shouldIncludeData()) {
461-
if (ds.isFailureStoreIndex(name)) {
462-
return group.failureStoreOnly;
463-
}
464-
return false == group.failureStoreOnly;
465-
} else {
466-
// TODO this is a simplification:
467-
return group.failureStoreOnly;
465+
// TODO is this right?
466+
if (indexAbstraction.isDataStreamConcreteFailureIndex()) {
467+
return group.checkSelector(IndexComponentSelector.FAILURES);
468468
}
469+
return group.checkSelector(selector);
469470
}
470471
}
471472
if (indexAbstraction != null && indexAbstraction.getType() == IndexAbstraction.Type.DATA_STREAM) {
472-
if (group.checkIndex(name)) {
473-
// no selector implicitly means include data (?)
474-
if (selector == null || selector.shouldIncludeData()) {
475-
return false == group.failureStoreOnly;
476-
} else {
477-
// TODO this is a simplification:
478-
return group.failureStoreOnly;
479-
}
480-
}
481-
return false;
473+
return group.checkIndex(name) && group.checkSelector(selector);
482474
}
475+
// TODO assertions around selector here?
483476
return group.checkIndex(name);
484477
}
485478

@@ -868,15 +861,15 @@ public static class Group {
868861
// users. Setting this flag true eliminates the special status for the purpose of this permission - restricted indices still have
869862
// to be covered by the "indices"
870863
private final boolean allowRestrictedIndices;
871-
public final boolean failureStoreOnly;
864+
public final IndexComponentSelector selector;
872865

873866
public Group(
874867
IndexPrivilege privilege,
875868
FieldPermissions fieldPermissions,
876869
@Nullable Set<BytesReference> query,
877870
boolean allowRestrictedIndices,
878871
RestrictedIndices restrictedIndices,
879-
boolean failureStoreOnly,
872+
IndexComponentSelector selector,
880873
String... indices
881874
) {
882875
assert indices.length != 0;
@@ -897,7 +890,7 @@ public Group(
897890
}
898891
this.fieldPermissions = Objects.requireNonNull(fieldPermissions);
899892
this.query = query;
900-
this.failureStoreOnly = failureStoreOnly;
893+
this.selector = selector;
901894
}
902895

903896
public IndexPrivilege privilege() {
@@ -926,6 +919,15 @@ private boolean checkIndex(String index) {
926919
return indexNameMatcher.test(index);
927920
}
928921

922+
private boolean checkSelector(@Nullable IndexComponentSelector selectorToCheck) {
923+
if (this.selector == IndexComponentSelector.ALL_APPLICABLE) {
924+
return true;
925+
}
926+
boolean includeData = selectorToCheck == null || selectorToCheck.shouldIncludeData();
927+
boolean includeFailures = selectorToCheck != null && selectorToCheck.shouldIncludeFailures();
928+
return includeData == this.selector.shouldIncludeData() && includeFailures == this.selector.shouldIncludeFailures();
929+
}
930+
929931
boolean hasQuery() {
930932
return query != null;
931933
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
package org.elasticsearch.xpack.core.security.authz.permission;
99

10+
import org.elasticsearch.action.support.IndexComponentSelector;
1011
import org.elasticsearch.common.bytes.BytesReference;
1112
import org.elasticsearch.core.Nullable;
1213
import org.elasticsearch.xpack.core.security.authz.RestrictedIndices;
@@ -67,7 +68,7 @@ public Builder addGroup(
6768
// but rather on the fulfilling cluster
6869
new RestrictedIndices(Automatons.EMPTY),
6970
// TODO handle failure store access
70-
false,
71+
IndexComponentSelector.DATA,
7172
indices
7273
)
7374
);

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

Lines changed: 44 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
import org.apache.lucene.util.automaton.Automaton;
1111
import org.elasticsearch.TransportVersion;
12+
import org.elasticsearch.action.support.IndexComponentSelector;
1213
import org.elasticsearch.cluster.metadata.Metadata;
1314
import org.elasticsearch.common.bytes.BytesReference;
1415
import org.elasticsearch.common.util.set.Sets;
@@ -252,7 +253,9 @@ public Builder runAs(Privilege privilege) {
252253
}
253254

254255
public Builder add(IndexPrivilege privilege, String... indices) {
255-
groups.add(new IndicesPermissionGroupDefinition(privilege, FieldPermissions.DEFAULT, null, false, false, indices));
256+
groups.add(
257+
new IndicesPermissionGroupDefinition(privilege, FieldPermissions.DEFAULT, null, false, IndexComponentSelector.DATA, indices)
258+
);
256259
return this;
257260
}
258261

@@ -264,28 +267,30 @@ public Builder add(
264267
boolean allowRestrictedIndices,
265268
String... indices
266269
) {
267-
return add(fieldPermissions, query, privilege, allowRestrictedIndices, false, indices);
270+
return add(fieldPermissions, query, privilege, allowRestrictedIndices, IndexComponentSelector.DATA, indices);
271+
}
272+
273+
public Builder add(
274+
FieldPermissions fieldPermissions,
275+
Set<BytesReference> query,
276+
IndexPrivilege privilege,
277+
boolean allowRestrictedIndices,
278+
boolean foo,
279+
String... indices
280+
) {
281+
return add(fieldPermissions, query, privilege, allowRestrictedIndices, IndexComponentSelector.DATA, indices);
268282
}
269283

270284
public Builder add(
271285
FieldPermissions fieldPermissions,
272286
Set<BytesReference> query,
273287
IndexPrivilege privilege,
274288
boolean allowRestrictedIndices,
275-
boolean allowFailureStoreAccess,
289+
IndexComponentSelector selector,
276290
String... indices
277291
) {
278292
// TODO assert no failures suffixes left
279-
groups.add(
280-
new IndicesPermissionGroupDefinition(
281-
privilege,
282-
fieldPermissions,
283-
query,
284-
allowRestrictedIndices,
285-
allowFailureStoreAccess,
286-
indices
287-
)
288-
);
293+
groups.add(new IndicesPermissionGroupDefinition(privilege, fieldPermissions, query, allowRestrictedIndices, selector, indices));
289294
return this;
290295
}
291296

@@ -299,7 +304,16 @@ public Builder addRemoteIndicesGroup(
299304
final String... indices
300305
) {
301306
remoteIndicesGroups.computeIfAbsent(remoteClusterAliases, k -> new ArrayList<>())
302-
.add(new IndicesPermissionGroupDefinition(privilege, fieldPermissions, query, allowRestrictedIndices, false, indices));
307+
.add(
308+
new IndicesPermissionGroupDefinition(
309+
privilege,
310+
fieldPermissions,
311+
query,
312+
allowRestrictedIndices,
313+
IndexComponentSelector.DATA,
314+
indices
315+
)
316+
);
303317
return this;
304318
}
305319

@@ -339,7 +353,7 @@ public SimpleRole build() {
339353
group.fieldPermissions,
340354
group.query,
341355
group.allowRestrictedIndices,
342-
group.allowFailureStoreAccess,
356+
group.selector,
343357
group.indices
344358
);
345359
}
@@ -388,22 +402,22 @@ private static class IndicesPermissionGroupDefinition {
388402
private final FieldPermissions fieldPermissions;
389403
private final @Nullable Set<BytesReference> query;
390404
private final boolean allowRestrictedIndices;
391-
private final boolean allowFailureStoreAccess;
405+
private final IndexComponentSelector selector;
392406
private final String[] indices;
393407

394408
private IndicesPermissionGroupDefinition(
395409
IndexPrivilege privilege,
396410
FieldPermissions fieldPermissions,
397411
@Nullable Set<BytesReference> query,
398412
boolean allowRestrictedIndices,
399-
boolean allowFailureStoreAccess,
413+
IndexComponentSelector selector,
400414
String... indices
401415
) {
402416
this.privilege = privilege;
403417
this.fieldPermissions = fieldPermissions;
404418
this.query = query;
405419
this.allowRestrictedIndices = allowRestrictedIndices;
406-
this.allowFailureStoreAccess = allowFailureStoreAccess;
420+
this.selector = selector;
407421
this.indices = indices;
408422
}
409423
}
@@ -442,21 +456,21 @@ static SimpleRole buildFromRoleDescriptor(
442456
indexPrivilege.getQuery() == null ? null : Collections.singleton(indexPrivilege.getQuery()),
443457
IndexPrivilege.get(Sets.newHashSet(indexPrivilege.getPrivileges())),
444458
indexPrivilege.allowRestrictedIndices(),
445-
true,
459+
IndexComponentSelector.ALL_APPLICABLE,
460+
indexPrivilege.getIndices()
461+
);
462+
} else {
463+
builder.add(
464+
fieldPermissionsCache.getFieldPermissions(
465+
new FieldPermissionsDefinition(indexPrivilege.getGrantedFields(), indexPrivilege.getDeniedFields())
466+
),
467+
indexPrivilege.getQuery() == null ? null : Collections.singleton(indexPrivilege.getQuery()),
468+
IndexPrivilege.get(Sets.newHashSet(indexPrivilege.getPrivileges())),
469+
indexPrivilege.allowRestrictedIndices(),
470+
IndexComponentSelector.DATA,
446471
indexPrivilege.getIndices()
447472
);
448473
}
449-
builder.add(
450-
fieldPermissionsCache.getFieldPermissions(
451-
new FieldPermissionsDefinition(indexPrivilege.getGrantedFields(), indexPrivilege.getDeniedFields())
452-
),
453-
indexPrivilege.getQuery() == null ? null : Collections.singleton(indexPrivilege.getQuery()),
454-
IndexPrivilege.get(Sets.newHashSet(indexPrivilege.getPrivileges())),
455-
indexPrivilege.allowRestrictedIndices(),
456-
// TODO properly handle this
457-
false,
458-
indexPrivilege.getIndices()
459-
);
460474
}
461475

462476
for (RoleDescriptor.RemoteIndicesPrivileges remoteIndicesPrivileges : roleDescriptor.getRemoteIndicesPrivileges()) {

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import org.apache.logging.log4j.LogManager;
1111
import org.apache.logging.log4j.Logger;
1212
import org.elasticsearch.TransportVersions;
13+
import org.elasticsearch.action.support.IndexComponentSelector;
1314
import org.elasticsearch.common.Strings;
1415
import org.elasticsearch.common.io.stream.StreamInput;
1516
import org.elasticsearch.common.io.stream.StreamOutput;
@@ -419,7 +420,7 @@ public ManageRolesPrivilege(List<ManageRolesIndexPermissionGroup> manageRolesInd
419420
FieldPermissions.DEFAULT,
420421
null,
421422
false,
422-
false,
423+
IndexComponentSelector.DATA,
423424
indexPatternPrivilege.indexPatterns()
424425
);
425426
}

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import org.elasticsearch.action.admin.indices.delete.TransportDeleteIndexAction;
1414
import org.elasticsearch.action.bulk.TransportBulkAction;
1515
import org.elasticsearch.action.search.TransportSearchAction;
16+
import org.elasticsearch.action.support.IndexComponentSelector;
1617
import org.elasticsearch.cluster.metadata.AliasMetadata;
1718
import org.elasticsearch.cluster.metadata.IndexAbstraction;
1819
import org.elasticsearch.cluster.metadata.IndexMetadata;
@@ -761,7 +762,15 @@ public void testHasPrivilegesForIndexPatterns() {
761762
}
762763
{
763764
fromRole = Role.builder(EMPTY_RESTRICTED_INDICES, "a-role")
764-
.add(FieldPermissions.DEFAULT, Collections.emptySet(), IndexPrivilege.READ, true, false, "ind-1*", ".security")
765+
.add(
766+
FieldPermissions.DEFAULT,
767+
Collections.emptySet(),
768+
IndexPrivilege.READ,
769+
true,
770+
IndexComponentSelector.DATA,
771+
"ind-1*",
772+
".security"
773+
)
765774
.build();
766775

767776
verifyResourcesPrivileges(

0 commit comments

Comments
 (0)