[Actions] System actions enhancements (elastic#161340)

## Summary This PR: - Handles the references for system actions in the rule - Forbids the creation of system actions through the `kibana.yml` - Adds telemetry for system actions - Allow system action types to be disabled from the `kibana.yml` Depends on: elastic#160983, elastic#161341 ### 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) --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
ThomThomson · Aug 1, 2023 · 96fd477 · 96fd477
1 parent 5c91d94
commit 96fd477
Show file tree

Hide file tree

Showing 20 changed files with 1,249 additions and 282 deletions.
diff --git a/x-pack/plugins/actions/server/action_type_registry.test.ts b/x-pack/plugins/actions/server/action_type_registry.test.ts
@@ -49,10 +49,10 @@ describe('actionTypeRegistry', () => {
  isSystemAction: false,
  },
  {
- actionTypeId: '.cases',
+ actionTypeId: 'test.system-action',
  config: {},
- id: 'system-connector-.cases',
- name: 'System action: .cases',
+ id: 'system-connector-test.system-action',
+ name: 'System action: test.system-action',
  secrets: {},
  isPreconfigured: false,
  isDeprecated: false,
@@ -393,7 +393,7 @@ describe('actionTypeRegistry', () => {
  const actionTypeRegistry = new ActionTypeRegistry(actionTypeRegistryParams);
 
  actionTypeRegistry.register({
- id: '.cases',
+ id: 'test.system-action',
  name: 'Cases',
  minimumLicenseRequired: 'platinum',
  supportedFeatureIds: ['alerting'],
@@ -410,7 +410,7 @@ describe('actionTypeRegistry', () => {
 
  expect(actionTypes).toEqual([
  {
- id: '.cases',
+ id: 'test.system-action',
  name: 'Cases',
  enabled: true,
  enabledInConfig: true,
@@ -497,13 +497,16 @@ describe('actionTypeRegistry', () => {
  expect(actionTypeRegistry.isActionExecutable('my-slack1', 'foo')).toEqual(true);
  });
 
- test('should return true when isActionTypeEnabled is false and isLicenseValidForActionType is true and it has system connectors', async () => {
+ test('should return false when isActionTypeEnabled is false and isLicenseValidForActionType is true and it has system connectors', async () => {
  mockedActionsConfig.isActionTypeEnabled.mockReturnValue(false);
  mockedLicenseState.isLicenseValidForActionType.mockReturnValue({ isValid: true });
 
  expect(
- actionTypeRegistry.isActionExecutable('system-connector-.cases', 'system-action-type')
- ).toEqual(true);
+ actionTypeRegistry.isActionExecutable(
+ 'system-connector-test.system-action',
+ 'system-action-type'
+ )
+ ).toEqual(false);
  });
 
  test('should call isLicenseValidForActionType of the license state with notifyUsage false by default', async () => {
@@ -662,7 +665,7 @@ describe('actionTypeRegistry', () => {
  const registry = new ActionTypeRegistry(actionTypeRegistryParams);
 
  registry.register({
- id: '.cases',
+ id: 'test.system-action',
  name: 'Cases',
  minimumLicenseRequired: 'platinum',
  supportedFeatureIds: ['alerting'],
@@ -675,7 +678,7 @@ describe('actionTypeRegistry', () => {
  executor,
  });
 
- const result = registry.isSystemActionType('.cases');
+ const result = registry.isSystemActionType('test.system-action');
  expect(result).toBe(true);
  });
 
@@ -720,7 +723,7 @@ describe('actionTypeRegistry', () => {
  const registry = new ActionTypeRegistry(actionTypeRegistryParams);
 
  registry.register({
- id: '.cases',
+ id: 'test.system-action',
  name: 'Cases',
  minimumLicenseRequired: 'platinum',
  supportedFeatureIds: ['alerting'],
@@ -734,15 +737,15 @@ describe('actionTypeRegistry', () => {
  executor,
  });
 
- const result = registry.getSystemActionKibanaPrivileges('.cases');
+ const result = registry.getSystemActionKibanaPrivileges('test.system-action');
  expect(result).toEqual(['test/create']);
  });
 
  it('should return an empty array if the system action does not define any kibana privileges', () => {
  const registry = new ActionTypeRegistry(actionTypeRegistryParams);
 
  registry.register({
- id: '.cases',
+ id: 'test.system-action',
  name: 'Cases',
  minimumLicenseRequired: 'platinum',
  supportedFeatureIds: ['alerting'],
@@ -755,7 +758,7 @@ describe('actionTypeRegistry', () => {
  executor,
  });
 
- const result = registry.getSystemActionKibanaPrivileges('.cases');
+ const result = registry.getSystemActionKibanaPrivileges('test.system-action');
  expect(result).toEqual([]);
  });
 
@@ -784,7 +787,7 @@ describe('actionTypeRegistry', () => {
  const getKibanaPrivileges = jest.fn().mockReturnValue(['test/create']);
 
  registry.register({
- id: '.cases',
+ id: 'test.system-action',
  name: 'Cases',
  minimumLicenseRequired: 'platinum',
  supportedFeatureIds: ['alerting'],
@@ -798,7 +801,7 @@ describe('actionTypeRegistry', () => {
  executor,
  });
 
- registry.getSystemActionKibanaPrivileges('.cases', { foo: 'bar' });
+ registry.getSystemActionKibanaPrivileges('test.system-action', { foo: 'bar' });
  expect(getKibanaPrivileges).toHaveBeenCalledWith({ params: { foo: 'bar' } });
  });
  });

diff --git a/x-pack/plugins/actions/server/action_type_registry.ts b/x-pack/plugins/actions/server/action_type_registry.ts
@@ -79,20 +79,21 @@ export class ActionTypeRegistry {
  }
 
  /**
- * Returns true if action type is enabled or it is an in memory action type.
+ * Returns true if action type is enabled or preconfigured.
+ * An action type can be disabled but used with a preconfigured action.
+ * This does not apply to system actions as those can be disabled.
  */
  public isActionExecutable(
  actionId: string,
  actionTypeId: string,
  options: { notifyUsage: boolean } = { notifyUsage: false }
  ) {
  const actionTypeEnabled = this.isActionTypeEnabled(actionTypeId, options);
- return (
- actionTypeEnabled ||
- (!actionTypeEnabled &&
- this.inMemoryConnectors.find((inMemoryConnector) => inMemoryConnector.id === actionId) !==
- undefined)
+ const inMemoryConnector = this.inMemoryConnectors.find(
+ (connector) => connector.id === actionId
  );
+
+ return actionTypeEnabled || (!actionTypeEnabled && inMemoryConnector?.isPreconfigured === true);
  }
 
  /**

diff --git a/x-pack/plugins/actions/server/actions_client.test.ts b/x-pack/plugins/actions/server/actions_client.test.ts
@@ -1659,7 +1659,7 @@ describe('getBulk()', () => {
  connectorTokenClient: connectorTokenClientMock.create(),
  getEventLogClient,
  });
- return actionsClient.getBulk(['1', 'testPreconfigured']);
+ return actionsClient.getBulk({ ids: ['1', 'testPreconfigured'] });
  }
 
  test('ensures user is authorised to get the type of action', async () => {
@@ -1709,7 +1709,7 @@ describe('getBulk()', () => {
  }
  );
 
- await actionsClient.getBulk(['1']);
+ await actionsClient.getBulk({ ids: ['1'] });
 
  expect(auditLogger.log).toHaveBeenCalledWith(
  expect.objectContaining({
@@ -1725,7 +1725,7 @@ describe('getBulk()', () => {
  test('logs audit event when not authorised to bulk get connectors', async () => {
  authorization.ensureAuthorized.mockRejectedValue(new Error('Unauthorized'));
 
- await expect(actionsClient.getBulk(['1'])).rejects.toThrow();
+ await expect(actionsClient.getBulk({ ids: ['1'] })).rejects.toThrow();
 
  expect(auditLogger.log).toHaveBeenCalledWith(
  expect.objectContaining({
@@ -1810,7 +1810,7 @@ describe('getBulk()', () => {
  getEventLogClient,
  });
 
- const result = await actionsClient.getBulk(['1', 'testPreconfigured']);
+ const result = await actionsClient.getBulk({ ids: ['1', 'testPreconfigured'] });
 
  expect(result).toEqual([
  {
@@ -1907,7 +1907,7 @@ describe('getBulk()', () => {
  });
 
  await expect(
- actionsClient.getBulk(['1', 'testPreconfigured', 'system-connector-.cases'])
+ actionsClient.getBulk({ ids: ['1', 'testPreconfigured', 'system-connector-.cases'] })
  ).rejects.toThrowErrorMatchingInlineSnapshot(`"Connector system-connector-.cases not found"`);
  });
 
@@ -1982,7 +1982,10 @@ describe('getBulk()', () => {
  });
 
  expect(
- await actionsClient.getBulk(['1', 'testPreconfigured', 'system-connector-.cases'], false)
+ await actionsClient.getBulk({
+ ids: ['1', 'testPreconfigured', 'system-connector-.cases'],
+ throwIfSystemAction: false,
+ })
  ).toEqual([
  {
  actionTypeId: '.slack',

diff --git a/x-pack/plugins/actions/server/actions_client.ts b/x-pack/plugins/actions/server/actions_client.ts
@@ -534,10 +534,13 @@ export class ActionsClient {
  /**
  * Get bulk actions with in-memory list
  */
- public async getBulk(
- ids: string[],
- throwIfSystemAction: boolean = true
- ): Promise<ActionResult[]> {
+ public async getBulk({
+ ids,
+ throwIfSystemAction = true,
+ }: {
+ ids: string[];
+ throwIfSystemAction?: boolean;
+ }): Promise<ActionResult[]> {
  try {
  await this.authorization.ensureAuthorized({ operation: 'get' });
  } catch (error) {