Remove MLs use of internal Spaces utilities (#46366) (#46502)

* remove MLs use of internal Spaces utilities * move logic to SpacesService for testability * make TS happy * Apply suggestions from code review Co-Authored-By: Brandon Kobel <brandon.kobel@gmail.com>
elastic · Sep 24, 2019 · 597602d · 597602d
1 parent 8b4f179
commit 597602d
Show file tree

Hide file tree

Showing 7 changed files with 118 additions and 46 deletions.
diff --git a/x-pack/legacy/plugins/ml/server/lib/spaces_utils.ts b/x-pack/legacy/plugins/ml/server/lib/spaces_utils.ts
@@ -5,26 +5,20 @@
  */
 
 import { Request } from 'hapi';
-import { KibanaConfig } from 'src/legacy/server/kbn_server';
-import { getActiveSpace } from '../../../spaces/server/lib/get_active_space';
+import { SpacesPlugin } from '../../../spaces';
 import { Space } from '../../../spaces/common/model/space';
 
 interface GetActiveSpaceResponse {
   valid: boolean;
   space?: Space;
 }
 
-export function spacesUtilsProvider(spacesPlugin: any, request: Request, config: KibanaConfig) {
+export function spacesUtilsProvider(spacesPlugin: SpacesPlugin, request: Request) {
   async function activeSpace(): Promise<GetActiveSpaceResponse> {
-    const spacesClient = await spacesPlugin.getScopedSpacesClient(request);
     try {
       return {
         valid: true,
-        space: await getActiveSpace(
-          spacesClient,
-          request.getBasePath(),
-          config.get('server.basePath')
-        ),
+        space: await spacesPlugin.getActiveSpace(request),
       };
     } catch (e) {
       return {

diff --git a/x-pack/legacy/plugins/ml/server/routes/system.js b/x-pack/legacy/plugins/ml/server/routes/system.js
@@ -23,7 +23,6 @@ export function systemRoutes({
   elasticsearchPlugin,
   route,
   xpackMainPlugin,
-  config,
   spacesPlugin
 }) {
   const callWithInternalUser = callWithInternalUserFactory(elasticsearchPlugin);
@@ -101,7 +100,7 @@ export function systemRoutes({
         const ignoreSpaces = request.query && request.query.ignoreSpaces === 'true';
         // if spaces is disabled force isMlEnabledInSpace to be true
         const { isMlEnabledInSpace } = spacesPlugin !== undefined ?
-          spacesUtilsProvider(spacesPlugin, request, config) :
+          spacesUtilsProvider(spacesPlugin, request) :
           { isMlEnabledInSpace: async () => true };
 
         const { getPrivileges } = privilegesProvider(callWithRequest, xpackMainPlugin, isMlEnabledInSpace, ignoreSpaces);

diff --git a/x-pack/legacy/plugins/security/server/lib/authorization/check_privileges_dynamically.test.ts b/x-pack/legacy/plugins/security/server/lib/authorization/check_privileges_dynamically.test.ts
@@ -22,6 +22,7 @@ test(`checkPrivileges.atSpace when spaces is enabled`, async () => {
     namespaceToSpaceId: jest.fn(),
     getBasePath: jest.fn(),
     getScopedSpacesClient: jest.fn(),
+    getActiveSpace: jest.fn(),
   } as OptionalPlugin<SpacesPlugin>;
   const request = Symbol();
   const privilegeOrPrivileges = ['foo', 'bar'];

diff --git a/x-pack/legacy/plugins/spaces/index.ts b/x-pack/legacy/plugins/spaces/index.ts
@@ -23,6 +23,7 @@ import { initSpaceSelectorView } from './server/routes/views';
 
 export interface SpacesPlugin {
   getSpaceId: SpacesServiceSetup['getSpaceId'];
+  getActiveSpace: SpacesServiceSetup['getActiveSpace'];
   spaceIdToNamespace: SpacesServiceSetup['spaceIdToNamespace'];
   namespaceToSpaceId: SpacesServiceSetup['namespaceToSpaceId'];
   getBasePath: SpacesServiceSetup['getBasePath'];
@@ -183,6 +184,7 @@ export const spaces = (kibana: Record<string, any>) =>
       initSpaceSelectorView(server);
 
       server.expose('getSpaceId', (request: any) => spacesService.getSpaceId(request));
+      server.expose('getActiveSpace', spacesService.getActiveSpace);
       server.expose('spaceIdToNamespace', spacesService.spaceIdToNamespace);
       server.expose('namespaceToSpaceId', spacesService.namespaceToSpaceId);
       server.expose('getBasePath', spacesService.getBasePath);

diff --git a/x-pack/legacy/plugins/spaces/server/new_platform/spaces_service/spaces_service.mock.ts b/x-pack/legacy/plugins/spaces/server/new_platform/spaces_service/spaces_service.mock.ts
@@ -17,6 +17,7 @@ const createSetupContractMock = (spaceId = DEFAULT_SPACE_ID) => {
     scopedClient: jest.fn().mockResolvedValue(spacesClientMock.create()),
     namespaceToSpaceId: jest.fn().mockImplementation(namespaceToSpaceId),
     spaceIdToNamespace: jest.fn().mockImplementation(spaceIdToNamespace),
+    getActiveSpace: jest.fn(),
   };
   return setupContract;
 };

diff --git a/x-pack/legacy/plugins/spaces/server/new_platform/spaces_service/spaces_service.test.ts b/x-pack/legacy/plugins/spaces/server/new_platform/spaces_service/spaces_service.test.ts
@@ -7,7 +7,7 @@ import * as Rx from 'rxjs';
 import { SpacesService } from './spaces_service';
 import { coreMock, elasticsearchServiceMock } from 'src/core/server/mocks';
 import { SpacesAuditLogger } from '../../lib/audit_logger';
-import { KibanaRequest, SavedObjectsService } from 'src/core/server';
+import { KibanaRequest, SavedObjectsService, SavedObjectsErrorHelpers } from 'src/core/server';
 import { DEFAULT_SPACE_ID } from '../../../common/constants';
 import { getSpaceIdFromPath } from '../../lib/spaces_url_parser';
 import { createOptionalPlugin } from '../../../../../server/lib/optional_plugin';
@@ -29,7 +29,30 @@ const createService = async (serverBasePath: string = '') => {
       serverBasePath,
     },
     savedObjects: ({
-      getSavedObjectsRepository: jest.fn().mockReturnValue(null),
+      getSavedObjectsRepository: jest.fn().mockReturnValue({
+        get: jest.fn().mockImplementation((type, id) => {
+          if (type === 'space' && id === 'foo') {
+            return Promise.resolve({
+              id: 'space:foo',
+              attributes: {
+                name: 'Foo Space',
+                disabledFeatures: [],
+              },
+            });
+          }
+          if (type === 'space' && id === 'default') {
+            return Promise.resolve({
+              id: 'space:default',
+              attributes: {
+                name: 'Default Space',
+                disabledFeatures: [],
+                _reserved: true,
+              },
+            });
+          }
+          throw SavedObjectsErrorHelpers.createGenericNotFoundError(type, id);
+        }),
+      }),
     } as unknown) as SavedObjectsService,
   } as LegacyAPI;
 
@@ -149,4 +172,46 @@ describe('SpacesService', () => {
       expect(spacesServiceSetup.namespaceToSpaceId('foo')).toEqual('foo');
     });
   });
+
+  describe('#getActiveSpace', () => {
+    it('returns the default space when in the default space', async () => {
+      const spacesServiceSetup = await createService();
+      const request = {
+        url: { path: 'app/kibana' },
+      } as KibanaRequest;
+
+      const activeSpace = await spacesServiceSetup.getActiveSpace(request);
+      expect(activeSpace).toEqual({
+        id: 'space:default',
+        name: 'Default Space',
+        disabledFeatures: [],
+        _reserved: true,
+      });
+    });
+
+    it('returns the space for the current (non-default) space', async () => {
+      const spacesServiceSetup = await createService();
+      const request = {
+        url: { path: '/s/foo/app/kibana' },
+      } as KibanaRequest;
+
+      const activeSpace = await spacesServiceSetup.getActiveSpace(request);
+      expect(activeSpace).toEqual({
+        id: 'space:foo',
+        name: 'Foo Space',
+        disabledFeatures: [],
+      });
+    });
+
+    it('propagates errors from the repository', async () => {
+      const spacesServiceSetup = await createService();
+      const request = {
+        url: { path: '/s/unknown-space/app/kibana' },
+      } as KibanaRequest;
+
+      expect(spacesServiceSetup.getActiveSpace(request)).rejects.toThrowErrorMatchingInlineSnapshot(
+        `"Saved object [space/unknown-space] not found"`
+      );
+    });
+  });
 });
diff --git a/x-pack/legacy/plugins/spaces/server/new_platform/spaces_service/spaces_service.ts b/x-pack/legacy/plugins/spaces/server/new_platform/spaces_service/spaces_service.ts
@@ -16,6 +16,7 @@ import { getSpaceIdFromPath, addSpaceIdToPath } from '../../lib/spaces_url_parse
 import { SpacesConfigType } from '../config';
 import { namespaceToSpaceId, spaceIdToNamespace } from '../../lib/utils/namespace';
 import { LegacyAPI } from '../plugin';
+import { Space } from '../../../common/model/space';
 
 type RequestFacade = KibanaRequest | Legacy.Request;
 
@@ -31,6 +32,8 @@ export interface SpacesServiceSetup {
   spaceIdToNamespace(spaceId: string): string | undefined;
 
   namespaceToSpaceId(namespace: string | undefined): string;
+
+  getActiveSpace(request: RequestFacade): Promise<Space>;
 }
 
 interface SpacesServiceDeps {
@@ -66,6 +69,41 @@ export class SpacesService {
       return spaceId;
     };
 
+    const getScopedClient = async (request: RequestFacade) => {
+      return combineLatest(elasticsearch.adminClient$, config$)
+        .pipe(
+          map(([clusterClient, config]) => {
+            const internalRepository = this.getLegacyAPI().savedObjects.getSavedObjectsRepository(
+              clusterClient.callAsInternalUser,
+              ['space']
+            );
+
+            const callCluster = clusterClient.asScoped(request).callAsCurrentUser;
+
+            const callWithRequestRepository = this.getLegacyAPI().savedObjects.getSavedObjectsRepository(
+              callCluster,
+              ['space']
+            );
+
+            const authorization = security.isEnabled ? security.authorization : null;
+
+            return new SpacesClient(
+              getSpacesAuditLogger(),
+              (message: string) => {
+                this.log.debug(message);
+              },
+              authorization,
+              callWithRequestRepository,
+              config,
+              internalRepository,
+              request
+            );
+          }),
+          take(1)
+        )
+        .toPromise();
+    };
+
     return {
       getSpaceId,
       getBasePath: (spaceId: string) => {
@@ -81,39 +119,11 @@ export class SpacesService {
       },
       spaceIdToNamespace,
       namespaceToSpaceId,
-      scopedClient: async (request: RequestFacade) => {
-        return combineLatest(elasticsearch.adminClient$, config$)
-          .pipe(
-            map(([clusterClient, config]) => {
-              const internalRepository = this.getLegacyAPI().savedObjects.getSavedObjectsRepository(
-                clusterClient.callAsInternalUser,
-                ['space']
-              );
-
-              const callCluster = clusterClient.asScoped(request).callAsCurrentUser;
-
-              const callWithRequestRepository = this.getLegacyAPI().savedObjects.getSavedObjectsRepository(
-                callCluster,
-                ['space']
-              );
-
-              const authorization = security.isEnabled ? security.authorization : null;
-
-              return new SpacesClient(
-                getSpacesAuditLogger(),
-                (message: string) => {
-                  this.log.debug(message);
-                },
-                authorization,
-                callWithRequestRepository,
-                config,
-                internalRepository,
-                request
-              );
-            }),
-            take(1)
-          )
-          .toPromise();
+      scopedClient: getScopedClient,
+      getActiveSpace: async (request: RequestFacade) => {
+        const spaceId = getSpaceId(request);
+        const spacesClient = await getScopedClient(request);
+        return spacesClient.get(spaceId);
       },
     };
   }