feat: Device Security — certificate management UI (CA settings, enrollment approval, revocation)#612
Open
dexion wants to merge 37 commits intonetbirdio:mainfrom
Open
feat: Device Security — certificate management UI (CA settings, enrollment approval, revocation)#612dexion wants to merge 37 commits intonetbirdio:mainfrom
dexion wants to merge 37 commits intonetbirdio:mainfrom
Conversation
added 30 commits
April 14, 2026 15:43
Add DeviceSecuritySettings form component with controls for authentication mode, enrollment mode, CA type, certificate validity, and OCSP settings. Includes cert-only mode warning when no devices have active certificates.
Add enrollments table with approve/reject actions for pending device enrollment requests. Follows existing DataTable patterns with Badge status indicators, truncated IDs, and auto-refresh via provider.
Add AddTrustedCAModal, TrustedCAsTable, and trusted-cas page for managing trusted Certificate Authorities in the device security dashboard section.
…evice security UI
…ent hints, and CRL section
Add descriptive aria-label to the expand/collapse button in CATestPanel StepRow, including the step name for screen reader context. The SettingRow Label in DeviceSecuritySettings.tsx is intentionally left without htmlFor — it is a visual label wrapping Radix Select components that manage their own accessibility internally.
- Add pre-save validation in handleSave for intune (tenant_id, client_id) and jamf (jamf_url) required fields using notify.error - Add aria-describedby="static-peers-help" on static peer IDs textarea and id="static-peers-help" on its HelpText - Extract PEER_SEPARATOR_RE regex constant and use it in the split call - Add notify.error helper to Notification component - Add id prop to HelpText component
…tCA, fix updateRef drift - Remove ocsp_enabled and fail_open_on_ocsp_unavailable from DeviceAuthSettings interface - Fix handleSave to call updateSettings and updateCAConfig as independent notify calls, aborting CA config save if settings save fails - Fix handleTestCA to handle null result and caught errors, always setting testResult so the user gets feedback even on network failure - Fix updateRef drift by calling updateRef after hydrating state from settings/caConfig so hasChanges starts false on fresh load - Extract shared PasswordField into its own file, removing duplicate copies from CAConfigSection.tsx and InventoryPage.tsx
…on providers, hasChanges - Replace single textarea with two-tab UI (NetBird Peers / Serial Numbers) - Peers tab: WireGuard public keys textarea with description - Serials tab: warning callout + serial numbers textarea, stored with serial: prefix - Add status callout at top of page driven by settings.enrollment_mode - success variant when attestation/both is active - warning variant when attestation is not enabled - Add fleetdm (Fleet) and webhook providers to type selector, disabled with Coming soon badge - Wire useHasChanges: Save button disabled until form has unsaved changes, updateRef called after successful save
- handleSave now awaits the save promise sequentially before calling updateRef, matching the pattern in DeviceSecuritySettings.tsx to ensure ref is only updated on confirmed success (no .then() race) - handleTypeChange gains an early-return guard for "fleetdm" and "webhook" types that are disabled/coming-soon, preventing state mutations from disabled radio inputs
- Replace EOL alpine:3.14 base with nginx:alpine (stable, maintained) - Remove supervisor/certbot/apk install - nginx:alpine has everything needed - Convert init_react_envs.sh from bash to POSIX sh (no bash dep needed) - Fix nginx include path: http.d -> conf.d (matches nginx:alpine layout) - Use /docker-entrypoint.d/ hook for runtime env var substitution
- Inventory page redesigned: three independent sources (Static/Intune/Jamf) each with FancyToggleSwitch toggle, enabled simultaneously (e.g. Intune for Windows + Jamf for Mac) - JamfForm updated to OAuth fields (client_id/client_secret) matching backend - Settings page: fix ca_type/mode/enrollment_mode fallback to defaults when backend returns empty strings; remove inventory_type from save payload - TrustedCAs: fix two Add buttons by using rightSide prop instead of children - CAConfigSection: replace gray-*/zinc-* palette with nb-gray-* tokens - trusted-cas/page: replace raw info callout with Callout variant='info' - DeviceAuthSettings interface: remove inventory_type field (removed from API)
Guard isTokenExpired() against undefined token (OIDC loading race) that caused API calls to fire with 'Authorization: Bearer undefined'. Also improve devices page layout (breadcrumbs, description, empty state).
Device Security Settings: - Add require_inventory_check toggle (FancyToggleSwitch) for manual/both enrollment modes - Syncs with backend via DeviceAuthSettings interface (new require_inventory_check field) - Links to Inventory config page in helpText Users / RBAC: - Add CertApprover role to Role enum - UserRoleSelector, UserRoleCell, UserInvitesTable, UsersProvider updated for cert_approver - isCertApprover helper in useLoggedInUser()
added 7 commits
April 14, 2026 15:43
… CA secrets - Add buildCleanCAConfig() helper that strips sub-objects for inactive CA types before save/test, preventing credential leakage of unused providers - Apply buildCleanCAConfig in handleSave and handleTestCA - Add useDialog confirmation dialogs for Approve and Reject enrollment actions - Pass explicit undefined for rejection reason with TODO for reason modal
|
beLIEai seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
📝 WalkthroughWalkthroughIntroduces a comprehensive Device Security feature with new management pages for device authentication settings, certificate enrollment, device certificates, and trusted CAs. Updates Docker configuration to use official nginx:alpine image, extends user roles with CertApprover, and creates supporting infrastructure including provider context and type definitions. Changes
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly Related PRs
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
7 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (5)
docker/init_react_envs.sh (2)
57-57: Declare and assign separately to avoid masking return values.The combined
export VAR="$(cmd)"pattern masks the exit code of the command substitution. If thesedcommand fails, the non-zero exit code is lost andset -ewon't catch it.♻️ Proposed fix
-export NETBIRD_MGMT_API_ENDPOINT="$(echo "$NETBIRD_MGMT_API_ENDPOINT" | sed -E 's/(:80|:443)$//')" +NETBIRD_MGMT_API_ENDPOINT="$(echo "$NETBIRD_MGMT_API_ENDPOINT" | sed -E 's/(:80|:443)$//')" +export NETBIRD_MGMT_API_ENDPOINT🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/init_react_envs.sh` at line 57, The export line for NETBIRD_MGMT_API_ENDPOINT is using a command substitution inline which masks the sed exit code; instead perform the substitution into NETBIRD_MGMT_API_ENDPOINT first (run the sed on the existing value, check its exit status), handle any non-zero result (return/exit or error), and only then export NETBIRD_MGMT_API_ENDPOINT so failures are caught by set -e; locate the assignment referencing NETBIRD_MGMT_API_ENDPOINT and the sed -E 's/(:80|:443)$//' invocation to update per this pattern.
73-77: Quote command substitution to handle filenames with spaces.The unquoted
$(grep ...)in the for loop will break if any matching file paths contain whitespace due to word splitting.♻️ Proposed fix using `find -exec` or quoted iteration
-for f in $(grep -R -l AUTH_SUPPORTED_SCOPES /usr/share/nginx/html); do +grep -R -l AUTH_SUPPORTED_SCOPES /usr/share/nginx/html | while IFS= read -r f; do cp "$f" "$f".copy envsubst "$ENV_STR" < "$f".copy > "$f" rm "$f".copy done🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/init_react_envs.sh` around lines 73 - 77, The for-loop `for f in $(grep -R -l AUTH_SUPPORTED_SCOPES /usr/share/nginx/html); do` will break on filenames with spaces; change to a null-safe iteration (e.g., use `grep -rlZ` piped into a `while IFS= read -r -d '' f; do ... done` or `find ... -print0 | while IFS= read -r -d '' f; do ... done`) so `f` is read intact, and keep the existing operations (`cp "$f" "$f".copy`, `envsubst "$ENV_STR" < "$f".copy > "$f"`, `rm "$f".copy`) inside that loop to preserve behavior.docker/Dockerfile (1)
1-1: **Consider using the unprivileged nginx image for improved security posture.**Thenginxinc/nginx-unprivilegedimage runs NGINX as a non-root, unprivileged user. Notable differences include the default listen port being 8080 instead of 80 and the PID file moved from/var/run/nginx.pidto/tmp/nginx.pid.Switching to this image would require updating the
EXPOSEdirective and adjustingnginx.confto use a writable PID path, but provides better container security by default.♻️ Proposed fix to use unprivileged image
-FROM nginx:alpine +FROM nginxinc/nginx-unprivileged:alpine -EXPOSE 80 +EXPOSE 8080You would also need to add to
docker/nginx.confat the top level:pid /tmp/nginx.pid;And update
docker/default.confto listen on 8080:listen 8080;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/Dockerfile` at line 1, Replace the base image FROM nginx:alpine with the unprivileged nginxinc/nginx-unprivileged image, update EXPOSE to 8080, and modify nginx configuration: add pid /tmp/nginx.pid; at top of docker/nginx.conf and change any listen directives (e.g., in docker/default.conf) from 80 to 8080 so NGINX runs as the unprivileged user and uses the writable PID path.src/modules/device-security/PasswordField.tsx (1)
34-35: Use a contextual toggle label for better screen-reader clarity.Line 34’s generic “Show/Hide” is ambiguous when several password fields exist on one form.
♿ Suggested tweak
- aria-label={show ? "Hide" : "Show"} + aria-label={show ? `Hide ${label}` : `Show ${label}`}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/device-security/PasswordField.tsx` around lines 34 - 35, The aria-label "Show"/"Hide" in PasswordField.tsx is ambiguous; update the toggle to use a contextual label (e.g., "Show password" / "Hide password" or "Show {fieldLabel} password" / "Hide {fieldLabel} password") by composing the string from the existing show state and a field identifier passed into the PasswordField component (use props like label, name, or id). Locate the aria-label on the toggle where the show variable is used and change it to include the field context so screen readers can distinguish multiple password fields.src/modules/device-security/InventoryPage.tsx (1)
353-361: Normalize theuseHasChangesbaseline with the same fallback shape.This effect hydrates
localConfigwithDEFAULT_*, butupdateRefuses rawinventoryConfig. If the API omitsstatic,intune, orjamf, the page starts dirty even before the user edits anything because the baseline and local state have different shapes.Suggested fix
useEffect(() => { if (!inventoryConfig) return; - setLocalConfig({ + const normalizedConfig = { static: inventoryConfig.static ?? DEFAULT_STATIC, intune: inventoryConfig.intune ?? DEFAULT_INTUNE, jamf: inventoryConfig.jamf ?? DEFAULT_JAMF, - }); - updateRef([inventoryConfig]); + }; + setLocalConfig(normalizedConfig); + updateRef([normalizedConfig]); }, [inventoryConfig, updateRef]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/device-security/InventoryPage.tsx` around lines 353 - 361, The baseline passed to the change-tracker is using the raw inventoryConfig while localConfig is hydrated with DEFAULT_STATIC/DEFAULT_INTUNE/DEFAULT_JAMF, causing initial dirty state; normalize the baseline by calling updateRef with the same fallback shape you set into localConfig (i.e., construct an object using inventoryConfig.static ?? DEFAULT_STATIC, inventoryConfig.intune ?? DEFAULT_INTUNE, inventoryConfig.jamf ?? DEFAULT_JAMF) so useHasChanges compares like-for-like with localConfig in the useEffect that currently references inventoryConfig, setLocalConfig, and updateRef.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/`(dashboard)/device-security/devices/page.tsx:
- Around line 20-26: The breadcrumb items are duplicating the same href; update
the parent breadcrumb so it points to the parent section rather than the current
page. In the Breadcrumbs.Item with label "Device Security" (using
ShieldCheckIcon) change href from "/device-security/devices" to the parent route
"/device-security", leaving the "Devices" Breadcrumbs.Item href as
"/device-security/devices" so users can navigate up.
In `@src/components/Notification.tsx`:
- Around line 218-223: The notify.error helper rejects with a plain Error which
lacks the expected code property used by the catch handler; change notify.error
(the notify.error function) to reject with an ErrorResponse-shaped object (e.g.
{ message, code: 500 } or another appropriate HTTP/error code) instead of
Promise.reject(new Error(message)) so the downstream .catch() can read err.code
and avoid the 418 fallback.
In `@src/layouts/Navigation.tsx`:
- Around line 211-247: The Device Security sidebar and child SidebarItem entries
in Navigation.tsx are gated only by !isRestricted which either wrongly hides
enrollments from cert_approver users or grants overly broad access; update the
rendering to perform explicit role/permission checks using useUserProfile() (or
add/consume a device_security permission on the Permission interface) so
Enrollments and actions like approveEnrollment/rejectEnrollment are shown only
to authorized roles (e.g., cert_approver) while other Device Security items
remain restricted; alternatively, if you rely on backend-only checks, add a
short frontend check stub that queries the same permission flag (e.g.,
userProfile.hasPermission('device_security') or userProfile.role ===
'cert_approver') before rendering the Enrollments SidebarItem and ensure the
Permission interface includes device_security for granular control.
In `@src/modules/device-security/AddTrustedCAModal.tsx`:
- Around line 70-82: handleSubmit allows multiple submissions while addTrustedCA
is in flight; add a guard and an "isSubmitting" state/ref (e.g., isSubmitting
via useState or useRef) and return early if true, then set isSubmitting = true
before calling notify(addTrustedCA(...)) and reset it to false in the
promise.finally (or onSuccess path) so duplicate addTrustedCA requests can't be
sent; also disable the submit button tied to the modal using this same
isSubmitting flag so the UI prevents repeated clicks (update handleSubmit,
addTrustedCA invocation, notify.promise handling, and the submit button props).
In `@src/modules/device-security/CAConfigSection.tsx`:
- Around line 104-105: The timeout number inputs currently only use min/max
attributes but allow out-of-range values to be stored because the onChange
handler passes parseInt(...) || 30 directly; update the onChange logic (for the
vault timeout bound to config.vault.timeout_seconds and any similar inputs at
the other instances noted) to clamp the parsed value into the allowed range
before calling updateVault (e.g., ensure value = Math.max(MIN, Math.min(MAX,
parsed)) and then call updateVault({ timeout_seconds: value })). Modify the
handler tied to the Input with id "vault-timeout" and the analogous handlers at
the other locations so state always contains a validated timeout within [1,300]
(or the appropriate min/max for those fields).
In `@src/modules/device-security/CATestPanel.tsx`:
- Around line 16-17: The expansion state is initialized once and can become
stale across reruns; in CATestPanel replace the static initialization of
expanded via useState(step.status === "error") with keeping controlled updates:
keep expanded and setExpanded but add a useEffect that watches step.status and
calls setExpanded(true) when step.status === "error" (and optionally
setExpanded(false) for non-error), and apply the same pattern for the other
expansion-related state used around lines 81-83 so the component auto-expands
newly failed steps on status changes; reference the expanded and setExpanded
state variables and the step.status property when implementing the effect.
In `@src/modules/device-security/DeviceSecuritySettings.tsx`:
- Around line 153-188: handleSave currently updates settings (including ca_type)
before attempting updateCAConfig, which can leave ca_type set when the CA config
fails; change the order so that when caType !== "builtin" you first call
updateCAConfig(buildCleanCAConfig(localCAConfig, caType)), await it (with
notify/promise handling identical to current flow), and only after that succeeds
call updateSettings({ mode, enrollment_mode: enrollmentMode, ca_type: caType,
cert_validity_days: certValidityDays, require_inventory_check:
requireInventoryCheck }); keep the same notify/error handling and finally call
updateRef([mode, enrollmentMode, caType, certValidityDays,
requireInventoryCheck, localCAConfig]) so state is only persisted when both CA
config and settings succeed (use the same function names: handleSave,
updateCAConfig, updateSettings, buildCleanCAConfig, updateRef, caType,
localCAConfig).
In `@src/modules/device-security/EnrollmentsTable.tsx`:
- Around line 277-284: The current check in EnrollmentsTable treats enrollments
=== undefined the same as an empty list, causing fetch failures to show the "No
enrollment requests" NoResults UI; change the rendering logic to distinguish
three states: loading (enrollmentsLoading), fetch error (enrollments ===
undefined after loading) and empty results (Array.isArray(enrollments) &&
enrollments.length === 0). Replace the single branch that returns NoResults with
separate branches: after enrollmentsLoading is false, if enrollments ===
undefined render an error/fallback UI (or an ErrorMessage component) to indicate
fetch failure, otherwise if enrollments is an empty array render the existing
NoResults; reference the enrollmentsLoading and enrollments variables and the
NoResults usage in EnrollmentsTable to locate where to split the conditions.
- Around line 107-135: The column currently uses accessorKey: "peer_id" so the
global filter searches peer IDs while the cell renders peer.name; update the
column to use an accessorFn that resolves the display name from peerMap (e.g.,
accessorFn: row => peerMap.get(row.peer_id)?.name ?? row.peer_id) so
row.getValue(columnId) returns the peer name for filtering, or alternatively add
a custom filterFn for this column that looks up
peerMap.get(row.original.peer_id)?.name when evaluating matches; modify the
column definition (which currently has accessorKey: "peer_id" and cell: ({ row
}) => { ... }) to use accessorFn or filterFn accordingly so searches by peer
name work.
In `@src/modules/device-security/InventoryPage.tsx`:
- Around line 381-393: The handleSave validation is incomplete: when
localConfig.intune.enabled you must also require
localConfig.intune.client_secret (in addition to tenant_id and client_id) and
when localConfig.jamf.enabled require localConfig.jamf.client_id and
localConfig.jamf.client_secret (in addition to jamf_url); update the handleSave
function to check these fields (trim values if needed) and call notify.error
with clear messages (e.g., "Client Secret is required for Intune" / "Client ID
and Client Secret are required for Jamf") and return early to prevent submitting
an incomplete config.
In `@src/modules/device-security/RevokeDeviceModal.tsx`:
- Around line 33-41: Add an in-flight guard to prevent repeated revoke requests
by introducing a boolean state (e.g. isRevoking) and using it in the
handleRevoke flow: if isRevoking return early, set isRevoking = true before
starting the revoke, call revokeDevice(device.id) inside the notify promise, and
in the promise finally set isRevoking = false and close the modal
(setOpen(false)) on success; also disable the revoke/confirm button while
isRevoking is true (e.g. disabled={isRevoking}). Apply the same guard pattern to
the other revoke handler referenced around the 84-87 region.
In `@src/modules/device-security/TrustedCAsTable.tsx`:
- Around line 181-195: The DataTable call in TrustedCAsTable.tsx currently
passes trustedCAs ?? [] which hides fetch failures (trustedCAs undefined when
loading finished with error); change the render logic to detect when trustedCAs
is undefined while trustedCAsLoading is false and render an explicit
error/failure UI (or a distinct error placeholder) instead of passing [] into
DataTable; update the DataTable invocation to only pass data={trustedCAs} when
trustedCAs is defined and use the existing emptyState/addButton for the true
empty-array case, referencing the trustedCAs, trustedCAsLoading, emptyState,
addButton and DataTable usage to locate the change.
---
Nitpick comments:
In `@docker/Dockerfile`:
- Line 1: Replace the base image FROM nginx:alpine with the unprivileged
nginxinc/nginx-unprivileged image, update EXPOSE to 8080, and modify nginx
configuration: add pid /tmp/nginx.pid; at top of docker/nginx.conf and change
any listen directives (e.g., in docker/default.conf) from 80 to 8080 so NGINX
runs as the unprivileged user and uses the writable PID path.
In `@docker/init_react_envs.sh`:
- Line 57: The export line for NETBIRD_MGMT_API_ENDPOINT is using a command
substitution inline which masks the sed exit code; instead perform the
substitution into NETBIRD_MGMT_API_ENDPOINT first (run the sed on the existing
value, check its exit status), handle any non-zero result (return/exit or
error), and only then export NETBIRD_MGMT_API_ENDPOINT so failures are caught by
set -e; locate the assignment referencing NETBIRD_MGMT_API_ENDPOINT and the sed
-E 's/(:80|:443)$//' invocation to update per this pattern.
- Around line 73-77: The for-loop `for f in $(grep -R -l AUTH_SUPPORTED_SCOPES
/usr/share/nginx/html); do` will break on filenames with spaces; change to a
null-safe iteration (e.g., use `grep -rlZ` piped into a `while IFS= read -r -d
'' f; do ... done` or `find ... -print0 | while IFS= read -r -d '' f; do ...
done`) so `f` is read intact, and keep the existing operations (`cp "$f"
"$f".copy`, `envsubst "$ENV_STR" < "$f".copy > "$f"`, `rm "$f".copy`) inside
that loop to preserve behavior.
In `@src/modules/device-security/InventoryPage.tsx`:
- Around line 353-361: The baseline passed to the change-tracker is using the
raw inventoryConfig while localConfig is hydrated with
DEFAULT_STATIC/DEFAULT_INTUNE/DEFAULT_JAMF, causing initial dirty state;
normalize the baseline by calling updateRef with the same fallback shape you set
into localConfig (i.e., construct an object using inventoryConfig.static ??
DEFAULT_STATIC, inventoryConfig.intune ?? DEFAULT_INTUNE, inventoryConfig.jamf
?? DEFAULT_JAMF) so useHasChanges compares like-for-like with localConfig in the
useEffect that currently references inventoryConfig, setLocalConfig, and
updateRef.
In `@src/modules/device-security/PasswordField.tsx`:
- Around line 34-35: The aria-label "Show"/"Hide" in PasswordField.tsx is
ambiguous; update the toggle to use a contextual label (e.g., "Show password" /
"Hide password" or "Show {fieldLabel} password" / "Hide {fieldLabel} password")
by composing the string from the existing show state and a field identifier
passed into the PasswordField component (use props like label, name, or id).
Locate the aria-label on the toggle where the show variable is used and change
it to include the field context so screen readers can distinguish multiple
password fields.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 62190539-b3f6-4962-913f-01b8eecf2eb5
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (33)
cypress/support/commands.tsdocker/Dockerfiledocker/init_react_envs.shdocker/nginx.confsrc/app/(dashboard)/device-security/devices/page.tsxsrc/app/(dashboard)/device-security/enrollments/page.tsxsrc/app/(dashboard)/device-security/inventory/page.tsxsrc/app/(dashboard)/device-security/layout.tsxsrc/app/(dashboard)/device-security/page.tsxsrc/app/(dashboard)/device-security/settings/page.tsxsrc/app/(dashboard)/device-security/trusted-cas/page.tsxsrc/assets/icons/DeviceSecurityIcon.tsxsrc/components/HelpText.tsxsrc/components/Notification.tsxsrc/contexts/DeviceSecurityProvider.tsxsrc/contexts/UsersProvider.tsxsrc/interfaces/DeviceSecurity.tssrc/interfaces/User.tssrc/layouts/Navigation.tsxsrc/modules/device-security/AddTrustedCAModal.tsxsrc/modules/device-security/CAConfigSection.tsxsrc/modules/device-security/CATestPanel.tsxsrc/modules/device-security/DeviceSecuritySettings.tsxsrc/modules/device-security/DevicesTable.tsxsrc/modules/device-security/EnrollmentsTable.tsxsrc/modules/device-security/InventoryPage.tsxsrc/modules/device-security/PasswordField.tsxsrc/modules/device-security/RevokeDeviceModal.tsxsrc/modules/device-security/TrustedCAsTable.tsxsrc/modules/users/UserInvitesTable.tsxsrc/modules/users/UserRoleSelector.tsxsrc/modules/users/table-cells/UserRoleCell.tsxsrc/utils/api.tsx
Comment on lines
+20
to
+26
| href={"/device-security/devices"} | ||
| label={"Device Security"} | ||
| icon={<ShieldCheckIcon size={13} />} | ||
| /> | ||
| <Breadcrumbs.Item | ||
| href={"/device-security/devices"} | ||
| label={"Devices"} |
There was a problem hiding this comment.
Breadcrumb hierarchy is duplicated; parent and current item point to the same URL.
Line 20 and Line 25 both target /device-security/devices, so users can’t navigate “up” from the first crumb.
🧭 Suggested fix
<Breadcrumbs.Item
- href={"/device-security/devices"}
+ href={"/device-security/settings"}
label={"Device Security"}
icon={<ShieldCheckIcon size={13} />}
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| href={"/device-security/devices"} | |
| label={"Device Security"} | |
| icon={<ShieldCheckIcon size={13} />} | |
| /> | |
| <Breadcrumbs.Item | |
| href={"/device-security/devices"} | |
| label={"Devices"} | |
| href={"/device-security/settings"} | |
| label={"Device Security"} | |
| icon={<ShieldCheckIcon size={13} />} | |
| /> | |
| <Breadcrumbs.Item | |
| href={"/device-security/devices"} | |
| label={"Devices"} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/`(dashboard)/device-security/devices/page.tsx around lines 20 - 26,
The breadcrumb items are duplicating the same href; update the parent breadcrumb
so it points to the parent section rather than the current page. In the
Breadcrumbs.Item with label "Device Security" (using ShieldCheckIcon) change
href from "/device-security/devices" to the parent route "/device-security",
leaving the "Devices" Breadcrumbs.Item href as "/device-security/devices" so
users can navigate up.
Comment on lines
+218
to
+223
| notify.error = function notifyError(message: string) { | ||
| return notify({ | ||
| title: "Error", | ||
| description: message, | ||
| promise: Promise.reject(new Error(message)), | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does creating Promise.reject(...) before attaching a catch handler later in React useEffect typically trigger unhandledrejection in browsers?
💡 Result:
No, creating Promise.reject(...) before attaching a .catch handler later in React useEffect does not typically trigger an unhandledrejection in browsers.
Citations:
- 1: https://stackoverflow.com/questions/64336083/unexpected-unhandledrejection-event-for-promise-which-rejection-does-get-handled
- 2: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/catch
- 3: https://developer.mozilla.org/en-US/docs/Web/Events/unhandledrejection
- 4: https://stackoverflow.com/questions/77995835/synchronously-rejecting-a-promise
🏁 Script executed:
cat -n src/components/Notification.tsx | head -250 | tail -50Repository: netbirdio/dashboard
Length of output: 1831
🏁 Script executed:
cat -n src/components/Notification.tsx | head -175Repository: netbirdio/dashboard
Length of output: 6289
Pass ErrorResponse object instead of Error to avoid Code 418 fallback.
Line 222 rejects with new Error(message), but the .catch() handler (line 124) expects an ErrorResponse with a code property. Since Error objects lack a code property, the fallback err.code || 418 evaluates to 418, resulting in error messages like "Code 418: ...".
Construct an ErrorResponse object instead:
💡 Proposed fix
notify.error = function notifyError(message: string) {
return notify({
title: "Error",
description: message,
- promise: Promise.reject(new Error(message)),
+ promise: Promise.reject({ code: 500, message }),
});
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/Notification.tsx` around lines 218 - 223, The notify.error
helper rejects with a plain Error which lacks the expected code property used by
the catch handler; change notify.error (the notify.error function) to reject
with an ErrorResponse-shaped object (e.g. { message, code: 500 } or another
appropriate HTTP/error code) instead of Promise.reject(new Error(message)) so
the downstream .catch() can read err.code and avoid the 418 fallback.
Comment on lines
+211
to
+247
| visible={!isRestricted} | ||
| > | ||
| <SidebarItem | ||
| label="Settings" | ||
| href="/device-security/settings" | ||
| isChild | ||
| exactPathMatch={true} | ||
| visible={!isRestricted} | ||
| /> | ||
| <SidebarItem | ||
| label="Inventory" | ||
| href="/device-security/inventory" | ||
| isChild | ||
| exactPathMatch={true} | ||
| visible={!isRestricted} | ||
| /> | ||
| <SidebarItem | ||
| label="Enrollments" | ||
| href="/device-security/enrollments" | ||
| isChild | ||
| exactPathMatch={true} | ||
| visible={!isRestricted} | ||
| /> | ||
| <SidebarItem | ||
| label="Devices" | ||
| href="/device-security/devices" | ||
| isChild | ||
| exactPathMatch={true} | ||
| visible={!isRestricted} | ||
| /> | ||
| <SidebarItem | ||
| label="Trusted CAs" | ||
| href="/device-security/trusted-cas" | ||
| isChild | ||
| exactPathMatch={true} | ||
| visible={!isRestricted} | ||
| /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether cert_approver is marked restricted and how sidebar visibility is intended.
set -euo pipefail
echo "== Locate permission/restriction derivation =="
fd -i "PermissionsProvider.tsx" .
rg -n -C3 --type=ts --type=tsx '\bisRestricted\b|\bcert_approver\b|usePermissions\('
echo
echo "== Locate RBAC role/permission mappings =="
rg -n -C3 --type=ts --type=tsx 'cert_approver|enrollment|device-security|permissions|roles'
echo
echo "== Confirm this navigation gating =="
rg -n -C3 --type=tsx 'Device Security|/device-security/enrollments|visible=\{!isRestricted\}' src/layouts/Navigation.tsxRepository: netbirdio/dashboard
Length of output: 193
🏁 Script executed:
cat -n ./src/contexts/PermissionsProvider.tsx | head -100Repository: netbirdio/dashboard
Length of output: 1190
🏁 Script executed:
rg -n 'cert_approver|isRestricted' ./src/contexts/PermissionsProvider.tsxRepository: netbirdio/dashboard
Length of output: 142
🏁 Script executed:
rg -n 'cert_approver' . -t ts -t tsx --max-count 20Repository: netbirdio/dashboard
Length of output: 90
🏁 Script executed:
grep -n 'Device Security' src/layouts/Navigation.tsx -B 5 -A 40Repository: netbirdio/dashboard
Length of output: 1955
🏁 Script executed:
find . -type f \( -name "*.ts" -o -name "*.tsx" \) -exec grep -l "cert_approver" {} \;Repository: netbirdio/dashboard
Length of output: 87
🏁 Script executed:
cat -n ./src/interfaces/Permission.ts | head -150Repository: netbirdio/dashboard
Length of output: 1195
🏁 Script executed:
cat -n ./src/interfaces/User.ts | head -150Repository: netbirdio/dashboard
Length of output: 1953
🏁 Script executed:
rg 'cert_approver' --no-filename -B 3 -A 3Repository: netbirdio/dashboard
Length of output: 189
🏁 Script executed:
rg 'is_restricted|enrollment' . --no-filename -B 2 -A 2 | head -100Repository: netbirdio/dashboard
Length of output: 2974
🏁 Script executed:
find . -type f \( -name "*.ts" -o -name "*.tsx" \) -exec grep -l 'device_security\|enrollments' {} \;Repository: netbirdio/dashboard
Length of output: 243
🏁 Script executed:
rg 'CertApprover|cert_approver' . --no-filename -B 5 -A 5 | head -150Repository: netbirdio/dashboard
Length of output: 1536
🏁 Script executed:
cat -n ./src/app/\(dashboard\)/device-security/enrollments/page.tsx | head -50Repository: netbirdio/dashboard
Length of output: 1675
🏁 Script executed:
cat -n ./src/contexts/DeviceSecurityProvider.tsxRepository: netbirdio/dashboard
Length of output: 8464
🏁 Script executed:
rg 'cert_approver.*enrollment|enrollment.*cert_approver|enrollments.*cert|approver.*permission' . --no-filename -B 3 -A 3Repository: netbirdio/dashboard
Length of output: 45
🏁 Script executed:
grep -r 'approval\|approver' ./src --include='*.tsx' --include='*.ts' | grep -i 'permission\|access\|restricted' | head -20Repository: netbirdio/dashboard
Length of output: 1123
🏁 Script executed:
rg 'enrollments|approveEnrollment|rejectEnrollment' ./src/modules/device-security/EnrollmentsTable.tsx -B 3 -A 3 | head -100Repository: netbirdio/dashboard
Length of output: 1503
🏁 Script executed:
grep -n 'usePermissions\|useUserProfile\|isCertApprover' ./src/modules/device-security/EnrollmentsTable.tsxRepository: netbirdio/dashboard
Length of output: 45
🏁 Script executed:
find . -name '*.md' -o -name 'README*' -o -name 'REQUIREMENTS*' | xargs grep -l 'cert_approver\|CertApprover' 2>/dev/nullRepository: netbirdio/dashboard
Length of output: 47
Add explicit role validation for enrollment operations; coarse-grained !isRestricted gate may incorrectly hide Device Security from cert_approver users.
Device Security menu (line 211) and all children, including Enrollments (line 228), are gated only by the global !isRestricted flag with no role-specific checks. The enrollments page and approveEnrollment/rejectEnrollment operations lack frontend validation that the user is authorized for these actions.
If cert_approver is classified as restricted, they cannot access enrollments despite the role name implying they should approve certificates. If not restricted, they can access all Device Security features (Settings, Inventory, Devices, Trusted CAs) without granular control.
Consider either:
- Explicitly validate
useUserProfile()role before rendering enrollments page - Add a
device_securitypermission module to the Permission interface for granular control - Or confirm backend API enforces
cert_approverpermission checks and document the contract
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/layouts/Navigation.tsx` around lines 211 - 247, The Device Security
sidebar and child SidebarItem entries in Navigation.tsx are gated only by
!isRestricted which either wrongly hides enrollments from cert_approver users or
grants overly broad access; update the rendering to perform explicit
role/permission checks using useUserProfile() (or add/consume a device_security
permission on the Permission interface) so Enrollments and actions like
approveEnrollment/rejectEnrollment are shown only to authorized roles (e.g.,
cert_approver) while other Device Security items remain restricted;
alternatively, if you rely on backend-only checks, add a short frontend check
stub that queries the same permission flag (e.g.,
userProfile.hasPermission('device_security') or userProfile.role ===
'cert_approver') before rendering the Enrollments SidebarItem and ensure the
Permission interface includes device_security for granular control.
Comment on lines
+70
to
+82
| const handleSubmit = () => { | ||
| const trimmedName = name.trim(); | ||
| const trimmedPem = pem.trim(); | ||
|
|
||
| notify({ | ||
| title: "Adding trusted CA", | ||
| description: `"${trimmedName}" was added successfully`, | ||
| promise: addTrustedCA(trimmedName, trimmedPem).then(() => { | ||
| onSuccess(); | ||
| }), | ||
| loadingMessage: "Adding trusted CA...", | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Prevent duplicate CA creation requests during submit.
The current handler allows repeated clicks while the request is in flight, which can send duplicate add requests.
🛠️ Suggested fix
function AddTrustedCAModalContent({ onSuccess }: Readonly<ContentProps>) {
const { addTrustedCA } = useDeviceSecurity();
const [name, setName] = useState("");
const [pem, setPem] = useState("");
+ const [isSubmitting, setIsSubmitting] = useState(false);
@@
- const handleSubmit = () => {
+ const handleSubmit = () => {
+ if (isSubmitting || isDisabled) return;
const trimmedName = name.trim();
const trimmedPem = pem.trim();
+ setIsSubmitting(true);
notify({
title: "Adding trusted CA",
description: `"${trimmedName}" was added successfully`,
- promise: addTrustedCA(trimmedName, trimmedPem).then(() => {
- onSuccess();
- }),
+ promise: addTrustedCA(trimmedName, trimmedPem)
+ .then(() => {
+ onSuccess();
+ })
+ .finally(() => {
+ setIsSubmitting(false);
+ }),
loadingMessage: "Adding trusted CA...",
});
};
@@
<Button
variant="primary"
onClick={handleSubmit}
- disabled={isDisabled}
+ disabled={isDisabled || isSubmitting}
>Also applies to: 128-132
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/modules/device-security/AddTrustedCAModal.tsx` around lines 70 - 82,
handleSubmit allows multiple submissions while addTrustedCA is in flight; add a
guard and an "isSubmitting" state/ref (e.g., isSubmitting via useState or
useRef) and return early if true, then set isSubmitting = true before calling
notify(addTrustedCA(...)) and reset it to false in the promise.finally (or
onSuccess path) so duplicate addTrustedCA requests can't be sent; also disable
the submit button tied to the modal using this same isSubmitting flag so the UI
prevents repeated clicks (update handleSubmit, addTrustedCA invocation,
notify.promise handling, and the submit button props).
Comment on lines
+104
to
+105
| <Label htmlFor="vault-timeout">Timeout (seconds)</Label> | ||
| <Input id="vault-timeout" type="number" min={1} max={300} value={config.vault?.timeout_seconds ?? 30} onChange={(e) => updateVault({ timeout_seconds: parseInt(e.target.value, 10) || 30 })} /> |
There was a problem hiding this comment.
Clamp timeout values in state, not just in the input attributes.
min/max on a number input does not stop 0, -1, or 999 from being typed, and the current handler stores those values unchanged except for falsy 0. Save/test can therefore send out-of-range timeouts.
Suggested fix
+function clampTimeout(raw: string): number {
+ const parsed = Number.parseInt(raw, 10);
+ if (Number.isNaN(parsed)) return 30;
+ return Math.min(300, Math.max(1, parsed));
+}
+
...
-<Input id="vault-timeout" type="number" min={1} max={300} value={config.vault?.timeout_seconds ?? 30} onChange={(e) => updateVault({ timeout_seconds: parseInt(e.target.value, 10) || 30 })} />
+<Input id="vault-timeout" type="number" min={1} max={300} value={config.vault?.timeout_seconds ?? 30} onChange={(e) => updateVault({ timeout_seconds: clampTimeout(e.target.value) })} />
...
-<Input id="step-timeout" type="number" min={1} max={300} value={config.smallstep?.timeout_seconds ?? 30} onChange={(e) => updateSmallstep({ timeout_seconds: parseInt(e.target.value, 10) || 30 })} />
+<Input id="step-timeout" type="number" min={1} max={300} value={config.smallstep?.timeout_seconds ?? 30} onChange={(e) => updateSmallstep({ timeout_seconds: clampTimeout(e.target.value) })} />
...
-<Input id="scep-timeout" type="number" min={1} max={300} value={config.scep?.timeout_seconds ?? 30} onChange={(e) => updateSCEP({ timeout_seconds: parseInt(e.target.value, 10) || 30 })} />
+<Input id="scep-timeout" type="number" min={1} max={300} value={config.scep?.timeout_seconds ?? 30} onChange={(e) => updateSCEP({ timeout_seconds: clampTimeout(e.target.value) })} />Also applies to: 129-130, 149-150
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/modules/device-security/CAConfigSection.tsx` around lines 104 - 105, The
timeout number inputs currently only use min/max attributes but allow
out-of-range values to be stored because the onChange handler passes
parseInt(...) || 30 directly; update the onChange logic (for the vault timeout
bound to config.vault.timeout_seconds and any similar inputs at the other
instances noted) to clamp the parsed value into the allowed range before calling
updateVault (e.g., ensure value = Math.max(MIN, Math.min(MAX, parsed)) and then
call updateVault({ timeout_seconds: value })). Modify the handler tied to the
Input with id "vault-timeout" and the analogous handlers at the other locations
so state always contains a validated timeout within [1,300] (or the appropriate
min/max for those fields).
Comment on lines
+107
to
+135
| accessorKey: "peer_id", | ||
| header: ({ column }) => ( | ||
| <DataTableHeader column={column}>Peer</DataTableHeader> | ||
| ), | ||
| sortingFn: "text", | ||
| cell: ({ row }) => { | ||
| const peer = peerMap.get(row.original.peer_id); | ||
| if (peer) { | ||
| return ( | ||
| <Link | ||
| href={`/peer?id=${peer.id}`} | ||
| className="group flex flex-col" | ||
| onClick={(e) => e.stopPropagation()} | ||
| > | ||
| <span className="font-medium text-nb-gray-100 group-hover:text-netbird transition-colors"> | ||
| {peer.name} | ||
| </span> | ||
| <span className="text-xs text-nb-gray-500" title={row.original.peer_id}> | ||
| {truncate(row.original.peer_id)} | ||
| </span> | ||
| </Link> | ||
| ); | ||
| } | ||
| return ( | ||
| <span className="text-nb-gray-400" title={row.original.peer_id}> | ||
| {truncate(row.original.peer_id)} | ||
| </span> | ||
| ); | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n--- DataTable implementation ---\n'
fd -i 'DataTable.tsx' src | while read -r file; do
echo "FILE: $file"
sed -n '1,260p' "$file"
done
printf '\n--- EnrollmentsTable peer column ---\n'
sed -n '107,145p' src/modules/device-security/EnrollmentsTable.tsxRepository: netbirdio/dashboard
Length of output: 9082
Peer-name search will not work with the current accessor.
This column exposes peer_id as the accessor and only renders peer.name in the cell. The global filter uses row.getValue(columnId) to extract filter values, so it will search against peer IDs, not peer names. Users searching for a peer by name will find no results. Switch to an accessorFn that returns the peer name, or implement a custom filter function for this column.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/modules/device-security/EnrollmentsTable.tsx` around lines 107 - 135, The
column currently uses accessorKey: "peer_id" so the global filter searches peer
IDs while the cell renders peer.name; update the column to use an accessorFn
that resolves the display name from peerMap (e.g., accessorFn: row =>
peerMap.get(row.peer_id)?.name ?? row.peer_id) so row.getValue(columnId) returns
the peer name for filtering, or alternatively add a custom filterFn for this
column that looks up peerMap.get(row.original.peer_id)?.name when evaluating
matches; modify the column definition (which currently has accessorKey:
"peer_id" and cell: ({ row }) => { ... }) to use accessorFn or filterFn
accordingly so searches by peer name work.
Comment on lines
+277
to
+284
| if (!enrollmentsLoading && (!enrollments || enrollments.length === 0)) { | ||
| return ( | ||
| <NoResults | ||
| title="No enrollment requests" | ||
| description="There are no device enrollment requests at this time." | ||
| icon={<ShieldAlertIcon size={20} />} | ||
| /> | ||
| ); |
There was a problem hiding this comment.
Don't treat an undefined enrollment list as "no requests".
After loading, undefined currently falls into the empty state. If the fetch fails, approvers will see “No enrollment requests” instead of an error and can miss pending devices.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/modules/device-security/EnrollmentsTable.tsx` around lines 277 - 284, The
current check in EnrollmentsTable treats enrollments === undefined the same as
an empty list, causing fetch failures to show the "No enrollment requests"
NoResults UI; change the rendering logic to distinguish three states: loading
(enrollmentsLoading), fetch error (enrollments === undefined after loading) and
empty results (Array.isArray(enrollments) && enrollments.length === 0). Replace
the single branch that returns NoResults with separate branches: after
enrollmentsLoading is false, if enrollments === undefined render an
error/fallback UI (or an ErrorMessage component) to indicate fetch failure,
otherwise if enrollments is an empty array render the existing NoResults;
reference the enrollmentsLoading and enrollments variables and the NoResults
usage in EnrollmentsTable to locate where to split the conditions.
Comment on lines
+381
to
+393
| const handleSave = useCallback(async () => { | ||
| if (localConfig.intune.enabled) { | ||
| if (!localConfig.intune.tenant_id || !localConfig.intune.client_id) { | ||
| notify.error("Tenant ID and Client ID are required for Intune"); | ||
| return; | ||
| } | ||
| } | ||
| if (localConfig.jamf.enabled) { | ||
| if (!localConfig.jamf.jamf_url) { | ||
| notify.error("Server URL is required for Jamf"); | ||
| return; | ||
| } | ||
| } |
There was a problem hiding this comment.
Validation still allows incomplete Intune/Jamf credentials through.
A newly enabled Intune config can be saved without a client secret, and Jamf can be saved without a client ID or secret. That pushes an unusable config to the API and fails late instead of blocking locally.
Suggested fix
const handleSave = useCallback(async () => {
if (localConfig.intune.enabled) {
- if (!localConfig.intune.tenant_id || !localConfig.intune.client_id) {
- notify.error("Tenant ID and Client ID are required for Intune");
+ if (
+ !localConfig.intune.tenant_id ||
+ !localConfig.intune.client_id ||
+ (!localConfig.intune.client_secret && !localConfig.intune.has_client_secret)
+ ) {
+ notify.error("Tenant ID, Client ID, and Client Secret are required for Intune");
return;
}
}
if (localConfig.jamf.enabled) {
- if (!localConfig.jamf.jamf_url) {
- notify.error("Server URL is required for Jamf");
+ if (
+ !localConfig.jamf.jamf_url ||
+ !localConfig.jamf.client_id ||
+ (!localConfig.jamf.client_secret && !localConfig.jamf.has_client_secret)
+ ) {
+ notify.error("Server URL, Client ID, and Client Secret are required for Jamf");
return;
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/modules/device-security/InventoryPage.tsx` around lines 381 - 393, The
handleSave validation is incomplete: when localConfig.intune.enabled you must
also require localConfig.intune.client_secret (in addition to tenant_id and
client_id) and when localConfig.jamf.enabled require localConfig.jamf.client_id
and localConfig.jamf.client_secret (in addition to jamf_url); update the
handleSave function to check these fields (trim values if needed) and call
notify.error with clear messages (e.g., "Client Secret is required for Intune" /
"Client ID and Client Secret are required for Jamf") and return early to prevent
submitting an incomplete config.
Comment on lines
+33
to
+41
| const handleRevoke = async () => { | ||
| notify({ | ||
| title: "Revoking device certificate", | ||
| description: "Device certificate was successfully revoked", | ||
| promise: revokeDevice(device.id).then(() => { | ||
| setOpen(false); | ||
| }), | ||
| loadingMessage: "Revoking device certificate...", | ||
| }); |
There was a problem hiding this comment.
Guard revoke action against repeated clicks.
Revoke can be triggered multiple times before the first request resolves. Add an in-flight guard and disable the action button while pending.
🛠️ Suggested fix
export default function RevokeDeviceModal({ device, children }: Readonly<Props>) {
const [open, setOpen] = useState(false);
+ const [isRevoking, setIsRevoking] = useState(false);
const { revokeDevice } = useDeviceSecurity();
- const handleRevoke = async () => {
+ const handleRevoke = () => {
+ if (isRevoking) return;
+ setIsRevoking(true);
notify({
title: "Revoking device certificate",
description: "Device certificate was successfully revoked",
- promise: revokeDevice(device.id).then(() => {
- setOpen(false);
- }),
+ promise: revokeDevice(device.id)
+ .then(() => {
+ setOpen(false);
+ })
+ .finally(() => {
+ setIsRevoking(false);
+ }),
loadingMessage: "Revoking device certificate...",
});
};
@@
- <Button variant="danger" onClick={handleRevoke}>
+ <Button variant="danger" onClick={handleRevoke} disabled={isRevoking}>
<ShieldOffIcon size={16} />
Revoke
</Button>Also applies to: 84-87
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/modules/device-security/RevokeDeviceModal.tsx` around lines 33 - 41, Add
an in-flight guard to prevent repeated revoke requests by introducing a boolean
state (e.g. isRevoking) and using it in the handleRevoke flow: if isRevoking
return early, set isRevoking = true before starting the revoke, call
revokeDevice(device.id) inside the notify promise, and in the promise finally
set isRevoking = false and close the modal (setOpen(false)) on success; also
disable the revoke/confirm button while isRevoking is true (e.g.
disabled={isRevoking}). Apply the same guard pattern to the other revoke handler
referenced around the 84-87 region.
Comment on lines
+181
to
+195
| return ( | ||
| <DataTable | ||
| text="Trusted CAs" | ||
| sorting={sorting} | ||
| setSorting={setSorting} | ||
| columns={columns} | ||
| data={trustedCAs ?? []} | ||
| isLoading={trustedCAsLoading} | ||
| searchPlaceholder="Search by name..." | ||
| headingTarget={headingTarget} | ||
| getStartedCard={emptyState} | ||
| rightSide={() => | ||
| trustedCAs && trustedCAs.length > 0 ? addButton : null | ||
| } | ||
| /> |
There was a problem hiding this comment.
Don't collapse fetch failures into the empty state.
When trustedCAsLoading is false and trustedCAs is still undefined, this renders the same UI as a real empty list. On this page that means a failed load of trusted roots looks like “No trusted CAs added,” which is misleading. Handle undefined separately before passing [] into DataTable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/modules/device-security/TrustedCAsTable.tsx` around lines 181 - 195, The
DataTable call in TrustedCAsTable.tsx currently passes trustedCAs ?? [] which
hides fetch failures (trustedCAs undefined when loading finished with error);
change the render logic to detect when trustedCAs is undefined while
trustedCAsLoading is false and render an explicit error/failure UI (or a
distinct error placeholder) instead of passing [] into DataTable; update the
DataTable invocation to only pass data={trustedCAs} when trustedCAs is defined
and use the existing emptyState/addButton for the true empty-array case,
referencing the trustedCAs, trustedCAsLoading, emptyState, addButton and
DataTable usage to locate the change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add a Device Security section to the dashboard covering the full certificate lifecycle introduced in netbirdio/netbird#5881.
Describe your changes
Device Security nav item (with Beta badge) links to five sub-pages:
Settings
require_inventory_checktoggle — rejects peers not in the serial-number allow-list even if their certificate is validEnrollments
adminand the newcert_approverRBAC roleDevice Certificates
Trusted CAs
Serial Number Inventory
Issue ticket number and link
Closes #4773
Related PRs
Stack
Checklist
Documentation
Select exactly one:
UI mirrors the API documented in
docs/DEVICE-SECURITY.mdin netbirdio/netbird#5881. No separate docs PR needed at this stage (Beta feature).Summary by CodeRabbit
New Features
Bug Fixes
Chores