Commit e93dc94a authored by Stefano Crocco's avatar Stefano Crocco Committed by David Faure
Browse files

Avoid having more than one certificate error dialog at the same time in the same window

Because QDialog::open is asynchronous, if a page generated several 
certificate errors, they could be shown at the same time, which could
make it difficult for the user to interact with them.

To avoid this, now dialogs belonging to the same window are queued so
that there can't be more than one of them visible at the same time.
parent 407803ac
Pipeline #117161 passed with stage
in 3 minutes and 54 seconds
......@@ -35,6 +35,7 @@ set(kwebenginepartlib_LIB_SRCS
spellcheckermanager.cpp
webenginepartcontrols.cpp
webenginepartcertificateerrordlg.cpp
certificateerrordialogmanager.cpp
)
ki18n_wrap_ui(kwebenginepartlib_LIB_SRCS webenginecustomizecacheablefieldsdlg.ui ui/credentialsdetailswidget.ui webenginepartcertificateerrordlg.ui)
......
/*
This file is part of the KDE project.
SPDX-FileCopyrightText: 2021 Stefano Crocco <stefano.crocco@alice.it>
SPDX-License-Identifier: GPL-2.0-only OR GPL-3.0-only OR LicenseRef-KDE-Accepted-GPL
*/
#include "certificateerrordialogmanager.h"
#include "webenginepage.h"
#include "webenginepartcertificateerrordlg.h"
#include "webengineview.h"
#include <algorithm>
#include <QWebEngineCertificateError>
#include <KSharedConfig>
#include <KConfigGroup>
using namespace KonqWebEnginePart;
CertificateErrorDialogManager::CertificateErrorDialogManager(QObject *parent) : QObject(parent)
{
}
CertificateErrorDialogManager::~CertificateErrorDialogManager()
{
}
bool CertificateErrorDialogManager::handleCertificateError(const QWebEngineCertificateError& _ce, WebEnginePage* page)
{
QWebEngineCertificateError ce(_ce);
if (!ce.isOverridable()) {
ce.rejectCertificate();
return false;
}
bool ignore = userAlreadyChoseToIgnoreError(ce);
if (ignore) {
ce.ignoreCertificateError();
} else {
ce.defer();
QPointer<WebEnginePage> ptr(page);
CertificateErrorData data{ce, ptr};
if (!displayDialogIfPossible(data)) {
m_certificates.append(data);
}
}
return true;
}
bool CertificateErrorDialogManager::userAlreadyChoseToIgnoreError(const QWebEngineCertificateError &ce)
{
int error = static_cast<int>(ce.error());
QString url = ce.url().url();
KConfigGroup grp(KSharedConfig::openConfig(), "CertificateExceptions");
QList<int> exceptionsForUrl = grp.readEntry(url, QList<int>{});
return (exceptionsForUrl.contains(error));
}
QWidget* CertificateErrorDialogManager::windowForPage(WebEnginePage* page)
{
if (page) {
QWidget *view = page->view();
if (view) {
return view->window();
}
}
return nullptr;
}
bool CertificateErrorDialogManager::displayDialogIfPossible(const CertificateErrorDialogManager::CertificateErrorData& data)
{
QWidget *window = windowForPage(data.page);
if (m_dialogs.contains(window)) {
return false;
} else {
displayDialog(data, window);
return true;
}
}
void CertificateErrorDialogManager::displayDialog(const CertificateErrorDialogManager::CertificateErrorData& data, QWidget *window)
{
if (!window) {
window = windowForPage(data.page);
}
Q_ASSERT(!m_dialogs.contains(window));
WebEnginePartCertificateErrorDlg *dlg = new WebEnginePartCertificateErrorDlg(data.error, data.page, window);
connect(dlg, &WebEnginePartCertificateErrorDlg::finished, this, [this, dlg](int){
applyUserChoice(dlg);});
connect(dlg, &WebEnginePartCertificateErrorDlg::destroyed, this, &CertificateErrorDialogManager::removeDestroyedDialog);
connect(window, &QWidget::destroyed, this, &CertificateErrorDialogManager::removeDestroyedWindow);
m_dialogs.insert(window, dlg);
dlg->open();
}
void CertificateErrorDialogManager::displayNextDialog(QWidget *window)
{
if (!window) {
return;
}
auto findNext = [window](const CertificateErrorData &data) {
return windowForPage(data.page) == window;
};
auto it = std::find_if(m_certificates.begin(), m_certificates.end(), findNext);
if (it == m_certificates.end()) {
return;
}
displayDialog(*it, window);
m_certificates.erase(it);
}
void CertificateErrorDialogManager::applyUserChoice(WebEnginePartCertificateErrorDlg *dlg)
{
QWebEngineCertificateError error = dlg->certificateError();
WebEnginePartCertificateErrorDlg::UserChoice choice = dlg->userChoice();
if (choice == WebEnginePartCertificateErrorDlg::UserChoice::DontIgnoreError) {
error.rejectCertificate();
} else {
error.ignoreCertificateError();
if (choice == WebEnginePartCertificateErrorDlg::UserChoice::IgnoreErrorForever) {
recordIgnoreForeverChoice(error);
}
}
dlg->deleteLater();
}
void CertificateErrorDialogManager::removeDestroyedDialog(QObject *dlg)
{
auto findItemForDialog = [dlg](const std::pair<QObject*, QObject*>& pair){return pair.second == dlg;};
auto it = std::find_if(m_dialogs.constKeyValueBegin(), m_dialogs.constKeyValueEnd(), findItemForDialog);
if (it == m_dialogs.constKeyValueEnd()) {
return;
}
QWidget *window = qobject_cast<QWidget*>(it->first);
m_dialogs.remove(it->first);
if (window) {
disconnect(window, nullptr, this, nullptr);
displayNextDialog(window);
}
}
void CertificateErrorDialogManager::removeDestroyedWindow(QObject *window)
{
if (!window) {
return;
}
m_dialogs.remove(window);
}
void CertificateErrorDialogManager::recordIgnoreForeverChoice(const QWebEngineCertificateError& ce)
{
KConfigGroup grp(KSharedConfig::openConfig(), "CertificateExceptions");
QString url = ce.url().url();
QList<int> exceptionsForUrl = grp.readEntry(url, QList<int>{});
exceptionsForUrl.append(ce.error());
grp.writeEntry(url, exceptionsForUrl);
grp.sync();
}
/*
This file is part of the KDE project.
SPDX-FileCopyrightText: 2021 Stefano Crocco <stefano.crocco@alice.it>
SPDX-License-Identifier: GPL-2.0-only OR GPL-3.0-only OR LicenseRef-KDE-Accepted-GPL
*/
#ifndef WEBENGINEPART_CERTIFICATEERRORDIALOGMANAGER_H
#define WEBENGINEPART_CERTIFICATEERRORDIALOGMANAGER_H
#include <QObject>
#include <QWebEngineCertificateError>
#include <QPointer>
class WebEnginePage;
namespace KonqWebEnginePart {
class WebEnginePartCertificateErrorDlg;
/**
* @brief Class which takes care of queuing dialogs reporting certificate errors.
*
* Since calls to WebEnginePage::certificateError are made asynchronously, without
* queuing it could happen that a second dialog is shown when there's another one
* still visible. Since the dialogs are modal and they can cover each other, this can
* cause issues for the user. To avoid this, this class ensures that only dialogs from
* different windows can be shown at the same time.
*
* @internal The whole issue is caused by the use of @c QDialog::open instead of
* @c QDialog::exec to display the dialog: since @c exec is synchronous, it will prevent
* displaying a second dialog before the first has been closed.
*/
class CertificateErrorDialogManager : public QObject
{
Q_OBJECT
public:
/**
* @brief Constructor
*
* @param parent the parent object
*/
CertificateErrorDialogManager(QObject *parent=nullptr);
/**
* @brief Destructor
*
*/
~CertificateErrorDialogManager();
/**
* @brief Displays a dialog regarding the certificate error or queues the certificate error
*
* If there's no currently displayed certificate error dialog for the window containing the page,
* the dialog for the new certificate is immediately shown, otherwise the certificate error
* is put in queue waiting to be displayed as soon as there aren't other dialogs for that
* window.
*
* In all cases, the appropriate function is called on the certificate: @c ignoreCertificateError
* or @c rejectCertificate if the dialog was displayed immediately or @c defer if it was queued.
* @param _ce the certificate error
* @param page the page which caused the certificate error
* @return @b true if the certificate was overridable and @b false if it wasn't overridable.
* @note In case the certificate wasn't overridable, @c rejectCertificate will be called on the
* certificate.
*/
bool handleCertificateError(const QWebEngineCertificateError &_ce, WebEnginePage *page);
private:
/**
* @brief Struct used to encapuslate information about certificate errors
*/
struct CertificateErrorData {
/**
* @brief The certificate error
*/
QWebEngineCertificateError error;
/**
* @brief The page which has caused the certificate error
*/
QPointer<WebEnginePage> page;
};
/**
* @brief Whether the decision to always ignore the given error is recorded in the user settings
*
* @param ce the certificate error
* @return @b true if the user has decided to always ignore the error and @b false otherwise
*/
static bool userAlreadyChoseToIgnoreError(const QWebEngineCertificateError &ce);
/**
* @brief Records the user's choice to forever ignore the given error
*
* @param ce the error
*/
static void recordIgnoreForeverChoice(const QWebEngineCertificateError &ce);
/**
* @brief The window (toplevel widget) associated to the given page
*
* @param page the page to retrieve the window for
* @return the window associated with @p page or @b nullptr if either @p page or its @c view are @b nullptr
*/
static QWidget* windowForPage(WebEnginePage *page);
/**
* @brief Displays a dialog for the given certificate error unless a certificate error dialog
* for the same window is already visible.
*
* @param data the data describing the certificate error
* @return @b true if the dialog was displayed and @b false if it wasn't
* @note the dialog is displayed asynchronously
*/
bool displayDialogIfPossible(const CertificateErrorData &data);
/**
* @brief Displays a dialog for the given certificate error
*
* The dialog is displayed asynchronously (using @c QDialog::open). The dialog result is used
* in dialogClosed.
*
* @warning This function doesn't check whether a dialog for the given window
* is already visible: it assumes that such check has been done by the caller.
* @warning This function @b doesn't use windowForPage to determine which window the page belongs to:
* it always uses @p window.
*
* @internal This function stores the new dialog and the corresponding window in #m_dialogs
*
* @param data the data describing the certificate error
* @param window the window the dialog should be displayed in
*/
void displayDialog(const CertificateErrorData &data, QWidget *window);
private slots:
/**
* @brief Removes a dialog from the list and displays the next dialog (if any) for the same window
*
* @param obj the dialog to remove. It is a @c QObject rather than a WebEnginePartCertificateErrorDlg
* because this slot is called in response to the dialog @c destroyed signal, which has a @c QObject* as argument
*/
void removeDestroyedDialog(QObject *dlg);
/**
* @brief Removes any dialog from the given window from the list
*
* @param obj the window. It is a @c QObject rather than a @c QWidget
* because this slot is called in response to the window @c destroyed signal, which has a @c QObject* as argument
*/
void removeDestroyedWindow(QObject *window);
/**
* @brief Applies the choice made by the user in the given dialog
*
* This function is called in response to the dialog's @b finished signal.
* It calls @c ignoreCertificateError or @c rejectCertificate on the certificate error (retrieved using
* WebEnginePartCertificateErrorDlg::certificateError) and, if the user chose to always ignore the error,
* records the choice.
*
* This function also deletes the dialog (using @c deleteLater).
*
* @note This function @b doesn't display the next dialog for the associated window: that task is left to
* removeDestroyedDialog, which is called in response to the dialog destruction.
*
* @param dlg the dialog
*/
void applyUserChoice(WebEnginePartCertificateErrorDlg *dlg);
/**
* @brief Displays the certificate error dialog for the next error caused by a page associated with the given window
*
* If no such certificate error exists, nothing is done.
*
* @param window the window to display the next error for.
*/
void displayNextDialog(QWidget *window);
private:
/**
* @brief A list of all the queued certificate errors together with the corresponding pages
*
* When a dialog for a certificate error is created, the corresponding entry is removed from the list
* @internal The entry is removed when the dialog is created, not when it's destroyed
*
*/
QVector<CertificateErrorData> m_certificates;
/**
* @brief A hash associating each certificate error dialog with its window
*
* If the window is destroyed, the corresponding entry is removed from the list.
* @note Keys and values are @c QObject* rather than more specific types because they're used
* in slots connected with @c QObject::destroyed signals, whose argument is a @c QObject*.
*/
QHash<QObject*, QObject*> m_dialogs;
};
}
#endif // WEBENGINEPART_CERTIFICATEERRORDIALOGMANAGER_H
......@@ -16,7 +16,7 @@
#include "webenginepartdownloadmanager.h"
#include "webenginewallet.h"
#include <webenginepart_debug.h>
#include "webenginepartcertificateerrordlg.h"
#include "webenginepartcontrols.h"
#include <QWebEngineCertificateError>
#include <QWebEngineSettings>
......@@ -393,47 +393,9 @@ static int errorCodeFromReply(QNetworkReply* reply)
}
#endif
bool WebEnginePage::certificateError(const QWebEngineCertificateError& _ce)
bool WebEnginePage::certificateError(const QWebEngineCertificateError& ce)
{
QWebEngineCertificateError ce(_ce);
if (!ce.isOverridable()) {
return false;
}
int error = static_cast<int>(ce.error());
QString url = ce.url().url();
KConfigGroup grp(KSharedConfig::openConfig(), "CertificateExceptions");
QList<int> exceptionsForUrl = grp.readEntry(url, QList<int>{});
if (exceptionsForUrl.contains(error)) {
return true;
} else {
ce.defer();
WebEnginePartCertificateErrorDlg *dlg = new WebEnginePartCertificateErrorDlg(ce, view());
connect(dlg, &WebEnginePartCertificateErrorDlg::finished, this, [this, dlg](int _){handleCertificateError(dlg);});
dlg->open();
return true;
}
}
void WebEnginePage::handleCertificateError(WebEnginePartCertificateErrorDlg *dlg) {
if (!dlg) {
return;
}
QWebEngineCertificateError error = dlg->certificateError();
WebEnginePartCertificateErrorDlg::UserChoice choice = dlg->userChoice();
dlg->deleteLater();
if (choice == WebEnginePartCertificateErrorDlg::UserChoice::DontIgnoreError) {
error.rejectCertificate();
} else {
error.ignoreCertificateError();
if (choice == WebEnginePartCertificateErrorDlg::UserChoice::IgnoreErrorForever) {
KConfigGroup grp(KSharedConfig::openConfig(), "CertificateExceptions");
QString url = error.url().url();
QList<int> exceptionsForUrl = grp.readEntry(url, QList<int>{});
exceptionsForUrl.append(error.error());
grp.writeEntry(url, exceptionsForUrl);
grp.sync();
}
}
return WebEnginePartControls::self()->handleCertificateError(ce, this);
}
WebEnginePart* WebEnginePage::part() const
......
......@@ -27,7 +27,6 @@ class WebSslInfo;
class WebEnginePart;
class KPasswdServerClient;
class WebEngineWallet;
class WebEnginePartCertificateErrorDlg;
class WebEnginePage : public QWebEnginePage
{
......@@ -133,17 +132,6 @@ private:
bool handleMailToUrl (const QUrl& , NavigationType type) const;
void setPageJScriptPolicy(const QUrl& url);
/**
* @brief Function called in response to the user closing the certificate error dialog
*
* Depending on the user's choice, this function will instruct the page to ignore the error or not
* and, if the user chose to forever ignore the error, it'll record this decision in the configuration
* file.
*
* @param dlg the certificate error dialog
*/
void handleCertificateError(WebEnginePartCertificateErrorDlg *dlg);
/**
* @brief Function called when the part is forced to save an URL to disk.
*
......
......@@ -23,12 +23,16 @@
#include "webenginepartcertificateerrordlg.h"
#include "ui_webenginepartcertificateerrordlg.h"
#include "webenginepage.h"
#include <KSslCertificateBox>
#include <QAbstractButton>
#include <QPushButton>
WebEnginePartCertificateErrorDlg::WebEnginePartCertificateErrorDlg(const QWebEngineCertificateError &error, QWidget* parent):
using namespace KonqWebEnginePart;
WebEnginePartCertificateErrorDlg::WebEnginePartCertificateErrorDlg(const QWebEngineCertificateError &error, WebEnginePage *page, QWidget* parent):
QDialog(parent),
m_ui(new Ui::WebEnginePartCertificateErrorDlg), m_error(error), m_choice(UserChoice::DontIgnoreError)
{
......@@ -43,12 +47,14 @@ WebEnginePartCertificateErrorDlg::WebEnginePartCertificateErrorDlg(const QWebEng
m_ui->details->hide();
QString translatedDesc = i18n(m_error.errorDescription().toUtf8());
QString text = i18n("<p>The server failed the authenticity check (%1). The error is:</p><p><tt>%2</tt></p>Do you want to ignore this error?",
qDebug() << "URL" << page->url();
QString text = i18n("<p>The server <tt>%1</tt> failed the authenticity check. The error is:</p><p><tt>%2</tt></p>Do you want to ignore this error?",
m_error.url().host(), translatedDesc);
m_ui->label->setText(text);
for (const QSslCertificate &cert : m_error.certificateChain()) {
m_ui->certificateChain->addItem(cert.subjectDisplayName());
}
setWindowTitle(i18nc("title of a dialog asking what to do about a SSL certificate error", "Certificate error"));
}
WebEnginePartCertificateErrorDlg::~WebEnginePartCertificateErrorDlg()
......
......@@ -23,6 +23,8 @@
#ifndef WEBENGINEPARTCERTIFICATEERRORDLG_H
#define WEBENGINEPARTCERTIFICATEERRORDLG_H
#include "webenginepage.h"
#include <QDialog>
#include <QWebEngineCertificateError>
......@@ -33,28 +35,85 @@ namespace Ui
class WebEnginePartCertificateErrorDlg;
}
/**
* @todo write docs
*/
class WebEnginePartCertificateErrorDlg : public QDialog
{
Q_OBJECT
public:
WebEnginePartCertificateErrorDlg(const QWebEngineCertificateError& error, QWidget* parent);
~WebEnginePartCertificateErrorDlg();
enum class UserChoice{DontIgnoreError, IgnoreErrorOnce, IgnoreErrorForever};
UserChoice userChoice() const;
QWebEngineCertificateError certificateError() const;
private slots:
void displayCertificate(int idx);
void updateUserChoice(QAbstractButton *btn);
private:
Ui::WebEnginePartCertificateErrorDlg *m_ui;
QWebEngineCertificateError m_error;
UserChoice m_choice;
};
namespace KonqWebEnginePart {
/**
* @brief Dialog which asks the user whether to ignore a SSL certificate error or not.
*
* The user can choose to ignore the error only once, forever or to not ignore it
*/
class WebEnginePartCertificateErrorDlg : public QDialog
{
Q_OBJECT
public:
/**
* @brief Constructor
* @param error the certificate error the dialog is about
* @param parent the parent widget
*/
WebEnginePartCertificateErrorDlg(const QWebEngineCertificateError& error, WebEnginePage* page, QWidget* parent);
/**
* @brief Destructor
*/
~WebEnginePartCertificateErrorDlg();
/**
* @brief Enum which describes the possible user choices
*/
enum class UserChoice{DontIgnoreError, IgnoreErrorOnce, IgnoreErrorForever};
/**
* @brief The choice made by the user
* @return the choice made by the user
* @warning This method should only be called *after* the user closed the dialog
* using one of the three buttons. In all other situations, the returned value is
* undefined.
*/
UserChoice userChoice() const;
/**
* @return The certificate error the dialog is about
*/
QWebEngineCertificateError certificateError() const;
private slots:
/**
* @brief Displays information about a certificate in the certificate chain
* @param idx the index of the certificate in the certificate chain
*/
void displayCertificate(int idx);
/**
* @brief Sets the variable containing the user's choice when he presses one of the dialog buttons
*
* @param btn the button pressed by the user
*/
void updateUserChoice(QAbstractButton *btn);
private:
/**
* @brief The Ui object
*/
Ui::WebEnginePartCertificateErrorDlg *m_ui;
/**
* @brief The error the dialog is about
*/