From 6a6ead8e6dc33338835346003b5661888a15c4f4 Mon Sep 17 00:00:00 2001 From: Xavier Michelon Date: Tue, 25 Apr 2023 18:39:23 +0200 Subject: [PATCH] feat(GODT-2540): notify user of wrong IMAP password. --- .../bridge-gui/bridge-gui/CMakeLists.txt | 3 +- internal/frontend/bridge-gui/bridge-gui/Pch.h | 1 + .../bridge-gui/bridge-gui/QMLBackend.cpp | 33 ++++++-- .../bridge-gui/bridge-gui/QMLBackend.h | 4 +- .../bridge-gui/bridge-gui/TrayIcon.cpp | 81 +++++++++++++++++-- .../frontend/bridge-gui/bridge-gui/TrayIcon.h | 4 +- .../bridge-gui/bridge-gui/qml/MainWindow.qml | 6 +- .../bridgepp/bridgepp/User/User.cpp | 20 ++--- .../bridge-gui/bridgepp/bridgepp/User/User.h | 12 ++- 9 files changed, 128 insertions(+), 36 deletions(-) diff --git a/internal/frontend/bridge-gui/bridge-gui/CMakeLists.txt b/internal/frontend/bridge-gui/bridge-gui/CMakeLists.txt index 77f06e49..2c14ed83 100644 --- a/internal/frontend/bridge-gui/bridge-gui/CMakeLists.txt +++ b/internal/frontend/bridge-gui/bridge-gui/CMakeLists.txt @@ -75,7 +75,7 @@ if(NOT UNIX) set(CMAKE_INSTALL_BINDIR ".") endif(NOT UNIX) -find_package(Qt6 COMPONENTS Core Quick Qml QuickControls2 Widgets REQUIRED) +find_package(Qt6 COMPONENTS Core Quick Qml QuickControls2 Widgets Svg REQUIRED) qt_standard_project_setup() set(CMAKE_AUTORCC ON) message(STATUS "Using Qt ${Qt6_VERSION}") @@ -147,6 +147,7 @@ target_link_libraries(bridge-gui Qt6::Quick Qt6::Qml Qt6::QuickControls2 + Qt6::Svg sentry::sentry bridgepp ) diff --git a/internal/frontend/bridge-gui/bridge-gui/Pch.h b/internal/frontend/bridge-gui/bridge-gui/Pch.h index b34e31d3..cb7bcb99 100644 --- a/internal/frontend/bridge-gui/bridge-gui/Pch.h +++ b/internal/frontend/bridge-gui/bridge-gui/Pch.h @@ -25,6 +25,7 @@ #include #include #include +#include #include diff --git a/internal/frontend/bridge-gui/bridge-gui/QMLBackend.cpp b/internal/frontend/bridge-gui/bridge-gui/QMLBackend.cpp index 2435426c..47e8e2db 100644 --- a/internal/frontend/bridge-gui/bridge-gui/QMLBackend.cpp +++ b/internal/frontend/bridge-gui/bridge-gui/QMLBackend.cpp @@ -994,15 +994,34 @@ void QMLBackend::onUserBadEvent(QString const &userID, QString const &) { void QMLBackend::onIMAPLoginFailed(QString const &username) { HANDLE_EXCEPTION( SPUser const user = users_->getUserWithUsernameOrEmail(username); - if ((!user) || (user->state() != UserState::SignedOut)) { // We want to pop-up only if a signed-out user has been detected + if (!user) { return; } - if (user->isInIMAPLoginFailureCooldown()) { - return; + + qint64 const cooldownDurationMs = 10 * 60 * 1000; // 10 minutes cooldown period for notifications + switch (user->state()) { + case UserState::SignedOut: + if (user->isNotificationInCooldown(User::ENotification::IMAPLoginWhileSignedOut)) { + return; + } + user->startNotificationCooldownPeriod(User::ENotification::IMAPLoginWhileSignedOut, cooldownDurationMs); + emit selectUser(user->id(), true); + emit imapLoginWhileSignedOut(username); + break; + + case UserState::Connected: + if (user->isNotificationInCooldown(User::ENotification::IMAPPasswordFailure)) { + return; + } + user->startNotificationCooldownPeriod(User::ENotification::IMAPPasswordFailure, cooldownDurationMs); + emit selectUser(user->id(), false); + trayIcon_->showErrorPopupNotification(tr("Incorrect password"), + tr("Your email client can't connect to Proton Bridge. Make sure you are using the local Bridge password shown in Bridge.")); + break; + + default: + break; } - user->startImapLoginFailureCooldown(60 * 60 * 1000); // 1 hour cooldown during which we will not display this notification to this user again. - emit selectUser(user->id()); - emit imapLoginWhileSignedOut(username); ) } @@ -1134,7 +1153,7 @@ void QMLBackend::displayBadEventDialog(QString const &userID) { emit userBadEvent(userID, tr("Bridge ran into an internal error and it is not able to proceed with the account %1. Synchronize your local database now or logout" " to do it later. Synchronization time depends on the size of your mailbox.").arg(elideLongString(user->primaryEmailOrUsername(), 30))); - emit selectUser(userID); + emit selectUser(userID, true); emit showMainWindow(); ) } diff --git a/internal/frontend/bridge-gui/bridge-gui/QMLBackend.h b/internal/frontend/bridge-gui/bridge-gui/QMLBackend.h index afde837e..7ee7e4f1 100644 --- a/internal/frontend/bridge-gui/bridge-gui/QMLBackend.h +++ b/internal/frontend/bridge-gui/bridge-gui/QMLBackend.h @@ -180,6 +180,8 @@ public slots: // slot for signals received from QML -> To be forwarded to Bridge void onVersionChanged(); ///< Slot for the version change signal. void setMailServerSettings(int imapPort, int smtpPort, bool useSSLForIMAP, bool useSSLForSMTP) const; ///< Forwards a connection mode change request from QML to gRPC void sendBadEventUserFeedback(QString const &userID, bool doResync); ///< Slot the providing user feedback for a bad event. + +public slots: // slots for functions that need to be processed locally. void setNormalTrayIcon(); ///< Set the tray icon to normal. void setErrorTrayIcon(QString const& stateString, QString const &statusIcon); ///< Set the tray icon to 'error' state. void setWarnTrayIcon(QString const& stateString, QString const &statusIcon); ///< Set the tray icon to 'warn' state. @@ -245,7 +247,7 @@ signals: // Signals received from the Go backend, to be forwarded to QML void hideMainWindow(); ///< Signal for the 'hideMainWindow' gRPC stream event. void showHelp(); ///< Signal for the 'showHelp' event (from the context menu). void showSettings(); ///< Signal for the 'showHelp' event (from the context menu). - void selectUser(QString const& userID); ///< Signal emitted in order to selected a user with a given ID in the list. + void selectUser(QString const& userID, bool forceShowWindow); ///< Signal emitted in order to selected a user with a given ID in the list. void genericError(QString const &title, QString const &description); ///< Signal for the 'genericError' gRPC stream event. void imapLoginWhileSignedOut(QString const& username); ///< Signal for the notification of IMAP login attempt on a signed out account. diff --git a/internal/frontend/bridge-gui/bridge-gui/TrayIcon.cpp b/internal/frontend/bridge-gui/bridge-gui/TrayIcon.cpp index b389d89e..ccb3528b 100644 --- a/internal/frontend/bridge-gui/bridge-gui/TrayIcon.cpp +++ b/internal/frontend/bridge-gui/bridge-gui/TrayIcon.cpp @@ -49,6 +49,50 @@ QIcon loadIconFromImage(QString const &path) { } +//**************************************************************************************************************************************************** +/// \brief Load a multi-resolution icon from a SVG file. The image is assumed to be square. SVG is rasterized in 256, 128, 64, 32 and 16px. +/// +/// Note: QPixmap can load SVG files directly, but our SVG file are defined in small shape size and QPixmap will rasterize them a very low resolution +/// by default (eg. 16x16), which is insufficient for some uses. As a consequence, we manually generate a multi-resolution icon that render smoothly +/// at any acceptable resolution for an icon. +/// +/// \param[in] path The path of the SVG file. +/// \return The icon. +//**************************************************************************************************************************************************** +QIcon loadIconFromSVG(QString const &path, QColor const &color = QColor()) { + QSvgRenderer renderer(path); + QIcon icon; + qint32 size = 256; + + while (size >= 16) { + QPixmap pixmap(size, size); + pixmap.fill(QColor(0, 0, 0, 0)); + QPainter painter(&pixmap); + renderer.render(&painter); + if (color.isValid()) { + painter.setCompositionMode(QPainter::CompositionMode_SourceIn); + painter.fillRect(pixmap.rect(), color); + } + painter.end(); + icon.addPixmap(pixmap); + size /= 2; + } + + return icon; +} + + +//**************************************************************************************************************************************************** +// +//**************************************************************************************************************************************************** +QIcon loadIcon(QString const& path) { + if (path.endsWith(".svg", Qt::CaseInsensitive)) { + return loadIconFromSVG(path); + } + return loadIconFromImage(path); +} + + //**************************************************************************************************************************************************** /// \brief Retrieve the color associated with a tray icon state. /// @@ -95,6 +139,18 @@ QString stateText(TrayIcon::State state) { } +//**************************************************************************************************************************************************** +/// \brief converts a QML resource path to Qt resource path. +/// QML resource paths are a bit different from qt resource paths +/// \param[in] path The resource path. +/// \return +//**************************************************************************************************************************************************** +QString qmlResourcePathToQt(QString const &path) { + QString result = path; + result.replace(QRegularExpression(R"(^\.\/)"), ":/qml/"); + return result; +} + } // anonymous namespace @@ -111,7 +167,8 @@ TrayIcon::TrayIcon() connect(menu_.get(), &QMenu::aboutToShow, this, &TrayIcon::onMenuAboutToShow); connect(this, &TrayIcon::selectUser, &app().backend(), &QMLBackend::selectUser); connect(this, &TrayIcon::activated, this, &TrayIcon::onActivated); - + // some OSes/Desktop managers will automatically show main window when clicked, but not all, so we do it manually. + connect(this, &TrayIcon::messageClicked, &app().backend(), &QMLBackend::showMainWindow); this->show(); this->setState(State::Normal, QString(), QString()); @@ -151,7 +208,7 @@ void TrayIcon::onUserClicked() { throw Exception("Could not retrieve context menu's selected user."); } - emit selectUser(userID); + emit selectUser(userID, true); } catch (Exception const &e) { app().log().error(e.qwhat()); } @@ -242,15 +299,23 @@ void TrayIcon::setState(TrayIcon::State state, QString const &stateString, QStri } +//**************************************************************************************************************************************************** +/// \param[in] title The title. +/// \param[in] message The message. +//**************************************************************************************************************************************************** +void TrayIcon::showErrorPopupNotification(QString const &title, QString const &message) { +// this->showMessage(title, message, loadIconFromSVG(":/qml/icons/ic-exclamation-circle-filled.svg", errorColor)); + this->showMessage(title, message, loadIconFromSVG(":/qml/icons/ic-alert.svg")); +} + + //**************************************************************************************************************************************************** /// \param[in] svgPath The path of the SVG file for the icon. /// \param[in] color The color to apply to the icon. //**************************************************************************************************************************************************** void TrayIcon::generateStatusIcon(QString const &svgPath, QColor const &color) { // We use the SVG path as pixmap mask and fill it with the appropriate color - QString resourcePath = svgPath; - resourcePath.replace(QRegularExpression(R"(^\.\/)"), ":/qml/"); // QML resource path are a bit different from the Qt resources path. - QPixmap pixmap(resourcePath); + QPixmap pixmap(qmlResourcePathToQt(svgPath)); QPainter painter(&pixmap); painter.setCompositionMode(QPainter::CompositionMode_SourceIn); painter.fillRect(pixmap.rect(), color); @@ -259,9 +324,9 @@ void TrayIcon::generateStatusIcon(QString const &svgPath, QColor const &color) { } -//********************************************************************************************************************** +//**************************************************************************************************************************************************** // -//********************************************************************************************************************** +//**************************************************************************************************************************************************** void TrayIcon::refreshContextMenu() { if (!menu_) { app().log().error("Native tray icon context menu is null."); @@ -294,3 +359,5 @@ void TrayIcon::refreshContextMenu() { menu_->addSeparator(); menu_->addAction(tr("&Quit Bridge"), QKeySequence("Ctrl+Q"), &app().backend(), &QMLBackend::quit); } + + diff --git a/internal/frontend/bridge-gui/bridge-gui/TrayIcon.h b/internal/frontend/bridge-gui/bridge-gui/TrayIcon.h index 74cfdc94..326d0d7b 100644 --- a/internal/frontend/bridge-gui/bridge-gui/TrayIcon.h +++ b/internal/frontend/bridge-gui/bridge-gui/TrayIcon.h @@ -41,10 +41,10 @@ public: // data members TrayIcon& operator=(TrayIcon const&) = delete; ///< Disabled assignment operator. TrayIcon& operator=(TrayIcon&&) = delete; ///< Disabled move assignment operator. void setState(State state, QString const& stateString, QString const &statusIconPath); ///< Set the state of the icon - void showNotificationPopup(QString const& title, QString const &message, QString const& iconPath); ///< Display a pop up notification. + void showErrorPopupNotification(QString const& title, QString const &message); ///< Display a pop up notification. signals: - void selectUser(QString const& userID); ///< Signal for selecting a user with a given userID + void selectUser(QString const& userID, bool forceShowWindow); ///< Signal for selecting a user with a given userID private slots: void onMenuAboutToShow(); ///< Slot called before the context menu is shown. diff --git a/internal/frontend/bridge-gui/bridge-gui/qml/MainWindow.qml b/internal/frontend/bridge-gui/bridge-gui/qml/MainWindow.qml index 9295d7f5..3de94f99 100644 --- a/internal/frontend/bridge-gui/bridge-gui/qml/MainWindow.qml +++ b/internal/frontend/bridge-gui/bridge-gui/qml/MainWindow.qml @@ -95,9 +95,11 @@ ApplicationWindow { root.showAndRise() } - function onSelectUser(userID) { + function onSelectUser(userID, forceShowWindow) { contentWrapper.selectUser(userID) - root.showAndRise() + if (forceShowWindow) { + root.showAndRise() + } } } diff --git a/internal/frontend/bridge-gui/bridgepp/bridgepp/User/User.cpp b/internal/frontend/bridge-gui/bridgepp/bridgepp/User/User.cpp index 041a4d12..0059f46a 100644 --- a/internal/frontend/bridge-gui/bridgepp/bridgepp/User/User.cpp +++ b/internal/frontend/bridge-gui/bridgepp/bridgepp/User/User.cpp @@ -34,9 +34,7 @@ SPUser User::newUser(QObject *parent) { /// \param[in] parent The parent object. //**************************************************************************************************************************************************** User::User(QObject *parent) - : QObject(parent) - , imapFailureCooldownEndTime_(QDateTime::currentDateTime()) { - + : QObject(parent) { } @@ -355,22 +353,18 @@ QString User::stateToString(UserState state) { //**************************************************************************************************************************************************** -/// We display a notification and pop the application window if an IMAP client tries to connect to a signed out account, but we do not want to -/// do it repeatedly, as it's an intrusive action. This function let's you define a period of time during which the notification should not be -/// displayed. -/// -/// \param durationMSecs The duration of the period in milliseconds. +/// \param[in] durationMSecs The duration of the period in milliseconds. //**************************************************************************************************************************************************** -void User::startImapLoginFailureCooldown(qint64 durationMSecs) { - imapFailureCooldownEndTime_ = QDateTime::currentDateTime().addMSecs(durationMSecs); +void User::startNotificationCooldownPeriod(User::ENotification notification, qint64 durationMSecs) { + notificationCooldownList_[notification] = QDateTime::currentDateTime().addMSecs(durationMSecs); } //**************************************************************************************************************************************************** -/// \return true if we currently are in a cooldown period for the notification +/// \return true iff the notification is currently in a cooldown period. //**************************************************************************************************************************************************** -bool User::isInIMAPLoginFailureCooldown() const { - return QDateTime::currentDateTime() < imapFailureCooldownEndTime_; +bool User::isNotificationInCooldown(User::ENotification notification) const { + return notificationCooldownList_.contains(notification) && (QDateTime::currentDateTime() < notificationCooldownList_[notification]); } diff --git a/internal/frontend/bridge-gui/bridgepp/bridgepp/User/User.h b/internal/frontend/bridge-gui/bridgepp/bridgepp/User/User.h index 78cfe8c9..d30c6b00 100644 --- a/internal/frontend/bridge-gui/bridgepp/bridgepp/User/User.h +++ b/internal/frontend/bridge-gui/bridgepp/bridgepp/User/User.h @@ -62,6 +62,12 @@ typedef std::shared_ptr SPUser; ///< Type definition for shared poin class User : public QObject { Q_OBJECT +public: // data types + enum class ENotification { + IMAPLoginWhileSignedOut, ///< An IMAP client tried to login while the user is signed out. + IMAPPasswordFailure, ///< An IMAP client provided an invalid password for the user. + }; + public: // static member function static SPUser newUser(QObject *parent); ///< Create a new user static QString stateToString(UserState state); ///< Return a string describing a user state. @@ -74,8 +80,8 @@ public: // member functions. User &operator=(User &&) = delete; ///< Disabled move assignment operator. void update(User const &user); ///< Update the user. Q_INVOKABLE QString primaryEmailOrUsername() const; ///< Return the user primary email, or, if unknown its username. - void startImapLoginFailureCooldown(qint64 durationMSecs); ///< Start the user cooldown period for the IMAP login attempt while signed-out notification. - bool isInIMAPLoginFailureCooldown() const; ///< Check if the user in a IMAP login failure notification. + void startNotificationCooldownPeriod(ENotification notification, qint64 durationMSecs); ///< Start the user cooldown period for a notification. + bool isNotificationInCooldown(ENotification notification) const; ///< Return true iff the notification is in a cooldown period. public slots: // slots for QML generated calls @@ -147,7 +153,7 @@ private: // member functions. User(QObject *parent); ///< Default constructor. private: // data members. - QDateTime imapFailureCooldownEndTime_; ///< The end date/time for the IMAP login failure notification cooldown period. + QMap notificationCooldownList_; ///< A list of cooldown period end time for notifications. QString id_; ///< The userID. QString username_; ///< The username QString password_; ///< The IMAP password of the user.