[Cases] Attachments RBAC #97756
Merged
[Cases] Attachments RBAC #97756
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jonathan-buttner
added
v8.0.0
release_note:skip
| @@ -27,6 +28,7 @@ export const CommentAttributesBasicRt = rt.type({ | |||
| ]), | |||
| created_at: rt.string, | |||
| created_by: UserRT, | |||
| owner: rt.string, | |||
There was a problem hiding this comment.
I'm adding owner here even though that means that patch requests must include the owner. This is to keep it consistent with how patch was already working. All of the fields need to be included in the patch and we have checks to make sure that certain fields like type and owner aren't different in the request than what is already defined in the object.
There was a problem hiding this comment.
I'm wondering if we should make a type for owner like:
const OwnerTypeRT = rt.union([rt.literal('securitySolution'), rt.literal('observability')]);
Not for this PR, but I think it would be good to have strict control over that type and not just accept any old string.
| @@ -47,3 +48,7 @@ export const getSubCaseUserActionUrl = (caseID: string, subCaseId: string): stri | |||
| export const getCasePushUrl = (caseId: string, connectorId: string): string => { | |||
| return CASE_PUSH_URL.replace('{case_id}', caseId).replace('{connector_id}', connectorId); | |||
| }; | |||
|
|
|||
| export const getCaseConfigurationDetailsUrl = (configureID: string): string => { | |||
There was a problem hiding this comment.
This is so the UI can send the correct patch request with the configuration_id.
| @@ -51,9 +51,9 @@ export class AuthorizationAuditLogger { | |||
| message: `${username ?? 'unknown user'} ${message}`, | |||
| event: { | |||
| action: operation.action, | |||
| category: EventCategory.DATABASE, | |||
There was a problem hiding this comment.
The security plugin no longer defines these. They're using the ECS library directly now. So I defined some constants for us.
There was a problem hiding this comment.
maybe something to come back to. I defined some ECS in the RAC work we did (no overlap). maybe we should use library?
There was a problem hiding this comment.
Yeah that could be helpful. The struggle was that the ECS library defines types but not the values from what I could tell 🤷♂️ . I don't anticipate needing to add more than what was in this PR but if we do then yeah maybe a library would be a good option.
| if (myComment.attributes.type !== queryRestAttributes.type) { | ||
| throw Boom.badRequest(`You cannot change the type of the comment.`); | ||
| } | ||
|
|
||
| if (myComment.attributes.owner !== queryRestAttributes.owner) { |
There was a problem hiding this comment.
Don't allow changing the owner of a comment
| @@ -68,7 +68,7 @@ export class CasePlugin { | |||
| private securityPluginSetup?: SecurityPluginSetup; | |||
|
|
|||
| constructor(private readonly initializerContext: PluginInitializerContext) { | |||
| this.log = this.initializerContext.logger.get('plugins', 'cases'); | |||
There was a problem hiding this comment.
This causes the console logs to look like [plugin][plugin][cases][cases]. If we just do get() the duplicates are removed.
| it('should return an empty find body correctly if no connectors are loaded', async () => { | ||
| const connectors = await getCaseConnectors(supertest); | ||
| expect(connectors).to.eql([]); | ||
| }); | ||
|
|
||
| it('should return case owned connectors', async () => { |
There was a problem hiding this comment.
This test would fail with a basic license. It mostly already existed in a trial test but I added the service now connector.
| expect(body).key('connector'); | ||
| expect(body).not.key('connector_id'); | ||
| expect(body.connector).to.eql({ | ||
| expect(body.length).to.be(1); |
There was a problem hiding this comment.
The get configuration returns an array.
| @@ -67,113 +52,6 @@ export default ({ getService }: FtrProviderContext): void => { | |||
| expect(data).to.eql({ ...getConfigurationOutput(true), closure_type: 'close-by-pushing' }); | |||
| }); | |||
|
|
|||
| it('should patch a configuration connector and create mappings', async () => { | |||
There was a problem hiding this comment.
This test would fail with a basic license, moved to trial.
| @@ -73,53 +59,6 @@ export default ({ getService }: FtrProviderContext): void => { | |||
| expect(configuration.length).to.be(1); | |||
| }); | |||
|
|
|||
| it('should create a configuration with mapping', async () => { | |||
There was a problem hiding this comment.
This test would fail with a basic license, moved to trial.
| @@ -38,7 +40,7 @@ export default ({ getService }: FtrProviderContext): void => { | |||
| .set('kbn-xsrf', 'true') | |||
| .send({ | |||
| name: 'An email action', | |||
| actionTypeId: '.email', | |||
| connector_type_id: '.email', | |||
There was a problem hiding this comment.
Actions api changed a couple months ago from actionTypeId to connector_type_id.
| /** | ||
| * Creates an AuditEvent describing the state of a request. | ||
| */ | ||
| function createAuditMsg({ |
There was a problem hiding this comment.
Moved this and removed export.
| await authorization.ensureAuthorized(owners, operation); | ||
|
|
||
| // log that we're attempting an operation | ||
| logSavedObjects({ outcome: ECS_OUTCOMES.unknown }); |
There was a problem hiding this comment.
This utility function now handles logging after an ensureAuthorized call to remove duplication in each of the routes.
|
Pinging @elastic/security-threat-hunting (Team:Threat Hunting) |
Comment on lines
22
to
23
| filter: KueryNode | undefined, | ||
| authorizationFilter: KueryNode | undefined |
There was a problem hiding this comment.
nit
Suggested change
| filter: KueryNode | undefined, | |
| authorizationFilter: KueryNode | undefined | |
| filter?: KueryNode, | |
| authorizationFilter?: KueryNode |
| export const includeFieldsRequiredForAuthentication = (fields: string[]): string[] => | ||
| uniq([...fields, 'owner']); | ||
| export const includeFieldsRequiredForAuthentication = ( | ||
| fields: string[] | undefined |
There was a problem hiding this comment.
nit again
Suggested change
| fields: string[] | undefined | |
| fields?: string[] |
| /** | ||
| * Creates a KueryNode from a string expression. Returns undefined if the expression is undefined. | ||
| */ | ||
| export function stringToKueryNode(expression: string | undefined): KueryNode | undefined { |
There was a problem hiding this comment.
nit
Suggested change
| export function stringToKueryNode(expression: string | undefined): KueryNode | undefined { | |
| export function stringToKueryNode(expression?: string): KueryNode | undefined { |
| @@ -119,6 +119,11 @@ export class CommentableCase { | |||
| return this.subCase?.id; | |||
| } | |||
|
|
|||
| private get owner(): string { | |||
| // TODO: check for subCase?.attributes.owner here | |||
There was a problem hiding this comment.
intentionally left for todo?
There was a problem hiding this comment.
Good point, I'll add it to the list of sub cases work and remove the TODO.
| @@ -15,6 +15,9 @@ import { BaseFeaturePrivilegeBuilder } from './feature_privilege_builder'; | |||
| const readOperations: string[] = [ | |||
| 'getCase', | |||
| 'findCases', | |||
| 'getComment', | |||
There was a problem hiding this comment.
nit but would be much easier to compare by 👀 if alphabetized
jportner
reviewed
Apr 29, 2021
Comment on lines
18
to
20
| 'getComment', | ||
| 'getAllComments', | ||
| 'findComments', |
There was a problem hiding this comment.
Do we really need separate feature privileges for getComment, getAllComments, and findComments? Or, to rephrase: will there ever be a user who can get a single comment but not all comments? Will there ever be a user who can get comments but not find comments?
If not, we should probably consider removing getAllComments and findComments, and instead just use getComment to authorize for this operation. Every separate action/operation in the feature privileges definition has a performance impact for authorization checks against Elasticsearch.
There was a problem hiding this comment.
Thanks for the comment @jportner ! Great points. @legrego when we had initially talked a while ago my interpretation was that we needed an operation per route. I'm happy to remove the extra ones though.
Also I didn't realize that having separate actions/operations would result in worse performance. So the larger these arrays are the worse it'll be?
There was a problem hiding this comment.
My recommendation for an operation-per-route was based on following our current model for general saved objects authorization:
That said, we don't have an actual need for this level of granularity, since our privilege system only distinguishes between all and read today. At one point this setup simplified our audit logging, but that's only partially true now.
Every separate action/operation in the feature privileges definition has a performance impact for authorization checks against Elasticsearch.
Also I didn't realize that having separate actions/operations would result in worse performance. So the larger these arrays are the worse it'll be?
The performance issues are mostly hypothetical. There's an upper bound to everything, but we don't know what this is for our privileges yet. The payload we exchange with Elasticsearch to describe each or our privileges is quite large.
The work that ES does to perform the authorization checks is opaque to me - we again haven't run into issues yet, but I want to make sure we don't end up with a surprise that's hard to recover from.
So given all of that, I don't have a strong opinion for Cases RBAC either way. If you're comfortable reducing this down to a smaller set (and diverging from the way we currently do things), then I have no objections. If there is too much momentum around the current approach to refactor this now, then I'm ok to leave it as-is.
There was a problem hiding this comment.
Thanks for the response @legrego. Makes sense. It's pretty easy to do the changes @jportner asked for, so I'll refactor it down to just a getComment and deleteComment 👍 The way we've implemented the audit logging we can still log whether it was a find comment/get comment/get all comments etc. So we should be good there.
Comment on lines
30
to
31
| 'deleteAllComments', | ||
| 'deleteComment', |
There was a problem hiding this comment.
Same as #97756 (comment), if we don't need to differentiate then we should consider using a single deleteComment operation here.
| @@ -74,13 +74,15 @@ export const userActionPostResp: CasesClientPostRequest = { | |||
| export const postCommentUserReq: CommentRequestUserType = { | |||
| comment: 'This is a cool comment', | |||
| type: CommentType.user, | |||
| owner: 'securitySolutionFixture', | |||
There was a problem hiding this comment.
hmm I know this is test data but I'm confused. why Fixture? is there significance?
There was a problem hiding this comment.
yeah, this needs to match the value here: https://github.com/elastic/kibana/blob/cases-rbac-poc/x-pack/test/case_api_integration/common/fixtures/plugins/security_solution/server/plugin.ts#L43 if the user making the request has access to the security solution. So we're defaulting it to securitySolutionFixture since most tests use that. Some of the rbac tests specifically look for observabilityFixture in which case we change the owner before we make the request.
| @@ -19,10 +19,18 @@ export const getOwnersFilter = (savedObjectType: string, owners: string[]): Kuer | |||
| }; | |||
|
|
|||
| export const combineFilterWithAuthorizationFilter = ( | |||
| filter: KueryNode, | |||
| authorizationFilter: KueryNode | |||
| filter?: KueryNode, | |||
💚 Build SucceededMetrics [docs]
History
To update your PR or re-run it, just comment with: |
stephmilovic
approved these changes
Apr 29, 2021
jonathan-buttner
added a commit
that referenced
this pull request
Jun 7, 2021
* Adding feature flag for auth * Hiding SOs and adding consumer field * First pass at adding security changes * Consumer as the app's plugin ID * Create addConsumerToSO migration helper * Fix mapping's SO consumer * Add test for CasesActions * Declare hidden types on SO client * Restructure integration tests * Init spaces_only integration tests * Implementing the cases security string * Adding security plugin tests for cases * Rough concept for authorization class * Adding comments * Fix merge * Get requiredPrivileges for classes * Check privillages * Ensure that all classes are available * Success if hasAllRequested is true * Failure if hasAllRequested is false * Adding schema updates for feature plugin * Seperate basic from trial * Enable SIR on integration tests * Starting the plumbing for authorization in plugin * Unit tests working * Move find route logic to case client * Create integration test helper functions * Adding auth to create call * Create getClassFilter helper * Add class attribute to find request * Create getFindAuthorizationFilter * Ensure savedObject is authorized in find method * Include fields for authorization * Combine authorization filter with cases & subcases filter * Fix isAuthorized flag * Fix merge issue * Create/delete spaces & users before and after tests * Add more user and roles * [Cases] Convert filters from strings to KueryNode (#95288) * [Cases] RBAC: Rename class to scope (#95535) * [Cases][RBAC] Rename scope to owner (#96035) * [Cases] RBAC: Create & Find integration tests (#95511) * [Cases] Cases client enchantment (#95923) * [Cases] Authorization and Client Audit Logger (#95477) * Starting audit logger * Finishing auth audit logger * Fixing tests and types * Adding audit event creator * Renaming class to scope * Adding audit logger messages to create and find * Adding comments and fixing import issue * Fixing type errors * Fixing tests and adding username to message * Addressing PR feedback * Removing unneccessary log and generating id * Fixing module issue and remove expect.anything * [Cases] Migrate sub cases routes to a client (#96461) * Adding sub cases client * Move sub case routes to case client * Throw when attempting to access the sub cases client * Fixing throw and removing user ans soclients * [Cases] RBAC: Migrate routes' unit tests to integration tests (#96374) Co-authored-by: Jonathan Buttner <[email protected]> * [Cases] Move remaining HTTP functionality to client (#96507) * Moving deletes and find for attachments * Moving rest of comment apis * Migrating configuration routes to client * Finished moving routes, starting utils refactor * Refactoring utilites and fixing integration tests * Addressing PR feedback * Fixing mocks and types * Fixing integration tests * Renaming status_stats * Fixing test type errors * Adding plugins to kibana.json * Adding cases to required plugin * [Cases] Refactoring authorization (#97483) * Refactoring authorization * Wrapping auth calls in helper for try catch * Reverting name change * Hardcoding the saved object types * Switching ensure to owner array * [Cases] Add authorization to configuration & cases routes (#97228) * [Cases] Attachments RBAC (#97756) * Starting rbac for comments * Adding authorization to rest of comment apis * Starting the comment rbac tests * Fixing some of the rbac tests * Adding some integration tests * Starting patch tests * Working tests for comments * Working tests * Fixing some tests * Fixing type issues from pulling in master * Fixing connector tests that only work in trial license * Attempting to fix cypress * Mock return of array for configure * Fixing cypress test * Cleaning up * Addressing PR comments * Reducing operations * [Cases] Add RBAC to remaining Cases APIs (#98762) * Starting rbac for comments * Adding authorization to rest of comment apis * Starting the comment rbac tests * Fixing some of the rbac tests * Adding some integration tests * Starting patch tests * Working tests for comments * Working tests * Fixing some tests * Fixing type issues from pulling in master * Fixing connector tests that only work in trial license * Attempting to fix cypress * Mock return of array for configure * Fixing cypress test * Cleaning up * Working case update tests * Addressing PR comments * Reducing operations * Working rbac push case tests * Starting stats apis * Working status tests * User action tests and fixing migration errors * Fixing type errors * including error in message * Addressing pr feedback * Fixing some type errors * [Cases] Add space only tests (#99409) * Starting spaces tests * Finishing space only tests * Refactoring createCaseWithConnector * Fixing spelling * Addressing PR feedback and creating alert tests * Fixing mocks * [Cases] Add security only tests (#99679) * Starting spaces tests * Finishing space only tests * Refactoring createCaseWithConnector * Fixing spelling * Addressing PR feedback and creating alert tests * Fixing mocks * Starting security only tests * Adding remainder security only tests * Using helper objects * Fixing type error for null space * Renaming utility variables * Refactoring users and roles for security only tests * Adding sub feature * [Cases] Cleaning up the services and TODOs (#99723) * Cleaning up the service intialization * Fixing type errors * Adding comments for the api * Working test for cases client * Fix type error * Adding generated docs * Adding more docs and cleaning up types * Cleaning up readme * More clean up and links * Changing some file names * Renaming docs * Integration tests for cases privs and fixes (#100038) * [Cases] RBAC on UI (#99478) Co-authored-by: Kibana Machine <[email protected]> * Fixing case ids by alert id route call * [Cases] Fixing UI feature permissions and adding UI tests (#100074) * Integration tests for cases privs and fixes * Fixing ui cases permissions and adding tests * Adding test for collection failure and fixing jest * Renaming variables * Fixing type error * Adding some comments * Validate cases features * Fix new schema * Adding owner param for the status stats * Fix get case status tests * Adjusting permissions text and fixing status * Address PR feedback * Adding top level feature back * Fixing feature privileges * Renaming * Removing uneeded else * Fixing tests and adding cases merge tests * [Cases][Security Solution] Basic license security solution API tests (#100925) * Cleaning up the fixture plugins * Adding basic feature test * renaming to unsecuredSavedObjectsClient (#101215) * [Cases] RBAC Refactoring audit logging (#100952) * Refactoring audit logging * Adding unit tests for authorization classes * Addressing feedback and adding util tests * return undefined on empty array * fixing eslint * [Cases] Cleaning up RBAC integration tests (#101324) * Adding tests for space permissions * Adding tests for testing a disable feature Co-authored-by: Christos Nasikas <[email protected]> Co-authored-by: Kibana Machine <[email protected]>
jonathan-buttner
added a commit
to jonathan-buttner/kibana
that referenced
this pull request
Jun 7, 2021
* Adding feature flag for auth * Hiding SOs and adding consumer field * First pass at adding security changes * Consumer as the app's plugin ID * Create addConsumerToSO migration helper * Fix mapping's SO consumer * Add test for CasesActions * Declare hidden types on SO client * Restructure integration tests * Init spaces_only integration tests * Implementing the cases security string * Adding security plugin tests for cases * Rough concept for authorization class * Adding comments * Fix merge * Get requiredPrivileges for classes * Check privillages * Ensure that all classes are available * Success if hasAllRequested is true * Failure if hasAllRequested is false * Adding schema updates for feature plugin * Seperate basic from trial * Enable SIR on integration tests * Starting the plumbing for authorization in plugin * Unit tests working * Move find route logic to case client * Create integration test helper functions * Adding auth to create call * Create getClassFilter helper * Add class attribute to find request * Create getFindAuthorizationFilter * Ensure savedObject is authorized in find method * Include fields for authorization * Combine authorization filter with cases & subcases filter * Fix isAuthorized flag * Fix merge issue * Create/delete spaces & users before and after tests * Add more user and roles * [Cases] Convert filters from strings to KueryNode (elastic#95288) * [Cases] RBAC: Rename class to scope (elastic#95535) * [Cases][RBAC] Rename scope to owner (elastic#96035) * [Cases] RBAC: Create & Find integration tests (elastic#95511) * [Cases] Cases client enchantment (elastic#95923) * [Cases] Authorization and Client Audit Logger (elastic#95477) * Starting audit logger * Finishing auth audit logger * Fixing tests and types * Adding audit event creator * Renaming class to scope * Adding audit logger messages to create and find * Adding comments and fixing import issue * Fixing type errors * Fixing tests and adding username to message * Addressing PR feedback * Removing unneccessary log and generating id * Fixing module issue and remove expect.anything * [Cases] Migrate sub cases routes to a client (elastic#96461) * Adding sub cases client * Move sub case routes to case client * Throw when attempting to access the sub cases client * Fixing throw and removing user ans soclients * [Cases] RBAC: Migrate routes' unit tests to integration tests (elastic#96374) Co-authored-by: Jonathan Buttner <[email protected]> * [Cases] Move remaining HTTP functionality to client (elastic#96507) * Moving deletes and find for attachments * Moving rest of comment apis * Migrating configuration routes to client * Finished moving routes, starting utils refactor * Refactoring utilites and fixing integration tests * Addressing PR feedback * Fixing mocks and types * Fixing integration tests * Renaming status_stats * Fixing test type errors * Adding plugins to kibana.json * Adding cases to required plugin * [Cases] Refactoring authorization (elastic#97483) * Refactoring authorization * Wrapping auth calls in helper for try catch * Reverting name change * Hardcoding the saved object types * Switching ensure to owner array * [Cases] Add authorization to configuration & cases routes (elastic#97228) * [Cases] Attachments RBAC (elastic#97756) * Starting rbac for comments * Adding authorization to rest of comment apis * Starting the comment rbac tests * Fixing some of the rbac tests * Adding some integration tests * Starting patch tests * Working tests for comments * Working tests * Fixing some tests * Fixing type issues from pulling in master * Fixing connector tests that only work in trial license * Attempting to fix cypress * Mock return of array for configure * Fixing cypress test * Cleaning up * Addressing PR comments * Reducing operations * [Cases] Add RBAC to remaining Cases APIs (elastic#98762) * Starting rbac for comments * Adding authorization to rest of comment apis * Starting the comment rbac tests * Fixing some of the rbac tests * Adding some integration tests * Starting patch tests * Working tests for comments * Working tests * Fixing some tests * Fixing type issues from pulling in master * Fixing connector tests that only work in trial license * Attempting to fix cypress * Mock return of array for configure * Fixing cypress test * Cleaning up * Working case update tests * Addressing PR comments * Reducing operations * Working rbac push case tests * Starting stats apis * Working status tests * User action tests and fixing migration errors * Fixing type errors * including error in message * Addressing pr feedback * Fixing some type errors * [Cases] Add space only tests (elastic#99409) * Starting spaces tests * Finishing space only tests * Refactoring createCaseWithConnector * Fixing spelling * Addressing PR feedback and creating alert tests * Fixing mocks * [Cases] Add security only tests (elastic#99679) * Starting spaces tests * Finishing space only tests * Refactoring createCaseWithConnector * Fixing spelling * Addressing PR feedback and creating alert tests * Fixing mocks * Starting security only tests * Adding remainder security only tests * Using helper objects * Fixing type error for null space * Renaming utility variables * Refactoring users and roles for security only tests * Adding sub feature * [Cases] Cleaning up the services and TODOs (elastic#99723) * Cleaning up the service intialization * Fixing type errors * Adding comments for the api * Working test for cases client * Fix type error * Adding generated docs * Adding more docs and cleaning up types * Cleaning up readme * More clean up and links * Changing some file names * Renaming docs * Integration tests for cases privs and fixes (elastic#100038) * [Cases] RBAC on UI (elastic#99478) Co-authored-by: Kibana Machine <[email protected]> * Fixing case ids by alert id route call * [Cases] Fixing UI feature permissions and adding UI tests (elastic#100074) * Integration tests for cases privs and fixes * Fixing ui cases permissions and adding tests * Adding test for collection failure and fixing jest * Renaming variables * Fixing type error * Adding some comments * Validate cases features * Fix new schema * Adding owner param for the status stats * Fix get case status tests * Adjusting permissions text and fixing status * Address PR feedback * Adding top level feature back * Fixing feature privileges * Renaming * Removing uneeded else * Fixing tests and adding cases merge tests * [Cases][Security Solution] Basic license security solution API tests (elastic#100925) * Cleaning up the fixture plugins * Adding basic feature test * renaming to unsecuredSavedObjectsClient (elastic#101215) * [Cases] RBAC Refactoring audit logging (elastic#100952) * Refactoring audit logging * Adding unit tests for authorization classes * Addressing feedback and adding util tests * return undefined on empty array * fixing eslint * [Cases] Cleaning up RBAC integration tests (elastic#101324) * Adding tests for space permissions * Adding tests for testing a disable feature Co-authored-by: Christos Nasikas <[email protected]> Co-authored-by: Kibana Machine <[email protected]> # Conflicts: # x-pack/plugins/cases/server/client/client.ts # x-pack/plugins/cases/server/client/mocks.ts # x-pack/plugins/cases/server/client/types.ts # x-pack/plugins/cases/server/index.ts # x-pack/plugins/cases/server/plugin.ts # x-pack/plugins/security_solution/server/endpoint/endpoint_app_context_services.ts # x-pack/plugins/security_solution/server/endpoint/mocks.ts # x-pack/plugins/security_solution/server/endpoint/routes/actions/isolation.ts
jonathan-buttner
added a commit
that referenced
this pull request
Jun 7, 2021
* [Cases] RBAC (#95058) * Adding feature flag for auth * Hiding SOs and adding consumer field * First pass at adding security changes * Consumer as the app's plugin ID * Create addConsumerToSO migration helper * Fix mapping's SO consumer * Add test for CasesActions * Declare hidden types on SO client * Restructure integration tests * Init spaces_only integration tests * Implementing the cases security string * Adding security plugin tests for cases * Rough concept for authorization class * Adding comments * Fix merge * Get requiredPrivileges for classes * Check privillages * Ensure that all classes are available * Success if hasAllRequested is true * Failure if hasAllRequested is false * Adding schema updates for feature plugin * Seperate basic from trial * Enable SIR on integration tests * Starting the plumbing for authorization in plugin * Unit tests working * Move find route logic to case client * Create integration test helper functions * Adding auth to create call * Create getClassFilter helper * Add class attribute to find request * Create getFindAuthorizationFilter * Ensure savedObject is authorized in find method * Include fields for authorization * Combine authorization filter with cases & subcases filter * Fix isAuthorized flag * Fix merge issue * Create/delete spaces & users before and after tests * Add more user and roles * [Cases] Convert filters from strings to KueryNode (#95288) * [Cases] RBAC: Rename class to scope (#95535) * [Cases][RBAC] Rename scope to owner (#96035) * [Cases] RBAC: Create & Find integration tests (#95511) * [Cases] Cases client enchantment (#95923) * [Cases] Authorization and Client Audit Logger (#95477) * Starting audit logger * Finishing auth audit logger * Fixing tests and types * Adding audit event creator * Renaming class to scope * Adding audit logger messages to create and find * Adding comments and fixing import issue * Fixing type errors * Fixing tests and adding username to message * Addressing PR feedback * Removing unneccessary log and generating id * Fixing module issue and remove expect.anything * [Cases] Migrate sub cases routes to a client (#96461) * Adding sub cases client * Move sub case routes to case client * Throw when attempting to access the sub cases client * Fixing throw and removing user ans soclients * [Cases] RBAC: Migrate routes' unit tests to integration tests (#96374) Co-authored-by: Jonathan Buttner <[email protected]> * [Cases] Move remaining HTTP functionality to client (#96507) * Moving deletes and find for attachments * Moving rest of comment apis * Migrating configuration routes to client * Finished moving routes, starting utils refactor * Refactoring utilites and fixing integration tests * Addressing PR feedback * Fixing mocks and types * Fixing integration tests * Renaming status_stats * Fixing test type errors * Adding plugins to kibana.json * Adding cases to required plugin * [Cases] Refactoring authorization (#97483) * Refactoring authorization * Wrapping auth calls in helper for try catch * Reverting name change * Hardcoding the saved object types * Switching ensure to owner array * [Cases] Add authorization to configuration & cases routes (#97228) * [Cases] Attachments RBAC (#97756) * Starting rbac for comments * Adding authorization to rest of comment apis * Starting the comment rbac tests * Fixing some of the rbac tests * Adding some integration tests * Starting patch tests * Working tests for comments * Working tests * Fixing some tests * Fixing type issues from pulling in master * Fixing connector tests that only work in trial license * Attempting to fix cypress * Mock return of array for configure * Fixing cypress test * Cleaning up * Addressing PR comments * Reducing operations * [Cases] Add RBAC to remaining Cases APIs (#98762) * Starting rbac for comments * Adding authorization to rest of comment apis * Starting the comment rbac tests * Fixing some of the rbac tests * Adding some integration tests * Starting patch tests * Working tests for comments * Working tests * Fixing some tests * Fixing type issues from pulling in master * Fixing connector tests that only work in trial license * Attempting to fix cypress * Mock return of array for configure * Fixing cypress test * Cleaning up * Working case update tests * Addressing PR comments * Reducing operations * Working rbac push case tests * Starting stats apis * Working status tests * User action tests and fixing migration errors * Fixing type errors * including error in message * Addressing pr feedback * Fixing some type errors * [Cases] Add space only tests (#99409) * Starting spaces tests * Finishing space only tests * Refactoring createCaseWithConnector * Fixing spelling * Addressing PR feedback and creating alert tests * Fixing mocks * [Cases] Add security only tests (#99679) * Starting spaces tests * Finishing space only tests * Refactoring createCaseWithConnector * Fixing spelling * Addressing PR feedback and creating alert tests * Fixing mocks * Starting security only tests * Adding remainder security only tests * Using helper objects * Fixing type error for null space * Renaming utility variables * Refactoring users and roles for security only tests * Adding sub feature * [Cases] Cleaning up the services and TODOs (#99723) * Cleaning up the service intialization * Fixing type errors * Adding comments for the api * Working test for cases client * Fix type error * Adding generated docs * Adding more docs and cleaning up types * Cleaning up readme * More clean up and links * Changing some file names * Renaming docs * Integration tests for cases privs and fixes (#100038) * [Cases] RBAC on UI (#99478) Co-authored-by: Kibana Machine <[email protected]> * Fixing case ids by alert id route call * [Cases] Fixing UI feature permissions and adding UI tests (#100074) * Integration tests for cases privs and fixes * Fixing ui cases permissions and adding tests * Adding test for collection failure and fixing jest * Renaming variables * Fixing type error * Adding some comments * Validate cases features * Fix new schema * Adding owner param for the status stats * Fix get case status tests * Adjusting permissions text and fixing status * Address PR feedback * Adding top level feature back * Fixing feature privileges * Renaming * Removing uneeded else * Fixing tests and adding cases merge tests * [Cases][Security Solution] Basic license security solution API tests (#100925) * Cleaning up the fixture plugins * Adding basic feature test * renaming to unsecuredSavedObjectsClient (#101215) * [Cases] RBAC Refactoring audit logging (#100952) * Refactoring audit logging * Adding unit tests for authorization classes * Addressing feedback and adding util tests * return undefined on empty array * fixing eslint * [Cases] Cleaning up RBAC integration tests (#101324) * Adding tests for space permissions * Adding tests for testing a disable feature Co-authored-by: Christos Nasikas <[email protected]> Co-authored-by: Kibana Machine <[email protected]> # Conflicts: # x-pack/plugins/cases/server/client/client.ts # x-pack/plugins/cases/server/client/mocks.ts # x-pack/plugins/cases/server/client/types.ts # x-pack/plugins/cases/server/index.ts # x-pack/plugins/cases/server/plugin.ts # x-pack/plugins/security_solution/server/endpoint/endpoint_app_context_services.ts # x-pack/plugins/security_solution/server/endpoint/mocks.ts # x-pack/plugins/security_solution/server/endpoint/routes/actions/isolation.ts * Fixing type errors * Addressing plugin type errors
jonathan-buttner
added a commit
that referenced
this pull request
Jun 22, 2021
* Adding feature flag for auth * Hiding SOs and adding consumer field * First pass at adding security changes * Consumer as the app's plugin ID * Create addConsumerToSO migration helper * Fix mapping's SO consumer * Add test for CasesActions * Declare hidden types on SO client * Restructure integration tests * Init spaces_only integration tests * Implementing the cases security string * Adding security plugin tests for cases * Rough concept for authorization class * Adding comments * Fix merge * Get requiredPrivileges for classes * Check privillages * Ensure that all classes are available * Success if hasAllRequested is true * Failure if hasAllRequested is false * Adding schema updates for feature plugin * Seperate basic from trial * Enable SIR on integration tests * Starting the plumbing for authorization in plugin * Unit tests working * Move find route logic to case client * Create integration test helper functions * Adding auth to create call * Create getClassFilter helper * Add class attribute to find request * Create getFindAuthorizationFilter * Ensure savedObject is authorized in find method * Include fields for authorization * Combine authorization filter with cases & subcases filter * Fix isAuthorized flag * Fix merge issue * Create/delete spaces & users before and after tests * Add more user and roles * [Cases] Convert filters from strings to KueryNode (#95288) * [Cases] RBAC: Rename class to scope (#95535) * [Cases][RBAC] Rename scope to owner (#96035) * [Cases] RBAC: Create & Find integration tests (#95511) * [Cases] Cases client enchantment (#95923) * [Cases] Authorization and Client Audit Logger (#95477) * Starting audit logger * Finishing auth audit logger * Fixing tests and types * Adding audit event creator * Renaming class to scope * Adding audit logger messages to create and find * Adding comments and fixing import issue * Fixing type errors * Fixing tests and adding username to message * Addressing PR feedback * Removing unneccessary log and generating id * Fixing module issue and remove expect.anything * [Cases] Migrate sub cases routes to a client (#96461) * Adding sub cases client * Move sub case routes to case client * Throw when attempting to access the sub cases client * Fixing throw and removing user ans soclients * [Cases] RBAC: Migrate routes' unit tests to integration tests (#96374) Co-authored-by: Jonathan Buttner <[email protected]> * [Cases] Move remaining HTTP functionality to client (#96507) * Moving deletes and find for attachments * Moving rest of comment apis * Migrating configuration routes to client * Finished moving routes, starting utils refactor * Refactoring utilites and fixing integration tests * Addressing PR feedback * Fixing mocks and types * Fixing integration tests * Renaming status_stats * Fixing test type errors * Adding plugins to kibana.json * Adding cases to required plugin * [Cases] Refactoring authorization (#97483) * Refactoring authorization * Wrapping auth calls in helper for try catch * Reverting name change * Hardcoding the saved object types * Switching ensure to owner array * [Cases] Add authorization to configuration & cases routes (#97228) * [Cases] Attachments RBAC (#97756) * Starting rbac for comments * Adding authorization to rest of comment apis * Starting the comment rbac tests * Fixing some of the rbac tests * Adding some integration tests * Starting patch tests * Working tests for comments * Working tests * Fixing some tests * Fixing type issues from pulling in master * Fixing connector tests that only work in trial license * Attempting to fix cypress * Mock return of array for configure * Fixing cypress test * Cleaning up * Addressing PR comments * Reducing operations * [Cases] Add RBAC to remaining Cases APIs (#98762) * Starting rbac for comments * Adding authorization to rest of comment apis * Starting the comment rbac tests * Fixing some of the rbac tests * Adding some integration tests * Starting patch tests * Working tests for comments * Working tests * Fixing some tests * Fixing type issues from pulling in master * Fixing connector tests that only work in trial license * Attempting to fix cypress * Mock return of array for configure * Fixing cypress test * Cleaning up * Working case update tests * Addressing PR comments * Reducing operations * Working rbac push case tests * Starting stats apis * Working status tests * User action tests and fixing migration errors * Fixing type errors * including error in message * Addressing pr feedback * Fixing some type errors * [Cases] Add space only tests (#99409) * Starting spaces tests * Finishing space only tests * Refactoring createCaseWithConnector * Fixing spelling * Addressing PR feedback and creating alert tests * Fixing mocks * [Cases] Add security only tests (#99679) * Starting spaces tests * Finishing space only tests * Refactoring createCaseWithConnector * Fixing spelling * Addressing PR feedback and creating alert tests * Fixing mocks * Starting security only tests * Adding remainder security only tests * Using helper objects * Fixing type error for null space * Renaming utility variables * Refactoring users and roles for security only tests * Adding sub feature * [Cases] Cleaning up the services and TODOs (#99723) * Cleaning up the service intialization * Fixing type errors * Adding comments for the api * Working test for cases client * Fix type error * Adding generated docs * Adding more docs and cleaning up types * Cleaning up readme * More clean up and links * Changing some file names * Renaming docs * Integration tests for cases privs and fixes (#100038) * [Cases] RBAC on UI (#99478) Co-authored-by: Kibana Machine <[email protected]> * Fixing case ids by alert id route call * [Cases] Fixing UI feature permissions and adding UI tests (#100074) * Integration tests for cases privs and fixes * Fixing ui cases permissions and adding tests * Adding test for collection failure and fixing jest * Renaming variables * Fixing type error * Adding some comments * Validate cases features * Fix new schema * Adding owner param for the status stats * Fix get case status tests * Adjusting permissions text and fixing status * Address PR feedback * Adding top level feature back * Fixing feature privileges * Renaming * Removing uneeded else * Fixing tests and adding cases merge tests * [Cases][Security Solution] Basic license security solution API tests (#100925) * Cleaning up the fixture plugins * Adding basic feature test * renaming to unsecuredSavedObjectsClient (#101215) * [Cases] RBAC Refactoring audit logging (#100952) * Refactoring audit logging * Adding unit tests for authorization classes * Addressing feedback and adding util tests * return undefined on empty array * fixing eslint * conditional rendering the recently created cases * Remove unnecessary Array.from * Cleaning up overview page for permissions * Fixing log message for attachments * hiding add to cases button * Disable the Cases app from the global nav * Hide the add to cases button from detections * Fixing merge * Making progress on removing icons * Hding edit icons on detail view * Trying to get connector error msg tests working * Removing test * Disable error callouts * Fixing spacing and removing cases tab one no read * Adding read only badge * Cleaning up and adding badge * Wrapping in use effect * Default toasting permissions errors * Removing actions icon on comments * Addressing feedback * Fixing type Co-authored-by: Christos Nasikas <[email protected]> Co-authored-by: Kibana Machine <[email protected]>
jonathan-buttner
added a commit
that referenced
this pull request
Jun 22, 2021
* Adding feature flag for auth * Hiding SOs and adding consumer field * First pass at adding security changes * Consumer as the app's plugin ID * Create addConsumerToSO migration helper * Fix mapping's SO consumer * Add test for CasesActions * Declare hidden types on SO client * Restructure integration tests * Init spaces_only integration tests * Implementing the cases security string * Adding security plugin tests for cases * Rough concept for authorization class * Adding comments * Fix merge * Get requiredPrivileges for classes * Check privillages * Ensure that all classes are available * Success if hasAllRequested is true * Failure if hasAllRequested is false * Adding schema updates for feature plugin * Seperate basic from trial * Enable SIR on integration tests * Starting the plumbing for authorization in plugin * Unit tests working * Move find route logic to case client * Create integration test helper functions * Adding auth to create call * Create getClassFilter helper * Add class attribute to find request * Create getFindAuthorizationFilter * Ensure savedObject is authorized in find method * Include fields for authorization * Combine authorization filter with cases & subcases filter * Fix isAuthorized flag * Fix merge issue * Create/delete spaces & users before and after tests * Add more user and roles * [Cases] Convert filters from strings to KueryNode (#95288) * [Cases] RBAC: Rename class to scope (#95535) * [Cases][RBAC] Rename scope to owner (#96035) * [Cases] RBAC: Create & Find integration tests (#95511) * [Cases] Cases client enchantment (#95923) * [Cases] Authorization and Client Audit Logger (#95477) * Starting audit logger * Finishing auth audit logger * Fixing tests and types * Adding audit event creator * Renaming class to scope * Adding audit logger messages to create and find * Adding comments and fixing import issue * Fixing type errors * Fixing tests and adding username to message * Addressing PR feedback * Removing unneccessary log and generating id * Fixing module issue and remove expect.anything * [Cases] Migrate sub cases routes to a client (#96461) * Adding sub cases client * Move sub case routes to case client * Throw when attempting to access the sub cases client * Fixing throw and removing user ans soclients * [Cases] RBAC: Migrate routes' unit tests to integration tests (#96374) Co-authored-by: Jonathan Buttner <[email protected]> * [Cases] Move remaining HTTP functionality to client (#96507) * Moving deletes and find for attachments * Moving rest of comment apis * Migrating configuration routes to client * Finished moving routes, starting utils refactor * Refactoring utilites and fixing integration tests * Addressing PR feedback * Fixing mocks and types * Fixing integration tests * Renaming status_stats * Fixing test type errors * Adding plugins to kibana.json * Adding cases to required plugin * [Cases] Refactoring authorization (#97483) * Refactoring authorization * Wrapping auth calls in helper for try catch * Reverting name change * Hardcoding the saved object types * Switching ensure to owner array * [Cases] Add authorization to configuration & cases routes (#97228) * [Cases] Attachments RBAC (#97756) * Starting rbac for comments * Adding authorization to rest of comment apis * Starting the comment rbac tests * Fixing some of the rbac tests * Adding some integration tests * Starting patch tests * Working tests for comments * Working tests * Fixing some tests * Fixing type issues from pulling in master * Fixing connector tests that only work in trial license * Attempting to fix cypress * Mock return of array for configure * Fixing cypress test * Cleaning up * Addressing PR comments * Reducing operations * [Cases] Add RBAC to remaining Cases APIs (#98762) * Starting rbac for comments * Adding authorization to rest of comment apis * Starting the comment rbac tests * Fixing some of the rbac tests * Adding some integration tests * Starting patch tests * Working tests for comments * Working tests * Fixing some tests * Fixing type issues from pulling in master * Fixing connector tests that only work in trial license * Attempting to fix cypress * Mock return of array for configure * Fixing cypress test * Cleaning up * Working case update tests * Addressing PR comments * Reducing operations * Working rbac push case tests * Starting stats apis * Working status tests * User action tests and fixing migration errors * Fixing type errors * including error in message * Addressing pr feedback * Fixing some type errors * [Cases] Add space only tests (#99409) * Starting spaces tests * Finishing space only tests * Refactoring createCaseWithConnector * Fixing spelling * Addressing PR feedback and creating alert tests * Fixing mocks * [Cases] Add security only tests (#99679) * Starting spaces tests * Finishing space only tests * Refactoring createCaseWithConnector * Fixing spelling * Addressing PR feedback and creating alert tests * Fixing mocks * Starting security only tests * Adding remainder security only tests * Using helper objects * Fixing type error for null space * Renaming utility variables * Refactoring users and roles for security only tests * Adding sub feature * [Cases] Cleaning up the services and TODOs (#99723) * Cleaning up the service intialization * Fixing type errors * Adding comments for the api * Working test for cases client * Fix type error * Adding generated docs * Adding more docs and cleaning up types * Cleaning up readme * More clean up and links * Changing some file names * Renaming docs * Integration tests for cases privs and fixes (#100038) * [Cases] RBAC on UI (#99478) Co-authored-by: Kibana Machine <[email protected]> * Fixing case ids by alert id route call * [Cases] Fixing UI feature permissions and adding UI tests (#100074) * Integration tests for cases privs and fixes * Fixing ui cases permissions and adding tests * Adding test for collection failure and fixing jest * Renaming variables * Fixing type error * Adding some comments * Validate cases features * Fix new schema * Adding owner param for the status stats * Fix get case status tests * Adjusting permissions text and fixing status * Address PR feedback * Adding top level feature back * Fixing feature privileges * Renaming * Removing uneeded else * Fixing tests and adding cases merge tests * [Cases][Security Solution] Basic license security solution API tests (#100925) * Cleaning up the fixture plugins * Adding basic feature test * renaming to unsecuredSavedObjectsClient (#101215) * [Cases] RBAC Refactoring audit logging (#100952) * Refactoring audit logging * Adding unit tests for authorization classes * Addressing feedback and adding util tests * return undefined on empty array * fixing eslint * conditional rendering the recently created cases * Remove unnecessary Array.from * Cleaning up overview page for permissions * Fixing log message for attachments * hiding add to cases button * Disable the Cases app from the global nav * Hide the add to cases button from detections * Fixing merge * Making progress on removing icons * Hding edit icons on detail view * Trying to get connector error msg tests working * Removing test * Disable error callouts * Fixing spacing and removing cases tab one no read * Adding read only badge * Cleaning up and adding badge * Wrapping in use effect * Default toasting permissions errors * Removing actions icon on comments * Addressing feedback * Fixing type Co-authored-by: Christos Nasikas <[email protected]> Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Christos Nasikas <[email protected]> Co-authored-by: Kibana Machine <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR implements RBAC for the comment/attachment APIs.
The APIs are:
DELETE /api/cases/{case_id}/comments- Delete all comments for a caseDELETE /api/cases/{case_id}/comments/{comment_id}- Delete a single commentGET /api/cases/{case_id}/comments/_find- Find comments for a caseGET /api/cases/{case_id}/comments- Get all comments for a caseGET /api/cases/{case_id}/comments/{comment_id}- Get single comment for a casePATCH /api/cases/{case_id}/comments- Update a commentPOST /api/cases/{case_id}/comments- Create a commentGetting CI Green
To get CI tests to complete successfully I had to make a few changes to some of the configuration cypress and UI unit tests. I left a bunch of TODOs so that we can go back over the changes and address anything that needs to be updated before the feature branch is merged into master. I'll try to comment on the areas in the PR review as well.
Testing
I added integration tests for each API to ensure that RBAC is working correctly