diff --git a/elasticsearch/src/main/java/org/elasticsearch/xpack/security/action/role/TransportGetRolesAction.java b/elasticsearch/src/main/java/org/elasticsearch/xpack/security/action/role/TransportGetRolesAction.java index 95183c5502d..507f75ea0d5 100644 --- a/elasticsearch/src/main/java/org/elasticsearch/xpack/security/action/role/TransportGetRolesAction.java +++ b/elasticsearch/src/main/java/org/elasticsearch/xpack/security/action/role/TransportGetRolesAction.java @@ -67,44 +67,15 @@ public class TransportGetRolesAction extends HandledTransportAction() { - @Override - public void onResponse(RoleDescriptor roleD) { - if (roleD != null) { - roles.add(roleD); - } - listener.onResponse(new GetRolesResponse(roles.toArray(new RoleDescriptor[roles.size()]))); - } - - @Override - public void onFailure(Exception t) { - logger.error((Supplier) () -> new ParameterizedMessage("failed to retrieve role [{}]", rolename), t); - listener.onFailure(t); - } - }); - } else if (specificRolesRequested && rolesToSearchFor.isEmpty()) { + if (specificRolesRequested && rolesToSearchFor.isEmpty()) { // specific roles were requested but they were built in only, no need to hit the store listener.onResponse(new GetRolesResponse(roles.toArray(new RoleDescriptor[roles.size()]))); } else { - nativeRolesStore.getRoleDescriptors( - rolesToSearchFor.toArray(new String[rolesToSearchFor.size()]), new ActionListener>() { - @Override - public void onResponse(List foundRoles) { - roles.addAll(foundRoles); - listener.onResponse(new GetRolesResponse(roles.toArray(new RoleDescriptor[roles.size()]))); - } - - @Override - public void onFailure(Exception t) { - logger.error( - (Supplier) () -> new ParameterizedMessage( - "failed to retrieve role [{}]", arrayToDelimitedString(request.names(), ",")), t); - listener.onFailure(t); - } - }); + String[] roleNames = rolesToSearchFor.toArray(new String[rolesToSearchFor.size()]); + nativeRolesStore.getRoleDescriptors(roleNames, ActionListener.wrap((foundRoles) -> { + roles.addAll(foundRoles); + listener.onResponse(new GetRolesResponse(roles.toArray(new RoleDescriptor[roles.size()]))); + }, listener::onFailure)); } } } diff --git a/elasticsearch/src/main/java/org/elasticsearch/xpack/security/authz/store/NativeRolesStore.java b/elasticsearch/src/main/java/org/elasticsearch/xpack/security/authz/store/NativeRolesStore.java index 31c87aed02f..ac3643eecff 100644 --- a/elasticsearch/src/main/java/org/elasticsearch/xpack/security/authz/store/NativeRolesStore.java +++ b/elasticsearch/src/main/java/org/elasticsearch/xpack/security/authz/store/NativeRolesStore.java @@ -64,6 +64,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.concurrent.ExecutionException; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; @@ -207,89 +208,77 @@ public class NativeRolesStore extends AbstractComponent implements ClusterStateL listener.onFailure(new IllegalStateException("roles cannot be retrieved as native role service has not been started")); return; } - try { - final List roles = new ArrayList<>(); - QueryBuilder query; - if (names == null || names.length == 0) { - query = QueryBuilders.matchAllQuery(); - } else { - query = QueryBuilders.boolQuery().filter(QueryBuilders.idsQuery(ROLE_DOC_TYPE).addIds(names)); - } - SearchRequest request = client.prepareSearch(SecurityTemplateService.SECURITY_INDEX_NAME) - .setTypes(ROLE_DOC_TYPE) - .setScroll(scrollKeepAlive) - .setQuery(query) - .setSize(scrollSize) - .setFetchSource(true) - .request(); - request.indicesOptions().ignoreUnavailable(); + if (names != null && names.length == 1) { + getRoleAndVersion(Objects.requireNonNull(names[0]), ActionListener.wrap(roleAndVersion -> + listener.onResponse(roleAndVersion == null || roleAndVersion.getRoleDescriptor() == null ? Collections.emptyList() + : Collections.singletonList(roleAndVersion.getRoleDescriptor())), listener::onFailure)); + } else { + try { + final List roles = new ArrayList<>(); + QueryBuilder query; + if (names == null || names.length == 0) { + query = QueryBuilders.matchAllQuery(); + } else { + query = QueryBuilders.boolQuery().filter(QueryBuilders.idsQuery(ROLE_DOC_TYPE).addIds(names)); + } + SearchRequest request = client.prepareSearch(SecurityTemplateService.SECURITY_INDEX_NAME) + .setTypes(ROLE_DOC_TYPE) + .setScroll(scrollKeepAlive) + .setQuery(query) + .setSize(scrollSize) + .setFetchSource(true) + .request(); + request.indicesOptions().ignoreUnavailable(); - // This function is MADNESS! But it works, don't think about it too hard... - client.search(request, new ActionListener() { + // This function is MADNESS! But it works, don't think about it too hard... + client.search(request, new ActionListener() { - private SearchResponse lastResponse = null; + private SearchResponse lastResponse = null; - @Override - public void onResponse(SearchResponse resp) { - lastResponse = resp; - boolean hasHits = resp.getHits().getHits().length > 0; - if (hasHits) { - for (SearchHit hit : resp.getHits().getHits()) { - RoleDescriptor rd = transformRole(hit.getId(), hit.getSourceRef(), logger); - if (rd != null) { - roles.add(rd); + @Override + public void onResponse(SearchResponse resp) { + lastResponse = resp; + boolean hasHits = resp.getHits().getHits().length > 0; + if (hasHits) { + for (SearchHit hit : resp.getHits().getHits()) { + RoleDescriptor rd = transformRole(hit.getId(), hit.getSourceRef(), logger); + if (rd != null) { + roles.add(rd); + } } + SearchScrollRequest scrollRequest = client.prepareSearchScroll(resp.getScrollId()) + .setScroll(scrollKeepAlive).request(); + client.searchScroll(scrollRequest, this); + } else { + if (resp.getScrollId() != null) { + clearScollRequest(resp.getScrollId()); + } + // Finally, return the list of users + listener.onResponse(Collections.unmodifiableList(roles)); } - SearchScrollRequest scrollRequest = client.prepareSearchScroll(resp.getScrollId()) - .setScroll(scrollKeepAlive).request(); - client.searchScroll(scrollRequest, this); - } else { - if (resp.getScrollId() != null) { - clearScollRequest(resp.getScrollId()); + } + + @Override + public void onFailure(Exception t) { + // attempt to clear the scroll request + if (lastResponse != null && lastResponse.getScrollId() != null) { + clearScollRequest(lastResponse.getScrollId()); + } + + if (t instanceof IndexNotFoundException) { + logger.trace("could not retrieve roles because security index does not exist"); + // since this is expected to happen at times, we just call the listener with an empty list + listener.onResponse(Collections.emptyList()); + } else { + listener.onFailure(t); } - // Finally, return the list of users - listener.onResponse(Collections.unmodifiableList(roles)); } - } - - @Override - public void onFailure(Exception t) { - // attempt to clear the scroll request - if (lastResponse != null && lastResponse.getScrollId() != null) { - clearScollRequest(lastResponse.getScrollId()); - } - - if (t instanceof IndexNotFoundException) { - logger.trace("could not retrieve roles because security index does not exist"); - // since this is expected to happen at times, we just call the listener with an empty list - listener.onResponse(Collections.emptyList()); - } else { - listener.onFailure(t); - } - } - }); - } catch (Exception e) { - logger.error((Supplier) () -> new ParameterizedMessage("unable to retrieve roles {}", Arrays.toString(names)), e); - listener.onFailure(e); - } - } - - public void getRoleDescriptor(final String role, final ActionListener listener) { - if (state() != State.STARTED) { - logger.trace("attempted to get role [{}] before service was started", role); - listener.onResponse(null); - } - getRoleAndVersion(role, new ActionListener() { - @Override - public void onResponse(RoleAndVersion roleAndVersion) { - listener.onResponse(roleAndVersion == null ? null : roleAndVersion.getRoleDescriptor()); - } - - @Override - public void onFailure(Exception e) { + }); + } catch (Exception e) { + logger.error((Supplier) () -> new ParameterizedMessage("unable to retrieve roles {}", Arrays.toString(names)), e); listener.onFailure(e); } - }); + } } public void deleteRole(final DeleteRoleRequest deleteRoleRequest, final ActionListener listener) { diff --git a/elasticsearch/src/test/java/org/elasticsearch/xpack/security/action/role/TransportGetRolesActionTests.java b/elasticsearch/src/test/java/org/elasticsearch/xpack/security/action/role/TransportGetRolesActionTests.java index 2d9d6d7dac5..2eff2902f16 100644 --- a/elasticsearch/src/test/java/org/elasticsearch/xpack/security/action/role/TransportGetRolesActionTests.java +++ b/elasticsearch/src/test/java/org/elasticsearch/xpack/security/action/role/TransportGetRolesActionTests.java @@ -71,15 +71,12 @@ public class TransportGetRolesActionTests extends ESTestCase { expectedNames.remove(KibanaRole.NAME); } - doAnswer(new Answer() { - @Override - public Void answer(InvocationOnMock invocation) throws Throwable { - Object[] args = invocation.getArguments(); - assert args.length == 2; - ActionListener> listener = (ActionListener>) args[1]; - listener.onResponse(Collections.emptyList()); - return null; - } + doAnswer(invocation -> { + Object[] args = invocation.getArguments(); + assert args.length == 2; + ActionListener> listener = (ActionListener>) args[1]; + listener.onResponse(Collections.emptyList()); + return null; }).when(rolesStore).getRoleDescriptors(aryEq(Strings.EMPTY_ARRAY), any(ActionListener.class)); GetRolesRequest request = new GetRolesRequest(); @@ -131,32 +128,13 @@ public class TransportGetRolesActionTests extends ESTestCase { GetRolesRequest request = new GetRolesRequest(); request.names(storeRoleDescriptors.stream().map(RoleDescriptor::getName).collect(Collectors.toList()).toArray(Strings.EMPTY_ARRAY)); - if (request.names().length == 1) { - doAnswer(new Answer() { - @Override - public Void answer(InvocationOnMock invocation) throws Throwable { - Object[] args = invocation.getArguments(); - assert args.length == 2; - String requestedName = (String) args[0]; - ActionListener listener = (ActionListener) args[1]; - Optional rd = - storeRoleDescriptors.stream().filter(r -> r.getName().equals(requestedName)).findFirst(); - listener.onResponse(rd.get()); - return null; - } - }).when(rolesStore).getRoleDescriptor(eq(request.names()[0]), any(ActionListener.class)); - } else { - doAnswer(new Answer() { - @Override - public Void answer(InvocationOnMock invocation) throws Throwable { - Object[] args = invocation.getArguments(); - assert args.length == 2; - ActionListener> listener = (ActionListener>) args[1]; - listener.onResponse(storeRoleDescriptors); - return null; - } - }).when(rolesStore).getRoleDescriptors(aryEq(request.names()), any(ActionListener.class)); - } + doAnswer(invocation -> { + Object[] args = invocation.getArguments(); + assert args.length == 2; + ActionListener> listener = (ActionListener>) args[1]; + listener.onResponse(storeRoleDescriptors); + return null; + }).when(rolesStore).getRoleDescriptors(aryEq(request.names()), any(ActionListener.class)); final AtomicReference throwableRef = new AtomicReference<>(); final AtomicReference responseRef = new AtomicReference<>(); @@ -219,40 +197,21 @@ public class TransportGetRolesActionTests extends ESTestCase { GetRolesRequest request = new GetRolesRequest(); request.names(requestedNames.toArray(Strings.EMPTY_ARRAY)); - if (specificStoreNames.size() == 1) { - doAnswer(new Answer() { - @Override - public Void answer(InvocationOnMock invocation) throws Throwable { - Object[] args = invocation.getArguments(); - assert args.length == 2; - String requestedName = (String) args[0]; - ActionListener listener = (ActionListener) args[1]; - Optional rd = - storeRoleDescriptors.stream().filter(r -> r.getName().equals(requestedName)).findFirst(); - listener.onResponse(rd.get()); - return null; - } - }).when(rolesStore).getRoleDescriptor(eq(specificStoreNames.get(0)), any(ActionListener.class)); - } else { - doAnswer(new Answer() { - @Override - public Void answer(InvocationOnMock invocation) throws Throwable { - Object[] args = invocation.getArguments(); - assert args.length == 2; - String[] requestedNames = (String[]) args[0]; - ActionListener> listener = (ActionListener>) args[1]; - if (requestedNames.length == 0) { - listener.onResponse(storeRoleDescriptors); - } else { - List requestedNamesList = Arrays.asList(requestedNames); - listener.onResponse(storeRoleDescriptors.stream() - .filter(r -> requestedNamesList.contains(r.getName())) - .collect(Collectors.toList())); - } - return null; - } - }).when(rolesStore).getRoleDescriptors(aryEq(specificStoreNames.toArray(Strings.EMPTY_ARRAY)), any(ActionListener.class)); - } + doAnswer(invocation -> { + Object[] args = invocation.getArguments(); + assert args.length == 2; + String[] requestedNames1 = (String[]) args[0]; + ActionListener> listener = (ActionListener>) args[1]; + if (requestedNames1.length == 0) { + listener.onResponse(storeRoleDescriptors); + } else { + List requestedNamesList = Arrays.asList(requestedNames1); + listener.onResponse(storeRoleDescriptors.stream() + .filter(r -> requestedNamesList.contains(r.getName())) + .collect(Collectors.toList())); + } + return null; + }).when(rolesStore).getRoleDescriptors(aryEq(specificStoreNames.toArray(Strings.EMPTY_ARRAY)), any(ActionListener.class)); final AtomicReference throwableRef = new AtomicReference<>(); final AtomicReference responseRef = new AtomicReference<>(); @@ -276,8 +235,6 @@ public class TransportGetRolesActionTests extends ESTestCase { if (all) { verify(rolesStore, times(1)).getRoleDescriptors(aryEq(Strings.EMPTY_ARRAY), any(ActionListener.class)); - } else if (specificStoreNames.size() == 1) { - verify(rolesStore, times(1)).getRoleDescriptor(eq(specificStoreNames.get(0)), any(ActionListener.class)); } else { verify(rolesStore, times(1)) .getRoleDescriptors(aryEq(specificStoreNames.toArray(Strings.EMPTY_ARRAY)), any(ActionListener.class)); @@ -297,29 +254,13 @@ public class TransportGetRolesActionTests extends ESTestCase { GetRolesRequest request = new GetRolesRequest(); request.names(storeRoleDescriptors.stream().map(RoleDescriptor::getName).collect(Collectors.toList()).toArray(Strings.EMPTY_ARRAY)); - if (request.names().length == 1) { - doAnswer(new Answer() { - @Override - public Void answer(InvocationOnMock invocation) throws Throwable { - Object[] args = invocation.getArguments(); - assert args.length == 2; - ActionListener listener = (ActionListener) args[1]; - listener.onFailure(e); - return null; - } - }).when(rolesStore).getRoleDescriptor(eq(request.names()[0]), any(ActionListener.class)); - } else { - doAnswer(new Answer() { - @Override - public Void answer(InvocationOnMock invocation) throws Throwable { - Object[] args = invocation.getArguments(); - assert args.length == 2; - ActionListener> listener = (ActionListener>) args[1]; - listener.onFailure(e); - return null; - } - }).when(rolesStore).getRoleDescriptors(aryEq(request.names()), any(ActionListener.class)); - } + doAnswer(invocation -> { + Object[] args = invocation.getArguments(); + assert args.length == 2; + ActionListener> listener = (ActionListener>) args[1]; + listener.onFailure(e); + return null; + }).when(rolesStore).getRoleDescriptors(aryEq(request.names()), any(ActionListener.class)); final AtomicReference throwableRef = new AtomicReference<>(); final AtomicReference responseRef = new AtomicReference<>();