Restricted requests to Get Roles API omit reserved roles

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/role/GetRolesRequest.java

@@ -9,12 +9,12 @@
 import org.elasticsearch.action.ActionRequest;
 import org.elasticsearch.action.ActionRequestValidationException;
 import org.elasticsearch.common.Strings;
-import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 
 import java.io.IOException;
 
 import static org.elasticsearch.action.ValidateActions.addValidationError;
+import static org.elasticsearch.action.support.TransportAction.localOnly;
 
 /**
  * Request to retrieve roles from the security index
@@ -23,10 +23,7 @@ public class GetRolesRequest extends ActionRequest {
 
     private String[] names = Strings.EMPTY_ARRAY;
 
-    public GetRolesRequest(StreamInput in) throws IOException {
-        super(in);
-        names = in.readStringArray();
-    }
+    private boolean nativeOnly = false;
 
     public GetRolesRequest() {}
 
@@ -47,9 +44,16 @@ public String[] names() {
         return names;
     }
 
+    public void setNativeOnly(boolean nativeOnly) {
+        this.nativeOnly = nativeOnly;
+    }
+
+    public boolean nativeOnly() {
+        return this.nativeOnly;
+    }
+
     @Override
     public void writeTo(StreamOutput out) throws IOException {
-        super.writeTo(out);
-        out.writeStringArray(names);
+        localOnly();
     }
 }

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/role/GetRolesRequestBuilder.java

@@ -22,4 +22,9 @@ public GetRolesRequestBuilder names(String... names) {
         request.names(names);
         return this;
     }
+
+    public GetRolesRequestBuilder nativeOnly(boolean nativeOnly) {
+        request.setNativeOnly(nativeOnly);
+        return this;
+    }
 }

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/role/TransportGetRolesAction.java

@@ -8,9 +8,8 @@
 
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.support.ActionFilters;
-import org.elasticsearch.action.support.HandledTransportAction;
+import org.elasticsearch.action.support.TransportAction;
 import org.elasticsearch.common.inject.Inject;
-import org.elasticsearch.common.util.concurrent.EsExecutors;
 import org.elasticsearch.tasks.Task;
 import org.elasticsearch.transport.TransportService;
 import org.elasticsearch.xpack.core.security.action.role.GetRolesAction;
@@ -25,7 +24,7 @@
 import java.util.List;
 import java.util.Set;
 
-public class TransportGetRolesAction extends HandledTransportAction<GetRolesRequest, GetRolesResponse> {
+public class TransportGetRolesAction extends TransportAction<GetRolesRequest, GetRolesResponse> {
 
     private final NativeRolesStore nativeRolesStore;
     private final ReservedRolesStore reservedRolesStore;
@@ -37,21 +36,22 @@ public TransportGetRolesAction(
         TransportService transportService,
         ReservedRolesStore reservedRolesStore
     ) {
-        super(GetRolesAction.NAME, transportService, actionFilters, GetRolesRequest::new, EsExecutors.DIRECT_EXECUTOR_SERVICE);
+        super(GetRolesAction.NAME, actionFilters, transportService.getTaskManager());
         this.nativeRolesStore = nativeRolesStore;
         this.reservedRolesStore = reservedRolesStore;
     }
 
     @Override
     protected void doExecute(Task task, final GetRolesRequest request, final ActionListener<GetRolesResponse> listener) {
         final String[] requestedRoles = request.names();
+        final boolean includeReservedRoles = false == request.nativeOnly();
         final boolean specificRolesRequested = requestedRoles != null && requestedRoles.length > 0;
         final Set<String> rolesToSearchFor = new HashSet<>();
         final List<RoleDescriptor> roles = new ArrayList<>();
 
         if (specificRolesRequested) {
             for (String role : requestedRoles) {
-                if (ReservedRolesStore.isReserved(role)) {
+                if (includeReservedRoles && ReservedRolesStore.isReserved(role)) {
                     RoleDescriptor rd = ReservedRolesStore.roleDescriptor(role);
                     if (rd != null) {
                         roles.add(rd);
@@ -63,7 +63,7 @@ protected void doExecute(Task task, final GetRolesRequest request, final ActionL
                     rolesToSearchFor.add(role);
                 }
             }
-        } else {
+        } else if (includeReservedRoles) {
             roles.addAll(ReservedRolesStore.roleDescriptors());
         }
 

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/role/RestGetRolesAction.java

@@ -21,7 +21,6 @@
 import org.elasticsearch.xpack.core.security.action.role.GetRolesRequestBuilder;
 import org.elasticsearch.xpack.core.security.action.role.GetRolesResponse;
 import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
-import org.elasticsearch.xpack.security.rest.action.SecurityBaseRestHandler;
 
 import java.io.IOException;
 import java.util.List;
@@ -30,12 +29,9 @@
 
 /**
  * Rest endpoint to retrieve a Role from the security index
- *
- * <strong>Note:</strong> This class does not extend {@link NativeRoleBaseRestHandler} because it handles both reserved roles and native
- * roles, and should still be available even if native role management is disabled.
  */
-@ServerlessScope(Scope.INTERNAL)
-public class RestGetRolesAction extends SecurityBaseRestHandler {
+@ServerlessScope(Scope.PUBLIC)
+public class RestGetRolesAction extends NativeRoleBaseRestHandler {
 
     public RestGetRolesAction(Settings settings, XPackLicenseState licenseState) {
         super(settings, licenseState);
@@ -57,25 +53,39 @@ public String getName() {
     @Override
     public RestChannelConsumer innerPrepareRequest(RestRequest request, NodeClient client) throws IOException {
         final String[] roles = request.paramAsStringArray("name", Strings.EMPTY_ARRAY);
-        return channel -> new GetRolesRequestBuilder(client).names(roles).execute(new RestBuilderListener<>(channel) {
-            @Override
-            public RestResponse buildResponse(GetRolesResponse response, XContentBuilder builder) throws Exception {
-                builder.startObject();
-                for (RoleDescriptor role : response.roles()) {
-                    builder.field(role.getName(), role);
-                }
-                builder.endObject();
+        final boolean restrictRequest = request.hasParam(RestRequest.RESPONSE_RESTRICTED);
+        return channel -> new GetRolesRequestBuilder(client).names(roles)
+            .nativeOnly(restrictRequest)
+            .execute(new RestBuilderListener<>(channel) {
+                @Override
+                public RestResponse buildResponse(GetRolesResponse response, XContentBuilder builder) throws Exception {
+                    builder.startObject();
+                    for (RoleDescriptor role : response.roles()) {
+                        builder.field(role.getName(), role);
+                    }
+                    builder.endObject();
+
+                    // if the user asked for specific roles, but none of them were found
+                    // we'll return an empty result and 404 status code
+                    if (roles.length != 0 && response.roles().length == 0) {
+                        return new RestResponse(RestStatus.NOT_FOUND, builder);
+                    }
 
-                // if the user asked for specific roles, but none of them were found
-                // we'll return an empty result and 404 status code
-                if (roles.length != 0 && response.roles().length == 0) {
-                    return new RestResponse(RestStatus.NOT_FOUND, builder);
+                    // either the user asked for all roles, or at least one of the roles
+                    // the user asked for was found
+                    return new RestResponse(RestStatus.OK, builder);
                 }
+            });
+    }
 
-                // either the user asked for all roles, or at least one of the roles
-                // the user asked for was found
-                return new RestResponse(RestStatus.OK, builder);
-            }
-        });
+    @Override
+    protected Exception innerCheckFeatureAvailable(RestRequest request) {
+        // Note: For non-restricted requests this action handles both reserved roles and native
+        // roles, and should still be available even if native role management is disabled.
+        // For restricted requests it should only be available if native role management is enabled
+        if (false == request.hasParam(RestRequest.RESPONSE_RESTRICTED)) {
+            return null;
+        }
+        return super.innerCheckFeatureAvailable(request);
     }
 }