Skip to content

[EDR Workflows][Device Control] Username option available only for Windows#233487

Merged
szwarckonrad merged 11 commits intoelastic:mainfrom
szwarckonrad:device-control-username-windows-only
Sep 5, 2025
Merged

[EDR Workflows][Device Control] Username option available only for Windows#233487
szwarckonrad merged 11 commits intoelastic:mainfrom
szwarckonrad:device-control-username-windows-only

Conversation

@szwarckonrad
Copy link
Contributor

@szwarckonrad szwarckonrad commented Aug 29, 2025

Updated trusted devices form to only allow the username field when Windows OS is selected exclusively. The field is now hidden for Mac only or Windows+Mac combinations, with automatic field reset when OS changes.

Changes

  • Frontend: Dynamic field filtering in trusted devices form with automatic reset logic
  • Backend: Server-side validation to enforce username + Windows-only restriction
  • Shared utilities: Added isTrustedDeviceFieldAvailableForOs() function and OS field availability constants
  • Tests: Updated form unit tests and added comprehensive API integration tests
  • Test data: Fixed generators to avoid invalid username + non-Windows combinations
Screen.Recording.2025-09-01.at.11.14.19.mov

@szwarckonrad szwarckonrad self-assigned this Aug 29, 2025
@szwarckonrad szwarckonrad added release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting Team:Defend Workflows “EDR Workflows” sub-team of Security Solution v9.2.0 labels Aug 29, 2025
] as const,

/** Fields available only for Windows OS exclusively */
WINDOWS_ONLY: [TrustedDeviceConditionEntryField.USERNAME] as const,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not used, added for clarity.

Copy link
Contributor

Choose a reason for hiding this comment

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

i'd suggest to use it. it can be easily used in the function below in the same style as the ALL_OS array. therefore we would have a single source of truth, because now you need to modify the array in two places

@szwarckonrad szwarckonrad marked this pull request as ready for review September 1, 2025 14:24
@szwarckonrad szwarckonrad requested review from a team as code owners September 1, 2025 14:24
@elasticmachine
Copy link
Contributor

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

@szwarckonrad szwarckonrad requested review from gergoabraham, paul-tavares and sdesalas and removed request for ashokaditya and pzl September 1, 2025 14:24
] as const,

/** Fields available only for Windows OS exclusively */
WINDOWS_ONLY: [TrustedDeviceConditionEntryField.USERNAME] as const,
Copy link
Contributor

Choose a reason for hiding this comment

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

i'd suggest to use it. it can be easily used in the function below in the same style as the ALL_OS array. therefore we would have a single source of truth, because now you need to modify the array in two places

Comment on lines +83 to +88
// USERNAME field is only available for Windows-only OS selection
if (field === TrustedDeviceConditionEntryField.USERNAME) {
return osTypes.length === 1 && osTypes.includes(OperatingSystem.WINDOWS);
}

// All other fields are available for any OS combination
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: i feel these comments are a bit redundant: if we use WINDOWS_ONLY above, and then ALL_OS here is already used, the comments are not only just taking up place without adding any additional value to an easily understandable function, but are also adding maintenance cost as they need to be updated all the time manually (copilotly?) when code changes happen.

setHasUserSelectedOs(true);
setHasFormChanged(true);

// Determine if we need to reset the field due to USERNAME availability change
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: again, i'd vote against this kind of comment verbosity in general - it gets easily outdated, adds maintenance cost, makes us avoid reading code because it's easier to read comments, but comments may lie.

i'm perfectly fine for adding comments when they it's sensible, because from code it is very hard to find out the whys, but otherwise, i'm all for clean code practices like using clean variable names, like you did here: shouldResetValue speaks on its own.

this is not a finding, these are just my thoughts, and please let me know yours.

Copy link
Contributor

@denar50 denar50 left a comment

Choose a reason for hiding this comment

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

Code owners review only. LGTM!

@maximpn maximpn requested review from maximpn and removed request for sdesalas September 4, 2025 11:20
Copilot AI review requested due to automatic review settings September 5, 2025 09:25
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements OS-specific field availability for trusted devices, restricting the USERNAME field to Windows-only OS selection while allowing other fields for all OS combinations. The changes ensure proper validation both on the frontend (form) and backend (server validation) with comprehensive test coverage.

  • Dynamic field filtering in the trusted devices form based on OS selection
  • Server-side validation to enforce USERNAME field restriction to Windows-only OS
  • Comprehensive test coverage for both form behavior and API validation scenarios

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
trusted_devices.ts Added API integration tests validating USERNAME field OS restrictions
trusted_device_validator.ts Added server-side validation for OS-specific field availability
form.tsx Implemented dynamic field filtering and automatic field reset logic
form.test.tsx Updated unit tests to cover OS-based field availability scenarios
artifacts.ts Fixed selector specificity for OS option selection
artifacts_page.ts Updated test data to use HOST field instead of USERNAME for compatibility
trusted_devices.cy.ts Updated Cypress tests to reflect USERNAME field Windows-only restriction
exceptions_list_item_generator.ts Changed default field from USERNAME to HOST for broader OS compatibility
index.ts Added utility function and constants for OS-specific field availability
index.test.ts Added comprehensive unit tests for the new field availability utility

@szwarckonrad szwarckonrad enabled auto-merge (squash) September 5, 2025 15:05
@szwarckonrad szwarckonrad merged commit 143220f into elastic:main Sep 5, 2025
12 checks passed
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

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
@kbn/securitysolution-utils 63 67 +4

Async chunks

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

id before after diff
lists 126.0KB 126.2KB +234.0B
securitySolution 10.3MB 10.3MB +1.2KB
total +1.4KB

Saved Objects .kibana field count

Every field in each saved object type adds overhead to Elasticsearch. Kibana needs to keep the total field count below Elasticsearch's default limit of 1000 fields. Only specify field mappings for the fields you wish to search on or query. See https://www.elastic.co/guide/en/kibana/master/saved-objects-service.html#_mappings

id before after diff
_data_stream_timestamp 1 - -1
_doc_count 1 - -1
_ignored_source 1 - -1
_index_mode 1 - -1
_inference_fields 1 - -1
_tier 1 - -1
apm-custom-dashboards 5 - -5
apm-server-schema 2 - -2
apm-service-group 5 - -5
application_usage_daily 2 - -2
config 2 - -2
config-global 2 - -2
coreMigrationVersion 1 - -1
created_at 1 - -1
created_by 1 - -1
entity-definition 9 - -9
entity-discovery-api-key 2 - -2
event_loop_delays_daily 2 - -2
favorites 4 - -4
file 11 - -11
file-upload-usage-collection-telemetry 3 - -3
fileShare 5 - -5
infra-custom-dashboards 4 - -4
infrastructure-monitoring-log-view 2 - -2
intercept_trigger_record 5 - -5
legacy-url-alias 7 - -7
managed 1 - -1
ml-job 6 - -6
ml-module 13 - -13
ml-trained-model 7 - -7
monitoring-telemetry 2 - -2
namespace 1 - -1
namespaces 1 - -1
observability-onboarding-state 2 - -2
originId 1 - -1
product-doc-install-status 7 - -7
references 4 - -4
sample-data-telemetry 3 - -3
security-ai-prompt 8 - -8
slo 11 - -11
space 5 - -5
synthetics-monitor 34 - -34
synthetics-monitor-multi-space 34 - -34
tag 4 - -4
type 1 - -1
typeMigrationVersion 1 - -1
ui-metric 2 - -2
updated_at 1 - -1
updated_by 1 - -1
upgrade-assistant-ml-upgrade-operation 3 - -3
upgrade-assistant-reindex-operation 3 - -3
uptime-synthetics-api-key 2 - -2
url 5 - -5
usage-counters 2 - -2
total -246
Unknown metric groups

API count

id before after diff
@kbn/securitysolution-utils 72 76 +4

History

cc @szwarckonrad

shahargl pushed a commit to shahargl/kibana that referenced this pull request Sep 7, 2025
…ndows (elastic#233487)

Updated trusted devices form to only allow the username field when
Windows OS is selected exclusively. The field is now hidden for Mac only
or Windows+Mac combinations, with automatic field reset when OS changes.

Changes

- Frontend: Dynamic field filtering in trusted devices form with
automatic reset logic
- Backend: Server-side validation to enforce username + Windows-only
restriction
- Shared utilities: Added isTrustedDeviceFieldAvailableForOs() function
and OS field availability constants
- Tests: Updated form unit tests and added comprehensive API integration
tests
- Test data: Fixed generators to avoid invalid username + non-Windows
combinations



https://github.com/user-attachments/assets/fcec7391-dd02-4b35-bef9-758815164901

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
KodeRad pushed a commit to KodeRad/kibana that referenced this pull request Sep 15, 2025
…ndows (elastic#233487)

Updated trusted devices form to only allow the username field when
Windows OS is selected exclusively. The field is now hidden for Mac only
or Windows+Mac combinations, with automatic field reset when OS changes.

Changes

- Frontend: Dynamic field filtering in trusted devices form with
automatic reset logic
- Backend: Server-side validation to enforce username + Windows-only
restriction
- Shared utilities: Added isTrustedDeviceFieldAvailableForOs() function
and OS field availability constants
- Tests: Updated form unit tests and added comprehensive API integration
tests
- Test data: Fixed generators to avoid invalid username + non-Windows
combinations



https://github.com/user-attachments/assets/fcec7391-dd02-4b35-bef9-758815164901

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Sep 24, 2025
…ndows (elastic#233487)

Updated trusted devices form to only allow the username field when
Windows OS is selected exclusively. The field is now hidden for Mac only
or Windows+Mac combinations, with automatic field reset when OS changes.

Changes

- Frontend: Dynamic field filtering in trusted devices form with
automatic reset logic
- Backend: Server-side validation to enforce username + Windows-only
restriction
- Shared utilities: Added isTrustedDeviceFieldAvailableForOs() function
and OS field availability constants
- Tests: Updated form unit tests and added comprehensive API integration
tests
- Test data: Fixed generators to avoid invalid username + non-Windows
combinations



https://github.com/user-attachments/assets/fcec7391-dd02-4b35-bef9-758815164901

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
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.

7 participants