Skip to content

Commit 1362ab6

Browse files
jaymodejaymode
authored
Replace AuthorizedIndices class with a List (elastic#37328)
This change replaces the AuthorizedIndices class with a simple list. The change to a simple list does remove the lazy loading of the authorized indices in favor of simpler code as the loading of this list is now an asynchronous operation that is delegated to the authorization engine.
1 parent 0246442 commit 1362ab6

File tree

7 files changed

+64
-101
lines changed

7 files changed

+64
-101
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ void authorizeIndexAction(Authentication authentication, TransportRequest reques
3434
Function<String, AliasOrIndex> aliasOrIndexFunction,
3535
ActionListener<IndexAuthorizationResult> listener);
3636

37-
List<String> loadAuthorizedIndices(Authentication authentication, String action, AuthorizationInfo info,
38-
Map<String, AliasOrIndex> aliasAndIndexLookup);
37+
void loadAuthorizedIndices(Authentication authentication, String action, AuthorizationInfo info,
38+
Map<String, AliasOrIndex> aliasAndIndexLookup, ActionListener<List<String>> listener);
3939

4040
interface AuthorizationInfo {
4141

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

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -201,10 +201,9 @@ private void authorizeAction(final Authentication authentication, final String a
201201
authzEngine.authorizeClusterAction(authentication, unwrappedRequest, action, authzInfo, clusterAuthzListener);
202202
} else if (IndexPrivilege.ACTION_MATCHER.test(action)) {
203203
final MetaData metaData = clusterService.state().metaData();
204-
final AsyncSupplier<AuthorizedIndices> authorizedIndicesSupplier = new CachingAsyncSupplier<>(
205-
authzIndicesListener -> authzIndicesListener.onResponse(new AuthorizedIndices(
206-
() -> authzEngine.loadAuthorizedIndices(authentication, action, authzInfo, metaData.getAliasAndIndexLookup())))
207-
);
204+
final AsyncSupplier<List<String>> authorizedIndicesSupplier = new CachingAsyncSupplier<>(authzIndicesListener ->
205+
authzEngine.loadAuthorizedIndices(authentication, action, authzInfo, metaData.getAliasAndIndexLookup(),
206+
authzIndicesListener));
208207
final AsyncSupplier<ResolvedIndices> resolvedIndicesAsyncSupplier = new CachingAsyncSupplier<>((resolvedIndicesListener) -> {
209208
authorizedIndicesSupplier.get(ActionListener.wrap(authorizedIndices -> {
210209
resolveIndexNames(unwrappedRequest, metaData, authorizedIndices, resolvedIndicesListener);
@@ -366,7 +365,7 @@ private void authorizeRunAs(final Authentication authentication, final String ac
366365
*/
367366
private void authorizeBulkItems(Authentication authentication, BulkShardRequest request, AuthorizationInfo authzInfo,
368367
AuthorizationEngine authzEngine, AsyncSupplier<ResolvedIndices> resolvedIndicesAsyncSupplier,
369-
AsyncSupplier<AuthorizedIndices> authorizedIndicesSupplier,
368+
AsyncSupplier<List<String>> authorizedIndicesSupplier,
370369
MetaData metaData, String requestId, ActionListener<Void> listener) {
371370
// Maps original-index -> expanded-index-name (expands date-math, but not aliases)
372371
final Map<String, String> resolvedIndexNames = new HashMap<>();
@@ -466,7 +465,7 @@ private static String getAction(BulkItemRequest item) {
466465
throw new IllegalArgumentException("No equivalent action for opType [" + docWriteRequest.opType() + "]");
467466
}
468467

469-
private void resolveIndexNames(TransportRequest request, MetaData metaData, AuthorizedIndices authorizedIndices,
468+
private void resolveIndexNames(TransportRequest request, MetaData metaData, List<String> authorizedIndices,
470469
ActionListener<ResolvedIndices> listener) {
471470
listener.onResponse(indicesAndAliasesResolver.resolve(request, metaData, authorizedIndices));
472471
}

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

Lines changed: 0 additions & 30 deletions
This file was deleted.

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

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ class IndicesAndAliasesResolver {
8787
* Otherwise, <em>N</em> will be added to the <em>local</em> index list.
8888
*/
8989

90-
ResolvedIndices resolve(TransportRequest request, MetaData metaData, AuthorizedIndices authorizedIndices) {
90+
ResolvedIndices resolve(TransportRequest request, MetaData metaData, List<String> authorizedIndices) {
9191
if (request instanceof IndicesAliasesRequest) {
9292
ResolvedIndices.Builder resolvedIndicesBuilder = new ResolvedIndices.Builder();
9393
IndicesAliasesRequest indicesAliasesRequest = (IndicesAliasesRequest) request;
@@ -107,7 +107,7 @@ ResolvedIndices resolve(TransportRequest request, MetaData metaData, AuthorizedI
107107
}
108108

109109

110-
ResolvedIndices resolveIndicesAndAliases(IndicesRequest indicesRequest, MetaData metaData, AuthorizedIndices authorizedIndices) {
110+
ResolvedIndices resolveIndicesAndAliases(IndicesRequest indicesRequest, MetaData metaData, List<String> authorizedIndices) {
111111
final ResolvedIndices.Builder resolvedIndicesBuilder = new ResolvedIndices.Builder();
112112
boolean indicesReplacedWithNoIndices = false;
113113
if (indicesRequest instanceof PutMappingRequest && ((PutMappingRequest) indicesRequest).getConcreteIndex() != null) {
@@ -134,7 +134,7 @@ ResolvedIndices resolveIndicesAndAliases(IndicesRequest indicesRequest, MetaData
134134
// check for all and return list of authorized indices
135135
if (IndexNameExpressionResolver.isAllIndices(indicesList(indicesRequest.indices()))) {
136136
if (replaceWildcards) {
137-
for (String authorizedIndex : authorizedIndices.get()) {
137+
for (String authorizedIndex : authorizedIndices) {
138138
if (isIndexVisible(authorizedIndex, indicesOptions, metaData)) {
139139
resolvedIndicesBuilder.addLocal(authorizedIndex);
140140
}
@@ -150,11 +150,11 @@ ResolvedIndices resolveIndicesAndAliases(IndicesRequest indicesRequest, MetaData
150150
split = new ResolvedIndices(Arrays.asList(indicesRequest.indices()), Collections.emptyList());
151151
}
152152
List<String> replaced = replaceWildcardsWithAuthorizedIndices(split.getLocal(), indicesOptions, metaData,
153-
authorizedIndices.get(), replaceWildcards);
153+
authorizedIndices, replaceWildcards);
154154
if (indicesOptions.ignoreUnavailable()) {
155155
//out of all the explicit names (expanded from wildcards and original ones that were left untouched)
156156
//remove all the ones that the current user is not authorized for and ignore them
157-
replaced = replaced.stream().filter(authorizedIndices.get()::contains).collect(Collectors.toList());
157+
replaced = replaced.stream().filter(authorizedIndices::contains).collect(Collectors.toList());
158158
}
159159
resolvedIndicesBuilder.addLocal(replaced);
160160
resolvedIndicesBuilder.addRemote(split.getRemote());
@@ -195,7 +195,7 @@ ResolvedIndices resolveIndicesAndAliases(IndicesRequest indicesRequest, MetaData
195195
AliasesRequest aliasesRequest = (AliasesRequest) indicesRequest;
196196
if (aliasesRequest.expandAliasesWildcards()) {
197197
List<String> aliases = replaceWildcardsWithAuthorizedAliases(aliasesRequest.aliases(),
198-
loadAuthorizedAliases(authorizedIndices.get(), metaData));
198+
loadAuthorizedAliases(authorizedIndices, metaData));
199199
aliasesRequest.replaceAliases(aliases.toArray(new String[aliases.size()]));
200200
}
201201
if (indicesReplacedWithNoIndices) {
@@ -226,9 +226,8 @@ ResolvedIndices resolveIndicesAndAliases(IndicesRequest indicesRequest, MetaData
226226
* request's concrete index is not in the list of authorized indices, then we need to look to
227227
* see if this can be authorized against an alias
228228
*/
229-
static String getPutMappingIndexOrAlias(PutMappingRequest request, AuthorizedIndices authorizedIndices, MetaData metaData) {
229+
static String getPutMappingIndexOrAlias(PutMappingRequest request, List<String> authorizedIndicesList, MetaData metaData) {
230230
final String concreteIndexName = request.getConcreteIndex().getName();
231-
final List<String> authorizedIndicesList = authorizedIndices.get();
232231

233232
// validate that the concrete index exists, otherwise there is no remapping that we could do
234233
final AliasOrIndex aliasOrIndex = metaData.getAliasAndIndexLookup().get(concreteIndexName);

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -278,13 +278,14 @@ private void authorizeIndexActionName(String action, AuthorizationInfo authoriza
278278
}
279279

280280
@Override
281-
public List<String> loadAuthorizedIndices(Authentication authentication, String action, AuthorizationInfo authorizationInfo,
282-
Map<String, AliasOrIndex> aliasAndIndexLookup) {
281+
public void loadAuthorizedIndices(Authentication authentication, String action, AuthorizationInfo authorizationInfo,
282+
Map<String, AliasOrIndex> aliasAndIndexLookup, ActionListener<List<String>> listener) {
283283
if (authorizationInfo instanceof RBACAuthorizationInfo) {
284284
final Role role = ((RBACAuthorizationInfo) authorizationInfo).getRole();
285-
return resolveAuthorizedIndicesFromRole(role, action, aliasAndIndexLookup);
285+
listener.onResponse(resolveAuthorizedIndicesFromRole(role, action, aliasAndIndexLookup));
286286
} else {
287-
throw new IllegalArgumentException("unsupported authorization info:" + authorizationInfo.getClass().getSimpleName());
287+
listener.onFailure(
288+
new IllegalArgumentException("unsupported authorization info:" + authorizationInfo.getClass().getSimpleName()));
288289
}
289290
}
290291

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

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,9 @@
3131
public class AuthorizedIndicesTests extends ESTestCase {
3232

3333
public void testAuthorizedIndicesUserWithoutRoles() {
34-
AuthorizedIndices authorizedIndices = new AuthorizedIndices(
35-
() -> RBACEngine.resolveAuthorizedIndicesFromRole(Role.EMPTY, "", MetaData.EMPTY_META_DATA.getAliasAndIndexLookup()));
36-
List<String> list = authorizedIndices.get();
37-
assertTrue(list.isEmpty());
34+
List<String> authorizedIndices =
35+
RBACEngine.resolveAuthorizedIndicesFromRole(Role.EMPTY, "", MetaData.EMPTY_META_DATA.getAliasAndIndexLookup());
36+
assertTrue(authorizedIndices.isEmpty());
3837
}
3938

4039
public void testAuthorizedIndicesUserWithSomeRoles() {
@@ -60,20 +59,18 @@ public void testAuthorizedIndicesUserWithSomeRoles() {
6059
final Set<RoleDescriptor> descriptors = Sets.newHashSet(aStarRole, bRole);
6160
CompositeRolesStore.buildRoleFromDescriptors(descriptors, new FieldPermissionsCache(Settings.EMPTY), null, future);
6261
Role roles = future.actionGet();
63-
AuthorizedIndices authorizedIndices = new AuthorizedIndices(
64-
() -> RBACEngine.resolveAuthorizedIndicesFromRole(roles, SearchAction.NAME, metaData.getAliasAndIndexLookup()));
65-
List<String> list = authorizedIndices.get();
62+
List<String> list =
63+
RBACEngine.resolveAuthorizedIndicesFromRole(roles, SearchAction.NAME, metaData.getAliasAndIndexLookup());
6664
assertThat(list, containsInAnyOrder("a1", "a2", "aaaaaa", "b", "ab"));
6765
assertFalse(list.contains("bbbbb"));
6866
assertFalse(list.contains("ba"));
6967
}
7068

7169
public void testAuthorizedIndicesUserWithSomeRolesEmptyMetaData() {
7270
Role role = Role.builder("role").add(IndexPrivilege.ALL, "*").build();
73-
AuthorizedIndices authorizedIndices = new AuthorizedIndices(
74-
() -> RBACEngine.resolveAuthorizedIndicesFromRole(role, SearchAction.NAME, MetaData.EMPTY_META_DATA.getAliasAndIndexLookup()));
75-
List<String> list = authorizedIndices.get();
76-
assertTrue(list.isEmpty());
71+
List<String> authorizedIndices =
72+
RBACEngine.resolveAuthorizedIndicesFromRole(role, SearchAction.NAME, MetaData.EMPTY_META_DATA.getAliasAndIndexLookup());
73+
assertTrue(authorizedIndices.isEmpty());
7774
}
7875

7976
public void testSecurityIndicesAreRemovedFromRegularUser() {
@@ -86,10 +83,9 @@ public void testSecurityIndicesAreRemovedFromRegularUser() {
8683
.numberOfShards(1).numberOfReplicas(0).build(), true)
8784
.build();
8885

89-
AuthorizedIndices authorizedIndices = new AuthorizedIndices(
90-
() -> RBACEngine.resolveAuthorizedIndicesFromRole(role, SearchAction.NAME, metaData.getAliasAndIndexLookup()));
91-
List<String> list = authorizedIndices.get();
92-
assertThat(list, containsInAnyOrder("an-index", "another-index"));
86+
List<String> authorizedIndices =
87+
RBACEngine.resolveAuthorizedIndicesFromRole(role, SearchAction.NAME, metaData.getAliasAndIndexLookup());
88+
assertThat(authorizedIndices, containsInAnyOrder("an-index", "another-index"));
9389
}
9490

9591
public void testSecurityIndicesAreNotRemovedFromSuperUsers() {
@@ -102,9 +98,8 @@ public void testSecurityIndicesAreNotRemovedFromSuperUsers() {
10298
.numberOfShards(1).numberOfReplicas(0).build(), true)
10399
.build();
104100

105-
AuthorizedIndices authorizedIndices = new AuthorizedIndices(
106-
() -> RBACEngine.resolveAuthorizedIndicesFromRole(role, SearchAction.NAME, metaData.getAliasAndIndexLookup()));
107-
List<String> list = authorizedIndices.get();
108-
assertThat(list, containsInAnyOrder("an-index", "another-index", SecurityIndexManager.SECURITY_INDEX_NAME));
101+
List<String> authorizedIndices =
102+
RBACEngine.resolveAuthorizedIndicesFromRole(role, SearchAction.NAME, metaData.getAliasAndIndexLookup());
103+
assertThat(authorizedIndices, containsInAnyOrder("an-index", "another-index", SecurityIndexManager.SECURITY_INDEX_NAME));
109104
}
110105
}

0 commit comments

Comments
 (0)