From b81224b1b6f92db3ac1fd5eab6e64d3cd2aa4e7e Mon Sep 17 00:00:00 2001 From: Raul Metsma Date: Thu, 28 Dec 2023 13:08:32 +0200 Subject: [PATCH] Don't add empty certificates to history IB-7886 Signed-off-by: Raul Metsma --- client/dialogs/AddRecipients.cpp | 131 +++----------------------- client/dialogs/AddRecipients.h | 11 +-- client/dialogs/CertificateHistory.cpp | 131 +++++++++++++++++++++++--- client/dialogs/CertificateHistory.h | 26 ++++- 4 files changed, 156 insertions(+), 143 deletions(-) diff --git a/client/dialogs/AddRecipients.cpp b/client/dialogs/AddRecipients.cpp index 19a5d2896..24675aa7b 100644 --- a/client/dialogs/AddRecipients.cpp +++ b/client/dialogs/AddRecipients.cpp @@ -38,12 +38,8 @@ #include #include #include -#include -#include -#include #include #include -#include #include AddRecipients::AddRecipients(ItemList* itemList, QWidget *parent) @@ -95,28 +91,6 @@ AddRecipients::AddRecipients(ItemList* itemList, QWidget *parent) connect(ui->fromFile, &QPushButton::clicked, this, &AddRecipients::addRecipientFromFile); connect(ui->fromHistory, &QPushButton::clicked, this, &AddRecipients::addRecipientFromHistory); - QFile f( path() ); - if( !f.open( QIODevice::ReadOnly ) ) - return; - - QXmlStreamReader xml( &f ); - if(!xml.readNextStartElement() || xml.name() != QLatin1String("History")) - return; - - while( xml.readNextStartElement() ) - { - if(xml.name() == QLatin1String("item")) - { - historyCertData.append({ - xml.attributes().value(QLatin1String("CN")).toString(), - xml.attributes().value(QLatin1String("type")).toString(), - xml.attributes().value(QLatin1String("issuer")).toString(), - xml.attributes().value(QLatin1String("expireDate")).toString() - }); - } - xml.skipCurrentElement(); - } - for(Item *item: itemList->items) addRecipientToRightPane((qobject_cast(item))->getKey(), false); } @@ -129,28 +103,27 @@ AddRecipients::~AddRecipients() void AddRecipients::addAllRecipientToRightPane() { - QList history; + QList history; for(AddressItem *value: leftList) { - if(!rightList.contains(value->getKey())) - { - addRecipientToRightPane(value); - history.append(toHistory(value->getKey().cert)); - } + if(rightList.contains(value->getKey())) + continue; + addRecipientToRightPane(value); + history.append(value->getKey().cert); } ui->confirm->setDisabled(rightList.isEmpty()); - rememberCerts(history); + historyCertData.addAndSave(history); } void AddRecipients::addRecipientFromCard() { - if(AddressItem *item = addRecipientToLeftPane(qApp->signer()->tokenauth().cert())) + if(auto *item = addRecipientToLeftPane(qApp->signer()->tokenauth().cert())) addRecipientToRightPane(item, true); } void AddRecipients::addRecipientFromFile() { - QString file = FileDialog::getOpenFileName( this, windowTitle(), QString(), + QString file = FileDialog::getOpenFileName(this, windowTitle(), {}, tr("Certificates (*.cer *.crt *.pem)") ); if( file.isEmpty() ) return; @@ -177,7 +150,7 @@ void AddRecipients::addRecipientFromFile() { WarningDialog::show(this, tr("This certificate cannot be used for encryption")); } - else if(AddressItem *item = addRecipientToLeftPane(cert)) + else if(auto *item = addRecipientToLeftPane(cert)) { addRecipientToRightPane(item, true); } @@ -186,9 +159,7 @@ void AddRecipients::addRecipientFromFile() void AddRecipients::addRecipientFromHistory() { auto *dlg = new CertificateHistory(historyCertData, this); - dlg->setAttribute(Qt::WA_DeleteOnClose); connect(dlg, &CertificateHistory::addSelectedCerts, this, &AddRecipients::addSelectedCerts); - connect(dlg, &CertificateHistory::removeSelectedCerts, this, &AddRecipients::removeSelectedCerts); dlg->open(); } @@ -222,8 +193,7 @@ bool AddRecipients::addRecipientToRightPane(const CKey &key, bool update) if(update) { - auto expiryDate = key.cert.expiryDate(); - if(expiryDate <= QDateTime::currentDateTime()) + if(auto expiryDate = key.cert.expiryDate(); expiryDate <= QDateTime::currentDateTime()) { if(Settings::CDOC2_DEFAULT && Settings::CDOC2_USE_KEYSERVER) { @@ -260,7 +230,7 @@ bool AddRecipients::addRecipientToRightPane(const CKey &key, bool update) connect(rightItem, &AddressItem::remove, this, &AddRecipients::removeRecipientFromRightPane); ui->rightPane->addWidget(rightItem); ui->confirm->setDisabled(rightList.isEmpty()); - rememberCerts({toHistory(key.cert)}); + historyCertData.addAndSave({key.cert}); return true; } @@ -311,31 +281,6 @@ QList AddRecipients::keys() return recipients; } -QString AddRecipients::path() -{ -#ifdef Q_OS_WIN - QSettings s( QSettings::IniFormat, QSettings::UserScope, Application::organizationName(), "qdigidoccrypto" ); - QFileInfo f( s.fileName() ); - return f.absolutePath() + "/" + f.baseName() + "/certhistory.xml"; -#else - return QStandardPaths::writableLocation(QStandardPaths::AppDataLocation) + QStringLiteral("/certhistory.xml"); -#endif -} - -void AddRecipients::rememberCerts(const QList& selectedCertData) -{ - if(selectedCertData.isEmpty()) - return; - - for(const auto &certData: selectedCertData) - { - if(!historyCertData.contains(certData)) - historyCertData.append(certData); - } - - saveHistory(); -} - void AddRecipients::removeRecipientFromRightPane(Item *toRemove) { auto *rightItem = qobject_cast(toRemove); @@ -349,39 +294,6 @@ void AddRecipients::removeRecipientFromRightPane(Item *toRemove) ui->confirm->setDisabled(rightList.isEmpty()); } -void AddRecipients::removeSelectedCerts(const QList& removeCertData) -{ - if(removeCertData.isEmpty()) - return; - for(const auto &remove: removeCertData) - historyCertData.removeOne(remove); - saveHistory(); -} - -void AddRecipients::saveHistory() -{ - QString p = path(); - QDir().mkpath( QFileInfo( p ).absolutePath() ); - QFile f( p ); - if( !f.open( QIODevice::WriteOnly|QIODevice::Truncate ) ) - return; - - QXmlStreamWriter xml( &f ); - xml.setAutoFormatting( true ); - xml.writeStartDocument(); - xml.writeStartElement(QStringLiteral("History")); - for(const HistoryCertData& certData : historyCertData) - { - xml.writeStartElement(QStringLiteral("item")); - xml.writeAttribute(QStringLiteral("CN"), certData.CN); - xml.writeAttribute(QStringLiteral("type"), certData.type); - xml.writeAttribute(QStringLiteral("issuer"), certData.issuer); - xml.writeAttribute(QStringLiteral("expireDate"), certData.expireDate); - xml.writeEndElement(); - } - xml.writeEndDocument(); -} - void AddRecipients::search(const QString &term, bool select, const QString &type) { QApplication::setOverrideCursor(Qt::WaitCursor); @@ -449,7 +361,7 @@ void AddRecipients::showResult(const QList &result, int resultC isEmpty = false; AddressItem *item = addRecipientToLeftPane(k); if(userData.value(QStringLiteral("select"), false).toBool() && - (userData.value(QStringLiteral("type")).isNull() || toType(SslCertificate(k)) == userData[QStringLiteral("type")])) + (userData.value(QStringLiteral("type")).isNull() || HistoryCertData::toType(SslCertificate(k)) == userData[QStringLiteral("type")])) addRecipientToRightPane(item, true); } } @@ -463,22 +375,3 @@ void AddRecipients::showResult(const QList &result, int resultC } QApplication::restoreOverrideCursor(); } - -HistoryCertData AddRecipients::toHistory(const SslCertificate &cert) -{ - return { - cert.subjectInfo("CN"), - toType(cert), - cert.issuerInfo("CN"), - cert.expiryDate().toLocalTime().toString(QStringLiteral("dd.MM.yyyy")), - }; -} - -QString AddRecipients::toType(const SslCertificate &cert) -{ - auto certType = cert.type(); - if(certType & SslCertificate::DigiIDType) return QString::number(2); - if(certType & SslCertificate::TempelType) return QString::number(1); - if(certType & SslCertificate::EstEidType) return QString::number(0); - return QString::number(3); -} diff --git a/client/dialogs/AddRecipients.h b/client/dialogs/AddRecipients.h index fbe907bf3..4f9e5d2d6 100644 --- a/client/dialogs/AddRecipients.h +++ b/client/dialogs/AddRecipients.h @@ -52,24 +52,15 @@ class AddRecipients final : public QDialog AddressItem * addRecipientToLeftPane(const QSslCertificate& cert); bool addRecipientToRightPane(const CKey &key, bool update = true); void addRecipientToRightPane(AddressItem *leftItem, bool update = true); - void addSelectedCerts(const QList& selectedCertData); - void enableRecipientFromCard(); - - void rememberCerts(const QList& selectedCertData); void removeRecipientFromRightPane(Item *toRemove); - void removeSelectedCerts(const QList& removeCertData); - void saveHistory(); void search(const QString &term, bool select = false, const QString &type = {}); void showError(const QString &msg, const QString &details = {}); void showResult(const QList &result, int resultCount, const QVariantMap &userData); static QString defaultUrl(QLatin1String key, const QString &defaultValue); - static QString path(); - static HistoryCertData toHistory(const SslCertificate &cert); - static QString toType(const SslCertificate &cert) ; Ui::AddRecipients *ui; QHash leftList; @@ -77,5 +68,5 @@ class AddRecipients final : public QDialog LdapSearch *ldap_person, *ldap_corp; bool updated = false; - QList historyCertData; + HistoryList historyCertData; }; diff --git a/client/dialogs/CertificateHistory.cpp b/client/dialogs/CertificateHistory.cpp index 4125004d8..385dce2be 100644 --- a/client/dialogs/CertificateHistory.cpp +++ b/client/dialogs/CertificateHistory.cpp @@ -20,7 +20,16 @@ #include "CertificateHistory.h" #include "ui_CertificateHistory.h" +#include "Application.h" #include "Styles.h" +#include "SslCertificate.h" + +#include +#include +#include +#include +#include +#include bool HistoryCertData::operator==(const HistoryCertData& other) const { @@ -30,28 +39,128 @@ bool HistoryCertData::operator==(const HistoryCertData& other) const expireDate == other.expireDate; } +QString HistoryCertData::toType(const SslCertificate &cert) +{ + auto certType = cert.type(); + if(certType & SslCertificate::DigiIDType) return QString::number(CertificateHistory::DigiID); + if(certType & SslCertificate::TempelType) return QString::number(CertificateHistory::TEMPEL); + if(certType & SslCertificate::EstEidType) return QString::number(CertificateHistory::IDCard); + return QString::number(CertificateHistory::Other); +} + QString HistoryCertData::typeName() const { switch (type.toInt()) { - case CertificateHistory::DigiID: - return CertificateHistory::tr("Digi-ID"); - case CertificateHistory::TEMPEL: - return CertificateHistory::tr("Certificate for Encryption"); - case CertificateHistory::IDCard: - return CertificateHistory::tr("ID-card"); - default: - return CertificateHistory::tr("Other"); + case CertificateHistory::DigiID: return CertificateHistory::tr("Digi-ID"); + case CertificateHistory::TEMPEL: return CertificateHistory::tr("Certificate for Encryption"); + case CertificateHistory::IDCard: return CertificateHistory::tr("ID-card"); + default: return CertificateHistory::tr("Other"); + } +} + +HistoryList::HistoryList() +{ +#ifdef Q_OS_WIN + QSettings s(QSettings::IniFormat, QSettings::UserScope, Application::organizationName(), QStringLiteral("qdigidoccrypto")); + QFileInfo f(s.fileName()); + path = f.absolutePath() + '/' + f.baseName() + QLatin1String("/certhistory.xml"); +#else + path = QStandardPaths::writableLocation(QStandardPaths::AppDataLocation) + QLatin1String("/certhistory.xml"); +#endif + + QFile f(path); + if(!f.open(QIODevice::ReadOnly)) + return; + + QXmlStreamReader xml(&f); + if(!xml.readNextStartElement() || xml.name() != QLatin1String("History")) + return; + + while(xml.readNextStartElement()) + { + if(xml.name() == QLatin1String("item")) + { + append({ + xml.attributes().value(QLatin1String("CN")).toString(), + xml.attributes().value(QLatin1String("type")).toString(), + xml.attributes().value(QLatin1String("issuer")).toString(), + xml.attributes().value(QLatin1String("expireDate")).toString() + }); + } + xml.skipCurrentElement(); + } +} + +void HistoryList::addAndSave(const QList &data) +{ + if(data.isEmpty()) + return; + bool changed = false; + for(const auto &cert: data) + { + if(cert.isNull()) + continue; + HistoryCertData certData{ + cert.subjectInfo("CN"), + HistoryCertData::toType(cert), + cert.issuerInfo("CN"), + cert.expiryDate().toLocalTime().toString(QStringLiteral("dd.MM.yyyy")), + }; + if(contains(certData)) + continue; + append(std::move(certData)); + changed = true; + } + if(changed) + save(); +} + +void HistoryList::removeAndSave(const QList &data) +{ + if(data.isEmpty()) + return; + bool changed = false; + for(const auto &item: data) + changed = std::max(changed, removeOne(item)); + if(changed) + save(); +} + +void HistoryList::save() +{ + QString p = path; + QDir().mkpath(QFileInfo(p).absolutePath()); + QFile f(p); + if(!f.open(QIODevice::WriteOnly|QIODevice::Truncate)) + return; + + QXmlStreamWriter xml(&f); + xml.setAutoFormatting(true); + xml.writeStartDocument(); + xml.writeStartElement(QStringLiteral("History")); + for(const HistoryCertData& certData : *this) + { + xml.writeStartElement(QStringLiteral("item")); + xml.writeAttribute(QStringLiteral("CN"), certData.CN); + xml.writeAttribute(QStringLiteral("type"), certData.type); + xml.writeAttribute(QStringLiteral("issuer"), certData.issuer); + xml.writeAttribute(QStringLiteral("expireDate"), certData.expireDate); + xml.writeEndElement(); } + xml.writeEndDocument(); } -CertificateHistory::CertificateHistory(QList &_historyCertData, QWidget *parent) + + +CertificateHistory::CertificateHistory(HistoryList &_historyCertData, QWidget *parent) : QDialog( parent ) , ui(new Ui::CertificateHistory) , historyCertData(_historyCertData) { ui->setupUi(this); setWindowFlags( Qt::Dialog | Qt::CustomizeWindowHint ); + setAttribute(Qt::WA_DeleteOnClose); setMinimumSize(parent->frameSize()); move(parent->frameGeometry().center() - frameGeometry().center()); @@ -73,7 +182,7 @@ CertificateHistory::CertificateHistory(QList &_historyCertData, }); connect(ui->select, &QPushButton::clicked, this, &CertificateHistory::reject); connect(ui->remove, &QPushButton::clicked, this, [this]{ - emit removeSelectedCerts(selectedItems()); + historyCertData.removeAndSave(selectedItems()); fillView(); }); connect(ui->view, &QTreeWidget::itemActivated, ui->select, &QPushButton::clicked); @@ -98,7 +207,7 @@ void CertificateHistory::fillView() ui->view->header()->setSortIndicatorShown(!historyCertData.isEmpty()); for(const HistoryCertData& certData : historyCertData) { - QTreeWidgetItem *i = new QTreeWidgetItem( ui->view ); + auto *i = new QTreeWidgetItem(ui->view); i->setText(0, certData.CN); i->setData(1, Qt::UserRole, certData.type); i->setText(1, certData.typeName()); diff --git a/client/dialogs/CertificateHistory.h b/client/dialogs/CertificateHistory.h index 51a1bb38c..adda94907 100644 --- a/client/dialogs/CertificateHistory.h +++ b/client/dialogs/CertificateHistory.h @@ -24,6 +24,7 @@ #include namespace Ui { class CertificateHistory; } +class SslCertificate; class HistoryCertData { @@ -33,10 +34,30 @@ class HistoryCertData QString issuer; QString expireDate; + static QString toType(const SslCertificate &cert); + bool operator==(const HistoryCertData& other) const; QString typeName() const; }; +#if QT_VERSION < QT_VERSION_CHECK(6, 0, 0) +class HistoryList: public QVector +#else +class HistoryList: public QList +#endif +{ +public: + HistoryList(); + + void addAndSave(const QList &data); + void removeAndSave(const QList &data); + +private: + void save(); + + QString path; +}; + class CertificateHistory: public QDialog { @@ -51,17 +72,16 @@ class CertificateHistory: public QDialog Other = 3 }; - CertificateHistory(QList& historyCertData, QWidget *parent = nullptr); + CertificateHistory(HistoryList &historyCertData, QWidget *parent = nullptr); ~CertificateHistory(); signals: void addSelectedCerts(const QList& selectedCertData); - void removeSelectedCerts(const QList& removeCertData); private: void fillView(); QList selectedItems() const; Ui::CertificateHistory *ui; - QList &historyCertData; + HistoryList &historyCertData; };