Skip to content

Commit d1d4e69

Browse files
fix: respect RBAC roles for device status
[dev] [Marfuen] mariano/fix-fleet-device-compliance-v2
1 parent 249e7e9 commit d1d4e69

File tree

4 files changed

+77
-10
lines changed

4 files changed

+77
-10
lines changed

apps/app/src/app/(app)/[orgId]/people/all/components/MemberRow.test.tsx

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,9 +90,11 @@ describe('MemberRow device status', () => {
9090
expect(screen.getByText('Not Installed').className).toContain('text-muted-foreground');
9191
});
9292

93-
it('shows "Not Installed" by default when deviceStatus is omitted', () => {
93+
it('shows dash when deviceStatus is omitted (no compliance obligation)', () => {
9494
renderMemberRow();
95-
expect(screen.getByText('Not Installed')).toBeInTheDocument();
95+
expect(screen.queryByText('Not Installed')).not.toBeInTheDocument();
96+
expect(screen.queryByText('Compliant')).not.toBeInTheDocument();
97+
expect(screen.queryByText('Non-Compliant')).not.toBeInTheDocument();
9698
});
9799

98100
it('shows "Compliant" with green dot when deviceStatus is compliant', () => {
@@ -177,4 +179,58 @@ describe('MemberRow device status', () => {
177179
const redDot = c3.querySelector('.bg-red-400');
178180
expect(redDot).toBeInTheDocument();
179181
});
182+
183+
it('does not show device status for member without compliance obligation (e.g. auditor)', () => {
184+
const auditorMember = {
185+
...baseMember,
186+
role: 'auditor',
187+
} as unknown as MemberWithUser;
188+
189+
render(
190+
<table>
191+
<tbody>
192+
<MemberRow
193+
member={auditorMember}
194+
onRemove={noop}
195+
onRemoveDevice={noop}
196+
onUpdateRole={noop}
197+
onReactivate={noop}
198+
canEdit={false}
199+
isCurrentUserOwner={false}
200+
// deviceStatus intentionally omitted — auditor won't be in the map
201+
/>
202+
</tbody>
203+
</table>,
204+
);
205+
206+
expect(screen.queryByText('Not Installed')).not.toBeInTheDocument();
207+
expect(screen.queryByText('Compliant')).not.toBeInTheDocument();
208+
expect(screen.queryByText('Non-Compliant')).not.toBeInTheDocument();
209+
});
210+
211+
it('still shows device status for member with compliance obligation', () => {
212+
const employeeMember = {
213+
...baseMember,
214+
role: 'employee',
215+
} as unknown as MemberWithUser;
216+
217+
render(
218+
<table>
219+
<tbody>
220+
<MemberRow
221+
member={employeeMember}
222+
onRemove={noop}
223+
onRemoveDevice={noop}
224+
onUpdateRole={noop}
225+
onReactivate={noop}
226+
canEdit={false}
227+
isCurrentUserOwner={false}
228+
deviceStatus="compliant"
229+
/>
230+
</tbody>
231+
</table>,
232+
);
233+
234+
expect(screen.getByText('Compliant')).toBeInTheDocument();
235+
});
180236
});

apps/app/src/app/(app)/[orgId]/people/all/components/MemberRow.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ export function MemberRow({
9595
isCurrentUserOwner,
9696
customRoles = [],
9797
taskCompletion,
98-
deviceStatus = 'not-installed',
98+
deviceStatus,
9999
}: MemberRowProps) {
100100
const { orgId } = useParams<{ orgId: string }>();
101101

@@ -233,7 +233,7 @@ export function MemberRow({
233233

234234
{/* DEVICE */}
235235
<TableCell>
236-
{isPlatformAdmin || isDeactivated ? (
236+
{isPlatformAdmin || isDeactivated || !deviceStatus ? (
237237
<Text size="sm" variant="muted">
238238
239239
</Text>
@@ -321,7 +321,7 @@ export function MemberRow({
321321
</DropdownMenuItem>
322322
)}
323323
{!isDeactivated &&
324-
(member.fleetDmLabelId || deviceStatus !== 'not-installed') &&
324+
(member.fleetDmLabelId || (deviceStatus && deviceStatus !== 'not-installed')) &&
325325
isCurrentUserOwner && (
326326
<DropdownMenuItem
327327
onSelect={() => {

apps/app/src/app/(app)/[orgId]/people/all/components/TeamMembersClient.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -470,7 +470,7 @@ export function TeamMembersClient({
470470
isCurrentUserOwner={isCurrentUserOwner}
471471
customRoles={customRoles}
472472
taskCompletion={taskCompletionMap[(item as MemberWithUser).id]}
473-
deviceStatus={deviceStatusMap[(item as MemberWithUser).id] ?? 'not-installed'}
473+
deviceStatus={deviceStatusMap[(item as MemberWithUser).id]}
474474
/>
475475
) : (
476476
<PendingInvitationRow

apps/app/src/app/(app)/[orgId]/people/page.tsx

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -157,12 +157,22 @@ export default async function PeoplePage({ params }: { params: Promise<{ orgId:
157157

158158
// Build unified device status map from the SAME data both tabs use.
159159
// This ensures the member list and compliance chart agree on compliance.
160+
// Only members with compliance obligations are checked (uses RBAC obligations,
161+
// not hardcoded role names). Members without compliance obligations (e.g.
162+
// auditor-only, or custom roles without compliance: true) are omitted and
163+
// will show "—" in the DEVICE column.
160164
const deviceStatusMap: Record<string, 'compliant' | 'non-compliant' | 'not-installed'> = {};
165+
const complianceMemberIds = new Set(employees.map((m) => m.id));
166+
167+
// Default all compliance members to "not-installed" — device data below overrides
168+
for (const id of complianceMemberIds) {
169+
deviceStatusMap[id] = 'not-installed';
170+
}
161171

162172
// Device-agent devices: compliant only if ALL of a member's devices pass
163173
const agentComplianceByMember = new Map<string, boolean>();
164174
for (const d of agentDevices) {
165-
if (!d.memberId) continue;
175+
if (!d.memberId || !complianceMemberIds.has(d.memberId)) continue;
166176
const prev = agentComplianceByMember.get(d.memberId);
167177
agentComplianceByMember.set(d.memberId, (prev ?? true) && d.isCompliant);
168178
}
@@ -173,12 +183,13 @@ export default async function PeoplePage({ params }: { params: Promise<{ orgId:
173183
// Fleet-only devices: use the same merged policy data the chart uses
174184
// (Fleet API automated checks + DB manual overrides, already combined by getFleetHosts)
175185
for (const host of filteredFleetDevices) {
176-
if (!host.member_id) continue;
186+
if (!host.member_id || !complianceMemberIds.has(host.member_id)) continue;
177187
// If already set by device-agent, skip (agent takes priority)
178188
if (agentComplianceByMember.has(host.member_id)) continue;
179189
const isCompliant = host.policies.every((p) => p.response === 'pass');
180-
// If multiple fleet devices for same member, non-compliant if ANY device fails
181-
if (!isCompliant || !deviceStatusMap[host.member_id]) {
190+
// If multiple fleet devices for same member, non-compliant if ANY device fails.
191+
// Once non-compliant, a later compliant device cannot upgrade it back.
192+
if (deviceStatusMap[host.member_id] !== 'non-compliant') {
182193
deviceStatusMap[host.member_id] = isCompliant ? 'compliant' : 'non-compliant';
183194
}
184195
}

0 commit comments

Comments
 (0)