1
0
mirror of https://github.com/bitwarden/browser synced 2026-02-05 11:13:44 +00:00

WIP: adding errors and warnings to UI

This commit is contained in:
John Harrington
2025-08-19 06:05:28 -07:00
parent 6afebe2553
commit a4e88cf2c7
4 changed files with 305 additions and 19 deletions

View File

@@ -11,10 +11,34 @@
{{ callout.message }}
</bit-callout>
<bit-callout
*ngIf="warningsCallout"
[type]="warningsCallout.type"
[title]="warningsCallout.title"
>
<div class="tw-flex tw-items-start tw-justify-between tw-gap-2">
<div class="tw-flex-1">{{ warningsCallout.message }}</div>
<button
*ngIf="warningsCallout.detailsButtonText"
type="button"
id="de-duplicate_button_view-warnings-details"
bitButton
buttonType="secondary"
(click)="openWarningsDetails()"
>
{{ warningsCallout.detailsButtonText }}
</button>
</div>
</bit-callout>
<div class="tw-flex tw-flex-col tw-gap-4">
<bit-form-field class="tw-max-w-[28rem]">
<bit-label>{{ "uriMatchingStrategy" | i18n }}</bit-label>
<bit-select [items]="strategyOptions" [(ngModel)]="selectedStrategy"></bit-select>
<bit-select
id="de-duplicate_select_uri-matching-strategy"
[items]="strategyOptions"
[(ngModel)]="selectedStrategy"
></bit-select>
<bit-hint>
{{ "uriMatchingStrategyHelp" | i18n }}
<a

View File

@@ -12,12 +12,17 @@ import {
FormFieldModule,
Option,
SelectModule,
DialogService,
} from "@bitwarden/components";
import { I18nPipe } from "@bitwarden/ui-common";
import { HeaderModule } from "../../layouts/header/header.module";
import { SharedModule } from "../../shared";
import { DeDuplicateService, UriMatchStrategy } from "../../vault/services/de-duplicate.service";
import {
DeDuplicateService,
UriMatchStrategy,
type DuplicateOperationWarnings,
} from "../../vault/services/de-duplicate.service";
@Component({
selector: "app-de-duplicate",
@@ -38,6 +43,14 @@ import { DeDuplicateService, UriMatchStrategy } from "../../vault/services/de-du
export class DeDuplicateComponent {
loading = false;
callout: { type: "success" | "warning"; title?: string; message: string } | null = null;
warningsCallout: {
type: "warning";
title?: string;
message: string;
detailsButtonText?: string;
details?: string[];
} | null = null;
private warnings: DuplicateOperationWarnings | null = null;
selectedStrategy: UriMatchStrategy = "Base";
strategyOptions: Option<UriMatchStrategy>[] = [
@@ -52,10 +65,12 @@ export class DeDuplicateComponent {
private i18nService: I18nService,
private accountService: AccountService,
private cdr: ChangeDetectorRef,
private dialogService: DialogService,
) {}
findDuplicates = async () => {
this.callout = null;
this.warningsCallout = null;
// Allow progress spinner to appear on button
await new Promise<void>((r) => setTimeout(r, 100));
@@ -84,6 +99,45 @@ export class DeDuplicateComponent {
message: parts.join(", ") + ".",
};
}
// Summarize warnings if any
const w = result.warnings;
if (w && (w.exactFallbackCount || w.unparseableUriCount || w.permissionDeniedCount)) {
const warningParts: string[] = [];
if (w.exactFallbackCount) {
const key =
w.exactFallbackCount === 1 ? "duplicateExactFallback" : "duplicateExactFallbackPlural";
warningParts.push(this.i18nService.t(key, w.exactFallbackCount));
}
if (w.unparseableUriCount) {
const key =
w.unparseableUriCount === 1
? "duplicateUnparseableUris"
: "duplicateUnparseableUrisPlural";
warningParts.push(this.i18nService.t(key, w.unparseableUriCount));
}
if (w.permissionDeniedCount) {
const key =
w.permissionDeniedCount === 1
? "duplicatePermissionDenied"
: "duplicatePermissionDeniedPlural";
warningParts.push(this.i18nService.t(key, w.permissionDeniedCount));
}
const title = this.i18nService.t("duplicateWarningsTitle");
this.warnings = w;
this.warningsCallout = {
type: "warning",
title,
message: warningParts.join(" \u2022 "),
detailsButtonText:
(w.unparseableUris?.length || 0) > 0 ||
(w.exactFallbackSamples?.length || 0) > 0 ||
(w.permissionDeniedNames?.length || 0) > 0
? this.i18nService.t("duplicateWarningsDetailsButton")
: undefined,
details: w.unparseableUris,
};
}
} catch (e) {
const message = this.i18nService.t("duplicateError");
this.callout = {
@@ -95,4 +149,32 @@ export class DeDuplicateComponent {
this.cdr.markForCheck?.();
}
};
async openWarningsDetails() {
const w = this.warnings;
const bodyLines: string[] = [];
if (w?.unparseableUris?.length) {
bodyLines.push(this.i18nService.t("duplicateWarningsDialogUnparseableTitle"));
bodyLines.push(...w.unparseableUris.slice(0, 10).map((d) => `${d}`));
bodyLines.push("");
}
if (w?.exactFallbackSamples?.length) {
bodyLines.push(this.i18nService.t("duplicateWarningsDialogExactSamplesTitle"));
bodyLines.push(...w.exactFallbackSamples.slice(0, 10).map((d) => `${d}`));
bodyLines.push("");
}
if (w?.permissionDeniedNames?.length) {
bodyLines.push(this.i18nService.t("duplicateWarningsDialogPermissionDeniedTitle"));
bodyLines.push(...w.permissionDeniedNames.slice(0, 10).map((n) => `${n}`));
bodyLines.push("");
}
bodyLines.push(this.i18nService.t("duplicateWarningsDialogExactHelp"));
bodyLines.push(this.i18nService.t("duplicateWarningsDialogPermHelp"));
await this.dialogService.openSimpleDialog({
title: this.i18nService.t("duplicateWarningsDialogTitle"),
content: bodyLines.join("\n"),
type: "info",
acceptButtonText: { key: "ok" },
});
}
}

View File

@@ -17,6 +17,16 @@ export interface DuplicateOperationResult {
setsFound: number;
trashed: number;
permanentlyDeleted: number;
warnings?: DuplicateOperationWarnings;
}
export interface DuplicateOperationWarnings {
exactFallbackCount: number;
unparseableUriCount: number;
permissionDeniedCount: number;
unparseableUris?: string[];
exactFallbackSamples?: string[];
permissionDeniedNames?: string[];
}
interface DuplicateSet {
@@ -35,6 +45,10 @@ interface ParsedUri {
path?: string;
query?: string;
fragment?: string;
/** True when URL parsing fell back to a best-effort/manual parse (not fully exact). */
approximate?: boolean;
/** True when the input could not be parsed into a host/authority at all. */
unparseable?: boolean;
}
@Injectable({
@@ -68,13 +82,28 @@ export class DeDuplicateService {
): Promise<DuplicateOperationResult> {
const uriStrategy = options?.uriStrategy ?? DeDuplicateService.DEFAULT_URI_STRATEGY;
const allCiphers = await this.cipherService.getAllDecrypted(userId);
const duplicateSets = this.findDuplicateSets(allCiphers, uriStrategy);
const warningAccumulator = this.createWarningAccumulator();
const duplicateSets = this.findDuplicateSets(allCiphers, uriStrategy, warningAccumulator);
if (duplicateSets.length > 0) {
const { trashed, permanentlyDeleted } = await this.handleDuplicates(duplicateSets, userId);
return { setsFound: duplicateSets.length, trashed, permanentlyDeleted };
const { trashed, permanentlyDeleted } = await this.handleDuplicates(
duplicateSets,
userId,
warningAccumulator,
);
return {
setsFound: duplicateSets.length,
trashed,
permanentlyDeleted,
warnings: this.toWarningsResult(warningAccumulator),
};
}
return { setsFound: 0, trashed: 0, permanentlyDeleted: 0 };
return {
setsFound: 0,
trashed: 0,
permanentlyDeleted: 0,
warnings: this.toWarningsResult(warningAccumulator),
};
}
/**
@@ -90,6 +119,7 @@ export class DeDuplicateService {
private findDuplicateSets(
ciphers: CipherView[],
uriStrategy: UriMatchStrategy = DeDuplicateService.DEFAULT_URI_STRATEGY,
warningAccumulator?: WarningAccumulator,
): DuplicateSet[] {
const uriBuckets = new Map<string, CipherView[]>();
const nameBuckets = new Map<string, CipherView[]>();
@@ -127,7 +157,7 @@ export class DeDuplicateService {
const uris = this.extractUriStrings(cipher);
if (uris.length > 0) {
// Collect unique normalized keys to avoid adding the same cipher twice to the same bucket
const keys = this.getUriKeysForStrategy(uris, uriStrategy);
const keys = this.getUriKeysForStrategy(uris, uriStrategy, warningAccumulator);
for (const k of keys) {
const key = `${username}||${k}`;
let bucket = uriBuckets.get(key);
@@ -239,13 +269,25 @@ export class DeDuplicateService {
/**
* Get normalized keys for the given URIs according to a strategy.
*/
private getUriKeysForStrategy(uris: string[], strategy: UriMatchStrategy): Set<string> {
private getUriKeysForStrategy(
uris: string[],
strategy: UriMatchStrategy,
warningAccumulator?: WarningAccumulator,
): Set<string> {
const keys = new Set<string>();
for (const raw of uris) {
const parsed = this.parseUri(raw);
if (!parsed) {
continue;
}
// Count unparseable URIs and skip them
if (parsed.unparseable) {
if (warningAccumulator) {
warningAccumulator.unparseableUriCount++;
warningAccumulator.unparseableUris.push(parsed.original);
}
continue;
}
switch (strategy) {
case "Base": {
const base = this.getBaseUri(parsed);
@@ -269,6 +311,12 @@ export class DeDuplicateService {
break;
}
case "Exact": {
if (parsed.approximate && warningAccumulator) {
warningAccumulator.exactFallbackCount++;
if (warningAccumulator.exactFallbackSamples.length < 10) {
warningAccumulator.exactFallbackSamples.push(parsed.original);
}
}
const exact = this.getExactUrlKey(parsed);
if (exact) {
keys.add(exact);
@@ -334,12 +382,13 @@ export class DeDuplicateService {
fragment,
};
} catch {
// Fallback manual authority extraction sufficient for host, hostname, and base strategies
// TODO if exact strategy is being used, the user should be notified URI matching won't be exact
// Fallback manual authority extraction sufficient for Hostname/Base/Host strategies.
// We mark the result as `approximate`; getUriKeysForStrategy counts a fallback warning
// only when the selected strategy is "Exact". Other strategies continue normally.
const authorityMatch = toParse.match(/^[a-z][a-z0-9+.-]*:\/\/([^/?#]+)/i);
if (!authorityMatch) {
// TODO the user should be informed that this URI couldn't be matched
return { original: input };
// Mark as unparseable so the caller can report and skip safely
return { original: input, unparseable: true };
}
let authority = authorityMatch[1];
const atIndex = authority.lastIndexOf("@");
@@ -375,7 +424,7 @@ export class DeDuplicateService {
}
}
host = host.replace(/\.$/, "").toLowerCase();
return { original: input, hostname: host, port };
return { original: input, hostname: host, port, approximate: true };
}
}
@@ -465,6 +514,7 @@ export class DeDuplicateService {
private async handleDuplicates(
duplicateSets: DuplicateSet[],
userId: UserId,
warningAccumulator?: WarningAccumulator,
): Promise<{ trashed: number; permanentlyDeleted: number }> {
// 1. Open the dialog to let the user review and select duplicates to delete.
const dialogRef = DuplicateReviewDialogComponent.open(this.dialogService, {
@@ -488,20 +538,40 @@ export class DeDuplicateService {
}
// 3. Filter the user's selected deletions based on their permissions.
// TODO: Determine why a user may not have permission to delete a cipher and explore ways of notifying them of this situation
// When users lack permission to delete an item (e.g., org-owned or restricted by collection permissions),
// record this so the UI can notify them.
const permissionChecks = uniqueDeleteIds.map(async (id) => {
const cipher = cipherIndex.get(id);
if (!cipher) {
return null;
return { cipher: null as CipherView | null, canDelete: false };
}
const canDelete = await firstValueFrom(
this.cipherAuthorizationService.canDeleteCipher$(cipher),
);
return canDelete ? cipher : null;
return { cipher, canDelete };
});
const permitted = (await Promise.all(permissionChecks)).filter(
(c): c is CipherView => c != null,
);
const permissionResults = await Promise.all(permissionChecks);
const permitted: CipherView[] = [];
const denied: CipherView[] = [];
for (const r of permissionResults) {
if (!r.cipher) {
continue;
}
if (r.canDelete) {
permitted.push(r.cipher);
} else {
denied.push(r.cipher);
}
}
if (warningAccumulator && denied.length > 0) {
warningAccumulator.permissionDeniedCount += denied.length;
for (const d of denied) {
const name = d.name?.trim();
if (name && warningAccumulator.permissionDeniedNames.length < 10) {
warningAccumulator.permissionDeniedNames.push(name);
}
}
}
if (permitted.length === 0) {
return { trashed: 0, permanentlyDeleted: 0 };
}
@@ -545,4 +615,40 @@ export class DeDuplicateService {
// 6. Return summary to display in a callout by the caller.
return { trashed: toSoftDelete.length, permanentlyDeleted: toPermanentlyDelete.length };
}
// ------------------------
// Warnings accumulator helpers
// ------------------------
private createWarningAccumulator(): WarningAccumulator {
return {
exactFallbackCount: 0,
unparseableUriCount: 0,
permissionDeniedCount: 0,
unparseableUris: [],
exactFallbackSamples: [],
permissionDeniedNames: [],
};
}
private toWarningsResult(acc: WarningAccumulator): DuplicateOperationWarnings {
return {
exactFallbackCount: acc.exactFallbackCount,
unparseableUriCount: acc.unparseableUriCount,
permissionDeniedCount: acc.permissionDeniedCount,
unparseableUris: acc.unparseableUris.length ? acc.unparseableUris : undefined,
exactFallbackSamples: acc.exactFallbackSamples.length ? acc.exactFallbackSamples : undefined,
permissionDeniedNames: acc.permissionDeniedNames.length
? acc.permissionDeniedNames
: undefined,
};
}
}
interface WarningAccumulator {
exactFallbackCount: number;
unparseableUriCount: number;
permissionDeniedCount: number;
unparseableUris: string[];
exactFallbackSamples: string[];
permissionDeniedNames: string[];
}

View File

@@ -11088,5 +11088,79 @@
"noValue": {
"message": "-",
"description": "Indicates that no value is present for a given item. First implemented when displaying a list of ciphers that may not belong to folders or organizations."
},
"duplicateWarningsTitle": {
"message": "Some items were skipped or handled differently",
"description": "Title for a warning callout summarizing non-fatal issues encountered during de-duplication."
},
"duplicateExactFallback": {
"message": "Exact matching failed for $COUNT$ URI.",
"placeholders": {
"count": { "content": "$1", "example": "3" }
},
"description": "Warning indicating that due to parsing limitations, exact URI matching could not be guaranteed for some items."
},
"duplicateExactFallbackPlural": {
"message": "Exact matching for for $COUNT$ URIs.",
"placeholders": {
"count": { "content": "$1", "example": "3" }
},
"description": "Plural form of duplicateExactFallback."
},
"duplicateUnparseableUris": {
"message": "$COUNT$ URI could not be parsed and was ignored.",
"placeholders": {
"count": { "content": "$1", "example": "5" }
},
"description": "Warning indicating that some URIs could not be parsed and were excluded from duplicate detection."
},
"duplicateUnparseableUrisPlural": {
"message": "$COUNT$ URIs could not be parsed and were ignored.",
"placeholders": {
"count": { "content": "$1", "example": "5" }
},
"description": "Plural form of duplicateUnparseableUris."
},
"duplicatePermissionDenied": {
"message": "$COUNT$ selected item could not be deleted due to permissions.",
"placeholders": {
"count": { "content": "$1", "example": "2" }
},
"description": "Warning indicating that the user lacked permission to delete some of the selected duplicate items."
},
"duplicatePermissionDeniedPlural": {
"message": "$COUNT$ selected items could not be deleted due to permissions.",
"placeholders": {
"count": { "content": "$1", "example": "2" }
},
"description": "Plural form of duplicatePermissionDenied."
},
"duplicateWarningsDialogTitle": {
"message": "De-duplication warnings",
"description": "Title of the dialog showing details for de-duplication warnings."
},
"duplicateWarningsDialogExactHelp": {
"message": "Fix invalid URIs or try a less strict match strategy (Host/Hostname/Base).",
"description": "Helper sentence shown in the warnings details dialog to guide users about URI-related issues."
},
"duplicateWarningsDialogPermHelp": {
"message": "Some items may be organization-owned or read-only. Contact an admin or adjust collection permissions.",
"description": "Helper sentence shown in the warnings details dialog to explain permission-denied deletions."
},
"duplicateWarningsDetailsButton": {
"message": "View details",
"description": "Label for a button that opens a dialog with more information about de-duplication warnings."
},
"duplicateWarningsDialogUnparseableTitle": {
"message": "Unparseable URIs",
"description": "Section title above the list of unparseable URIs."
},
"duplicateWarningsDialogExactSamplesTitle": {
"message": "Exact matching fallbacks (sample)",
"description": "Section title above the sample of URIs where exact matching could not be guaranteed."
},
"duplicateWarningsDialogPermissionDeniedTitle": {
"message": "Items not deleted (permission denied)",
"description": "Section title above the list of cipher names that could not be deleted due to permissions."
}
}