Skip to content

Commit e2bf537

Browse files
Aetfdroidmonkey
Aetf
authored and
droidmonkey
committed
FdoSecrets: ask to unlock the database when creating items
Also only emit databaseUnlockFinished after the database is unlocked Fix #7989
1 parent c5467c4 commit e2bf537

6 files changed

Lines changed: 129 additions & 42 deletions

File tree

‎src/fdosecrets/objects/Prompt.cpp‎

Lines changed: 65 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,6 @@ namespace FdoSecrets
210210
m_collections.reserve(colls.size());
211211
for (const auto& coll : asConst(colls)) {
212212
m_collections << coll;
213-
connect(coll, &Collection::doneUnlockCollection, this, &UnlockPrompt::collectionUnlockFinished);
214213
}
215214
for (const auto& item : asConst(items)) {
216215
m_items[item->collection()] << item;
@@ -234,6 +233,7 @@ namespace FdoSecrets
234233
bool waitingForCollections = false;
235234
for (const auto& c : asConst(m_collections)) {
236235
if (c) {
236+
connect(c, &Collection::doneUnlockCollection, this, &UnlockPrompt::collectionUnlockFinished);
237237
// doUnlock is nonblocking, execution will continue in collectionUnlockFinished
238238
// it is ok to call doUnlock multiple times before it's actually unlocked by the user
239239
c->doUnlock();
@@ -242,7 +242,7 @@ namespace FdoSecrets
242242
}
243243

244244
// unlock items directly if no collection unlocking pending
245-
// o.w. do it in collectionUnlockFinished
245+
// o.w. doing it in collectionUnlockFinished
246246
if (!waitingForCollections) {
247247
unlockItems();
248248
}
@@ -400,18 +400,49 @@ namespace FdoSecrets
400400
return PromptResult::accepted(false);
401401
}
402402

403-
bool locked = true;
404-
auto ret = m_coll->locked(locked);
405-
if (locked) {
406-
// collection was locked
407-
return DBusResult{DBUS_ERROR_SECRET_IS_LOCKED};
408-
}
409-
410403
// save a weak reference to the client which may be used asynchronously later
411404
m_client = client;
412405

406+
// give the user a chance to unlock the collection
407+
// UnlockPrompt will handle the case of collection already unlocked
408+
auto prompt = PromptBase::Create<UnlockPrompt>(service(), QSet<Collection*>{m_coll.data()}, QSet<Item*>{});
409+
if (!prompt) {
410+
return DBusResult{QDBusError::InternalError};
411+
}
412+
// postpone anything after the prompt
413+
connect(prompt, &PromptBase::completed, this, [this, windowId](bool dismissed) {
414+
if (dismissed) {
415+
finishPrompt(dismissed);
416+
} else {
417+
auto res = createItem(windowId);
418+
if (res.err()) {
419+
qWarning() << "FdoSecrets:" << res;
420+
finishPrompt(true);
421+
}
422+
}
423+
});
424+
425+
auto ret = prompt->prompt(client, windowId);
426+
if (ret.err()) {
427+
return ret;
428+
}
429+
return PromptResult::Pending;
430+
}
431+
432+
DBusResult CreateItemPrompt::createItem(const QString& windowId)
433+
{
434+
auto client = m_client.lock();
435+
if (!client) {
436+
// client already gone
437+
return {};
438+
}
439+
440+
if (!m_coll) {
441+
return DBusResult{DBUS_ERROR_SECRET_NO_SUCH_OBJECT};
442+
}
443+
413444
// get itemPath to create item and
414-
// try finding an existing item using attributes
445+
// try to find an existing item using attributes
415446
QString itemPath{};
416447
auto iterAttr = m_properties.find(DBUS_INTERFACE_SECRET_ITEM + ".Attributes");
417448
if (iterAttr != m_properties.end()) {
@@ -425,7 +456,7 @@ namespace FdoSecrets
425456

426457
// check existing item using attributes
427458
QList<Item*> existing;
428-
ret = m_coll->searchItems(client, attributes, existing);
459+
auto ret = m_coll->searchItems(client, attributes, existing);
429460
if (ret.err()) {
430461
return ret;
431462
}
@@ -444,31 +475,29 @@ namespace FdoSecrets
444475
}
445476

446477
// the item may be locked due to authorization
447-
ret = m_item->locked(client, locked);
448-
if (ret.err()) {
449-
return ret;
450-
}
451-
if (locked) {
452-
// give the user a chance to unlock the item
453-
auto prompt = PromptBase::Create<UnlockPrompt>(service(), QSet<Collection*>{}, QSet<Item*>{m_item});
454-
if (!prompt) {
455-
return DBusResult{QDBusError::InternalError};
456-
}
457-
// postpone anything after the confirmation
458-
connect(prompt, &PromptBase::completed, this, [this]() {
478+
// give the user a chance to unlock the item
479+
auto prompt = PromptBase::Create<UnlockPrompt>(service(), QSet<Collection*>{}, QSet<Item*>{m_item});
480+
if (!prompt) {
481+
return DBusResult{QDBusError::InternalError};
482+
}
483+
// postpone anything after the confirmation
484+
connect(prompt, &PromptBase::completed, this, [this](bool dismissed) {
485+
if (dismissed) {
486+
finishPrompt(dismissed);
487+
} else {
459488
auto res = updateItem();
460-
finishPrompt(res.err());
461-
});
462-
463-
ret = prompt->prompt(client, windowId);
464-
if (ret.err()) {
465-
return ret;
489+
if (res.err()) {
490+
qWarning() << "FdoSecrets:" << res;
491+
finishPrompt(true);
492+
}
466493
}
467-
return PromptResult::Pending;
468-
}
494+
});
469495

470-
// the item can be updated directly
471-
return updateItem();
496+
auto ret = prompt->prompt(client, windowId);
497+
if (ret.err()) {
498+
return ret;
499+
}
500+
return {};
472501
}
473502

474503
DBusResult CreateItemPrompt::updateItem()
@@ -493,6 +522,9 @@ namespace FdoSecrets
493522
if (ret.err()) {
494523
return ret;
495524
}
525+
526+
// finally can finish the prompt without dismissing it
527+
finishPrompt(false);
496528
return {};
497529
}
498530
} // namespace FdoSecrets

‎src/fdosecrets/objects/Prompt.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,7 @@ namespace FdoSecrets
211211
PromptResult promptSync(const DBusClientPtr& client, const QString& windowId) override;
212212
QVariant currentResult() const override;
213213

214+
DBusResult createItem(const QString& windowId);
214215
DBusResult updateItem();
215216

216217
QPointer<Collection> m_coll;

‎src/fdosecrets/objects/Service.cpp‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -523,18 +523,37 @@ namespace FdoSecrets
523523
return;
524524
}
525525

526-
// mark the db as being unlocked to prevent multiple dialogs for the same db
526+
// check if the db is already being unlocked to prevent multiple dialogs for the same db
527527
if (m_unlockingDb.contains(dbWidget)) {
528528
return;
529529
}
530-
m_unlockingDb.insert(dbWidget);
531530

531+
// insert a dummy one here, just to prevent multiple dialogs
532+
// the real one will be inserted in onDatabaseUnlockDialogFinished
533+
m_unlockingDb[dbWidget] = {};
534+
535+
// actually show the dialog
532536
m_databases->unlockDatabaseInDialog(dbWidget, DatabaseOpenDialog::Intent::None);
533537
}
534538

535539
void Service::onDatabaseUnlockDialogFinished(bool accepted, DatabaseWidget* dbWidget)
536540
{
537-
m_unlockingDb.remove(dbWidget);
538-
emit doneUnlockDatabaseInDialog(accepted, dbWidget);
541+
if (!m_unlockingDb.contains(dbWidget)) {
542+
// not our concern
543+
return;
544+
}
545+
546+
if (!accepted) {
547+
emit doneUnlockDatabaseInDialog(false, dbWidget);
548+
m_unlockingDb.remove(dbWidget);
549+
} else {
550+
// delay the done signal to when the database is actually done with unlocking
551+
// this is a oneshot connection to prevent superfluous signals
552+
auto conn = connect(dbWidget, &DatabaseWidget::databaseUnlocked, this, [dbWidget, this]() {
553+
emit doneUnlockDatabaseInDialog(true, dbWidget);
554+
disconnect(m_unlockingDb.take(dbWidget));
555+
});
556+
m_unlockingDb[dbWidget] = conn;
557+
}
539558
}
540559
} // namespace FdoSecrets

‎src/fdosecrets/objects/Service.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@ namespace FdoSecrets
128128
private slots:
129129
void ensureDefaultAlias();
130130

131+
void onDatabaseUnlockDialogFinished(bool accepted, DatabaseWidget* dbWidget);
131132
void onDatabaseTabOpened(DatabaseWidget* dbWidget, bool emitSignal);
132133
void monitorDatabaseExposedGroup(DatabaseWidget* dbWidget);
133134

@@ -136,8 +137,6 @@ namespace FdoSecrets
136137

137138
void onCollectionAliasRemoved(const QString& alias);
138139

139-
void onDatabaseUnlockDialogFinished(bool accepted, DatabaseWidget* dbWidget);
140-
141140
private:
142141
bool initialize();
143142

@@ -166,7 +165,8 @@ namespace FdoSecrets
166165
QList<Session*> m_sessions{};
167166

168167
bool m_insideEnsureDefaultAlias{false};
169-
QSet<const DatabaseWidget*> m_unlockingDb{}; // list of db being unlocking
168+
// list of db currently has unlock dialog shown
169+
QHash<const DatabaseWidget*, QMetaObject::Connection> m_unlockingDb{};
170170
QSet<const DatabaseWidget*> m_lockingDb{}; // list of db being locking
171171
};
172172

‎tests/gui/TestGuiFdoSecrets.cpp‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1094,6 +1094,31 @@ void TestGuiFdoSecrets::testItemCreate()
10941094
}
10951095
}
10961096

1097+
void TestGuiFdoSecrets::testItemCreateUnlock()
1098+
{
1099+
auto service = enableService();
1100+
VERIFY(service);
1101+
auto coll = getDefaultCollection(service);
1102+
VERIFY(coll);
1103+
auto sess = openSession(service, DhIetf1024Sha256Aes128CbcPkcs7::Algorithm);
1104+
VERIFY(sess);
1105+
1106+
// NOTE: entries are no longer valid after locking
1107+
lockDatabaseInBackend();
1108+
1109+
QSignalSpy spyItemCreated(coll.data(), SIGNAL(ItemCreated(QDBusObjectPath)));
1110+
VERIFY(spyItemCreated.isValid());
1111+
1112+
// create item
1113+
StringStringMap attributes{
1114+
{"application", "fdosecrets-test"},
1115+
{"attr-i[bute]", "![some] -value*"},
1116+
};
1117+
1118+
auto item = createItem(sess, coll, "abc", "Password", attributes, false, false, true);
1119+
VERIFY(item);
1120+
}
1121+
10971122
void TestGuiFdoSecrets::testItemChange()
10981123
{
10991124
auto service = enableService();
@@ -1678,7 +1703,8 @@ QSharedPointer<ItemProxy> TestGuiFdoSecrets::createItem(const QSharedPointer<Ses
16781703
const QString& pass,
16791704
const StringStringMap& attr,
16801705
bool replace,
1681-
bool expectPrompt)
1706+
bool expectPrompt,
1707+
bool expectUnlockPrompt)
16821708
{
16831709
VERIFY(sess);
16841710
VERIFY(coll);
@@ -1703,6 +1729,10 @@ QSharedPointer<ItemProxy> TestGuiFdoSecrets::createItem(const QSharedPointer<Ses
17031729

17041730
// drive the prompt
17051731
DBUS_VERIFY(prompt->Prompt(""));
1732+
1733+
bool unlockFound = driveUnlockDialog();
1734+
COMPARE(unlockFound, expectUnlockPrompt);
1735+
17061736
bool found = driveAccessControlDialog();
17071737
COMPARE(found, expectPrompt);
17081738

@@ -1800,6 +1830,9 @@ bool TestGuiFdoSecrets::driveUnlockDialog()
18001830
processEvents();
18011831
auto dbOpenDlg = m_tabWidget->findChild<DatabaseOpenDialog*>();
18021832
VERIFY(dbOpenDlg);
1833+
if (!dbOpenDlg->isVisible()) {
1834+
return false;
1835+
}
18031836
auto editPassword = dbOpenDlg->findChild<PasswordWidget*>("editPassword")->findChild<QLineEdit*>("passwordEdit");
18041837
VERIFY(editPassword);
18051838
editPassword->setFocus();

‎tests/gui/TestGuiFdoSecrets.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ private slots:
8484
void testCollectionChange();
8585

8686
void testItemCreate();
87+
void testItemCreateUnlock();
8788
void testItemChange();
8889
void testItemReplace();
8990
void testItemReplaceExistingLocked();
@@ -122,7 +123,8 @@ private slots:
122123
const QString& pass,
123124
const FdoSecrets::wire::StringStringMap& attr,
124125
bool replace,
125-
bool expectPrompt = false);
126+
bool expectPrompt = false,
127+
bool expectUnlockPrompt = false);
126128
FdoSecrets::wire::Secret
127129
encryptPassword(QByteArray value, QString contentType, const QSharedPointer<SessionProxy>& sess);
128130
template <typename Proxy> QSharedPointer<Proxy> getProxy(const QDBusObjectPath& path) const

0 commit comments

Comments
 (0)