Fix OPVault import when there are multiple OTP fields

* Fix #8371 - store multiple OTP fields as `otp_#` instead of silently discarding them.
This commit is contained in:
Jonathan White 2022-09-01 06:54:34 -04:00
parent 28328a7080
commit f3d448485a
2 changed files with 20 additions and 1 deletions

View File

@ -84,7 +84,16 @@ void OpVaultReader::fillFromSectionField(Entry* entry, const QString& sectionNam
auto kind = field["k"].toString();
if (attrName.startsWith("TOTP_")) {
if (attrValue.startsWith("otpauth://")) {
if (entry->hasTotp()) {
// Store multiple TOTP definitions as additional otp attributes
int i = 0;
QString name("otp");
auto attributes = entry->attributes()->keys();
while (attributes.contains(name)) {
name = QString("otp_%1").arg(++i);
}
entry->attributes()->set(name, attrValue);
} else if (attrValue.startsWith("otpauth://")) {
QUrlQuery query(attrValue);
// at least as of 1Password 7, they don't append the digits= and period= which totp.cpp requires
if (!query.hasQueryItem("digits")) {

View File

@ -24,6 +24,7 @@
#include "format/OpVaultReader.h"
#include "totp/totp.h"
#include <QJsonObject>
#include <QList>
#include <QStringList>
#include <QTest>
@ -110,6 +111,15 @@ void TestOpVaultReader::testReadIntoDatabase()
QCOMPARE(totpSettings->digits, static_cast<unsigned int>(8));
QCOMPARE(totpSettings->step, static_cast<unsigned int>(45));
// Add another OTP to this entry to confirm it doesn't overwrite the existing one
auto field = QJsonObject::fromVariantMap({{"n", "TOTP_SETTINGS"}, {"v", "otpauth://test.url?digits=6"}});
reader.fillFromSectionField(entry, "", field);
QVERIFY(entry->hasTotp());
totpSettings = entry->totpSettings();
QCOMPARE(totpSettings->digits, static_cast<unsigned int>(8));
QCOMPARE(totpSettings->step, static_cast<unsigned int>(45));
QVERIFY(entry->attributes()->contains("otp_1"));
// Confirm trashed entries are sent to the recycle bin
auto recycleBin = db->metadata()->recycleBin();
QVERIFY(recycleBin);