Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Cases] Add RBAC to remaining Cases APIs #98762

Merged
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
39b3950
Starting rbac for comments
jonathan-buttner Apr 20, 2021
6fb28ce
Adding authorization to rest of comment apis
jonathan-buttner Apr 21, 2021
2e0239c
Starting the comment rbac tests
jonathan-buttner Apr 21, 2021
5a63e2e
Fixing some of the rbac tests
jonathan-buttner Apr 22, 2021
a2c7842
Adding some integration tests
jonathan-buttner Apr 23, 2021
f8dd425
Starting patch tests
jonathan-buttner Apr 26, 2021
d670b1c
Merge branch 'cases-rbac-poc' of github.com:elastic/kibana into attac…
jonathan-buttner Apr 26, 2021
ba380e9
Working tests for comments
jonathan-buttner Apr 26, 2021
7734644
Working tests
jonathan-buttner Apr 26, 2021
86180fa
Fixing some tests
jonathan-buttner Apr 27, 2021
eb26150
Merge branch 'cases-rbac-poc' of github.com:elastic/kibana into attac…
jonathan-buttner Apr 27, 2021
3d4726d
Fixing type issues from pulling in master
jonathan-buttner Apr 27, 2021
518db99
Fixing connector tests that only work in trial license
jonathan-buttner Apr 27, 2021
a32b1ba
Attempting to fix cypress
jonathan-buttner Apr 27, 2021
28f8b62
Mock return of array for configure
jonathan-buttner Apr 27, 2021
7e2778c
Fixing cypress test
jonathan-buttner Apr 27, 2021
221d17c
Cleaning up
jonathan-buttner Apr 28, 2021
de78d74
Working case update tests
jonathan-buttner Apr 29, 2021
d048e07
Addressing PR comments
jonathan-buttner Apr 29, 2021
f7ae701
Reducing operations
jonathan-buttner Apr 29, 2021
5e46358
Working rbac push case tests
jonathan-buttner Apr 29, 2021
6cfd24f
Merge branch 'attachments-rbac' into remaining-rbac-apis
jonathan-buttner Apr 29, 2021
efc6e82
Starting stats apis
jonathan-buttner Apr 29, 2021
0d39f83
Merge branch 'cases-rbac-poc' of github.com:elastic/kibana into remai…
jonathan-buttner Apr 30, 2021
8543b5b
Working status tests
jonathan-buttner Apr 30, 2021
8841e65
User action tests and fixing migration errors
jonathan-buttner Apr 30, 2021
ba7df51
Fixing type errors
jonathan-buttner May 3, 2021
fbf5784
including error in message
jonathan-buttner May 3, 2021
9fae3d9
Addressing pr feedback
jonathan-buttner May 3, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions x-pack/plugins/cases/common/api/cases/case.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { CommentResponseRt } from './comment';
import { CasesStatusResponseRt, CaseStatusRt } from './status';
import { CaseConnectorRt, ESCaseConnector } from '../connectors';
import { SubCaseResponseRt } from './sub_case';
import { OWNER_FIELD } from './constants';

export enum CaseType {
collection = 'collection',
Expand All @@ -38,8 +39,7 @@ const CaseBasicRt = rt.type({
[caseTypeField]: CaseTypeRt,
connector: CaseConnectorRt,
settings: SettingsRt,
// TODO: should a user be able to update the owner?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are not allowing the owner field to be updated.

owner: rt.string,
[OWNER_FIELD]: rt.string,
});

const CaseExternalServiceBasicRt = rt.type({
Expand Down Expand Up @@ -80,7 +80,7 @@ const CasePostRequestNoTypeRt = rt.type({
title: rt.string,
connector: CaseConnectorRt,
settings: SettingsRt,
owner: rt.string,
[OWNER_FIELD]: rt.string,
});

/**
Expand Down Expand Up @@ -115,7 +115,7 @@ export const CasesFindRequestRt = rt.partial({
searchFields: rt.union([rt.array(rt.string), rt.string]),
sortField: rt.string,
sortOrder: rt.union([rt.literal('desc'), rt.literal('asc')]),
owner: rt.union([rt.array(rt.string), rt.string]),
[OWNER_FIELD]: rt.union([rt.array(rt.string), rt.string]),
});

export const CaseResponseRt = rt.intersection([
Expand Down Expand Up @@ -177,7 +177,7 @@ export const ExternalServiceResponseRt = rt.intersection([
]);

export const AllTagsFindRequestRt = rt.partial({
owner: rt.union([rt.array(rt.string), rt.string]),
[OWNER_FIELD]: rt.union([rt.array(rt.string), rt.string]),
});

export const AllReportersFindRequestRt = AllTagsFindRequestRt;
Expand Down
7 changes: 4 additions & 3 deletions x-pack/plugins/cases/common/api/cases/comment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import * as rt from 'io-ts';
import { OWNER_FIELD } from './constants';
import { SavedObjectFindOptionsRt } from '../saved_object';

import { UserRT } from '../user';
Expand All @@ -28,7 +29,7 @@ export const CommentAttributesBasicRt = rt.type({
]),
created_at: rt.string,
created_by: UserRT,
owner: rt.string,
[OWNER_FIELD]: rt.string,
pushed_at: rt.union([rt.string, rt.null]),
pushed_by: rt.union([UserRT, rt.null]),
updated_at: rt.union([rt.string, rt.null]),
Expand All @@ -44,7 +45,7 @@ export enum CommentType {
export const ContextTypeUserRt = rt.type({
comment: rt.string,
type: rt.literal(CommentType.user),
owner: rt.string,
[OWNER_FIELD]: rt.string,
});

/**
Expand All @@ -60,7 +61,7 @@ export const AlertCommentRequestRt = rt.type({
id: rt.union([rt.string, rt.null]),
name: rt.union([rt.string, rt.null]),
}),
owner: rt.string,
[OWNER_FIELD]: rt.string,
});

const AttributesTypeUserRt = rt.intersection([ContextTypeUserRt, CommentAttributesBasicRt]);
Expand Down
11 changes: 7 additions & 4 deletions x-pack/plugins/cases/common/api/cases/configure.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,20 @@ import * as rt from 'io-ts';
import { UserRT } from '../user';
import { CaseConnectorRt, ConnectorMappingsRt, ESCaseConnector } from '../connectors';
import { OmitProp } from '../runtime_types';
import { OWNER_FIELD } from './constants';

// TODO: we will need to add this type rt.literal('close-by-third-party')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't need to be done this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naa, not for the rbac work. Looks like that TODO was added 7 months ago or so. I don't believe we support closing by third party right now anyway 🤷‍♂️

const ClosureTypeRT = rt.union([rt.literal('close-by-user'), rt.literal('close-by-pushing')]);

const CasesConfigureBasicRt = rt.type({
connector: CaseConnectorRt,
closure_type: ClosureTypeRT,
owner: rt.string,
[OWNER_FIELD]: rt.string,
});

const CasesConfigureBasicWithoutOwnerRt = rt.type(OmitProp(CasesConfigureBasicRt.props, 'owner'));
const CasesConfigureBasicWithoutOwnerRt = rt.type(
OmitProp(CasesConfigureBasicRt.props, OWNER_FIELD)
);

export const CasesConfigureRequestRt = CasesConfigureBasicRt;
export const CasesConfigurePatchRt = rt.intersection([
Expand All @@ -45,12 +48,12 @@ export const CaseConfigureResponseRt = rt.intersection([
id: rt.string,
version: rt.string,
error: rt.union([rt.string, rt.null]),
owner: rt.string,
[OWNER_FIELD]: rt.string,
}),
]);

export const GetConfigureFindRequestRt = rt.partial({
owner: rt.union([rt.array(rt.string), rt.string]),
[OWNER_FIELD]: rt.union([rt.array(rt.string), rt.string]),
});

export const CaseConfigureRequestParamsRt = rt.type({
Expand Down
11 changes: 11 additions & 0 deletions x-pack/plugins/cases/common/api/cases/constants.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/*
* 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.
*/

/**
* The field used for authorization in various entities within cases.
*/
export const OWNER_FIELD = 'owner';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using this so we don't have to use the owner string a number of different places in the types and when constructing saved object filters.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity, why this field. I feel like you could technically do this for id, version, and really any other common api field. Doesn't hurt anything, just not sure if it's necessary

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah true, I guess I was thinking this might be helpful for the places where we're constructing a filter string to avoid accidental spelling errors in the future 🤷‍♂️ it's probably not necessary.

1 change: 1 addition & 0 deletions x-pack/plugins/cases/common/api/cases/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ export * from './comment';
export * from './status';
export * from './user_actions';
export * from './sub_case';
export * from './constants';
2 changes: 2 additions & 0 deletions x-pack/plugins/cases/common/api/cases/sub_case.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import * as rt from 'io-ts';
import { NumberFromString } from '../saved_object';
import { UserRT } from '../user';
import { CommentResponseRt } from './comment';
import { OWNER_FIELD } from './constants';
import { CasesStatusResponseRt } from './status';
import { CaseStatusRt } from './status';

Expand All @@ -26,6 +27,7 @@ export const SubCaseAttributesRt = rt.intersection([
created_by: rt.union([UserRT, rt.null]),
updated_at: rt.union([rt.string, rt.null]),
updated_by: rt.union([UserRT, rt.null]),
[OWNER_FIELD]: rt.string,
}),
]);

Expand Down
4 changes: 3 additions & 1 deletion x-pack/plugins/cases/common/api/cases/user_actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import * as rt from 'io-ts';
import { OWNER_FIELD } from './constants';

import { UserRT } from '../user';

Expand All @@ -22,7 +23,7 @@ const UserActionFieldTypeRt = rt.union([
rt.literal('status'),
rt.literal('settings'),
rt.literal('sub_case'),
rt.literal('owner'),
rt.literal(OWNER_FIELD),
]);
const UserActionFieldRt = rt.array(UserActionFieldTypeRt);
const UserActionRt = rt.union([
Expand All @@ -41,6 +42,7 @@ const CaseUserActionBasicRT = rt.type({
action_by: UserRT,
new_value: rt.union([rt.string, rt.null]),
old_value: rt.union([rt.string, rt.null]),
[OWNER_FIELD]: rt.string,
});

const CaseUserActionResponseRT = rt.intersection([
Expand Down
3 changes: 2 additions & 1 deletion x-pack/plugins/cases/common/api/connectors/mappings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import * as rt from 'io-ts';
import { OWNER_FIELD } from '../cases/constants';

const ActionTypeRT = rt.union([
rt.literal('append'),
Expand All @@ -31,7 +32,7 @@ export const ConnectorMappingsAttributesRT = rt.type({

export const ConnectorMappingsRt = rt.type({
mappings: rt.array(ConnectorMappingsAttributesRT),
owner: rt.string,
[OWNER_FIELD]: rt.string,
});

export type ConnectorMappingsAttributes = rt.TypeOf<typeof ConnectorMappingsAttributesRT>;
Expand Down
27 changes: 27 additions & 0 deletions x-pack/plugins/cases/server/authorization/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
CASE_COMMENT_SAVED_OBJECT,
CASE_CONFIGURE_SAVED_OBJECT,
CASE_SAVED_OBJECT,
CASE_USER_ACTION_SAVED_OBJECT,
} from '../../common/constants';
import { Verbs, ReadOperations, WriteOperations, OperationDetails } from './types';

Expand Down Expand Up @@ -101,6 +102,14 @@ export const Operations: Record<ReadOperations | WriteOperations, OperationDetai
docType: 'case',
savedObjectType: CASE_SAVED_OBJECT,
},
[WriteOperations.PushCase]: {
type: EVENT_TYPES.change,
name: WriteOperations.PushCase,
action: 'push-case',
verbs: updateVerbs,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit?: Should the push have separate verbs defined? 'push, pushing, pushed'?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm 🤔 good point, right now the verbs are tied to the valid ECS event types that we use access, creation, deletion, change (push isn't a valid one). Pushing a case does act as a change as it could close the case and it changes the push_at timestamp. I think for now I'm going to stick with the update verbs but I'll make a note of it and discuss it with Christos when he gets back.

docType: 'case',
savedObjectType: CASE_SAVED_OBJECT,
},
[WriteOperations.CreateConfiguration]: {
type: EVENT_TYPES.creation,
name: WriteOperations.CreateConfiguration,
Expand Down Expand Up @@ -214,4 +223,22 @@ export const Operations: Record<ReadOperations | WriteOperations, OperationDetai
docType: 'comments',
savedObjectType: CASE_COMMENT_SAVED_OBJECT,
},
// stats operations
[ReadOperations.GetCaseStatuses]: {
type: EVENT_TYPES.access,
name: ACCESS_CASE_OPERATION,
action: 'find-case-statuses',
verbs: accessVerbs,
docType: 'cases',
savedObjectType: CASE_SAVED_OBJECT,
},
// user actions operations
[ReadOperations.GetUserActions]: {
type: EVENT_TYPES.access,
name: ReadOperations.GetUserActions,
action: 'get-user-actions',
verbs: accessVerbs,
docType: 'user actions',
savedObjectType: CASE_USER_ACTION_SAVED_OBJECT,
},
};
3 changes: 3 additions & 0 deletions x-pack/plugins/cases/server/authorization/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,14 @@ export type GetSpaceFn = (request: KibanaRequest) => Promise<Space | undefined>;
export enum ReadOperations {
GetCase = 'getCase',
FindCases = 'findCases',
GetCaseStatuses = 'getCaseStatuses',
GetComment = 'getComment',
GetAllComments = 'getAllComments',
FindComments = 'findComments',
GetTags = 'getTags',
GetReporters = 'getReporters',
FindConfigurations = 'findConfigurations',
GetUserActions = 'getUserActions',
}

/**
Expand All @@ -47,6 +49,7 @@ export enum WriteOperations {
CreateCase = 'createCase',
DeleteCase = 'deleteCase',
UpdateCase = 'updateCase',
PushCase = 'pushCase',
CreateComment = 'createComment',
DeleteAllComments = 'deleteAllComments',
DeleteComment = 'deleteComment',
Expand Down
5 changes: 3 additions & 2 deletions x-pack/plugins/cases/server/authorization/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@

import { remove, uniq } from 'lodash';
import { nodeBuilder, KueryNode } from '../../../../../src/plugins/data/common';
import { OWNER_FIELD } from '../../common/api';

export const getOwnersFilter = (savedObjectType: string, owners: string[]): KueryNode => {
return nodeBuilder.or(
owners.reduce<KueryNode[]>((query, owner) => {
ensureFieldIsSafeForQuery('owner', owner);
ensureFieldIsSafeForQuery(OWNER_FIELD, owner);
query.push(nodeBuilder.is(`${savedObjectType}.attributes.owner`, owner));
return query;
}, [])
Expand Down Expand Up @@ -53,5 +54,5 @@ export const includeFieldsRequiredForAuthentication = (fields?: string[]): strin
if (fields === undefined) {
return;
}
return uniq([...fields, 'owner']);
return uniq([...fields, OWNER_FIELD]);
};
3 changes: 3 additions & 0 deletions x-pack/plugins/cases/server/client/attachments/add.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ async function getSubCase({
subCaseId: newSubCase.id,
fields: ['status', 'sub_case'],
newValue: JSON.stringify({ status: newSubCase.attributes.status }),
owner: newSubCase.attributes.owner,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding the owner to user actions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you do end up keeping the owner constant, it's probably worth updating these to use it as well

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah good point

}),
],
});
Expand Down Expand Up @@ -222,6 +223,7 @@ const addGeneratedAlerts = async (
commentId: newComment.id,
fields: ['comment'],
newValue: JSON.stringify(query),
owner: newComment.attributes.owner,
}),
],
});
Expand Down Expand Up @@ -396,6 +398,7 @@ export const addComment = async (
commentId: newComment.id,
fields: ['comment'],
newValue: JSON.stringify(query),
owner: newComment.attributes.owner,
}),
],
});
Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/cases/server/client/attachments/delete.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ export async function deleteAll(
subCaseId: subCaseID,
commentId: comment.id,
fields: ['comment'],
owner: comment.attributes.owner,
Copy link
Contributor

@michaelolo24 michaelolo24 May 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here: [OWNER_FIELD]? You get the idea, not gonna add more comments after this one 😂

})
),
});
Expand Down Expand Up @@ -167,6 +168,7 @@ export async function deleteComment(
subCaseId: subCaseID,
commentId: attachmentID,
fields: ['comment'],
owner: myComment.attributes.owner,
}),
],
});
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/cases/server/client/attachments/update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ export async function update(
// myComment.attribute contains also CommentAttributesBasicRt attributes
pick(Object.keys(queryRestAttributes), myComment.attributes)
),
owner: myComment.attributes.owner,
}),
],
});
Expand Down
4 changes: 3 additions & 1 deletion x-pack/plugins/cases/server/client/cases/create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
CasesClientPostRequestRt,
CasePostRequest,
CaseType,
OWNER_FIELD,
} from '../../../common/api';
import { buildCaseUserActionItem } from '../../services/user_actions/helpers';
import { ensureAuthorized, getConnectorFromConfiguration } from '../utils';
Expand Down Expand Up @@ -108,8 +109,9 @@ export const create = async (
actionAt: createdDate,
actionBy: { username, full_name, email },
caseId: newCase.id,
fields: ['description', 'status', 'tags', 'title', 'connector', 'settings', 'owner'],
fields: ['description', 'status', 'tags', 'title', 'connector', 'settings', OWNER_FIELD],
newValue: JSON.stringify(query),
owner: newCase.attributes.owner,
}),
],
});
Expand Down
8 changes: 5 additions & 3 deletions x-pack/plugins/cases/server/client/cases/delete.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { AttachmentService, CaseService } from '../../services';
import { buildCaseUserActionItem } from '../../services/user_actions/helpers';
import { Operations } from '../../authorization';
import { ensureAuthorized } from '../utils';
import { OWNER_FIELD } from '../../../common/api';

async function deleteSubCases({
attachmentService,
Expand Down Expand Up @@ -133,23 +134,24 @@ export async function deleteCases(ids: string[], clientArgs: CasesClientArgs): P

await userActionService.bulkCreate({
soClient,
actions: ids.map((id) =>
actions: cases.saved_objects.map((caseInfo) =>
buildCaseUserActionItem({
action: 'delete',
actionAt: deleteDate,
actionBy: user,
caseId: id,
caseId: caseInfo.id,
fields: [
'description',
'status',
'tags',
'title',
'connector',
'settings',
'owner',
OWNER_FIELD,
'comment',
...(ENABLE_CASE_CONNECTOR ? ['sub_case'] : []),
],
owner: caseInfo.attributes.owner,
})
),
});
Expand Down
Loading