diff --git a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceAgentDevicesList.test.tsx b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceAgentDevicesList.test.tsx index abe8e40774..704fe6ef39 100644 --- a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceAgentDevicesList.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceAgentDevicesList.test.tsx @@ -1,4 +1,4 @@ -import { render, screen, fireEvent } from '@testing-library/react'; +import { render, screen, fireEvent, within } from '@testing-library/react'; import { describe, expect, it, vi, beforeEach } from 'vitest'; import type { DeviceWithChecks } from '../types'; @@ -261,9 +261,34 @@ describe('DeviceAgentDevicesList — integration-imported devices', () => { expect(screen.getByText('Imported Mac')).toBeInTheDocument(); fireEvent.change(screen.getByLabelText('Filter by source'), { - target: { value: 'Kandji' }, + target: { value: 'integration:kandji' }, }); expect(screen.queryByText('Agent Mac')).not.toBeInTheDocument(); expect(screen.getByText('Imported Mac')).toBeInTheDocument(); }); + + it('keeps distinct providers separate in the filter even with the same display name', () => { + render( + , + ); + const select = screen.getByLabelText('Filter by source'); + // "All sources" + two distinct providers (not merged into one "MDM"). + expect(within(select).getAllByRole('option')).toHaveLength(3); + fireEvent.change(select, { target: { value: 'integration:intune' } }); + expect(screen.queryByText('Device X')).not.toBeInTheDocument(); + expect(screen.getByText('Device Y')).toBeInTheDocument(); + }); }); diff --git a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceAgentDevicesList.tsx b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceAgentDevicesList.tsx index 82a16b88cc..4b47758dac 100644 --- a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceAgentDevicesList.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceAgentDevicesList.tsx @@ -30,7 +30,7 @@ import { downloadDevicesCsv, } from '../lib/devices-csv'; import { DeviceTableRow } from './DeviceListCells'; -import { sourceLabel } from '../lib/device-source'; +import { sourceKey, sourceLabel } from '../lib/device-source'; import { DeviceDetails } from './DeviceDetails'; import { RemoveDeviceAlert } from '../../all/components/RemoveDeviceAlert'; @@ -55,17 +55,20 @@ export const DeviceAgentDevicesList = ({ const [isRemoveDeviceAlertOpen, setIsRemoveDeviceAlertOpen] = useState(false); const [isRemovingDevice, setIsRemovingDevice] = useState(false); - // Distinct source labels present, so the filter only offers sources that exist - // (e.g. "Comp Agent", "Kandji"). + // Distinct sources present, keyed by a stable id (so two providers that share + // a display name don't collapse into one option) with a label for display. const sourceOptions = useMemo(() => { - const labels = new Set(devices.map((d) => sourceLabel(d))); - return Array.from(labels).sort(); + const byKey = new Map(); + for (const d of devices) byKey.set(sourceKey(d), sourceLabel(d)); + return Array.from(byKey, ([key, label]) => ({ key, label })).sort((a, b) => + a.label.localeCompare(b.label), + ); }, [devices]); const filteredDevices = useMemo(() => { const query = searchQuery.toLowerCase(); return devices.filter((device) => { - if (sourceFilter !== 'all' && sourceLabel(device) !== sourceFilter) { + if (sourceFilter !== 'all' && sourceKey(device) !== sourceFilter) { return false; } if (!query) return true; @@ -154,8 +157,8 @@ export const DeviceAgentDevicesList = ({ aria-label="Filter by source" > - {sourceOptions.map((label) => ( - ))} diff --git a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceDetails.tsx b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceDetails.tsx index eca2ac60c3..536229aaa2 100644 --- a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceDetails.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceDetails.tsx @@ -18,63 +18,19 @@ import { import { ArrowLeft, Information } from '@trycompai/design-system/icons'; import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from '@trycompai/ui/tooltip'; import type { DeviceWithChecks } from '../types'; +import { + CHECK_FIELDS, + PLATFORM_LABELS, + isDeviceOnline, + staleLabel, + staleTooltipCopy, +} from '../lib/device-source'; +import { NotTrackedBadge } from './DeviceListCells'; import { RevokeAgentAccessDialog } from './RevokeAgentAccessDialog'; -const CHECK_FIELDS = [ - { key: 'diskEncryptionEnabled' as const, dbKey: 'disk_encryption', label: 'Disk Encryption' }, - { key: 'antivirusEnabled' as const, dbKey: 'antivirus', label: 'Antivirus' }, - { key: 'passwordPolicySet' as const, dbKey: 'password_policy', label: 'Password Policy' }, - { key: 'screenLockEnabled' as const, dbKey: 'screen_lock', label: 'Screen Lock' }, -]; - -const PLATFORM_LABELS: Record = { - macos: 'macOS', - windows: 'Windows', - linux: 'Linux', -}; - -/** Device is considered online if it checked in within the last 2 hours */ -function isDeviceOnline(lastCheckIn: string | null): boolean { - if (!lastCheckIn) return false; - const diffMs = Date.now() - new Date(lastCheckIn).getTime(); - return diffMs < 2 * 60 * 60 * 1000; -} - -function staleLabel(daysSinceLastCheckIn: number | null): string { - return daysSinceLastCheckIn === null ? 'Stale' : `Stale (${daysSinceLastCheckIn}d)`; -} - -function staleTooltipCopy(daysSinceLastCheckIn: number | null): string { - return daysSinceLastCheckIn === null - ? "This device was registered but hasn't sent a compliance check yet. If it's not new, the agent may not be running or the device may be offline." - : "This device hasn't reported to CompAI in over 7 days, so we can't verify its current compliance. It may be offline, the agent may need to be updated, or the device may no longer be in use. Check with the employee."; -} - function DeviceComplianceBadge({ device }: { device: DeviceWithChecks }) { if (device.source === 'integration') { - const provider = device.integrationProvider?.name ?? 'an integration'; - return ( -
- Not tracked - - - - - - - {`This device was imported from ${provider}. CompAI doesn't collect compliance checks for imported devices — install the CompAI agent to track its security posture.`} - - - -
- ); + return ; } if (device.complianceStatus === 'stale') { return ( diff --git a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceListCells.tsx b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceListCells.tsx index 2654cbca9e..74db9d1380 100644 --- a/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceListCells.tsx +++ b/apps/app/src/app/(app)/[orgId]/people/devices/components/DeviceListCells.tsx @@ -23,50 +23,17 @@ import { } from '@trycompai/ui/tooltip'; import Link from 'next/link'; import type { DeviceWithChecks } from '../types'; -import { isComplianceTracked, sourceLabel } from '../lib/device-source'; - -export const CHECK_FIELDS = [ - { key: 'diskEncryptionEnabled' as const, label: 'Disk Encryption' }, - { key: 'antivirusEnabled' as const, label: 'Antivirus' }, - { key: 'passwordPolicySet' as const, label: 'Password Policy' }, - { key: 'screenLockEnabled' as const, label: 'Screen Lock' }, -]; - -export const PLATFORM_LABELS: Record = { - macos: 'macOS', - windows: 'Windows', - linux: 'Linux', -}; - -export function formatTimeAgo(dateString: string | null): string { - if (!dateString) return 'Never'; - const date = new Date(dateString); - const now = new Date(); - const diffMs = now.getTime() - date.getTime(); - const diffHours = Math.floor(diffMs / (1000 * 60 * 60)); - - if (diffHours < 1) return 'Just now'; - if (diffHours < 24) return `${diffHours}h ago`; - const diffDays = Math.floor(diffHours / 24); - return `${diffDays}d ago`; -} - -/** Device is considered online if it checked in within the last 2 hours. */ -export function isDeviceOnline(lastCheckIn: string | null): boolean { - if (!lastCheckIn) return false; - const diffMs = Date.now() - new Date(lastCheckIn).getTime(); - return diffMs < 2 * 60 * 60 * 1000; -} - -function staleLabel(daysSinceLastCheckIn: number | null): string { - return daysSinceLastCheckIn === null ? 'Stale' : `Stale (${daysSinceLastCheckIn}d)`; -} - -function staleTooltipCopy(daysSinceLastCheckIn: number | null): string { - return daysSinceLastCheckIn === null - ? "This device was registered but hasn't sent a compliance check yet. If it's not new, the agent may not be running or the device may be offline." - : "This device hasn't reported to CompAI in over 7 days, so we can't verify its current compliance. It may be offline, the agent may need to be updated, or the device may no longer be in use. Check with the employee."; -} +import { + CHECK_FIELDS, + PLATFORM_LABELS, + formatTimeAgo, + isComplianceTracked, + isDeviceOnline, + notTrackedTooltipCopy, + sourceLabel, + staleLabel, + staleTooltipCopy, +} from '../lib/device-source'; function InfoTooltip({ label, copy }: { label: string; copy: string }) { return ( @@ -124,21 +91,28 @@ export function UserNameCell({ ); } +/** + * Compliance badge for an integration-imported device. Shared with the details + * panel so the "Not tracked" copy and provider fallback stay consistent. + */ +export function NotTrackedBadge({ device }: { device: DeviceWithChecks }) { + return ( +
+ Not tracked + +
+ ); +} + export function CompliantBadge({ device }: { device: DeviceWithChecks }) { // Integration-imported devices are inventory records, not compliance records — // CompAI never ran security checks on them, so showing "No" (red) would be a // false negative. Present them as untracked instead. if (!isComplianceTracked(device)) { - const provider = device.integrationProvider?.name ?? 'an integration'; - return ( -
- Not tracked - -
- ); + return ; } if (device.complianceStatus === 'stale') { diff --git a/apps/app/src/app/(app)/[orgId]/people/devices/lib/device-source.ts b/apps/app/src/app/(app)/[orgId]/people/devices/lib/device-source.ts index 0ea9f8adb1..5658cae312 100644 --- a/apps/app/src/app/(app)/[orgId]/people/devices/lib/device-source.ts +++ b/apps/app/src/app/(app)/[orgId]/people/devices/lib/device-source.ts @@ -1,8 +1,8 @@ import type { DeviceWithChecks } from '../types'; /** - * Human label for where a device came from. Shared by the devices table and the - * CSV export so the two can't drift on source labels. + * Human label for where a device came from. Shared by the devices table, the + * details panel, and the CSV export so they can't drift on source labels. */ export function sourceLabel(device: DeviceWithChecks): string { if (device.source === 'integration') { @@ -12,6 +12,18 @@ export function sourceLabel(device: DeviceWithChecks): string { return 'Comp Agent'; } +/** + * Stable identifier for a device's source — used to key the source filter so two + * distinct providers that happen to share a display name don't collapse into one + * option. (sourceLabel is for display only.) + */ +export function sourceKey(device: DeviceWithChecks): string { + if (device.source === 'integration') { + return `integration:${device.integrationProvider?.slug ?? 'unknown'}`; + } + return device.source; // 'device_agent' | 'fleet' +} + /** * True for devices whose compliance posture CompAI actually collects. Only * integration-imported devices are inventory-only ("Not tracked"); agent and @@ -20,3 +32,52 @@ export function sourceLabel(device: DeviceWithChecks): string { export function isComplianceTracked(device: DeviceWithChecks): boolean { return device.source !== 'integration'; } + +// --------------------------------------------------------------------------- +// Shared device presentation helpers (used by both the list and details views +// so copy/labels/thresholds can't drift between them). +// --------------------------------------------------------------------------- + +export const PLATFORM_LABELS: Record = { + macos: 'macOS', + windows: 'Windows', + linux: 'Linux', +}; + +export const CHECK_FIELDS = [ + { key: 'diskEncryptionEnabled' as const, dbKey: 'disk_encryption', label: 'Disk Encryption' }, + { key: 'antivirusEnabled' as const, dbKey: 'antivirus', label: 'Antivirus' }, + { key: 'passwordPolicySet' as const, dbKey: 'password_policy', label: 'Password Policy' }, + { key: 'screenLockEnabled' as const, dbKey: 'screen_lock', label: 'Screen Lock' }, +]; + +export function formatTimeAgo(dateString: string | null): string { + if (!dateString) return 'Never'; + const diffMs = Date.now() - new Date(dateString).getTime(); + const diffHours = Math.floor(diffMs / (1000 * 60 * 60)); + if (diffHours < 1) return 'Just now'; + if (diffHours < 24) return `${diffHours}h ago`; + return `${Math.floor(diffHours / 24)}d ago`; +} + +/** Device is considered online if it checked in within the last 2 hours. */ +export function isDeviceOnline(lastCheckIn: string | null): boolean { + if (!lastCheckIn) return false; + return Date.now() - new Date(lastCheckIn).getTime() < 2 * 60 * 60 * 1000; +} + +export function staleLabel(daysSinceLastCheckIn: number | null): string { + return daysSinceLastCheckIn === null ? 'Stale' : `Stale (${daysSinceLastCheckIn}d)`; +} + +export function staleTooltipCopy(daysSinceLastCheckIn: number | null): string { + return daysSinceLastCheckIn === null + ? "This device was registered but hasn't sent a compliance check yet. If it's not new, the agent may not be running or the device may be offline." + : "This device hasn't reported to CompAI in over 7 days, so we can't verify its current compliance. It may be offline, the agent may need to be updated, or the device may no longer be in use. Check with the employee."; +} + +/** Tooltip copy for the "Not tracked" badge on integration-imported devices. */ +export function notTrackedTooltipCopy(device: DeviceWithChecks): string { + const provider = device.integrationProvider?.name ?? 'an integration'; + return `This device was imported from ${provider}. CompAI doesn't collect compliance checks for imported devices — install the CompAI agent to track its security posture.`; +} diff --git a/apps/app/src/app/(app)/[orgId]/people/devices/lib/devices-csv.ts b/apps/app/src/app/(app)/[orgId]/people/devices/lib/devices-csv.ts index 36b4cf23cc..638ab9aaf9 100644 --- a/apps/app/src/app/(app)/[orgId]/people/devices/lib/devices-csv.ts +++ b/apps/app/src/app/(app)/[orgId]/people/devices/lib/devices-csv.ts @@ -1,5 +1,5 @@ import type { DeviceWithChecks } from '../types'; -import { sourceLabel } from './device-source'; +import { isComplianceTracked, sourceLabel } from './device-source'; export const DEVICES_CSV_HEADER = [ 'Device Name', @@ -38,9 +38,9 @@ function yesNo(value: boolean): 'yes' | 'no' { export function buildDevicesCsv(devices: DeviceWithChecks[]): string { const rows = devices.map((d) => { - // Only agent devices carry real compliance data. For imported/fleet devices, - // export "not_tracked"/"n/a" rather than a misleading non_compliant + "no". - const tracked = d.source === 'device_agent'; + // Integration-imported devices are inventory-only; agent + Fleet carry real + // compliance. Use the shared helper so the CSV matches the rest of the UI. + const tracked = isComplianceTracked(d); const status = tracked ? d.complianceStatus : 'not_tracked'; const check = (value: boolean) => (tracked ? yesNo(value) : 'n/a'); return [ diff --git a/apps/app/src/app/api/people/agent-devices/route.test.ts b/apps/app/src/app/api/people/agent-devices/route.test.ts index d61d763aeb..cc1b65d736 100644 --- a/apps/app/src/app/api/people/agent-devices/route.test.ts +++ b/apps/app/src/app/api/people/agent-devices/route.test.ts @@ -1,15 +1,8 @@ import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; +import { NextResponse } from 'next/server'; -vi.mock('@/utils/auth', () => ({ - auth: { - api: { - getSession: vi.fn(), - }, - }, -})); - -vi.mock('next/headers', () => ({ - headers: vi.fn(async () => new Headers()), +vi.mock('@/lib/permissions.server', () => ({ + requireApiPermission: vi.fn(), })); vi.mock('@db/server', () => ({ @@ -17,14 +10,17 @@ vi.mock('@db/server', () => ({ device: { findMany: vi.fn(), }, + integrationConnection: { + findMany: vi.fn(async () => []), + }, }, })); -import { auth } from '@/utils/auth'; +import { requireApiPermission } from '@/lib/permissions.server'; import { db } from '@db/server'; import { GET } from './route'; -const mockedGetSession = vi.mocked(auth.api.getSession); +const mockedRequire = vi.mocked(requireApiPermission); const mockedFindMany = vi.mocked( (db as unknown as { device: { findMany: ReturnType } }).device.findMany, ); @@ -32,6 +28,18 @@ const mockedFindMany = vi.mocked( // Freeze "now" so day math is deterministic. const FIXED_NOW = new Date('2026-04-17T12:00:00.000Z'); +function req() { + return new Request('http://test/api/people/agent-devices'); +} + +function grant() { + mockedRequire.mockResolvedValue({ + organizationId: 'org_1', + userId: 'u_1', + permissions: {}, + } as unknown as Awaited>); +} + beforeAll(() => { vi.useFakeTimers(); vi.setSystemTime(FIXED_NOW); @@ -43,9 +51,7 @@ afterAll(() => { beforeEach(() => { vi.clearAllMocks(); - mockedGetSession.mockResolvedValue({ - session: { activeOrganizationId: 'org_1' }, - } as unknown as Awaited>); + grant(); }); function deviceRow(overrides: Partial> = {}) { @@ -73,15 +79,18 @@ function deviceRow(overrides: Partial> = {}) { } describe('GET /api/people/agent-devices', () => { - it('returns 401 when no organization is active', async () => { - mockedGetSession.mockResolvedValue({ session: {} } as never); - const res = await GET(); - expect(res.status).toBe(401); + it('forwards the 401/403 response when the RBAC guard denies access', async () => { + mockedRequire.mockResolvedValue( + NextResponse.json({ error: 'Forbidden' }, { status: 403 }), + ); + const res = await GET(req()); + expect(res.status).toBe(403); + expect(mockedFindMany).not.toHaveBeenCalled(); }); it('marks a fresh + isCompliant device as compliant', async () => { mockedFindMany.mockResolvedValue([deviceRow({ lastCheckIn: new Date(FIXED_NOW) })]); - const res = await GET(); + const res = await GET(req()); const body = await res.json(); expect(body.data[0].complianceStatus).toBe('compliant'); expect(body.data[0].daysSinceLastCheckIn).toBe(0); @@ -95,7 +104,7 @@ describe('GET /api/people/agent-devices', () => { lastCheckIn: new Date(FIXED_NOW), }), ]); - const res = await GET(); + const res = await GET(req()); const body = await res.json(); expect(body.data[0].complianceStatus).toBe('non_compliant'); }); @@ -103,7 +112,7 @@ describe('GET /api/people/agent-devices', () => { it('marks a device with lastCheckIn >= 7 days ago as stale', async () => { const eightDaysAgo = new Date(FIXED_NOW.getTime() - 8 * 24 * 60 * 60 * 1000); mockedFindMany.mockResolvedValue([deviceRow({ lastCheckIn: eightDaysAgo })]); - const res = await GET(); + const res = await GET(req()); const body = await res.json(); expect(body.data[0].complianceStatus).toBe('stale'); expect(body.data[0].daysSinceLastCheckIn).toBe(8); @@ -111,7 +120,7 @@ describe('GET /api/people/agent-devices', () => { it('marks a device with null lastCheckIn as stale', async () => { mockedFindMany.mockResolvedValue([deviceRow({ lastCheckIn: null })]); - const res = await GET(); + const res = await GET(req()); const body = await res.json(); expect(body.data[0].complianceStatus).toBe('stale'); expect(body.data[0].daysSinceLastCheckIn).toBeNull(); @@ -127,7 +136,7 @@ describe('GET /api/people/agent-devices', () => { deviceRow({ id: 'dev_expired', agentSession: { expiresAt: past } }), ]); - const res = await GET(); + const res = await GET(req()); const body = await res.json(); const byId = Object.fromEntries( body.data.map((d: { id: string }) => [d.id, d]), diff --git a/apps/app/src/app/api/people/agent-devices/route.ts b/apps/app/src/app/api/people/agent-devices/route.ts index 30c14386f5..c44fa85ae4 100644 --- a/apps/app/src/app/api/people/agent-devices/route.ts +++ b/apps/app/src/app/api/people/agent-devices/route.ts @@ -1,7 +1,6 @@ -import { auth } from '@/utils/auth'; import { db } from '@db/server'; -import { headers } from 'next/headers'; import { NextResponse } from 'next/server'; +import { requireApiPermission } from '@/lib/permissions.server'; import { daysSinceCheckIn, getDeviceComplianceStatus, @@ -15,13 +14,14 @@ function mapSource(source: string): DeviceWithChecks['source'] { return 'device_agent'; } -export async function GET() { - const session = await auth.api.getSession({ headers: await headers() }); - const organizationId = session?.session.activeOrganizationId; - - if (!organizationId) { - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }); - } +export async function GET(req: Request) { + // Enforce the same RBAC as the People area (route permission 'people' = + // member:read). The session-only check was insufficient — this route returns + // org device + integration-provider data and can be called directly by any + // active-org session, so gate it explicitly. + const ctx = await requireApiPermission(req, 'member', 'read'); + if (ctx instanceof NextResponse) return ctx; + const { organizationId } = ctx; const devices = await db.device.findMany({ where: { diff --git a/apps/app/src/app/api/people/fleet-hosts/route.ts b/apps/app/src/app/api/people/fleet-hosts/route.ts index 4d65c52961..4046f585bb 100644 --- a/apps/app/src/app/api/people/fleet-hosts/route.ts +++ b/apps/app/src/app/api/people/fleet-hosts/route.ts @@ -1,19 +1,17 @@ -import { auth } from '@/utils/auth'; import { getFleetInstance } from '@/lib/fleet'; import { db } from '@db/server'; -import { headers } from 'next/headers'; import { NextResponse } from 'next/server'; +import { requireApiPermission } from '@/lib/permissions.server'; import type { Host } from '@/app/(app)/[orgId]/people/devices/types'; const MDM_POLICY_ID = -9999; -export async function GET() { - const session = await auth.api.getSession({ headers: await headers() }); - const organizationId = session?.session.activeOrganizationId; - - if (!organizationId) { - return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }); - } +export async function GET(req: Request) { + // Same RBAC as the People area (member:read) — this returns org device data + // and must not be reachable by an active-org session lacking people access. + const ctx = await requireApiPermission(req, 'member', 'read'); + if (ctx instanceof NextResponse) return ctx; + const { organizationId } = ctx; const fleet = await getFleetInstance();