Android: Fix workflow issues in package signing password dialogs

Task-number: QTCREATORBUG-17545
Change-Id: Ide0c322a50455997c7b8fb2350dbdef1d76257c9
Reviewed-by: Tobias Hunger <tobias.hunger@qt.io>

src/plugins/android/androidbuildapkstep.cpp

@@ -44,10 +44,18 @@
 #include <qtsupport/qtkitinformation.h>
 
 #include <utils/synchronousprocess.h>
+#include <utils/utilsicons.h>
 
-#include <QInputDialog>
+#include <QDialogButtonBox>
+#include <QHBoxLayout>
+#include <QLabel>
+#include <QLineEdit>
 #include <QMessageBox>
 #include <QProcess>
+#include <QPushButton>
+#include <QVBoxLayout>
+
+#include <memory>
 
 namespace Android {
 using namespace Internal;
@@ -58,6 +66,31 @@ const QLatin1String BuildTargetSdkKey("BuildTargetSdk");
 const QLatin1String VerboseOutputKey("VerboseOutput");
 const QLatin1String UseGradleKey("UseGradle");
 
+
+class PasswordInputDialog : public QDialog {
+public:
+    enum Context{
+      KeystorePassword = 1,
+      CertificatePassword
+    };
+
+    PasswordInputDialog(Context context, std::function<bool (const QString &)> callback,
+                        const QString &extraContextStr, QWidget *parent = nullptr);
+
+    static QString getPassword(Context context, std::function<bool (const QString &)> callback,
+                               const QString &extraContextStr, bool *ok = nullptr,
+                               QWidget *parent = nullptr);
+
+private:
+    std::function<bool (const QString &)> verifyCallback = [](const QString &) { return true; };
+    QLabel *inputContextlabel = new QLabel(this);
+    QLineEdit *inputEdit = new QLineEdit(this);
+    QLabel *warningIcon = new QLabel(this);
+    QLabel *warningLabel = new QLabel(this);
+    QDialogButtonBox *buttonBox = new QDialogButtonBox(QDialogButtonBox::Ok | QDialogButtonBox::Cancel,
+                                                       this);
+};
+
 AndroidBuildApkStep::AndroidBuildApkStep(ProjectExplorer::BuildStepList *parent, const Core::Id id)
     : ProjectExplorer::AbstractProcessStep(parent, id),
       m_buildTargetSdk(AndroidConfig::apiLevelNameFor(AndroidConfigurations::currentConfig().highestAndroidSdk()))
@@ -95,16 +128,8 @@ bool AndroidBuildApkStep::init(QList<const BuildStep *> &earlierSteps)
 
     if (m_signPackage) {
         // check keystore and certificate passwords
-        while (!AndroidManager::checkKeystorePassword(m_keystorePath.toString(), m_keystorePasswd)) {
+        if (!verifyKeystorePassword() || !verifyCertificatePassword())
-            if (!keystorePassword())
+            return false;
-                return false; // user canceled
-        }
-
-        while (!AndroidManager::checkCertificatePassword(m_keystorePath.toString(), m_keystorePasswd, m_certificateAlias, m_certificatePasswd)) {
-            if (!certificatePassword())
-                return false; // user canceled
-        }
-
 
         if (bc->buildType() != ProjectExplorer::BuildConfiguration::Release)
             emit addOutput(tr("Warning: Signing a debug or profile package."),
@@ -155,6 +180,37 @@ void AndroidBuildApkStep::processFinished(int exitCode, QProcess::ExitStatus sta
         QMetaObject::invokeMethod(this, "showInGraphicalShell", Qt::QueuedConnection);
 }
 
+bool AndroidBuildApkStep::verifyKeystorePassword()
+{
+    if (AndroidManager::checkKeystorePassword(m_keystorePath.toString(), m_keystorePasswd))
+        return true;
+
+    bool success = false;
+    auto verifyCallback = std::bind(&AndroidManager::checkKeystorePassword,
+                                    m_keystorePath.toString(), std::placeholders::_1);
+    m_keystorePasswd = PasswordInputDialog::getPassword(PasswordInputDialog::KeystorePassword,
+                                                        verifyCallback, "", &success);
+    return success;
+}
+
+bool AndroidBuildApkStep::verifyCertificatePassword()
+{
+    if (AndroidManager::checkCertificatePassword(m_keystorePath.toString(), m_keystorePasswd,
+                                                 m_certificateAlias, m_certificatePasswd)) {
+        return true;
+    }
+
+    bool success = false;
+    auto verifyCallback = std::bind(&AndroidManager::checkCertificatePassword,
+                                    m_keystorePath.toString(), m_keystorePasswd,
+                                    m_certificateAlias, std::placeholders::_1);
+
+    m_certificatePasswd = PasswordInputDialog::getPassword(PasswordInputDialog::CertificatePassword,
+                                                           verifyCallback, m_certificateAlias,
+                                                           &success);
+    return success;
+}
+
 bool AndroidBuildApkStep::fromMap(const QVariantMap &map)
 {
     m_deployAction = AndroidDeployAction(map.value(DeployActionKey, BundleLibrariesDeployment).toInt());
@@ -280,15 +336,15 @@ bool AndroidBuildApkStep::verboseOutput() const
 
 QAbstractItemModel *AndroidBuildApkStep::keystoreCertificates()
 {
-    QString rawCerts;
+    // check keystore passwords
-    while (!rawCerts.length() || !m_keystorePasswd.length()) {
+    if (!verifyKeystorePassword())
+        return nullptr;
+
+    CertificatesModel *model = nullptr;
     QStringList params
             = { QLatin1String("-list"), QLatin1String("-v"), QLatin1String("-keystore"),
                 m_keystorePath.toUserOutput(), QLatin1String("-storepass") };
-        if (!m_keystorePasswd.length())
+
-            keystorePassword();
-        if (!m_keystorePasswd.length())
-            return nullptr;
     params << m_keystorePasswd;
     params << QLatin1String("-J-Duser.language=en");
 
@@ -296,47 +352,80 @@ QAbstractItemModel *AndroidBuildApkStep::keystoreCertificates()
     keytoolProc.setTimeoutS(30);
     const Utils::SynchronousProcessResponse response
             = keytoolProc.run(AndroidConfigurations::currentConfig().keytoolPath().toString(), params);
-        if (response.result != Utils::SynchronousProcessResponse::Finished) {
+    if (response.result != Utils::SynchronousProcessResponse::Finished)
-            QMessageBox::critical(0, tr("Error"),
+        QMessageBox::critical(0, tr("Error"), tr("Failed to run keytool."));
-                                  tr("Failed to run keytool."));
+    else
-            return nullptr;
+        model = new CertificatesModel(response.stdOut(), this);
+
+    return model;
 }
 
-        if (response.exitCode != 0) {
+PasswordInputDialog::PasswordInputDialog(PasswordInputDialog::Context context,
-            QMessageBox::critical(0, tr("Error"), tr("Invalid password."));
+                                         std::function<bool (const QString &)> callback,
-            m_keystorePasswd.clear();
+                                         const QString &extraContextStr,
-        }
+                                         QWidget *parent) :
-        rawCerts = response.stdOut();
+    QDialog(parent, Qt::Dialog | Qt::CustomizeWindowHint | Qt::WindowTitleHint),
-    }
+    verifyCallback(callback)
-    return new CertificatesModel(rawCerts, this);
-}
 
-bool AndroidBuildApkStep::keystorePassword()
 {
-    m_keystorePasswd.clear();
+    inputEdit->setEchoMode(QLineEdit::Password);
-    bool ok;
+
-    QString text = QInputDialog::getText(0, tr("Keystore"),
+    warningIcon->setPixmap(Utils::Icons::WARNING.pixmap());
-                                         tr("Keystore password:"), QLineEdit::Password,
+    warningIcon->setSizePolicy(QSizePolicy(QSizePolicy::Maximum, QSizePolicy::Maximum));
-                                         QString(), &ok);
+    warningIcon->hide();
-    if (ok && !text.isEmpty()) {
+
-        m_keystorePasswd = text;
+    warningLabel->hide();
-        return true;
+
+    auto warningLayout = new QHBoxLayout;
+    warningLayout->addWidget(warningIcon);
+    warningLayout->addWidget(warningLabel);
+
+    auto mainLayout = new QVBoxLayout(this);
+    mainLayout->addWidget(inputContextlabel);
+    mainLayout->addWidget(inputEdit);
+    mainLayout->addLayout(warningLayout);
+    mainLayout->addWidget(buttonBox);
+
+    connect(inputEdit, &QLineEdit::textChanged,[this](const QString &text) {
+        buttonBox->button(QDialogButtonBox::Ok)->setEnabled(!text.isEmpty());
+    });
+
+    connect(buttonBox, &QDialogButtonBox::accepted, [this]() {
+        if (verifyCallback(inputEdit->text())) {
+            accept(); // Dialog accepted.
+        } else {
+            warningIcon->show();
+            warningLabel->show();
+            warningLabel->setText(tr("Incorrect password."));
+            inputEdit->clear();
+            adjustSize();
         }
-    return false;
+    });
+
+    connect(buttonBox, &QDialogButtonBox::rejected, this, &QDialog::reject);
+
+    setWindowTitle(context == KeystorePassword ? tr("Keystore") : tr("Certificate"));
+
+    QString contextStr;
+    if (context == KeystorePassword)
+        contextStr = tr("Enter keystore password");
+    else
+        contextStr = tr("Enter certificate password");
+
+    contextStr += extraContextStr.isEmpty() ? QStringLiteral(":") :
+                                              QStringLiteral(" (%1):").arg(extraContextStr);
+    inputContextlabel->setText(contextStr);
 }
 
-bool AndroidBuildApkStep::certificatePassword()
+QString PasswordInputDialog::getPassword(Context context, std::function<bool (const QString &)> callback,
+                                         const QString &extraContextStr, bool *ok, QWidget *parent)
 {
-    m_certificatePasswd.clear();
+    std::unique_ptr<PasswordInputDialog> dlg(new PasswordInputDialog(context, callback,
-    bool ok;
+                                                                     extraContextStr, parent));
-    QString text = QInputDialog::getText(0, tr("Certificate"),
+    bool isAccepted = dlg->exec() == QDialog::Accepted;
-                                         tr("Certificate password (%1):").arg(m_certificateAlias), QLineEdit::Password,
+    if (ok)
-                                         QString(), &ok);
+        *ok = isAccepted;
-    if (ok && !text.isEmpty()) {
+    return isAccepted ? dlg->inputEdit->text() : "";
-        m_certificatePasswd = text;
-        return true;
-    }
-    return false;
 }
 
 } // namespace Android

src/plugins/android/androidbuildapkstep.h

@@ -87,13 +87,13 @@ protected:
 
     AndroidBuildApkStep(ProjectExplorer::BuildStepList *bc,
         AndroidBuildApkStep *other);
-    bool keystorePassword();
-    bool certificatePassword();
 
     bool init(QList<const BuildStep *> &earlierSteps) override;
     ProjectExplorer::BuildStepConfigWidget *createConfigWidget() override;
     bool immutable() const override { return true; }
     void processFinished(int exitCode, QProcess::ExitStatus status) override;
+    bool verifyKeystorePassword();
+    bool verifyCertificatePassword();
 
 protected:
     AndroidDeployAction m_deployAction = BundleLibrariesDeployment;

src/plugins/android/androidbuildapkwidget.cpp

@@ -208,9 +208,11 @@ void AndroidBuildApkWidget::createKeyStore()
 void AndroidBuildApkWidget::setCertificates()
 {
     QAbstractItemModel *certificates = m_step->keystoreCertificates();
+    if (certificates) {
         m_ui->signPackageCheckBox->setChecked(certificates);
         m_ui->certificatesAliasComboBox->setModel(certificates);
     }
+}
 
 void AndroidBuildApkWidget::updateKeyStorePath(const QString &path)
 {