-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[PM-1214] device management screen #12455
base: main
Are you sure you want to change the base?
Changes from 24 commits
79327d8
d4c0091
efda9ae
a4d1740
dc2596e
4c49954
5eb9955
99f6af0
f1a59e9
ebddea0
b078c74
660aba2
80760c8
278fa9b
eef346a
74363fb
bcd847d
0561cd8
f336afc
82dbc98
d995fe5
3eb0ef7
c3c73cc
6ca912a
99a647c
d01ca4d
0932e18
3934101
4bb0459
c8dc459
66e497c
59cae5b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,66 @@ | ||
<div class="tabbed-content"> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question: Why And a slight UI adjustment: The header for the Devices table isn't quite formatted according to Figma. There should be a bit more spacing between the heading and subheading/paragraph text. (It also doesn't look the same as the headers in the other tabs: Master Password, Two-step login). Figma Current Implementation |
||
<h2> | ||
{{ "devices" | i18n }} | ||
<i class="bwi bwi-question-circle tw-h-5 tw-w-5 bwi-color-primary" aria-hidden="true"></i> | ||
</h2> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The figma calls for a loading icon here when async actions like remove device or log out are being taken There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
<p>{{ "deviceListDescription" | i18n }}</p> | ||
|
||
<bit-table-scroll [dataSource]="dataSource" [rowSize]="50"> | ||
<ng-container header> | ||
<th | ||
*ngFor="let col of columnConfig" | ||
[class]="col.headerClass" | ||
bitCell | ||
[bitSortable]="col.sortable ? col.name : null" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
[default]="col.name === 'displayName' ? 'desc' : null" | ||
scope="col" | ||
role="columnheader" | ||
> | ||
{{ col.title }} | ||
</th> | ||
<th bitCell scope="col" role="columnheader"></th> | ||
</ng-container> | ||
<ng-template bitRowDef let-row> | ||
<td bitCell class="tw-flex tw-gap-2"> | ||
<div class="tw-flex tw-items-center tw-justify-center tw-w-10"> | ||
<i [class]="getDeviceIcon(row.type)" class="bwi-lg" aria-hidden="true"></i> | ||
</div> | ||
<div> | ||
{{ row.displayName }} | ||
<span *ngIf="row.trusted" class="tw-text-sm tw-text-muted tw-block"> | ||
{{ "trusted" | i18n }} | ||
</span> | ||
</div> | ||
</td> | ||
<td bitCell> | ||
<span *ngIf="isCurrentDevice(row)" bitBadge variant="primary">{{ | ||
"currentSession" | i18n | ||
}}</span> | ||
<span *ngIf="hasPendingAuthRequest(row)" bitBadge variant="warning">{{ | ||
"requestPending" | i18n | ||
}}</span> | ||
</td> | ||
<td bitCell>{{ row.firstLogin | date: "medium" }}</td> | ||
<td bitCell> | ||
<button | ||
type="button" | ||
bitIconButton="bwi-ellipsis-v" | ||
[bitMenuTriggerFor]="optionsMenu" | ||
></button> | ||
<bit-menu #optionsMenu> | ||
<button | ||
type="button" | ||
bitMenuItem | ||
(click)="removeDevice(row)" | ||
[disabled]="isCurrentDevice(row)" | ||
> | ||
<span [class]="isCurrentDevice(row) ? 'tw-text-muted' : 'tw-text-danger'"> | ||
<i class="bwi bwi-trash" aria-hidden="true"></i> | ||
{{ "remove" | i18n }} | ||
JaredSnider-Bitwarden marked this conversation as resolved.
Show resolved
Hide resolved
|
||
</span> | ||
</button> | ||
</bit-menu> | ||
</td> | ||
</ng-template> | ||
</bit-table-scroll> | ||
</div> |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Excellent docs throughout. Thank you! |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,207 @@ | ||
import { CommonModule } from "@angular/common"; | ||
import { Component } from "@angular/core"; | ||
import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; | ||
import { firstValueFrom } from "rxjs"; | ||
|
||
import { DeviceTrustServiceAbstraction } from "@bitwarden/common/auth/abstractions/device-trust.service.abstraction"; | ||
import { DevicesServiceAbstraction } from "@bitwarden/common/auth/abstractions/devices/devices.service.abstraction"; | ||
import { DeviceView } from "@bitwarden/common/auth/abstractions/devices/views/device.view"; | ||
import { DeviceType, DeviceTypeMetadata } from "@bitwarden/common/enums"; | ||
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; | ||
import { DialogService, ToastService, TableDataSource, TableModule } from "@bitwarden/components"; | ||
|
||
import { SharedModule } from "../../../shared"; | ||
|
||
interface DeviceTableData { | ||
id: string; | ||
type: DeviceType; | ||
displayName: string; | ||
loginStatus: string; | ||
firstLogin: Date; | ||
trusted: boolean; | ||
devicePendingAuthRequest: object | null; | ||
} | ||
|
||
/** | ||
* Provides a table of devices and allows the user to log out, approve or remove a device | ||
*/ | ||
@Component({ | ||
selector: "app-device-management", | ||
templateUrl: "./device-management.component.html", | ||
standalone: true, | ||
imports: [CommonModule, SharedModule, TableModule], | ||
}) | ||
export class DeviceManagementComponent { | ||
protected readonly tableId = "device-management-table"; | ||
protected dataSource = new TableDataSource<DeviceTableData>(); | ||
protected currentDevice: DeviceView | undefined; | ||
|
||
constructor( | ||
private i18nService: I18nService, | ||
private devicesService: DevicesServiceAbstraction, | ||
private deviceTrustService: DeviceTrustServiceAbstraction, | ||
private dialogService: DialogService, | ||
private toastService: ToastService, | ||
) { | ||
// Get current device | ||
this.deviceTrustService | ||
.getCurrentDevice$() | ||
.pipe(takeUntilDestroyed()) | ||
.subscribe((device) => { | ||
this.currentDevice = new DeviceView(device); | ||
}); | ||
|
||
// Get all devices and map them for the table | ||
this.devicesService | ||
.getDevices$() | ||
.pipe(takeUntilDestroyed()) | ||
.subscribe((devices) => { | ||
this.dataSource.data = devices.map((device) => { | ||
return { | ||
id: device.id, | ||
type: device.type, | ||
displayName: this.getHumanReadableDeviceType(device.type), | ||
loginStatus: this.getLoginStatus(device), | ||
devicePendingAuthRequest: device.response.devicePendingAuthRequest, | ||
firstLogin: new Date(device.creationDate), | ||
trusted: device.response.isTrusted, | ||
}; | ||
}); | ||
}); | ||
} | ||
|
||
/** | ||
* Column configuration for the table | ||
*/ | ||
protected readonly columnConfig = [ | ||
{ | ||
name: "displayName", | ||
title: this.i18nService.t("device"), | ||
headerClass: "tw-w-1/3", | ||
sortable: true, | ||
}, | ||
{ | ||
name: "loginStatus", | ||
title: this.i18nService.t("loginStatus"), | ||
headerClass: "tw-w-1/3", | ||
sortable: true, | ||
}, | ||
{ | ||
name: "firstLogin", | ||
title: this.i18nService.t("firstLogin"), | ||
headerClass: "tw-w-1/3", | ||
sortable: true, | ||
}, | ||
]; | ||
|
||
/** | ||
* Get the icon for a device type | ||
* @param type - The device type | ||
* @returns The icon for the device type | ||
*/ | ||
getDeviceIcon(type: DeviceType): string { | ||
const defaultIcon = "bwi bwi-desktop"; | ||
const categoryIconMap: Record<string, string> = { | ||
webVault: "bwi bwi-browser", | ||
desktop: "bwi bwi-desktop", | ||
mobile: "bwi bwi-mobile", | ||
cli: "bwi bwi-cli", | ||
extension: "bwi bwi-puzzle", | ||
sdk: "bwi bwi-desktop", | ||
}; | ||
|
||
const metadata = DeviceTypeMetadata[type]; | ||
return metadata ? (categoryIconMap[metadata.category] ?? defaultIcon) : defaultIcon; | ||
} | ||
|
||
/** | ||
* Get the login status of a device | ||
* It will return the current session if the device is the current device | ||
* It will return the date of the pending auth request when available | ||
* @param device - The device | ||
* @returns The login status | ||
*/ | ||
private getLoginStatus(device: DeviceView): string { | ||
if (this.isCurrentDevice(device)) { | ||
return this.i18nService.t("currentSession"); | ||
} | ||
|
||
if (device.response.devicePendingAuthRequest?.creationDate) { | ||
return this.i18nService.t("requestPending"); | ||
} | ||
|
||
return ""; | ||
} | ||
|
||
/** | ||
* Get a human readable device type from the DeviceType enum | ||
* @param type - The device type | ||
* @returns The human readable device type | ||
*/ | ||
private getHumanReadableDeviceType(type: DeviceType): string { | ||
const metadata = DeviceTypeMetadata[type]; | ||
if (!metadata) { | ||
return this.i18nService.t("unknownDevice"); | ||
} | ||
|
||
const category = this.i18nService.t(metadata.category); | ||
return metadata.platform ? `${category} - ${metadata.platform}` : category; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. โ : Nice job building out this metadata out. Do you think we should translate "Unknown" as a platform? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you elaborate on that? Are you talking about the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I'm sorry that comment was not as clear as I meant for it to be. "Unknown" is the only non proper noun so we could translate it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah ok that makes sense! 4bb0459 |
||
} | ||
|
||
/** | ||
* Check if a device is the current device | ||
* @param device - The device or device table data | ||
* @returns True if the device is the current device, false otherwise | ||
*/ | ||
protected isCurrentDevice(device: DeviceView | DeviceTableData): boolean { | ||
return "response" in device | ||
? device.id === this.currentDevice?.id | ||
: device.id === this.currentDevice?.id; | ||
} | ||
|
||
/** | ||
* Check if a device has a pending auth request | ||
* @param device - The device | ||
* @returns True if the device has a pending auth request, false otherwise | ||
*/ | ||
protected hasPendingAuthRequest(device: DeviceTableData): boolean { | ||
return ( | ||
device.devicePendingAuthRequest !== undefined && device.devicePendingAuthRequest !== null | ||
); | ||
} | ||
|
||
/** | ||
* Remove a device | ||
* @param device - The device | ||
*/ | ||
protected async removeDevice(device: DeviceTableData) { | ||
const confirmed = await this.dialogService.openSimpleDialog({ | ||
title: { key: "removeDevice" }, | ||
content: { key: "removeDeviceConfirmation" }, | ||
type: "warning", | ||
}); | ||
|
||
if (!confirmed) { | ||
return; | ||
} | ||
|
||
try { | ||
await firstValueFrom(this.devicesService.deactivateDevice$(device.id)); | ||
|
||
// Remove the device from the data source | ||
this.dataSource.data = this.dataSource.data.filter((d) => d.id !== device.id); | ||
|
||
this.toastService.showToast({ | ||
title: "", | ||
message: this.i18nService.t("deviceRemoved"), | ||
variant: "success", | ||
}); | ||
} catch (e) { | ||
this.toastService.showToast({ | ||
title: "", | ||
message: this.i18nService.t("errorOccurred"), | ||
variant: "warning", | ||
}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. โ : can we change this to be a call to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ | |
import { ChangePasswordComponent } from "../change-password.component"; | ||
import { TwoFactorSetupComponent } from "../two-factor/two-factor-setup.component"; | ||
|
||
import { DeviceManagementComponent } from "./device-management.component"; | ||
import { SecurityKeysComponent } from "./security-keys.component"; | ||
import { SecurityComponent } from "./security.component"; | ||
|
||
|
@@ -29,6 +30,11 @@ | |
component: SecurityKeysComponent, | ||
data: { titleId: "keys" }, | ||
}, | ||
{ | ||
path: "device-management", | ||
component: DeviceManagementComponent, | ||
data: { titleId: "devices" }, | ||
}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
], | ||
}, | ||
]; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
๐ก : upon watching the video, I think we should implement a loading state for the table which doesn't show the table until the data is loaded.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added a loading state for when the table is loading, and added the spinner for when async actions are waiting to complete: 0932e18