[Synthetics] Remove fleet permission requirement for private location…

… monitor cruds (#159378) Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Dominique Belcher <[email protected]> Co-authored-by: florent-leborgne <[email protected]>
elastic · Jun 14, 2023 · cf9f5bb · cf9f5bb
1 parent b7dd5ba
commit cf9f5bb
Show file tree

Hide file tree

Showing 51 changed files with 506 additions and 916 deletions.
diff --git a/x-pack/plugins/fleet/server/routes/agent_policy/handlers.ts b/x-pack/plugins/fleet/server/routes/agent_policy/handlers.ts
@@ -51,7 +51,7 @@ import type {
 import { defaultFleetErrorHandler, AgentPolicyNotFoundError } from '../../errors';
 import { createAgentPolicyWithPackages } from '../../services/agent_policy_create';
 
-async function populateAssignedAgentsCount(
+export async function populateAssignedAgentsCount(
   esClient: ElasticsearchClient,
   soClient: SavedObjectsClientContract,
   agentPolicies: AgentPolicy[]
@@ -81,7 +81,9 @@ export const getAgentPoliciesHandler: FleetRequestHandler<
   try {
     const { items, total, page, perPage } = await agentPolicyService.list(soClient, {
       withPackagePolicies,
+      esClient,
       ...restOfQuery,
+      withAgentCount: !noAgentCount,
     });
 
     const body: GetAgentPoliciesResponse = {
@@ -90,11 +92,6 @@ export const getAgentPoliciesHandler: FleetRequestHandler<
       page,
       perPage,
     };
-    if (!noAgentCount) {
-      await populateAssignedAgentsCount(esClient, soClient, items);
-    } else {
-      items.forEach((item) => (item.agents = 0));
-    }
     return response.ok({ body });
   } catch (error) {
     return defaultFleetErrorHandler({ error, response });

diff --git a/x-pack/plugins/fleet/server/services/agent_policy.ts b/x-pack/plugins/fleet/server/services/agent_policy.ts
@@ -22,6 +22,8 @@ import type { BulkResponseItem } from '@elastic/elasticsearch/lib/api/typesWithB
 
 import { DEFAULT_SPACE_ID } from '@kbn/spaces-plugin/common/constants';
 
+import { populateAssignedAgentsCount } from '../routes/agent_policy/handlers';
+
 import type { HTTPAuthorizationHeader } from '../../common/http_authorization_header';
 
 import {
@@ -370,6 +372,8 @@ class AgentPolicyService {
     options: ListWithKuery & {
       withPackagePolicies?: boolean;
       fields?: string[];
+      esClient?: ElasticsearchClient;
+      withAgentCount?: boolean;
     }
   ): Promise<{
     items: AgentPolicy[];
@@ -385,6 +389,8 @@ class AgentPolicyService {
       kuery,
       withPackagePolicies = false,
       fields,
+      esClient,
+      withAgentCount = false,
     } = options;
 
     const baseFindParams = {
@@ -424,14 +430,8 @@ class AgentPolicyService {
           ...agentPolicySO.attributes,
         };
         if (withPackagePolicies) {
-          const agentPolicyWithPackagePolicies = await this.get(
-            soClient,
-            agentPolicySO.id,
-            withPackagePolicies
-          );
-          if (agentPolicyWithPackagePolicies) {
-            agentPolicy.package_policies = agentPolicyWithPackagePolicies.package_policies;
-          }
+          agentPolicy.package_policies =
+            (await packagePolicyService.findAllForAgentPolicy(soClient, agentPolicySO.id)) || [];
         }
         return agentPolicy;
       },
@@ -445,6 +445,11 @@ class AgentPolicyService {
         savedObjectType: AGENT_POLICY_SAVED_OBJECT_TYPE,
       });
     }
+    if (esClient && withAgentCount) {
+      await populateAssignedAgentsCount(esClient, soClient, agentPolicies);
+    } else {
+      agentPolicies.forEach((item) => (item.agents = 0));
+    }
 
     return {
       items: agentPolicies,

diff --git a/x-pack/plugins/synthetics/common/constants/synthetics/rest_api.ts b/x-pack/plugins/synthetics/common/constants/synthetics/rest_api.ts
@@ -25,6 +25,7 @@ export enum SYNTHETICS_API_URLS {
   OVERVIEW_STATUS = `/internal/synthetics/overview_status`,
   INDEX_SIZE = `/internal/synthetics/index_size`,
   PARAMS = `/internal/synthetics/params`,
+  AGENT_POLICIES = `/internal/synthetics/agent_policies`,
   PRIVATE_LOCATIONS = `/internal/synthetics/private_locations`,
   PRIVATE_LOCATIONS_MONITORS = `/internal/synthetics/private_locations/monitors`,
   SYNC_GLOBAL_PARAMS = `/internal/synthetics/sync_global_params`,

diff --git a/x-pack/plugins/synthetics/e2e/synthetics/journeys/private_locations.journey.ts b/x-pack/plugins/synthetics/e2e/synthetics/journeys/private_locations.journey.ts
@@ -159,8 +159,9 @@ journey(`PrivateLocationsSettings`, async ({ page, params }) => {
   step('viewer user cannot add locations', async () => {
     await syntheticsApp.navigateToSettings(false);
     await page.click('text=Private Locations');
+    await page.hover(byTestId('syntheticsEmptyLocationsButton'), { force: true });
     await page.waitForSelector(
-      `text="You're missing some Kibana privileges to manage private locations"`
+      `text="You do not have sufficient permissions to perform this action."`
     );
     const createLocationBtn = await page.getByRole('button', { name: 'Create location' });
     expect(await createLocationBtn.getAttribute('disabled')).toEqual('');

diff --git a/...ck/plugins/synthetics/public/apps/synthetics/components/common/components/permissions.tsx b/...ck/plugins/synthetics/public/apps/synthetics/components/common/components/permissions.tsx
@@ -14,30 +14,28 @@ export const FleetPermissionsCallout = () => {
   return (
     <EuiCallOut title={NEED_PERMISSIONS_PRIVATE_LOCATIONS} color="warning" iconType="help">
       <p>{NEED_PRIVATE_LOCATIONS_PERMISSION}</p>
+      <p>
+        <FormattedMessage
+          id="xpack.synthetics.privateLocations.needFleetPermission.description"
+          defaultMessage="Once there is an agent policy available, you'll be able to manage private locations and monitors with the regular Synthetics app privileges."
+        />
+      </p>
     </EuiCallOut>
   );
 };
 
 /**
- * If any of the canEditSynthetics or canUpdatePrivateMonitor is false, then wrap the children with a tooltip
+ * If canEditSynthetics  is false, then wrap the children with a tooltip
  * so that a reason can be conveyed to the user explaining why the action is disabled.
  */
 export const NoPermissionsTooltip = ({
   canEditSynthetics = true,
-  canUpdatePrivateMonitor = true,
-  canAddPrivateMonitor = true,
   children,
 }: {
   canEditSynthetics?: boolean;
-  canUpdatePrivateMonitor?: boolean;
-  canAddPrivateMonitor?: boolean;
   children: ReactNode;
 }) => {
-  const disabledMessage = getRestrictionReasonLabel(
-    canEditSynthetics,
-    canUpdatePrivateMonitor,
-    canAddPrivateMonitor
-  );
+  const disabledMessage = getRestrictionReasonLabel(canEditSynthetics);
   if (disabledMessage) {
     return (
       <EuiToolTip content={disabledMessage}>
@@ -49,18 +47,8 @@ export const NoPermissionsTooltip = ({
   return <>{children}</>;
 };
 
-function getRestrictionReasonLabel(
-  canEditSynthetics = true,
-  canUpdatePrivateMonitor = true,
-  canAddPrivateMonitor = true
-): string | undefined {
-  return !canEditSynthetics
-    ? CANNOT_PERFORM_ACTION_SYNTHETICS
-    : !canUpdatePrivateMonitor
-    ? CANNOT_PERFORM_ACTION_FLEET
-    : !canAddPrivateMonitor
-    ? PRIVATE_LOCATIONS_NOT_ALLOWED_MESSAGE
-    : undefined;
+function getRestrictionReasonLabel(canEditSynthetics = true): string | undefined {
+  return !canEditSynthetics ? CANNOT_PERFORM_ACTION_SYNTHETICS : undefined;
 }
 
 export const NEED_PERMISSIONS_PRIVATE_LOCATIONS = i18n.translate(
@@ -77,40 +65,16 @@ export const ALL = i18n.translate('xpack.synthetics.monitorManagement.priviledge
 export const NEED_PRIVATE_LOCATIONS_PERMISSION = (
   <FormattedMessage
     id="xpack.synthetics.monitorManagement.privateLocations.needFleetPermission"
-    defaultMessage="You are not authorized to manage private locations. It requires the {all} Kibana privilege for both Fleet and Integrations."
+    defaultMessage="In order to create private locations, you need an agent policy. You are not authorized to create Fleet agent policies. It requires the {all} Kibana privilege for Fleet."
     values={{
       all: <EuiCode>{`"${ALL}"`}</EuiCode>,
     }}
   />
 );
 
-export const CANNOT_SAVE_INTEGRATION_LABEL = i18n.translate(
-  'xpack.synthetics.monitorManagement.cannotSaveIntegration',
-  {
-    defaultMessage:
-      'You are not authorized to manage private locations. It requires the "All" Kibana privilege for both Fleet and Integrations.',
-  }
-);
-
-const CANNOT_PERFORM_ACTION_FLEET = i18n.translate(
-  'xpack.synthetics.monitorManagement.noFleetPermission',
-  {
-    defaultMessage:
-      'You are not authorized to perform this action. It requires the "All" Kibana privilege for Integrations.',
-  }
-);
-
 export const CANNOT_PERFORM_ACTION_SYNTHETICS = i18n.translate(
   'xpack.synthetics.monitorManagement.noSyntheticsPermissions',
   {
     defaultMessage: 'You do not have sufficient permissions to perform this action.',
   }
 );
-
-const PRIVATE_LOCATIONS_NOT_ALLOWED_MESSAGE = i18n.translate(
-  'xpack.synthetics.monitorManagement.privateLocationsNotAllowedMessage',
-  {
-    defaultMessage:
-      'You do not have permission to add monitors to private locations. Contact your administrator to request access.',
-  }
-);
diff --git a/...cs/public/apps/synthetics/components/common/screenshot/journey_screenshot_dialog.test.tsx b/...cs/public/apps/synthetics/components/common/screenshot/journey_screenshot_dialog.test.tsx
@@ -57,22 +57,22 @@ describe('JourneyScreenshotDialog', () => {
   });
 
   it('shows loading indicator when image is loading', async () => {
-    const { queryByTestId } = render(<JourneyScreenshotDialog {...testProps} />);
+    const { getByTestId, queryByTestId } = render(<JourneyScreenshotDialog {...testProps} />);
 
     expect(queryByTestId('screenshotImageLoadingProgress')).not.toBeInTheDocument();
-    userEvent.click(queryByTestId('screenshotImageNextButton'));
+    userEvent.click(getByTestId('screenshotImageNextButton'));
   });
 
   it('respects maxSteps', () => {
-    const { queryByTestId } = render(<JourneyScreenshotDialog {...testProps} />);
+    const { getByTestId, queryByTestId } = render(<JourneyScreenshotDialog {...testProps} />);
 
     expect(queryByTestId('screenshotImageLoadingProgress')).not.toBeInTheDocument();
-    userEvent.click(queryByTestId('screenshotImageNextButton'));
-    expect(queryByTestId('screenshotImageNextButton')).toHaveProperty('disabled');
+    userEvent.click(getByTestId('screenshotImageNextButton'));
+    expect(getByTestId('screenshotImageNextButton')).toHaveProperty('disabled');
   });
 
   it('shows correct image source and step name', () => {
-    const { queryByTestId, getByText } = render(<JourneyScreenshotDialog {...testProps} />);
+    const { getByText, queryByTestId } = render(<JourneyScreenshotDialog {...testProps} />);
     expect(queryByTestId('stepScreenshotThumbnail')).toHaveProperty(
       'src',
       'http://localhost/test-img-url-1'

diff --git a/...ynthetics/public/apps/synthetics/components/getting_started/getting_started_page.test.tsx b/...ynthetics/public/apps/synthetics/components/getting_started/getting_started_page.test.tsx
@@ -10,6 +10,9 @@ import * as permissionsHooks from '../../hooks/use_fleet_permissions';
 import { render } from '../../utils/testing/rtl_helpers';
 import { GettingStartedPage } from './getting_started_page';
 import * as privateLocationsHooks from '../settings/private_locations/hooks/use_locations_api';
+import * as settingsHooks from '../../contexts/synthetics_settings_context';
+import { SyntheticsSettingsContextValues } from '../../contexts/synthetics_settings_context';
+import { fireEvent } from '@testing-library/react';
 
 describe('GettingStartedPage', () => {
   beforeEach(() => {
@@ -66,6 +69,10 @@ describe('GettingStartedPage', () => {
   });
 
   it('shows need agent flyout when isAddingNewPrivateLocation is true and agentPolicies.length === 0', async () => {
+    jest.spyOn(settingsHooks, 'useSyntheticsSettingsContext').mockReturnValue({
+      canSave: true,
+    } as SyntheticsSettingsContextValues);
+
     const { getByText, getByRole, queryByLabelText } = render(<GettingStartedPage />, {
       state: {
         serviceLocations: {
@@ -87,7 +94,7 @@ describe('GettingStartedPage', () => {
 
     expect(getByRole('heading', { name: 'Create private location', level: 2 }));
     expect(getByText('No agent policies found')).toBeInTheDocument();
-    expect(getByRole('link', { name: 'Create agent policy' })).toBeEnabled();
+    expect(getByRole('button', { name: 'Create agent policy' })).not.toBeEnabled();
     expect(queryByLabelText('Location name')).not.toBeInTheDocument();
     expect(queryByLabelText('Agent policy')).not.toBeInTheDocument();
   });
@@ -119,57 +126,41 @@ describe('GettingStartedPage', () => {
     expect(getByLabelText('Agent policy')).toBeInTheDocument();
   });
 
-  it('shows permissions callout and hides form when agent policies are available but the user does not have permissions', async () => {
-    jest.spyOn(permissionsHooks, 'useCanManagePrivateLocation').mockReturnValue(false);
-    const { getByText, getByRole, queryByLabelText, queryByRole } = render(<GettingStartedPage />, {
-      state: {
-        serviceLocations: {
-          locations: [],
-          locationsLoaded: true,
-          loading: false,
-        },
-        agentPolicies: {
-          data: {
-            total: 1,
-            items: [{}],
+  it('shows permissions tooltip when the user does not have permissions', async () => {
+    jest.spyOn(settingsHooks, 'useSyntheticsSettingsContext').mockReturnValue({
+      canSave: false,
+    } as SyntheticsSettingsContextValues);
+    const { getByText, getByRole, queryByLabelText, queryByRole, findByText } = render(
+      <GettingStartedPage />,
+      {
+        state: {
+          serviceLocations: {
+            locations: [],
+            locationsLoaded: true,
+            loading: false,
+          },
+          agentPolicies: {
+            data: {
+              total: 1,
+              items: [{}],
+            },
+            isAddingNewPrivateLocation: true,
           },
-          isAddingNewPrivateLocation: true,
-        },
-      },
-    });
-
-    // page is loaded
-    expect(getByText('Get started with synthetic monitoring')).toBeInTheDocument();
-
-    expect(getByRole('heading', { name: 'Create private location', level: 2 }));
-    expect(queryByLabelText('Location name')).not.toBeInTheDocument();
-    expect(queryByLabelText('Agent policy')).not.toBeInTheDocument();
-    expect(queryByRole('button', { name: 'Save' })).not.toBeInTheDocument();
-    expect(getByText("You're missing some Kibana privileges to manage private locations"));
-  });
-
-  it('shows permissions callout when agent policy is needed but the user does not have permissions', async () => {
-    jest.spyOn(permissionsHooks, 'useCanManagePrivateLocation').mockReturnValue(false);
-    const { getByText, getByRole, queryByLabelText } = render(<GettingStartedPage />, {
-      state: {
-        serviceLocations: {
-          locations: [],
-          locationsLoaded: true,
-          loading: false,
-        },
-        agentPolicies: {
-          data: undefined, // data will be undefined when user does not have permissions
-          isAddingNewPrivateLocation: true,
         },
-      },
-    });
+      }
+    );
 
     // page is loaded
     expect(getByText('Get started with synthetic monitoring')).toBeInTheDocument();
 
     expect(getByRole('heading', { name: 'Create private location', level: 2 }));
-    expect(queryByLabelText('Location name')).not.toBeInTheDocument();
-    expect(queryByLabelText('Agent policy')).not.toBeInTheDocument();
-    expect(getByText("You're missing some Kibana privileges to manage private locations"));
+    expect(queryByLabelText('Location name')).toBeInTheDocument();
+    expect(queryByLabelText('Agent policy')).toBeInTheDocument();
+    expect(queryByRole('button', { name: 'Save' })).toBeInTheDocument();
+    expect(queryByRole('button', { name: 'Save' })).toBeDisabled();
+    fireEvent.mouseOver(getByRole('button', { name: 'Save' }));
+    expect(
+      await findByText(/You do not have sufficient permissions to perform this action./)
+    ).toBeInTheDocument();
   });
 });
diff --git a/...ins/synthetics/public/apps/synthetics/components/getting_started/getting_started_page.tsx b/...ins/synthetics/public/apps/synthetics/components/getting_started/getting_started_page.tsx
@@ -20,7 +20,7 @@ import { useHistory } from 'react-router-dom';
 import { i18n } from '@kbn/i18n';
 import { FormattedMessage } from '@kbn/i18n-react';
 import styled from 'styled-components';
-import { useBreadcrumbs, useLocations, useFleetPermissions } from '../../hooks';
+import { useBreadcrumbs, useLocations } from '../../hooks';
 import { usePrivateLocationsAPI } from '../settings/private_locations/hooks/use_locations_api';
 import { LoadingState } from '../monitors_page/overview/overview/monitor_detail_flyout';
 import {
@@ -40,17 +40,14 @@ export const GettingStartedPage = () => {
   const dispatch = useDispatch();
   const history = useHistory();
 
-  const { canReadAgentPolicies } = useFleetPermissions();
-
   useEffect(() => {
     dispatch(getServiceLocations());
-    if (canReadAgentPolicies) {
-      dispatch(getAgentPoliciesAction.get());
-    }
+    dispatch(getAgentPoliciesAction.get());
+
     return () => {
       dispatch(cleanMonitorListState());
     };
-  }, [canReadAgentPolicies, dispatch]);
+  }, [dispatch]);
 
   useBreadcrumbs([{ text: MONITORING_OVERVIEW_LABEL }]); // No extra breadcrumbs on overview