Skip to content

Commit

Permalink
Introduced legacy impl behind a feature flag
Browse files Browse the repository at this point in the history
Signed-off-by: Nils Bandener <[email protected]>
  • Loading branch information
nibix committed Dec 30, 2024
1 parent 3743a1e commit cc1d22a
Show file tree
Hide file tree
Showing 23 changed files with 3,771 additions and 887 deletions.
1 change: 1 addition & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -751,6 +751,7 @@ dependencies {
integrationTestImplementation "org.apache.httpcomponents:fluent-hc:4.5.14"
integrationTestImplementation "org.apache.httpcomponents:httpcore:4.4.16"
integrationTestImplementation "org.apache.httpcomponents:httpasyncclient:4.1.5"
integrationTestImplementation "org.mockito:mockito-core:5.14.2"

//spotless
implementation('com.google.googlejavaformat:google-java-format:1.25.2') {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
/*
* 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.privileges.legacy;

import java.util.Map;

import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
import org.apache.http.HttpStatus;
import org.junit.ClassRule;
import org.junit.Test;
import org.junit.runner.RunWith;

import org.opensearch.script.mustache.MustachePlugin;
import org.opensearch.script.mustache.RenderSearchTemplateAction;
import org.opensearch.security.privileges.PrivilegesEvaluator;
import org.opensearch.test.framework.TestSecurityConfig;
import org.opensearch.test.framework.TestSecurityConfig.Role;
import org.opensearch.test.framework.cluster.ClusterManager;
import org.opensearch.test.framework.cluster.LocalCluster;
import org.opensearch.test.framework.cluster.TestRestClient;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.equalTo;
import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL;

@RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class)
@ThreadLeakScope(ThreadLeakScope.Scope.NONE)
public class PrivilegesEvaluatorTest {

protected final static TestSecurityConfig.User NEGATIVE_LOOKAHEAD = new TestSecurityConfig.User("negative_lookahead_user").roles(
new Role("negative_lookahead_role").indexPermissions("read").on("/^(?!t.*).*/").clusterPermissions("cluster_composite_ops")
);

protected final static TestSecurityConfig.User NEGATED_REGEX = new TestSecurityConfig.User("negated_regex_user").roles(
new Role("negated_regex_role").indexPermissions("read").on("/^[a-z].*/").clusterPermissions("cluster_composite_ops")
);

protected final static TestSecurityConfig.User SEARCH_TEMPLATE = new TestSecurityConfig.User("search_template_user").roles(
new Role("search_template_role").indexPermissions("read").on("services").clusterPermissions("cluster_composite_ops")
);

protected final static TestSecurityConfig.User RENDER_SEARCH_TEMPLATE = new TestSecurityConfig.User("render_search_template_user")
.roles(
new Role("render_search_template_role").indexPermissions("read")
.on("services")
.clusterPermissions(RenderSearchTemplateAction.NAME)
);

private String TEST_QUERY =
"{\"source\":{\"query\":{\"match\":{\"service\":\"{{service_name}}\"}}},\"params\":{\"service_name\":\"Oracle\"}}";

private String TEST_DOC = "{\"source\": {\"title\": \"Spirited Away\"}}";

private String TEST_RENDER_SEARCH_TEMPLATE_QUERY =
"{\"params\":{\"status\":[\"pending\",\"published\"]},\"source\":\"{\\\"query\\\": {\\\"terms\\\": {\\\"status\\\": [\\\"{{#status}}\\\",\\\"{{.}}\\\",\\\"{{/status}}\\\"]}}}\"}";

@ClassRule
public static LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.THREE_CLUSTER_MANAGERS)
.authc(AUTHC_HTTPBASIC_INTERNAL)
.users(NEGATIVE_LOOKAHEAD, NEGATED_REGEX, SEARCH_TEMPLATE, RENDER_SEARCH_TEMPLATE, TestSecurityConfig.User.USER_ADMIN)
.plugin(MustachePlugin.class)
.nodeSettings(Map.of(PrivilegesEvaluator.USE_LEGACY_PRIVILEGE_EVALUATOR.getKey(), true))
.build();

@Test
public void testNegativeLookaheadPattern() throws Exception {

try (TestRestClient client = cluster.getRestClient(NEGATIVE_LOOKAHEAD)) {
assertThat(client.get("*/_search").getStatusCode(), equalTo(HttpStatus.SC_FORBIDDEN));
assertThat(client.get("r*/_search").getStatusCode(), equalTo(HttpStatus.SC_OK));
}
}

@Test
public void testRegexPattern() throws Exception {

try (TestRestClient client = cluster.getRestClient(NEGATED_REGEX)) {
assertThat(client.get("*/_search").getStatusCode(), equalTo(HttpStatus.SC_FORBIDDEN));
assertThat(client.get("r*/_search").getStatusCode(), equalTo(HttpStatus.SC_OK));
}

}

@Test
public void testSearchTemplateRequestSuccess() {
// Insert doc into services index with admin user
try (TestRestClient client = cluster.getRestClient(TestSecurityConfig.User.USER_ADMIN)) {
TestRestClient.HttpResponse response = client.postJson("services/_doc", TEST_DOC);
assertThat(response.getStatusCode(), equalTo(HttpStatus.SC_CREATED));
}

try (TestRestClient client = cluster.getRestClient(SEARCH_TEMPLATE)) {
final String searchTemplateOnServicesIndex = "services/_search/template";
final TestRestClient.HttpResponse searchTemplateOnAuthorizedIndexResponse = client.getWithJsonBody(
searchTemplateOnServicesIndex,
TEST_QUERY
);
assertThat(searchTemplateOnAuthorizedIndexResponse.getStatusCode(), equalTo(HttpStatus.SC_OK));
}
}

@Test
public void testSearchTemplateRequestUnauthorizedIndex() {
try (TestRestClient client = cluster.getRestClient(SEARCH_TEMPLATE)) {
final String searchTemplateOnMoviesIndex = "movies/_search/template";
final TestRestClient.HttpResponse searchTemplateOnUnauthorizedIndexResponse = client.getWithJsonBody(
searchTemplateOnMoviesIndex,
TEST_QUERY
);
assertThat(searchTemplateOnUnauthorizedIndexResponse.getStatusCode(), equalTo(HttpStatus.SC_FORBIDDEN));
}
}

@Test
public void testSearchTemplateRequestUnauthorizedAllIndices() {
try (TestRestClient client = cluster.getRestClient(SEARCH_TEMPLATE)) {
final String searchTemplateOnAllIndices = "_search/template";
final TestRestClient.HttpResponse searchOnAllIndicesResponse = client.getWithJsonBody(searchTemplateOnAllIndices, TEST_QUERY);
assertThat(searchOnAllIndicesResponse.getStatusCode(), equalTo(HttpStatus.SC_FORBIDDEN));
}
}

@Test
public void testRenderSearchTemplateRequestFailure() {
try (TestRestClient client = cluster.getRestClient(SEARCH_TEMPLATE)) {
final String renderSearchTemplate = "_render/template";
final TestRestClient.HttpResponse renderSearchTemplateResponse = client.postJson(
renderSearchTemplate,
TEST_RENDER_SEARCH_TEMPLATE_QUERY
);
assertThat(renderSearchTemplateResponse.getStatusCode(), equalTo(HttpStatus.SC_FORBIDDEN));
}
}

@Test
public void testRenderSearchTemplateRequestSuccess() {
try (TestRestClient client = cluster.getRestClient(RENDER_SEARCH_TEMPLATE)) {
final String renderSearchTemplate = "_render/template";
final TestRestClient.HttpResponse renderSearchTemplateResponse = client.postJson(
renderSearchTemplate,
TEST_RENDER_SEARCH_TEMPLATE_QUERY
);
assertThat(renderSearchTemplateResponse.getStatusCode(), equalTo(HttpStatus.SC_OK));
}
}
}
54 changes: 38 additions & 16 deletions src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java
Original file line number Diff line number Diff line change
Expand Up @@ -1121,20 +1121,43 @@ public Collection<Object> createComponents(

final CompatConfig compatConfig = new CompatConfig(environment, transportPassiveAuthSetting);

evaluator = new PrivilegesEvaluator(
clusterService,
clusterService::state,
threadPool,
threadPool.getThreadContext(),
cr,
resolver,
auditLog,
settings,
privilegesInterceptor,
cih,
irr,
namedXContentRegistry.get()
);
if (PrivilegesEvaluator.USE_LEGACY_PRIVILEGE_EVALUATOR.get(settings)) {
// Use legacy implementation (non-default)
evaluator = new org.opensearch.security.privileges.legacy.PrivilegesEvaluatorImpl(
clusterService,
threadPool,
cr,
resolver,
auditLog,
settings,
privilegesInterceptor,
cih,
irr,
namedXContentRegistry.get()
);

restLayerEvaluator = new org.opensearch.security.privileges.legacy.RestLayerPrivilegesEvaluatorImpl(clusterService, threadPool);
} else {
// Use new implementation (default)
evaluator = new org.opensearch.security.privileges.PrivilegesEvaluatorImpl(
clusterService,
clusterService::state,
threadPool,
threadPool.getThreadContext(),
cr,
resolver,
auditLog,
settings,
privilegesInterceptor,
cih,
irr,
namedXContentRegistry.get()
);

restLayerEvaluator = new org.opensearch.security.privileges.RestLayerPrivilegesEvaluatorImpl(
(org.opensearch.security.privileges.PrivilegesEvaluatorImpl) evaluator
);
}

sf = new SecurityFilter(settings, evaluator, adminDns, dlsFlsValve, auditLog, threadPool, cs, compatConfig, irr, xffResolver);

Expand All @@ -1146,8 +1169,6 @@ public Collection<Object> createComponents(
principalExtractor = ReflectionHelper.instantiatePrincipalExtractor(principalExtractorClass);
}

restLayerEvaluator = new RestLayerPrivilegesEvaluator(evaluator);

securityRestHandler = new SecurityRestFilter(
backendRegistry,
restLayerEvaluator,
Expand Down Expand Up @@ -2062,6 +2083,7 @@ public List<Setting<?>> getSettings() {

// Privileges evaluation
settings.add(ActionPrivileges.PRECOMPUTED_PRIVILEGES_MAX_HEAP_SIZE);
settings.add(PrivilegesEvaluator.USE_LEGACY_PRIVILEGE_EVALUATOR);
}

return settings;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@
import org.opensearch.security.privileges.PrivilegesEvaluationContext;
import org.opensearch.security.privileges.PrivilegesEvaluator;
import org.opensearch.security.privileges.PrivilegesEvaluatorResponse;
import org.opensearch.security.resolver.IndexResolverReplacer;
import org.opensearch.security.securityconf.ConfigModel;
import org.opensearch.security.support.ConfigConstants;
import org.opensearch.security.support.HeaderHelper;
Expand Down Expand Up @@ -170,8 +169,7 @@ protected final boolean isBlockedSystemIndexRequest() {

String permission = ConfigConstants.SYSTEM_INDEX_PERMISSION;
PrivilegesEvaluationContext context = evaluator.createContext(user, permission);
PrivilegesEvaluatorResponse result = evaluator.getActionPrivileges()
.hasExplicitIndexPrivilege(context, Set.of(permission), IndexResolverReplacer.Resolved.ofIndex(index.getName()));
PrivilegesEvaluatorResponse result = evaluator.hasExplicitIndexPrivilege(context, Set.of(permission), index.getName());

return !result.isAllowed();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public class PrivilegesEvaluationContext {
*/
private final Map<String, WildcardMatcher> renderedPatternTemplateCache = new HashMap<>();

PrivilegesEvaluationContext(
public PrivilegesEvaluationContext(
User user,
ImmutableSet<String> mappedRoles,
String action,
Expand Down Expand Up @@ -135,7 +135,7 @@ public ImmutableSet<String> getMappedRoles() {
* However, this method should be only used for this one particular phase. Normally, all roles should be determined
* upfront and stay constant during the whole privilege evaluation process.
*/
void setMappedRoles(ImmutableSet<String> mappedRoles) {
public void setMappedRoles(ImmutableSet<String> mappedRoles) {
this.mappedRoles = mappedRoles;
}

Expand Down
Loading

0 comments on commit cc1d22a

Please sign in to comment.