diff --git a/docs/api/synthetics/params/edit-param.asciidoc b/docs/api/synthetics/params/edit-param.asciidoc index e615dd0c0bd1f..07a2568207dfe 100644 --- a/docs/api/synthetics/params/edit-param.asciidoc +++ b/docs/api/synthetics/params/edit-param.asciidoc @@ -26,13 +26,13 @@ You must have `all` privileges for the *Synthetics* feature in the *{observabili [[parameter-edit-request-body]] ==== Request body -The request body should contain the following attributes: +The request body can contain the following attributes, it can't be empty at least one attribute is required.: `key`:: -(Required, string) The key of the parameter. +(Optional, string) The key of the parameter. `value`:: -(Required, string) The updated value associated with the parameter. +(Optional, string) The updated value associated with the parameter. `description`:: (Optional, string) The updated description of the parameter. diff --git a/x-pack/plugins/observability_solution/synthetics/e2e/synthetics/journeys/global_parameters.journey.ts b/x-pack/plugins/observability_solution/synthetics/e2e/synthetics/journeys/global_parameters.journey.ts index 831f8d107f36a..b328b273836a7 100644 --- a/x-pack/plugins/observability_solution/synthetics/e2e/synthetics/journeys/global_parameters.journey.ts +++ b/x-pack/plugins/observability_solution/synthetics/e2e/synthetics/journeys/global_parameters.journey.ts @@ -76,8 +76,8 @@ journey(`GlobalParameters`, async ({ page, params }) => { await page.click('text=Delete ParameterEdit Parameter >> :nth-match(button, 2)'); await page.click('[aria-label="Key"]'); await page.fill('[aria-label="Key"]', 'username2'); - await page.click('[aria-label="Value"]'); - await page.fill('[aria-label="Value"]', 'elastic2'); + await page.click('[aria-label="New value"]'); + await page.fill('[aria-label="New value"]', 'elastic2'); await page.click('.euiComboBox__inputWrap'); await page.fill('[aria-label="Tags"]', 'staging'); await page.press('[aria-label="Tags"]', 'Enter'); diff --git a/x-pack/plugins/observability_solution/synthetics/public/apps/synthetics/components/settings/components/optional_text.tsx b/x-pack/plugins/observability_solution/synthetics/public/apps/synthetics/components/settings/components/optional_text.tsx new file mode 100644 index 0000000000000..a764cf3b27cdc --- /dev/null +++ b/x-pack/plugins/observability_solution/synthetics/public/apps/synthetics/components/settings/components/optional_text.tsx @@ -0,0 +1,20 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { EuiText } from '@elastic/eui'; +import { i18n } from '@kbn/i18n'; +import React from 'react'; + +export function OptionalText() { + return ( + + {i18n.translate('xpack.synthetics.sloEdit.optionalLabel', { + defaultMessage: 'Optional', + })} + + ); +} diff --git a/x-pack/plugins/observability_solution/synthetics/public/apps/synthetics/components/settings/global_params/add_param_flyout.tsx b/x-pack/plugins/observability_solution/synthetics/public/apps/synthetics/components/settings/global_params/add_param_flyout.tsx index 3fd17335d2ea5..70c2eb77526af 100644 --- a/x-pack/plugins/observability_solution/synthetics/public/apps/synthetics/components/settings/global_params/add_param_flyout.tsx +++ b/x-pack/plugins/observability_solution/synthetics/public/apps/synthetics/components/settings/global_params/add_param_flyout.tsx @@ -22,6 +22,7 @@ import { FormProvider } from 'react-hook-form'; import { useKibana } from '@kbn/kibana-react-plugin/public'; import { i18n } from '@kbn/i18n'; import { useDispatch, useSelector } from 'react-redux'; +import { isEmpty } from 'lodash'; import { NoPermissionsTooltip } from '../../common/components/permissions'; import { addNewGlobalParamAction, @@ -80,18 +81,29 @@ export const AddParamFlyout = ({ const onSubmit = (formData: SyntheticsParams) => { const { namespaces, ...paramRequest } = formData; const shareAcrossSpaces = namespaces?.includes(ALL_SPACES_ID); + const newParamData = { + ...paramRequest, + }; + + if (isEditingItem && id) { + // omit value if it's empty + if (isEmpty(newParamData.value)) { + // @ts-ignore this is a valid check + delete newParamData.value; + } + } if (isEditingItem && id) { dispatch( editGlobalParamAction.get({ id, - paramRequest: { ...paramRequest, share_across_spaces: shareAcrossSpaces }, + paramRequest, }) ); } else { dispatch( addNewGlobalParamAction.get({ - ...paramRequest, + ...newParamData, share_across_spaces: shareAcrossSpaces, }) ); diff --git a/x-pack/plugins/observability_solution/synthetics/public/apps/synthetics/components/settings/global_params/add_param_form.tsx b/x-pack/plugins/observability_solution/synthetics/public/apps/synthetics/components/settings/global_params/add_param_form.tsx index 1b219a0f6fec4..d472ec62237e9 100644 --- a/x-pack/plugins/observability_solution/synthetics/public/apps/synthetics/components/settings/global_params/add_param_form.tsx +++ b/x-pack/plugins/observability_solution/synthetics/public/apps/synthetics/components/settings/global_params/add_param_form.tsx @@ -6,16 +6,11 @@ */ import React from 'react'; import { ALL_SPACES_ID } from '@kbn/security-plugin/public'; -import { - EuiCheckbox, - EuiComboBox, - EuiFieldText, - EuiForm, - EuiFormRow, - EuiTextArea, -} from '@elastic/eui'; +import { EuiCheckbox, EuiComboBox, EuiFieldText, EuiForm, EuiFormRow } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { Controller, useFormContext, useFormState } from 'react-hook-form'; +import { OptionalText } from '../components/optional_text'; +import { ParamValueField } from './param_value_field'; import { SyntheticsParams } from '../../../../../../common/runtime_types'; import { ListParamItem } from './params_list'; @@ -61,25 +56,8 @@ export const AddParamForm = ({ })} /> - - - - + + }> - + }> { + const { register } = useFormContext(); + const { errors } = useFormState(); + + if (isEditingItem) { + return ( + <> + } + > + + + + + + ); + } + + return ( + + + + ); +}; + +export const NEW_VALUE_LABEL = i18n.translate( + 'xpack.synthetics.monitorManagement.paramForm.newValue', + { + defaultMessage: 'New value', + } +); diff --git a/x-pack/plugins/observability_solution/synthetics/public/apps/synthetics/state/global_params/api.ts b/x-pack/plugins/observability_solution/synthetics/public/apps/synthetics/state/global_params/api.ts index ce7f9bd81ea3d..33eb4622bf6c5 100644 --- a/x-pack/plugins/observability_solution/synthetics/public/apps/synthetics/state/global_params/api.ts +++ b/x-pack/plugins/observability_solution/synthetics/public/apps/synthetics/state/global_params/api.ts @@ -5,6 +5,7 @@ * 2.0. */ +import { isEmpty } from 'lodash'; import { INITIAL_REST_VERSION, SYNTHETICS_API_URLS } from '../../../../../common/constants'; import { DeleteParamsResponse, @@ -35,16 +36,22 @@ export const editGlobalParam = async ({ id, }: { id: string; - paramRequest: SyntheticsParamRequest; -}): Promise => - apiService.put( + paramRequest: Partial; +}): Promise => { + const data = paramRequest; + if (isEmpty(paramRequest.value)) { + // omit empty value + delete data.value; + } + return await apiService.put( SYNTHETICS_API_URLS.PARAMS + `/${id}`, - paramRequest, + data, SyntheticsParamsCodec, { version: INITIAL_REST_VERSION, } ); +}; export const deleteGlobalParams = async (ids: string[]): Promise => apiService.delete( diff --git a/x-pack/plugins/observability_solution/synthetics/server/routes/common.ts b/x-pack/plugins/observability_solution/synthetics/server/routes/common.ts index 0bdc7989b8a8a..2a906f3cf6a4d 100644 --- a/x-pack/plugins/observability_solution/synthetics/server/routes/common.ts +++ b/x-pack/plugins/observability_solution/synthetics/server/routes/common.ts @@ -9,6 +9,7 @@ import { schema, TypeOf } from '@kbn/config-schema'; import { SavedObjectsFindResponse } from '@kbn/core/server'; import { isEmpty } from 'lodash'; import { escapeQuotes } from '@kbn/es-query'; +import { DEFAULT_SPACE_ID } from '@kbn/spaces-plugin/common'; import { RouteContext } from './types'; import { MonitorSortFieldSchema } from '../../common/runtime_types/monitor_management/sort_field'; import { getAllLocations } from '../synthetics_service/get_all_locations'; @@ -269,3 +270,26 @@ function parseMappingKey(key: string | undefined) { return key; } } + +export const validateRouteSpaceName = async (routeContext: RouteContext) => { + const { spaceId, server, request, response } = routeContext; + if (spaceId === DEFAULT_SPACE_ID) { + // default space is always valid + return { spaceId: DEFAULT_SPACE_ID }; + } + + try { + await server.spaces?.spacesService.getActiveSpace(request); + } catch (error) { + if (error.output?.statusCode === 404) { + return { + spaceId, + invalidResponse: response.notFound({ + body: { message: `Kibana space '${spaceId}' does not exist` }, + }), + }; + } + } + + return { invalidResponse: undefined }; +}; diff --git a/x-pack/plugins/observability_solution/synthetics/server/routes/settings/params/add_param.ts b/x-pack/plugins/observability_solution/synthetics/server/routes/settings/params/add_param.ts index a51079f366eff..7d0cac7d7e57c 100644 --- a/x-pack/plugins/observability_solution/synthetics/server/routes/settings/params/add_param.ts +++ b/x-pack/plugins/observability_solution/synthetics/server/routes/settings/params/add_param.ts @@ -19,8 +19,12 @@ import { syntheticsParamType } from '../../../../common/types/saved_objects'; import { SYNTHETICS_API_URLS } from '../../../../common/constants'; const ParamsObjectSchema = schema.object({ - key: schema.string(), - value: schema.string(), + key: schema.string({ + minLength: 1, + }), + value: schema.string({ + minLength: 1, + }), description: schema.maybe(schema.string()), tags: schema.maybe(schema.arrayOf(schema.string())), share_across_spaces: schema.maybe(schema.boolean()), diff --git a/x-pack/plugins/observability_solution/synthetics/server/routes/settings/params/edit_param.ts b/x-pack/plugins/observability_solution/synthetics/server/routes/settings/params/edit_param.ts index 3555963b76bf1..eb9f41696da97 100644 --- a/x-pack/plugins/observability_solution/synthetics/server/routes/settings/params/edit_param.ts +++ b/x-pack/plugins/observability_solution/synthetics/server/routes/settings/params/edit_param.ts @@ -6,8 +6,9 @@ */ import { schema, TypeOf } from '@kbn/config-schema'; -import { SavedObject } from '@kbn/core/server'; -import { DEFAULT_SPACE_ID } from '@kbn/spaces-plugin/common'; +import { SavedObject, SavedObjectsErrorHelpers } from '@kbn/core/server'; +import { isEmpty } from 'lodash'; +import { validateRouteSpaceName } from '../../common'; import { SyntheticsRestApiRouteFactory } from '../../types'; import { SyntheticsParamRequest, SyntheticsParams } from '../../../../common/runtime_types'; import { syntheticsParamType } from '../../../../common/types/saved_objects'; @@ -20,7 +21,7 @@ const RequestParamsSchema = schema.object({ type RequestParams = TypeOf; export const editSyntheticsParamsRoute: SyntheticsRestApiRouteFactory< - SyntheticsParams, + SyntheticsParams | undefined, RequestParams > = () => ({ method: 'PUT', @@ -30,46 +31,63 @@ export const editSyntheticsParamsRoute: SyntheticsRestApiRouteFactory< request: { params: RequestParamsSchema, body: schema.object({ - key: schema.string(), - value: schema.string(), + key: schema.maybe( + schema.string({ + minLength: 1, + }) + ), + value: schema.maybe( + schema.string({ + minLength: 1, + }) + ), description: schema.maybe(schema.string()), tags: schema.maybe(schema.arrayOf(schema.string())), - share_across_spaces: schema.maybe(schema.boolean()), }), }, }, - handler: async ({ savedObjectsClient, request, server, response }) => { + handler: async (routeContext) => { + const { savedObjectsClient, request, response, spaceId, server } = routeContext; + const { invalidResponse } = await validateRouteSpaceName(routeContext); + if (invalidResponse) return invalidResponse; + + const { id: paramId } = request.params; + const data = request.body as SyntheticsParamRequest; + if (isEmpty(data)) { + return response.badRequest({ body: { message: 'Request body cannot be empty' } }); + } + const encryptedSavedObjectsClient = server.encryptedSavedObjects.getClient(); + try { - const { id: _spaceId } = (await server.spaces?.spacesService.getActiveSpace(request)) ?? { - id: DEFAULT_SPACE_ID, + const existingParam = + await encryptedSavedObjectsClient.getDecryptedAsInternalUser( + syntheticsParamType, + paramId, + { namespace: spaceId } + ); + + const newParam = { + ...existingParam.attributes, + ...data, }; - const { id } = request.params; - const { share_across_spaces: _shareAcrossSpaces, ...data } = - request.body as SyntheticsParamRequest & { - id: string; - }; - const { value } = data; + // value from data since we aren't using encrypted client + const { value } = existingParam.attributes; const { id: responseId, attributes: { key, tags, description }, namespaces, - } = (await savedObjectsClient.update( + } = (await savedObjectsClient.update( syntheticsParamType, - id, - data + paramId, + newParam )) as SavedObject; return { id: responseId, key, tags, description, namespaces, value }; - } catch (error) { - if (error.output?.statusCode === 404) { - const spaceId = server.spaces?.spacesService.getSpaceId(request) ?? DEFAULT_SPACE_ID; - return response.notFound({ - body: { message: `Kibana space '${spaceId}' does not exist` }, - }); + } catch (getErr) { + if (SavedObjectsErrorHelpers.isNotFoundError(getErr)) { + return response.notFound({ body: { message: 'Param not found' } }); } - - throw error; } }, }); diff --git a/x-pack/plugins/observability_solution/synthetics/server/routes/settings/params/params.ts b/x-pack/plugins/observability_solution/synthetics/server/routes/settings/params/params.ts index 01f2dd6465dfd..da0a2e250557a 100644 --- a/x-pack/plugins/observability_solution/synthetics/server/routes/settings/params/params.ts +++ b/x-pack/plugins/observability_solution/synthetics/server/routes/settings/params/params.ts @@ -7,7 +7,7 @@ import { SavedObject, SavedObjectsFindResult } from '@kbn/core-saved-objects-api-server'; import { schema, TypeOf } from '@kbn/config-schema'; -import { SyntheticsRestApiRouteFactory } from '../../types'; +import { RouteContext, SyntheticsRestApiRouteFactory } from '../../types'; import { syntheticsParamType } from '../../../../common/types/saved_objects'; import { SYNTHETICS_API_URLS } from '../../../../common/constants'; import { SyntheticsParams, SyntheticsParamsReadonly } from '../../../../common/runtime_types'; @@ -30,45 +30,13 @@ export const getSyntheticsParamsRoute: SyntheticsRestApiRouteFactory< params: RequestParamsSchema, }, }, - handler: async ({ savedObjectsClient, request, response, server, spaceId }) => { + handler: async (routeContext) => { + const { savedObjectsClient, request, response, spaceId } = routeContext; try { const { id: paramId } = request.params; - const encryptedSavedObjectsClient = server.encryptedSavedObjects.getClient(); - - const canSave = - ( - await server.coreStart?.capabilities.resolveCapabilities(request, { - capabilityPath: 'uptime.*', - }) - ).uptime.save ?? false; - - if (canSave) { - if (paramId) { - const savedObject = - await encryptedSavedObjectsClient.getDecryptedAsInternalUser( - syntheticsParamType, - paramId, - { namespace: spaceId } - ); - return toClientResponse(savedObject); - } - - const finder = - await encryptedSavedObjectsClient.createPointInTimeFinderDecryptedAsInternalUser( - { - type: syntheticsParamType, - perPage: 1000, - namespaces: [spaceId], - } - ); - - const hits: Array> = []; - for await (const result of finder.find()) { - hits.push(...result.saved_objects); - } - - return hits.map((savedObject) => toClientResponse(savedObject)); + if (await isAnAdminUser(routeContext)) { + return getDecryptedParams(routeContext, paramId); } else { if (paramId) { const savedObject = await savedObjectsClient.get( @@ -78,11 +46,7 @@ export const getSyntheticsParamsRoute: SyntheticsRestApiRouteFactory< return toClientResponse(savedObject); } - const data = await savedObjectsClient.find({ - type: syntheticsParamType, - perPage: 10000, - }); - return data.saved_objects.map((savedObject) => toClientResponse(savedObject)); + return findAllParams(routeContext); } } catch (error) { if (error.output?.statusCode === 404) { @@ -94,6 +58,70 @@ export const getSyntheticsParamsRoute: SyntheticsRestApiRouteFactory< }, }); +const isAnAdminUser = async (routeContext: RouteContext) => { + const { request, server } = routeContext; + const user = server.coreStart.security.authc.getCurrentUser(request); + + const isSuperUser = user?.roles.includes('superuser'); + const isAdmin = user?.roles.includes('kibana_admin'); + + const canSave = + ( + await server.coreStart?.capabilities.resolveCapabilities(request, { + capabilityPath: 'uptime.*', + }) + ).uptime.save ?? false; + + return (isSuperUser || isAdmin) && canSave; +}; + +const getDecryptedParams = async ({ server, spaceId }: RouteContext, paramId?: string) => { + const encryptedSavedObjectsClient = server.encryptedSavedObjects.getClient(); + + if (paramId) { + const savedObject = + await encryptedSavedObjectsClient.getDecryptedAsInternalUser( + syntheticsParamType, + paramId, + { namespace: spaceId } + ); + return toClientResponse(savedObject); + } + const finder = + await encryptedSavedObjectsClient.createPointInTimeFinderDecryptedAsInternalUser( + { + type: syntheticsParamType, + perPage: 1000, + namespaces: [spaceId], + } + ); + + const hits: Array> = []; + for await (const result of finder.find()) { + hits.push(...result.saved_objects); + } + + void finder.close(); + + return hits.map((savedObject) => toClientResponse(savedObject)); +}; + +const findAllParams = async ({ savedObjectsClient }: RouteContext) => { + const finder = savedObjectsClient.createPointInTimeFinder({ + type: syntheticsParamType, + perPage: 1000, + }); + + const hits: Array> = []; + for await (const result of finder.find()) { + hits.push(...result.saved_objects); + } + + void finder.close(); + + return hits.map((savedObject) => toClientResponse(savedObject)); +}; + const toClientResponse = ( savedObject: SavedObject ) => { diff --git a/x-pack/test/api_integration/apis/synthetics/add_edit_params.ts b/x-pack/test/api_integration/apis/synthetics/add_edit_params.ts index 4de02eb80b30c..7b27aaa621f46 100644 --- a/x-pack/test/api_integration/apis/synthetics/add_edit_params.ts +++ b/x-pack/test/api_integration/apis/synthetics/add_edit_params.ts @@ -10,6 +10,7 @@ import { pick } from 'lodash'; import { SYNTHETICS_API_URLS } from '@kbn/synthetics-plugin/common/constants'; import expect from '@kbn/expect'; import { syntheticsParamType } from '@kbn/synthetics-plugin/common/types/saved_objects'; +import { SyntheticsMonitorTestService } from './services/synthetics_monitor_test_service'; import { FtrProviderContext } from '../../ftr_provider_context'; import { PrivateLocationTestService } from './services/private_location_test_service'; @@ -21,12 +22,15 @@ export default function ({ getService }: FtrProviderContext) { describe('AddEditParams', function () { this.tags('skipCloud'); const supertestAPI = getService('supertest'); + const supertestWithoutAuth = getService('supertestWithoutAuth'); + const kServer = getService('kibanaServer'); const testParam = { key: 'test', value: 'test', }; const testPrivateLocations = new PrivateLocationTestService(getService); + const monitorTestService = new SyntheticsMonitorTestService(getService); before(async () => { await testPrivateLocations.installSyntheticsPackage(); @@ -93,6 +97,12 @@ export default function ({ getService }: FtrProviderContext) { const param = getResponse.body[0]; assertHas(param, testParam); + await supertestAPI + .put(SYNTHETICS_API_URLS.PARAMS + '/' + param.id) + .set('kbn-xsrf', 'true') + .send({}) + .expect(400); + await supertestAPI .put(SYNTHETICS_API_URLS.PARAMS + '/' + param.id) .set('kbn-xsrf', 'true') @@ -107,6 +117,55 @@ export default function ({ getService }: FtrProviderContext) { assertHas(actualUpdatedParam, expectedUpdatedParam); }); + it('handles partial editing a param', async () => { + const newParam = { + key: 'testUpdated', + value: 'testUpdated', + tags: ['a tag'], + description: 'test description', + }; + + const response = await supertestAPI + .post(SYNTHETICS_API_URLS.PARAMS) + .set('kbn-xsrf', 'true') + .send(newParam) + .expect(200); + const paramId = response.body.id; + + const getResponse = await supertestAPI + .get(SYNTHETICS_API_URLS.PARAMS + '/' + paramId) + .set('kbn-xsrf', 'true') + .expect(200); + assertHas(getResponse.body, newParam); + + await supertestAPI + .put(SYNTHETICS_API_URLS.PARAMS + '/' + paramId) + .set('kbn-xsrf', 'true') + .send({ + key: 'testUpdated', + }) + .expect(200); + + await supertestAPI + .put(SYNTHETICS_API_URLS.PARAMS + '/' + paramId) + .set('kbn-xsrf', 'true') + .send({ + key: 'testUpdatedAgain', + value: 'testUpdatedAgain', + }) + .expect(200); + + const updatedGetResponse = await supertestAPI + .get(SYNTHETICS_API_URLS.PARAMS + '/' + paramId) + .set('kbn-xsrf', 'true') + .expect(200); + assertHas(updatedGetResponse.body, { + ...newParam, + key: 'testUpdatedAgain', + value: 'testUpdatedAgain', + }); + }); + it('handles spaces', async () => { const SPACE_ID = `test-space-${uuidv4()}`; const SPACE_NAME = `test-space-name ${uuidv4()}`; @@ -277,5 +336,22 @@ export default function ({ getService }: FtrProviderContext) { expect(getResponse.body[0].namespaces).eql(['*']); assertHas(getResponse.body[0], testParam); }); + + it('should not return values for non admin user', async () => { + const { username, password } = await monitorTestService.addsNewSpace(); + const resp = await supertestWithoutAuth + .get(`${SYNTHETICS_API_URLS.PARAMS}`) + .auth(username, password) + .set('kbn-xsrf', 'true') + .send() + .expect(200); + + const params = resp.body; + expect(params.length).to.eql(6); + params.forEach((param: any) => { + expect(param.value).to.eql(undefined); + expect(param.key).to.not.empty(); + }); + }); }); }