Members of the KDE Community are recommended to subscribe to the kde-community mailing list at https://mail.kde.org/mailman/listinfo/kde-community to allow them to participate in important discussions and receive other important announcements

Commit b70feaf6 authored by Johan Ouwerkerk's avatar Johan Ouwerkerk

Track loaded/error state of AccountStorage objects

Two bits of boolean state are introduces to track whether or not:

 - an error has occurred
 - accounts have been loaded from storage yet

This change paves the way for having error handling UX.
parent 1d8d6976
......@@ -18,6 +18,7 @@
static QString emptyIniResource(QLatin1String("empty-accounts.ini"));
static QString corpusIniResource(QLatin1String("sample-accounts.ini"));
static QString invalidIniResource(QLatin1String("invalid-accounts.ini"));
class LoadAccountsTest: public QObject
{
......@@ -26,6 +27,7 @@ private Q_SLOTS:
void initTestCase(void);
void emptyAccountsFile(void);
void sampleAccountsFile(void);
void invalidSampleAccountsFile(void);
private:
accounts::AccountSecret m_secret {&test::fakeRandom};
};
......@@ -38,6 +40,7 @@ void LoadAccountsTest::initTestCase(void)
QVERIFY2(test::ensureOutputDirectory(), "output directory should be available");
QVERIFY2(test::copyResource(":/load-accounts/empty-accounts.ini", emptyIniResource), "empty INI resource should be available as file");
QVERIFY2(test::copyResource(":/load-accounts/sample-accounts.ini", corpusIniResource), "test corpus INI resource should be available as file");
QVERIFY2(test::copyResource(":/load-accounts/invalid-accounts.ini", invalidIniResource), "invalid INI resource should be available as file");
QVERIFY2(test::useDummyPassword(&m_secret), "should be able to set up the master key");
}
......@@ -55,6 +58,7 @@ void LoadAccountsTest::emptyAccountsFile(void)
QSignalSpy hotpFound(&uut, &accounts::LoadAccounts::foundHotp);
QSignalSpy totpFound(&uut, &accounts::LoadAccounts::foundTotp);
QSignalSpy loadingError(&uut, &accounts::LoadAccounts::failedToLoadAllAccounts);
QSignalSpy jobFinished(&uut, &accounts::LoadAccounts::finished);
uut.run();
......@@ -63,6 +67,7 @@ void LoadAccountsTest::emptyAccountsFile(void)
QVERIFY2(actionRun, "accounts action should have run");
QCOMPARE(hotpFound.count(), 0);
QCOMPARE(totpFound.count(), 0);
QCOMPARE(loadingError.count(), 0);
}
void LoadAccountsTest::sampleAccountsFile(void)
......@@ -79,6 +84,7 @@ void LoadAccountsTest::sampleAccountsFile(void)
QSignalSpy hotpFound(&uut, &accounts::LoadAccounts::foundHotp);
QSignalSpy totpFound(&uut, &accounts::LoadAccounts::foundTotp);
QSignalSpy loadingError(&uut, &accounts::LoadAccounts::failedToLoadAllAccounts);
QSignalSpy jobFinished(&uut, &accounts::LoadAccounts::finished);
uut.run();
......@@ -87,6 +93,7 @@ void LoadAccountsTest::sampleAccountsFile(void)
QVERIFY2(actionRun, "accounts action should have run");
QCOMPARE(hotpFound.count(), 1);
QCOMPARE(totpFound.count(), 1);
QCOMPARE(loadingError.count(), 0);
const auto firstHotp = hotpFound.at(0);
QCOMPARE(firstHotp.at(0).toUuid(), QUuid(QLatin1String("072a645d-6c26-57cc-81eb-d9ef3b9b39e2")));
......@@ -105,6 +112,32 @@ void LoadAccountsTest::sampleAccountsFile(void)
QCOMPARE(firstTotp.at(5).toInt(), 6);
}
void LoadAccountsTest::invalidSampleAccountsFile(void)
{
bool actionRun = false;
const accounts::SettingsProvider settings([&actionRun](const accounts::PersistenceAction &action) -> void
{
QSettings data(test::path(invalidIniResource), QSettings::IniFormat);
actionRun = true;
action(data);
});
accounts::LoadAccounts uut(settings, &m_secret);
QSignalSpy hotpFound(&uut, &accounts::LoadAccounts::foundHotp);
QSignalSpy totpFound(&uut, &accounts::LoadAccounts::foundTotp);
QSignalSpy loadingError(&uut, &accounts::LoadAccounts::failedToLoadAllAccounts);
QSignalSpy jobFinished(&uut, &accounts::LoadAccounts::finished);
uut.run();
QVERIFY2(test::signal_eventually_emitted_once(jobFinished), "job should be finished");
QVERIFY2(actionRun, "accounts action should have run");
QCOMPARE(hotpFound.count(), 0);
QCOMPARE(totpFound.count(), 0);
QCOMPARE(loadingError.count(), 1);
}
QTEST_MAIN(LoadAccountsTest)
#include "load-accounts.moc"
[%7B6814578e-2019-4ecb-b7c6-844c318379a5%7D]
account=
counter=0
nonce=QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB
pinLength=6
secret="8juE9gJFLp3OgL4CxJ5v5q8sw+h7Vbn06+NY4uc="
type=hotp
[%7Bb2167344-25d2-49fe-a420-7096aa511f67%7D]
account=invalid-hotp-sample-2
counter=-1
nonce=QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB
pinLength=6
secret="8juE9gJFLp3OgL4CxJ5v5q8sw+h7Vbn06+NY4uc="
type=hotp
[%7Bf6fd1348-b8af-4eb3-945f-fbc94026a6c7%7D]
account=invalid-hotp-sample-3
counter=0
nonce=QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFZ
pinLength=6
secret="8juE9gJFLp3OgL4CxJ5v5q8sw+h7Vbn06+NY4uc="
type=hotp
[%7B75394fe2-35bf-4ca3-8e0c-9102810b9d55%7D]
account=invalid-hotp-sample-4
counter=0
nonce=QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB
pinLength=0
secret="8juE9gJFLp3OgL4CxJ5v5q8sw+h7Vbn06+NY4uc="
type=hotp
[%7Bc02ae4fe-b6fb-4191-93fb-304a7197320c%7D]
account=invalid-hotp-sample-5
counter=
nonce=QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB
pinLength=6
secret="8juE9gJFLp3OgL4CxJ5v5q8sw+h7Vbn06+NY4uc="
type=hotp
[%7Bfff1868c-e1db-4cd7-b931-7d1e51154c64%7D]
account=invalid-totp-sample-1
nonce=QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB
pinLength=6
secret="8juE9gJFLp3OgL4CxJ5v5q8sw+h7Vbn06+NY4uc="
timeStep=0
type=totp
[%7B5aef5e62-d494-4a8d-aa44-a49b7f4c2b28%7D]
account=invalid-totp-sample-2
nonce=QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB
pinLength=6
secret="8juE9gJFLp3OgL4CxJ5v5q8sw+h7Vbn06+NY4uc="
timeStep=
type=totp
[%7Bd80d6d02-ea5e-47a0-bf51-e4b37b016ec2%7D]
account=invalid-totp-sample-3
nonce=QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB
pinLength=
secret="8juE9gJFLp3OgL4CxJ5v5q8sw+h7Vbn06+NY4uc="
timeStep=30
type=totp
[%7Be59f254d-729f-4631-8d7f-69fa314ed664%7D]
account=invalid-totp-sample-4
nonce=QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB
pinLength=6
secret="8juE9gJFLp3OgL4CxJ5v5q8sw+h7Vbn06+NY4uc="
timeStep=30
type=foo
[%7B0a1c83c9-62a8-4854-b985-7f364251dbf1%7D]
account=valid-totp-sample-5
nonce=QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB
pinLength=6
secret="FB1uuIu7qNWfqyGhFcemi7pxgcZgLNXoZZZtbdA="
timeStep=30
type=totp
SPDX-License-Identifier: GPL-3.0-or-later
SPDX-FileCopyrightText: 2020 Johan Ouwerkerk <jm.ouwerkerk@gmail.com>
......@@ -6,6 +6,7 @@
<RCC>
<qresource prefix="/">
<file>load-accounts/sample-accounts.ini</file>
<file>load-accounts/invalid-accounts.ini</file>
<file>load-accounts/empty-accounts.ini</file>
<file>save-hotp/expected-accounts.ini</file>
<file>save-totp/expected-accounts.ini</file>
......
......@@ -64,6 +64,8 @@ void HotpCounterUpdateTest::testCounterUpdate(void)
QSignalSpy secretCleaned(secret, &accounts::AccountSecret::destroyed);
accounts::AccountStorage *uut = new accounts::AccountStorage(settings, thread, secret);
QSignalSpy error(uut, &accounts::AccountStorage::error);
QSignalSpy loaded(uut, &accounts::AccountStorage::loaded);
QSignalSpy accountAdded(uut, &accounts::AccountStorage::added);
QSignalSpy accountRemoved(uut, &accounts::AccountStorage::removed);
QSignalSpy storageDisposed(uut, &accounts::AccountStorage::disposed);
......@@ -84,7 +86,11 @@ void HotpCounterUpdateTest::testCounterUpdate(void)
QVERIFY2(test::signal_eventually_emitted_once(keyAvailable, 2500), "key should have been derived by now");
// expect that loading is scheduled automatically, so advancing the event loop should trigger the signal
QVERIFY2(test::signal_eventually_emitted_once(accountAdded), "sample account should be loaded by now");
QVERIFY2(test::signal_eventually_emitted_once(loaded), "sample account should be loaded by now");
QCOMPARE(uut->isLoaded(), true);
QCOMPARE(uut->hasError(), false);
QCOMPARE(error.count(), 0);
QCOMPARE(accountAdded.count(), 1);
QCOMPARE(accountAdded.at(0).at(0), sampleAccountName);
accounts::Account *sampleAccount = uut->get(sampleAccountName);
......@@ -128,6 +134,8 @@ void HotpCounterUpdateTest::testCounterUpdate(void)
QVERIFY2(test::signal_eventually_emitted_twice(sampleAccountTokenUpdated), "sample account should be updated in storage by now");
QCOMPARE(sampleAccountTokenUpdated.at(1).at(0), updatedToken);
QCOMPARE(uut->hasError(), false);
QCOMPARE(error.count(), 0);
QFile afterUpdatingCounterLockFile(test::path(testIniLockFile));
QVERIFY2(!afterUpdatingCounterLockFile.exists(), "after updating counter: lock file should not be present anymore");
......@@ -146,6 +154,8 @@ void HotpCounterUpdateTest::testCounterUpdate(void)
// fifth phase: check the sum-total effects
QCOMPARE(error.count(), 0);
QCOMPARE(loaded.count(), 1);
QCOMPARE(accountAdded.count(), 1);
QCOMPARE(accountRemoved.count(), 0);
QCOMPARE(sampleAccountRemoved.count(), 0);
......
......@@ -39,6 +39,8 @@ void StorageAbortLifeCycleTest::testLifecycle(void)
});
accounts::AccountStorage *uut = accounts::AccountStorage::open(settings);
QSignalSpy error(uut, &accounts::AccountStorage::error);
QSignalSpy loaded(uut, &accounts::AccountStorage::loaded);
QSignalSpy accountAdded(uut, &accounts::AccountStorage::added);
QSignalSpy storageDisposed(uut, &accounts::AccountStorage::disposed);
QSignalSpy storageCleaned(uut, &accounts::AccountStorage::destroyed);
......@@ -66,6 +68,8 @@ void StorageAbortLifeCycleTest::testLifecycle(void)
QCOMPARE(passwordAvailable.count(), 0);
QCOMPARE(keyAvailable.count(), 0);
QCOMPARE(accountAdded.count(), 0);
QCOMPARE(loaded.count(), 0);
QCOMPARE(error.count(), 1);
}
QTEST_MAIN(StorageAbortLifeCycleTest)
......
......@@ -43,6 +43,8 @@ void StorageDefaultLifeCycleTest::testLifecycle(void)
});
accounts::AccountStorage *uut = accounts::AccountStorage::open(settings);
QSignalSpy error(uut, &accounts::AccountStorage::error);
QSignalSpy loaded(uut, &accounts::AccountStorage::loaded);
QSignalSpy accountAdded(uut, &accounts::AccountStorage::added);
QSignalSpy storageDisposed(uut, &accounts::AccountStorage::disposed);
QSignalSpy storageCleaned(uut, &accounts::AccountStorage::destroyed);
......@@ -56,6 +58,8 @@ void StorageDefaultLifeCycleTest::testLifecycle(void)
QSignalSpy secretCleaned(secret, &accounts::AccountSecret::destroyed);
// first phase: check that account objects can be loaded from storage
QCOMPARE(uut->isLoaded(), false);
QCOMPARE(uut->hasError(), false);
// expect that unlocking is scheduled automatically, so advancing the event loop should trigger the signal
QVERIFY2(test::signal_eventually_emitted_once(existingPasswordNeeded), "(existing) password should be asked by now");
......@@ -70,7 +74,11 @@ void StorageDefaultLifeCycleTest::testLifecycle(void)
QVERIFY2(test::signal_eventually_emitted_once(keyAvailable, 2500), "key should have been derived by now");
// expect that loading is scheduled automatically, so advancing the event loop should trigger the signal
QVERIFY2(test::signal_eventually_emitted_once(accountAdded), "sample account should be loaded by now");
QVERIFY2(test::signal_eventually_emitted_once(loaded), "sample account should be loaded by now");
QCOMPARE(uut->isLoaded(), true);
QCOMPARE(uut->hasError(), false);
QCOMPARE(error.count(), 0);
QCOMPARE(accountAdded.count(), 1);
QCOMPARE(accountAdded.at(0).at(0), sampleAccountName);
accounts::Account *sampleAccount = uut->get(sampleAccountName);
......
......@@ -64,13 +64,19 @@ void StorageLifeCyclesTest::testLifecycle(void)
QSignalSpy secretCleaned(secret, &accounts::AccountSecret::destroyed);
accounts::AccountStorage *uut = new accounts::AccountStorage(settings, thread, secret);
QSignalSpy error(uut, &accounts::AccountStorage::error);
QSignalSpy loaded(uut, &accounts::AccountStorage::loaded);
QSignalSpy accountAdded(uut, &accounts::AccountStorage::added);
QSignalSpy accountRemoved(uut, &accounts::AccountStorage::removed);
QSignalSpy storageDisposed(uut, &accounts::AccountStorage::disposed);
QSignalSpy storageCleaned(uut, &accounts::AccountStorage::destroyed);
// first phase: check that account objects can be loaded from storage
QCOMPARE(uut->isLoaded(), false);
QCOMPARE(uut->hasError(), false);
QCOMPARE(accountAdded.count(), 0);
QCOMPARE(loaded.count(), 0);
QCOMPARE(error.count(), 0);
QVERIFY2(uut->isNameStillAvailable(initialAccountName), "sample account name should still be available");
QVERIFY2(uut->isNameStillAvailable(addedAccountName), "new account name should still be available");
QCOMPARE(uut->accounts(), QVector<QString>());
......@@ -88,7 +94,11 @@ void StorageLifeCyclesTest::testLifecycle(void)
QVERIFY2(test::signal_eventually_emitted_once(keyAvailable, 2500), "key should have been derived by now");
// expect that loading is scheduled automatically, so advancing the event loop should trigger the signal
QVERIFY2(test::signal_eventually_emitted_once(accountAdded), "sample account should be loaded by now");
QVERIFY2(test::signal_eventually_emitted_once(loaded), "sample account should be loaded by now");
QCOMPARE(uut->isLoaded(), true);
QCOMPARE(uut->hasError(), false);
QCOMPARE(error.count(), 0);
QCOMPARE(accountAdded.count(), 1);
QCOMPARE(accountAdded.at(0).at(0), initialAccountName);
QVERIFY2(!uut->isNameStillAvailable(initialAccountName), "sample account name should no longer be available");
......@@ -148,6 +158,7 @@ void StorageLifeCyclesTest::testLifecycle(void)
uut->addTotp(addedAccountName, QLatin1String("NBSWY3DPFQQHO33SNRSCC==="), 42, 8);
QVERIFY2(test::signal_eventually_emitted_twice(accountAdded), "new account should be added to storage by now");
QCOMPARE(error.count(), 0);
QCOMPARE(accountAdded.at(1).at(0), addedAccountName);
QVERIFY2(uut->isNameStillAvailable(initialAccountName), "sample account name should again still be available");
......@@ -202,6 +213,8 @@ void StorageLifeCyclesTest::testLifecycle(void)
// fifth phase: check the sum-total effects
QCOMPARE(error.count(), 0);
QCOMPARE(loaded.count(), 1);
QCOMPARE(addedAccountRemoved.count(), 0);
QCOMPARE(accountAdded.count(), 2);
QCOMPARE(accountRemoved.count(), 1);
......
......@@ -128,6 +128,7 @@ namespace accounts
const std::function<void(RequestAccountPassword*)> handler([this](RequestAccountPassword *job) -> void
{
QObject::connect(job, &RequestAccountPassword::unlocked, this, &AccountStorage::load);
QObject::connect(job, &RequestAccountPassword::failed, this, &AccountStorage::handleError);
});
d->unlock(handler);
}
......@@ -139,6 +140,8 @@ namespace accounts
{
QObject::connect(job, &LoadAccounts::foundHotp, this, &AccountStorage::handleHotp);
QObject::connect(job, &LoadAccounts::foundTotp, this, &AccountStorage::handleTotp);
QObject::connect(job, &LoadAccounts::finished, this, &AccountStorage::handleLoaded);
QObject::connect(job, &LoadAccounts::failedToLoadAllAccounts, this, &AccountStorage::handleError);
});
d->load(handler);
}
......@@ -173,8 +176,11 @@ namespace accounts
const std::function<void(SaveHotp*)> handler([this](SaveHotp *job) -> void
{
QObject::connect(job, &SaveHotp::saved, this, &AccountStorage::handleHotp);
QObject::connect(job, &SaveHotp::invalid, this, &AccountStorage::handleError);
});
d->addHotp(handler, name, secret, counter, tokenLength, offset, addChecksum);
if (!d->addHotp(handler, name, secret, counter, tokenLength, offset, addChecksum)) {
Q_EMIT error();
}
}
void AccountStorage::addTotp(const QString &name, const QString &secret, int timeStep, int tokenLength, const QDateTime &epoch, Account::Hash hash)
......@@ -183,8 +189,11 @@ namespace accounts
const std::function<void(SaveTotp*)> handler([this](SaveTotp *job) -> void
{
QObject::connect(job, &SaveTotp::saved, this, &AccountStorage::handleTotp);
QObject::connect(job, &SaveTotp::invalid, this, &AccountStorage::handleError);
});
d->addTotp(handler, name, secret, timeStep, tokenLength, epoch, hash);
if (!d->addTotp(handler, name, secret, timeStep, tokenLength, epoch, hash)) {
Q_EMIT error();
}
}
void AccountStorage::accountRemoved(void)
......@@ -286,4 +295,34 @@ namespace accounts
Q_D(AccountStorage);
d->acceptDisposal();
}
bool AccountStorage::hasError(void) const
{
Q_D(const AccountStorage);
return d->hasError();
}
void AccountStorage::clearError(void)
{
Q_D(AccountStorage);
d->clearError();
}
void AccountStorage::handleError(void)
{
Q_D(AccountStorage);
d->notifyError();
}
void AccountStorage::handleLoaded(void)
{
Q_D(AccountStorage);
d->notifyLoaded();
}
bool AccountStorage::isLoaded(void) const
{
Q_D(const AccountStorage);
return d->isLoaded();
}
}
......@@ -87,15 +87,22 @@ namespace accounts
int tokenLength = 6,
const QDateTime &epoch = QDateTime::fromMSecsSinceEpoch(0),
Account::Hash hash = Account::Hash::Default);
void clearError(void);
bool hasError(void) const;
bool isLoaded(void) const;
Q_SIGNALS:
void added(const QString name);
void removed(const QString name);
void error(void);
void loaded(void);
void disposed(void);
private Q_SLOTS:
void unlock(void);
void load(void);
void accountRemoved(void);
void handleDisposal(void);
void handleError(void);
void handleLoaded(void);
void handleHotp(const QUuid id, const QString name, const QByteArray secret, const QByteArray nonce, quint64 counter, int tokenLength);
void handleTotp(const QUuid id, const QString name, const QByteArray secret, const QByteArray nonce, uint timeStep, int tokenLength);
private:
......
......@@ -97,7 +97,7 @@ namespace accounts
SaveHotp *job = new SaveHotp(m_storage->settings(),m_id, m_name, m_secret, counter, m_tokenLength);
m_actions->queueAndProceed(job, [counter, job, q, this](void) -> void
{
new HandleCounterUpdate(this, counter, job, q);
new HandleCounterUpdate(this, m_storage, counter, job, q);
});
}
......@@ -453,24 +453,24 @@ namespace accounts
return checkTokenLength(tokenLength) && checkName(name) && isNameStillAvailable(name) && checkSecret(secret);
}
void AccountStoragePrivate::addHotp(const std::function<void(SaveHotp*)> &handler, const QString &name, const QString &secret, quint64 counter, int tokenLength, int offset, bool addChecksum)
bool AccountStoragePrivate::addHotp(const std::function<void(SaveHotp*)> &handler, const QString &name, const QString &secret, quint64 counter, int tokenLength, int offset, bool addChecksum)
{
Q_UNUSED(offset);
Q_UNUSED(addChecksum);
if (!m_is_still_open) {
qCDebug(logger) << "Will not add new HOTP account: storage no longer open";
return;
return false;
}
if (!validateGenericNewToken(name, secret, tokenLength)) {
qCDebug(logger) << "Will not add new HOTP account: invalid account details";
return;
return false;
}
std::optional<secrets::EncryptedSecret> encryptedSecret = encrypt(secret);
if (!encryptedSecret) {
qCDebug(logger) << "Will not add new HOTP account: failed to encrypt secret";
return;
return false;
}
QUuid id = generateId(name);
......@@ -482,26 +482,27 @@ namespace accounts
{
handler(job);
});
return true;
}
void AccountStoragePrivate::addTotp(const std::function<void(SaveTotp*)> &handler, const QString &name, const QString &secret, uint timeStep, int tokenLength, const QDateTime &epoch, Account::Hash hash)
bool AccountStoragePrivate::addTotp(const std::function<void(SaveTotp*)> &handler, const QString &name, const QString &secret, uint timeStep, int tokenLength, const QDateTime &epoch, Account::Hash hash)
{
Q_UNUSED(epoch);
Q_UNUSED(hash);
if (!m_is_still_open) {
qCDebug(logger) << "Will not add new TOTP account: storage no longer open";
return;
return false;
}
if (!validateGenericNewToken(name, secret, tokenLength) || !checkTimeStep(timeStep)) {
qCDebug(logger) << "Will not add new TOTP account: invalid account details";
return;
return false;
}
std::optional<secrets::EncryptedSecret> encryptedSecret = encrypt(secret);
if (!encryptedSecret) {
qCDebug(logger) << "Will not add new TOTP account: failed to encrypt secret";
return;
return false;
}
QUuid id = generateId(name);
......@@ -513,6 +514,7 @@ namespace accounts
{
handler(job);
});
return true;
}
void AccountStoragePrivate::unlock(const std::function<void(RequestAccountPassword*)> &handler)
......@@ -580,13 +582,42 @@ namespace accounts
return m_accounts[id];
}
bool AccountStoragePrivate::isLoaded(void) const
{
return m_is_loaded;
}
void AccountStoragePrivate::notifyLoaded(void)
{
Q_Q(AccountStorage);
m_is_loaded = true;
Q_EMIT q->loaded();
}
void AccountStoragePrivate::notifyError(void)
{
Q_Q(AccountStorage);
m_has_error = true;
Q_EMIT q->error();
}
void AccountStoragePrivate::clearError(void)
{
m_has_error = false;
}
bool AccountStoragePrivate::hasError(void) const
{
return m_has_error;
}
AccountStoragePrivate::AccountStoragePrivate(const SettingsProvider &settings, AccountSecret *secret, AccountStorage *storage, Dispatcher *dispatcher) :
q_ptr(storage), m_is_still_open(true), m_actions(dispatcher), m_settings(settings), m_secret(secret)
q_ptr(storage), m_is_loaded(false), m_has_error(false), m_is_still_open(true), m_actions(dispatcher), m_settings(settings), m_secret(secret)
{
}
HandleCounterUpdate::HandleCounterUpdate(AccountPrivate *account, quint64 counter, SaveHotp *job, QObject *parent) :
QObject(parent), m_accept_on_finish(true), m_counter(counter), m_account(account)
HandleCounterUpdate::HandleCounterUpdate(AccountPrivate *account, AccountStoragePrivate *storage, quint64 counter, SaveHotp *job, QObject *parent) :
QObject(parent), m_accept_on_finish(true), m_counter(counter), m_account(account), m_storage(storage)
{
QObject::connect(job, &SaveHotp::invalid, this, &HandleCounterUpdate::rejected);
QObject::connect(job, &SaveHotp::finished, this, &HandleCounterUpdate::finished);
......@@ -595,6 +626,7 @@ namespace accounts
void HandleCounterUpdate::rejected(void)
{
m_accept_on_finish = false;
m_storage->notifyError();
}
void HandleCounterUpdate::finished(void)
......
......@@ -106,20 +106,25 @@ namespace accounts
int tokenLength = 6,
const QDateTime &epoch = QDateTime::fromMSecsSinceEpoch(0),
Account::Hash hash = Account::Hash::Default);
void addHotp(const std::function<void(SaveHotp*)> &handler,
bool addHotp(const std::function<void(SaveHotp*)> &handler,
const QString &name,
const QString &secret,
quint64 counter = 0ULL,
int tokenLength = 6,
int offset = -1,
bool addChecksum = false);
void addTotp(const std::function<void(SaveTotp*)> &handler,
bool addTotp(const std::function<void(SaveTotp*)> &handler,
const QString &name,
const QString &secret,
uint timeStep = 30,
int tokenLength = 6,
const QDateTime &epoch = QDateTime::fromMSecsSinceEpoch(0),
Account::Hash hash = Account::Hash::Default);
void notifyLoaded(void);
bool isLoaded(void) const;
void notifyError(void);
void clearError(void);
bool hasError(void) const;
private:
bool validateGenericNewToken(const QString &name, const QString &secret, int tokenLength) const;
std::optional<secrets::EncryptedSecret> encrypt(const QString &secret) const;
......@@ -129,6 +134,8 @@ namespace accounts
Q_DISABLE_COPY(AccountStoragePrivate);
AccountStorage * const q_ptr;
private:
bool m_is_loaded;
bool m_has_error;
bool m_is_still_open;
Dispatcher * const m_actions;
const SettingsProvider m_settings;
......@@ -144,11 +151,12 @@ namespace accounts
{
Q_OBJECT
public:
explicit HandleCounterUpdate(AccountPrivate *account, quint64 counter, SaveHotp * job, QObject *parent = nullptr);
explicit HandleCounterUpdate(AccountPrivate *account, AccountStoragePrivate *storage, quint64 counter, SaveHotp *job, QObject *parent = nullptr);
private:
bool m_accept_on_finish;
const quint64 m_counter;
AccountPrivate * const m_account;
AccountStoragePrivate * const m_storage;
private Q_SLOTS:
void rejected(void);
void finished(void);
......@@ -158,8 +166,8 @@ namespace accounts
{
Q_OBJECT
public:
explicit HandleTokenUpdate(AccountPrivate *account, ComputeHotp * job, QObject *parent = nullptr);
explicit HandleTokenUpdate(AccountPrivate *account, ComputeTotp * job, QObject *parent = nullptr);
explicit HandleTokenUpdate(AccountPrivate *account, ComputeHotp *job, QObject *parent = nullptr);
explicit HandleTokenUpdate(AccountPrivate *account, ComputeTotp *job, QObject *parent = nullptr);
private:
AccountPrivate * const m_account;
private Q_SLOTS:
......
......@@ -322,7 +322,8 @@ namespace accounts
return;
}
const PersistenceAction act([this](QSettings &settings) -> void
bool failed = false;
const PersistenceAction act([this, &failed](QSettings &settings) -> void
{
qCInfo(logger, "Loading accounts from storage");
const QStringList entries = settings.childGroups();
......@@ -336,6 +337,7 @@ namespace accounts
qCDebug(logger)
<< "Ignoring:" << group
<< "Not an account section";
failed = true;
continue;
}
......@@ -350,12 +352,13 @@ namespace accounts
continue;
}
const QString type = settings.value("type", "hotp").toString();
const QString type = settings.value("type").toString();
if (type != QLatin1String("hotp") && type != QLatin1String("totp")) {
qCWarning(logger)
<< "Skipping invalid account:" << id
<< "Invalid account type";
settings.endGroup();
failed = true;
continue;
}
......@@ -366,6 +369,7 @@ namespace accounts
<< "Skipping invalid account:" << id
<< "Invalid token length";
settings.endGroup();
failed = true;
continue;
}
......@@ -380,6 +384,7 @@ namespace accounts
<< "Skipping invalid account:" << id
<< "Invalid token secret";
settings.endGroup();
failed = true;
continue;
}
......@@ -389,6 +394,7 @@ namespace accounts
<< "Skipping invalid account:" << id
<< "Unable to decrypt token secret";
settings.endGroup();
failed = true;