[Actions] Do not return system actions from GET APIs (#161845)

## Summary

This PR removes the ability to get system actions from `GET` APIs.
Specifically:

- The Get action API throws a 404 error when requesting a system action
- The Bulk Get action API throws a 404 error when requesting a system
action
- The Get All API filters out system actions
- The Get List Types API filters out system actions

### Checklist

Delete any items that are not applicable to this PR.

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

### For maintainers

- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

x-pack/plugins/actions/server/actions_client.ts

@@ -133,6 +133,15 @@ export interface UpdateOptions {
   action: ActionUpdate;
 }
 
+interface GetAllOptions {
+  includeSystemActions?: boolean;
+}
+
+interface ListTypesOptions {
+  featureId?: string;
+  includeSystemActionTypes?: boolean;
+}
+
 export class ActionsClient {
   private readonly logger: Logger;
   private readonly kibanaIndices: string[];
@@ -394,7 +403,13 @@ export class ActionsClient {
   /**
    * Get an action
    */
-  public async get({ id }: { id: string }): Promise<ActionResult> {
+  public async get({
+    id,
+    throwIfSystemAction = true,
+  }: {
+    id: string;
+    throwIfSystemAction?: boolean;
+  }): Promise<ActionResult> {
     try {
       await this.authorization.ensureAuthorized({ operation: 'get' });
     } catch (error) {
@@ -410,6 +425,19 @@ export class ActionsClient {
 
     const foundInMemoryConnector = this.inMemoryConnectors.find((connector) => connector.id === id);
 
+    /**
+     * Getting system connector is not allowed
+     * if throwIfSystemAction is set to true.
+     * Default behavior is to throw
+     */
+    if (
+      foundInMemoryConnector !== undefined &&
+      foundInMemoryConnector.isSystemAction &&
+      throwIfSystemAction
+    ) {
+      throw Boom.notFound(`Connector ${id} not found`);
+    }
+
     if (foundInMemoryConnector !== undefined) {
       this.auditLogger?.log(
         connectorAuditEvent({
@@ -452,7 +480,9 @@ export class ActionsClient {
   /**
    * Get all actions with in-memory connectors
    */
-  public async getAll(): Promise<FindActionResult[]> {
+  public async getAll({ includeSystemActions = false }: GetAllOptions = {}): Promise<
+    FindActionResult[]
+  > {
     try {
       await this.authorization.ensureAuthorized({ operation: 'get' });
     } catch (error) {
@@ -483,9 +513,13 @@ export class ActionsClient {
       )
     );
 
+    const inMemoryConnectorsFiltered = includeSystemActions
+      ? this.inMemoryConnectors
+      : this.inMemoryConnectors.filter((connector) => !connector.isSystemAction);
+
     const mergedResult = [
       ...savedObjectsActions,
-      ...this.inMemoryConnectors.map((inMemoryConnector) => ({
+      ...inMemoryConnectorsFiltered.map((inMemoryConnector) => ({
         id: inMemoryConnector.id,
         actionTypeId: inMemoryConnector.actionTypeId,
         name: inMemoryConnector.name,
@@ -500,7 +534,10 @@ export class ActionsClient {
   /**
    * Get bulk actions with in-memory list
    */
-  public async getBulk(ids: string[]): Promise<ActionResult[]> {
+  public async getBulk(
+    ids: string[],
+    throwIfSystemAction: boolean = true
+  ): Promise<ActionResult[]> {
     try {
       await this.authorization.ensureAuthorized({ operation: 'get' });
     } catch (error) {
@@ -523,6 +560,15 @@ export class ActionsClient {
         (inMemoryConnector) => inMemoryConnector.id === actionId
       );
 
+      /**
+       * Getting system connector is not allowed
+       * if throwIfSystemAction is set to true.
+       * Default behavior is to throw
+       */
+      if (action !== undefined && action.isSystemAction && throwIfSystemAction) {
+        throw Boom.notFound(`Connector ${action.id} not found`);
+      }
+
       if (action !== undefined) {
         actionResults.push(action);
       }
@@ -823,8 +869,21 @@ export class ActionsClient {
     return this.ephemeralExecutionEnqueuer(this.unsecuredSavedObjectsClient, options);
   }
 
-  public async listTypes(featureId?: string): Promise<ActionType[]> {
-    return this.actionTypeRegistry.list(featureId);
+  /**
+   * Return all available action types
+   * expect system action types
+   */
+  public async listTypes({
+    featureId,
+    includeSystemActionTypes = false,
+  }: ListTypesOptions = {}): Promise<ActionType[]> {
+    const actionTypes = this.actionTypeRegistry.list(featureId);
+
+    const filteredActionTypes = includeSystemActionTypes
+      ? actionTypes
+      : actionTypes.filter((actionType) => !Boolean(actionType.isSystemActionType));
+
+    return filteredActionTypes;
   }
 
   public isActionTypeEnabled(

x-pack/plugins/actions/server/plugin.ts

@@ -491,7 +491,13 @@ export class ActionsPlugin implements Plugin<PluginSetupContract, PluginStartCon
       return (objects?: SavedObjectsBulkGetObject[]) =>
         objects
           ? Promise.all(
-              objects.map(async (objectItem) => await (await client).get({ id: objectItem.id }))
+              objects.map(
+                async (objectItem) =>
+                  /**
+                   * TODO: Change with getBulk
+                   */
+                  await (await client).get({ id: objectItem.id, throwIfSystemAction: false })
+              )
             )
           : Promise.resolve([]);
     });

x-pack/plugins/actions/server/routes/connector_types.test.ts

@@ -142,7 +142,9 @@ describe('connectorTypesRoute', () => {
     expect(actionsClient.listTypes).toHaveBeenCalledTimes(1);
     expect(actionsClient.listTypes.mock.calls[0]).toMatchInlineSnapshot(`
       Array [
-        "alerting",
+        Object {
+          "featureId": "alerting",
+        },
       ]
     `);
 

x-pack/plugins/actions/server/routes/connector_types.ts

@@ -51,7 +51,7 @@ export const connectorTypesRoute = (
       verifyAccessAndContext(licenseState, async function (context, req, res) {
         const actionsClient = (await context.actions).getActionsClient();
         return res.ok({
-          body: rewriteBodyRes(await actionsClient.listTypes(req.query?.feature_id)),
+          body: rewriteBodyRes(await actionsClient.listTypes({ featureId: req.query?.feature_id })),
         });
       })
     )

x-pack/plugins/actions/server/routes/legacy/list_action_types.ts

@@ -12,6 +12,10 @@ import { BASE_ACTION_API_PATH } from '../../../common';
 import { ActionsRequestHandlerContext } from '../../types';
 import { trackLegacyRouteUsage } from '../../lib/track_legacy_route_usage';
 
+/**
+ * Return all available action types
+ * expect system action types
+ */
 export const listActionTypesRoute = (
   router: IRouter<ActionsRequestHandlerContext>,
   licenseState: ILicenseState,

x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/actions/get.ts

@@ -161,7 +161,7 @@ export default function getActionTests({ getService }: FtrProviderContext) {
           }
         });
 
-        it('should handle get system action request appropriately', async () => {
+        it('should throw when requesting a system action', async () => {
           const response = await supertestWithoutAuth
             .get(
               `${getUrlPrefix(space.id)}/api/actions/connector/system-connector-test.system-action`
@@ -183,14 +183,11 @@ export default function getActionTests({ getService }: FtrProviderContext) {
             case 'superuser at space1':
             case 'space_1_all at space1':
             case 'space_1_all_with_restricted_fixture at space1':
-              expect(response.statusCode).to.eql(200);
+              expect(response.statusCode).to.eql(404);
               expect(response.body).to.eql({
-                id: 'system-connector-test.system-action',
-                connector_type_id: 'test.system-action',
-                name: 'System action: test.system-action',
-                is_preconfigured: false,
-                is_system_action: true,
-                is_deprecated: false,
+                statusCode: 404,
+                error: 'Not Found',
+                message: 'Connector system-connector-test.system-action not found',
               });
               break;
             default:

x-pack/test/alerting_api_integration/security_and_spaces/group2/tests/actions/get_all.ts

@@ -125,24 +125,6 @@ export default function getAllActionTests({ getService }: FtrProviderContext) {
                   name: 'Slack#xyz',
                   referenced_by_count: 0,
                 },
-                {
-                  connector_type_id: 'test.system-action',
-                  id: 'system-connector-test.system-action',
-                  is_deprecated: false,
-                  is_preconfigured: false,
-                  is_system_action: true,
-                  name: 'System action: test.system-action',
-                  referenced_by_count: 0,
-                },
-                {
-                  connector_type_id: 'test.system-action-kibana-privileges',
-                  id: 'system-connector-test.system-action-kibana-privileges',
-                  is_deprecated: false,
-                  is_preconfigured: false,
-                  is_system_action: true,
-                  name: 'System action: test.system-action-kibana-privileges',
-                  referenced_by_count: 0,
-                },
                 {
                   id: 'custom-system-abc-connector',
                   is_preconfigured: true,
@@ -303,24 +285,6 @@ export default function getAllActionTests({ getService }: FtrProviderContext) {
                   name: 'Slack#xyz',
                   referenced_by_count: 0,
                 },
-                {
-                  connector_type_id: 'test.system-action',
-                  id: 'system-connector-test.system-action',
-                  is_deprecated: false,
-                  is_preconfigured: false,
-                  is_system_action: true,
-                  name: 'System action: test.system-action',
-                  referenced_by_count: 0,
-                },
-                {
-                  connector_type_id: 'test.system-action-kibana-privileges',
-                  id: 'system-connector-test.system-action-kibana-privileges',
-                  is_deprecated: false,
-                  is_preconfigured: false,
-                  is_system_action: true,
-                  name: 'System action: test.system-action-kibana-privileges',
-                  referenced_by_count: 0,
-                },
                 {
                   id: 'custom-system-abc-connector',
                   is_preconfigured: true,
@@ -444,24 +408,6 @@ export default function getAllActionTests({ getService }: FtrProviderContext) {
                   name: 'Slack#xyz',
                   referenced_by_count: 0,
                 },
-                {
-                  connector_type_id: 'test.system-action',
-                  id: 'system-connector-test.system-action',
-                  is_deprecated: false,
-                  is_preconfigured: false,
-                  is_system_action: true,
-                  name: 'System action: test.system-action',
-                  referenced_by_count: 0,
-                },
-                {
-                  connector_type_id: 'test.system-action-kibana-privileges',
-                  id: 'system-connector-test.system-action-kibana-privileges',
-                  is_deprecated: false,
-                  is_preconfigured: false,
-                  is_system_action: true,
-                  name: 'System action: test.system-action-kibana-privileges',
-                  referenced_by_count: 0,
-                },
                 {
                   id: 'custom-system-abc-connector',
                   is_preconfigured: true,

x-pack/test/alerting_api_integration/spaces_only/tests/actions/connector_types.ts

@@ -34,6 +34,16 @@ export default function listActionTypesTests({ getService }: FtrProviderContext)
       ).to.be(true);
     });
 
+    it('should filter out system action types', async () => {
+      const response = await supertest.get(
+        `${getUrlPrefix(Spaces.space1.id)}/api/actions/connector_types`
+      );
+
+      const actionTypes = response.body as Array<{ is_system_action_type: boolean }>;
+
+      expect(actionTypes.every((actionType) => !actionType.is_system_action_type)).to.be(true);
+    });
+
     describe('legacy', () => {
       it('should return 200 with list of action types containing defaults', async () => {
         const response = await supertest.get(
@@ -53,6 +63,16 @@ export default function listActionTypesTests({ getService }: FtrProviderContext)
           response.body.some(createActionTypeMatcher('test.index-record', 'Test: Index Record'))
         ).to.be(true);
       });
+
+      it('should filter out system action types', async () => {
+        const response = await supertest.get(
+          `${getUrlPrefix(Spaces.space1.id)}/api/actions/list_action_types`
+        );
+
+        const actionTypes = response.body as Array<{ is_system_action_type: boolean }>;
+
+        expect(actionTypes.every((actionType) => !actionType.is_system_action_type)).to.be(true);
+      });
     });
   });
 }

x-pack/test/alerting_api_integration/spaces_only/tests/actions/get.ts

@@ -90,21 +90,14 @@ export default function getActionTests({ getService }: FtrProviderContext) {
         });
     });
 
-    it('should handle get a system connector', async () => {
+    it('should return 404 when trying to get a system connector', async () => {
       await supertest
         .get(
           `${getUrlPrefix(
             Spaces.space1.id
           )}/api/actions/connector/system-connector-test.system-action`
         )
-        .expect(200, {
-          id: 'system-connector-test.system-action',
-          connector_type_id: 'test.system-action',
-          name: 'System action: test.system-action',
-          is_preconfigured: false,
-          is_system_action: true,
-          is_deprecated: false,
-        });
+        .expect(404);
     });
 
     it('should handle get a deprecated connector', async () => {
@@ -206,21 +199,14 @@ export default function getActionTests({ getService }: FtrProviderContext) {
           });
       });
 
-      it('should handle get a system connector', async () => {
+      it('should return 404 when trying to get a system connector', async () => {
         await supertest
           .get(
             `${getUrlPrefix(
               Spaces.space1.id
             )}/api/actions/action/system-connector-test.system-action`
           )
-          .expect(200, {
-            id: 'system-connector-test.system-action',
-            actionTypeId: 'test.system-action',
-            name: 'System action: test.system-action',
-            isPreconfigured: false,
-            isSystemAction: true,
-            isDeprecated: false,
-          });
+          .expect(404);
       });
     });
   });