[Actions] Do not return system actions from GET APIs (elastic#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);
  });
  });
  });