Skip to content

Commit a9347c9

Browse files
gwbrowngwbrown
committed
Fixes + tests for fixes for core failure store authentication
1 parent 2aa8c14 commit a9347c9

File tree

2 files changed

+101
-57
lines changed
  • x-pack/plugin
    • core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission
    • security/src/test/java/org/elasticsearch/xpack/security/authz/accesscontrol

2 files changed

+101
-57
lines changed

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

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -159,9 +159,21 @@ public IndicesPermission.AuthorizedComponents check(String name, IndexAbstractio
159159
return IndicesPermission.AuthorizedComponents.NONE;
160160
} else {
161161
if (resource == null) {
162-
return group.indexNameMatcher.test(name)
163-
? IndicesPermission.AuthorizedComponents.DATA
164-
: IndicesPermission.AuthorizedComponents.NONE;
162+
if (group.indexNameMatcher.test(name)) {
163+
if (isReadAction) {
164+
if (actionMatches && group.hasReadFailuresPrivilege) {
165+
return IndicesPermission.AuthorizedComponents.ALL;
166+
} else if (actionMatches) {
167+
return IndicesPermission.AuthorizedComponents.DATA;
168+
} else if (group.hasReadFailuresPrivilege) {
169+
return IndicesPermission.AuthorizedComponents.FAILURES;
170+
}
171+
} else { // not a read action
172+
return IndicesPermission.AuthorizedComponents.ALL;
173+
}
174+
} else {
175+
return IndicesPermission.AuthorizedComponents.NONE;
176+
}
165177
}
166178
assert name.equals(resource.getName());
167179
return switch (resource.getType()) {
@@ -180,7 +192,7 @@ public IndicesPermission.AuthorizedComponents check(String name, IndexAbstractio
180192
// And authorize it as a failure store index (i.e. no DLS/FLS)
181193
yield IndicesPermission.AuthorizedComponents.FAILURES;
182194
}
183-
} else { // not a failure store index
195+
} else if (actionMatches) { // not a failure store index
184196
if ((authByDataStream && group.indexNameMatcher.test(ds.getName()))
185197
|| group.indexNameMatcher.test(resource.getName())) {
186198
yield IndicesPermission.AuthorizedComponents.DATA;

‎x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/accesscontrol/IndicesPermissionTests.java‎

Lines changed: 85 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import java.util.ArrayList;
4848
import java.util.Collections;
4949
import java.util.List;
50+
import java.util.Map;
5051
import java.util.Set;
5152
import java.util.SortedMap;
5253
import java.util.stream.Collectors;
@@ -686,61 +687,92 @@ public void testIndicesPermissionHasFieldOrDocumentLevelSecurity() {
686687
assertThat(indicesPermission3.hasFieldOrDocumentLevelSecurity(), is(false));
687688
}
688689

689-
// ATHE: Make sure this functionality is tested elsewhere
690-
public void testResourceAuthorizedPredicateForDatastreams() {
690+
public void testFailureStoreReadAuthorization() {
691691
String dataStreamName = "logs-datastream";
692-
Metadata.Builder mb = Metadata.builder(
693-
DataStreamTestHelper.getClusterStateWithDataStreams(
694-
List.of(Tuple.tuple(dataStreamName, 1)),
695-
List.of(),
696-
Instant.now().toEpochMilli(),
697-
builder().build(),
698-
1
699-
).getMetadata()
700-
);
701-
DataStream dataStream = mb.dataStream(dataStreamName);
702-
IndexAbstraction backingIndex = new IndexAbstraction.ConcreteIndex(
703-
DataStreamTestHelper.createBackingIndex(dataStreamName, 1).build(),
704-
dataStream
705-
);
706-
IndexAbstraction concreteIndex = new IndexAbstraction.ConcreteIndex(
707-
IndexMetadata.builder("logs-index").settings(indexSettings(IndexVersion.current(), 1, 0)).build()
708-
);
709-
AliasMetadata aliasMetadata = new AliasMetadata.Builder("logs-alias").build();
710-
IndexAbstraction alias = new IndexAbstraction.Alias(
711-
aliasMetadata,
712-
List.of(
713-
IndexMetadata.builder("logs-index").settings(indexSettings(IndexVersion.current(), 1, 0)).putAlias(aliasMetadata).build()
692+
693+
IndexMetadata backingIndexMd = DataStreamTestHelper.createBackingIndex(dataStreamName, 1).build();
694+
IndexMetadata failureIndexMd = DataStreamTestHelper.createFailureStore(dataStreamName, 1).build();
695+
IndexMetadata regularIndex = IndexMetadata.builder("logs-index").settings(indexSettings(IndexVersion.current(), 1, 0)).build();
696+
697+
Metadata.Builder mb = Metadata.builder()
698+
.dataStreams(
699+
Map.of(
700+
dataStreamName,
701+
DataStreamTestHelper.newInstance(dataStreamName, List.of(backingIndexMd.getIndex()), List.of(failureIndexMd.getIndex()))
702+
),
703+
Map.of()
714704
)
715-
);
716-
// IndicesPermission.IsResourceAuthorizedPredicate predicate = new IndicesPermission.IsResourceAuthorizedPredicate(
717-
// (String name, IndexAbstraction abstraction) -> {
718-
// StringMatcher regularNames = StringMatcher.of("other");
719-
// StringMatcher nonDatastreamNames = StringMatcher.of(
720-
// dataStreamName,
721-
// backingIndex.getName(),
722-
// concreteIndex.getName(),
723-
// alias.getName()
724-
// );
725-
// if (abstraction.getType() != IndexAbstraction.Type.DATA_STREAM && abstraction.getParentDataStream() == null) {
726-
// return regularNames.test(name)
727-
// ? IndicesPermission.AuthorizedComponents.DATA
728-
// : IndicesPermission.AuthorizedComponents.NONE;
729-
// } else {
730-
// return nonDatastreamNames.test(name)
731-
// ? IndicesPermission.AuthorizedComponents.DATA
732-
// : IndicesPermission.AuthorizedComponents.NONE;
733-
// }
734-
// }
735-
// );
736-
// assertThat(predicate.test(dataStream), is(false));
737-
// // test authorization for a missing resource with the datastream's name
738-
// assertThat(predicate.test(dataStream.getName(), null), is(IndicesPermission.AuthorizedComponents.DATA));
739-
// assertThat(predicate.test(backingIndex), is(false));
740-
// // test authorization for a missing resource with the backing index's name
741-
// assertThat(predicate.test(backingIndex.getName(), null), is(true));
742-
// assertThat(predicate.test(concreteIndex), is(true));
743-
// assertThat(predicate.test(alias), is(true));
705+
.indices(
706+
Map.of(
707+
backingIndexMd.getIndex().getName(),
708+
backingIndexMd,
709+
failureIndexMd.getIndex().getName(),
710+
failureIndexMd,
711+
regularIndex.getIndex().getName(),
712+
regularIndex
713+
)
714+
);
715+
DataStream dataStream = mb.dataStream(dataStreamName);
716+
717+
IndexAbstraction backingIndex = new IndexAbstraction.ConcreteIndex(backingIndexMd, dataStream);
718+
IndexAbstraction failureIndex = new IndexAbstraction.ConcreteIndex(failureIndexMd, dataStream);
719+
720+
IndexAbstraction concreteIndex = new IndexAbstraction.ConcreteIndex(regularIndex);
721+
final IndicesPermission everythingPermission = new IndicesPermission.Builder(RESTRICTED_INDICES).addGroup(
722+
IndexPrivilege.get(Set.of("all", "read_failures")),
723+
FieldPermissions.DEFAULT,
724+
null,
725+
true,
726+
"logs-data*"
727+
).addGroup(IndexPrivilege.NONE, FieldPermissions.DEFAULT, null, randomBoolean(), "logs-data*").build();
728+
final IndicesPermission dataOnlyPermission = new IndicesPermission.Builder(RESTRICTED_INDICES).addGroup(
729+
IndexPrivilege.get(Set.of("all")),
730+
FieldPermissions.DEFAULT,
731+
null,
732+
true,
733+
"logs-data*"
734+
).addGroup(IndexPrivilege.NONE, FieldPermissions.DEFAULT, null, randomBoolean(), "logs-data*").build();
735+
final IndicesPermission failuresOnlyPermission = new IndicesPermission.Builder(RESTRICTED_INDICES).addGroup(
736+
IndexPrivilege.get(Set.of("read_failures")),
737+
FieldPermissions.DEFAULT,
738+
null,
739+
true,
740+
"logs-data*"
741+
).addGroup(IndexPrivilege.NONE, FieldPermissions.DEFAULT, null, randomBoolean(), "logs-data*").build();
742+
743+
IsResourceAuthorizedPredicate everythingChecker = everythingPermission.allowedIndicesMatcher("indices:data/read/search");
744+
IsResourceAuthorizedPredicate onlyDataChecker = dataOnlyPermission.allowedIndicesMatcher("indices:data/read/search");
745+
IsResourceAuthorizedPredicate onlyFailuresChecker = failuresOnlyPermission.allowedIndicesMatcher("indices:data/read/search");
746+
747+
// Baseline check, nothing should be authorized for the concrete index
748+
assertThat(everythingChecker.check(concreteIndex).isDataAuthorized(), is(false));
749+
assertThat(onlyDataChecker.check(concreteIndex).isDataAuthorized(), is(false));
750+
assertThat(onlyFailuresChecker.check(concreteIndex).isDataAuthorized(), is(false));
751+
assertThat(everythingChecker.check(concreteIndex).isFailuresAuthorized(), is(false));
752+
assertThat(onlyDataChecker.check(concreteIndex).isFailuresAuthorized(), is(false));
753+
assertThat(onlyFailuresChecker.check(concreteIndex).isFailuresAuthorized(), is(false));
754+
755+
// Check the data stream name directly
756+
assertThat(everythingChecker.check(dataStreamName, null).isDataAuthorized(), is(true));
757+
assertThat(everythingChecker.check(dataStreamName, null).isFailuresAuthorized(), is(true));
758+
assertThat(onlyDataChecker.check(dataStreamName, null).isDataAuthorized(), is(true));
759+
assertThat(onlyDataChecker.check(dataStreamName, null).isFailuresAuthorized(), is(false));
760+
assertThat(onlyFailuresChecker.check(dataStreamName, null).isDataAuthorized(), is(false));
761+
assertThat(onlyFailuresChecker.check(dataStreamName, null).isFailuresAuthorized(), is(true));
762+
763+
assertThat(everythingChecker.check(concreteIndex).isFailuresAuthorized(), is(false));
764+
assertThat(onlyDataChecker.check(concreteIndex).isFailuresAuthorized(), is(false));
765+
assertThat(onlyFailuresChecker.check(concreteIndex).isFailuresAuthorized(), is(false));
766+
767+
// Check data index
768+
assertThat(everythingChecker.check(backingIndex).isDataAuthorized(), is(true));
769+
assertThat(onlyDataChecker.check(backingIndex).isDataAuthorized(), is(true));
770+
assertThat(onlyFailuresChecker.check(backingIndex).isDataAuthorized(), is(false));
771+
772+
// Check failures index
773+
assertThat(everythingChecker.check(failureIndex).isFailuresAuthorized(), is(true));
774+
assertThat(onlyDataChecker.check(failureIndex).isFailuresAuthorized(), is(false));
775+
assertThat(onlyFailuresChecker.check(failureIndex).isFailuresAuthorized(), is(true));
744776
}
745777

746778
public void testResourceAuthorizedPredicateAnd() {

0 commit comments

Comments
 (0)