Revert "Rest admin permissions (#2411) (#2466)"

config/roles.yml

@@ -9,20 +9,7 @@ kibana_read_only:
 # The security REST API access role is used to assign specific users access to change the security settings through the REST API.
 security_rest_api_access:
   reserved: true
-
-security_rest_api_full_access:
-  reserved: true
-  cluster_permissions:
-    - 'restapi:admin/actiongroups'
-    - 'restapi:admin/allowlist'
-    - 'restapi:admin/internalusers'
-    - 'restapi:admin/nodesdn'
-    - 'restapi:admin/roles'
-    - 'restapi:admin/rolesmapping'
-    - 'restapi:admin/ssl/certs/info'
-    - 'restapi:admin/ssl/certs/reload'
-    - 'restapi:admin/tenants'
-
+
 # Allows users to view monitors, destinations and alerts
 alerting_read_access:
   reserved: true

src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java

@@ -156,6 +156,8 @@
 import org.opensearch.security.ssl.OpenSearchSecuritySSLPlugin;
 import org.opensearch.security.ssl.SslExceptionHandler;
 import org.opensearch.security.ssl.http.netty.ValidatingDispatcher;
+import org.opensearch.security.ssl.rest.SecuritySSLCertsInfoAction;
+import org.opensearch.security.ssl.rest.SecuritySSLReloadCertsAction;
 import org.opensearch.security.ssl.transport.DefaultPrincipalExtractor;
 import org.opensearch.security.ssl.transport.SecuritySSLNettyTransport;
 import org.opensearch.security.ssl.util.SSLConfigConstants;
@@ -464,25 +466,18 @@ public List<RestHandler> getRestHandlers(Settings settings, RestController restC
             if(!SSLConfig.isSslOnlyMode()) {
                 handlers.add(new SecurityInfoAction(settings, restController, Objects.requireNonNull(evaluator), Objects.requireNonNull(threadPool)));
                 handlers.add(new SecurityHealthAction(settings, restController, Objects.requireNonNull(backendRegistry)));
+                handlers.add(new SecuritySSLCertsInfoAction(settings, restController, sks, Objects.requireNonNull(threadPool), Objects.requireNonNull(adminDns)));
                 handlers.add(new DashboardsInfoAction(settings, restController, Objects.requireNonNull(evaluator), Objects.requireNonNull(threadPool)));
                 handlers.add(new TenantInfoAction(settings, restController, Objects.requireNonNull(evaluator), Objects.requireNonNull(threadPool),
-                        Objects.requireNonNull(cs), Objects.requireNonNull(adminDns), Objects.requireNonNull(cr)));
-                handlers.add(new SecurityConfigUpdateAction(settings, restController, Objects.requireNonNull(threadPool), adminDns, configPath, principalExtractor));
-                handlers.add(new SecurityWhoAmIAction(settings, restController, Objects.requireNonNull(threadPool), adminDns, configPath, principalExtractor));
-                handlers.addAll(
-                        SecurityRestApiActions.getHandler(
-                                settings,
-                                configPath,
-                                restController,
-                                localClient,
-                                adminDns,
-                                cr, cs, principalExtractor,
-                                evaluator,
-                                threadPool,
-                                Objects.requireNonNull(auditLog), sks,
-                                sslCertReloadEnabled)
-                );
-                log.debug("Added {} rest handler(s)", handlers.size());
+				Objects.requireNonNull(cs), Objects.requireNonNull(adminDns), Objects.requireNonNull(cr)));
+                handlers.add(new SecurityConfigUpdateAction(settings, restController,Objects.requireNonNull(threadPool), adminDns, configPath, principalExtractor));
+                handlers.add(new SecurityWhoAmIAction(settings ,restController,Objects.requireNonNull(threadPool), adminDns, configPath, principalExtractor));
+                if (sslCertReloadEnabled) {
+                    handlers.add(new SecuritySSLReloadCertsAction(settings, restController, sks, Objects.requireNonNull(threadPool), Objects.requireNonNull(adminDns)));
+                }
+                final Collection<RestHandler> apiHandlers = SecurityRestApiActions.getHandler(settings, configPath, restController, localClient, adminDns, cr, cs, principalExtractor, evaluator, threadPool, Objects.requireNonNull(auditLog));
+                handlers.addAll(apiHandlers);
+                log.debug("Added {} management rest handler(s)", apiHandlers.size());
             }
         }
 

src/main/java/org/opensearch/security/dlic/rest/api/AbstractApiAction.java

@@ -75,9 +75,9 @@ public abstract class AbstractApiAction extends BaseRestHandler {
 	final ThreadPool threadPool;
 	protected String opendistroIndex;
 	private final RestApiPrivilegesEvaluator restApiPrivilegesEvaluator;
-	protected final RestApiAdminPrivilegesEvaluator restApiAdminPrivilegesEvaluator;
 	protected final AuditLog auditLog;
 	protected final Settings settings;
+	private AdminDNs adminDNs;
 
 	protected AbstractApiAction(final Settings settings, final Path configPath, final RestController controller,
                                 final Client client, final AdminDNs adminDNs, final ConfigurationRepository cl,
@@ -88,13 +88,12 @@ protected AbstractApiAction(final Settings settings, final Path configPath, fina
 		this.opendistroIndex = settings.get(ConfigConstants.SECURITY_CONFIG_INDEX_NAME,
 				ConfigConstants.OPENDISTRO_SECURITY_DEFAULT_CONFIG_INDEX);
 
+		this.adminDNs = adminDNs;
 		this.cl = cl;
 		this.cs = cs;
 		this.threadPool = threadPool;
 		this.restApiPrivilegesEvaluator = new RestApiPrivilegesEvaluator(settings, adminDNs, evaluator,
 				principalExtractor, configPath, threadPool);
-		this.restApiAdminPrivilegesEvaluator =
-				new RestApiAdminPrivilegesEvaluator(threadPool.getThreadContext(), evaluator, adminDNs);
 		this.auditLog = auditLog;
 	}
 
@@ -196,12 +195,7 @@ protected void handlePut(final RestChannel channel, final RestRequest request, f
 		}
 
 		boolean existed = existingConfiguration.exists(name);
-		final Object newContent = DefaultObjectMapper.readTree(content, existingConfiguration.getImplementingClass());
-		if (!hasPermissionsToCreate(existingConfiguration, newContent, getResourceName())) {
-			forbidden(channel, "No permissions");
-			return;
-		}
-		existingConfiguration.putCObject(name, newContent);
+		existingConfiguration.putCObject(name, DefaultObjectMapper.readTree(content, existingConfiguration.getImplementingClass()));
 
 		saveAnUpdateConfigs(client, request, getConfigName(), existingConfiguration, new OnSucessActionListener<IndexResponse>(channel) {
 
@@ -222,12 +216,6 @@ protected void handlePost(final RestChannel channel, final RestRequest request,
 		notImplemented(channel, Method.POST);
 	}
 
-	protected boolean hasPermissionsToCreate(final SecurityDynamicConfiguration<?> dynamicConfigFactory,
-											 final Object content,
-											 final String resourceName) throws IOException {
-		return false;
-	}
-
 	protected void handleGet(final RestChannel channel, RestRequest request, Client client, final JsonNode content)
 			throws IOException{
 
@@ -460,6 +448,7 @@ protected static XContentBuilder convertToJson(RestChannel channel, ToXContent t
 	}
 
 	protected void response(RestChannel channel, RestStatus status, String message) {
+
 		try {
 			final XContentBuilder builder = channel.newBuilder();
 			builder.startObject();
@@ -569,7 +558,8 @@ public String getName() {
 	protected abstract Endpoint getEndpoint();
 
 	protected boolean isSuperAdmin() {
-		return restApiAdminPrivilegesEvaluator.isCurrentUserRestApiAdminFor(getEndpoint());
+		User user = threadPool.getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER);
+		return adminDNs.isAdmin(user);
 	}
 
 	/**

src/main/java/org/opensearch/security/dlic/rest/api/AccountApiAction.java

@@ -83,13 +83,6 @@ public AccountApiAction(Settings settings,
         this.threadContext = threadPool.getThreadContext();
     }
 
-    @Override
-    protected boolean hasPermissionsToCreate(final SecurityDynamicConfiguration<?> dynamicConfigFactory,
-                                             final Object content,
-                                             final String resourceName) {
-        return true;
-    }
-
     @Override
     public List<Route> routes() {
         return routes;

src/main/java/org/opensearch/security/dlic/rest/api/ActionGroupsApiAction.java

@@ -118,35 +118,12 @@ protected void handlePut(RestChannel channel, RestRequest request, Client client
 
 		// Prevent the case where action group references to itself in the allowed_actions.
 		final SecurityDynamicConfiguration<?> existingActionGroupsConfig = load(getConfigName(), false);
-		final Object actionGroup = DefaultObjectMapper.readTree(content, existingActionGroupsConfig.getImplementingClass());
-		existingActionGroupsConfig.putCObject(name, actionGroup);
+		existingActionGroupsConfig.putCObject(name, DefaultObjectMapper.readTree(content, existingActionGroupsConfig.getImplementingClass()));
 		if (hasActionGroupSelfReference(existingActionGroupsConfig, name)) {
 			badRequestResponse(channel, name + " cannot be an allowed_action of itself");
 			return;
 		}
-		// prevent creation of groups for REST admin api
-		if (restApiAdminPrivilegesEvaluator.containsRestApiAdminPermissions(actionGroup)) {
-			forbidden(channel, "Not allowed");
-			return;
-		}
-		super.handlePut(channel, request, client, content);
-	}
 
-	@Override
-	protected boolean hasPermissionsToCreate(final SecurityDynamicConfiguration<?> dynamicConfiguration,
-											 final Object content,
-											 final String resourceName) throws IOException {
-		if (restApiAdminPrivilegesEvaluator.containsRestApiAdminPermissions(content)) {
-			return false;
-		}
-		return true;
-	}
-
-	@Override
-	protected boolean isReadOnly(SecurityDynamicConfiguration<?> existingConfiguration, String name) {
-		if (restApiAdminPrivilegesEvaluator.containsRestApiAdminPermissions(existingConfiguration.getCEntry(name))) {
-			return true;
-		}
-		return super.isReadOnly(existingConfiguration, name);
+		super.handlePut(channel, request, client, content);
 	}
 }

src/main/java/org/opensearch/security/dlic/rest/api/AllowlistApiAction.java

@@ -98,13 +98,6 @@ public AllowlistApiAction(final Settings settings, final Path configPath, final
         super(settings, configPath, controller, client, adminDNs, cl, cs, principalExtractor, evaluator, threadPool, auditLog);
     }
 
-    @Override
-    protected boolean hasPermissionsToCreate(final SecurityDynamicConfiguration<?> dynamicConfigFactory,
-                                             final Object content,
-                                             final String resourceName) {
-        return true;
-    }
-
     @Override
     protected void handleApiRequest(final RestChannel channel, final RestRequest request, final Client client) throws IOException {
         if (!isSuperAdmin()) {

src/main/java/org/opensearch/security/dlic/rest/api/AuditApiAction.java

@@ -165,13 +165,6 @@ public AuditApiAction(final Settings settings,
         }
     }
 
-    @Override
-    protected boolean hasPermissionsToCreate(final SecurityDynamicConfiguration<?> dynamicConfigFactory,
-                                             final Object content,
-                                             final String resourceName) {
-        return true;
-    }
-
     @Override
     public List<Route> routes() {
         return routes;

src/main/java/org/opensearch/security/dlic/rest/api/AuthTokenProcessorAction.java

@@ -34,7 +34,6 @@
 import org.opensearch.security.dlic.rest.validation.NoOpValidator;
 import org.opensearch.security.privileges.PrivilegesEvaluator;
 import org.opensearch.security.securityconf.impl.CType;
-import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration;
 import org.opensearch.security.ssl.transport.PrincipalExtractor;
 import org.opensearch.threadpool.ThreadPool;
 
@@ -54,13 +53,6 @@ public AuthTokenProcessorAction(final Settings settings, final Path configPath,
 				auditLog);
 	}
 
-	@Override
-	protected boolean hasPermissionsToCreate(final SecurityDynamicConfiguration<?> dynamicConfigFactory,
-											 final Object content,
-											 final String resourceName) {
-		return true;
-	}
-
 	@Override
 	public List<Route> routes() {
 		return routes;

src/main/java/org/opensearch/security/dlic/rest/api/Endpoint.java

@@ -28,6 +28,5 @@ public enum Endpoint {
     VALIDATE,
     WHITELIST,
     ALLOWLIST,
-    NODESDN,
-    SSL;
+    NODESDN;
 }

src/main/java/org/opensearch/security/dlic/rest/api/FlushCacheApiAction.java

@@ -38,7 +38,6 @@
 import org.opensearch.security.dlic.rest.validation.NoOpValidator;
 import org.opensearch.security.privileges.PrivilegesEvaluator;
 import org.opensearch.security.securityconf.impl.CType;
-import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration;
 import org.opensearch.security.ssl.transport.PrincipalExtractor;
 import org.opensearch.threadpool.ThreadPool;
 
@@ -59,13 +58,6 @@ public FlushCacheApiAction(final Settings settings, final Path configPath, final
 		super(settings, configPath, controller, client, adminDNs, cl, cs, principalExtractor, evaluator, threadPool, auditLog);
 	}
 
-	@Override
-	protected boolean hasPermissionsToCreate(final SecurityDynamicConfiguration<?> dynamicConfigFactory,
-											 final Object content,
-											 final String resourceName) {
-		return true;
-	}
-
 	@Override
 	public List<Route> routes() {
 		return routes;

src/main/java/org/opensearch/security/dlic/rest/api/InternalUsersApiAction.java

@@ -78,13 +78,6 @@ public InternalUsersApiAction(final Settings settings, final Path configPath, fi
                 auditLog);
     }
 
-    @Override
-    protected boolean hasPermissionsToCreate(final SecurityDynamicConfiguration<?> dynamicConfigFactory,
-                                             final Object content,
-                                             final String resourceName) {
-        return true;
-    }
-
     @Override
     public List<Route> routes() {
         return routes;

src/main/java/org/opensearch/security/dlic/rest/api/MigrateApiAction.java

@@ -94,13 +94,6 @@ protected Endpoint getEndpoint() {
         return Endpoint.MIGRATE;
     }
 
-    @Override
-    protected boolean hasPermissionsToCreate(final SecurityDynamicConfiguration<?> dynamicConfigFactory,
-                                             final Object content,
-                                             final String resourceName) {
-        return true;
-    }
-
     @SuppressWarnings("unchecked")
     @Override
     protected void handlePost(RestChannel channel, RestRequest request, Client client, final JsonNode content) throws IOException {

src/main/java/org/opensearch/security/dlic/rest/api/NodesDnApiAction.java

@@ -77,13 +77,6 @@ public NodesDnApiAction(final Settings settings, final Path configPath, final Re
         this.staticNodesDnFromEsYml = settings.getAsList(ConfigConstants.SECURITY_NODES_DN, Collections.emptyList());
     }
 
-    @Override
-    protected boolean hasPermissionsToCreate(final SecurityDynamicConfiguration<?> dynamicConfigFactory,
-                                             final Object content,
-                                             final String resourceName) {
-        return true;
-    }
-
     @Override
     public List<Route> routes() {
         if (settings.getAsBoolean(ConfigConstants.SECURITY_NODES_DN_DYNAMIC_CONFIG_ENABLED, false)) {

src/main/java/org/opensearch/security/dlic/rest/api/PatchableResourceApiAction.java

@@ -55,9 +55,9 @@ public abstract class PatchableResourceApiAction extends AbstractApiAction {
     protected final Logger log = LogManager.getLogger(this.getClass());
 
     public PatchableResourceApiAction(Settings settings, Path configPath, RestController controller, Client client,
-                                         AdminDNs adminDNs, ConfigurationRepository cl, ClusterService cs,
-                                         PrincipalExtractor principalExtractor, PrivilegesEvaluator evaluator, ThreadPool threadPool,
-                                         AuditLog auditLog) {
+                                      AdminDNs adminDNs, ConfigurationRepository cl, ClusterService cs,
+                                      PrincipalExtractor principalExtractor, PrivilegesEvaluator evaluator, ThreadPool threadPool,
+                                      AuditLog auditLog) {
         super(settings, configPath, controller, client, adminDNs, cl, cs, principalExtractor, evaluator, threadPool,
                 auditLog);
     }
@@ -146,7 +146,7 @@ private void handleSinglePatch(RestChannel channel, RestRequest request, Client
 
         if (!validator.validate()) {
             request.params().clear();
-            badRequestResponse(channel, validator);
+                badRequestResponse(channel, validator);
             return;
         }
 
@@ -156,7 +156,7 @@ private void handleSinglePatch(RestChannel channel, RestRequest request, Client
                                    , existingConfiguration.getVersion(), existingConfiguration.getSeqNo(), existingConfiguration.getPrimaryTerm());
 
         if (existingConfiguration.getCType().equals(CType.ACTIONGROUPS)) {
-            if (hasActionGroupSelfReference(mdc, name)) {
+            if(hasActionGroupSelfReference(mdc, name)) {
                 badRequestResponse(channel, name + " cannot be an allowed_action of itself");
                 return;
             }
@@ -188,6 +188,7 @@ private void handleBulkPatch(RestChannel channel, RestRequest request, Client cl
         for (String resourceName : existingConfiguration.getCEntries().keySet()) {
             JsonNode oldResource = existingAsObjectNode.get(resourceName);
             JsonNode patchedResource = patchedAsJsonNode.get(resourceName);
+
             if (oldResource != null && !oldResource.equals(patchedResource) && !isWriteable(channel, existingConfiguration, resourceName)) {
                 return;
             }
@@ -205,7 +206,7 @@ private void handleBulkPatch(RestChannel channel, RestRequest request, Client cl
             if(originalValidator != null) {
                 if (!originalValidator.validate()) {
                     request.params().clear();
-                    badRequestResponse(channel, originalValidator);
+                        badRequestResponse(channel, originalValidator);
                     return;
                 }
             }
@@ -221,13 +222,7 @@ private void handleBulkPatch(RestChannel channel, RestRequest request, Client cl
 
                 if (!validator.validate()) {
                     request.params().clear();
-                    badRequestResponse(channel, validator);
-                    return;
-                }
-                final Object newContent = DefaultObjectMapper.readTree(patchedResource, existingConfiguration.getImplementingClass());
-                if (!hasPermissionsToCreate(existingConfiguration, newContent, resourceName)) {
-                    request.params().clear();
-                    forbidden(channel, "No permissions");
+                        badRequestResponse(channel, validator);
                     return;
                 }
             }