Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

1Password import changes username field, data loss (value dropped when username field is not username) #9577

Closed
foss- opened this issue Jun 24, 2023 · 10 comments · Fixed by #9815

Comments

@foss-
Copy link

foss- commented Jun 24, 2023

Overview

When importing from 1Password opvault file using KeePassXC 2.7.5 on macOS 13.4.1, username field has unexpected value in some cases.

Steps to Reproduce

Requirements for the problem to occur are:

  • login uses email as username (some platforms only allow email + password combination in their login form, looking at you, discord)
  • login has additional username field for the action username

Expected Behavior

After import, in such a case, it would be expected for the username to be the email address, which indeed resides in the username field in the 1Password login entry.

Actual Behavior

Unexpectedly the value from the top username field in 1P is dropped entirely resulting in data loss or missing data after import. In the example given, the username field of the imported entry shows the actual username and not the email address, which is used in top username field.

Context

It seems the fact that in this scenario, where a login does not allow the actual username, KeePassXC is unable to handle the two different fields which both use username as descriptive name.

One way to workaround this is to change the field name in 1Password holding the actual username to something different. That resolves the issue with having two values for a field called username, which seems to confuse KeePassXC.

However it would still be nice, if this was handled more gracefulyl without dropping information on import. Maybe using suffix numbers for fields that exist twice would be a better option, clearly creates overhead, not sure how best to solve this.

Edit: I just discovered, this is not 100 % reproducible. A similar 1P entry with same structure ended up with the email being indeed used as username and instead dropping the actual username value. So I am not sure what determines, which of the two field values is dropped.

@foss- foss- added the bug label Jun 24, 2023
@droidmonkey
Copy link
Member

The value is most likely in the advanced attributes of the entry. Please confirm.

@foss-
Copy link
Author

foss- commented Jun 24, 2023

I did check that and curiously in the case where the email is kept, the username value ended up in Advanced > Abschnitt_username. That is great.

However for the other case where username (instead of email) is used, email is indeed dropped. There is no username entry in advanced holding the email, which was, what made me file this issue as data loss / dropped value initially.

I am having a hard time understanding who the entries in 1P differ so that the import results differ. I checked Saved Form Details in 1P and in the case where the email was used, that value came from the web form and field name is user[email]. Then again for the case where the email saved as username in top section in 1P is dropped, the web form details show field name username and the email address as value. So not that much difference.

@droidmonkey
Copy link
Member

1Password is horrific to import, they have zero consistency in their field names and where values are actually located. This is likely another special case that will need to be accounted for. However, I really don't want to invest any more time in the opvault import, we will have the much better 1PUX importer integrated in the next release.

@foss-
Copy link
Author

foss- commented Jun 24, 2023

Sure, I am in the process of going through all entries and resolving this by renaming the username field to user. Feel free to close this, however the problem is easy to overlook and if then 1Password is removed the data may be hard to recover. I am glad I discovered this early enough to take care of the problem manually.

@foss-
Copy link
Author

foss- commented Jun 27, 2023

Since work is planned on improving 1PUX format and that format is exclusive to 1Password 8, this would leave 1Password 7 users with this issue. I can imaging many users looking to switch to KeePassXC are 1Password 7 users, which is the last version without subscription.

Would it make sense to show a disclaimer when importing opvault files about the limitation of the current import implementation to prevent data loss and raise awareness?

@droidmonkey
Copy link
Member

droidmonkey commented Jun 27, 2023

If you are able to provide a sample opvault that has this issue, then it can be fixed.

@foss-
Copy link
Author

foss- commented Jun 27, 2023

Created a test file with the following structure:

Login1 uses an email address in the 1Password v7 top section username field. Some websites make email a requirement and disallow username for login.

The results when importing an opvault file in KeePassXC 2.7.5 varied. Sometimes the email was imported as username while actual_username was dropped. In other cases the username (actual_username) from the section below the top section was imported as username while email from top field was dropped.

Login2 goes a bit further and uses three fields titled "username". Login 3 uses four fields titled "username".

Hope this is sufficient to reproduce and resolve this.

Thanks for making KeePassXC awesome ❤️

30 days, so feel free to download and store / share locally as needed: https://upload.disroot.org/r/Hcd7gSsW#ZY7kDrw9VBgLo492/IwPm1/AjrgMjPyiTYrPOra0CuQ=
Vault Password: 123

@droidmonkey
Copy link
Member

you need to zip up the .opvault folder and send it as that. You shared a 0 byte file above.

@foss-
Copy link
Author

foss- commented Jun 27, 2023

Please retry now.

@droidmonkey
Copy link
Member

Figured it out and fixed, we were blindly overwriting fields with the same name. This fix will be incorporated into the overarching import wizard feature.

image

@droidmonkey droidmonkey added this to the v2.8.0 milestone Jun 27, 2023
droidmonkey added a commit that referenced this issue Aug 31, 2023
* Closes #7545 - Support 1Password 1PUX import format based on https://support.1password.com/1pux-format/

* Closes #8367 - Support Bitwarden JSON import format (both unencrypted and encrypted) based on https://bitwarden.com/help/encrypted-export/

* Fixes #9577 - OPVault import when fields have the same name or type

* Introduce the import wizard to handle all import tasks (CSV, KDBX1, OPVault, 1PUX, JSON)

* Clean up CSV parser code to make it much more efficient and easier to read

* Combine all importer tests (except CSV) into one test file
droidmonkey added a commit that referenced this issue Sep 1, 2023
* Closes #7545 - Support 1Password 1PUX import format based on https://support.1password.com/1pux-format/

* Closes #8367 - Support Bitwarden JSON import format (both unencrypted and encrypted) based on https://bitwarden.com/help/encrypted-export/

* Fixes #9577 - OPVault import when fields have the same name or type

* Introduce the import wizard to handle all import tasks (CSV, KDBX1, OPVault, 1PUX, JSON)

* Clean up CSV parser code to make it much more efficient and easier to read

* Combine all importer tests (except CSV) into one test file
droidmonkey added a commit that referenced this issue Sep 27, 2023
* Closes #7545 - Support 1Password 1PUX import format based on https://support.1password.com/1pux-format/

* Closes #8367 - Support Bitwarden JSON import format (both unencrypted and encrypted) based on https://bitwarden.com/help/encrypted-export/

* Fixes #9577 - OPVault import when fields have the same name or type

* Introduce the import wizard to handle all import tasks (CSV, KDBX1, OPVault, 1PUX, JSON)

* Clean up CSV parser code to make it much more efficient and easier to read

* Combine all importer tests (except CSV) into one test file
droidmonkey added a commit that referenced this issue Sep 29, 2023
* Closes #7545 - Support 1Password 1PUX import format based on https://support.1password.com/1pux-format/

* Closes #8367 - Support Bitwarden JSON import format (both unencrypted and encrypted) based on https://bitwarden.com/help/encrypted-export/

* Fixes #9577 - OPVault import when fields have the same name or type

* Introduce the import wizard to handle all import tasks (CSV, KDBX1, OPVault, 1PUX, JSON)

* Clean up CSV parser code to make it much more efficient and easier to read

* Combine all importer tests (except CSV) into one test file
droidmonkey added a commit that referenced this issue Sep 29, 2023
* Closes #7545 - Support 1Password 1PUX import format based on https://support.1password.com/1pux-format/

* Closes #8367 - Support Bitwarden JSON import format (both unencrypted and encrypted) based on https://bitwarden.com/help/encrypted-export/

* Fixes #9577 - OPVault import when fields have the same name or type

* Introduce the import wizard to handle all import tasks (CSV, KDBX1, OPVault, 1PUX, JSON)

* Clean up CSV parser code to make it much more efficient and easier to read

* Combine all importer tests (except CSV) into one test file
droidmonkey added a commit that referenced this issue Oct 1, 2023
* Closes #7545 - Support 1Password 1PUX import format based on https://support.1password.com/1pux-format/

* Closes #8367 - Support Bitwarden JSON import format (both unencrypted and encrypted) based on https://bitwarden.com/help/encrypted-export/

* Fixes #9577 - OPVault import when fields have the same name or type

* Introduce the import wizard to handle all import tasks (CSV, KDBX1, OPVault, 1PUX, JSON)

* Clean up CSV parser code to make it much more efficient and easier to read

* Combine all importer tests (except CSV) into one test file
droidmonkey added a commit that referenced this issue Oct 1, 2023
* Closes #7545 - Support 1Password 1PUX import format based on https://support.1password.com/1pux-format/

* Closes #8367 - Support Bitwarden JSON import format (both unencrypted and encrypted) based on https://bitwarden.com/help/encrypted-export/

* Fixes #9577 - OPVault import when fields have the same name or type

* Introduce the import wizard to handle all import tasks (CSV, KDBX1, OPVault, 1PUX, JSON)

* Clean up CSV parser code to make it much more efficient and easier to read

* Combine all importer tests (except CSV) into one test file
droidmonkey added a commit that referenced this issue Jan 15, 2024
* Closes #7545 - Support 1Password 1PUX import format based on https://support.1password.com/1pux-format/

* Closes #8367 - Support Bitwarden JSON import format (both unencrypted and encrypted) based on https://bitwarden.com/help/encrypted-export/

* Fixes #9577 - OPVault import when fields have the same name or type

* Introduce the import wizard to handle all import tasks (CSV, KDBX1, OPVault, 1PUX, JSON)

* Clean up CSV parser code to make it much more efficient and easier to read

* Combine all importer tests (except CSV) into one test file
droidmonkey added a commit that referenced this issue Jan 16, 2024
* Closes #7545 - Support 1Password 1PUX import format based on https://support.1password.com/1pux-format/

* Closes #8367 - Support Bitwarden JSON import format (both unencrypted and encrypted) based on https://bitwarden.com/help/encrypted-export/

* Fixes #9577 - OPVault import when fields have the same name or type

* Introduce the import wizard to handle all import tasks (CSV, KDBX1, OPVault, 1PUX, JSON)

* Clean up CSV parser code to make it much more efficient and easier to read

* Combine all importer tests (except CSV) into one test file
droidmonkey added a commit that referenced this issue Feb 3, 2024
* Closes #7545 - Support 1Password 1PUX import format based on https://support.1password.com/1pux-format/

* Closes #8367 - Support Bitwarden JSON import format (both unencrypted and encrypted) based on https://bitwarden.com/help/encrypted-export/

* Fixes #9577 - OPVault import when fields have the same name or type

* Introduce the import wizard to handle all import tasks (CSV, KDBX1, OPVault, 1PUX, JSON)

* Clean up CSV parser code to make it much more efficient and easier to read

* Combine all importer tests (except CSV) into one test file
droidmonkey added a commit that referenced this issue Mar 3, 2024
* Closes #7545 - Support 1Password 1PUX import format based on https://support.1password.com/1pux-format/

* Closes #8367 - Support Bitwarden JSON import format (both unencrypted and encrypted) based on https://bitwarden.com/help/encrypted-export/

* Fixes #9577 - OPVault import when fields have the same name or type

* Introduce the import wizard to handle all import tasks (CSV, KDBX1, OPVault, 1PUX, JSON)

* Clean up CSV parser code to make it much more efficient and easier to read

* Combine all importer tests (except CSV) into one test file
droidmonkey added a commit that referenced this issue Mar 6, 2024
* Closes #7545 - Support 1Password 1PUX import format based on https://support.1password.com/1pux-format/

* Closes #8367 - Support Bitwarden JSON import format (both unencrypted and encrypted) based on https://bitwarden.com/help/encrypted-export/

* Fixes #9577 - OPVault import when fields have the same name or type

* Introduce the import wizard to handle all import tasks (CSV, KDBX1, OPVault, 1PUX, JSON)

* Clean up CSV parser code to make it much more efficient and easier to read

* Combine all importer tests (except CSV) into one test file
droidmonkey added a commit that referenced this issue Mar 7, 2024
* Closes #7545 - Support 1Password 1PUX import format based on https://support.1password.com/1pux-format/

* Closes #8367 - Support Bitwarden JSON import format (both unencrypted and encrypted) based on https://bitwarden.com/help/encrypted-export/

* Fixes #9577 - OPVault import when fields have the same name or type

* Introduce the import wizard to handle all import tasks (CSV, KDBX1, OPVault, 1PUX, JSON)

* Clean up CSV parser code to make it much more efficient and easier to read

* Combine all importer tests (except CSV) into one test file
droidmonkey added a commit that referenced this issue Mar 8, 2024
* Closes #7545 - Support 1Password 1PUX import format based on https://support.1password.com/1pux-format/

* Closes #8367 - Support Bitwarden JSON import format (both unencrypted and encrypted) based on https://bitwarden.com/help/encrypted-export/

* Fixes #9577 - OPVault import when fields have the same name or type

* Introduce the import wizard to handle all import tasks (CSV, KDBX1, OPVault, 1PUX, JSON)

* Clean up CSV parser code to make it much more efficient and easier to read

* Combine all importer tests (except CSV) into one test file
droidmonkey added a commit that referenced this issue Mar 9, 2024
* Closes #7545 - Support 1Password 1PUX import format based on https://support.1password.com/1pux-format/

* Closes #8367 - Support Bitwarden JSON import format (both unencrypted and encrypted) based on https://bitwarden.com/help/encrypted-export/

* Fixes #9577 - OPVault import when fields have the same name or type

* Introduce the import wizard to handle all import tasks (CSV, KDBX1, OPVault, 1PUX, JSON)

* Clean up CSV parser code to make it much more efficient and easier to read

* Combine all importer tests (except CSV) into one test file
droidmonkey added a commit that referenced this issue Mar 9, 2024
* Closes #7545 - Support 1Password 1PUX import format based on https://support.1password.com/1pux-format/

* Closes #8367 - Support Bitwarden JSON import format (both unencrypted and encrypted) based on https://bitwarden.com/help/encrypted-export/

* Fixes #9577 - OPVault import when fields have the same name or type

* Introduce the import wizard to handle all import tasks (CSV, KDBX1, OPVault, 1PUX, JSON)

* Clean up CSV parser code to make it much more efficient and easier to read

* Combine all importer tests (except CSV) into one test file
droidmonkey added a commit that referenced this issue Mar 9, 2024
* Closes #7545 - Support 1Password 1PUX import format based on https://support.1password.com/1pux-format/

* Closes #8367 - Support Bitwarden JSON import format (both unencrypted and encrypted) based on https://bitwarden.com/help/encrypted-export/

* Fixes #9577 - OPVault import when fields have the same name or type

* Introduce the import wizard to handle all import tasks (CSV, KDBX1, OPVault, 1PUX, JSON)

* Clean up CSV parser code to make it much more efficient and easier to read

* Combine all importer tests (except CSV) into one test file
droidmonkey added a commit that referenced this issue Mar 9, 2024
* Closes #7545 - Support 1Password 1PUX import format based on https://support.1password.com/1pux-format/

* Closes #8367 - Support Bitwarden JSON import format (both unencrypted and encrypted) based on https://bitwarden.com/help/encrypted-export/

* Fixes #9577 - OPVault import when fields have the same name or type

* Introduce the import wizard to handle all import tasks (CSV, KDBX1, OPVault, 1PUX, JSON)

* Clean up CSV parser code to make it much more efficient and easier to read

* Combine all importer tests (except CSV) into one test file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants