diff --git a/src/integrationTest/java/org/opensearch/security/DoNotFailOnForbiddenTests.java b/src/integrationTest/java/org/opensearch/security/DoNotFailOnForbiddenTests.java index 88c57c3321..456d1ebada 100644 --- a/src/integrationTest/java/org/opensearch/security/DoNotFailOnForbiddenTests.java +++ b/src/integrationTest/java/org/opensearch/security/DoNotFailOnForbiddenTests.java @@ -278,7 +278,7 @@ public void shouldSearchForDocumentsViaAll_negative() throws IOException { @Test public void shouldMGetDocument_positive() throws IOException { try (RestHighLevelClient restHighLevelClient = cluster.getRestHighLevelClient(LIMITED_USER)) { - MultiGetRequest request = new MultiGetRequest().add(BOTH_INDEX_PATTERN, ID_1).add(BOTH_INDEX_PATTERN, ID_4); + MultiGetRequest request = new MultiGetRequest().add(MARVELOUS_SONGS, ID_1).add(MARVELOUS_SONGS, ID_4); MultiGetResponse response = restHighLevelClient.mget(request, DEFAULT); @@ -296,12 +296,35 @@ public void shouldMGetDocument_positive() throws IOException { } } + @Test + public void shouldMGetDocument_partial() throws Exception { + try (RestHighLevelClient restHighLevelClient = cluster.getRestHighLevelClient(LIMITED_USER)) { + MultiGetRequest request = new MultiGetRequest().add(MARVELOUS_SONGS, ID_1).add(HORRIBLE_SONGS, ID_4); + + MultiGetResponse response = restHighLevelClient.mget(request, DEFAULT); + + MultiGetItemResponse[] responses = response.getResponses(); + assertThat(responses, arrayWithSize(2)); + MultiGetItemResponse firstResult = responses[0]; + MultiGetItemResponse secondResult = responses[1]; + assertThat(firstResult.getFailure(), nullValue()); + assertThat( + firstResult.getResponse(), + allOf(containDocument(MARVELOUS_SONGS, ID_1), documentContainField(FIELD_TITLE, TITLE_MAGNUM_OPUS)) + ); + assertThat(secondResult.getFailure().getMessage(), containsString("no permissions for [indices:data/read/mget[shard]]")); + } + } + @Test public void shouldMGetDocument_negative() throws IOException { try (RestHighLevelClient restHighLevelClient = cluster.getRestHighLevelClient(LIMITED_USER)) { MultiGetRequest request = new MultiGetRequest().add(HORRIBLE_SONGS, ID_4); - - assertThatThrownBy(() -> restHighLevelClient.mget(request, DEFAULT), statusException(FORBIDDEN)); + MultiGetResponse response = restHighLevelClient.mget(request, DEFAULT); + MultiGetItemResponse[] responses = response.getResponses(); + assertThat(responses, arrayWithSize(1)); + MultiGetItemResponse firstResult = responses[0]; + assertThat(firstResult.getFailure().getMessage(), containsString("no permissions for [indices:data/read/mget[shard]]")); } } @@ -331,8 +354,10 @@ public void shouldMSearchDocument_negative() throws IOException { try (RestHighLevelClient restHighLevelClient = cluster.getRestHighLevelClient(LIMITED_USER)) { MultiSearchRequest request = new MultiSearchRequest(); request.add(queryStringQueryRequest(FORBIDDEN_INDEX_ALIAS, QUERY_TITLE_POISON)); - - assertThatThrownBy(() -> restHighLevelClient.msearch(request, DEFAULT), statusException(FORBIDDEN)); + MultiSearchResponse response = restHighLevelClient.msearch(request, DEFAULT); + MultiSearchResponse.Item[] responses = response.getResponses(); + assertThat(responses, Matchers.arrayWithSize(1)); + assertThat(responses[0].getFailure().getMessage(), containsString("no permissions for [indices:data/read/search]")); } } diff --git a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java index 23a8022945..b2ae499879 100644 --- a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java +++ b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java @@ -411,34 +411,6 @@ public PrivilegesEvaluatorResponse evaluate( } } - if (dnfofEnabled && (action0.startsWith("indices:data/read/")) && !requestedResolved.getAllIndices().isEmpty()) { - - if (requestedResolved.getAllIndices().isEmpty()) { - presponse.missingPrivileges.clear(); - presponse.allowed = true; - return presponse; - } - - Set reduced = securityRoles.reduce( - requestedResolved, - user, - new String[] { action0 }, - resolver, - clusterService - ); - - if (reduced.isEmpty()) { - presponse.allowed = false; - return presponse; - } - - if (irr.replace(request, true, reduced.toArray(new String[0]))) { - presponse.missingPrivileges.clear(); - presponse.allowed = true; - return presponse; - } - } - if (isDebugEnabled) { log.debug("Allowed because we have cluster permissions for {}", action0); } diff --git a/src/main/java/org/opensearch/security/resolver/IndexResolverReplacer.java b/src/main/java/org/opensearch/security/resolver/IndexResolverReplacer.java index 4a4e714348..2e479f00e2 100644 --- a/src/main/java/org/opensearch/security/resolver/IndexResolverReplacer.java +++ b/src/main/java/org/opensearch/security/resolver/IndexResolverReplacer.java @@ -176,9 +176,9 @@ private AlreadyResolvedKey(final IndicesOptions indicesOptions, final boolean en } private AlreadyResolvedKey( - final IndicesOptions indicesOptions, - final boolean enableCrossClusterResolution, - final String[] original + final IndicesOptions indicesOptions, + final boolean enableCrossClusterResolution, + final String[] original ) { this.indicesOptions = indicesOptions; this.enableCrossClusterResolution = enableCrossClusterResolution; @@ -191,8 +191,8 @@ public boolean equals(Object o) { if (o == null || getClass() != o.getClass()) return false; AlreadyResolvedKey that = (AlreadyResolvedKey) o; return enableCrossClusterResolution == that.enableCrossClusterResolution - && Objects.equals(indicesOptions, that.indicesOptions) - && Arrays.equals(original, that.original); + && Objects.equals(indicesOptions, that.indicesOptions) + && Arrays.equals(original, that.original); } @Override @@ -213,10 +213,10 @@ public int hashCode() { } private void resolveIndexPatterns( - final String name, - final IndicesOptions indicesOptions, - final boolean enableCrossClusterResolution, - final String[] original + final String name, + final IndicesOptions indicesOptions, + final boolean enableCrossClusterResolution, + final String[] original ) { final boolean isTraceEnabled = log.isTraceEnabled(); if (isTraceEnabled) { @@ -239,11 +239,11 @@ private void resolveIndexPatterns( if (remoteClusterService.isCrossClusterSearchEnabled() && enableCrossClusterResolution) { remoteIndices = new HashSet<>(); final Map remoteClusterIndices = OpenSearchSecurityPlugin.GuiceHolder.getRemoteClusterService() - .groupIndices(indicesOptions, original, idx -> resolver.hasIndexAbstraction(idx, clusterService.state())); + .groupIndices(indicesOptions, original, idx -> resolver.hasIndexAbstraction(idx, clusterService.state())); final Set remoteClusters = remoteClusterIndices.keySet() - .stream() - .filter(k -> !RemoteClusterService.LOCAL_CLUSTER_GROUP_KEY.equals(k)) - .collect(Collectors.toSet()); + .stream() + .filter(k -> !RemoteClusterService.LOCAL_CLUSTER_GROUP_KEY.equals(k)) + .collect(Collectors.toSet()); for (String remoteCluster : remoteClusters) { for (String remoteIndex : remoteClusterIndices.get(remoteCluster).indices()) { remoteIndices.add(RemoteClusterService.buildRemoteIndexName(remoteCluster, remoteIndex)); @@ -261,10 +261,10 @@ private void resolveIndexPatterns( if (isTraceEnabled) { log.trace( - "CCS is enabled, we found this local patterns " - + localRequestedPatterns - + " and this remote patterns: " - + remoteIndices + "CCS is enabled, we found this local patterns " + + localRequestedPatterns + + " and this remote patterns: " + + remoteIndices ); } @@ -294,31 +294,31 @@ private void resolveIndexPatterns( else { final ClusterState state = clusterService.state(); final Set dateResolvedLocalRequestedPatterns = localRequestedPatterns.stream() - .map(resolver::resolveDateMathExpression) - .collect(Collectors.toSet()); + .map(resolver::resolveDateMathExpression) + .collect(Collectors.toSet()); final WildcardMatcher dateResolvedMatcher = WildcardMatcher.from(dateResolvedLocalRequestedPatterns); // fill matchingAliases final Map lookup = state.metadata().getIndicesLookup(); matchingAliases = lookup.entrySet() - .stream() - .filter(e -> e.getValue().getType() == ALIAS) - .map(Map.Entry::getKey) - .filter(dateResolvedMatcher) - .collect(Collectors.toSet()); + .stream() + .filter(e -> e.getValue().getType() == ALIAS) + .map(Map.Entry::getKey) + .filter(dateResolvedMatcher) + .collect(Collectors.toSet()); final boolean isDebugEnabled = log.isDebugEnabled(); try { matchingAllIndices = Arrays.asList( - resolver.concreteIndexNames(state, indicesOptions, localRequestedPatterns.toArray(new String[0])) + resolver.concreteIndexNames(state, indicesOptions, localRequestedPatterns.toArray(new String[0])) ); matchingDataStreams = resolver.dataStreamNames(state, indicesOptions, localRequestedPatterns.toArray(new String[0])); if (isDebugEnabled) { log.debug( - "Resolved pattern {} to indices: {} and data-streams: {}", - localRequestedPatterns, - matchingAllIndices, - matchingDataStreams + "Resolved pattern {} to indices: {} and data-streams: {}", + localRequestedPatterns, + matchingAllIndices, + matchingDataStreams ); } } catch (IndexNotFoundException e1) { @@ -336,15 +336,15 @@ private void resolveIndexPatterns( if (isTraceEnabled) { log.trace( - "Resolved patterns {} for {} ({}) to [aliases {}, allIndices {}, dataStreams {}, originalRequested{}, remote indices {}]", - original, - name, - this.name, - matchingAliases, - matchingAllIndices, - matchingDataStreams, - Arrays.toString(original), - remoteIndices + "Resolved patterns {} for {} ({}) to [aliases {}, allIndices {}, dataStreams {}, originalRequested{}, remote indices {}]", + original, + name, + this.name, + matchingAliases, + matchingAllIndices, + matchingDataStreams, + Arrays.toString(original), + remoteIndices ); } @@ -358,11 +358,11 @@ private void resolveToLocalAll() { } private void resolveTo( - Iterable matchingAliases, - Iterable matchingAllIndices, - Iterable matchingDataStreams, - String[] original, - Iterable remoteIndices + Iterable matchingAliases, + Iterable matchingAllIndices, + Iterable matchingDataStreams, + String[] original, + Iterable remoteIndices ) { aliases.addAll(matchingAliases); allIndices.addAll(matchingAllIndices); @@ -375,8 +375,8 @@ private void resolveTo( public String[] provide(String[] original, Object localRequest, boolean supportsReplace) { final IndicesOptions indicesOptions = indicesOptionsFrom(localRequest); final boolean enableCrossClusterResolution = localRequest instanceof FieldCapabilitiesRequest - || localRequest instanceof SearchRequest - || localRequest instanceof ResolveIndexAction.Request; + || localRequest instanceof SearchRequest + || localRequest instanceof ResolveIndexAction.Request; // skip the whole thing if we have seen this exact resolveIndexPatterns request final AlreadyResolvedKey alreadyResolvedKey; if (original != null) { @@ -392,8 +392,8 @@ public String[] provide(String[] original, Object localRequest, boolean supports Resolved resolved(IndicesOptions indicesOptions) { final Resolved resolved = alreadyResolved.isEmpty() - ? Resolved._LOCAL_ALL - : new Resolved(aliases.build(), allIndices.build(), originalRequested.build(), remoteIndices.build(), indicesOptions); + ? Resolved._LOCAL_ALL + : new Resolved(aliases.build(), allIndices.build(), originalRequested.build(), remoteIndices.build(), indicesOptions); if (log.isTraceEnabled()) { log.trace("Finally resolved for {}: {}", name, resolved); @@ -413,7 +413,7 @@ public String[] provide(String[] original, Object request, boolean supportsRepla if (retainMode && !isAllWithNoRemote(original)) { final Resolved resolved = resolveRequest(request); final List retained = WildcardMatcher.from(resolved.getAllIndices()) - .getMatchAny(replacements, Collectors.toList()); + .getMatchAny(replacements, Collectors.toList()); retained.addAll(resolved.getRemoteIndices()); return retained.toArray(new String[0]); } @@ -442,11 +442,11 @@ public final static class Resolved { private static final ImmutableSet All_SET = ImmutableSet.of(ANY); private static final Set types = All_SET; public static final Resolved _LOCAL_ALL = new Resolved( - All_SET, - All_SET, - All_SET, - ImmutableSet.of(), - SearchRequest.DEFAULT_INDICES_OPTIONS + All_SET, + All_SET, + All_SET, + ImmutableSet.of(), + SearchRequest.DEFAULT_INDICES_OPTIONS ); private final Set aliases; @@ -457,18 +457,18 @@ public final static class Resolved { private final IndicesOptions indicesOptions; public Resolved( - final ImmutableSet aliases, - final ImmutableSet allIndices, - final ImmutableSet originalRequested, - final ImmutableSet remoteIndices, - IndicesOptions indicesOptions + final ImmutableSet aliases, + final ImmutableSet allIndices, + final ImmutableSet originalRequested, + final ImmutableSet remoteIndices, + IndicesOptions indicesOptions ) { this.aliases = aliases; this.allIndices = allIndices; this.originalRequested = originalRequested; this.remoteIndices = remoteIndices; this.isLocalAll = IndexResolverReplacer.isLocalAll(originalRequested.toArray(new String[0])) - || (aliases.contains("*") && allIndices.contains("*")); + || (aliases.contains("*") && allIndices.contains("*")); this.indicesOptions = indicesOptions; } @@ -507,16 +507,16 @@ public Set getRemoteIndices() { @Override public String toString() { return "Resolved [aliases=" - + aliases - + ", allIndices=" - + allIndices - + ", types=" - + types - + ", originalRequested=" - + originalRequested - + ", remoteIndices=" - + remoteIndices - + "]"; + + aliases + + ", allIndices=" + + allIndices + + ", types=" + + types + + ", originalRequested=" + + originalRequested + + ", remoteIndices=" + + remoteIndices + + "]"; } @Override @@ -690,14 +690,14 @@ private boolean getOrReplaceAllIndices(final Object request, final IndicesProvid if (snapshotInfo == null) { log.warn( - "snapshot repository '" + restoreRequest.repository() + "', snapshot '" + restoreRequest.snapshot() + "' not found" + "snapshot repository '" + restoreRequest.repository() + "', snapshot '" + restoreRequest.snapshot() + "' not found" ); provider.provide(new String[] { "*" }, request, false); } else { final List requestedResolvedIndices = IndexUtils.filterIndices( - snapshotInfo.indices(), - restoreRequest.indices(), - restoreRequest.indicesOptions() + snapshotInfo.indices(), + restoreRequest.indices(), + restoreRequest.indicesOptions() ); final List renamedTargetIndices = renamedIndices(restoreRequest, requestedResolvedIndices); // final Set indices = new HashSet<>(requestedResolvedIndices); diff --git a/src/test/java/org/opensearch/security/IntegrationTests.java b/src/test/java/org/opensearch/security/IntegrationTests.java index 0777594834..ebe1f799e9 100644 --- a/src/test/java/org/opensearch/security/IntegrationTests.java +++ b/src/test/java/org/opensearch/security/IntegrationTests.java @@ -659,7 +659,9 @@ public void testDnfof() throws Exception { + System.lineSeparator(); resc = rh.executePostRequest("_msearch?pretty", msearchBody, encodeBasicHeader("user_b", "user_b")); - Assert.assertEquals(403, resc.getStatusCode()); + Assert.assertEquals(resc.getBody(), 200, resc.getStatusCode()); + Assert.assertEquals(resc.getBody(), "security_exception", resc.findValueInJson("responses[0].error.type")); + Assert.assertEquals(resc.getBody(), "security_exception", resc.findValueInJson("responses[1].error.type")); String mgetBody = "{" + "\"docs\" : [" @@ -696,7 +698,9 @@ public void testDnfof() throws Exception { + "}"; resc = rh.executePostRequest("_mget?pretty", mgetBody, encodeBasicHeader("user_b", "user_b")); - Assert.assertEquals(403, resc.getStatusCode()); + Assert.assertEquals(resc.getBody(), 200, resc.getStatusCode()); + Assert.assertEquals(resc.getBody(), "index_not_found_exception", resc.findValueInJson("docs[0].error.type")); + Assert.assertEquals(resc.getBody(), "index_not_found_exception", resc.findValueInJson("docs[1].error.type")); Assert.assertEquals( HttpStatus.SC_OK,