UI: Set Mac Recovery Lock passwords - #41166
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #41166 +/- ##
==========================================
- Coverage 66.31% 66.31% -0.01%
==========================================
Files 2470 2476 +6
Lines 197878 198233 +355
Branches 8774 8924 +150
==========================================
+ Hits 131232 131454 +222
- Misses 54767 54900 +133
Partials 11879 11879
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughAdds frontend support for Recovery Lock password features: new MDM config fields at global and team scope, new activity types and activity item components, a Passwords card under OS Settings with toggle and save behavior, a host Recovery Lock Password modal and related host action option, a new hosts service method and endpoint for fetching passwords, updated OS settings navigation and side‑nav typing, and UI/SCSS for the new screens and components. Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/pages/hosts/details/HostDetailsPage/HostActionsDropdown/helpers.tsx (1)
79-105:⚠️ Potential issue | 🟠 Major
darwinis too broad for Recovery Lock.The Passwords card already says this feature is only available on Apple silicon hosts, but this gate exposes the action on every macOS host. On unsupported Macs users will still see "Show Recovery Lock password" even though the flow can't succeed. Add a host capability flag to
IHostActionConfigOptionsand require it here before keeping the option.Also applies to: 290-307
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/pages/hosts/details/HostDetailsPage/HostActionsDropdown/helpers.tsx` around lines 79 - 105, Add a specific capability flag to the host action options (e.g., add hostSupportsRecoveryLock: boolean to the IHostActionConfigOptions interface) and use that flag in the decision that enables the "Show Recovery Lock password" action (the gating logic that currently checks isMacMdmEnabledAndConfigured / darwin mac checks). Update all places that construct/consume IHostActionConfigOptions to supply hostSupportsRecoveryLock, and change the condition that shows the Recovery Lock action to require hostSupportsRecoveryLock === true in addition to any existing macOS checks so the action only appears on Apple silicon hosts that support Recovery Lock.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/interfaces/host.ts`:
- Around line 115-130: The backend is missing a handler for the GET
/fleet/hosts/{id}/recovery_lock_password endpoint required by frontend
hostAPI.getRecoveryLockPassword and the IHostRecoveryLockPasswordResponse
contract; implement a new Go HTTP handler (and register its route) that accepts
host id, validates permissions, loads the host recovery lock entry, and returns
JSON shaped as { host_id: <id>, recovery_lock_password: { updated_at:
<timestamp>, password: <string|null> } } (use the same field names/types as the
frontend IHostRecoveryLockPasswordResponse), and ensure proper error responses
and tests cover missing host, unauthorized access, and empty password cases.
In
`@frontend/pages/hosts/details/HostDetailsPage/modals/RecoveryLockPasswordModal/_styles.scss`:
- Around line 2-3: The font-family declaration inside the .input-field rule uses
quoted "SourceCodePro" which Stylelint flags; update the .input-field
font-family to use an unquoted identifier (e.g., font-family: SourceCodePro,
$monospace;) so SourceCodePro is not wrapped in quotes while keeping the
fallback $monospace variable intact.
In
`@frontend/pages/hosts/details/HostDetailsPage/modals/RecoveryLockPasswordModal/RecoveryLockPasswordModal.tsx`:
- Around line 34-40: The RecoveryLockPasswordModal is caching the plaintext
password via the useQuery call (["hostRecoveryLockPassword", hostId] ->
hostAPI.getRecoveryLockPassword), so update that useQuery options to avoid
caching sensitive data by adding cacheTime: 0 and retry: false (e.g., merge into
the existing DEFAULT_USE_QUERY_OPTIONS) or alternatively ensure the query is
removed on modal close by calling
queryClient.removeQueries(["hostRecoveryLockPassword", hostId]) from the
cleanup/close handler; target the useQuery invocation in
RecoveryLockPasswordModal to apply the change.
In
`@frontend/pages/hosts/details/OSSettingsModal/OSSettingsTable/OSSettingsTableConfig.tsx`:
- Around line 166-173: The current early return in makeDarwinRows when profiles
is falsy causes valid synthetic macOS settings (e.g., recovery_lock_password
from os_settings) to be dropped; instead of returning null inside
makeDarwinRows, initialize an empty rows array when profiles is missing and
continue building rows so you can still push the synthetic entry based on
macos_settings or os_settings.recovery_lock_password; update the logic that
creates the synthetic row to check os_settings.recovery_lock_password and
macos_settings and append it to rows even if profiles is undefined, and return
rows (or null only if no rows were produced).
In `@frontend/pages/ManageControlsPage/OSSettings/cards/Passwords/Passwords.tsx`:
- Around line 54-82: The form allows submission before the team query succeeds
because enableRecoveryLockPassword is initialized to false and the useQuery call
(teamsAPI.load) doesn't handle errors; update the logic so the query explicitly
handles onError (e.g., set an error flag or surface a toast) and only considers
the team value valid when the query is successful (use isSuccess/isError from
useQuery); change enableRecoveryLockPassword's initial state to undefined (or a
nullable type) to represent "unknown", update the save handler to block
submission unless currentTeamId === API_NO_TEAM_ID or the query is not loading
and isSuccess is true, and update showLoading (or add showBlockedState) to
reflect isLoadingTeam || isError so the form is disabled until the team data is
successfully loaded; reference useQuery(["team", currentTeamId], ...),
onSuccess, onError, isLoadingTeam, isSuccess, enableRecoveryLockPassword,
setEnableRecoveryLockPassword, and showLoading when making these changes.
---
Outside diff comments:
In
`@frontend/pages/hosts/details/HostDetailsPage/HostActionsDropdown/helpers.tsx`:
- Around line 79-105: Add a specific capability flag to the host action options
(e.g., add hostSupportsRecoveryLock: boolean to the IHostActionConfigOptions
interface) and use that flag in the decision that enables the "Show Recovery
Lock password" action (the gating logic that currently checks
isMacMdmEnabledAndConfigured / darwin mac checks). Update all places that
construct/consume IHostActionConfigOptions to supply hostSupportsRecoveryLock,
and change the condition that shows the Recovery Lock action to require
hostSupportsRecoveryLock === true in addition to any existing macOS checks so
the action only appears on Apple silicon hosts that support Recovery Lock.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ac12d473-a8f7-4251-bcc1-db694c98bd4d
📒 Files selected for processing (32)
frontend/__mocks__/configMock.tsfrontend/docs/patterns.mdfrontend/interfaces/activity.tsfrontend/interfaces/config.tsfrontend/interfaces/host.tsfrontend/interfaces/mdm.tsfrontend/interfaces/team.tsfrontend/pages/DashboardPage/cards/ActivityFeed/GlobalActivityItem/GlobalActivityItem.tests.tsxfrontend/pages/DashboardPage/cards/ActivityFeed/GlobalActivityItem/GlobalActivityItem.tsxfrontend/pages/ManageControlsPage/OSSettings/OSSettings.tsxfrontend/pages/ManageControlsPage/OSSettings/OSSettingsNavItems.tsxfrontend/pages/ManageControlsPage/OSSettings/cards/Passwords/Passwords.tsxfrontend/pages/ManageControlsPage/OSSettings/cards/Passwords/_styles.scssfrontend/pages/ManageControlsPage/OSSettings/cards/Passwords/index.tsfrontend/pages/admin/components/SideNav/SideNav.tsxfrontend/pages/hosts/details/HostDetailsPage/HostActionsDropdown/HostActionsDropdown.tests.tsxfrontend/pages/hosts/details/HostDetailsPage/HostActionsDropdown/HostActionsDropdown.tsxfrontend/pages/hosts/details/HostDetailsPage/HostActionsDropdown/helpers.tsxfrontend/pages/hosts/details/HostDetailsPage/HostDetailsPage.tsxfrontend/pages/hosts/details/HostDetailsPage/modals/RecoveryLockPasswordModal/RecoveryLockPasswordModal.tsxfrontend/pages/hosts/details/HostDetailsPage/modals/RecoveryLockPasswordModal/_styles.scssfrontend/pages/hosts/details/HostDetailsPage/modals/RecoveryLockPasswordModal/index.tsfrontend/pages/hosts/details/OSSettingsModal/OSSettingsTable/OSSettingsTableConfig.tsxfrontend/pages/hosts/details/cards/Activity/ActivityConfig.tsxfrontend/pages/hosts/details/cards/Activity/ActivityItems/SetHostRecoveryLockPassword/SetHostRecoveryLockPassword.tsxfrontend/pages/hosts/details/cards/Activity/ActivityItems/SetHostRecoveryLockPassword/index.tsfrontend/pages/hosts/details/cards/Activity/ActivityItems/ViewedHostRecoveryLockPassword/ViewedHostRecoveryLockPassword.tsxfrontend/pages/hosts/details/cards/Activity/ActivityItems/ViewedHostRecoveryLockPassword/index.tsfrontend/pages/hosts/details/helpers.tsfrontend/router/paths.tsfrontend/services/entities/hosts.tsfrontend/utilities/endpoints.ts
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/pages/hosts/details/OSSettingsModal/OSSettingsTable/OSSettingsTableConfig.tsx (1)
166-194: Inconsistent return behavior compared to other platform handlers.
makeWindowsRowsandmakeLinuxRowsreturnnullwhen there are no rows and no profiles (lines 133-134, 159-160), butmakeDarwinRowsnow always returns an array (possibly empty). This inconsistency could affect callers that check fornullto determine if data exists.Consider adding the same guard at the end:
♻️ Suggested fix
if (os_settings?.recovery_lock_password?.status) { rows = [ ...rows, generateRecoveryLockPasswordSetting( os_settings.recovery_lock_password.status, os_settings.recovery_lock_password.detail ), ]; } + if (rows.length === 0 && !profiles) { + return null; + } + return rows; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/pages/hosts/details/OSSettingsModal/OSSettingsTable/OSSettingsTableConfig.tsx` around lines 166 - 194, makeDarwinRows currently always returns an array (possibly empty) which is inconsistent with makeWindowsRows/makeLinuxRows that return null when there are no rows; update makeDarwinRows (function name: makeDarwinRows, variables: rows, profiles) to return null when rows is empty (i.e., no profiles and no added settings) by adding a final guard that returns rows if non-empty and null otherwise.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@frontend/pages/hosts/details/OSSettingsModal/OSSettingsTable/OSSettingsTableConfig.tsx`:
- Around line 166-194: makeDarwinRows currently always returns an array
(possibly empty) which is inconsistent with makeWindowsRows/makeLinuxRows that
return null when there are no rows; update makeDarwinRows (function name:
makeDarwinRows, variables: rows, profiles) to return null when rows is empty
(i.e., no profiles and no added settings) by adding a final guard that returns
rows if non-empty and null otherwise.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 15ee0a23-55bb-43fd-9058-b8758e1f6e4b
📒 Files selected for processing (8)
frontend/interfaces/host.tsfrontend/pages/ManageControlsPage/OSSettings/cards/Passwords/Passwords.tsxfrontend/pages/hosts/details/HostDetailsPage/HostActionsDropdown/HostActionsDropdown.tsxfrontend/pages/hosts/details/HostDetailsPage/HostActionsDropdown/helpers.tsxfrontend/pages/hosts/details/HostDetailsPage/HostDetailsPage.tsxfrontend/pages/hosts/details/HostDetailsPage/modals/RecoveryLockPasswordModal/RecoveryLockPasswordModal.tsxfrontend/pages/hosts/details/HostDetailsPage/modals/RecoveryLockPasswordModal/_styles.scssfrontend/pages/hosts/details/OSSettingsModal/OSSettingsTable/OSSettingsTableConfig.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/pages/hosts/details/HostDetailsPage/modals/RecoveryLockPasswordModal/_styles.scss
- frontend/interfaces/host.ts
- frontend/pages/hosts/details/HostDetailsPage/modals/RecoveryLockPasswordModal/RecoveryLockPasswordModal.tsx
|
This is a big PR. I'm starting to look at it now. |
| If you are dealing with a page that *updates* any kind of config, set the local | ||
| config with the response of your update call to make sure it has the latest. |
| <Button | ||
| disabled={isFormDisabled || gitopsDisabled} | ||
| className={`${baseClass}__save-button`} | ||
| onClick={onUpdateRecoveryLockPassword} |
There was a problem hiding this comment.
Is there a pattern when to have loading state on save and when not to?
There was a problem hiding this comment.
generally we should
Related issue: Resolves #39723
Checklist for submitter
If some of the following don't apply, delete the relevant line.
changes/Summary by CodeRabbit
New Features
Documentation