Skip to content

Commit

Permalink
Separate config option to enable restapi: permissions (#2605) (#2818)
Browse files Browse the repository at this point in the history 
Added config settings
plugins.security.restapi.admin.enabled which enables/disables :resapi permissions.
Default is false

Signed-off-by: Andrey Pleskach <ples@aiven.io>
(cherry picked from commit 6446268)

Co-authored-by: Andrey Pleskach <ples@aiven.io>
  • Loading branch information
@DarshitChanpura @willyborankin
DarshitChanpura and willyborankin authored May 31, 2023
1 parent ce5b295 commit 570d617
Show file tree
Hide file tree
Showing 12 changed files with 84 additions and 54 deletions.
3 changes: 3 additions & 0 deletions src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1041,6 +1041,8 @@ public List<Setting<?>> getSettings() {
settings.add(Setting.listSetting(ConfigConstants.SECURITY_RESTAPI_ROLES_ENABLED, Collections.emptyList(), Function.identity(), Property.NodeScope)); //not filtered here
settings.add(Setting.groupSetting(ConfigConstants.SECURITY_RESTAPI_ENDPOINTS_DISABLED + ".", Property.NodeScope));

settings.add(Setting.boolSetting(ConfigConstants.SECURITY_RESTAPI_ADMIN_ENABLED, false, Property.NodeScope, Property.Filtered));

settings.add(Setting.simpleString(ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_REGEX, Property.NodeScope, Property.Filtered));
settings.add(Setting.simpleString(ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_ERROR_MESSAGE, Property.NodeScope, Property.Filtered));

Expand Down Expand Up @@ -1192,6 +1194,7 @@ public static class GuiceHolder implements LifecycleComponent {
private static RepositoriesService repositoriesService;
private static RemoteClusterService remoteClusterService;
private static IndicesService indicesService;

private static PitService pitService;

private static ExtensionsManager extensionsManager;
Expand Down
6 changes: 5 additions & 1 deletion src/main/java/org/opensearch/security/dlic/rest/api/AbstractApiAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@
import org.opensearch.security.user.User;
import org.opensearch.threadpool.ThreadPool;

import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_ADMIN_ENABLED;

public abstract class AbstractApiAction extends BaseRestHandler {

protected final Logger log = LogManager.getLogger(this.getClass());
Expand Down Expand Up @@ -94,7 +96,9 @@ protected AbstractApiAction(final Settings settings, final Path configPath, fina
this.restApiPrivilegesEvaluator = new RestApiPrivilegesEvaluator(settings, adminDNs, evaluator,
principalExtractor, configPath, threadPool);
this.restApiAdminPrivilegesEvaluator =
new RestApiAdminPrivilegesEvaluator(threadPool.getThreadContext(), evaluator, adminDNs);
new RestApiAdminPrivilegesEvaluator(
threadPool.getThreadContext(), evaluator, adminDNs,
settings.getAsBoolean(SECURITY_RESTAPI_ADMIN_ENABLED, false));
this.auditLog = auditLog;
}

Expand Down
33 changes: 25 additions & 8 deletions src/main/java/org/opensearch/security/dlic/rest/api/RestApiAdminPrivilegesEvaluator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
import org.opensearch.security.support.WildcardMatcher;
import org.opensearch.security.user.User;

import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_ADMIN_ENABLED;

public class RestApiAdminPrivilegesEvaluator {

protected final Logger logger = LogManager.getLogger(RestApiAdminPrivilegesEvaluator.class);
Expand Down Expand Up @@ -85,13 +87,17 @@ default String build() {

private final AdminDNs adminDNs;

private final boolean restapiAdminEnabled;

public RestApiAdminPrivilegesEvaluator(
final ThreadContext threadContext,
final PrivilegesEvaluator privilegesEvaluator,
final AdminDNs adminDNs) {
final AdminDNs adminDNs,
final boolean restapiAdminEnabled) {
this.threadContext = threadContext;
this.privilegesEvaluator = privilegesEvaluator;
this.adminDNs = adminDNs;
this.restapiAdminEnabled = restapiAdminEnabled;
}

public boolean isCurrentUserRestApiAdminFor(final Endpoint endpoint, final String action) {
Expand All @@ -108,20 +114,31 @@ public boolean isCurrentUserRestApiAdminFor(final Endpoint endpoint, final Strin
return true;
}
if (!ENDPOINTS_WITH_PERMISSIONS.containsKey(endpoint)) {
if (logger.isDebugEnabled()) {
logger.debug("No permission found for {} endpoint", endpoint);
}
logger.debug("No permission found for {} endpoint", endpoint);
return false;
}
final String permission = ENDPOINTS_WITH_PERMISSIONS.get(endpoint).build(action);
if (logger.isDebugEnabled()) {
logger.debug("Checking permission {} for endpoint {}", permission, endpoint);
}
return privilegesEvaluator.hasRestAdminPermissions(
final boolean hasAccess = privilegesEvaluator.hasRestAdminPermissions(
userAndRemoteAddress.getLeft(),
userAndRemoteAddress.getRight(),
permission
);
if (logger.isDebugEnabled()) {
logger.debug(
"User {} with permission {} {} access to endpoint {}",
userAndRemoteAddress.getLeft().getName(),
permission,
hasAccess ? "has" : "has no",
endpoint
);
logger.debug(
"{} set to {}. {} use access decision",
SECURITY_RESTAPI_ADMIN_ENABLED,
restapiAdminEnabled,
restapiAdminEnabled ? "Will" : "Will not"
);
}
return hasAccess && restapiAdminEnabled;
}

public boolean containsRestApiAdminPermissions(final Object configObject) {
Expand Down
4 changes: 0 additions & 4 deletions src/main/java/org/opensearch/security/dlic/rest/api/SecuritySSLCertsAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,6 @@ public class SecuritySSLCertsAction extends AbstractApiAction {

private final boolean certificatesReloadEnabled;

private final RestApiAdminPrivilegesEvaluator restApiAdminPrivilegesEvaluator;

private final boolean httpsEnabled;

public SecuritySSLCertsAction(final Settings settings,
Expand All @@ -91,8 +89,6 @@ public SecuritySSLCertsAction(final Settings settings,
final boolean certificatesReloadEnabled) {
super(settings, configPath, controller, client, adminDNs, cl, cs, principalExtractor, privilegesEvaluator, threadPool, auditLog);
this.securityKeyStore = securityKeyStore;
this.restApiAdminPrivilegesEvaluator =
new RestApiAdminPrivilegesEvaluator(threadPool.getThreadContext(), privilegesEvaluator, adminDNs);
this.certificatesReloadEnabled = certificatesReloadEnabled;
this.httpsEnabled = settings.getAsBoolean(SSLConfigConstants.SECURITY_SSL_HTTP_ENABLED, true);
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/opensearch/security/support/ConfigConstants.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,13 +250,13 @@ public enum RolesMappingResolution {
public static final String SECURITY_DLS_MODE = "plugins.security.dls.mode";
// REST API
public static final String SECURITY_RESTAPI_ROLES_ENABLED = "plugins.security.restapi.roles_enabled";
public static final String SECURITY_RESTAPI_ADMIN_ENABLED = "plugins.security.restapi.admin.enabled";
public static final String SECURITY_RESTAPI_ENDPOINTS_DISABLED = "plugins.security.restapi.endpoints_disabled";
public static final String SECURITY_RESTAPI_PASSWORD_VALIDATION_REGEX = "plugins.security.restapi.password_validation_regex";
public static final String SECURITY_RESTAPI_PASSWORD_VALIDATION_ERROR_MESSAGE = "plugins.security.restapi.password_validation_error_message";
public static final String SECURITY_RESTAPI_PASSWORD_MIN_LENGTH = "plugins.security.restapi.password_min_length";
public static final String SECURITY_RESTAPI_PASSWORD_SCORE_BASED_VALIDATION_STRENGTH = "plugins.security.restapi.password_score_based_validation_strength";

// Illegal Opcodes from here on
public static final String SECURITY_UNSUPPORTED_DISABLE_REST_AUTH_INITIALLY = "plugins.security.unsupported.disable_rest_auth_initially";
public static final String SECURITY_UNSUPPORTED_DISABLE_INTERTRANSPORT_AUTH_INITIALLY = "plugins.security.unsupported.disable_intertransport_auth_initially";
public static final String SECURITY_UNSUPPORTED_PASSIVE_INTERTRANSPORT_AUTH_INITIALLY = "plugins.security.unsupported.passive_intertransport_auth_initially";
Expand Down
7 changes: 4 additions & 3 deletions src/test/java/org/opensearch/security/dlic/rest/api/ActionGroupsApiTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.opensearch.security.test.helper.rest.RestHelper.HttpResponse;

import static org.opensearch.security.OpenSearchSecurityPlugin.PLUGINS_PREFIX;
import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_ADMIN_ENABLED;

public class ActionGroupsApiTest extends AbstractRestApiUnitTest {
private final String ENDPOINT;
Expand Down Expand Up @@ -363,7 +364,7 @@ void verifyPatchForSuperAdmin(final Header[] header, final boolean userAdminCert

@Test
public void testActionGroupsApiForRestAdmin() throws Exception {
setupWithRestRoles();
setupWithRestRoles(Settings.builder().put(SECURITY_RESTAPI_ADMIN_ENABLED, true).build());
rh.sendAdminCertificate = false;
// create index
setupStarfleetIndex();
Expand All @@ -381,7 +382,7 @@ public void testActionGroupsApiForRestAdmin() throws Exception {

@Test
public void testActionGroupsApiForActionGroupsRestApiAdmin() throws Exception {
setupWithRestRoles();
setupWithRestRoles(Settings.builder().put(SECURITY_RESTAPI_ADMIN_ENABLED, true).build());
rh.sendAdminCertificate = false;
// create index
setupStarfleetIndex();
Expand All @@ -399,7 +400,7 @@ public void testActionGroupsApiForActionGroupsRestApiAdmin() throws Exception {

@Test
public void testCreateActionGroupWithRestAdminPermissionsForbidden() throws Exception {
setupWithRestRoles();
setupWithRestRoles(Settings.builder().put(SECURITY_RESTAPI_ADMIN_ENABLED, true).build());
rh.sendAdminCertificate = false;
final Header restApiAdminHeader = encodeBasicHeader("rest_api_admin_user", "rest_api_admin_user");
final Header restApiAdminActionGroupsHeader = encodeBasicHeader("rest_api_admin_actiongroups", "rest_api_admin_actiongroups");
Expand Down
5 changes: 3 additions & 2 deletions src/test/java/org/opensearch/security/dlic/rest/api/AllowlistApiTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import static org.hamcrest.Matchers.equalTo;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_ADMIN_ENABLED;

/**
* Testing class to verify that {@link AllowlistApiAction} works correctly.
Expand Down Expand Up @@ -158,7 +159,7 @@ public void testAllowlistApi() throws Exception {

@Test
public void testAllowlistApiWithPermissions() throws Exception {
setupWithRestRoles();
setupWithRestRoles(Settings.builder().put(SECURITY_RESTAPI_ADMIN_ENABLED, true).build());

final Header restApiAdminHeader = encodeBasicHeader("rest_api_admin_user", "rest_api_admin_user");
final Header restApiAllowlistHeader = encodeBasicHeader("rest_api_admin_allowlist", "rest_api_admin_allowlist");
Expand All @@ -170,7 +171,7 @@ public void testAllowlistApiWithPermissions() throws Exception {

@Test
public void testAllowlistApiWithAllowListPermissions() throws Exception {
setupWithRestRoles();
setupWithRestRoles(Settings.builder().put(SECURITY_RESTAPI_ADMIN_ENABLED, true).build());

final Header restApiAllowlistHeader = encodeBasicHeader("rest_api_admin_allowlist", "rest_api_admin_allowlist");
final Header restApiUserHeader = encodeBasicHeader("test", "test");
Expand Down
6 changes: 5 additions & 1 deletion src/test/java/org/opensearch/security/dlic/rest/api/NodesDnApiTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.equalTo;
import static org.opensearch.security.OpenSearchSecurityPlugin.PLUGINS_PREFIX;
import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_ADMIN_ENABLED;

public class NodesDnApiTest extends AbstractRestApiUnitTest {
private HttpResponse response;
Expand Down Expand Up @@ -184,7 +185,10 @@ public void testNodesDnApi() throws Exception {

@Test
public void testNodesDnApiWithPermissions() throws Exception {
Settings settings = Settings.builder().put(ConfigConstants.SECURITY_NODES_DN_DYNAMIC_CONFIG_ENABLED, true)
Settings settings =
Settings.builder()
.put(ConfigConstants.SECURITY_NODES_DN_DYNAMIC_CONFIG_ENABLED, true)
.put(SECURITY_RESTAPI_ADMIN_ENABLED, true)
.build();
setupWithRestRoles(settings);
final Header restApiAdminHeader = encodeBasicHeader("rest_api_admin_user", "rest_api_admin_user");
Expand Down
15 changes: 9 additions & 6 deletions src/test/java/org/opensearch/security/dlic/rest/api/RolesApiTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.opensearch.security.test.helper.rest.RestHelper.HttpResponse;

import static org.opensearch.security.OpenSearchSecurityPlugin.PLUGINS_PREFIX;
import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_ADMIN_ENABLED;

public class RolesApiTest extends AbstractRestApiUnitTest {
private final String ENDPOINT;
Expand Down Expand Up @@ -77,15 +78,17 @@ public void testAllRolesForSuperAdmin() throws Exception {

@Test
public void testAllRolesForRestAdmin() throws Exception {
setupWithRestRoles();
setupWithRestRoles(Settings.builder().put(SECURITY_RESTAPI_ADMIN_ENABLED, true).build());
final Header restApiAdminHeader = encodeBasicHeader("rest_api_admin_user", "rest_api_admin_user");
rh.sendAdminCertificate = false;
checkSuperAdminRoles(new Header[]{restApiAdminHeader});
}

@Test
public void testAllRolesForRolesRestAdmin() throws Exception {
setupWithRestRoles();
setupWithRestRoles(Settings.builder().put(SECURITY_RESTAPI_ADMIN_ENABLED, true).build());
final Header restApiAdminRolesHeader = encodeBasicHeader("rest_api_admin_roles", "rest_api_admin_roles");
rh.sendAdminCertificate = false;
checkSuperAdminRoles(new Header[]{restApiAdminRolesHeader});
}

Expand Down Expand Up @@ -520,7 +523,7 @@ void verifyPatchForSuperAdmin(final Header[] header, final boolean sendAdminCert

@Test
public void testRolesApiWithAllRestApiPermissions() throws Exception {
setupWithRestRoles();
setupWithRestRoles(Settings.builder().put(SECURITY_RESTAPI_ADMIN_ENABLED, true).build());

final Header restApiAdminHeader = encodeBasicHeader("rest_api_admin_user", "rest_api_admin_user");

Expand All @@ -540,7 +543,7 @@ public void testRolesApiWithAllRestApiPermissions() throws Exception {

@Test
public void testRolesApiWithRestApiRolePermission() throws Exception {
setupWithRestRoles();
setupWithRestRoles(Settings.builder().put(SECURITY_RESTAPI_ADMIN_ENABLED, true).build());

final Header restApiRolesHeader = encodeBasicHeader("rest_api_admin_roles", "rest_api_admin_roles");

Expand All @@ -561,7 +564,7 @@ public void testRolesApiWithRestApiRolePermission() throws Exception {

@Test
public void testCreateOrUpdateRestApiAdminRoleForbiddenForNonSuperAdmin() throws Exception {
setupWithRestRoles();
setupWithRestRoles(Settings.builder().put(SECURITY_RESTAPI_ADMIN_ENABLED, true).build());
rh.sendAdminCertificate = false;

final Header restApiAdminHeader = encodeBasicHeader("rest_api_admin_user", "rest_api_admin_user");
Expand Down Expand Up @@ -633,7 +636,7 @@ public void testCreateOrUpdateRestApiAdminRoleForbiddenForNonSuperAdmin() throws

@Test
public void testDeleteRestApiAdminRoleForbiddenForNonSuperAdmin() throws Exception {
setupWithRestRoles();
setupWithRestRoles(Settings.builder().put(SECURITY_RESTAPI_ADMIN_ENABLED, true).build());
rh.sendAdminCertificate = false;

final Header restApiAdminHeader = encodeBasicHeader("rest_api_admin_user", "rest_api_admin_user");
Expand Down
5 changes: 3 additions & 2 deletions src/test/java/org/opensearch/security/dlic/rest/api/RolesMappingApiTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.opensearch.security.test.helper.rest.RestHelper.HttpResponse;

import static org.opensearch.security.OpenSearchSecurityPlugin.PLUGINS_PREFIX;
import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_ADMIN_ENABLED;

public class RolesMappingApiTest extends AbstractRestApiUnitTest {
private final String ENDPOINT;
Expand Down Expand Up @@ -98,7 +99,7 @@ public void testRolesMappingApi() throws Exception {

@Test
public void testRolesMappingApiWithFullPermissions() throws Exception {
setupWithRestRoles();
setupWithRestRoles(Settings.builder().put(SECURITY_RESTAPI_ADMIN_ENABLED, true).build());
rh.sendAdminCertificate = false;

final Header restApiAdminHeader = encodeBasicHeader("rest_api_admin_user", "rest_api_admin_user");
Expand Down Expand Up @@ -466,7 +467,7 @@ void verifyNonSuperAdminUser(final Header[] header) throws Exception {

@Test
public void testChangeRestApiAdminRoleMappingForbiddenForNonSuperAdmin() throws Exception {
setupWithRestRoles();
setupWithRestRoles(Settings.builder().put(SECURITY_RESTAPI_ADMIN_ENABLED, true).build());
rh.sendAdminCertificate = false;

final Header restApiAdminHeader = encodeBasicHeader("rest_api_admin_user", "rest_api_admin_user");
Expand Down
Loading

0 comments on commit 570d617

Please sign in to comment.