Skip to content

Commit

Permalink
[8.x] Unauthorized route migration for routes owned by obs-ux-managem…
Browse files Browse the repository at this point in the history
…ent-team (#198374) (#201298)

# Backport

This will backport the following commits from `main` to `8.x`:
- [Unauthorized route migration for routes owned by
obs-ux-management-team
(#198374)](#198374)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Kibana
Machine","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-21T22:03:55Z","message":"Unauthorized
route migration for routes owned by obs-ux-management-team
(#198374)\n\n### Authz API migration for unauthorized routes\r\n\r\nThis
PR migrates unauthorized routes owned by your team to a new\r\nsecurity
configuration.\r\nPlease refer to the documentation for more
information:
[Authorization\r\nAPI](https://docs.elastic.dev/kibana-dev-docs/key-concepts/security-api-authorization)\r\n\r\n###
**Before migration:**\r\n```ts\r\nrouter.get({\r\n path:
'/api/path',\r\n ...\r\n}, handler);\r\n```\r\n\r\n### **After
migration:**\r\n```ts\r\nrouter.get({\r\n path: '/api/path',\r\n
security: {\r\n authz: {\r\n enabled: false,\r\n reason: 'This route is
opted out from authorization because ...',\r\n },\r\n },\r\n ...\r\n},
handler);\r\n```\r\n\r\n### What to do next?\r\n1. Review the changes in
this PR.\r\n2. Elaborate on the reasoning to opt-out of
authorization.\r\n3. Routes without a compelling reason to opt-out of
authorization should\r\nplan to introduce them as soon as
possible.\r\n2. You might need to update your tests to reflect the new
security\r\nconfiguration:\r\n - If you have snapshot tests that include
the route definition.\r\n\r\n## Any questions?\r\nIf you have any
questions or need help with API authorization, please\r\nreach out to
the `@elastic/kibana-security`
team.\r\n\r\n---------\r\n\r\nCo-authored-by: Dominique Belcher
<[email protected]>\r\nCo-authored-by: Shahzad
<[email protected]>","sha":"0b193ec81ed70e1e36cfb824e89d9bf12b7c5b0b","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["enhancement","release_note:skip","Feature:Security/Authorization","v9.0.0","backport:prev-minor","ci:project-deploy-observability","Team:obs-ux-management","Authz:
API migration"],"title":"Unauthorized route migration for routes owned
by
obs-ux-management-team","number":198374,"url":"https://github.com/elastic/kibana/pull/198374","mergeCommit":{"message":"Unauthorized
route migration for routes owned by obs-ux-management-team
(#198374)\n\n### Authz API migration for unauthorized routes\r\n\r\nThis
PR migrates unauthorized routes owned by your team to a new\r\nsecurity
configuration.\r\nPlease refer to the documentation for more
information:
[Authorization\r\nAPI](https://docs.elastic.dev/kibana-dev-docs/key-concepts/security-api-authorization)\r\n\r\n###
**Before migration:**\r\n```ts\r\nrouter.get({\r\n path:
'/api/path',\r\n ...\r\n}, handler);\r\n```\r\n\r\n### **After
migration:**\r\n```ts\r\nrouter.get({\r\n path: '/api/path',\r\n
security: {\r\n authz: {\r\n enabled: false,\r\n reason: 'This route is
opted out from authorization because ...',\r\n },\r\n },\r\n ...\r\n},
handler);\r\n```\r\n\r\n### What to do next?\r\n1. Review the changes in
this PR.\r\n2. Elaborate on the reasoning to opt-out of
authorization.\r\n3. Routes without a compelling reason to opt-out of
authorization should\r\nplan to introduce them as soon as
possible.\r\n2. You might need to update your tests to reflect the new
security\r\nconfiguration:\r\n - If you have snapshot tests that include
the route definition.\r\n\r\n## Any questions?\r\nIf you have any
questions or need help with API authorization, please\r\nreach out to
the `@elastic/kibana-security`
team.\r\n\r\n---------\r\n\r\nCo-authored-by: Dominique Belcher
<[email protected]>\r\nCo-authored-by: Shahzad
<[email protected]>","sha":"0b193ec81ed70e1e36cfb824e89d9bf12b7c5b0b"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/198374","number":198374,"mergeCommit":{"message":"Unauthorized
route migration for routes owned by obs-ux-management-team
(#198374)\n\n### Authz API migration for unauthorized routes\r\n\r\nThis
PR migrates unauthorized routes owned by your team to a new\r\nsecurity
configuration.\r\nPlease refer to the documentation for more
information:
[Authorization\r\nAPI](https://docs.elastic.dev/kibana-dev-docs/key-concepts/security-api-authorization)\r\n\r\n###
**Before migration:**\r\n```ts\r\nrouter.get({\r\n path:
'/api/path',\r\n ...\r\n}, handler);\r\n```\r\n\r\n### **After
migration:**\r\n```ts\r\nrouter.get({\r\n path: '/api/path',\r\n
security: {\r\n authz: {\r\n enabled: false,\r\n reason: 'This route is
opted out from authorization because ...',\r\n },\r\n },\r\n ...\r\n},
handler);\r\n```\r\n\r\n### What to do next?\r\n1. Review the changes in
this PR.\r\n2. Elaborate on the reasoning to opt-out of
authorization.\r\n3. Routes without a compelling reason to opt-out of
authorization should\r\nplan to introduce them as soon as
possible.\r\n2. You might need to update your tests to reflect the new
security\r\nconfiguration:\r\n - If you have snapshot tests that include
the route definition.\r\n\r\n## Any questions?\r\nIf you have any
questions or need help with API authorization, please\r\nreach out to
the `@elastic/kibana-security`
team.\r\n\r\n---------\r\n\r\nCo-authored-by: Dominique Belcher
<[email protected]>\r\nCo-authored-by: Shahzad
<[email protected]>","sha":"0b193ec81ed70e1e36cfb824e89d9bf12b7c5b0b"}}]}]
BACKPORT-->
  • Loading branch information
kibanamachine authored Nov 21, 2024
1 parent 8f29268 commit 44be16f
Show file tree
Hide file tree
Showing 11 changed files with 276 additions and 65 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,12 @@ export function registerAnnotationAPIs({
router.post(
{
path: '/api/observability/annotation',
security: {
authz: {
enabled: false,
reason: 'This route delegates authorization to Elasticsearch',
},
},
validate: {
body: unknowns,
},
Expand All @@ -110,6 +116,12 @@ export function registerAnnotationAPIs({
router.put(
{
path: '/api/observability/annotation/{id}',
security: {
authz: {
enabled: false,
reason: 'This route delegates authorization to Elasticsearch',
},
},
validate: {
body: unknowns,
},
Expand All @@ -125,6 +137,12 @@ export function registerAnnotationAPIs({
router.delete(
{
path: '/api/observability/annotation/{id}',
security: {
authz: {
enabled: false,
reason: 'This route delegates authorization to Elasticsearch',
},
},
validate: {
params: unknowns,
},
Expand All @@ -140,6 +158,12 @@ export function registerAnnotationAPIs({
router.get(
{
path: '/api/observability/annotation/{id}',
security: {
authz: {
enabled: false,
reason: 'This route delegates authorization to Elasticsearch',
},
},
validate: {
params: unknowns,
},
Expand All @@ -155,6 +179,12 @@ export function registerAnnotationAPIs({
router.get(
{
path: '/api/observability/annotation/find',
security: {
authz: {
enabled: false,
reason: 'This route delegates authorization to Elasticsearch',
},
},
validate: {
query: unknowns,
},
Expand All @@ -170,6 +200,12 @@ export function registerAnnotationAPIs({
router.get(
{
path: '/api/observability/annotation/permissions',
security: {
authz: {
enabled: false,
reason: 'This route delegates authorization to Elasticsearch',
},
},
validate: {
query: unknowns,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/
import { schema } from '@kbn/config-schema';
import { DEFAULT_SPACE_ID } from '@kbn/spaces-plugin/common';
import { isEmpty } from 'lodash';
import { PrivateLocationAttributes } from '../../runtime_types/private_locations';
import { getPrivateLocationsForMonitor } from '../monitor_cruds/add_monitor/utils';
import { SyntheticsRestApiRouteFactory } from '../types';
Expand All @@ -31,6 +32,9 @@ export const runOnceSyntheticsMonitorRoute: SyntheticsRestApiRouteFactory = () =
}): Promise<any> => {
const monitor = request.body as MonitorFields;
const { monitorId } = request.params;
if (isEmpty(monitor)) {
return response.badRequest({ body: { message: 'Monitor data is empty.' } });
}

const validationResult = validateMonitor(monitor);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
*/
import { schema } from '@kbn/config-schema';
import { v4 as uuidv4 } from 'uuid';
import { SavedObjectsErrorHelpers } from '@kbn/core-saved-objects-server';
import { IKibanaResponse } from '@kbn/core-http-server';
import { getDecryptedMonitor } from '../../saved_objects/synthetics_monitor';
import { PrivateLocationAttributes } from '../../runtime_types/private_locations';
import { RouteContext, SyntheticsRestApiRouteFactory } from '../types';
Expand All @@ -14,6 +16,7 @@ import { ConfigKey, MonitorFields } from '../../../common/runtime_types';
import { SYNTHETICS_API_URLS } from '../../../common/constants';
import { normalizeSecrets } from '../../synthetics_service/utils/secrets';
import { getPrivateLocationsForMonitor } from '../monitor_cruds/add_monitor/utils';
import { getMonitorNotFoundResponse } from './service_errors';

export const testNowMonitorRoute: SyntheticsRestApiRouteFactory<TestNowResponse> = () => ({
method: 'POST',
Expand All @@ -33,48 +36,56 @@ export const testNowMonitorRoute: SyntheticsRestApiRouteFactory<TestNowResponse>
export const triggerTestNow = async (
monitorId: string,
routeContext: RouteContext
): Promise<TestNowResponse> => {
const { server, spaceId, syntheticsMonitorClient, savedObjectsClient } = routeContext;
): Promise<TestNowResponse | IKibanaResponse<any>> => {
const { server, spaceId, syntheticsMonitorClient, savedObjectsClient, response } = routeContext;

const monitorWithSecrets = await getDecryptedMonitor(server, monitorId, spaceId);
const normalizedMonitor = normalizeSecrets(monitorWithSecrets);
try {
const monitorWithSecrets = await getDecryptedMonitor(server, monitorId, spaceId);
const normalizedMonitor = normalizeSecrets(monitorWithSecrets);

const { [ConfigKey.SCHEDULE]: schedule, [ConfigKey.LOCATIONS]: locations } =
monitorWithSecrets.attributes;
const { [ConfigKey.SCHEDULE]: schedule, [ConfigKey.LOCATIONS]: locations } =
monitorWithSecrets.attributes;

const privateLocations: PrivateLocationAttributes[] = await getPrivateLocationsForMonitor(
savedObjectsClient,
normalizedMonitor.attributes
);
const testRunId = uuidv4();
const privateLocations: PrivateLocationAttributes[] = await getPrivateLocationsForMonitor(
savedObjectsClient,
normalizedMonitor.attributes
);
const testRunId = uuidv4();

const [, errors] = await syntheticsMonitorClient.testNowConfigs(
{
monitor: normalizedMonitor.attributes as MonitorFields,
id: monitorId,
testRunId,
},
savedObjectsClient,
privateLocations,
spaceId
);
const [, errors] = await syntheticsMonitorClient.testNowConfigs(
{
monitor: normalizedMonitor.attributes as MonitorFields,
id: monitorId,
testRunId,
},
savedObjectsClient,
privateLocations,
spaceId
);

if (errors && errors?.length > 0) {
return {
errors,
testRunId,
schedule,
locations,
configId: monitorId,
monitor: normalizedMonitor.attributes,
};
}

if (errors && errors?.length > 0) {
return {
errors,
testRunId,
schedule,
locations,
configId: monitorId,
monitor: normalizedMonitor.attributes,
};
}
} catch (getErr) {
if (SavedObjectsErrorHelpers.isNotFoundError(getErr)) {
return getMonitorNotFoundResponse(response, monitorId);
}

return {
testRunId,
schedule,
locations,
configId: monitorId,
monitor: normalizedMonitor.attributes,
};
throw getErr;
}
};
23 changes: 8 additions & 15 deletions x-pack/plugins/observability_solution/synthetics/server/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export const initSyntheticsServer = (
) => {
const { router } = server;
syntheticsAppRestApiRoutes.forEach((route) => {
const { method, options, handler, validate, path } = syntheticsRouteWrapper(
const { method, options, handler, validate, path, security } = syntheticsRouteWrapper(
createSyntheticsRouteWithAuth(route),
server,
syntheticsMonitorClient
Expand All @@ -30,6 +30,7 @@ export const initSyntheticsServer = (
const routeDefinition = {
path,
validate,
security,
options,
};

Expand All @@ -52,11 +53,8 @@ export const initSyntheticsServer = (
});

syntheticsAppPublicRestApiRoutes.forEach((route) => {
const { method, options, handler, validate, path, validation } = syntheticsRouteWrapper(
createSyntheticsRouteWithAuth(route),
server,
syntheticsMonitorClient
);
const { method, options, handler, validate, path, validation, security } =
syntheticsRouteWrapper(createSyntheticsRouteWithAuth(route), server, syntheticsMonitorClient);

const routeDefinition = {
path,
Expand All @@ -70,13 +68,11 @@ export const initSyntheticsServer = (
.get({
access: 'public',
path: routeDefinition.path,
options: {
tags: options?.tags,
},
})
.addVersion(
{
version: '2023-10-31',
security,
validate: validation ?? false,
},
handler
Expand All @@ -87,13 +83,11 @@ export const initSyntheticsServer = (
.put({
access: 'public',
path: routeDefinition.path,
options: {
tags: options?.tags,
},
})
.addVersion(
{
version: '2023-10-31',
security,
validate: validation ?? false,
},
handler
Expand All @@ -104,13 +98,11 @@ export const initSyntheticsServer = (
.post({
access: 'public',
path: routeDefinition.path,
options: {
tags: options?.tags,
},
})
.addVersion(
{
version: '2023-10-31',
security,
validate: validation ?? false,
},
handler
Expand All @@ -128,6 +120,7 @@ export const initSyntheticsServer = (
.addVersion(
{
version: '2023-10-31',
security,
validate: validation ?? false,
},
handler
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,13 @@ export const syntheticsRouteWrapper: SyntheticsRouteWrapper = (
) => ({
...uptimeRoute,
options: {
tags: ['access:uptime-read', ...(uptimeRoute?.writeAccess ? ['access:uptime-write'] : [])],
...(uptimeRoute.options ?? {}),
},
security: {
authz: {
requiredPrivileges: ['uptime-read', ...(uptimeRoute?.writeAccess ? ['uptime-write'] : [])],
},
},
handler: async (context, request, response) => {
const { elasticsearch, savedObjects, uiSettings } = await context.core;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,12 @@ import { UptimeEsClient } from '../lib/lib';
export const uptimeRouteWrapper: UMKibanaRouteWrapper = (uptimeRoute, server) => ({
...uptimeRoute,
options: {
tags: [
'oas-tag:uptime',
'access:uptime-read',
...(uptimeRoute?.writeAccess ? ['access:uptime-write'] : []),
],
tags: ['oas-tag:uptime'],
},
security: {
authz: {
requiredPrivileges: ['uptime-read', ...(uptimeRoute?.writeAccess ? ['uptime-write'] : [])],
},
},
handler: async (context, request, response) => {
const coreContext = await context.core;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export const initUptimeServer = (
router: UptimeRouter
) => {
legacyUptimeRestApiRoutes.forEach((route) => {
const { method, options, handler, validate, path } = uptimeRouteWrapper(
const { method, options, handler, validate, path, security } = uptimeRouteWrapper(
createRouteWithAuth(libs, route),
server
);
Expand All @@ -50,6 +50,7 @@ export const initUptimeServer = (
path,
validate,
options,
security,
};

switch (method) {
Expand All @@ -71,33 +72,28 @@ export const initUptimeServer = (
});

legacyUptimePublicRestApiRoutes.forEach((route) => {
const { method, options, handler, path, ...rest } = uptimeRouteWrapper(
const { method, options, handler, path, security, ...rest } = uptimeRouteWrapper(
createRouteWithAuth(libs, route),
server
);

const validate = rest.validate ? getRequestValidation(rest.validate) : rest.validate;

const routeDefinition = {
path,
validate,
options,
};

switch (method) {
case 'GET':
router.versioned
.get({
access: 'public',
description: `Get uptime settings`,
path: routeDefinition.path,
path,
options: {
tags: options?.tags,
},
})
.addVersion(
{
version: INITIAL_REST_VERSION,
security,
validate: {
request: {
body: validate ? validate?.body : undefined,
Expand All @@ -117,14 +113,15 @@ export const initUptimeServer = (
.put({
access: 'public',
description: `Update uptime settings`,
path: routeDefinition.path,
path,
options: {
tags: options?.tags,
},
})
.addVersion(
{
version: INITIAL_REST_VERSION,
security,
validate: {
request: {
body: validate ? validate?.body : undefined,
Expand Down
4 changes: 3 additions & 1 deletion x-pack/test/api_integration/apis/synthetics/add_monitor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,9 @@ export default function ({ getService }: FtrProviderContext) {
.send(httpMonitorJson);

expect(apiResponse.status).eql(403);
expect(apiResponse.body.message).eql('Forbidden');
expect(apiResponse.body.message).eql(
'API [POST /api/synthetics/monitors] is unauthorized for user, this action is granted by the Kibana privileges [uptime-write]'
);
});
});

Expand Down
Loading

0 comments on commit 44be16f

Please sign in to comment.