Service to delete duplicate items

[harr1424]

✅ Code Contribution Proposal

Community identified issue

A community forum post from 2018 with considerable engagement describes a scenario where using the import tool with different sources can result in a large quantity of duplicates. Community members are requesting a tool that will allow for de-duplication.

👀 Jump to latest update

My experience

Please also see PR 15967 that demonstrates how this is working and current proposed code changes (the code shown on this discussion has changed).

First, I imported credentials into Bitwarden from my preferred browser. Next, I imported credentials from a secondary browser, which had nearly all of the same login items. This resulted in the size of my vault nearly doubling. Upon inspecting the duplicated credentials, I noticed:

The original credential had the following structure:

Item name: "${FULLY_QUALIFIED_DOMAIN_NAME}"
Username: "${USERNAME}'
Password: "${PASSWORD}" 

The duplicate credential (created when performing the second import) had the following structure:

Item name: "${FULLY_QUALIFIED_DOMAIN_NAME} (${USERNAME})"
Username: "${USERNAME}'
Password: "${PASSWORD}" 

Where username and password values are shared between the duplicates.

I have not yet identified how Bitwarden will handle duplicates beyond this level (n > 2) since it appears Item names must be unique. Before doing so, I wanted to share my initial approach at implementing a de-duplication service.

Initial strategy for removing duplicate credentials

My current approach relies on the username being appended to the fully qualified domain name in order to generate the Item name during import. This is not a robust solution, but will allow for initial testing in my specific situation.

git checkout -b feature/duplicate-deletion

Added apps/web/src/app/vault/services/duplicate-cipher.service.ts:

import { Injectable } from "@angular/core";

import { UserId } from "@bitwarden/common/types/guid";
import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
import { DialogService } from "@bitwarden/components";

@Injectable({
  providedIn: "root",
})
export class DuplicateCipherService {
  constructor(
    private cipherService: CipherService,
    private dialogService: DialogService,
  ) {}

  async findAndHandleDuplicates(userId: UserId) {
    const allCiphers = await this.cipherService.getAllDecrypted(userId);
    const duplicateSets = this.findDuplicateSets(allCiphers);

    if (duplicateSets.length > 0) {
      await this.handleDuplicates(duplicateSets, userId);
    }
  }

  private findDuplicateSets(ciphers: CipherView[]): CipherView[][] {
    // duplicateSets[0] corresponds to first identified duplicated CipherView, as identified by sharing a username and password (name will be used later)
    // duplicateSets[0][0] corresponds to the first CipherView of an identified duplicate
    const duplicateSets: CipherView[][] = [];

    const cipherMap = new Map<string, CipherView[]>();

    ciphers.forEach((cipher) => {
      if (!cipher.login) {return;}

      const { username, password } = cipher.login;
      if (!username || !password) {return;}

      const key = `${username}|${password}`;
      if (!cipherMap.has(key)) {
        cipherMap.set(key, []);
      }
      cipherMap.get(key).push(cipher);
    });

    cipherMap.forEach((group) => {
      if (group.length < 2) {return;}

      group.forEach((cipher1, i) => {
        group.slice(i + 1).forEach((cipher2) => {
          if (this.isDuplicateByName(cipher1, cipher2)) {
            if (!duplicateSets.some((set) => set.includes(cipher1) && set.includes(cipher2))) {
              duplicateSets.push([cipher1, cipher2]);
            }
          }
        });
      });
    });

    return duplicateSets;
  }

  private isDuplicateByName(cipher1: CipherView, cipher2: CipherView): boolean {
    const username = cipher1.login?.username || "";
    return (
      cipher1.name === cipher2.name ||
      // the pattern below represents how duplicated entries are imported in my scenario, may need to check other scenarios 
      cipher1.name === `${cipher2.name} (${username})` ||
      cipher2.name === `${cipher1.name} (${username})`
    );
  }

  // TODO add support for identifying duplicates by organizationId and possibly folderId

  // TODO the approach below will present a dialog for each duplicate, consider options for displaying a list in order to review and delete in batch 
  private async handleDuplicates(duplicateSets: CipherView[][], userId: UserId) {
    for (const set of duplicateSets) {
      const [cipher1, cipher2] = set;
      const confirmed = await this.dialogService.openSimpleDialog({
        title: "Duplicate Items Found",
        content: {
          key: "duplicateItemsFound",
          placeholders: [cipher1.name, cipher2.name],
        },
        type: "warning",
        acceptButtonText: "Delete Duplicate",
        cancelButtonText: "Keep Both",
      });

      if (confirmed) {
        const toDelete = cipher1.name.includes(cipher1.login?.username || "") ? cipher1 : cipher2;
        await this.cipherService.delete(toDelete.id, userId);
      }
    }
  }
}

[github-actions[bot]]

✨ Thank you for your code contribution proposal! While the Bitwarden team reviews your submission, we encourage you to check out our contribution guidelines.

Please ensure that your code contribution includes a detailed description of what you would like to contribute, along with any relevant screenshots and links to existing feature requests. This information helps us gather feedback from the community and Bitwarden team members before you start writing code.

To keep discussions focused, posts that do not include a proposal for a code contribution will be removed.

• 💡 For feature requests and general discussion, please visit the Bitwarden Community Forums.
• ℹ️ For questions and support, visit the Bitwarden Help Center.
• 🐛 To report a potential bug, please visit the corresponding repo. Server | Clients | iOS | Android

Thank you for contributing to Bitwarden!

[harr1424]

Please see PR 15967 opened for this feature integration.

[bwbug]

My current approach relies on the username being appended to the fully qualified domain name in order to generate the Item name during import.

FYI, Bitwarden does not alter the Name field when importing a duplicate. In your case, the username must have been appended by the original password manager. If an item with the same Name as an existing item is imported, the vault will contain duplicated names:

 

Also, in your PR, I don't understand why you are using folder and collection as criteria for identifying duplicates. IMO, matching should be done based on URI and username.

 

---

P.S. I have no affiliation with Bitwarden, so feel free to ignore my comment.

[harr1424]

Hi, thanks for your feedback.

My apologies for not updating this discussion, but as I was working on my PR I realized what you have pointed out regarding how usernames are interpolated into the name field. This is not performed by Bitwarden, and name fields do not need to have unique values. In my case, this was something that Safari did when exporting my login credentials.

The situation shown in your screenshot will be correctly handled by my current service, this is something that I added test coverage for. However, I did not consider using URI as matching criteria. This would be much more robust than relying on the Name field corresponding to the URI. Interestingly, it appears that login CipherTypes store URIs in an array, meaning that they could have more than one, but I don't see how the UI would allow a user to add more than one URI for a login item.

Regarding the inclusion of folder and organization IDs as matching criteria, this is something I thought would be helpful, but is easy to remove outright or else make optional.

Thanks for your suggestion on matching criteria, I agree that URI and username would be sufficient here, and would eliminate any need to normalize the name field. I'll start working on this change.

[bwbug]

but I don't see how the UI would allow a user to add more than one URI for a login item.

This is pretty common in Bitwarden, using the "Add Website" function (or the "Fill and Save" function in the browser extensions and Android app):

[audreyality]

1. We have UX engineers and product folks looking into how the duplicate finder can be improved.
2. Note that if you include uri matching, you should also be aware of the matching rules the login uses.
3. Also note that bwbug is a member of the Bitwarden community, not our staff.

[harr1424]

[PM-24630] feature: de-duplication tool with test coverage PR #15967

Duplicate Detection Logic

Currently only login ciphers are fully supported. Two or more ciphers are considered duplicate if any of the following are true:

• identical login.username and one or more matched normalized login.uri values¹
• identical login.username and matched normalized cipher.name values²
• null login.username and matched normalized cipher.name values (this situation applies to all cipher types, not only logins)

Important clarification and consideration

• For each login cipher, potential duplicates will determined by combining the login username with every URI associated with that cipher. This means that if a cipher has three URIs, three username+uri combinations will be added to a duplicate "bucket." As the bucket grows, each addition to the bucket will be checked to determine if that username+uri combination is already in the bucket. If it does exist, that is a duplicate, and that cipher will be added to a list of duplicate ciphers.
• This has an important effect which may need to be considered, the ordering of duplicates in the duplicate review dialog is determined by the order in which the duplicates were encountered in the vault. This in turn determines which ciphers are automatically selected/suggested for deletion. The below screenshot shows that the cipher with the normalized URI is suggested for deletion, which may be undesirable.

Multiple shared URI example

Consider the following login ciphers:

"1_TEST": {"username": "tester", "uris": ["test.domain.org", "forum.test.domain.org", "forum.test.domain.org/login"] }

"2_TEST": {"username": "tester", "uris": ["test.domain.org"] }

"3_TEST": {"username": "tester", "uris": ["forum.test.domain.org"] }

The example above is instructive to better understand how URI normalization has been used in this feature: URIs are normalized such that only subdomain, domain, and top-level domain are retained ( see hostname ). So the above set of ciphers (5 URIs total, 3 of which are unique), only yeilds 2 normalized ciphers.

The screenshot below demonstrates that the duplicate review dialog presents "sets" of duplicate logins, which correspond to the buckets described earlier. Since the username+uri combinations resulting from URI normalization yielded 2 buckets, two sets will be presented to the user.

Because 3 ciphers were present in these 2 sets, the user is informed that 3 duplicates were found.

Tool Workflow

The initial use of this tool moves ciphers selected for deletion to the trash. From there they can be easily recovered. The user can decide to empty their vault trash, or else run the tool a second time to permanently delete the ciphers.

Initial page state:

Progress spinner inside button, similar to reporting tools:

If a user decides to cancel the operation:

The initial pass will detect duplicates (which in the example below are not currently in the trash, but could be):

First time duplicate deletion moves items to trash:

Duplicate Review Dialog (which in this example shows logins moved to trash in previous 'pass'):

Second 'pass' over duplicates will delete permanently:

If no duplicates are found (migrated from toast to callout):

Question and answer regarding the workflow

if you chose 1_TEST in the first bucket for deletion, then 1_TEST would be auto-selected/marked in your second bucket (and possible other buckets) as well?

Unselecting 1_TEST in one grouping will unselect it from all groupings, and vice-versa. If a cipher is selected for deletion or retention in any grouping, that choice will extend to all groupings.

Future Efforts

🌱 The Duplicate Review Dialog displays rows much wider than the dialog box itself, and so considerable horizontal scrolling is necessary
🌱 The Duplicate Review Dialog does not display consistent column widths
🌱 I wanted to explore formatting duplicate groupings as cards, and then placing these cards into a vertical scroll, as well as displaying more information regarding each login (TOTP status, notes, attachments)

Footnotes

1. URI normalization (yields only hostname:
   • Prepends "https://" if the string lacks a scheme so standard parsing works.
   • Uses npm's URL parser (new URL) yielding punycoded ASCII for IDNs.
   • Falls back to a lightweight regex parse if URL parsing fails (also yields hostname)
   • Strips userinfo, port, enclosing IPv6 brackets, and a trailing dot; lowercases result.
   • Returns "" if a host can't be derived. ↩

2. Name normalization involves removing all whitespace characters (internal & external) and lowercasing the result ↩

[harr1424]

Known Issue: Match logic in this feature is inconsistent with existing Bitwarden match logic Implemented and present in PR #15967

Bitwarden allows users to match URIs (in order to autofill form fields) using the following criteria, explained at the link above::

• Base: matches on second-level and top-level domain only
• Host: matches on host which is same as above but requires a match on subdomain and (when specified) port
• Starts With: matches when resource starts with the URI, regardless of what follows it.
• Regular Expression: See link, the possibilities for custom match criteria and mistakes are extensive
• Exact: exact correspondence between full URI and website visited in the browser

The current implementation of this feature does not directly correspond to any of these existing matching strategies. This feature will perform a base match but will add the additional match criteria of subdomain, effectively corresponding to a hostname match. However, hostname is not currently an option in the Bitwarden match hierarchy, which jumps from Base to Host.

I plan to work on a future revision of this service that allow the user to select from the following strategies in order to detect duplicates:

• Base
• Hostname (already implemented)
• Host
• Exact

The other match strategies are valid options for duplicate detection, but are more complex and will require more careful testing.

I would also like to contribute a hostname based match strategy to the existing autofill match options, so that these two areas of the application offer consistent choices.

Other potential issues related to URI matching Also implemented in PR #15967

• This service prepends URIs lacking a scheme with https, but existing logic will prepend URIs with http when they lack a scheme and are launched from within the app.
• This service will not currently work with the androidapp:// scheme, but support has already been added and I am testing it locally:

  // androidapp scheme used by Bitwarden to reference Android package IDs
  // Supports both forms: androidapp://com.package[...optional path] and androidapp:com.package
  // Normalizes to just the package id (lowercased) for matching.
  {
    const m1 = input.match(/^androidapp:\/\/([^\/?#]+)(?:[\/?#].*)?$/i);
    if (m1 && m1[1]) {
      return m1[1].trim().replace(/\.$/, "").toLowerCase();
    }
    const m2 = input.match(/^androidapp:([^\s\/?#]+).*$/i);
    if (m2 && m2[1]) {
      return m2[1].trim().replace(/\.$/, "").toLowerCase();
    }
  }

[harr1424]

Discussion on default URI matching for duplicate detection

Should default duplicate URI matching behavior differ from default autofill matching?

The comment above describes some changes I made to PR #15967 in order to integrate the same URI matching logic that is currently used in the web extension autofill functionality. Currently, I've set the default URI match to use Base matching, which is the same default used when matching URIs for autofill.

This may not be ideal, because the base match strategy is eager. Assuming no permissive "Starts with" or permissive regular expressions (both are discouraged by Bitwarden), the "Base" matching strategy is most likely to match generously because it doesn't consider subdomains, ports, paths, etc. and is only concerned with top and second-level domains.

In the context of autofilling credentials, this may lead to slightly annoying behavior if different credentials are used across subdomains. In the context of credential deletion, this may lead to unintended deletions, a destructive action. The image below demonstrates how the "Base" strategy identified duplicated logins which would not have been identified using "Hostname" or "Host," which may be more suitable defaults for this feature.

[audreyality]

@sukhleenb - We might want to consider using a heuristic that produces a confidence score. We could use the "base" comparison to get a provisional list, then pass it through the heuristics in order to score how likely we think a match is. Exact matches would be at or close to 100%, whereas having an "autofill-powered" match might weaken the score based on the match type. If we fall below some threshold (say, a p-value of 5%), then we can skip listing it in the report.

[harr1424]

Please also see bitwarden/sdk-internal#418