-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[SECURITY] Deprecated sub feature cases in security solutions #112695
Conversation
Pinging @elastic/kibana-security (Team:Security) |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
const statusCode = getErrorStatusCode(e); | ||
const isUnauthorized = statusCode === 403; | ||
const message = isUnauthorized | ||
? `You must have the 'manage_security' cluster privilege to fix role deprecations.` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need to convert that to i18n and the other ones
x-pack/plugins/security_solution/server/deprecation_privileges/index.test.ts
Show resolved
Hide resolved
x-pack/plugins/security_solution/server/deprecation_privileges/index.test.ts
Show resolved
Hide resolved
`); | ||
}); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also imagine we can have [minimal_read, cases_read, cases_all]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will add them just in case
}, []); | ||
|
||
const casePrivileges = | ||
siemPrivileges.includes('minimal_read') || siemPrivileges.includes('minimal_all') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is probably an impossible scenario but could a role ever have:
siemPrivileges: [all, cases_read]
If that happens we'd give the role cases all
when it had only read
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not think that's possible, let me ask the expert @legrego
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@XavierM The UI will not permit this, but there is nothing stopping an API consumer from specifying this. If both all
and cases_read
are specified, then they will get the sum of those two privileges, which I expect would just be all
, as cases_read
is a subset of all
IIRC.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will add these tests too
I created this PR instead because the deprecation can only go in 7.x and I created another PR to add privilege deprecations services in security plugin I applied all the review form @jonathan-buttner in the PR below |
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
Public APIs missing exports
Page load bundle
History
To update your PR or re-run it, just comment with: |
Summary
Create an Upgrade Assistant automated fix to rewrite roles that are granting access to the Cases sub-feature to use the new top-level Cases feature
#109158
Checklist
Delete any items that are not applicable to this PR.