-
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 Solution][Endpoint] Adds new integration tests for Endpoint Artifacts API RBAC #143273
[Security Solution][Endpoint] Adds new integration tests for Endpoint Artifacts API RBAC #143273
Conversation
…ration_tests-4921
… using new test roles
Pinging @elastic/security-onboarding-and-lifecycle-mgt (Team:Onboarding and Lifecycle Mgt) |
@elasticmachine merge upstream |
…ration_tests-4921
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.
Looks good. Thank you.
I did notice something that did not appear right to me - some tests are checking for a expect(400)
error. 400
errors are normally due to user input, and I would not have expected any of these tests to return a 400
. Do you know why that is?
@paul-tavares Yes, I think all
Does that makes sense to you? |
Ok... Cool. thanks for that and yes, that makes sense. Did not realize we were doing schema type of validation from FTR. 👍 |
💛 Build succeeded, but was flaky
Failed CI StepsTest FailuresMetrics [docs]Unknown metric groupsESLint disabled in files
ESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
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.
Thanks for all the detailed tests 🔥 I have a minor suggestion but this is good to 🚢
'item_id' | 'namespace_type' | 'os_types' | 'tags' | 'entries' | ||
>; | ||
type HostIsolationExceptionApiCallsInterface<BodyGetter = UnknownBodyGetter> = Array<{ | ||
method: keyof Pick<typeof supertest, 'post' | 'put' | 'get' | 'delete' | 'patch'>; |
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.
Don't have to change but you could use RouteMethod
here instead that is imported as
import type { RouteMethod } from '@kbn/core-http-server';
describe('and has authorization to manage endpoint security', () => { | ||
for (const blocklistApiCall of blocklistApiCalls) { | ||
it(`should error on [${blocklistApiCall.method}] if invalid condition entry fields are used`, async () => { | ||
const body = blocklistApiCall.getBody(); | ||
|
||
body.entries[0].field = 'some.invalid.field'; | ||
await supertest[blocklistApiCall.method](blocklistApiCall.path) | ||
await supertestWithoutAuth[blocklistApiCall.method](blocklistApiCall.path) | ||
.auth(ROLE.analyst_hunter, 'changeme') |
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.
Consider updating this role to have a username and the default password so you don't have to duplicate the default password in every test. So it can be used as:
.auth(ROLE.analyst_hunter.username, ROLE.analyst_hunter.password)
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.
Interesting, we can work on this on a subsequent pr! Thanks for the suggestion!
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.
The fleet side does something similar as I learned recently.
… Artifacts API RBAC (elastic#143273) ## Summary - Add new test cases checking RBAC privileges for all artifacts integration tests ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) Co-authored-by: Kibana Machine <[email protected]>
Summary
For maintainers