Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Multiple TOTP fields lost when importing 1Password #8371

Closed
chriswayg opened this issue Aug 16, 2022 · 2 comments · Fixed by #8436
Closed

Multiple TOTP fields lost when importing 1Password #8371

chriswayg opened this issue Aug 16, 2022 · 2 comments · Fixed by #8436

Comments

@chriswayg
Copy link

Overview

Successfully imported a 1Password opvault in KeePassXC. One of my accounts is a major Bitcoin Exchange which uses multiple different TOTPs for various functions: login, trading, funding.

Of the 3 One-Time Password entries in the Bitcoin Exchange account, KeePassXC imported only the last one and dropped the other two entries completely.

As a result I was unable to login to that Bitcoin Exchange. I had to return to a backup of my 1Password installation to recover the TOTP data deleted by KeePassXC.

Steps to Reproduce

  1. Import opvault as described in Migrating from 1Password to KeePass, KeePassXC and KeePassium | KeePassium and Importing 1Password OPVault
  2. Go to the imported item or account that contained multiple TOTP entries in 1Password
  3. Open 'Additional Attributes' and discover that only the last TOTP entry was imported and the other two have been dropped/deleted by KeePassXC

(I reproduced this behavior twice)

Expected Behavior

I would expect at least one of the following to happen:

  • Warning Message: at the minimum there should be a warning message after import such as: "WARNING: The following items could not be imported successfully, as they contain multiple One-Time Passwords which KeePassXC in unable to handle: Bitcoin Exchange."
  • Preserve Data: there should never be any deletion of data essential to access the site. If KeePassXC cannot properly handle multiple TOTP fields, then it should import the additional fields as regular text fields such as "Login TOTP:" "otpauth://totp/BitcoinExchange?secret=ABCDEFGHIJKL&period=30&digits=6&issuer=BitCoinExchange"
  • Multiple TOTP implementation: as a feature request, multiple TOTP fields could be implemented as otp, otp2, otp3 for example, and codes shown in the GUI on the General or Advanced tab.

Actual Behavior

only the last TOTP entry was imported and the other two have been dropped/deleted by KeePassXC

Context

KeePassXC - Version 2.7.1
Revision: 5916a8f
Operating System: macOS 12.5

@droidmonkey
Copy link
Member

Well this is an interesting edge case, shouldn't be too hard to fix. What I suspect is happening is that we are correctly identifying each field as TOTP and overwriting the one otp field in keepassxc. If so I will just use your suggestion to make an otp2, otp3, etc when importing from 1Password. We won't support multiple totp per entry (for now) so you'll have to copy those over to a new entry each.

@droidmonkey droidmonkey changed the title Loss of TOTP data when importing from 1Password.opvault - locked out of Bitcoin Exchange Multiple TOTP fields lost when importing 1Password Aug 16, 2022
@droidmonkey
Copy link
Member

Confirmed, relevant code starts here:

if (attrValue.startsWith("otpauth://")) {

droidmonkey added a commit that referenced this issue Sep 1, 2022
* Fix #8371 - store multiple OTP fields as `otp_#` instead of silently discarding them.
droidmonkey added a commit that referenced this issue Sep 3, 2022
* Fix #8371 - store multiple OTP fields as `otp_#` instead of silently discarding them.
droidmonkey added a commit that referenced this issue Sep 5, 2022
* Fix #8371 - store multiple OTP fields as `otp_#` instead of silently discarding them.
droidmonkey added a commit that referenced this issue Sep 7, 2022
* Fix #8371 - store multiple OTP fields as `otp_#` instead of silently discarding them.
pull bot pushed a commit to annihilatorrrr/keepassxc that referenced this issue Sep 7, 2022
* Fix keepassxreboot#8371 - store multiple OTP fields as `otp_#` instead of silently discarding them.
pull bot pushed a commit to tigerwill90/keepassxc that referenced this issue Sep 7, 2022
* Fix keepassxreboot#8371 - store multiple OTP fields as `otp_#` instead of silently discarding them.
droidmonkey added a commit that referenced this issue Sep 11, 2022
* Fix #8371 - store multiple OTP fields as `otp_#` instead of silently discarding them.
droidmonkey added a commit that referenced this issue Sep 22, 2022
* Fix #8371 - store multiple OTP fields as `otp_#` instead of silently discarding them.
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants