Skip to content

Commit

Permalink
Fix OPVault import when there are multiple OTP fields
Browse files Browse the repository at this point in the history
* Fix keepassxreboot#8371 - store multiple OTP fields as `otp_#` instead of silently discarding them.
  • Loading branch information
droidmonkey committed Sep 7, 2022
1 parent bdeef63 commit 9e81c31
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 1 deletion.
11 changes: 10 additions & 1 deletion src/format/OpVaultReaderSections.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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")) {
Expand Down
10 changes: 10 additions & 0 deletions tests/TestOpVaultReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "format/OpVaultReader.h"
#include "totp/totp.h"

#include <QJsonObject>
#include <QList>
#include <QStringList>
#include <QTest>
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 9e81c31

Please sign in to comment.