Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement new extension points in IdentityPlugin and add ContextProvidingPluginSubject #4665

Closed
wants to merge 57 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
57 commits
Select commit Hold shift + click to select a range
e65b248
Implement new extension points in IdentityPlugin and add ContextProvi…
cwperks Aug 19, 2024
55a7e45
Remove code from UserService
cwperks Aug 20, 2024
44c0d76
Remove commented out code
cwperks Aug 20, 2024
3e17b57
Use guice dependencies
cwperks Aug 20, 2024
0add3d8
Move TransportActionDependencies
cwperks Aug 20, 2024
b76a5b8
Use NamedPrincipal
cwperks Aug 20, 2024
2da6f2a
Add end to end test to show stronger index protection for plugins
cwperks Aug 20, 2024
9cf80ae
Add end to end test to show stronger index protection for plugins
cwperks Aug 20, 2024
21fc225
Move to constructor
cwperks Aug 20, 2024
f08f311
Remove unused constant
cwperks Aug 20, 2024
2de9a5f
Add test that demonstrates cluster action forbidden when running with…
cwperks Aug 20, 2024
22795df
Add bulk index test
cwperks Aug 20, 2024
e075ae8
Ensure bulk request with mix of actions blocks as expected
cwperks Aug 20, 2024
e12f673
Use subclass instead of attribute
cwperks Aug 21, 2024
f9adcc4
Rename TransportActionDependencies to PluginContextSwitcher
cwperks Aug 21, 2024
2074dab
Prevent pluginUser from being serialized to nodes before 2.17.0 for b…
cwperks Aug 22, 2024
0960ce0
Demonstrate how a role can be created in-memory to re-use authz
cwperks Aug 22, 2024
8c2b7ba
Add missing license headers
cwperks Aug 22, 2024
675f20a
Remove unused code
cwperks Aug 22, 2024
8d3eb1c
Remove unused field
cwperks Aug 23, 2024
115fa0e
Implement getCurrentSubject
cwperks Aug 27, 2024
f6650eb
Use transport for cluster health tests
cwperks Aug 28, 2024
77b728d
Show use of default
cwperks Aug 28, 2024
a649257
Show an example of switching subjects within a transport action
cwperks Aug 28, 2024
7170c82
Show example of attaching subject to ActionRequest
cwperks Aug 28, 2024
9c205a6
Revert "Show example of attaching subject to ActionRequest"
cwperks Aug 28, 2024
690144e
Remove copy of SystemIndexRegistry and use exposed methods from core
cwperks Aug 29, 2024
b18d3aa
Merge branch 'main' into security-subject
cwperks Aug 29, 2024
e781e37
Fix failing tests
cwperks Aug 29, 2024
3c60808
Fix SystemIndexPermissionEnabledTests tests
cwperks Aug 30, 2024
e42a4a0
Fix tests
cwperks Aug 31, 2024
41e83b1
Fix disabled tests
cwperks Aug 31, 2024
c9f1ff7
Fix more tests
cwperks Aug 31, 2024
72c155b
Merge branch 'main' into security-subject
cwperks Sep 3, 2024
0a696ff
Add license headers
cwperks Sep 3, 2024
c4b5e17
Add tests
cwperks Sep 3, 2024
1d1bb57
Test PluginContextSwitcher
cwperks Sep 3, 2024
27be3c3
Test getPrincipal
cwperks Sep 3, 2024
634cf7b
Add test on createSecurityRole
cwperks Sep 3, 2024
1af4b65
Increase test coverage
cwperks Sep 3, 2024
64fb161
Allow import for AcknowledgedResponse
cwperks Sep 3, 2024
dc8868e
Address code review comments
cwperks Sep 6, 2024
565fa20
Address code review feedback
cwperks Sep 6, 2024
23e62bf
Merge branch 'main' into security-subject
cwperks Sep 20, 2024
e5855ab
Change calls to isPluginUser and create InMemorySecurityRoles
cwperks Sep 20, 2024
b8c333c
Add missing license header
cwperks Sep 20, 2024
9b0c3df
Use plugin: prefix
cwperks Sep 23, 2024
abea484
Remove PluginUser
cwperks Sep 23, 2024
86c3336
Merge branch 'main' into security-subject
cwperks Oct 1, 2024
ed13ecb
Remove Identity FeatureFlag
cwperks Oct 1, 2024
68ff270
Fix tests
cwperks Oct 1, 2024
70bb6ef
Merge branch 'main' into security-subject
cwperks Oct 2, 2024
dbb723a
Merge branch 'main' into security-subject
cwperks Oct 4, 2024
275a346
Merge branch 'main' into security-subject
cwperks Oct 14, 2024
0216cc0
Address code review feedback
cwperks Oct 14, 2024
05890d5
Create common method for getting SecurityRoles and move plugin specif…
cwperks Oct 14, 2024
877fd42
Merge branch 'main' into security-subject
cwperks Oct 24, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,25 @@
import java.util.Map;

import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
import org.junit.Before;
import org.junit.ClassRule;
import org.junit.Test;
import org.junit.runner.RunWith;

import org.opensearch.core.rest.RestStatus;
import org.opensearch.security.http.ExampleSystemIndexPlugin;
import org.opensearch.security.plugin.SystemIndexPlugin1;
import org.opensearch.security.plugin.SystemIndexPlugin2;
import org.opensearch.test.framework.TestSecurityConfig.AuthcDomain;
import org.opensearch.test.framework.cluster.ClusterManager;
import org.opensearch.test.framework.cluster.LocalCluster;
import org.opensearch.test.framework.cluster.TestRestClient;
import org.opensearch.test.framework.cluster.TestRestClient.HttpResponse;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.opensearch.security.plugin.SystemIndexPlugin1.SYSTEM_INDEX_1;
import static org.opensearch.security.plugin.SystemIndexPlugin2.SYSTEM_INDEX_2;
import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_ROLES_ENABLED;
import static org.opensearch.security.support.ConfigConstants.SECURITY_SYSTEM_INDICES_ENABLED_KEY;
import static org.opensearch.test.framework.TestSecurityConfig.Role.ALL_ACCESS;
Expand All @@ -39,11 +44,11 @@ public class SystemIndexTests {
public static final AuthcDomain AUTHC_DOMAIN = new AuthcDomain("basic", 0).httpAuthenticatorWithChallenge("basic").backend("internal");

@ClassRule
public static final LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.SINGLENODE)
public static final LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.DEFAULT)
.anonymousAuth(false)
.authc(AUTHC_DOMAIN)
.users(USER_ADMIN)
.plugin(ExampleSystemIndexPlugin.class)
.plugin(SystemIndexPlugin1.class, SystemIndexPlugin2.class)
.nodeSettings(
Map.of(
SECURITY_RESTAPI_ROLES_ENABLED,
Expand All @@ -54,6 +59,14 @@ public class SystemIndexTests {
)
.build();

@Before
public void wipeAllIndices() {
try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) {
client.delete(".system-index1");
client.delete(".system-index2");
}
}

@Test
public void adminShouldNotBeAbleToDeleteSecurityIndex() {
try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
Expand All @@ -69,7 +82,7 @@ public void adminShouldNotBeAbleToDeleteSecurityIndex() {

assertThat(response2.getStatusCode(), equalTo(RestStatus.OK.getStatus()));

// regular use can create system index
// regular user can create system index
HttpResponse response3 = client.put(".system-index1");

assertThat(response3.getStatusCode(), equalTo(RestStatus.OK.getStatus()));
Expand All @@ -81,6 +94,101 @@ public void adminShouldNotBeAbleToDeleteSecurityIndex() {
}
}

@Test
public void testPluginShouldBeAbleToIndexDocumentIntoItsSystemIndex() {
try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
HttpResponse response = client.put("try-create-and-index/" + SYSTEM_INDEX_1);

assertThat(response.getStatusCode(), equalTo(RestStatus.OK.getStatus()));
assertThat(response.getBody(), containsString(SystemIndexPlugin1.class.getCanonicalName()));
}
}

@Test
public void testPluginShouldNotBeAbleToIndexDocumentIntoSystemIndexRegisteredByOtherPlugin() {
try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
HttpResponse response = client.put("try-create-and-index/" + SYSTEM_INDEX_2);

assertThat(response.getStatusCode(), equalTo(RestStatus.FORBIDDEN.getStatus()));
assertThat(
response.getBody(),
containsString(
"no permissions for [indices:admin/create] and User [name=plugin:org.opensearch.security.plugin.SystemIndexPlugin1"
)
);
}
}

@Test
public void testPluginShouldBeAbleToCreateSystemIndexButUserShouldNotBeAbleToIndex() {
try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
HttpResponse response = client.put("try-create-and-index/" + SYSTEM_INDEX_1 + "?runAs=user");

assertThat(response.getStatusCode(), equalTo(RestStatus.FORBIDDEN.getStatus()));
assertThat(response.getBody(), containsString("no permissions for [indices:data/write/index] and User [name=admin"));
}
}

@Test
public void testPluginShouldNotBeAbleToRunClusterActions() {
try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
HttpResponse response = client.get("try-cluster-health/plugin");

assertThat(response.getStatusCode(), equalTo(RestStatus.FORBIDDEN.getStatus()));
assertThat(
response.getBody(),
containsString(
"no permissions for [cluster:monitor/health] and User [name=plugin:org.opensearch.security.plugin.SystemIndexPlugin1"
)
);
}
}

@Test
public void testAdminUserShouldBeAbleToRunClusterActions() {
try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
HttpResponse response = client.get("try-cluster-health/user");

assertThat(response.getStatusCode(), equalTo(RestStatus.OK.getStatus()));
}
}

@Test
public void testAuthenticatedUserShouldBeAbleToRunClusterActions() {
try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
HttpResponse response = client.get("try-cluster-health/default");

assertThat(response.getStatusCode(), equalTo(RestStatus.OK.getStatus()));
}
}

@Test
public void testPluginShouldBeAbleToBulkIndexDocumentIntoItsSystemIndex() {
try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
HttpResponse response = client.put("try-create-and-bulk-index/" + SYSTEM_INDEX_1);

assertThat(response.getStatusCode(), equalTo(RestStatus.OK.getStatus()));
}
}

@Test
public void testPluginShouldNotBeAbleToBulkIndexDocumentIntoMixOfSystemIndexWhereAtLeastOneDoesNotBelongToPlugin() {
try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) {
client.put(".system-index1");
client.put(".system-index2");
}
try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
HttpResponse response = client.put("try-create-and-bulk-mixed-index");

assertThat(
response.getBody(),
containsString(
"no permissions for [indices:data/write/bulk[s]] and User [name=plugin:org.opensearch.security.plugin.SystemIndexPlugin1"
)
);
}
}

@Test
public void regularUserShouldGetNoResultsWhenSearchingSystemIndex() {
// Create system index and index a dummy document as the super admin user, data returned to super admin
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/*
* Copyright OpenSearch Contributors
* 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.plugin;

import org.opensearch.action.ActionType;

public class IndexDocumentIntoSystemIndexAction extends ActionType<IndexDocumentIntoSystemIndexResponse> {
public static final IndexDocumentIntoSystemIndexAction INSTANCE = new IndexDocumentIntoSystemIndexAction();
public static final String NAME = "mock:systemindex/index";

private IndexDocumentIntoSystemIndexAction() {
super(NAME, IndexDocumentIntoSystemIndexResponse::new);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
* Copyright OpenSearch Contributors
* 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.plugin;

import java.io.IOException;

import org.opensearch.action.ActionRequest;
import org.opensearch.action.ActionRequestValidationException;
import org.opensearch.core.common.io.stream.StreamInput;

public class IndexDocumentIntoSystemIndexRequest extends ActionRequest {

private final String indexName;

private final String runAs;

public IndexDocumentIntoSystemIndexRequest(String indexName, String runAs) {
this.indexName = indexName;
this.runAs = runAs;
}

public IndexDocumentIntoSystemIndexRequest(StreamInput in) throws IOException {
super(in);
this.indexName = in.readString();
this.runAs = in.readOptionalString();
}

@Override
public ActionRequestValidationException validate() {
return null;
}

public String getIndexName() {
return this.indexName;
}

public String getRunAs() {
return this.runAs;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
* Copyright OpenSearch Contributors
* 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.plugin;

// CS-SUPPRESS-SINGLE: RegexpSingleline It is not possible to use phrase "cluster manager" instead of master here
import java.io.IOException;

import org.opensearch.action.support.master.AcknowledgedResponse;
import org.opensearch.core.common.io.stream.StreamInput;
import org.opensearch.core.common.io.stream.StreamOutput;
import org.opensearch.core.xcontent.ToXContent;
import org.opensearch.core.xcontent.ToXContentObject;
import org.opensearch.core.xcontent.XContentBuilder;
// CS-ENFORCE-SINGLE

public class IndexDocumentIntoSystemIndexResponse extends AcknowledgedResponse implements ToXContentObject {

private String plugin;

public IndexDocumentIntoSystemIndexResponse(boolean status, String plugin) {
super(status);
this.plugin = plugin;
}

public IndexDocumentIntoSystemIndexResponse(StreamInput in) throws IOException {
super(in);
}

@Override
public void writeTo(StreamOutput out) throws IOException {
super.writeTo(out);
out.writeString(plugin);
}

@Override
public void addCustomFields(XContentBuilder builder, ToXContent.Params params) throws IOException {
super.addCustomFields(builder, params);
builder.field("plugin", plugin);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/*
* Copyright OpenSearch Contributors
* 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.plugin;

import java.util.List;

import org.opensearch.action.bulk.BulkRequest;
import org.opensearch.action.bulk.BulkRequestBuilder;
import org.opensearch.action.index.IndexRequest;
import org.opensearch.action.support.WriteRequest;
import org.opensearch.client.Client;
import org.opensearch.client.node.NodeClient;
import org.opensearch.common.xcontent.XContentType;
import org.opensearch.core.action.ActionListener;
import org.opensearch.core.rest.RestStatus;
import org.opensearch.core.xcontent.ToXContent;
import org.opensearch.rest.BaseRestHandler;
import org.opensearch.rest.BytesRestResponse;
import org.opensearch.rest.RestChannel;
import org.opensearch.rest.RestRequest;
import org.opensearch.security.identity.PluginContextSwitcher;

import static java.util.Collections.singletonList;
import static org.opensearch.rest.RestRequest.Method.PUT;
import static org.opensearch.security.plugin.SystemIndexPlugin1.SYSTEM_INDEX_1;
import static org.opensearch.security.plugin.SystemIndexPlugin2.SYSTEM_INDEX_2;

public class RestBulkIndexDocumentIntoMixOfSystemIndexAction extends BaseRestHandler {

private final Client client;
private final PluginContextSwitcher contextSwitcher;

public RestBulkIndexDocumentIntoMixOfSystemIndexAction(Client client, PluginContextSwitcher contextSwitcher) {
this.client = client;
this.contextSwitcher = contextSwitcher;
}

@Override
public List<Route> routes() {
return singletonList(new Route(PUT, "/try-create-and-bulk-mixed-index"));
}

@Override
public String getName() {
return "test_bulk_index_document_into_mix_of_system_index_action";
}

@Override
public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) {
return new RestChannelConsumer() {

@Override
public void accept(RestChannel channel) throws Exception {
contextSwitcher.runAs(() -> {
BulkRequestBuilder builder = client.prepareBulk();
builder.add(new IndexRequest(SYSTEM_INDEX_1).source("{\"content\":1}", XContentType.JSON));
builder.add(new IndexRequest(SYSTEM_INDEX_2).source("{\"content\":1}", XContentType.JSON));
builder.setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE);
BulkRequest bulkRequest = builder.request();
client.bulk(bulkRequest, ActionListener.wrap(r -> {
channel.sendResponse(
new BytesRestResponse(RestStatus.OK, r.toXContent(channel.newBuilder(), ToXContent.EMPTY_PARAMS))
);
}, fr -> { channel.sendResponse(new BytesRestResponse(RestStatus.FORBIDDEN, String.valueOf(fr))); }));
return null;
});
}
};
}
}
Loading
Loading