Capability resolver: minor optimizations (#152784)

## Summary Related to #146881 I tried to fix the issue, but couldn't so I kept the tiny optimizations I made on the way: - optimize reduce blocks to re-use the memo instead of spreading into a new object - update most of the existing resolver to only return the list of changes rather than the whole capability objects (which was how it was supposed to work) - minor perf improvement when merging the result of the resolvers
elastic · Mar 9, 2023 · e058c06 · e058c06
1 parent ef918af
commit e058c06
Show file tree

Hide file tree

Showing 8 changed files with 33 additions and 69 deletions.
diff --git a/packages/core/capabilities/core-capabilities-server-internal/src/merge_capabilities.ts b/packages/core/capabilities/core-capabilities-server-internal/src/merge_capabilities.ts
@@ -10,7 +10,7 @@ import { mergeWith } from 'lodash';
 import type { Capabilities } from '@kbn/core-capabilities-common';
 
 export const mergeCapabilities = (...sources: Array<Partial<Capabilities>>): Capabilities =>
-  mergeWith({}, ...sources, (a: any, b: any) => {
+  mergeWith({}, ...sources, (a: unknown, b: unknown) => {
     if (
       (typeof a === 'boolean' && typeof b === 'object') ||
       (typeof a === 'object' && typeof b === 'boolean')

diff --git a/packages/core/capabilities/core-capabilities-server-internal/src/resolve_capabilities.ts b/packages/core/capabilities/core-capabilities-server-internal/src/resolve_capabilities.ts
@@ -46,16 +46,14 @@ export const resolveCapabilities = async (
   applications: string[],
   useDefaultCapabilities: boolean
 ): Promise<Capabilities> => {
-  const mergedCaps = cloneDeep({
+  const mergedCaps: Capabilities = cloneDeep({
     ...capabilities,
-    navLinks: applications.reduce(
-      (acc, app) => ({
-        ...acc,
-        [app]: true,
-      }),
-      capabilities.navLinks
-    ),
+    navLinks: applications.reduce((acc, app) => {
+      acc[app] = true;
+      return acc;
+    }, capabilities.navLinks),
   });
+
   return switchers.reduce(async (caps, switcher) => {
     const resolvedCaps = await caps;
     const changes = await switcher(request, resolvedCaps, useDefaultCapabilities);
@@ -79,11 +77,8 @@ function recursiveApplyChanges<
       }
       return [key, typeof orig === typeof changed ? changed : orig];
     })
-    .reduce(
-      (acc, [key, value]) => ({
-        ...acc,
-        [key]: value,
-      }),
-      {} as TDestination
-    );
+    .reduce((acc, [key, value]) => {
+      acc[key as keyof TDestination] = value;
+      return acc;
+    }, {} as TDestination);
 }
diff --git a/x-pack/plugins/file_upload/server/capabilities.test.ts b/x-pack/plugins/file_upload/server/capabilities.test.ts
@@ -44,16 +44,9 @@ describe('setupCapabilities', () => {
 
     const request = httpServerMock.createKibanaRequest();
 
-    await expect(switcher(request, capabilities, false)).resolves.toMatchInlineSnapshot(`
-            Object {
-              "catalogue": Object {},
-              "fileUpload": Object {
-                "show": true,
-              },
-              "management": Object {},
-              "navLinks": Object {},
-            }
-          `);
+    await expect(switcher(request, capabilities, false)).resolves.toMatchInlineSnapshot(
+      `Object {}`
+    );
   });
 
   it('registers a capabilities switcher that returns unaltered capabilities when default capabilities are requested', async () => {
@@ -81,16 +74,7 @@ describe('setupCapabilities', () => {
 
     const request = httpServerMock.createKibanaRequest();
 
-    await expect(switcher(request, capabilities, true)).resolves.toMatchInlineSnapshot(`
-            Object {
-              "catalogue": Object {},
-              "fileUpload": Object {
-                "show": true,
-              },
-              "management": Object {},
-              "navLinks": Object {},
-            }
-          `);
+    await expect(switcher(request, capabilities, true)).resolves.toMatchInlineSnapshot(`Object {}`);
 
     expect(security.authz.mode.useRbacForRequest).not.toHaveBeenCalled();
     expect(security.authz.checkPrivilegesDynamicallyWithRequest).not.toHaveBeenCalled();
@@ -166,16 +150,9 @@ describe('setupCapabilities', () => {
 
     const request = httpServerMock.createKibanaRequest();
 
-    await expect(switcher(request, capabilities, false)).resolves.toMatchInlineSnapshot(`
-            Object {
-              "catalogue": Object {},
-              "fileUpload": Object {
-                "show": true,
-              },
-              "management": Object {},
-              "navLinks": Object {},
-            }
-          `);
+    await expect(switcher(request, capabilities, false)).resolves.toMatchInlineSnapshot(
+      `Object {}`
+    );
 
     expect(security.authz.mode.useRbacForRequest).toHaveBeenCalledTimes(1);
     expect(security.authz.mode.useRbacForRequest).toHaveBeenCalledWith(request);
@@ -249,16 +226,9 @@ describe('setupCapabilities', () => {
 
     const request = httpServerMock.createKibanaRequest();
 
-    await expect(switcher(request, capabilities, false)).resolves.toMatchInlineSnapshot(`
-            Object {
-              "catalogue": Object {},
-              "fileUpload": Object {
-                "show": true,
-              },
-              "management": Object {},
-              "navLinks": Object {},
-            }
-          `);
+    await expect(switcher(request, capabilities, false)).resolves.toMatchInlineSnapshot(
+      `Object {}`
+    );
 
     expect(security.authz.mode.useRbacForRequest).toHaveBeenCalledTimes(1);
     expect(security.authz.mode.useRbacForRequest).toHaveBeenCalledWith(request);

diff --git a/x-pack/plugins/file_upload/server/capabilities.ts b/x-pack/plugins/file_upload/server/capabilities.ts
@@ -22,7 +22,7 @@ export const setupCapabilities = (
 
   core.capabilities.registerSwitcher(async (request, capabilities, useDefaultCapabilities) => {
     if (useDefaultCapabilities) {
-      return capabilities;
+      return {};
     }
     const [, { security }] = await core.getStartServices();
 
@@ -42,6 +42,6 @@ export const setupCapabilities = (
       };
     }
 
-    return capabilities;
+    return {};
   });
 };
diff --git a/x-pack/plugins/ml/server/lib/capabilities/capabilities_switcher.ts b/x-pack/plugins/ml/server/lib/capabilities/capabilities_switcher.ts
@@ -23,9 +23,8 @@ export const setupCapabilitiesSwitcher = (
 function getSwitcher(license$: Observable<ILicense>, logger: Logger): CapabilitiesSwitcher {
   return async (request, capabilities) => {
     const isAnonymousRequest = !request.route.options.authRequired;
-
     if (isAnonymousRequest) {
-      return capabilities;
+      return {};
     }
 
     try {
@@ -34,11 +33,11 @@ function getSwitcher(license$: Observable<ILicense>, logger: Logger): Capabiliti
 
       // full license, leave capabilities as they were
       if (mlEnabled && isFullLicense(license)) {
-        return capabilities;
+        return {};
       }
 
-      const mlCaps = capabilities.ml as MlCapabilities;
-      const originalCapabilities = cloneDeep(mlCaps);
+      const originalCapabilities = capabilities.ml as MlCapabilities;
+      const mlCaps = cloneDeep(originalCapabilities);
 
       // not full licence, switch off all capabilities
       Object.keys(mlCaps).forEach((k) => {
@@ -50,10 +49,10 @@ function getSwitcher(license$: Observable<ILicense>, logger: Logger): Capabiliti
         basicLicenseMlCapabilities.forEach((c) => (mlCaps[c] = originalCapabilities[c]));
       }
 
-      return capabilities;
+      return { ml: mlCaps };
     } catch (e) {
       logger.debug(`Error updating capabilities for ML based on licensing: ${e}`);
-      return capabilities;
+      return {};
     }
   };
 }
diff --git a/x-pack/plugins/security/server/authorization/authorization_service.tsx b/x-pack/plugins/security/server/authorization/authorization_service.tsx
@@ -156,7 +156,7 @@ export class AuthorizationService {
         // If we have a license which doesn't enable security, or we're a legacy user we shouldn't
         // disable any ui capabilities
         if (!mode.useRbacForRequest(request)) {
-          return uiCapabilities;
+          return {};
         }
 
         const disableUICapabilities = disableUICapabilitiesFactory(

diff --git a/x-pack/plugins/spaces/server/capabilities/capabilities_switcher.test.ts b/x-pack/plugins/spaces/server/capabilities/capabilities_switcher.test.ts
@@ -173,7 +173,7 @@ describe('capabilitiesSwitcher', () => {
 
     const result = await switcher(request, capabilities, false);
 
-    expect(result).toEqual(buildCapabilities());
+    expect(result).toEqual({});
     expect(spacesService.getActiveSpace).not.toHaveBeenCalled();
   });
 
@@ -191,7 +191,7 @@ describe('capabilitiesSwitcher', () => {
 
     const result = await switcher(request, capabilities, true);
 
-    expect(result).toEqual(buildCapabilities());
+    expect(result).toEqual({});
     expect(spacesService.getActiveSpace).not.toHaveBeenCalled();
   });
 

diff --git a/x-pack/plugins/spaces/server/capabilities/capabilities_switcher.ts b/x-pack/plugins/spaces/server/capabilities/capabilities_switcher.ts
@@ -24,7 +24,7 @@ export function setupCapabilitiesSwitcher(
     const shouldNotToggleCapabilities = isAuthRequiredOrOptional || useDefaultCapabilities;
 
     if (shouldNotToggleCapabilities) {
-      return capabilities;
+      return {};
     }
 
     try {