[Cases] Make metrics routes internal (#162506)

Fixes #162406 ## Summary - Made `getCaseMetrics` and `getCasesMetrics` internal APIs. - There was no documentation to begin with so there was none to remove. - The allowed/available `features` now come from the `CaseMetricsFeature` _enum_ instead of being hardcoded everywhere. - We now **also** check for the values passed to the `features` field in case metrics requests using io-ts. - There was already some validation before which I decided to leave. When doing `buildHandlers` the function `checkAndThrowIfInvalidFeatures` is always called. Right now, this is only used by `getCasesMetrics` and `getCaseMetrics` where we already decode the params so that validation is redundant, but if it starts being used somewhere else it will be nice to have this extra security guarantee. ### Release Notes The get case metrics APIs are now internal. --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
elastic · Aug 1, 2023 · 6085444 · 6085444
1 parent b10cde1
commit 6085444
Show file tree

Hide file tree

Showing 52 changed files with 366 additions and 207 deletions.
diff --git a/x-pack/plugins/cases/common/api/helpers.ts b/x-pack/plugins/cases/common/api/helpers.ts
@@ -7,7 +7,7 @@
 
 import {
   CASE_DETAILS_URL,
-  CASE_METRICS_DETAILS_URL,
+  INTERNAL_CASE_METRICS_DETAILS_URL,
   CASE_COMMENTS_URL,
   CASE_USER_ACTIONS_URL,
   CASE_PUSH_URL,
@@ -28,7 +28,7 @@ export const getCaseDetailsUrl = (id: string): string => {
 };
 
 export const getCaseDetailsMetricsUrl = (id: string): string => {
-  return CASE_METRICS_DETAILS_URL.replace('{case_id}', id);
+  return INTERNAL_CASE_METRICS_DETAILS_URL.replace('{case_id}', id);
 };
 
 export const getCaseCommentsUrl = (id: string): string => {

diff --git a/x-pack/plugins/cases/common/api/metrics/case.test.ts b/x-pack/plugins/cases/common/api/metrics/case.test.ts
@@ -5,17 +5,19 @@
  * 2.0.
  */
 
+import { PathReporter } from 'io-ts/lib/PathReporter';
 import {
   SingleCaseMetricsRequestRt,
   CasesMetricsRequestRt,
   SingleCaseMetricsResponseRt,
   CasesMetricsResponseRt,
+  CaseMetricsFeature,
 } from './case';
 
 describe('Metrics case', () => {
   describe('SingleCaseMetricsRequestRt', () => {
     const defaultRequest = {
-      features: ['alerts.count', 'lifespan'],
+      features: [CaseMetricsFeature.ALERTS_COUNT, CaseMetricsFeature.LIFESPAN],
     };
 
     it('has expected attributes in request', () => {
@@ -38,10 +40,27 @@ describe('Metrics case', () => {
         right: defaultRequest,
       });
     });
+
+    describe('errors', () => {
+      it('has invalid feature in request', () => {
+        expect(
+          PathReporter.report(
+            SingleCaseMetricsRequestRt.decode({
+              features: [CaseMetricsFeature.MTTR],
+            })
+          )[0]
+        ).toContain('Invalid value "mttr" supplied');
+      });
+    });
   });
 
   describe('CasesMetricsRequestRt', () => {
-    const defaultRequest = { features: ['mttr'], to: 'now-1d', from: 'now-1d', owner: ['cases'] };
+    const defaultRequest = {
+      features: [CaseMetricsFeature.MTTR],
+      to: 'now-1d',
+      from: 'now-1d',
+      owner: ['cases'],
+    };
 
     it('has expected attributes in request', () => {
       const query = CasesMetricsRequestRt.decode(defaultRequest);
@@ -65,16 +84,31 @@ describe('Metrics case', () => {
     });
 
     it('removes foo:bar attributes from when partial fields', () => {
-      const query = CasesMetricsRequestRt.decode({ features: ['mttr'], to: 'now-1d', foo: 'bar' });
+      const query = CasesMetricsRequestRt.decode({
+        features: [CaseMetricsFeature.MTTR],
+        to: 'now-1d',
+        foo: 'bar',
+      });
 
       expect(query).toStrictEqual({
         _tag: 'Right',
         right: {
-          features: ['mttr'],
+          features: [CaseMetricsFeature.MTTR],
           to: 'now-1d',
         },
       });
     });
+    describe('errors', () => {
+      it('has invalid feature in request', () => {
+        expect(
+          PathReporter.report(
+            CasesMetricsRequestRt.decode({
+              features: ['foobar'],
+            })
+          )[0]
+        ).toContain('Invalid value "foobar" supplied');
+      });
+    });
   });
 
   describe('SingleCaseMetricsResponseRt', () => {

diff --git a/x-pack/plugins/cases/common/api/metrics/case.ts b/x-pack/plugins/cases/common/api/metrics/case.ts
@@ -15,6 +15,30 @@ export type AlertHostsMetrics = rt.TypeOf<typeof AlertHostsMetricsRt>;
 export type AlertUsersMetrics = rt.TypeOf<typeof AlertUsersMetricsRt>;
 export type StatusInfo = rt.TypeOf<typeof StatusInfoRt>;
 
+export enum CaseMetricsFeature {
+  ALERTS_COUNT = 'alerts.count',
+  ALERTS_USERS = 'alerts.users',
+  ALERTS_HOSTS = 'alerts.hosts',
+  ACTIONS_ISOLATE_HOST = 'actions.isolateHost',
+  CONNECTORS = 'connectors',
+  LIFESPAN = 'lifespan',
+  MTTR = 'mttr',
+}
+
+export const SingleCaseMetricsFeatureFieldRt = rt.union([
+  rt.literal(CaseMetricsFeature.ALERTS_COUNT),
+  rt.literal(CaseMetricsFeature.ALERTS_USERS),
+  rt.literal(CaseMetricsFeature.ALERTS_HOSTS),
+  rt.literal(CaseMetricsFeature.ACTIONS_ISOLATE_HOST),
+  rt.literal(CaseMetricsFeature.CONNECTORS),
+  rt.literal(CaseMetricsFeature.LIFESPAN),
+]);
+
+export const CasesMetricsFeatureFieldRt = rt.union([
+  SingleCaseMetricsFeatureFieldRt,
+  rt.literal(CaseMetricsFeature.MTTR),
+]);
+
 const StatusInfoRt = rt.strict({
   /**
    * Duration the case was in the open status in milliseconds
@@ -76,15 +100,15 @@ export const SingleCaseMetricsRequestRt = rt.strict({
   /**
    * The metrics to retrieve.
    */
-  features: rt.array(rt.string),
+  features: rt.array(SingleCaseMetricsFeatureFieldRt),
 });
 
 export const CasesMetricsRequestRt = rt.intersection([
   rt.strict({
     /**
      * The metrics to retrieve.
      */
-    features: rt.array(rt.string),
+    features: rt.array(CasesMetricsFeatureFieldRt),
   }),
   rt.exact(
     rt.partial({
@@ -188,3 +212,6 @@ export const CasesMetricsResponseRt = rt.exact(
     mttr: rt.union([rt.number, rt.null]),
   })
 );
+
+export type CasesMetricsFeatureField = rt.TypeOf<typeof CasesMetricsFeatureFieldRt>;
+export type SingleCaseMetricsFeatureField = rt.TypeOf<typeof SingleCaseMetricsFeatureFieldRt>;
diff --git a/x-pack/plugins/cases/common/constants/index.ts b/x-pack/plugins/cases/common/constants/index.ts
@@ -61,9 +61,6 @@ export const CASE_FIND_USER_ACTIONS_URL = `${CASE_USER_ACTIONS_URL}/_find` as co
 export const CASE_ALERTS_URL = `${CASES_URL}/alerts/{alert_id}` as const;
 export const CASE_DETAILS_ALERTS_URL = `${CASE_DETAILS_URL}/alerts` as const;
 
-export const CASE_METRICS_URL = `${CASES_URL}/metrics` as const;
-export const CASE_METRICS_DETAILS_URL = `${CASES_URL}/metrics/{case_id}` as const;
-
 /**
  * Internal routes
  */
@@ -83,6 +80,8 @@ export const INTERNAL_CASE_USERS_URL = `${CASES_INTERNAL_URL}/{case_id}/_users`
 export const INTERNAL_DELETE_FILE_ATTACHMENTS_URL =
   `${CASES_INTERNAL_URL}/{case_id}/attachments/files/_bulk_delete` as const;
 export const INTERNAL_GET_CASE_CATEGORIES_URL = `${CASES_INTERNAL_URL}/categories` as const;
+export const INTERNAL_CASE_METRICS_URL = `${CASES_INTERNAL_URL}/metrics` as const;
+export const INTERNAL_CASE_METRICS_DETAILS_URL = `${CASES_INTERNAL_URL}/metrics/{case_id}` as const;
 
 /**
  * Action routes

diff --git a/x-pack/plugins/cases/common/index.ts b/x-pack/plugins/cases/common/index.ts
@@ -58,3 +58,4 @@ export { getCasesFromAlertsUrl, getCaseFindUserActionsUrl, throwErrors } from '.
 export { StatusAll } from './ui/types';
 export { createUICapabilities } from './utils/capabilities';
 export { getApiTags } from './utils/api_tags';
+export { CaseMetricsFeature } from './api/metrics/case';
diff --git a/x-pack/plugins/cases/common/ui/types.ts b/x-pack/plugins/cases/common/ui/types.ts
@@ -12,7 +12,6 @@ import type {
   READ_CASES_CAPABILITY,
   UPDATE_CASES_CAPABILITY,
 } from '..';
-import type { SingleCaseMetricsResponse, CasesMetricsResponse } from '../api';
 import type { PUSH_CASES_CAPABILITY } from '../constants';
 import type { SnakeToCamelCase } from '../types';
 import type {
@@ -37,6 +36,7 @@ import type {
   UserActionFindRequestTypes,
   UserActionFindResponse,
 } from '../types/api';
+import type { CaseMetricsFeature, CasesMetricsResponse, SingleCaseMetricsResponse } from '../api';
 
 type DeepRequired<T> = { [K in keyof T]: DeepRequired<T[K]> } & Required<T>;
 
@@ -153,13 +153,7 @@ export interface FilterOptions {
 export type PartialFilterOptions = Partial<FilterOptions>;
 
 export type SingleCaseMetrics = SingleCaseMetricsResponse;
-export type SingleCaseMetricsFeature =
-  | 'alerts.count'
-  | 'alerts.users'
-  | 'alerts.hosts'
-  | 'actions.isolateHost'
-  | 'connectors'
-  | 'lifespan';
+export type SingleCaseMetricsFeature = Exclude<CaseMetricsFeature, CaseMetricsFeature.MTTR>;
 
 export enum SortFieldCase {
   closedAt = 'closedAt',

diff --git a/x-pack/plugins/cases/public/api/index.test.ts b/x-pack/plugins/cases/public/api/index.test.ts
@@ -5,6 +5,7 @@
  * 2.0.
  */
 
+import { CaseMetricsFeature } from '../../common/api/metrics/case';
 import { httpServiceMock } from '@kbn/core/public/mocks';
 import { bulkGetCases, getCases, getCasesMetrics } from '.';
 import { allCases, allCasesSnake, casesSnake } from '../containers/mock';
@@ -36,14 +37,17 @@ describe('api', () => {
 
     it('should return the correct response', async () => {
       expect(
-        await getCasesMetrics({ http, query: { features: ['mttr'], from: 'now-1d' } })
+        await getCasesMetrics({
+          http,
+          query: { features: [CaseMetricsFeature.MTTR], from: 'now-1d' },
+        })
       ).toEqual({ mttr: 0 });
     });
 
     it('should have been called with the correct path', async () => {
-      await getCasesMetrics({ http, query: { features: ['mttr'], to: 'now-1d' } });
-      expect(http.get).toHaveBeenCalledWith('/api/cases/metrics', {
-        query: { features: ['mttr'], to: 'now-1d' },
+      await getCasesMetrics({ http, query: { features: [CaseMetricsFeature.MTTR], to: 'now-1d' } });
+      expect(http.get).toHaveBeenCalledWith('/internal/cases/metrics', {
+        query: { features: [CaseMetricsFeature.MTTR], to: 'now-1d' },
       });
     });
   });

diff --git a/x-pack/plugins/cases/public/api/index.ts b/x-pack/plugins/cases/public/api/index.ts
@@ -17,7 +17,7 @@ import type {
 import type { CasesStatus, CasesMetrics, CasesFindResponseUI } from '../../common/ui';
 import {
   CASE_FIND_URL,
-  CASE_METRICS_URL,
+  INTERNAL_CASE_METRICS_URL,
   CASE_STATUS_URL,
   INTERNAL_BULK_GET_CASES_URL,
 } from '../../common/constants';
@@ -62,7 +62,7 @@ export const getCasesMetrics = async ({
   signal,
   query,
 }: HTTPService & { query: CasesMetricsRequest }): Promise<CasesMetrics> => {
-  const res = await http.get<CasesMetricsResponse>(CASE_METRICS_URL, { signal, query });
+  const res = await http.get<CasesMetricsResponse>(INTERNAL_CASE_METRICS_URL, { signal, query });
   return convertToCamelCase(decodeCasesMetricsResponse(res));
 };
 

diff --git a/x-pack/plugins/cases/public/client/api/index.test.ts b/x-pack/plugins/cases/public/client/api/index.test.ts
@@ -5,6 +5,7 @@
  * 2.0.
  */
 
+import { CaseMetricsFeature } from '../../../common/api/metrics/case';
 import { httpServiceMock } from '@kbn/core/public/mocks';
 import { createClientAPI } from '.';
 import { allCases, allCasesSnake, casesSnake } from '../../containers/mock';
@@ -68,15 +69,17 @@ describe('createClientAPI', () => {
       http.get.mockResolvedValue({ mttr: 0 });
 
       it('should return the correct response', async () => {
-        expect(await api.cases.getCasesMetrics({ features: ['mttr'], from: 'now-1d' })).toEqual({
+        expect(
+          await api.cases.getCasesMetrics({ features: [CaseMetricsFeature.MTTR], from: 'now-1d' })
+        ).toEqual({
           mttr: 0,
         });
       });
 
       it('should have been called with the correct path', async () => {
-        await api.cases.getCasesMetrics({ features: ['mttr'], from: 'now-1d' });
-        expect(http.get).toHaveBeenCalledWith('/api/cases/metrics', {
-          query: { features: ['mttr'], from: 'now-1d' },
+        await api.cases.getCasesMetrics({ features: [CaseMetricsFeature.MTTR], from: 'now-1d' });
+        expect(http.get).toHaveBeenCalledWith('/internal/cases/metrics', {
+          query: { features: [CaseMetricsFeature.MTTR], from: 'now-1d' },
         });
       });
     });

diff --git a/x-pack/plugins/cases/public/common/use_cases_features.test.tsx b/x-pack/plugins/cases/public/common/use_cases_features.test.tsx
@@ -14,6 +14,7 @@ import { useCasesFeatures } from './use_cases_features';
 import { TestProviders } from './mock/test_providers';
 import type { LicenseType } from '@kbn/licensing-plugin/common/types';
 import { LICENSE_TYPE } from '@kbn/licensing-plugin/common/types';
+import { CaseMetricsFeature } from '../../common/api/metrics/case';
 
 describe('useCasesFeatures', () => {
   // isAlertsEnabled, isSyncAlertsEnabled, alerts
@@ -53,14 +54,16 @@ describe('useCasesFeatures', () => {
   it('returns the metrics correctly', async () => {
     const { result } = renderHook<{}, UseCasesFeatures>(() => useCasesFeatures(), {
       wrapper: ({ children }) => (
-        <TestProviders features={{ metrics: ['connectors'] }}>{children}</TestProviders>
+        <TestProviders features={{ metrics: [CaseMetricsFeature.CONNECTORS] }}>
+          {children}
+        </TestProviders>
       ),
     });
 
     expect(result.current).toEqual({
       isAlertsEnabled: true,
       isSyncAlertsEnabled: true,
-      metricsFeatures: ['connectors'],
+      metricsFeatures: [CaseMetricsFeature.CONNECTORS],
       caseAssignmentAuthorized: false,
       pushToServiceAuthorized: false,
     });

diff --git a/x-pack/plugins/cases/public/components/case_action_bar/index.test.tsx b/x-pack/plugins/cases/public/components/case_action_bar/index.test.tsx
@@ -22,6 +22,7 @@ import {
 import { useGetCaseConnectors } from '../../containers/use_get_case_connectors';
 import { useRefreshCaseViewPage } from '../case_view/use_on_refresh_case_view_page';
 import { getCaseConnectorsMockResponse } from '../../common/mock/connectors';
+import { CaseMetricsFeature } from '../../../common/api/metrics/case';
 
 jest.mock('../../containers/use_get_case_connectors');
 jest.mock('../case_view/use_on_refresh_case_view_page');
@@ -152,7 +153,7 @@ describe('CaseActionBar', () => {
   it('should not show the Case open text when the lifespan feature is enabled', () => {
     const props: CaseActionBarProps = { ...defaultProps };
     const { queryByText } = render(
-      <TestProviders features={{ metrics: ['lifespan'] }}>
+      <TestProviders features={{ metrics: [CaseMetricsFeature.LIFESPAN] }}>
         <CaseActionBar {...props} />
       </TestProviders>
     );

diff --git a/x-pack/plugins/cases/public/components/case_action_bar/index.tsx b/x-pack/plugins/cases/public/components/case_action_bar/index.tsx
@@ -16,8 +16,9 @@ import {
   EuiFlexItem,
   EuiIconTip,
 } from '@elastic/eui';
-import type { CaseUI } from '../../../common/ui/types';
 import type { CaseStatuses } from '../../../common/types/domain';
+import type { CaseUI } from '../../../common/ui/types';
+import { CaseMetricsFeature } from '../../../common/api';
 import * as i18n from '../case_view/translations';
 import { Actions } from './actions';
 import { StatusContextMenu } from './status_context_menu';
@@ -100,7 +101,7 @@ const CaseActionBarComponent: React.FC<CaseActionBarProps> = ({
                 />
               </EuiDescriptionListDescription>
             </EuiFlexItem>
-            {!metricsFeatures.includes('lifespan') ? (
+            {!metricsFeatures.includes(CaseMetricsFeature.LIFESPAN) ? (
               <EuiFlexItem grow={false}>
                 <EuiDescriptionListTitle>{title}</EuiDescriptionListTitle>
                 <EuiDescriptionListDescription>

diff --git a/x-pack/plugins/cases/public/components/case_view/case_view_page.test.tsx b/x-pack/plugins/cases/public/components/case_view/case_view_page.test.tsx
@@ -41,6 +41,7 @@ import { useInfiniteFindCaseUserActions } from '../../containers/use_infinite_fi
 import { useGetCaseUserActionsStats } from '../../containers/use_get_case_user_actions_stats';
 import { createQueryWithMarkup } from '../../common/test_utils';
 import { useCasesFeatures } from '../../common/use_cases_features';
+import { CaseMetricsFeature } from '../../../common/api/metrics/case';
 
 jest.mock('../../containers/use_get_action_license');
 jest.mock('../../containers/use_update_case');
@@ -176,7 +177,7 @@ describe('CaseViewPage', () => {
     useGetTagsMock.mockReturnValue({ data: [], isLoading: false });
     useGetCaseUsersMock.mockReturnValue({ isLoading: false, data: caseUsers });
     useCasesFeaturesMock.mockReturnValue({
-      metricsFeatures: ['alerts.count'],
+      metricsFeatures: [CaseMetricsFeature.ALERTS_COUNT],
       pushToServiceAuthorized: true,
       caseAssignmentAuthorized: true,
       isAlertsEnabled: true,

diff --git a/x-pack/plugins/cases/public/components/case_view/components/case_view_activity.test.tsx b/x-pack/plugins/cases/public/components/case_view/components/case_view_activity.test.tsx
@@ -37,6 +37,7 @@ import { useInfiniteFindCaseUserActions } from '../../../containers/use_infinite
 import { useOnUpdateField } from '../use_on_update_field';
 import { useCasesFeatures } from '../../../common/use_cases_features';
 import { ConnectorTypes, UserActionTypes } from '../../../../common/types/domain';
+import { CaseMetricsFeature } from '../../../../common/api/metrics/case';
 
 jest.mock('../../../containers/use_infinite_find_case_user_actions');
 jest.mock('../../../containers/use_find_case_user_actions');
@@ -116,7 +117,7 @@ const caseProps = {
 
 const caseUsers = getCaseUsersMockResponse();
 const useGetCasesFeaturesRes = {
-  metricsFeatures: ['alerts.count'],
+  metricsFeatures: [CaseMetricsFeature.ALERTS_COUNT],
   pushToServiceAuthorized: true,
   caseAssignmentAuthorized: true,
   isAlertsEnabled: true,