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

[Security Solution][Endpoint] Add authz to file/download apis in support of SentinelOne processes response action #189127

Merged

Conversation

paul-tavares
Copy link
Contributor

@paul-tavares paul-tavares commented Jul 24, 2024

Summary

The following changes were done for Response actions:

  • The file access APIs (file info + file download) were refactored to ensure they properly validate the required authz for each type of action
    • Now that these APIs are being used for different response actions, we need to add logic that ensures that a user with access to one response action (ex. running-processes for SentinelOne) is not allowed to access file information for a different type of action (ex. get-file)
  • The Response Actions History Log was updated so that the output of a processes response action is now displayed to the user when the response action row in the table is expanded
    • Note that for SentinelOne hosts, the output of the processes response action is a file download - which will only be visible if user has authz to Processes operations
    • The height of the expandable row was also increased in order to provide a larger viewing area for the content that is displayed inside of it (the action results)

image

Checklist

@paul-tavares paul-tavares added release_note:skip Skip the PR/issue when compiling release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution v8.16.0 labels Jul 24, 2024
@paul-tavares paul-tavares self-assigned this Jul 24, 2024
@paul-tavares
Copy link
Contributor Author

/ci

@paul-tavares
Copy link
Contributor Author

/ci

@paul-tavares
Copy link
Contributor Author

/ci

@paul-tavares paul-tavares marked this pull request as ready for review July 29, 2024 14:34
@paul-tavares paul-tavares requested a review from a team as a code owner July 29, 2024 14:34
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-defend-workflows (Team:Defend Workflows)

@paul-tavares paul-tavares requested review from ashokaditya and removed request for szwarckonrad July 29, 2024 14:34
Copy link
Member

@ashokaditya ashokaditya left a comment

Choose a reason for hiding this comment

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

Looks good. I also tested it out. Needs that small change in css argument, else it's good to 🚢 .

const wrappingClassname = useMemo(() => {
return css({
'.accordion-host-name-button-content': {
'font-size': 'inherit',
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
'font-size': 'inherit',
fontSize: 'inherit',

else you get this error on console
Screenshot 2024-07-30 at 15 49 05

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. I don't remember getting this warning 🤔

When you made the change, did you check the UI to ensure it was picking up the correct font size? (no worries if you did not... I can test it out)

Copy link
Member

Choose a reason for hiding this comment

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

yeah the font size was correct with the change.
Screenshot 2024-07-30 at 16 08 40

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cool. thanks.
I'll make this change in the next PR - since I got all approvals here and a green build. Hope that is cool with you.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah that's alright.

@kibana-ci
Copy link
Collaborator

kibana-ci commented Jul 30, 2024

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Jest Integration Tests #4 / Basic smoke test it can start Kibana running against serverless ES

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 5621 5623 +2

Async chunks

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

id before after diff
securitySolution 20.4MB 20.5MB +16.0KB

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @paul-tavares

@paul-tavares paul-tavares merged commit 4cf9d18 into elastic:main Jul 30, 2024
48 checks passed
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Jul 30, 2024
@paul-tavares paul-tavares deleted the task/olm-8131-fix-authz-on-file-apis branch July 30, 2024 15:19
paul-tavares added a commit to paul-tavares/kibana that referenced this pull request Jul 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution v8.16.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants