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] [Security Solution] New cases subfeatures, add comments and reopen cases #194898

Merged

Conversation

kqualters-elastic
Copy link
Contributor

@kqualters-elastic kqualters-elastic commented Oct 4, 2024

Summary

This pr adds 2 new sub feature permissions to the cases plugin in stack/security/observability, that behave as follows. The first is for controlling the ability to reopen cases. When Cases has the read permission, and the reopen permission is not enabled, users have permissions as before. When enabled, users can move cases from closed to open/in progress, but nothing else. If a user has all and this permission, they can do anything as before, if the option is unselected, they can change case properties, and change a case from open to anything, in progress to anything, but if the case is closed, are unable to reopen it.

The 2nd permission is 'Add comment'. When enabled and the user has case read permissions, users can add comments, but not make any other changes to the case. When the user has read and this deselected, read functions as before. When a user has this permission and cases is all, this functions as all. When they have all but this permission is deselected, the user can do everything normally, except add cases comments.

Checklist

@kqualters-elastic kqualters-elastic added v8.16.0 v9.0.0 release_note:feature Makes this part of the condensed release notes backport:version Backport to applied version labels labels Oct 7, 2024
@kqualters-elastic kqualters-elastic marked this pull request as ready for review October 7, 2024 02:30
@kqualters-elastic kqualters-elastic requested review from a team as code owners October 7, 2024 02:30
@botelastic botelastic bot added ci:project-deploy-observability Create an Observability project Team:obs-ux-management Observability Management User Experience Team labels Oct 7, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ux-management-team (Team:obs-ux-management)

@kibana-ci

This comment was marked as outdated.

Copy link
Contributor

@jloleysens jloleysens left a comment

Choose a reason for hiding this comment

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

Focussed on features plugin and left a few nits that would be nice to address before merging.

Comment on lines 219 to 238
createComment?: readonly string[];
reopenCases?: readonly string[];
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: missing doc comments like other interface members

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will update, although I think the grammar for all of these doc blocks is a little unclear. Proposed change for all of them: "List of case owners whose users should have settings access when granted this privilege.", "List of case owners whose users should have comment creation access when granted this privilege." etc. cc @cnasikas

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good to me. Also could you please add a comment on top of the update to mention that it does not include reopening a case?

@@ -151,6 +151,14 @@ function mergeWithSubFeatures(
mergedConfig.cases?.settings ?? [],
subFeaturePrivilege.cases?.settings ?? []
),
createComment: mergeArrays(
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: would you mind including these in the relevant unit test in x-pack/plugins/features/server/feature_privilege_iterator/feature_privilege_iterator.test.ts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ya will update, although that seems like something that typescript should catch

Copy link
Member

Choose a reason for hiding this comment

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

+1 for adding unit tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

updated

Copy link
Contributor

@cauemarcondes cauemarcondes left a comment

Choose a reason for hiding this comment

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

Obs shared LGTM

@azasypkin azasypkin self-requested a review October 7, 2024 11:15
Copy link
Contributor

@lgestc lgestc left a comment

Choose a reason for hiding this comment

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

looks okay, will also try to test it later today

Copy link
Member

@cnasikas cnasikas left a comment

Choose a reason for hiding this comment

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

Hey! I did a first pass of the PR focusing mostly on the backend code. I left some comments.

@@ -171,6 +171,8 @@ export const DELETE_CASES_CAPABILITY = 'delete_cases' as const;
export const PUSH_CASES_CAPABILITY = 'push_cases' as const;
export const CASES_SETTINGS_CAPABILITY = 'cases_settings' as const;
export const CASES_CONNECTORS_CAPABILITY = 'cases_connectors' as const;
export const REOPEN_CASES_CAPABILITY = 'reopen_cases' as const;
export const COMMENT_CASES_CAPABILITY = 'create_comment' as const;
Copy link
Member

Choose a reason for hiding this comment

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

super nit: COMMENT_CASES_CAPABILITY -> CREATE_COMMENT_CAPABILITY

Copy link
Contributor

Choose a reason for hiding this comment

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

Done

Comment on lines 219 to 238
createComment?: readonly string[];
reopenCases?: readonly string[];
Copy link
Member

Choose a reason for hiding this comment

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

Sounds good to me. Also could you please add a comment on top of the update to mention that it does not include reopening a case?

@@ -151,6 +151,14 @@ function mergeWithSubFeatures(
mergedConfig.cases?.settings ?? [],
subFeaturePrivilege.cases?.settings ?? []
),
createComment: mergeArrays(
Copy link
Member

Choose a reason for hiding this comment

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

+1 for adding unit tests.

@@ -148,6 +148,8 @@ export enum SecuritySubFeatureId {
export enum CasesSubFeatureId {
deleteCases = 'deleteCasesSubFeature',
casesSettings = 'casesSettingsSubFeature',
addComment = 'addCommentSubFeature',
Copy link
Member

Choose a reason for hiding this comment

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

nit: Let's stick with the createComment* terminology.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

Copy link
Contributor

Choose a reason for hiding this comment

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

done

Comment on lines 37 to 38
const createCommentOperations = ['createComment'] as const;
const reopenOperations = ['reopenCases'] as const;
Copy link
Member

Choose a reason for hiding this comment

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

nit: Because they are single operations I think it is better if we do const createComment = 'createComment' etc. Wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer actually if we left this as is for consistency in the getCasesPrivilege fn and the way allOperations is built. Also the naming paradigm operations maintains that consistency as well. Willing to make the change if you'd still like us to, but think this way is a bit simpler

@@ -51,15 +51,15 @@ describe('useStatusAction', () => {
},
Object {
"data-test-subj": "cases-bulk-action-status-in-progress",
"disabled": false,
Copy link
Member

Choose a reason for hiding this comment

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

Let's keep the test with disabled: false and then add some tests that test the disabled behavior.

Copy link
Contributor

Choose a reason for hiding this comment

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

Done

@@ -52,47 +52,6 @@ describe('useBulkActions', () => {
Object {
"id": 0,
"items": Array [
Object {
Copy link
Member

Choose a reason for hiding this comment

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

Let's keep the test without removing the actions and then add some tests that test the disabled behavior.

const canChangeStatus = useMemo(() => {
// User has full permissions
if (permissions.update && permissions.reopenCases) {
return false;
Copy link
Member

Choose a reason for hiding this comment

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

Should we return true?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yea, had to rethink this a bit

// User has full permissions
if (permissions.update && permissions.reopenCases) {
return false;
} else {
Copy link
Member

Choose a reason for hiding this comment

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

nit: What do you think of escaping early the if statements?

Copy link
Contributor

Choose a reason for hiding this comment

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

Rethought this file a bit. Hopefully it makes more sense now

import { useCasesContext } from '../cases_context/use_cases_context';
import { CaseStatuses } from '../../../common/types/domain';

export const useUserPermissions = ({ status }: { status?: CaseStatuses }) => {
Copy link
Member

Choose a reason for hiding this comment

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

nit: status -> statusToAuthorize

Copy link
Contributor

@dominiqueclarke dominiqueclarke left a comment

Choose a reason for hiding this comment

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

obs-ux-management changes LGTM

Copy link
Contributor

@semd semd left a comment

Choose a reason for hiding this comment

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

Security Threat Hunting Explore code LGTM

@michaelolo24 michaelolo24 force-pushed the cases-subfeatures-main branch from 8ec412d to 10d7756 Compare October 14, 2024 21:34
@kqualters-elastic kqualters-elastic removed the ci:project-deploy-observability Create an Observability project label Nov 19, 2024
Copy link
Member

@azasypkin azasypkin left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! Left a few minor nits.

notice: i18n.translate(
'securitySolutionPackages.features.featureRegistry.linkSecuritySolutionCase.deprecationMessage',
{
defaultMessage:
Copy link
Member

Choose a reason for hiding this comment

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

Note

We don’t need to do anything right now, but eventually, when we start displaying this to our users, we might need to rephrase it since interactive users won’t understand what CASES_FEATURE_ID_V2 means.

minimal: [
{
feature: FEATURE_ID_V2,
privileges: ['minimal_all', 'create_comment', 'case_reopen'],
Copy link
Member

Choose a reason for hiding this comment

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

I think we sorted this out! We only need minimal_all, create_comment, case_reopen.

@@ -97,6 +123,7 @@ export const getCasesKibanaFeature = (): KibanaFeatureConfig => {
delete: [APP_ID],
},
ui: capabilities.delete,
replacedBy: [{ feature: FEATURE_ID_V2, privileges: ['cases_delete'] }],
Copy link
Member

Choose a reason for hiding this comment

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

Can we use const here (same for settings)?

Suggested change
replacedBy: [{ feature: FEATURE_ID_V2, privileges: ['cases_delete'] }],
replacedBy: [{ feature: FEATURE_ID_V2, privileges: [CASES_DELETE_SUB_PRIVILEGE_ID] }],

@@ -148,15 +148,15 @@ export default function catalogueTests({ getService }: FtrProviderContext) {
expect(uiCapabilities.value!.catalogue).to.eql(expected);
break;
}
case 'global_read at nothing_space':
Copy link
Member

Choose a reason for hiding this comment

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

question: do you need this change? I believe these xxx at nothing_space should stay where they were, under the corresponding the nothing_space has no Kibana features enabled,.... comment.

@@ -80,11 +80,11 @@ export default function navLinksTests({ getService }: FtrProviderContext) {
navLinksBuilder.only('kibana', 'foo', 'management')
);
break;
case 'foo_all at nothing_space':
Copy link
Member

Choose a reason for hiding this comment

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

nit: same comment here, there is no need to re-order these case branches.

@@ -504,6 +506,9 @@ soc_manager:
- feature_siem.execute_operations_all
- feature_siem.scan_operations_all
- feature_securitySolutionCases.all
- feature_securitySolutionCasesV2.all
- feature_observabilityCases.all
- feature_observabilityCasesV2.all
Copy link
Member

Choose a reason for hiding this comment

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

I'm not quite sure why we need both v1 and v2 privileges in this file, cannot we just keep v2 ones?

@botelastic botelastic bot added the ci:project-deploy-observability Create an Observability project label Nov 19, 2024
Copy link
Contributor

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

@kqualters-elastic
Copy link
Contributor Author

I'm not quite sure why we need both v1 and v2 privileges in this file, cannot we just keep v2 ones?

@azasypkin good catch, those were intended to be done in conjunction with https://github.com/elastic/elasticsearch-controller/pull/762 but since we are going to merge this pr, and then after the next serverless release, convert to v2 instead of having both defined at once, I removed them in 3aa925d

@kqualters-elastic kqualters-elastic enabled auto-merge (squash) November 19, 2024 18:06
@elasticmachine
Copy link
Contributor

elasticmachine commented Nov 19, 2024

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
cases 821 823 +2

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
cases 95 106 +11
observability 686 687 +1
total +12

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
cases 491.9KB 492.7KB +846.0B
ml 4.7MB 4.7MB +6.0B
observability 472.5KB 472.5KB +7.0B
securitySolution 13.4MB 13.4MB +147.0B
threatIntelligence 57.3KB 57.3KB +7.0B
total +1013.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
cases 158.3KB 159.1KB +791.0B
observability 103.6KB 103.7KB +74.0B
observabilityShared 93.6KB 93.7KB +64.0B
securitySolution 86.1KB 86.1KB +4.0B
total +933.0B
Unknown metric groups

API count

id before after diff
cases 115 126 +11
observability 694 695 +1
total +12

ESLint disabled line counts

id before after diff
@kbn/test-suites-xpack 722 723 +1

Total ESLint disabled count

id before after diff
@kbn/test-suites-xpack 747 748 +1

Unreferenced deprecated APIs

id before after diff
cases 0 1 +1
observability 0 1 +1
total +2

History

cc @adcoelho @cnasikas

@kqualters-elastic kqualters-elastic merged commit 0afae42 into elastic:main Nov 19, 2024
53 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11920118581

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 19, 2024
…eopen cases (elastic#194898)

## Summary

This pr adds 2 new sub feature permissions to the cases plugin in
stack/security/observability, that behave as follows. The first is for
controlling the ability to reopen cases. When Cases has the read
permission, and the reopen permission is not enabled, users have
permissions as before. When enabled, users can move cases from closed to
open/in progress, but nothing else. If a user has all and this
permission, they can do anything as before, if the option is unselected,
they can change case properties, and change a case from open to
anything, in progress to anything, but if the case is closed, are unable
to reopen it.

The 2nd permission is 'Add comment'. When enabled and the user has case
read permissions, users can add comments, but not make any other changes
to the case. When the user has read and this deselected, read functions
as before. When a user has this permission and cases is all, this
functions as all. When they have all but this permission is deselected,
the user can do everything normally, except add cases comments.

### Checklist

- [x] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: Michael Olorunnisola <michael.olorunnisola@elastic.co>
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
(cherry picked from commit 0afae42)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Nov 19, 2024
… and reopen cases (#194898) (#200807)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Cases] [Security Solution] New cases subfeatures, add comments and
reopen cases (#194898)](#194898)

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

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

<!--BACKPORT [{"author":{"name":"Kevin
Qualters","email":"56408403+kqualters-elastic@users.noreply.github.com"},"sourceCommit":{"committedDate":"2024-11-19T19:15:38Z","message":"[Cases]
[Security Solution] New cases subfeatures, add comments and reopen cases
(#194898)\n\n## Summary\r\n\r\nThis pr adds 2 new sub feature
permissions to the cases plugin in\r\nstack/security/observability, that
behave as follows. The first is for\r\ncontrolling the ability to reopen
cases. When Cases has the read\r\npermission, and the reopen permission
is not enabled, users have\r\npermissions as before. When enabled, users
can move cases from closed to\r\nopen/in progress, but nothing else. If
a user has all and this\r\npermission, they can do anything as before,
if the option is unselected,\r\nthey can change case properties, and
change a case from open to\r\nanything, in progress to anything, but if
the case is closed, are unable\r\nto reopen it.\r\n\r\nThe 2nd
permission is 'Add comment'. When enabled and the user has case\r\nread
permissions, users can add comments, but not make any other
changes\r\nto the case. When the user has read and this deselected, read
functions\r\nas before. When a user has this permission and cases is
all, this\r\nfunctions as all. When they have all but this permission is
deselected,\r\nthe user can do everything normally, except add cases
comments.\r\n\r\n### Checklist\r\n\r\n- [x] Any text added follows
[EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [x] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: Michael Olorunnisola
<michael.olorunnisola@elastic.co>\r\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"0afae423443ba13c47a263c4cbc270ea09942148","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["v9.0.0","release_note:feature","ci:project-deploy-observability","Team:obs-ux-management","apm:review","backport:version","v8.17.0"],"title":"[Cases]
[Security Solution] New cases subfeatures, add comments and reopen
cases","number":194898,"url":"https://github.com/elastic/kibana/pull/194898","mergeCommit":{"message":"[Cases]
[Security Solution] New cases subfeatures, add comments and reopen cases
(#194898)\n\n## Summary\r\n\r\nThis pr adds 2 new sub feature
permissions to the cases plugin in\r\nstack/security/observability, that
behave as follows. The first is for\r\ncontrolling the ability to reopen
cases. When Cases has the read\r\npermission, and the reopen permission
is not enabled, users have\r\npermissions as before. When enabled, users
can move cases from closed to\r\nopen/in progress, but nothing else. If
a user has all and this\r\npermission, they can do anything as before,
if the option is unselected,\r\nthey can change case properties, and
change a case from open to\r\nanything, in progress to anything, but if
the case is closed, are unable\r\nto reopen it.\r\n\r\nThe 2nd
permission is 'Add comment'. When enabled and the user has case\r\nread
permissions, users can add comments, but not make any other
changes\r\nto the case. When the user has read and this deselected, read
functions\r\nas before. When a user has this permission and cases is
all, this\r\nfunctions as all. When they have all but this permission is
deselected,\r\nthe user can do everything normally, except add cases
comments.\r\n\r\n### Checklist\r\n\r\n- [x] Any text added follows
[EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [x] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: Michael Olorunnisola
<michael.olorunnisola@elastic.co>\r\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"0afae423443ba13c47a263c4cbc270ea09942148"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/194898","number":194898,"mergeCommit":{"message":"[Cases]
[Security Solution] New cases subfeatures, add comments and reopen cases
(#194898)\n\n## Summary\r\n\r\nThis pr adds 2 new sub feature
permissions to the cases plugin in\r\nstack/security/observability, that
behave as follows. The first is for\r\ncontrolling the ability to reopen
cases. When Cases has the read\r\npermission, and the reopen permission
is not enabled, users have\r\npermissions as before. When enabled, users
can move cases from closed to\r\nopen/in progress, but nothing else. If
a user has all and this\r\npermission, they can do anything as before,
if the option is unselected,\r\nthey can change case properties, and
change a case from open to\r\nanything, in progress to anything, but if
the case is closed, are unable\r\nto reopen it.\r\n\r\nThe 2nd
permission is 'Add comment'. When enabled and the user has case\r\nread
permissions, users can add comments, but not make any other
changes\r\nto the case. When the user has read and this deselected, read
functions\r\nas before. When a user has this permission and cases is
all, this\r\nfunctions as all. When they have all but this permission is
deselected,\r\nthe user can do everything normally, except add cases
comments.\r\n\r\n### Checklist\r\n\r\n- [x] Any text added follows
[EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [x] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: Michael Olorunnisola
<michael.olorunnisola@elastic.co>\r\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"0afae423443ba13c47a263c4cbc270ea09942148"}},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Kevin Qualters <56408403+kqualters-elastic@users.noreply.github.com>
kqualters-elastic added a commit that referenced this pull request Nov 25, 2024
… tests for failing case (#201517)

## Summary

Currently, the partitionPatchRequest in
x-pack/plugins/cases/server/client/cases/bulk_update.ts will not check a
case properly if a case is being re-opened, instead of an else if in the
loop comparing cases to cases in the request, the status logic should be
outside of this set of if statements. If a case is being re-opened, the
final else is never run, and casesToAuthorize is 0. This results in
ensureAuthorized being called with an empty array of entities, and so
the check always succeeds. To fix this, reopenedCases is still populated
in the same loop, just not when casesToAuthorize logic is running as
well. Also adds some missing tests for this and the create comment
permission as requested in
#194898.

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 25, 2024
… tests for failing case (elastic#201517)

## Summary

Currently, the partitionPatchRequest in
x-pack/plugins/cases/server/client/cases/bulk_update.ts will not check a
case properly if a case is being re-opened, instead of an else if in the
loop comparing cases to cases in the request, the status logic should be
outside of this set of if statements. If a case is being re-opened, the
final else is never run, and casesToAuthorize is 0. This results in
ensureAuthorized being called with an empty array of entities, and so
the check always succeeds. To fix this, reopenedCases is still populated
in the same loop, just not when casesToAuthorize logic is running as
well. Also adds some missing tests for this and the create comment
permission as requested in
elastic#194898.

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

(cherry picked from commit 1c3bcea)
kibanamachine added a commit that referenced this pull request Nov 25, 2024
…gration tests for failing case (#201517) (#201554)

# Backport

This will backport the following commits from `main` to `8.17`:
- [[Cases] Fix an issue with the reopen case permission, add integration
tests for failing case
(#201517)](#201517)

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

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

<!--BACKPORT [{"author":{"name":"Kevin
Qualters","email":"56408403+kqualters-elastic@users.noreply.github.com"},"sourceCommit":{"committedDate":"2024-11-25T11:12:43Z","message":"[Cases]
Fix an issue with the reopen case permission, add integration tests for
failing case (#201517)\n\n## Summary\r\n\r\nCurrently, the
partitionPatchRequest
in\r\nx-pack/plugins/cases/server/client/cases/bulk_update.ts will not
check a\r\ncase properly if a case is being re-opened, instead of an
else if in the\r\nloop comparing cases to cases in the request, the
status logic should be\r\noutside of this set of if statements. If a
case is being re-opened, the\r\nfinal else is never run, and
casesToAuthorize is 0. This results in\r\nensureAuthorized being called
with an empty array of entities, and so\r\nthe check always succeeds. To
fix this, reopenedCases is still populated\r\nin the same loop, just not
when casesToAuthorize logic is running as\r\nwell. Also adds some
missing tests for this and the create comment\r\npermission as requested
in\r\nhttps://github.com//pull/194898.\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"1c3bceacc06f5f8a01a2ffde2ec24f114717b396","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:skip","Team:ResponseOps","v9.0.0","Team:Threat
Hunting:Investigations","backport:version","v8.17.0"],"title":"[Cases]
Fix an issue with the reopen case permission, add integration tests for
failing
case","number":201517,"url":"https://github.com/elastic/kibana/pull/201517","mergeCommit":{"message":"[Cases]
Fix an issue with the reopen case permission, add integration tests for
failing case (#201517)\n\n## Summary\r\n\r\nCurrently, the
partitionPatchRequest
in\r\nx-pack/plugins/cases/server/client/cases/bulk_update.ts will not
check a\r\ncase properly if a case is being re-opened, instead of an
else if in the\r\nloop comparing cases to cases in the request, the
status logic should be\r\noutside of this set of if statements. If a
case is being re-opened, the\r\nfinal else is never run, and
casesToAuthorize is 0. This results in\r\nensureAuthorized being called
with an empty array of entities, and so\r\nthe check always succeeds. To
fix this, reopenedCases is still populated\r\nin the same loop, just not
when casesToAuthorize logic is running as\r\nwell. Also adds some
missing tests for this and the create comment\r\npermission as requested
in\r\nhttps://github.com//pull/194898.\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"1c3bceacc06f5f8a01a2ffde2ec24f114717b396"}},"sourceBranch":"main","suggestedTargetBranches":["8.17"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/201517","number":201517,"mergeCommit":{"message":"[Cases]
Fix an issue with the reopen case permission, add integration tests for
failing case (#201517)\n\n## Summary\r\n\r\nCurrently, the
partitionPatchRequest
in\r\nx-pack/plugins/cases/server/client/cases/bulk_update.ts will not
check a\r\ncase properly if a case is being re-opened, instead of an
else if in the\r\nloop comparing cases to cases in the request, the
status logic should be\r\noutside of this set of if statements. If a
case is being re-opened, the\r\nfinal else is never run, and
casesToAuthorize is 0. This results in\r\nensureAuthorized being called
with an empty array of entities, and so\r\nthe check always succeeds. To
fix this, reopenedCases is still populated\r\nin the same loop, just not
when casesToAuthorize logic is running as\r\nwell. Also adds some
missing tests for this and the create comment\r\npermission as requested
in\r\nhttps://github.com//pull/194898.\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"1c3bceacc06f5f8a01a2ffde2ec24f114717b396"}},{"branch":"8.17","label":"v8.17.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Kevin Qualters <56408403+kqualters-elastic@users.noreply.github.com>
paulinashakirova pushed a commit to paulinashakirova/kibana that referenced this pull request Nov 26, 2024
…eopen cases (elastic#194898)

## Summary

This pr adds 2 new sub feature permissions to the cases plugin in
stack/security/observability, that behave as follows. The first is for
controlling the ability to reopen cases. When Cases has the read
permission, and the reopen permission is not enabled, users have
permissions as before. When enabled, users can move cases from closed to
open/in progress, but nothing else. If a user has all and this
permission, they can do anything as before, if the option is unselected,
they can change case properties, and change a case from open to
anything, in progress to anything, but if the case is closed, are unable
to reopen it.

The 2nd permission is 'Add comment'. When enabled and the user has case
read permissions, users can add comments, but not make any other changes
to the case. When the user has read and this deselected, read functions
as before. When a user has this permission and cases is all, this
functions as all. When they have all but this permission is deselected,
the user can do everything normally, except add cases comments.

### Checklist

- [x] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: Michael Olorunnisola <michael.olorunnisola@elastic.co>
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
paulinashakirova pushed a commit to paulinashakirova/kibana that referenced this pull request Nov 26, 2024
… tests for failing case (elastic#201517)

## Summary

Currently, the partitionPatchRequest in
x-pack/plugins/cases/server/client/cases/bulk_update.ts will not check a
case properly if a case is being re-opened, instead of an else if in the
loop comparing cases to cases in the request, the status logic should be
outside of this set of if statements. If a case is being re-opened, the
final else is never run, and casesToAuthorize is 0. This results in
ensureAuthorized being called with an empty array of entities, and so
the check always succeeds. To fix this, reopenedCases is still populated
in the same loop, just not when casesToAuthorize logic is running as
well. Also adds some missing tests for this and the create comment
permission as requested in
elastic#194898.

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Dec 12, 2024
…eopen cases (elastic#194898)

## Summary

This pr adds 2 new sub feature permissions to the cases plugin in
stack/security/observability, that behave as follows. The first is for
controlling the ability to reopen cases. When Cases has the read
permission, and the reopen permission is not enabled, users have
permissions as before. When enabled, users can move cases from closed to
open/in progress, but nothing else. If a user has all and this
permission, they can do anything as before, if the option is unselected,
they can change case properties, and change a case from open to
anything, in progress to anything, but if the case is closed, are unable
to reopen it.

The 2nd permission is 'Add comment'. When enabled and the user has case
read permissions, users can add comments, but not make any other changes
to the case. When the user has read and this deselected, read functions
as before. When a user has this permission and cases is all, this
functions as all. When they have all but this permission is deselected,
the user can do everything normally, except add cases comments.

### Checklist

- [x] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: Michael Olorunnisola <michael.olorunnisola@elastic.co>
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Dec 12, 2024
… tests for failing case (elastic#201517)

## Summary

Currently, the partitionPatchRequest in
x-pack/plugins/cases/server/client/cases/bulk_update.ts will not check a
case properly if a case is being re-opened, instead of an else if in the
loop comparing cases to cases in the request, the status logic should be
outside of this set of if statements. If a case is being re-opened, the
final else is never run, and casesToAuthorize is 0. This results in
ensureAuthorized being called with an empty array of entities, and so
the check always succeeds. To fix this, reopenedCases is still populated
in the same loop, just not when casesToAuthorize logic is running as
well. Also adds some missing tests for this and the create comment
permission as requested in
elastic#194898.

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
apm:review backport:version Backport to applied version labels ci:project-deploy-observability Create an Observability project release_note:feature Makes this part of the condensed release notes Team:obs-ux-management Observability Management User Experience Team v8.17.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.