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.

Sign up for GitHub

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

Jump to bottom

Plugin selector has wrong inital state. #891

Closed
VakarisZ opened this issue Nov 26, 2020 · 2 comments · Fixed by #920
Closed

Plugin selector has wrong inital state. #891

VakarisZ opened this issue Nov 26, 2020 · 2 comments · Fixed by #920
Assignees
@mssalvatore
Labels
Bug An error, flaw, misbehavior or failure in the Monkey or Monkey Island. Complexity: Low Impact: Low UI User Interface

Comments

@VakarisZ
Copy link
Contributor

Describe the bug

If you disable all plugins in the config UI, after refreshing plugin selector initiates with a checkmark for checked instead of empty.

image

To Reproduce

Steps to reproduce the behavior:

  1. Disable all post breach actions
  2. Submit config
  3. Refresh page
  4. Post breach action checkmark is checked even though none is selected.

Expected behavior

If no plugins are selected, "check all" checkmark shouldn't be selected
@VakarisZ VakarisZ added Bug An error, flaw, misbehavior or failure in the Monkey or Monkey Island. UI User Interface Impact: Low Complexity: Low labels Nov 26, 2020
@mssalvatore
Copy link
Collaborator

This issue also affects "System info collectors".

@mssalvatore mssalvatore self-assigned this Jan 5, 2021
@VakarisZ
Copy link
Contributor Author

VakarisZ commented Jan 7, 2021

Yes, this bug is in the UI component code, it affects all fields, which are displayed using this component. As this component is reused, you'll only need to solve it in one place - monkey_island/cc/ui/src/components/ui-components/AdvancedMultiSelect.js

mssalvatore added a commit to mssalvatore/monkey that referenced this issue Jan 12, 2021
@mssalvatore
WIP: ui: Enable mixed-state behavior for master checkbox in AdavncedM…
5d667f5 
…ultiSelect

The AdvancedMultiSelect should adhere to some set of human interface
guidelines. In the absence of a formal, agreed upon set of guidelines
for Infection Monkey, this commit uses KDE's guidelines for checkboxes:
https://hig.kde.org/components/editing/checkbox.html

When child checkboxes are not all checked, the master checkbox displays
a mixed-state icon, instead of a checked icon. Clicking the mixed-state
icon checks all child checkboxes. Clicking an unchecked master checkbox
also enables all child checkboxes.

In the past, clicking an unchecked master checkbox checked only the
*default* child checkboxes. While this may seem desirable so that unsafe
exploits do not accidentally get selected by the user, it will confuse
and frustrate users, as master/child checkboxes do not normally function
this way. If there is concern that users may unknowingly select unsafe
exploits/options, we should pop up a warning to inform the user when the
config is saved/submitted.

Issue guardicore#891
mssalvatore added a commit to mssalvatore/monkey that referenced this issue Jan 12, 2021
@mssalvatore
WIP: ui: Enable mixed-state behavior for master checkbox in AdavncedM…
11c52dc 
…ultiSelect

The AdvancedMultiSelect should adhere to some set of human interface
guidelines. In the absence of a formal, agreed upon set of guidelines
for Infection Monkey, this commit uses KDE's guidelines for checkboxes:
https://hig.kde.org/components/editing/checkbox.html

When child checkboxes are not all checked, the master checkbox displays
a mixed-state icon, instead of a checked icon. Clicking the mixed-state
icon checks all child checkboxes. Clicking an unchecked master checkbox
also enables all child checkboxes.

In the past, clicking an unchecked master checkbox checked only the
*default* child checkboxes. While this may seem desirable so that unsafe
exploits do not accidentally get selected by the user, it will confuse
and frustrate users, as master/child checkboxes do not normally function
this way. If there is concern that users may unknowingly select unsafe
exploits/options, we should pop up a warning to inform the user when the
config is saved/submitted.

Issue guardicore#891
mssalvatore added a commit to mssalvatore/monkey that referenced this issue Jan 12, 2021
@mssalvatore
ui: Enable mixed-state behavior for master checkbox in AdavncedMultiS…
19bc091 
…elect

The AdvancedMultiSelect should adhere to some set of human interface
guidelines. In the absence of a formal, agreed upon set of guidelines
for Infection Monkey, this commit uses KDE's guidelines for checkboxes:
https://hig.kde.org/components/editing/checkbox.html

When child checkboxes are not all checked, the master checkbox displays
a mixed-state icon, instead of a checked icon. Clicking the mixed-state
icon checks all child checkboxes. Clicking an unchecked master checkbox
also enables all child checkboxes.

In the past, clicking an unchecked master checkbox checked only the
*default* child checkboxes. While this may seem desirable so that unsafe
exploits do not accidentally get selected by the user, it will confuse
and frustrate users, as master/child checkboxes do not normally function
this way. If there is concern that users may unknowingly select unsafe
exploits/options, we should pop up a warning to inform the user when the
config is saved/submitted.

Issue guardicore#891
@mssalvatore mssalvatore mentioned this issue Jan 12, 2021
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mssalvatore mssalvatore

Labels
Bug An error, flaw, misbehavior or failure in the Monkey or Monkey Island. Complexity: Low Impact: Low UI User Interface
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Improve advanced multiselect mssalvatore/monkey
2 participants
@mssalvatore @VakarisZ