Skip to content

Commit

Permalink
[Backport 2.x] Use DelegatingRestHandler to delegate to the original …
Browse files Browse the repository at this point in the history
…handler (#3547)

Backport 1166c1f from #3517.

Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
1 parent b5622c4 commit 89910d9
Show file tree
Hide file tree
Showing 4 changed files with 199 additions and 51 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/

package org.opensearch.security.filter;

import org.opensearch.client.node.NodeClient;
import org.opensearch.rest.RestChannel;
import org.opensearch.rest.RestHandler;
import org.opensearch.rest.RestRequest;

import java.util.List;
import java.util.Objects;

/**
* Delegating RestHandler that delegates all implementations to original handler
*
* @opensearch.api
*/
public class DelegatingRestHandler implements RestHandler {

protected final RestHandler delegate;

public DelegatingRestHandler(RestHandler delegate) {
Objects.requireNonNull(delegate, "RestHandler delegate can not be null");
this.delegate = delegate;
}

@Override
public void handleRequest(RestRequest request, RestChannel channel, NodeClient client) throws Exception {
delegate.handleRequest(request, channel, client);
}

@Override
public boolean canTripCircuitBreaker() {
return delegate.canTripCircuitBreaker();
}

@Override
public boolean supportsContentStream() {
return delegate.supportsContentStream();
}

@Override
public boolean allowsUnsafeBuffers() {
return delegate.allowsUnsafeBuffers();
}

@Override
public List<Route> routes() {
return delegate.routes();
}

@Override
public List<DeprecatedRoute> deprecatedRoutes() {
return delegate.deprecatedRoutes();
}

@Override
public List<ReplacedRoute> replacedRoutes() {
return delegate.replacedRoutes();
}

@Override
public boolean allowSystemIndexAccessByDefault() {
return delegate.allowSystemIndexAccessByDefault();
}

@Override
public String toString() {
return delegate.toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,13 @@
import org.greenrobot.eventbus.Subscribe;

import org.opensearch.OpenSearchException;
import org.opensearch.client.node.NodeClient;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.util.concurrent.ThreadContext;
import org.opensearch.rest.NamedRoute;
import org.opensearch.rest.RestChannel;
import org.opensearch.rest.RestHandler;
import org.opensearch.rest.RestRequest;
import org.opensearch.rest.RestRequest.Method;
import org.opensearch.security.auditlog.AuditLog;
import org.opensearch.security.auditlog.AuditLog.Origin;
Expand Down Expand Up @@ -116,22 +119,16 @@ public SecurityRestFilter(
this.allowlistingSettings = new AllowlistingSettings();
}

/**
* This function wraps around all rest requests
* If the request is authenticated, then it goes through a allowlisting check.
* The allowlisting check works as follows:
* If allowlisting is not enabled, then requests are handled normally.
* If allowlisting is enabled, then SuperAdmin is allowed access to all APIs, regardless of what is currently allowlisted.
* If allowlisting is enabled, then Non-SuperAdmin is allowed to access only those APIs that are allowlisted in {@link #requests}
* For example: if allowlisting is enabled and requests = ["/_cat/nodes"], then SuperAdmin can access all APIs, but non SuperAdmin
* can only access "/_cat/nodes"
* Further note: Some APIs are only accessible by SuperAdmin, regardless of allowlisting. For example: /_opendistro/_security/api/whitelist is only accessible by SuperAdmin.
* See {@link AllowlistApiAction} for the implementation of this API.
* SuperAdmin is identified by credentials, which can be passed in the curl request.
*/
public RestHandler wrap(RestHandler original, AdminDNs adminDNs) {
return (request, channel, client) -> {
class AuthczRestHandler extends DelegatingRestHandler {
private final AdminDNs adminDNs;

public AuthczRestHandler(RestHandler original, AdminDNs adminDNs) {
super(original);
this.adminDNs = adminDNs;
}

@Override
public void handleRequest(RestRequest request, RestChannel channel, NodeClient client) throws Exception {
final Optional<SecurityResponse> maybeSavedResponse = NettyAttribute.popFrom(request, EARLY_RESPONSE);
if (maybeSavedResponse.isPresent()) {
NettyAttribute.clearAttribute(request, CONTEXT_TO_RESTORE);
Expand Down Expand Up @@ -165,7 +162,7 @@ public RestHandler wrap(RestHandler original, AdminDNs adminDNs) {
final User user = threadContext.getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER);
if (userIsSuperAdmin(user, adminDNs)) {
// Super admins are always authorized
original.handleRequest(request, channel, client);
delegate.handleRequest(request, channel, client);
return;
}

Expand All @@ -177,15 +174,32 @@ public RestHandler wrap(RestHandler original, AdminDNs adminDNs) {
return;
}

authorizeRequest(original, requestChannel, user);
authorizeRequest(delegate, requestChannel, user);
if (requestChannel.getQueuedResponse().isPresent()) {
channel.sendResponse(requestChannel.getQueuedResponse().get().asRestResponse());
return;
}

// Caller was authorized, forward the request to the handler
original.handleRequest(request, channel, client);
};
delegate.handleRequest(request, channel, client);
}
}

/**
* This function wraps around all rest requests
* If the request is authenticated, then it goes through a allowlisting check.
* The allowlisting check works as follows:
* If allowlisting is not enabled, then requests are handled normally.
* If allowlisting is enabled, then SuperAdmin is allowed access to all APIs, regardless of what is currently allowlisted.
* If allowlisting is enabled, then Non-SuperAdmin is allowed to access only those APIs that are allowlisted in {@link #requests}
* For example: if allowlisting is enabled and requests = ["/_cat/nodes"], then SuperAdmin can access all APIs, but non SuperAdmin
* can only access "/_cat/nodes"
* Further note: Some APIs are only accessible by SuperAdmin, regardless of allowlisting. For example: /_opendistro/_security/api/whitelist is only accessible by SuperAdmin.
* See {@link AllowlistApiAction} for the implementation of this API.
* SuperAdmin is identified by credentials, which can be passed in the curl request.
*/
public RestHandler wrap(RestHandler original, AdminDNs adminDNs) {
return new AuthczRestHandler(original, adminDNs);
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/

package org.opensearch.security.filter;

import org.junit.Test;
import org.opensearch.client.node.NodeClient;
import org.opensearch.core.common.bytes.BytesArray;
import org.opensearch.core.rest.RestStatus;
import org.opensearch.rest.BytesRestResponse;
import org.opensearch.rest.RestChannel;
import org.opensearch.rest.RestHandler;
import org.opensearch.rest.RestRequest;

import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.util.Arrays;
import java.util.List;
import java.util.stream.Collectors;

import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;

public class DelegatingRestHandlerTests {

@Test
public void testDelegatingRestHandlerShouldActAsOriginal() throws Exception {
RestHandler rh = new RestHandler() {
@Override
public void handleRequest(RestRequest request, RestChannel channel, NodeClient client) throws Exception {
new BytesRestResponse(RestStatus.OK, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY);
}
};
RestHandler handlerSpy = spy(rh);
DelegatingRestHandler drh = new DelegatingRestHandler(handlerSpy);

List<Method> overridableMethods = Arrays.stream(RestHandler.class.getMethods())
.filter(
m -> !(Modifier.isPrivate(m.getModifiers()) || Modifier.isStatic(m.getModifiers()) || Modifier.isFinal(m.getModifiers()))
)
.collect(Collectors.toList());

for (Method method : overridableMethods) {
int argCount = method.getParameterCount();
Object[] args = new Object[argCount];
for (int i = 0; i < argCount; i++) {
args[i] = any();
}
if (args.length > 0) {
method.invoke(drh, args);
} else {
method.invoke(drh);
}
method.invoke(verify(handlerSpy, times(1)), args);
}
verifyNoMoreInteractions(handlerSpy);
}
}
Original file line number Diff line number Diff line change
@@ -1,14 +1,23 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*
* Modifications Copyright OpenSearch Contributors. See
* GitHub history for details.
*/

package org.opensearch.security.filter;

import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;
import org.opensearch.client.node.NodeClient;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.util.concurrent.ThreadContext;
import org.opensearch.core.common.bytes.BytesArray;
import org.opensearch.core.rest.RestStatus;
import org.opensearch.core.xcontent.NamedXContentRegistry;
import org.opensearch.rest.BytesRestResponse;
import org.opensearch.rest.RestChannel;
import org.opensearch.rest.RestHandler;
Expand All @@ -19,17 +28,15 @@
import org.opensearch.security.configuration.CompatConfig;
import org.opensearch.security.privileges.RestLayerPrivilegesEvaluator;
import org.opensearch.security.ssl.transport.PrincipalExtractor;
import org.opensearch.test.rest.FakeRestRequest;
import org.opensearch.threadpool.ThreadPool;

import java.nio.file.Path;
import java.util.List;
import java.util.Map;

import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.verify;

Expand Down Expand Up @@ -65,47 +72,31 @@ public void setUp() throws NoSuchMethodException {
);
}

@Ignore
/**
* Tests to ensure that the output of {@link SecurityRestFilter#wrap} is an instance of AuthczRestHandler
*/
@Test
public void testDoesCallDelegateOnSuccessfulAuthorization() throws Exception {
SecurityRestFilter filterSpy = spy(sf);
public void testSecurityRestFilterWrap() throws Exception {
AdminDNs adminDNs = mock(AdminDNs.class);

RestHandler testRestHandlerSpy = spy(testRestHandler);
RestHandler wrappedRestHandler = filterSpy.wrap(testRestHandlerSpy, adminDNs);

doReturn(false).when(filterSpy).userIsSuperAdmin(any(), any());
// doReturn(true).when(filterSpy).authorizeRequest(any(), any(), any());
RestHandler wrappedRestHandler = sf.wrap(testRestHandler, adminDNs);

FakeRestRequest fakeRequest = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY).withPath("/test")
.withMethod(RestRequest.Method.POST)
.withHeaders(Map.of("Content-Type", List.of("application/json")))
.build();

wrappedRestHandler.handleRequest(fakeRequest, mock(RestChannel.class), mock(NodeClient.class));

verify(testRestHandlerSpy).handleRequest(any(), any(), any());
assertTrue(wrappedRestHandler instanceof SecurityRestFilter.AuthczRestHandler);
assertFalse(wrappedRestHandler instanceof TestRestHandler);
}

@Ignore
@Test
public void testDoesNotCallDelegateOnUnauthorized() throws Exception {
public void testDoesCallDelegateOnSuccessfulAuthorization() throws Exception {
SecurityRestFilter filterSpy = spy(sf);
AdminDNs adminDNs = mock(AdminDNs.class);

RestHandler testRestHandlerSpy = spy(testRestHandler);
RestHandler wrappedRestHandler = filterSpy.wrap(testRestHandlerSpy, adminDNs);

doReturn(false).when(filterSpy).userIsSuperAdmin(any(), any());
// doReturn(false).when(filterSpy).authorizeRequest(any(), any(), any());

FakeRestRequest fakeRequest = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY).withPath("/test")
.withMethod(RestRequest.Method.POST)
.withHeaders(Map.of("Content-Type", List.of("application/json")))
.build();

wrappedRestHandler.handleRequest(fakeRequest, mock(RestChannel.class), mock(NodeClient.class));
wrappedRestHandler.handleRequest(mock(RestRequest.class), mock(RestChannel.class), mock(NodeClient.class));

verify(testRestHandlerSpy, never()).handleRequest(any(), any(), any());
verify(testRestHandlerSpy).handleRequest(any(), any(), any());
}
}

0 comments on commit 89910d9

Please sign in to comment.