migrate getCurrentUser calls in reporting to core security service (#…

…186913) ## Summary Part of #186574 Background: This PR is an example of a plugin migrating away from depending on the Security plugin, which is a high-priority effort for the last release before 9.0. The Reporting plugin uses `authc.getCurrentUser` from the security plugin's start contract on the server side. This PR migrates `authc.getCurrentUser` from the security plugin start contract to the core security service. ### Checklist - [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 --------- Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Tim Sullivan <[email protected]>
elastic · Jun 28, 2024 · bebb273 · bebb273
1 parent 64a01ef
commit bebb273
Show file tree

Hide file tree

Showing 13 changed files with 34 additions and 34 deletions.
diff --git a/x-pack/plugins/reporting/server/core.ts b/x-pack/plugins/reporting/server/core.ts
@@ -8,7 +8,7 @@
 import * as Rx from 'rxjs';
 import { map, take } from 'rxjs';
 
-import {
+import type {
   AnalyticsServiceStart,
   CoreSetup,
   DocLinksServiceSetup,
@@ -19,6 +19,7 @@ import {
   PackageInfo,
   PluginInitializerContext,
   SavedObjectsServiceStart,
+  SecurityServiceStart,
   StatusServiceSetup,
   UiSettingsServiceStart,
 } from '@kbn/core/server';
@@ -38,7 +39,7 @@ import { PngExportType } from '@kbn/reporting-export-types-png';
 import type { ReportingConfigType } from '@kbn/reporting-server';
 import { ExportType } from '@kbn/reporting-server';
 import { ScreenshottingStart } from '@kbn/screenshotting-plugin/server';
-import type { SecurityPluginSetup, SecurityPluginStart } from '@kbn/security-plugin/server';
+import type { SecurityPluginSetup } from '@kbn/security-plugin/server';
 import { DEFAULT_SPACE_ID } from '@kbn/spaces-plugin/common';
 import type { SpacesPluginSetup } from '@kbn/spaces-plugin/server';
 import type {
@@ -82,7 +83,7 @@ export interface ReportingInternalStart {
   licensing: LicensingPluginStart;
   logger: Logger;
   screenshotting?: ScreenshottingStart;
-  security?: SecurityPluginStart;
+  securityService: SecurityServiceStart;
   taskManager: TaskManagerStartContract;
 }
 
@@ -214,7 +215,7 @@ export class ReportingCore {
    */
   private getExportTypes(): ExportType[] {
     const { csv, pdf, png } = this.config.export_types;
-    const exportTypes = [];
+    const exportTypes: ExportType[] = [];
 
     if (csv.enabled) {
       // NOTE: CsvSearchSourceExportType should be deprecated and replaced with V2 in the UI: https://github.com/elastic/kibana/issues/151190

diff --git a/x-pack/plugins/reporting/server/plugin.ts b/x-pack/plugins/reporting/server/plugin.ts
@@ -117,6 +117,7 @@ export class ReportingPlugin
         savedObjects,
         uiSettings,
         store,
+        securityService: core.security,
         ...plugins,
       });
 

diff --git a/x-pack/plugins/reporting/server/routes/common/authorized_user_pre_routing.test.ts b/x-pack/plugins/reporting/server/routes/common/authorized_user_pre_routing.test.ts
@@ -49,7 +49,7 @@ describe('authorized_user_pre_routing', function () {
 
     mockStartDeps = await createMockPluginStart(
       {
-        security: {
+        securityService: {
           authc: {
             getCurrentUser: () => ({ id: '123', roles: ['superuser'], username: 'Tom Riddle' }),
           },
@@ -97,7 +97,7 @@ describe('authorized_user_pre_routing', function () {
       security: { license: { isEnabled: () => true } },
     });
     mockStartDeps = await createMockPluginStart(
-      { security: { authc: { getCurrentUser: () => null } } },
+      { securityService: { authc: { getCurrentUser: () => null } } },
       mockReportingConfig
     );
     mockCore = await createMockReportingCore(mockReportingConfig, mockSetupDeps, mockStartDeps);
@@ -126,7 +126,7 @@ describe('authorized_user_pre_routing', function () {
     it(`should return with 403 when security is enabled but user doesn't have the allowed role`, async function () {
       mockStartDeps = await createMockPluginStart(
         {
-          security: {
+          securityService: {
             authc: {
               getCurrentUser: () => ({ id: '123', roles: ['peasant'], username: 'Tom Riddle' }),
             },
@@ -154,7 +154,7 @@ describe('authorized_user_pre_routing', function () {
     it('should return from handler when security is enabled and user has explicitly allowed role', async function () {
       mockStartDeps = await createMockPluginStart(
         {
-          security: {
+          securityService: {
             authc: {
               getCurrentUser: () => ({ username: 'friendlyuser', roles: ['reporting_user'] }),
             },
@@ -176,7 +176,7 @@ describe('authorized_user_pre_routing', function () {
     it('should return from handler when security is enabled and user has superuser role', async function () {
       mockStartDeps = await createMockPluginStart(
         {
-          security: {
+          securityService: {
             authc: { getCurrentUser: () => ({ username: 'friendlyuser', roles: ['superuser'] }) },
           },
         },

diff --git a/x-pack/plugins/reporting/server/routes/common/authorized_user_pre_routing.ts b/x-pack/plugins/reporting/server/routes/common/authorized_user_pre_routing.ts
@@ -30,15 +30,15 @@ export const authorizedUserPreRouting = <P, Q, B>(
   reporting: ReportingCore,
   handler: RequestHandlerUser<P, Q, B>
 ): RequestHandler<P, Q, B, ReportingRequestHandlerContext, RouteMethod> => {
-  const { logger, security, docLinks } = reporting.getPluginSetupDeps();
+  const { logger, security: securitySetup, docLinks } = reporting.getPluginSetupDeps(); // ReportingInternalSetup.security?: SecurityPluginSetup | undefined
 
   return async (context, req, res) => {
-    const { security: securityStart } = await reporting.getPluginStartDeps();
+    const { securityService } = await reporting.getPluginStartDeps();
     try {
       let user: ReportingRequestUser = false;
-      if (security && security.license.isEnabled()) {
-        // find the authenticated user, or null if security is not enabled
-        user = getUser(req, securityStart);
+      if (securitySetup && securitySetup.license.isEnabled()) {
+        // find the authenticated user, only if license is enabled
+        user = getUser(req, securityService);
         if (!user) {
           // security is enabled but the user is null
           return res.unauthorized({ body: `Sorry, you aren't authenticated` });

diff --git a/x-pack/plugins/reporting/server/routes/common/get_user.ts b/x-pack/plugins/reporting/server/routes/common/get_user.ts
@@ -5,9 +5,8 @@
  * 2.0.
  */
 
-import { KibanaRequest } from '@kbn/core/server';
-import { SecurityPluginStart } from '@kbn/security-plugin/server';
+import { KibanaRequest, SecurityServiceStart } from '@kbn/core/server';
 
-export function getUser(request: KibanaRequest, security?: SecurityPluginStart) {
-  return security?.authc.getCurrentUser(request) ?? false;
+export function getUser(request: KibanaRequest, securityService: SecurityServiceStart) {
+  return securityService.authc.getCurrentUser(request) ?? false;
 }
diff --git a/x-pack/plugins/reporting/server/routes/common/jobs/job_management_pre_routing.test.ts b/x-pack/plugins/reporting/server/routes/common/jobs/job_management_pre_routing.test.ts
@@ -39,7 +39,7 @@ beforeEach(async () => {
 
   mockStartDeps = await createMockPluginStart(
     {
-      security: {
+      securityService: {
         authc: {
           getCurrentUser: () => ({ id: '123', roles: ['superuser'], username: 'Tom Riddle' }),
         },

diff --git a/...rting/server/routes/internal/generate/integration_tests/generation_from_jobparams.test.ts b/...rting/server/routes/internal/generate/integration_tests/generation_from_jobparams.test.ts
@@ -77,7 +77,7 @@ describe(`POST ${INTERNAL_ROUTES.GENERATE_PREFIX}`, () => {
           ...licensingMock.createStart(),
           license$: new BehaviorSubject({ isActive: true, isAvailable: true, type: 'gold' }),
         },
-        security: {
+        securityService: {
           authc: {
             getCurrentUser: () => ({ id: '123', roles: ['superuser'], username: 'Tom Riddle' }),
           },

diff --git a/x-pack/plugins/reporting/server/routes/internal/management/integration_tests/jobs.test.ts b/x-pack/plugins/reporting/server/routes/internal/management/integration_tests/jobs.test.ts
@@ -98,7 +98,7 @@ describe(`Reporting Job Management Routes: Internal`, () => {
           ...licensingMock.createStart(),
           license$: new BehaviorSubject({ isActive: true, isAvailable: true, type: 'gold' }),
         },
-        security: {
+        securityService: {
           authc: {
             getCurrentUser: () => ({ id: '123', roles: ['superuser'], username: 'Tom Riddle' }),
           },
@@ -175,7 +175,7 @@ describe(`Reporting Job Management Routes: Internal`, () => {
             ...licensingMock.createStart(),
             license$: new BehaviorSubject({ isActive: true, isAvailable: true, type: 'gold' }),
           },
-          security: { authc: { getCurrentUser: () => undefined } },
+          securityService: { authc: { getCurrentUser: () => undefined } }, // security comes from core here
         },
         mockConfigSchema
       );
@@ -389,7 +389,7 @@ describe(`Reporting Job Management Routes: Internal`, () => {
             ...licensingMock.createStart(),
             license$: new BehaviorSubject({ isActive: true, isAvailable: true, type: 'gold' }),
           },
-          security: {
+          securityService: {
             authc: {
               getCurrentUser: () => ({ id: '123', roles: ['peasant'], username: 'Tom Riddle' }),
             },

diff --git a/...lugins/reporting/server/routes/public/integration_tests/generation_from_jobparams.test.ts b/...lugins/reporting/server/routes/public/integration_tests/generation_from_jobparams.test.ts
@@ -76,7 +76,7 @@ describe(`POST ${PUBLIC_ROUTES.GENERATE_PREFIX}`, () => {
           ...licensingMock.createStart(),
           license$: new BehaviorSubject({ isActive: true, isAvailable: true, type: 'gold' }),
         },
-        security: {
+        securityService: {
           authc: {
             getCurrentUser: () => ({ id: '123', roles: ['superuser'], username: 'Tom Riddle' }),
           },

diff --git a/x-pack/plugins/reporting/server/routes/public/integration_tests/jobs.test.ts b/x-pack/plugins/reporting/server/routes/public/integration_tests/jobs.test.ts
@@ -95,7 +95,7 @@ describe(`Reporting Job Management Routes: Public`, () => {
           ...licensingMock.createStart(),
           license$: new BehaviorSubject({ isActive: true, isAvailable: true, type: 'gold' }),
         },
-        security: {
+        securityService: {
           authc: {
             getCurrentUser: () => ({ id: '123', roles: ['superuser'], username: 'Tom Riddle' }),
           },
@@ -165,7 +165,7 @@ describe(`Reporting Job Management Routes: Public`, () => {
             ...licensingMock.createStart(),
             license$: new BehaviorSubject({ isActive: true, isAvailable: true, type: 'gold' }),
           },
-          security: { authc: { getCurrentUser: () => undefined } },
+          securityService: { authc: { getCurrentUser: () => undefined } },
         },
         mockConfigSchema
       );

diff --git a/x-pack/plugins/reporting/server/test_helpers/create_mock_reportingplugin.ts b/x-pack/plugins/reporting/server/test_helpers/create_mock_reportingplugin.ts
@@ -31,7 +31,7 @@ import { securityMock } from '@kbn/security-plugin/server/mocks';
 import { taskManagerMock } from '@kbn/task-manager-plugin/server/mocks';
 import { ReportingCore } from '..';
 
-import { ReportingInternalSetup, ReportingInternalStart } from '../core';
+import type { ReportingInternalSetup, ReportingInternalStart } from '../core';
 import { ReportingStore } from '../lib';
 
 export const createMockPluginSetup = (
@@ -51,6 +51,7 @@ export const createMockPluginSetup = (
 };
 
 const coreSetupMock = coreMock.createSetup();
+const coreStartMock = coreMock.createStart();
 const logger = loggingSystemMock.createLogger();
 
 const createMockReportingStore = async (config: ReportingConfigType) => {
@@ -81,9 +82,10 @@ export const createMockPluginStart = async (
       ...licensingMock.createStart(),
       license$: new BehaviorSubject({ isAvailable: true, isActive: true, type: 'basic' }),
     },
+    securityService: coreStartMock.security, // we need authc from core.security start
     logger,
     screenshotting: createMockScreenshottingStart(),
-    ...startMock,
+    ...startMock, // allows to override with test instances
   };
 };
 

diff --git a/x-pack/plugins/reporting/server/types.ts b/x-pack/plugins/reporting/server/types.ts
@@ -21,11 +21,7 @@ import type {
   PngScreenshotOptions as BasePngScreenshotOptions,
   ScreenshottingStart,
 } from '@kbn/screenshotting-plugin/server';
-import type {
-  AuthenticatedUser,
-  SecurityPluginSetup,
-  SecurityPluginStart,
-} from '@kbn/security-plugin/server';
+import type { SecurityPluginSetup } from '@kbn/security-plugin/server';
 import type { SpacesPluginSetup } from '@kbn/spaces-plugin/server';
 import type {
   TaskManagerSetupContract,
@@ -34,6 +30,7 @@ import type {
 import type { UsageCollectionSetup } from '@kbn/usage-collection-plugin/server';
 
 import { ExportTypesRegistry } from '@kbn/reporting-server/export_types_registry';
+import type { AuthenticatedUser } from '@kbn/core-security-common';
 
 /**
  * Plugin Setup Contract
@@ -70,7 +67,6 @@ export interface ReportingStartDeps {
   licensing: LicensingPluginStart;
   taskManager: TaskManagerStartContract;
   screenshotting?: ScreenshottingStart;
-  security?: SecurityPluginStart;
 }
 
 export type ReportingRequestHandlerContext = CustomRequestHandlerContext<{

diff --git a/x-pack/plugins/reporting/tsconfig.json b/x-pack/plugins/reporting/tsconfig.json
@@ -50,6 +50,7 @@
     "@kbn/reporting-csv-share-panel",
     "@kbn/react-kibana-context-render",
     "@kbn/react-kibana-mount",
+    "@kbn/core-security-common",
   ],
   "exclude": [
     "target/**/*",