[Actions] System actions enhancements (#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: #160983, #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 <[email protected]>
elastic · Jul 28, 2023 · f8a1600 · f8a1600
1 parent 971c6bd
commit f8a1600
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) {