From ead6edfa4ed15884fa674226ac4c6111ab0c03db Mon Sep 17 00:00:00 2001 From: Jonathan Buttner Date: Thu, 3 Jun 2021 15:44:12 -0400 Subject: [PATCH 1/2] Adding tests for space permissions --- .../tests/common/cases/delete_cases.ts | 25 ++++++++++++--- .../tests/common/comments/delete_comment.ts | 31 +++++++++++++++++++ .../tests/common/configure/patch_configure.ts | 26 ++++++++++++++++ .../tests/common/comments/get_comment.ts | 2 +- 4 files changed, 79 insertions(+), 5 deletions(-) diff --git a/x-pack/test/case_api_integration/security_and_spaces/tests/common/cases/delete_cases.ts b/x-pack/test/case_api_integration/security_and_spaces/tests/common/cases/delete_cases.ts index bbb9624c4b14be..964e9135aba7b7 100644 --- a/x-pack/test/case_api_integration/security_and_spaces/tests/common/cases/delete_cases.ts +++ b/x-pack/test/case_api_integration/security_and_spaces/tests/common/cases/delete_cases.ts @@ -285,10 +285,27 @@ export default ({ getService }: FtrProviderContext): void => { ); /** - * We expect a 404 because the bulkGet inside the delete - * route should return a 404 when requesting a case from - * a different space. - * */ + * secOnly does not have access to space2 so it should 403 + */ + await deleteCases({ + supertest: supertestWithoutAuth, + caseIDs: [postedCase.id], + expectedHttpCode: 403, + auth: { user: secOnly, space: 'space2' }, + }); + }); + + it('should NOT delete a case created in space2 by making a request to space1', async () => { + const postedCase = await createCase( + supertestWithoutAuth, + getPostCaseRequest({ owner: 'securitySolutionFixture' }), + 200, + { + user: superUser, + space: 'space2', + } + ); + await deleteCases({ supertest: supertestWithoutAuth, caseIDs: [postedCase.id], diff --git a/x-pack/test/case_api_integration/security_and_spaces/tests/common/comments/delete_comment.ts b/x-pack/test/case_api_integration/security_and_spaces/tests/common/comments/delete_comment.ts index 3336abfa47e7c4..fc0b62ff924b51 100644 --- a/x-pack/test/case_api_integration/security_and_spaces/tests/common/comments/delete_comment.ts +++ b/x-pack/test/case_api_integration/security_and_spaces/tests/common/comments/delete_comment.ts @@ -320,6 +320,37 @@ export default ({ getService }: FtrProviderContext): void => { auth: { user: superUser, space: 'space2' }, }); + await deleteComment({ + supertest: supertestWithoutAuth, + caseId: postedCase.id, + commentId: commentResp.comments![0].id, + auth: { user: secOnly, space: 'space2' }, + expectedHttpCode: 403, + }); + + await deleteAllComments({ + supertest: supertestWithoutAuth, + caseId: postedCase.id, + auth: { user: secOnly, space: 'space2' }, + expectedHttpCode: 403, + }); + }); + + it('should NOT delete a comment created in space2 by making a request to space1', async () => { + const postedCase = await createCase( + supertestWithoutAuth, + getPostCaseRequest({ owner: 'securitySolutionFixture' }), + 200, + { user: superUser, space: 'space2' } + ); + + const commentResp = await createComment({ + supertest: supertestWithoutAuth, + caseId: postedCase.id, + params: postCommentUserReq, + auth: { user: superUser, space: 'space2' }, + }); + await deleteComment({ supertest: supertestWithoutAuth, caseId: postedCase.id, diff --git a/x-pack/test/case_api_integration/security_and_spaces/tests/common/configure/patch_configure.ts b/x-pack/test/case_api_integration/security_and_spaces/tests/common/configure/patch_configure.ts index ced727f8e4e75e..323b1b377e5555 100644 --- a/x-pack/test/case_api_integration/security_and_spaces/tests/common/configure/patch_configure.ts +++ b/x-pack/test/case_api_integration/security_and_spaces/tests/common/configure/patch_configure.ts @@ -224,6 +224,32 @@ export default ({ getService }: FtrProviderContext): void => { } ); + await updateConfiguration( + supertestWithoutAuth, + configuration.id, + { + closure_type: 'close-by-pushing', + version: configuration.version, + }, + 403, + { + user: secOnly, + space: 'space2', + } + ); + }); + + it('should NOT update a configuration created in space2 by making a request to space1', async () => { + const configuration = await createConfiguration( + supertestWithoutAuth, + { ...getConfigurationRequest(), owner: 'securitySolutionFixture' }, + 200, + { + user: superUser, + space: 'space2', + } + ); + await updateConfiguration( supertestWithoutAuth, configuration.id, diff --git a/x-pack/test/case_api_integration/spaces_only/tests/common/comments/get_comment.ts b/x-pack/test/case_api_integration/spaces_only/tests/common/comments/get_comment.ts index b53b2e6e59cfb4..048700993087db 100644 --- a/x-pack/test/case_api_integration/spaces_only/tests/common/comments/get_comment.ts +++ b/x-pack/test/case_api_integration/spaces_only/tests/common/comments/get_comment.ts @@ -46,7 +46,7 @@ export default ({ getService }: FtrProviderContext): void => { expect(comment).to.eql(patchedCase.comments![0]); }); - it('should not get a comment in space2', async () => { + it('should not get a comment in space2 when it was created in space1', async () => { const postedCase = await createCase(supertest, postCaseReq, 200, authSpace1); const patchedCase = await createComment({ supertest, From f3293ad597d40ffff6b26889b373280b0177f792 Mon Sep 17 00:00:00 2001 From: Jonathan Buttner Date: Fri, 4 Jun 2021 12:00:05 -0400 Subject: [PATCH 2/2] Adding tests for testing a disable feature --- .../security_solution/server/plugin.ts | 33 +++++++++++++++++++ .../common/lib/authentication/roles.ts | 26 +++++++++++++++ .../common/lib/authentication/spaces.ts | 2 +- .../common/lib/authentication/users.ts | 8 +++++ .../case_api_integration/common/lib/utils.ts | 8 ++++- .../tests/common/cases/post_case.ts | 17 ++++++++++ .../tests/common/configure/post_configure.ts | 17 ++++++++++ 7 files changed, 109 insertions(+), 2 deletions(-) diff --git a/x-pack/test/case_api_integration/common/fixtures/plugins/security_solution/server/plugin.ts b/x-pack/test/case_api_integration/common/fixtures/plugins/security_solution/server/plugin.ts index 9083d65e9a3495..bd3569ef528164 100644 --- a/x-pack/test/case_api_integration/common/fixtures/plugins/security_solution/server/plugin.ts +++ b/x-pack/test/case_api_integration/common/fixtures/plugins/security_solution/server/plugin.ts @@ -54,6 +54,39 @@ export class FixturePlugin implements Plugin & { + overrides?: Record; +}; + export const getConfigurationRequest = ({ id = 'none', name = 'none', type = ConnectorTypes.none, fields = null, -}: Partial = {}): CasesConfigureRequest => { + overrides, +}: ConfigRequestParams = {}): CasesConfigureRequest => { return { connector: { id, @@ -288,6 +293,7 @@ export const getConfigurationRequest = ({ } as CaseConnector, closure_type: 'close-by-user', owner: 'securitySolutionFixture', + ...overrides, }; }; diff --git a/x-pack/test/case_api_integration/security_and_spaces/tests/common/cases/post_case.ts b/x-pack/test/case_api_integration/security_and_spaces/tests/common/cases/post_case.ts index 787ce533dbaf4d..e8337fa9db5023 100644 --- a/x-pack/test/case_api_integration/security_and_spaces/tests/common/cases/post_case.ts +++ b/x-pack/test/case_api_integration/security_and_spaces/tests/common/cases/post_case.ts @@ -32,6 +32,7 @@ import { obsOnlyRead, obsSecRead, noKibanaPrivileges, + testDisabled, } from '../../../../common/lib/authentication/users'; import { FtrProviderContext } from '../../../../common/ftr_provider_context'; @@ -240,6 +241,22 @@ export default ({ getService }: FtrProviderContext): void => { }); describe('rbac', () => { + it('returns a 403 when attempting to create a case with an owner that was from a disabled feature in the space', async () => { + const theCase = ((await createCase( + supertestWithoutAuth, + getPostCaseRequest({ owner: 'testDisabledFixture' }), + 403, + { + user: testDisabled, + space: 'space1', + } + )) as unknown) as { message: string }; + + expect(theCase.message).to.eql( + 'Unauthorized to create case with owners: "testDisabledFixture"' + ); + }); + it('User: security solution only - should create a case', async () => { const theCase = await createCase( supertestWithoutAuth, diff --git a/x-pack/test/case_api_integration/security_and_spaces/tests/common/configure/post_configure.ts b/x-pack/test/case_api_integration/security_and_spaces/tests/common/configure/post_configure.ts index f1dae9f319109b..44ec24f688f201 100644 --- a/x-pack/test/case_api_integration/security_and_spaces/tests/common/configure/post_configure.ts +++ b/x-pack/test/case_api_integration/security_and_spaces/tests/common/configure/post_configure.ts @@ -28,6 +28,7 @@ import { globalRead, obsSecRead, superUser, + testDisabled, } from '../../../../common/lib/authentication/users'; // eslint-disable-next-line import/no-default-export @@ -196,6 +197,22 @@ export default ({ getService }: FtrProviderContext): void => { }); describe('rbac', () => { + it('returns a 403 when attempting to create a configuration with an owner that was from a disabled feature in the space', async () => { + const configuration = ((await createConfiguration( + supertestWithoutAuth, + getConfigurationRequest({ overrides: { owner: 'testDisabledFixture' } }), + 403, + { + user: testDisabled, + space: 'space1', + } + )) as unknown) as { message: string }; + + expect(configuration.message).to.eql( + 'Unauthorized to create case configuration with owners: "testDisabledFixture"' + ); + }); + it('User: security solution only - should create a configuration', async () => { const configuration = await createConfiguration( supertestWithoutAuth,