Skip to content
This repository was archived by the owner on Jun 17, 2022. It is now read-only.

Fixes and improvements to MykiCsvImporter #707

Merged
merged 8 commits into from
Mar 7, 2022
Merged

Fixes and improvements to MykiCsvImporter #707

merged 8 commits into from
Mar 7, 2022

Conversation

@djsmith85
Copy link
Contributor

@djsmith85 djsmith85 commented Mar 6, 2022 •
edited
Loading

Type of change

  • Bug fix
  • New feature development
  • Tech debt (refactoring, code cleanup, dependency upgrades, etc)
  • Build/deploy pipeline (DevOps)
  • Other

Objective

With the announcement of Myki discontinuing their existing products on April 10th, 2022, after being aquired by JumpCloud, it was time to revisit our Myki importer.

This PR fixes some minor issues and adds support for previously not imported exports types (UserIdCard.csv and UserTwoFA.csv).

Asana task: https://app.asana.com/0/1200578627114106/1201930330138516/f

Rough overview of imported ciphers/fields:

  • Empty fields are skipped

  • All unmapped columns are imported as custom fields (for example: status and tags)

  • Login items - UserAccount.csv

    • Username
    • Password
    • TOTP
    • Notes

  • TwoFA - UserTwoFa.csv

    • nickname -> CipherTitle
    • TOTP
    • Notes

  • CreditCard items - UserCreditCard.csv

    • Cardholder
    • CC Number
    • CVV
    • Expiry date
    • Card brand is set using CC Number and only to our supported brands
    • Notes

  • Secure Notes- UserNote.csv

    • Title
    • Content

  • Identity items - UserIdentity.csv

    • Firstname
    • MiddleName
    • LastName
    • Email
    • Address
    • Notes

  • IdCard items - UserIdCard.csv

    • All idCards are converted to Bitwarden identities.
    • FirstName
    • MiddleName
    • LastName
    • For Passport and Social Security the respective fields are used. All other types fill the LicenseNumber field
    • Notes

Code changes

  • common/src/importers/mykiCsvImporter.ts:
    • Fixed 2fa seed not being copied on account records
    • Fixedx secure note title not being set
    • Added support for importing userIdCard.csv and userTwoFa.csv
    • Add importing of all unmapped columns into custom fields
  • spec/common/importers/mykiCsvImporter.spec.ts: added test suite
  • spec/common/importers/testData/mykiCsv/*: Example exports used for testing

Testing requirements

Ensure that items are imported as described

Before you submit

  • I have checked for linting errors (npm run lint) (required)
  • I have added unit tests where it makes sense to do so (encouraged but not required)
  • This change requires a documentation update (notify the documentation team)
  • This change has particular deployment requirements (notify the DevOps team)

@djsmith85 djsmith85 requested a review from a team March 7, 2022 18:20
@djsmith85 djsmith85 marked this pull request as ready for review March 7, 2022 18:21
@djsmith85 djsmith85 enabled auto-merge (squash) March 7, 2022 19:07
differsthecat
differsthecat reviewed Mar 7, 2022
View reviewed changes
common/src/importers/mykiCsvImporter.ts
Comment on lines +106 to +117
// case "Driver's License":
// case "ID Card":
// case "Outdoor License":
// case "Software License":
// case "Tax Number":
// case "Bank Account":
// case "Insurance Card":
// case "Health Card":
// case "Membership":
// case "Database":
// case "Reward Program":
// case "Tour Visa":
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing MyKi doesn't have these id types?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does have those idType but as there were no other good identity fields to map to, it uses the switch's default to map them to cipher.identity.licenseNumber

@djsmith85 djsmith85 merged commit fa3a95f into master Mar 7, 2022
differsthecat
differsthecat approved these changes Mar 7, 2022
View reviewed changes
Copy link
Member

@differsthecat differsthecat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left one question, but it looks good and I'm really digging all of the tests you've been adding!

@djsmith85 djsmith85 deleted the myki-improvements branch March 7, 2022 22:20
@djsmith85 djsmith85 mentioned this pull request Mar 7, 2022
Merged
8 tasks
This was referenced Mar 14, 2022
Merged
Merged
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@differsthecat differsthecat differsthecat approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@djsmith85 @differsthecat