Skip to content

[EDR Workflows][Device Control] Trusted Devices cypress coverage#232269

Merged
szwarckonrad merged 165 commits intoelastic:mainfrom
szwarckonrad:device-control-cypress
Aug 28, 2025
Merged

[EDR Workflows][Device Control] Trusted Devices cypress coverage#232269
szwarckonrad merged 165 commits intoelastic:mainfrom
szwarckonrad:device-control-cypress

Conversation

@szwarckonrad
Copy link
Copy Markdown
Contributor

@szwarckonrad szwarckonrad commented Aug 19, 2025

Prerequisite #231888
Followup #232374
This PR brings comprehensive Cypress test coverage to Trusted Devices, mirroring the established Trusted Apps testing patterns. We implemented full CRUD operations testing, RBAC permissions validation, and serverless PLI/tier access controls.

szwarckonrad and others added 30 commits July 22, 2025 13:02
szwarckonrad added a commit that referenced this pull request Aug 26, 2025
Prerequisite #231167
Followup #232269

This PR implements server-side validation for trusted devices in the
Kibana by extending the existing list plugin extension points system.
The implementation follows established patterns from trusted apps but
supports trusted device-specific requirements, including allowing both
Windows and Mac OS types (unlike other artifact types that only support
single OS per entry) and validating the 5 supported device fields
(`username`, `host`, `device ID`, `manufacturer`, `product ID`). The
changes include a new `TrustedDeviceValidator` class, integration across
all 9 exception list handlers, and comprehensive API integration tests
to ensure proper validation and authorization.

  Key changes:
- New `TrustedDeviceValidator` class extending BaseValidator with full
space awareness
- Updated all 9 list plugin extension point handlers to include trusted
device validation
- Added comprehensive integration tests covering unique trusted device
validation requirements
- Maintains consistency with existing artifact validation patterns while
supporting device-specific schema

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
@szwarckonrad szwarckonrad marked this pull request as ready for review August 26, 2025 11:21
@szwarckonrad szwarckonrad requested review from a team as code owners August 26, 2025 11:21
@elasticmachine
Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown
Contributor

@tomsonpl tomsonpl left a comment

Choose a reason for hiding this comment

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

Nice! I haven't written cypress tests in a while, so even more happy to see you doing it :) LGTM

Copy link
Copy Markdown
Contributor

@gergoabraham gergoabraham left a comment

Choose a reason for hiding this comment

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

nice work on the tests! 🚀

my only issue is the discrepancy between role definitions, would be great to sort those out

@szwarckonrad szwarckonrad requested a review from a team as a code owner August 27, 2025 09:02
- feature_siemV3.endpoint_list_all
- feature_siemV3.global_artifact_management_all
- feature_siemV3.trusted_applications_all
- feature_siemV3.trusted_devices_all
Copy link
Copy Markdown
Member

@pheyos pheyos Aug 27, 2025

Choose a reason for hiding this comment

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

It looks like this change in not present in the elasticsearch-controller (yet?). We usually try to keep the role definitions in sync. Otherwise, any testing that relies on this extra privilege might pass locally but fail in MKI runs. What are your plans around this here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This feature targets 9.2 and we will most likely add this privilege before the feature freeze. I'm okay with reverting this change and revisiting once we merged controller. CC @gergoabraham

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@pheyos, as we do not have custom roles for serverless in our test suites at the moment, I think there must be a discrepancy somewhere to be able to use the new privileges in cy/ftr tests. the discrepancy is either between elasticsearch-controller and kibana (as the current PR introduces it), or between (elasticsearch-controller + kibana's kbn-es resource) and (kibana's test resource).

do you think one is better than the other? i think updating elasticsearch-controller later should be fine, as this allows us to locally test the new roles, and have CI tests in place for the new features, while these tests are skipped in MKI. but i guess we can go the other way around as well - keeping the new privileges out of kbn-es resource

Copy link
Copy Markdown
Contributor Author

@szwarckonrad szwarckonrad Aug 27, 2025

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@gergoabraham @szwarckonrad it's true that we can't always be in perfect sync and there's risk in every version of discrepancy.

CI tests in place for the new features, while these tests are skipped in MKI

In this case, I think it makes sense to go this route. If any FTR tests start failing in MKI due to this role definition change (unlikely unless we explicitly add specific tests for it - the tests seem to be mostly in Cypress), then we would have to skip these tests for MKI runs until the role definition in elasticsearch-controller has caught up. With that, I'm going to approve this PR from the AppEx QA point of view.

@szwarckonrad szwarckonrad requested a review from pheyos August 27, 2025 13:35
Copy link
Copy Markdown
Member

@pheyos pheyos left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Contributor

@gergoabraham gergoabraham left a comment

Choose a reason for hiding this comment

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

thanks for the tests and the changes! 🚀

@szwarckonrad szwarckonrad merged commit 56dabb9 into elastic:main Aug 28, 2025
12 checks passed
qn895 pushed a commit to qn895/kibana that referenced this pull request Sep 2, 2025
…stic#232269)

Prerequisite elastic#231888
Followup elastic#232374
This PR brings comprehensive Cypress test coverage to Trusted Devices,
mirroring the established Trusted Apps testing patterns. We implemented
full CRUD operations testing, RBAC permissions validation, and
serverless PLI/tier access controls.

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
szwarckonrad added a commit that referenced this pull request Oct 16, 2025
)

Prerequisite #232269
Enabled on `9.2` with #237302
This PR allows us to:
1. Verify that enabling the feature doesn't introduce regressions
2. Ensure all Device Control-related tests pass with flag turned on
3. Prepare for seamless cloud deployments with unified configuration

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Gergő Ábrahám <gergo.abraham@elastic.co>
nickpeihl pushed a commit to nickpeihl/kibana that referenced this pull request Oct 23, 2025
…tic#232374)

Prerequisite elastic#232269
Enabled on `9.2` with elastic#237302
This PR allows us to:
1. Verify that enabling the feature doesn't introduce regressions
2. Ensure all Device Control-related tests pass with flag turned on
3. Prepare for seamless cloud deployments with unified configuration

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Gergő Ábrahám <gergo.abraham@elastic.co>
NicholasPeretti pushed a commit to NicholasPeretti/kibana that referenced this pull request Oct 27, 2025
…tic#232374)

Prerequisite elastic#232269
Enabled on `9.2` with elastic#237302
This PR allows us to:
1. Verify that enabling the feature doesn't introduce regressions
2. Ensure all Device Control-related tests pass with flag turned on
3. Prepare for seamless cloud deployments with unified configuration

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Gergő Ábrahám <gergo.abraham@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR 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 v9.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants