Skip to content

Fix handling of --quiet flag in fleetctl query - #2

Merged
zwass merged 1 commit into
fleetdm:masterfrom
zwass:fix-fleetctl-query-quiet
Nov 3, 2020
Merged

Fix handling of --quiet flag in fleetctl query#2
zwass merged 1 commit into
fleetdm:masterfrom
zwass:fix-fleetctl-query-quiet

Conversation

@zwass

@zwass zwass commented Nov 3, 2020

Copy link
Copy Markdown
Member

Fixes an issue in which the status was erroneously displayed after results were printed.

@zwass zwass added the bug Something isn't working as documented label Nov 3, 2020
@zwass zwass closed this Nov 3, 2020
@zwass zwass reopened this Nov 3, 2020
@zwass
zwass merged commit 72b0efc into fleetdm:master Nov 3, 2020
@zwass
zwass deleted the fix-fleetctl-query-quiet branch November 3, 2020 23:50
@ghernandez345 ghernandez345 mentioned this pull request Jan 25, 2023
17 tasks
@sharon-fdm sharon-fdm mentioned this pull request May 1, 2023
14 tasks
@marcosd4h marcosd4h mentioned this pull request Sep 20, 2023
2 tasks
akuthiala pushed a commit that referenced this pull request May 18, 2026
Add article on choosing supported Linux distributions for organizations
lucasmrod added a commit that referenced this pull request Jul 1, 2026
#48351)

Resolves #48378 (issue found
while working on the Google Workspace IdP integration).

## Summary

Fixes a bug where an IdP user associated with **multiple hosts** only
had IdP host vitals populated on **one** of them.

`maybeAssociateScimUserWithHostMDMIdP` (called when a SCIM/IdP user is
created) matched all hosts whose MDM IdP account corresponds to the
user, but then deliberately linked only `hostIDs[0]` (with a `// TODO:
confirm desired behavior` / "just use the first one"). So when a user is
created *after* the hosts already enrolled — e.g. a directory sync
creating users for people who each have a laptop and a desktop — only
the first host got a `host_scim_user` row, and therefore only that host
received the user's IdP host vitals and profile-variable resends.

The fix links **every** matching host. `associateHostWithScimUser` is
keyed on `host_id` (`INSERT … ON DUPLICATE KEY UPDATE`) and triggers its
own per-host profile resend, so calling it once per host is safe and
idempotent.

This is shared SCIM linking code, so the fix benefits all IdP sources
(Okta/Entra SCIM as well as the Google Workspace directory sync that
surfaced it). Deletes and updates already handled multiple hosts
correctly; only the initial reverse-link was capped.

## Testing

Added `testScimUserCreateAssociatesAllMatchingHosts`
(`server/datastore/mysql/scim_test.go`): two hosts share one MDM IdP
account, then a SCIM user is created — both hosts must resolve to it via
`ScimUserByHostID`. Fails before the fix (host #2 unlinked), passes
after.

**Related issue:** Resolves #48378

# Checklist for submitter

- [x] Changes file added for user-visible changes in `changes/`.
- [x] Input data is properly validated, `SELECT *` is avoided, SQL
injection is prevented (using placeholders for values in statements).

## Testing

- [x] Added/updated automated tests
- [X] QA'd all new/changed functionality manually

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Bug Fixes**
* SCIM/IdP user provisioning now associates a new SCIM user with **all**
matching hosts, not just the first match.
* Host end-user details (including IdP username/full name) are now
populated consistently on every associated host.
* **Tests**
* Added SCIM integration and datastore regression coverage to ensure
multiple hosts linked to the same IdP account are all associated during
user creation.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
RachelElysia added a commit that referenced this pull request Jul 21, 2026
- (#1) Swap useEffect/[showSearch] focus for native autoFocus on the
  search input. react-select unmounts <Menu> on close, so the input is a
  fresh DOM node on each open — autoFocus fires reliably without
  depending on effect deps semantics.
- (#2) Key CustomMenuList's useLayoutEffect on React.Children.count(...)
  so it only re-measures scroll dimensions when the option list actually
  changes (search filter shrink/expand). onScroll still covers user-
  driven position changes.
- (#3) menu.zIndex 6 -> 99 (page-overlay tier), __scroll-fade z-index
  1 -> 9 (in-flow overlay tier), matching the 9/99/999 convention.
- (#4) Add optional onAddFleet callback prop. If provided, fires
  instead of the browserHistory.push fallback — lets consumers use
  their own router.push. Nine current callers unaffected.
- (#8) TODOL -> TODO: typo in the tests file (inherited from the
  deleted TeamsDropdown.tests.tsx).
RachelElysia added a commit that referenced this pull request Jul 21, 2026
- (#1) Extract getHiddenInput() helper. The double-cast to reach
  react-select's undocumented `inputRef` field was duplicated at two
  sites (menu-open focus effect + forwardNavKey bridge). Both now
  read through the helper — if react-select ever renames the field,
  the escape hatch fails in one place, not two.

- (#2) Stash onClose in onCloseRef so an inline callback from a
  parent doesn't retrigger the menuIsOpen-transition effect on every
  parent render. The effect deps are back to [menuIsOpen] alone and
  reads onCloseRef.current?.() at fire time.

- (#5) Extract isPrimoModeEnabled / isGitOpsModeEnabled locals
  instead of inlining the three nested optional-chain checks. Matches
  the permissions.isPrimoMode / permissions.isSandboxMode naming
  used elsewhere in the codebase and reads at a glance.
RachelElysia added a commit that referenced this pull request Jul 22, 2026
  stale comment, side effects in state updater

- (#2) Rework the scroll-fade to a 0-height sticky anchor with an
  absolutely-positioned ::before pseudo-element toggled via opacity on
  a --visible modifier. The old approach mounted/unmounted a 35px
  sticky div, so MenuList.scrollHeight grew and shrank as the fade
  toggled — browsers can clamp scrollTop when scrollHeight shrinks,
  causing a visible jump near the end of the list. The anchor is
  always in the DOM at 0 flow height; scroll metrics stay stable.

- (#3) Chain react-select's own innerProps.onScroll and innerProps.
  onMouseDown before our custom logic. The previous spread-then-
  override pattern silently dropped whatever react-select provided,
  which could break its own scroll-to-highlighted-option or focus
  tracking on a future version.

- (#5) Fix stale comment on the hidden-input-focus useEffect. It said
  "CustomMenuList focuses the search input on mount," but the search
  input now lives in CustomMenu and uses native autoFocus. Corrected
  the comment to reflect where focus actually comes from.

- (#6) Move side effects (onOpen? / setSearchQuery) out of the
  setMenuIsOpen state updater in toggleMenu. React's Strict Mode
  double-invokes state updaters, which would double-fire onOpen and
  double-clear searchQuery. Reading menuIsOpen directly is safe — a
  single user click can't race with itself.
RachelElysia added a commit that referenced this pull request Jul 22, 2026
- (#1) preventDefault on Enter in addFleetKeyDown. Fleet Button's own
  handleKeyDown synthesizes onClick from Enter, so without preventDefault
  the browser ALSO synthesizes a native click after keyup, firing
  onClickAddFleet a second time. jsdom didn't catch this (it doesn't
  synthesize click from keyDown); the double-fire only manifests in a
  real browser. Space is left with stopPropagation only — Fleet Button
  has no Space handler, and native click synthesis is Space's only
  activation path.

- (#2) Dev-only console.warn in getHiddenInput() when
  SelectInstance.inputRef is unexpectedly missing. Surfaces a
  react-select upgrade breaking the keydown-forwarding bridge loudly
  in dev instead of silently regressing keyboard nav.

- (#4) Consolidate searchQuery clearing into the menuIsOpen transition
  effect. The same setSearchQuery("") was duplicated at 5 close origins
  (click-outside, toggleMenu, handleChange, onClickAddFleet,
  onMenuClose). All 5 removed; the transition effect is now the single
  source of truth for close-side state cleanup.

- (#6) Story decorator: height: 600 -> minHeight: 780. The old fixed
  height clipped scrollable-list stories when the open menu grew past
  600px; minHeight 780 (menu maxHeight 715 + trigger + margin) lets the
  full menu render without clipping.
lucasmrod pushed a commit that referenced this pull request Aug 6, 2026
**Related issue:** Resolves #49483

## What & why

The `/os_versions` API endpoint returned misleading success responses
for three invalid inputs. This PR makes each return a proper error:

1. **Invalid `platform` filter** (e.g. `?platform=notrealplatform`)
previously returned `count: 0` with `200 OK`, indistinguishable from "no
matching OS versions." It now returns a `422` validation error listing
the supported platforms (`darwin`, `windows`, `linux`, `chrome`, `ios`,
`ipados`, `android` — matching the documented filter values).

2. **Unknown OS version id** (e.g. `/os_versions/99999`) previously
returned `200 OK` with a null/zero-filled `os_version` object. It now
returns a not-found (`404`) error.

3. **Negative `max_vulnerabilities`** (e.g. `?max_vulnerabilities=-5`)
returned a message reading `must be >= 0` — Go's JSON encoder
HTML-escapes `>`. The message is reworded to `max_vulnerabilities cannot
be negative`, which is clearer and avoids the escaped character.

### ⚠️ Note for reviewer (fix #2)
The single-version handler previously swallowed the datastore's
not-found error and returned an empty result on purpose, with the
comment: *"It is possible the os version exists, but the aggregation job
has not run yet."* This PR removes that swallow so a missing id returns
`404`. If you'd prefer to preserve the empty-result behavior for the
"not yet aggregated" case, I'm happy to adjust — flagging so the change
is intentional and visible.

# Checklist for submitter

- [x] Changes file added for user-visible changes in `changes/`.
- [x] Input data is properly validated, `SELECT *` is avoided, SQL
injection is prevented.

## Testing

- [x] Added/updated automated tests (`TestOSVersionsErrorHandling` in
`server/service/hosts_test.go`, covering all three cases).
- [x] QA'd all new/changed functionality manually

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Bug Fixes**
* Unsupported OS platform filters now return a clear validation error
instead of an empty-like result.
* Unknown OS version IDs now return HTTP **404 Not Found** rather than a
success response with null/zero fields.
* `max_vulnerabilities` validation now rejects negative values with an
accurate, readable message and consistent HTTP **422** responses.
* Error responses for OS versions endpoints now reflect the correct
status codes.
* **Tests**
* Updated and added coverage to assert the new error-handling and HTTP
status expectations.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working as documented

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant