Skip to content

Commit

Permalink
[8.13] [Synthetics] Simplify write access default behavior (elastic#1…
Browse files Browse the repository at this point in the history
…77088) (elastic#177228)

# Backport

This will backport the following commits from `main` to `8.13`:
- [[Synthetics] Simplify write access default behavior
(elastic#177088)](elastic#177088)

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

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

<!--BACKPORT [{"author":{"name":"Justin
Kambic","email":"jk@elastic.co"},"sourceCommit":{"committedDate":"2024-02-19T18:11:03Z","message":"[Synthetics]
Simplify write access default behavior (elastic#177088)\n\n##
Summary\r\n\r\nSimplifies the override functionality. Now, `writeAccess`
is the only\r\nflag controlling this. All non-GET routes are defaulted
to requiring\r\nwrite access. Also applies write access restriction to
the trigger\r\nroute, which is a GET.\r\n\r\n## Testing
instructions\r\n\r\nTest the override routes, and the default
behavior.\r\n\r\n```shell\r\n# Create a test user with user/pass:
testuser/testuser\r\n\r\n# Override: trigger route should return
403\r\ncurl -X GET
http://localhost:5601/internal/synthetics/service/monitors/trigger/{monitorId}
-u testuser:testuser \r\n\r\n# Override: enablement route should work
for read user\r\ncurl -X PUT
http://localhost:5601/internal/synthetics/service/enablement -u
testuser:testuser -H \"kbn-xsrf: true\"\r\n\r\n# Override: screenshot
blocks should work\r\ncurl -X POST
http://localhost:5601/internal/synthetics/journey/screenshot/block -u
testuser:testuser -H \"kbn-xsrf: true\"\r\n\r\n# a normal GET route
returns 200\r\ncurl -X GET
http://localhost:5601/internal/synthetics/service/monitor/{monitorId} -u
testuser:testuser \r\n\r\n# a normal non-GET route returns 403\r\ncurl
-X POST
http://localhost:5601/internal/synthetics/enable_default_alerting -u
testuser:testuser -H \"kbn-xsrf:
true\"\r\n```","sha":"b8cdae452ef9e7c83b49832b07d30f69a56b5698","branchLabelMapping":{"^v8.14.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:fix","Team:obs-ux-infra_services","v8.13.0","v8.12.2","v8.14.0"],"title":"[Synthetics]
Simplify write access default
behavior","number":177088,"url":"https://github.com/elastic/kibana/pull/177088","mergeCommit":{"message":"[Synthetics]
Simplify write access default behavior (elastic#177088)\n\n##
Summary\r\n\r\nSimplifies the override functionality. Now, `writeAccess`
is the only\r\nflag controlling this. All non-GET routes are defaulted
to requiring\r\nwrite access. Also applies write access restriction to
the trigger\r\nroute, which is a GET.\r\n\r\n## Testing
instructions\r\n\r\nTest the override routes, and the default
behavior.\r\n\r\n```shell\r\n# Create a test user with user/pass:
testuser/testuser\r\n\r\n# Override: trigger route should return
403\r\ncurl -X GET
http://localhost:5601/internal/synthetics/service/monitors/trigger/{monitorId}
-u testuser:testuser \r\n\r\n# Override: enablement route should work
for read user\r\ncurl -X PUT
http://localhost:5601/internal/synthetics/service/enablement -u
testuser:testuser -H \"kbn-xsrf: true\"\r\n\r\n# Override: screenshot
blocks should work\r\ncurl -X POST
http://localhost:5601/internal/synthetics/journey/screenshot/block -u
testuser:testuser -H \"kbn-xsrf: true\"\r\n\r\n# a normal GET route
returns 200\r\ncurl -X GET
http://localhost:5601/internal/synthetics/service/monitor/{monitorId} -u
testuser:testuser \r\n\r\n# a normal non-GET route returns 403\r\ncurl
-X POST
http://localhost:5601/internal/synthetics/enable_default_alerting -u
testuser:testuser -H \"kbn-xsrf:
true\"\r\n```","sha":"b8cdae452ef9e7c83b49832b07d30f69a56b5698"}},"sourceBranch":"main","suggestedTargetBranches":["8.13","8.12"],"targetPullRequestStates":[{"branch":"8.13","label":"v8.13.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.12","label":"v8.12.2","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.14.0","branchLabelMappingKey":"^v8.14.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/177088","number":177088,"mergeCommit":{"message":"[Synthetics]
Simplify write access default behavior (elastic#177088)\n\n##
Summary\r\n\r\nSimplifies the override functionality. Now, `writeAccess`
is the only\r\nflag controlling this. All non-GET routes are defaulted
to requiring\r\nwrite access. Also applies write access restriction to
the trigger\r\nroute, which is a GET.\r\n\r\n## Testing
instructions\r\n\r\nTest the override routes, and the default
behavior.\r\n\r\n```shell\r\n# Create a test user with user/pass:
testuser/testuser\r\n\r\n# Override: trigger route should return
403\r\ncurl -X GET
http://localhost:5601/internal/synthetics/service/monitors/trigger/{monitorId}
-u testuser:testuser \r\n\r\n# Override: enablement route should work
for read user\r\ncurl -X PUT
http://localhost:5601/internal/synthetics/service/enablement -u
testuser:testuser -H \"kbn-xsrf: true\"\r\n\r\n# Override: screenshot
blocks should work\r\ncurl -X POST
http://localhost:5601/internal/synthetics/journey/screenshot/block -u
testuser:testuser -H \"kbn-xsrf: true\"\r\n\r\n# a normal GET route
returns 200\r\ncurl -X GET
http://localhost:5601/internal/synthetics/service/monitor/{monitorId} -u
testuser:testuser \r\n\r\n# a normal non-GET route returns 403\r\ncurl
-X POST
http://localhost:5601/internal/synthetics/enable_default_alerting -u
testuser:testuser -H \"kbn-xsrf:
true\"\r\n```","sha":"b8cdae452ef9e7c83b49832b07d30f69a56b5698"}}]}]
BACKPORT-->

Co-authored-by: Justin Kambic <jk@elastic.co>
  • Loading branch information
kibanamachine and justinkambic authored Feb 19, 2024
1 parent a011ab4 commit 1dc7a70
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,37 @@
*/

import { createSyntheticsRouteWithAuth } from './create_route_with_auth';
import { SupportedMethod } from './types';

const methods: SupportedMethod[][] = [['GET'], ['POST'], ['PUT'], ['DELETE']];

describe('createSyntheticsRouteWithAuth', () => {
it('should create a route with auth', () => {
it.each(
methods
.map<[SupportedMethod, boolean]>((m) => [m[0], true])
.concat(methods.map((m) => [m[0], false]))
)('%s methods continues to support the writeAccess %s flag', (mStr, writeAccess) => {
const method: SupportedMethod = mStr as SupportedMethod;
const route = createSyntheticsRouteWithAuth(() => ({
method,
path: '/foo',
validate: {},
writeAccess,
handler: async () => {
return { success: true };
},
}));

expect(route).toEqual({
method,
path: '/foo',
validate: {},
handler: expect.any(Function),
writeAccess,
});
});

it('by default allows read access for GET by default', () => {
const route = createSyntheticsRouteWithAuth(() => ({
method: 'GET',
path: '/foo',
Expand All @@ -27,11 +55,9 @@ describe('createSyntheticsRouteWithAuth', () => {
});
});

it.each([['POST'], ['PUT'], ['DELETE']])(
'requires write permissions for %s requests',
it.each(methods.filter((m) => m[0] !== 'GET'))(
'by default requires write access for %s route requests',
(method) => {
if (method !== 'POST' && method !== 'PUT' && method !== 'DELETE')
throw Error('Invalid method');
const route = createSyntheticsRouteWithAuth(() => ({
method,
path: '/foo',
Expand All @@ -50,29 +76,4 @@ describe('createSyntheticsRouteWithAuth', () => {
});
}
);

it.each([['POST'], ['PUT'], ['DELETE']])(
'allows write access override for %s requests',
(method) => {
if (method !== 'POST' && method !== 'PUT' && method !== 'DELETE')
throw Error('Invalid method');
const route = createSyntheticsRouteWithAuth(() => ({
method,
path: '/foo',
validate: {},
handler: async () => {
return { success: true };
},
writeAccessOverride: true,
}));

expect(route).toEqual({
method,
path: '/foo',
validate: {},
handler: expect.any(Function),
writeAccess: undefined,
});
}
);
});
19 changes: 11 additions & 8 deletions x-pack/plugins/synthetics/server/routes/create_route_with_auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,23 @@ import {
LICENSE_NOT_ACTIVE_ERROR,
LICENSE_NOT_SUPPORTED_ERROR,
} from '../../common/constants';
import { SyntheticsRestApiRouteFactory, SyntheticsRoute, SyntheticsRouteHandler } from './types';
import {
SupportedMethod,
SyntheticsRestApiRouteFactory,
SyntheticsRoute,
SyntheticsRouteHandler,
} from './types';

function getWriteAccessFlag(method: string, writeAccessOverride?: boolean, writeAccess?: boolean) {
// if route includes an override, skip write-only access with `undefined`
// otherwise, if route is not a GET, require write access
// if route is get, use writeAccess value with `false` as default
return writeAccessOverride === true ? undefined : method !== 'GET' ? true : writeAccess ?? false;
function getDefaultWriteAccessFlag(method: SupportedMethod) {
// if the method is not GET, it defaults to requiring write access
return method !== 'GET';
}

export const createSyntheticsRouteWithAuth = <ClientContract = unknown>(
routeCreator: SyntheticsRestApiRouteFactory
): SyntheticsRoute<ClientContract> => {
const restRoute = routeCreator();
const { handler, method, path, options, writeAccess, writeAccessOverride, ...rest } = restRoute;
const { handler, method, path, options, writeAccess, ...rest } = restRoute;
const licenseCheckHandler: SyntheticsRouteHandler<ClientContract> = async ({
context,
response,
Expand Down Expand Up @@ -56,7 +59,7 @@ export const createSyntheticsRouteWithAuth = <ClientContract = unknown>(
options,
handler: licenseCheckHandler,
...rest,
writeAccess: getWriteAccessFlag(method, writeAccessOverride, writeAccess),
writeAccess: writeAccess ?? getDefaultWriteAccessFlag(method),
};
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export const createJourneyScreenshotBlocksRoute: SyntheticsRestApiRouteFactory =
hashes: schema.arrayOf(schema.string()),
}),
},
writeAccessOverride: true,
writeAccess: false,
handler: async (routeProps) => {
return await journeyScreenshotBlocksHandler(routeProps);
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {
export const getSyntheticsEnablementRoute: SyntheticsRestApiRouteFactory = () => ({
method: 'PUT',
path: SYNTHETICS_API_URLS.SYNTHETICS_ENABLEMENT,
writeAccessOverride: true,
writeAccess: false,
validate: {},
handler: async ({ savedObjectsClient, request, server }): Promise<any> => {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export const testNowMonitorRoute: SyntheticsRestApiRouteFactory<TestNowResponse>
const { monitorId } = routeContext.request.params;
return triggerTestNow(monitorId, routeContext);
},
writeAccess: true,
});

export const triggerTestNow = async (
Expand Down
5 changes: 3 additions & 2 deletions x-pack/plugins/synthetics/server/routes/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,14 @@ export type SyntheticsRequest = KibanaRequest<
Record<string, any>
>;

export type SupportedMethod = 'GET' | 'POST' | 'PUT' | 'DELETE';

/**
* Defines the basic properties employed by Uptime routes.
*/
export interface UMServerRoute<T> {
method: 'GET' | 'PUT' | 'POST' | 'DELETE';
method: SupportedMethod;
writeAccess?: boolean;
writeAccessOverride?: boolean;
handler: T;
validation?: FullValidationConfig<any, any, any>;
streamHandler?: (
Expand Down

0 comments on commit 1dc7a70

Please sign in to comment.