1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-11 05:43:41 +00:00

fix(device-management-sorting): [Auth/PM-24046] Device Management Sorting (#15889)

This PR makes it so the devices are always sorted in this order (by default):

1. Has Pending Auth Request (if any) comes first
2. Current Device comes second (or first if there are no pending auth requests)
3. First Login Date - the rest of the devices are sorted by first login date (newest to oldest)

This sort order is preserved even after a user approves/denies and auth request - that is, the approved/denied device will re-sort to its correct position according to it's first login date.

Feature Flag: `PM14938_BrowserExtensionLoginApproval`
This commit is contained in:
rr-bw
2025-08-07 13:46:02 -07:00
committed by GitHub
parent b57238ca99
commit c5e5417abf
7 changed files with 51 additions and 71 deletions

View File

@@ -6,8 +6,7 @@
bit-item-content bit-item-content
type="button" type="button"
[attr.tabindex]="device.pendingAuthRequest != null ? 0 : null" [attr.tabindex]="device.pendingAuthRequest != null ? 0 : null"
(click)="approveOrDenyAuthRequest(device.pendingAuthRequest)" (click)="answerAuthRequest(device.pendingAuthRequest)"
(keydown.enter)="approveOrDenyAuthRequest(device.pendingAuthRequest)"
> >
<!-- Default Content --> <!-- Default Content -->
<span class="tw-text-base">{{ device.displayName }}</span> <span class="tw-text-base">{{ device.displayName }}</span>
@@ -21,7 +20,7 @@
<!-- Secondary Content --> <!-- Secondary Content -->
<span slot="secondary" class="tw-text-sm"> <span slot="secondary" class="tw-text-sm">
<span>{{ "needsApproval" | i18n }}</span> <br />
<div> <div>
<span class="tw-font-semibold"> {{ "firstLogin" | i18n }}: </span> <span class="tw-font-semibold"> {{ "firstLogin" | i18n }}: </span>
<span>{{ device.firstLogin | date: "medium" }}</span> <span>{{ device.firstLogin | date: "medium" }}</span>

View File

@@ -1,15 +1,11 @@
import { CommonModule } from "@angular/common"; import { CommonModule } from "@angular/common";
import { Component, Input } from "@angular/core"; import { Component, EventEmitter, Input, Output } from "@angular/core";
import { firstValueFrom } from "rxjs";
import { DevicePendingAuthRequest } from "@bitwarden/common/auth/abstractions/devices/responses/device.response"; import { DevicePendingAuthRequest } from "@bitwarden/common/auth/abstractions/devices/responses/device.response";
import { BadgeModule, DialogService, ItemModule } from "@bitwarden/components"; import { BadgeModule, ItemModule } from "@bitwarden/components";
import { I18nPipe } from "@bitwarden/ui-common"; import { I18nPipe } from "@bitwarden/ui-common";
import { LoginApprovalDialogComponent } from "../login-approval/login-approval-dialog.component";
import { DeviceDisplayData } from "./device-management.component"; import { DeviceDisplayData } from "./device-management.component";
import { clearAuthRequestAndResortDevices } from "./resort-devices.helper";
/** Displays user devices in an item list view */ /** Displays user devices in an item list view */
@Component({ @Component({
@@ -20,24 +16,12 @@ import { clearAuthRequestAndResortDevices } from "./resort-devices.helper";
}) })
export class DeviceManagementItemGroupComponent { export class DeviceManagementItemGroupComponent {
@Input() devices: DeviceDisplayData[] = []; @Input() devices: DeviceDisplayData[] = [];
@Output() onAuthRequestAnswered = new EventEmitter<DevicePendingAuthRequest>();
constructor(private dialogService: DialogService) {} protected answerAuthRequest(pendingAuthRequest: DevicePendingAuthRequest | null) {
protected async approveOrDenyAuthRequest(pendingAuthRequest: DevicePendingAuthRequest | null) {
if (pendingAuthRequest == null) { if (pendingAuthRequest == null) {
return; return;
} }
this.onAuthRequestAnswered.emit(pendingAuthRequest);
const loginApprovalDialog = LoginApprovalDialogComponent.open(this.dialogService, {
notificationId: pendingAuthRequest.id,
});
const result = await firstValueFrom(loginApprovalDialog.closed);
if (result !== undefined && typeof result === "boolean") {
// Auth request was approved or denied, so clear the
// pending auth request and re-sort the device array
this.devices = clearAuthRequestAndResortDevices(this.devices, pendingAuthRequest);
}
} }
} }

View File

@@ -1,4 +1,4 @@
<bit-table-scroll [dataSource]="tableDataSource" [rowSize]="50"> <bit-table-scroll [dataSource]="tableDataSource" [rowSize]="64">
<!-- Table Header --> <!-- Table Header -->
<ng-container header> <ng-container header>
<th <th
@@ -6,7 +6,6 @@
[class]="column.headerClass" [class]="column.headerClass"
bitCell bitCell
[bitSortable]="column.sortable ? column.name : ''" [bitSortable]="column.sortable ? column.name : ''"
[default]="column.name === 'loginStatus' ? 'desc' : false"
scope="col" scope="col"
role="columnheader" role="columnheader"
> >
@@ -17,24 +16,17 @@
<!-- Table Rows --> <!-- Table Rows -->
<ng-template bitRowDef let-device> <ng-template bitRowDef let-device>
<!-- Column: Device Name --> <!-- Column: Device Name -->
<td bitCell class="tw-flex tw-gap-2"> <td bitCell class="tw-flex tw-gap-2 tw-items-center tw-h-16">
<div class="tw-flex tw-items-center tw-justify-center tw-w-10"> <div class="tw-flex tw-items-center tw-justify-center tw-w-10">
<i [class]="device.icon" class="bwi-lg" aria-hidden="true"></i> <i [class]="device.icon" class="bwi-lg" aria-hidden="true"></i>
</div> </div>
<div> <div>
@if (device.pendingAuthRequest) { @if (device.pendingAuthRequest) {
<a <a bitLink href="#" appStopClick (click)="answerAuthRequest(device.pendingAuthRequest)">
bitLink
href="#"
appStopClick
(click)="approveOrDenyAuthRequest(device.pendingAuthRequest)"
>
{{ device.displayName }} {{ device.displayName }}
</a> </a>
<div class="tw-text-sm tw-text-muted"> <br />
{{ "needsApproval" | i18n }}
</div>
} @else { } @else {
<span>{{ device.displayName }}</span> <span>{{ device.displayName }}</span>
<div *ngIf="device.isTrusted" class="tw-text-sm tw-text-muted"> <div *ngIf="device.isTrusted" class="tw-text-sm tw-text-muted">

View File

@@ -1,6 +1,5 @@
import { CommonModule } from "@angular/common"; import { CommonModule } from "@angular/common";
import { Component, Input, OnChanges, SimpleChanges } from "@angular/core"; import { Component, EventEmitter, Input, OnChanges, Output, SimpleChanges } from "@angular/core";
import { firstValueFrom } from "rxjs";
import { JslibModule } from "@bitwarden/angular/jslib.module"; import { JslibModule } from "@bitwarden/angular/jslib.module";
import { DevicePendingAuthRequest } from "@bitwarden/common/auth/abstractions/devices/responses/device.response"; import { DevicePendingAuthRequest } from "@bitwarden/common/auth/abstractions/devices/responses/device.response";
@@ -8,16 +7,12 @@ import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.servic
import { import {
BadgeModule, BadgeModule,
ButtonModule, ButtonModule,
DialogService,
LinkModule, LinkModule,
TableDataSource, TableDataSource,
TableModule, TableModule,
} from "@bitwarden/components"; } from "@bitwarden/components";
import { LoginApprovalDialogComponent } from "../login-approval/login-approval-dialog.component";
import { DeviceDisplayData } from "./device-management.component"; import { DeviceDisplayData } from "./device-management.component";
import { clearAuthRequestAndResortDevices } from "./resort-devices.helper";
/** Displays user devices in a sortable table view */ /** Displays user devices in a sortable table view */
@Component({ @Component({
@@ -28,6 +23,8 @@ import { clearAuthRequestAndResortDevices } from "./resort-devices.helper";
}) })
export class DeviceManagementTableComponent implements OnChanges { export class DeviceManagementTableComponent implements OnChanges {
@Input() devices: DeviceDisplayData[] = []; @Input() devices: DeviceDisplayData[] = [];
@Output() onAuthRequestAnswered = new EventEmitter<DevicePendingAuthRequest>();
protected tableDataSource = new TableDataSource<DeviceDisplayData>(); protected tableDataSource = new TableDataSource<DeviceDisplayData>();
protected readonly columnConfig = [ protected readonly columnConfig = [
@@ -51,10 +48,7 @@ export class DeviceManagementTableComponent implements OnChanges {
}, },
]; ];
constructor( constructor(private i18nService: I18nService) {}
private i18nService: I18nService,
private dialogService: DialogService,
) {}
ngOnChanges(changes: SimpleChanges): void { ngOnChanges(changes: SimpleChanges): void {
if (changes.devices) { if (changes.devices) {
@@ -62,24 +56,10 @@ export class DeviceManagementTableComponent implements OnChanges {
} }
} }
protected async approveOrDenyAuthRequest(pendingAuthRequest: DevicePendingAuthRequest | null) { protected answerAuthRequest(pendingAuthRequest: DevicePendingAuthRequest | null) {
if (pendingAuthRequest == null) { if (pendingAuthRequest == null) {
return; return;
} }
this.onAuthRequestAnswered.emit(pendingAuthRequest);
const loginApprovalDialog = LoginApprovalDialogComponent.open(this.dialogService, {
notificationId: pendingAuthRequest.id,
});
const result = await firstValueFrom(loginApprovalDialog.closed);
if (result !== undefined && typeof result === "boolean") {
// Auth request was approved or denied, so clear the
// pending auth request and re-sort the device array
this.tableDataSource.data = clearAuthRequestAndResortDevices(
this.devices,
pendingAuthRequest,
);
}
} }
} }

View File

@@ -30,11 +30,13 @@
<auth-device-management-table <auth-device-management-table
ngClass="tw-hidden md:tw-block" ngClass="tw-hidden md:tw-block"
[devices]="devices" [devices]="devices"
(onAuthRequestAnswered)="handleAuthRequestAnswered($event)"
></auth-device-management-table> ></auth-device-management-table>
<!-- List View: displays on small screens --> <!-- List View: displays on small screens -->
<auth-device-management-item-group <auth-device-management-item-group
ngClass="md:tw-hidden" ngClass="md:tw-hidden"
[devices]="devices" [devices]="devices"
(onAuthRequestAnswered)="handleAuthRequestAnswered($event)"
></auth-device-management-item-group> ></auth-device-management-item-group>
} }

View File

@@ -16,14 +16,18 @@ import { DeviceType, DeviceTypeMetadata } from "@bitwarden/common/enums";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { ValidationService } from "@bitwarden/common/platform/abstractions/validation.service"; import { ValidationService } from "@bitwarden/common/platform/abstractions/validation.service";
import { MessageListener } from "@bitwarden/common/platform/messaging"; import { MessageListener } from "@bitwarden/common/platform/messaging";
import { ButtonModule, PopoverModule } from "@bitwarden/components"; import { ButtonModule, DialogService, PopoverModule } from "@bitwarden/components";
import { I18nPipe } from "@bitwarden/ui-common"; import { I18nPipe } from "@bitwarden/ui-common";
import { LoginApprovalDialogComponent } from "../login-approval";
import { DeviceManagementComponentServiceAbstraction } from "./device-management-component.service.abstraction"; import { DeviceManagementComponentServiceAbstraction } from "./device-management-component.service.abstraction";
import { DeviceManagementItemGroupComponent } from "./device-management-item-group.component"; import { DeviceManagementItemGroupComponent } from "./device-management-item-group.component";
import { DeviceManagementTableComponent } from "./device-management-table.component"; import { DeviceManagementTableComponent } from "./device-management-table.component";
import { clearAuthRequestAndResortDevices, resortDevices } from "./resort-devices.helper";
export interface DeviceDisplayData { export interface DeviceDisplayData {
creationDate: string;
displayName: string; displayName: string;
firstLogin: Date; firstLogin: Date;
icon: string; icon: string;
@@ -66,6 +70,7 @@ export class DeviceManagementComponent implements OnInit {
private destroyRef: DestroyRef, private destroyRef: DestroyRef,
private deviceManagementComponentService: DeviceManagementComponentServiceAbstraction, private deviceManagementComponentService: DeviceManagementComponentServiceAbstraction,
private devicesService: DevicesServiceAbstraction, private devicesService: DevicesServiceAbstraction,
private dialogService: DialogService,
private i18nService: I18nService, private i18nService: I18nService,
private messageListener: MessageListener, private messageListener: MessageListener,
private validationService: ValidationService, private validationService: ValidationService,
@@ -130,6 +135,7 @@ export class DeviceManagementComponent implements OnInit {
} }
return { return {
creationDate: device.creationDate,
displayName: this.devicesService.getReadableDeviceTypeName(device.type), displayName: this.devicesService.getReadableDeviceTypeName(device.type),
firstLogin: device.creationDate ? new Date(device.creationDate) : new Date(), firstLogin: device.creationDate ? new Date(device.creationDate) : new Date(),
icon: this.getDeviceIcon(device.type), icon: this.getDeviceIcon(device.type),
@@ -141,7 +147,8 @@ export class DeviceManagementComponent implements OnInit {
pendingAuthRequest: device.response?.devicePendingAuthRequest ?? null, pendingAuthRequest: device.response?.devicePendingAuthRequest ?? null,
}; };
}) })
.filter((device) => device !== null); .filter((device) => device !== null)
.sort(resortDevices);
} }
private async upsertDeviceWithPendingAuthRequest(authRequestId: string) { private async upsertDeviceWithPendingAuthRequest(authRequestId: string) {
@@ -151,6 +158,7 @@ export class DeviceManagementComponent implements OnInit {
} }
const upsertDevice: DeviceDisplayData = { const upsertDevice: DeviceDisplayData = {
creationDate: "",
displayName: this.devicesService.getReadableDeviceTypeName( displayName: this.devicesService.getReadableDeviceTypeName(
authRequestResponse.requestDeviceTypeValue, authRequestResponse.requestDeviceTypeValue,
), ),
@@ -174,8 +182,9 @@ export class DeviceManagementComponent implements OnInit {
); );
if (existingDevice?.id && existingDevice.creationDate) { if (existingDevice?.id && existingDevice.creationDate) {
upsertDevice.id = existingDevice.id; upsertDevice.creationDate = existingDevice.creationDate;
upsertDevice.firstLogin = new Date(existingDevice.creationDate); upsertDevice.firstLogin = new Date(existingDevice.creationDate);
upsertDevice.id = existingDevice.id;
} }
} }
@@ -186,10 +195,10 @@ export class DeviceManagementComponent implements OnInit {
if (existingDeviceIndex >= 0) { if (existingDeviceIndex >= 0) {
// Update existing device in device list // Update existing device in device list
this.devices[existingDeviceIndex] = upsertDevice; this.devices[existingDeviceIndex] = upsertDevice;
this.devices = [...this.devices]; this.devices = [...this.devices].sort(resortDevices);
} else { } else {
// Add new device to device list // Add new device to device list
this.devices = [upsertDevice, ...this.devices]; this.devices = [upsertDevice, ...this.devices].sort(resortDevices);
} }
} }
@@ -227,4 +236,18 @@ export class DeviceManagementComponent implements OnInit {
const metadata = DeviceTypeMetadata[type]; const metadata = DeviceTypeMetadata[type];
return metadata ? (categoryIconMap[metadata.category] ?? defaultIcon) : defaultIcon; return metadata ? (categoryIconMap[metadata.category] ?? defaultIcon) : defaultIcon;
} }
protected async handleAuthRequestAnswered(pendingAuthRequest: DevicePendingAuthRequest) {
const loginApprovalDialog = LoginApprovalDialogComponent.open(this.dialogService, {
notificationId: pendingAuthRequest.id,
});
const result = await firstValueFrom(loginApprovalDialog.closed);
if (result !== undefined && typeof result === "boolean") {
// Auth request was approved or denied, so clear the
// pending auth request and re-sort the device array
this.devices = clearAuthRequestAndResortDevices(this.devices, pendingAuthRequest);
}
}
} }

View File

@@ -23,7 +23,7 @@ export function clearAuthRequestAndResortDevices(
* *
* This is a helper function that gets passed to the `Array.sort()` method * This is a helper function that gets passed to the `Array.sort()` method
*/ */
function resortDevices(deviceA: DeviceDisplayData, deviceB: DeviceDisplayData) { export function resortDevices(deviceA: DeviceDisplayData, deviceB: DeviceDisplayData) {
// Devices with a pending auth request should be first // Devices with a pending auth request should be first
if (deviceA.pendingAuthRequest) { if (deviceA.pendingAuthRequest) {
return -1; return -1;
@@ -40,11 +40,11 @@ function resortDevices(deviceA: DeviceDisplayData, deviceB: DeviceDisplayData) {
return 1; return 1;
} }
// Then sort the rest by display name (alphabetically) // Then sort the rest by creation date (newest to oldest)
if (deviceA.displayName < deviceB.displayName) { if (deviceA.creationDate > deviceB.creationDate) {
return -1; return -1;
} }
if (deviceA.displayName > deviceB.displayName) { if (deviceA.creationDate < deviceB.creationDate) {
return 1; return 1;
} }