Skip to content

Commit 11bd365

Browse files
nibixnibix
committed
wip
Signed-off-by: Nils Bandener <[email protected]>
1 parent be66450 commit 11bd365

File tree

8 files changed

+235
-95
lines changed

8 files changed

+235
-95
lines changed

‎src/integrationTest/java/org/opensearch/security/privileges/IndexAuthorizationReadOnlyIntTests.java‎

Lines changed: 105 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,11 @@
1414
import java.util.ArrayList;
1515
import java.util.Collection;
1616
import java.util.List;
17+
import java.util.function.Function;
18+
import java.util.function.Supplier;
1719

1820
import com.google.common.collect.ImmutableList;
21+
import org.junit.AfterClass;
1922
import org.junit.ClassRule;
2023
import org.junit.Test;
2124
import org.junit.runner.RunWith;
@@ -27,6 +30,8 @@
2730
import org.opensearch.test.framework.TestSecurityConfig;
2831
import org.opensearch.test.framework.cluster.LocalCluster;
2932
import org.opensearch.test.framework.cluster.TestRestClient;
33+
import org.slf4j.Logger;
34+
import org.slf4j.LoggerFactory;
3035

3136
import static org.hamcrest.MatcherAssert.assertThat;
3237
import static org.hamcrest.Matchers.containsString;
@@ -204,18 +209,27 @@ public class IndexAuthorizationReadOnlyIntTests {
204209
UNLIMITED_USER,
205210
SUPER_UNLIMITED_USER
206211
);
212+
private static final Logger log = LoggerFactory.getLogger(IndexAuthorizationReadOnlyIntTests.class);
213+
214+
static LocalCluster.Builder clusterBuilder() {
215+
return new LocalCluster.Builder().singleNode()
216+
.anonymousAuth(false)
217+
.authc(AUTHC_HTTPBASIC_INTERNAL)
218+
.users(USERS)//
219+
.indices(index_a1, index_a2, index_a3, index_b1, index_b2, index_b3, index_c1, index_hidden, index_hidden_dot)//
220+
.aliases(alias_ab1, alias_c1);
221+
}
207222

208-
@ClassRule
209-
public static LocalCluster cluster = new LocalCluster.Builder().singleNode()
210-
.anonymousAuth(false)
211-
.authc(AUTHC_HTTPBASIC_INTERNAL)
212-
.users(USERS)//
213-
.indices(index_a1, index_a2, index_a3, index_b1, index_b2, index_b3, index_c1, index_hidden, index_hidden_dot)//
214-
.aliases(alias_ab1, alias_c1)//
215-
.doNotFailOnForbidden(true)
216-
.build();
223+
@AfterClass
224+
public static void stopClusters() {
225+
for (ClusterConfig clusterConfig : ClusterConfig.values()) {
226+
clusterConfig.shutdown();
227+
}
228+
}
217229

218230
final TestSecurityConfig.User user;
231+
final LocalCluster cluster;
232+
final ClusterConfig clusterConfig;
219233

220234
@Test
221235
public void search_noPattern() throws Exception {
@@ -234,7 +248,11 @@ public void search_noPattern() throws Exception {
234248
public void search_noPattern_noWildcards() throws Exception {
235249
try (TestRestClient restClient = cluster.getRestClient(user)) {
236250
TestRestClient.HttpResponse httpResponse = restClient.get("/_search?size=1000&expand_wildcards=none");
237-
assertThat(httpResponse, containsExactly().at("hits.hits[*]._index").whenEmpty(200));
251+
if (user == UNLIMITED_USER || user == SUPER_UNLIMITED_USER) {
252+
assertThat(httpResponse, isOk());
253+
} else {
254+
assertThat(httpResponse, isForbidden("/error/root_cause/0/reason", "no permissions for [indices:data/read/search]"));
255+
}
238256
}
239257
}
240258

@@ -268,7 +286,11 @@ public void search_all() throws Exception {
268286
public void search_all_noWildcards() throws Exception {
269287
try (TestRestClient restClient = cluster.getRestClient(user)) {
270288
TestRestClient.HttpResponse httpResponse = restClient.get("_all/_search?size=1000&expand_wildcards=none");
271-
assertThat(httpResponse, containsExactly().at("hits.hits[*]._index").whenEmpty(200));
289+
if (user == UNLIMITED_USER || user == SUPER_UNLIMITED_USER) {
290+
assertThat(httpResponse, isOk());
291+
} else {
292+
assertThat(httpResponse, isForbidden("/error/root_cause/0/reason", "no permissions for [indices:data/read/search]"));
293+
}
272294
}
273295
}
274296

@@ -311,7 +333,11 @@ public void search_wildcard() throws Exception {
311333
public void search_wildcard_noWildcards() throws Exception {
312334
try (TestRestClient restClient = cluster.getRestClient(user)) {
313335
TestRestClient.HttpResponse httpResponse = restClient.get("*/_search?size=1000&expand_wildcards=none");
314-
assertThat(httpResponse, containsExactly().at("hits.hits[*]._index").whenEmpty(200));
336+
if (user == UNLIMITED_USER || user == SUPER_UNLIMITED_USER) {
337+
assertThat(httpResponse, isOk());
338+
} else {
339+
assertThat(httpResponse, isForbidden("/error/root_cause/0/reason", "no permissions for [indices:data/read/search]"));
340+
}
315341
}
316342
}
317343

@@ -439,17 +465,14 @@ public void search_indexPattern_noWildcards() throws Exception {
439465
TestRestClient.HttpResponse httpResponse = restClient.get(
440466
"index_a*,index_b*/_search?size=1000&expand_wildcards=none&ignore_unavailable=true"
441467
);
442-
assertThat(httpResponse, containsExactly().at("hits.hits[*]._index").but(user.indexMatcher("search")).whenEmpty(200));
443-
}
444-
}
445-
446-
@Test
447-
public void search_indexPatternAndStatic_noWildcards() throws Exception {
448-
try (TestRestClient restClient = cluster.getRestClient(user)) {
449-
TestRestClient.HttpResponse httpResponse = restClient.get(
450-
"index_a*,index_b1/_search?size=1000&expand_wildcards=none&ignore_unavailable=true"
451-
);
452-
assertThat(httpResponse, containsExactly(index_b1).at("hits.hits[*]._index").but(user.indexMatcher("search")).whenEmpty(403));
468+
// We have to specify the users here explicitly because here we need to check privileges for the
469+
// non-existing (and invalidly named) indices "index_a*" and "index_b*". Only users with privileges for "index_a*"
470+
// and "index_b*" will get a ok response.
471+
if (user == LIMITED_USER_A || user == LIMITED_USER_B || user == LIMITED_USER_A_HIDDEN || user == UNLIMITED_USER || user == SUPER_UNLIMITED_USER) {
472+
assertThat(httpResponse, isOk());
473+
} else {
474+
assertThat(httpResponse, isForbidden("", ""));
475+
}
453476
}
454477
}
455478

@@ -510,12 +533,18 @@ public void search_alias_pattern_negation() throws Exception {
510533
public void search_aliasAndIndex() throws Exception {
511534
try (TestRestClient restClient = cluster.getRestClient(user)) {
512535
TestRestClient.HttpResponse httpResponse = restClient.get("alias_ab1,index_b2/_search?size=1000&ignore_unavailable=true");
513-
assertThat(
514-
httpResponse,
515-
containsExactly(index_a1, index_a2, index_a3, index_b1, index_b2).at("hits.hits[*]._index")
516-
.but(user.indexMatcher("search"))
517-
.whenEmpty(403)
518-
);
536+
if (clusterConfig == ClusterConfig.LEGACY_PRIVILEGES_EVALUATION) {
537+
// The legacy privilege evaluation with dnfof enabled can replace aliases by a sub-set of its member indices
538+
assertThat(
539+
httpResponse,
540+
containsExactly(index_a1, index_a2, index_a3, index_b1, index_b2).at("hits.hits[*]._index")
541+
.but(user.indexMatcher("search"))
542+
.whenEmpty(403)
543+
);
544+
} else {
545+
// The new privilege evaluation never replaces aliases
546+
if (user.indexMatcher("search").covers(alias_ab1))
547+
}
519548
}
520549
}
521550

@@ -891,15 +920,6 @@ public void getAlias_aliasPattern() throws Exception {
891920
}
892921
}
893922

894-
@Test
895-
public void getAlias_aliasPattern_noWildcards() throws Exception {
896-
try (TestRestClient restClient = cluster.getRestClient(user)) {
897-
TestRestClient.HttpResponse httpResponse = restClient.get("_alias/alias_ab*?expand_wildcards=none");
898-
assertThat(httpResponse, isOk());
899-
assertThat(httpResponse.bodyAsJsonNode().isEmpty(), equalTo(true));
900-
}
901-
}
902-
903923
@Test
904924
public void getAlias_mixed() throws Exception {
905925
try (TestRestClient restClient = cluster.getRestClient(user)) {
@@ -1043,7 +1063,7 @@ public void field_caps_indexPattern() throws Exception {
10431063
TestRestClient.HttpResponse httpResponse = restClient.get("index_b*/_field_caps?fields=*");
10441064
assertThat(
10451065
httpResponse,
1046-
containsExactly(index_b1, index_b2, index_b3).at("indices").but(user.indexMatcher("read")).whenEmpty(403)
1066+
containsExactly(index_b1, index_b2, index_b3).at("indices").but(user.indexMatcher("read")).whenEmpty( clusterConfig == ClusterConfig.NEXT_GEN_PRIVILEGES_EVALUATION ? 200 : 403)
10471067
);
10481068
}
10491069
}
@@ -1155,19 +1175,60 @@ public void field_caps_indexPattern_minus() throws Exception {
11551175
}
11561176
}
11571177

1158-
@Parameterized.Parameters(name = "{1}")
1178+
@Parameterized.Parameters(name = "{0}, {2}")
11591179
public static Collection<Object[]> params() {
11601180
List<Object[]> result = new ArrayList<>();
11611181

1162-
for (TestSecurityConfig.User user : USERS) {
1163-
result.add(new Object[] { user, user.getDescription() });
1182+
for (ClusterConfig clusterConfig : ClusterConfig.values()) {
1183+
for (TestSecurityConfig.User user : USERS) {
1184+
result.add(new Object[]{clusterConfig, user, user.getDescription()});
1185+
}
11641186
}
1165-
11661187
return result;
11671188
}
11681189

1169-
public IndexAuthorizationReadOnlyIntTests(TestSecurityConfig.User user, String description) throws Exception {
1190+
public IndexAuthorizationReadOnlyIntTests(ClusterConfig clusterConfig, TestSecurityConfig.User user, String description) throws Exception {
11701191
this.user = user;
1192+
this.cluster = clusterConfig.cluster();
1193+
this.clusterConfig = clusterConfig;
1194+
}
1195+
1196+
public enum ClusterConfig {
1197+
LEGACY_PRIVILEGES_EVALUATION("legacy", c -> c.doNotFailOnForbidden(true)),
1198+
NEXT_GEN_PRIVILEGES_EVALUATION("next_gen", c-> c.privilegesEvaluationType("next_gen"));
1199+
1200+
final String name;
1201+
final Function<LocalCluster.Builder, LocalCluster.Builder> clusterConfiguration;
1202+
private LocalCluster cluster;
1203+
1204+
ClusterConfig(String name, Function<LocalCluster.Builder, LocalCluster.Builder> clusterConfiguration) {
1205+
this.name = name;
1206+
this.clusterConfiguration = clusterConfiguration;
1207+
}
1208+
1209+
LocalCluster cluster() {
1210+
if (cluster == null) {
1211+
cluster = this.clusterConfiguration.apply(clusterBuilder()).build();
1212+
cluster.before();
1213+
}
1214+
return cluster;
1215+
}
1216+
1217+
void shutdown() {
1218+
if (cluster != null) {
1219+
try {
1220+
cluster.close();
1221+
} catch (Exception e) {
1222+
e.printStackTrace();
1223+
}
1224+
cluster = null;
1225+
}
1226+
}
1227+
1228+
@Override
1229+
public String toString() {
1230+
return name;
1231+
}
11711232
}
11721233

11731234
}

‎src/integrationTest/java/org/opensearch/test/framework/TestSecurityConfig.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,11 @@ public TestSecurityConfig doNotFailOnForbidden(boolean doNotFailOnForbidden) {
139139
return this;
140140
}
141141

142+
public TestSecurityConfig privilegesEvaluationType(String privilegesEvaluationType) {
143+
config.privilegesEvaluationType(privilegesEvaluationType);
144+
return this;
145+
}
146+
142147
public TestSecurityConfig xff(XffConfig xffConfig) {
143148
config.xffConfig(xffConfig);
144149
return this;
@@ -263,6 +268,7 @@ public static class Config implements ToXContentObject {
263268
private boolean anonymousAuth;
264269

265270
private Boolean doNotFailOnForbidden;
271+
private String privilegesEvaluationType;
266272
private XffConfig xffConfig;
267273
private OnBehalfOfConfig onBehalfOfConfig;
268274
private Map<String, AuthcDomain> authcDomainMap = new LinkedHashMap<>();
@@ -280,6 +286,11 @@ public Config doNotFailOnForbidden(Boolean doNotFailOnForbidden) {
280286
return this;
281287
}
282288

289+
public Config privilegesEvaluationType(String privilegesEvaluationType) {
290+
this.privilegesEvaluationType = privilegesEvaluationType;
291+
return this;
292+
}
293+
283294
public Config xffConfig(XffConfig xffConfig) {
284295
this.xffConfig = xffConfig;
285296
return this;
@@ -325,6 +336,9 @@ public XContentBuilder toXContent(XContentBuilder xContentBuilder, Params params
325336
if (doNotFailOnForbidden != null) {
326337
xContentBuilder.field("do_not_fail_on_forbidden", doNotFailOnForbidden);
327338
}
339+
if (privilegesEvaluationType != null) {
340+
xContentBuilder.field("privileges_evaluation_type", privilegesEvaluationType);
341+
}
328342
xContentBuilder.field("authc", authcDomainMap);
329343
if (authzDomainMap.isEmpty() == false) {
330344
xContentBuilder.field("authz", authzDomainMap);

‎src/integrationTest/java/org/opensearch/test/framework/cluster/LocalCluster.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -597,6 +597,11 @@ public Builder doNotFailOnForbidden(boolean doNotFailOnForbidden) {
597597
return this;
598598
}
599599

600+
public Builder privilegesEvaluationType(String privilegesEvaluationType) {
601+
testSecurityConfig.privilegesEvaluationType(privilegesEvaluationType);
602+
return this;
603+
}
604+
600605
public Builder defaultConfigurationInitDirectory(String defaultConfigurationInitDirectory) {
601606
this.defaultConfigurationInitDirectory = defaultConfigurationInitDirectory;
602607
return this;

‎src/main/java/org/opensearch/security/privileges/PrivilegesConfiguration.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,10 +117,12 @@ public PrivilegesConfiguration(
117117
PrivilegesEvaluationType privilegesEvaluationType = PrivilegesEvaluationType.getFrom(
118118
configurationRepository.getConfiguration(CType.CONFIG)
119119
);
120-
PrivilegesEvaluationType currentEvaluationType = currentPrivilegesEvaluator == null ? null
121-
: currentPrivilegesEvaluator instanceof org.opensearch.security.privileges.actionlevel.legacy.PrivilegesEvaluator
120+
PrivilegesEvaluationType currentEvaluationType =
121+
currentPrivilegesEvaluator instanceof org.opensearch.security.privileges.actionlevel.legacy.PrivilegesEvaluator
122122
? PrivilegesEvaluationType.LEGACY
123-
: PrivilegesEvaluationType.NEXT_GEN;
123+
: currentPrivilegesEvaluator instanceof org.opensearch.security.privileges.actionlevel.nextgen.PrivilegesEvaluator ?
124+
PrivilegesEvaluationType.NEXT_GEN
125+
: null;
124126

125127
if (privilegesEvaluationType != currentEvaluationType) {
126128
if (privilegesEvaluationType == PrivilegesEvaluationType.LEGACY) {

‎src/main/java/org/opensearch/security/privileges/PrivilegesEvaluatorResponse.java‎

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,18 @@
3131
import java.util.Collections;
3232
import java.util.HashSet;
3333
import java.util.List;
34+
import java.util.Objects;
35+
import java.util.Optional;
3436
import java.util.Set;
3537

38+
import com.google.common.collect.ImmutableList;
3639
import com.google.common.collect.ImmutableSet;
3740

3841
import org.opensearch.action.admin.indices.create.CreateIndexRequestBuilder;
3942

4043
import com.selectivem.collections.CheckTable;
44+
import org.opensearch.cluster.metadata.OptionallyResolvedIndices;
45+
import org.opensearch.cluster.metadata.ResolvedIndices;
4146

4247
public class PrivilegesEvaluatorResponse {
4348
boolean allowed = false;
@@ -46,6 +51,7 @@ public class PrivilegesEvaluatorResponse {
4651
CreateIndexRequestBuilder createIndexRequestBuilder;
4752
private Set<String> onlyAllowedForIndices = ImmutableSet.of();
4853
private CheckTable<String, String> indexToActionCheckTable;
54+
private ImmutableList<PrivilegesEvaluatorResponse> subResults = ImmutableList.of();
4955
private String privilegeMatrix;
5056
private String reason;
5157

@@ -127,7 +133,23 @@ public String getPrivilegeMatrix() {
127133
String result = this.privilegeMatrix;
128134

129135
if (result == null) {
130-
result = this.indexToActionCheckTable.toTableString("ok", "MISSING");
136+
String topLevelMatrix;
137+
138+
if (this.indexToActionCheckTable != null) {
139+
topLevelMatrix = this.indexToActionCheckTable.toTableString("ok", "MISSING");
140+
} else {
141+
topLevelMatrix = "n/a";
142+
}
143+
144+
if (subResults.isEmpty()) {
145+
result = topLevelMatrix;
146+
} else {
147+
StringBuilder resultBuilder = new StringBuilder(topLevelMatrix);
148+
for (PrivilegesEvaluatorResponse subResult : subResults) {
149+
resultBuilder.append("\n");
150+
resultBuilder.append(subResult.getPrivilegeMatrix());
151+
}
152+
}
131153
this.privilegeMatrix = result;
132154
}
133155
return result;
@@ -159,6 +181,18 @@ public boolean isPending() {
159181
return this.state == PrivilegesEvaluatorResponseState.PENDING;
160182
}
161183

184+
public PrivilegesEvaluatorResponse insufficient(List<PrivilegesEvaluatorResponse> subResults) {
185+
String reason = this.reason;
186+
if (reason == null) {
187+
reason = subResults.stream().map(result -> result.reason).filter(Objects::nonNull).findFirst().orElse(null);
188+
}
189+
PrivilegesEvaluatorResponse result = new PrivilegesEvaluatorResponse();
190+
result.allowed = false;
191+
result.indexToActionCheckTable = this.indexToActionCheckTable;
192+
result.subResults = ImmutableList.copyOf(subResults);
193+
return result;
194+
}
195+
162196
@Override
163197
public String toString() {
164198
return "PrivEvalResponse [\nallowed="
@@ -176,6 +210,13 @@ public static PrivilegesEvaluatorResponse ok() {
176210
return response;
177211
}
178212

213+
public static PrivilegesEvaluatorResponse ok(CheckTable<String, String> indexToActionCheckTable) {
214+
PrivilegesEvaluatorResponse response = new PrivilegesEvaluatorResponse();
215+
response.indexToActionCheckTable = indexToActionCheckTable;
216+
response.allowed = true;
217+
return response;
218+
}
219+
179220
public static PrivilegesEvaluatorResponse ok(CreateIndexRequestBuilder createIndexRequestBuilder) {
180221
PrivilegesEvaluatorResponse response = new PrivilegesEvaluatorResponse();
181222
response.allowed = true;

0 commit comments

Comments
 (0)