Fix SSL mode combobox did not work properly

pglab/ConnectionConfigurationWidget.cpp

@@ -117,10 +117,9 @@ ConnectionConfigurationWidget::ConnectionConfigurationWidget(
 	SET_OBJECT_NAME(lblSsl);
 	cmbbxSsl = new QComboBox;
 	SET_OBJECT_NAME(cmbbxSsl);
-	cmbbxSsl->setModelColumn(0);
+	cmbbxSsl->setModelColumn(SslModeModel::Name);
 	auto ssl_model = new SslModeModel(this);
 	cmbbxSsl->setModel(ssl_model);
-    cmbbxSsl->setEditable(true);
 	lblSsl->setBuddy(cmbbxSsl);
 
 	lblCert = new QLabel;
@@ -207,7 +206,13 @@ void ConnectionConfigurationWidget::setData(const ConnectionConfig &cfg)
     edtPassword->setText("");
 
     cmbDbname->setCurrentText(cfg.dbname());
-    cmbbxSsl->setCurrentText(cfg.getParameter("sslmode"));
+    auto sslMode = cfg.getParameter("sslmode");
+    if (sslMode == "verify_ca")
+        sslMode = "verify-ca";
+    else if (sslMode == "verify_full")
+        sslMode = "verify-full";
+    auto sslIndex = cmbbxSsl->findData(sslMode);
+    cmbbxSsl->setCurrentIndex(sslIndex < 0 ? 0 : sslIndex);
 	edtCert->setText(cfg.sslCert());
 	edtKey->setText(cfg.sslKey());
 	edtRootCert->setText(cfg.sslRootCert());
@@ -234,7 +239,7 @@ ConnectionConfig ConnectionConfigurationWidget::data() const
 	cfg.setUser(edtUser->text());
     cfg.setPassword(edtPassword->text());
     cfg.setDbname(cmbDbname->currentText());
-    cfg.setParameter("sslmode", cmbbxSsl->currentText());
+    cfg.setParameter("sslmode", cmbbxSsl->currentData().toString());
 	cfg.setSslCert(edtCert->text());
 	cfg.setSslKey(edtKey->text());
 	cfg.setSslRootCert(edtRootCert->text());

pglab/SslModeModel.cpp

@@ -1,12 +1,20 @@
 #include "SslModeModel.h"
 
+#include <QStringList>
+
+namespace {
+
+const QStringList sslModes = {QString(), "disable", "allow", "prefer", "require", "verify-ca", "verify-full"};
+
+}
+
 SslModeModel::SslModeModel(QObject *parent)
 	: QAbstractListModel(parent)
 {}
 
-int SslModeModel::rowCount(const QModelIndex &) const
+int SslModeModel::rowCount(const QModelIndex &parent) const
 {
-	return 6;
+	return parent.isValid() ? 0 : static_cast<int>(sslModes.size());
 }
 
 int SslModeModel::columnCount(const QModelIndex &) const
@@ -16,49 +24,43 @@ int SslModeModel::columnCount(const QModelIndex &) const
 
 QVariant SslModeModel::data(const QModelIndex &index, int role) const
 {
+	if (!index.isValid() || index.model() != this || index.row() < 0 || index.row() >= rowCount()
+		|| index.column() < 0 || index.column() >= ColCount)
+		return {};
+
+	if (index.column() == Name) {
+		const auto &mode = sslModes.at(index.row());
+		if (role == Qt::UserRole)
+			return mode;
+		if (role == Qt::DisplayRole || role == Qt::EditRole)
+			return index.row() == 0 ? tr("Default") : mode;
+		return {};
+	}
+
 	QVariant v;
 	if (role == Qt::DisplayRole) {
 		switch (index.column()) {
-		case Name:
-			switch(index.row()) {
-			case 0:
-				v = tr("disable");
-				break;
-			case 1:
-				v = tr("allow");
-				break;
-			case 2:
-				v = tr("prefer");
-				break;
-			case 3:
-				v = tr("require");
-				break;
-			case 4:
-				v = tr("verify_ca");
-				break;
-			case 5:
-				v = tr("verify_full");
-				break;
-			}
-			break;
 		case Description:
 			switch(index.row()) {
 			case 0:
-				v = tr("try a non encrypted connection only");
+				v = tr("use the PostgreSQL default SSL mode");
 				break;
 			case 1:
-				v = tr("try no encryption first then try encrypted");
+				v = tr("try a non encrypted connection only");
 				break;
 			case 2:
-				v = tr("try encrypted first then not encrypted");
+				v = tr("try no encryption first then try encrypted");
 				break;
 			case 3:
-				v = tr("require an encrypted connection");
+				v = tr("try encrypted first then not encrypted");
 				break;
 			case 4:
-				v = tr("verify encryption certificate has a valid signature");
+				v = tr("require an encrypted connection");
 				break;
 			case 5:
+				v = tr("verify encryption certificate has a valid signature");
+				break;
+			case 6:
 				v = tr("verify encryption certificate has a valid signature and matches the host");
 				break;
 			}

releasenotes/notes/fix-ssl-mode-selection-45cc41625d61af90.yaml

@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    Fixed SSL mode selection and saving in the connection configuration dialog.
+    The field now offers the supported modes, including the corrected verify-ca
+    and verify-full values, and a Default option to leave the mode unset.
+    Existing verify_ca and verify_full values are corrected when saved.

tests/pglabtests/CMakeLists.txt

@@ -21,10 +21,12 @@ add_executable(pglabtests
     tst_Range.cpp
     ResultSourceMappingTest.cpp
     tst_RecordPanel.cpp
-    ${CMAKE_SOURCE_DIR}/pglab/crud/RecordPanel.cpp
+    tst_SslModeModel.cpp
+    tst_ConnectionConfigurationWidget.cpp
     )
 
 target_link_libraries(pglabtests PRIVATE
+    pglabapp
     pglablib
     pgsql
     core

tests/pglabtests/tst_ConnectionConfigurationWidget.cpp

@@ -0,0 +1,111 @@
+#include <gtest/gtest.h>
+
+#include <QApplication>
+#include <QComboBox>
+#include <QDialog>
+#include <QDialogButtonBox>
+#include <QEvent>
+#include <QPushButton>
+
+#include "ConnectionConfig.h"
+#include "ConnectionConfigurationWidget.h"
+#include "ConnectionController.h"
+#include "PrintTo_Qt.h"
+
+class ConnectionConfigurationWidgetTest : public ::testing::Test {
+protected:
+    // SSL settings do not need the controller's database or connection tree initialized.
+    ConnectionController controller;
+};
+
+TEST_F(ConnectionConfigurationWidgetTest, SelectedSslModesRoundTripThroughTheWidget)
+{
+    ConnectionConfigurationWidget widget(&controller);
+    widget.setData(ConnectionConfig());
+    auto combo = widget.findChild<QComboBox *>("cmbbxSsl");
+    ASSERT_NE(combo, nullptr);
+    EXPECT_FALSE(combo->isEditable());
+
+    const QStringList modes = {"disable", "allow", "prefer", "require", "verify-ca", "verify-full"};
+    for (const auto &mode : modes) {
+        SCOPED_TRACE(mode.toStdString());
+        auto index = combo->findData(mode);
+        ASSERT_GT(index, 0);
+        combo->setCurrentIndex(index);
+        EXPECT_EQ(combo->currentText(), mode);
+
+        auto saved = widget.data();
+        EXPECT_EQ(saved.getParameter("sslmode"), mode);
+        ConnectionConfigurationWidget reopened(&controller);
+        reopened.setData(saved);
+        EXPECT_EQ(reopened.data().getParameter("sslmode"), mode);
+        EXPECT_EQ(reopened.findChild<QComboBox *>("cmbbxSsl")->currentText(), mode);
+    }
+
+    combo->setCurrentIndex(combo->findData(QString()));
+    EXPECT_TRUE(widget.data().getParameter("sslmode").isEmpty());
+    EXPECT_EQ(widget.data().getParameters().count("sslmode"), 0);
+}
+
+TEST_F(ConnectionConfigurationWidgetTest, LegacyVerificationModesAreCanonicalized)
+{
+    ConnectionConfigurationWidget widget(&controller);
+    for (const auto &mode : {QStringLiteral("verify_ca"), QStringLiteral("verify_full")}) {
+        ConnectionConfig config;
+        config.setParameter("sslmode", mode);
+        widget.setData(config);
+
+        auto canonical = QString(mode).replace('_', '-');
+        EXPECT_EQ(widget.findChild<QComboBox *>("cmbbxSsl")->currentText(), canonical);
+        EXPECT_EQ(widget.data().getParameter("sslmode"), canonical);
+        EXPECT_EQ(config.getParameter("sslmode"), mode);
+    }
+}
+
+TEST_F(ConnectionConfigurationWidgetTest, MissingOrUnknownModesResetToDefault)
+{
+    ConnectionConfigurationWidget widget(&controller);
+    for (const auto &mode : {QString(), QStringLiteral("unknown")}) {
+        ConnectionConfig config;
+        config.setParameter("sslmode", "require");
+        widget.setData(config);
+        config.setParameter("sslmode", mode);
+        widget.setData(config);
+
+        EXPECT_EQ(widget.findChild<QComboBox *>("cmbbxSsl")->currentText(), QStringLiteral("Default"));
+        EXPECT_TRUE(widget.data().getParameter("sslmode").isEmpty());
+    }
+}
+
+TEST_F(ConnectionConfigurationWidgetTest, DialogOnlySavesOnAccept)
+{
+    ConnectionConfig config;
+    config.setParameter("sslmode", "allow");
+    for (bool accept : {false, true}) {
+        bool saved = false;
+        ConnectionConfigurationWidget::editExistingInWindow(&controller, config,
+            [&](ConnectionConfigurationWidget &widget) {
+                saved = true;
+                EXPECT_EQ(widget.data().getParameter("sslmode"), QStringLiteral("require"));
+            });
+
+        QDialog *dialog = nullptr;
+        for (auto window : QApplication::topLevelWidgets()) {
+            if (auto candidate = qobject_cast<QDialog *>(window);
+                candidate && candidate->findChild<ConnectionConfigurationWidget *>()) {
+                dialog = candidate;
+                break;
+            }
+        }
+        ASSERT_NE(dialog, nullptr);
+        auto combo = dialog->findChild<QComboBox *>("cmbbxSsl");
+        ASSERT_NE(combo, nullptr);
+        combo->setCurrentIndex(combo->findData(QStringLiteral("require")));
+        auto buttons = dialog->findChild<QDialogButtonBox *>();
+        ASSERT_NE(buttons, nullptr);
+        buttons->button(accept ? QDialogButtonBox::Ok : QDialogButtonBox::Cancel)->click();
+        EXPECT_EQ(saved, accept);
+        EXPECT_EQ(config.getParameter("sslmode"), QStringLiteral("allow"));
+        QCoreApplication::sendPostedEvents(nullptr, QEvent::DeferredDelete);
+    }
+}

tests/pglabtests/tst_SslModeModel.cpp

@@ -0,0 +1,67 @@
+#include <gtest/gtest.h>
+
+#include <QComboBox>
+
+#include "ConnectionConfig.h"
+#include "PrintTo_Qt.h"
+#include "SslModeModel.h"
+
+TEST(SslModeModelTest, SelectedModesKeepTheirTextAndCanonicalValue)
+{
+    SslModeModel model;
+    QComboBox combo;
+    combo.setModel(&model);
+
+    ASSERT_FALSE(combo.isEditable());
+    const QStringList modes = {"disable", "allow", "prefer", "require", "verify-ca", "verify-full"};
+    ASSERT_EQ(combo.count(), modes.size() + 1);
+
+    for (const auto &mode : modes) {
+        SCOPED_TRACE(mode.toStdString());
+        auto index = combo.findData(mode);
+        ASSERT_GT(index, 0);
+        combo.setCurrentIndex(index);
+        EXPECT_EQ(combo.currentText(), mode);
+        EXPECT_EQ(combo.currentData().toString(), mode);
+
+        ConnectionConfig config;
+        config.setParameter("sslmode", combo.currentData().toString());
+        EXPECT_EQ(config.getParameter("sslmode"), mode);
+        EXPECT_TRUE(config.connectionString().contains("sslmode=" + mode));
+
+        QComboBox reopened;
+        reopened.setModel(&model);
+        reopened.setCurrentIndex(reopened.findData(config.getParameter("sslmode")));
+        EXPECT_EQ(reopened.currentText(), mode);
+        EXPECT_EQ(reopened.currentData().toString(), mode);
+    }
+}
+
+TEST(SslModeModelTest, DefaultClearsAnExplicitMode)
+{
+    SslModeModel model;
+    QComboBox combo;
+    combo.setModel(&model);
+
+    EXPECT_EQ(combo.currentText(), QStringLiteral("Default"));
+    EXPECT_EQ(combo.currentData().toString(), QString());
+    EXPECT_EQ(combo.findData(QString()), 0);
+
+    ConnectionConfig config;
+    config.setParameter("sslmode", "require");
+    combo.setCurrentIndex(combo.findData(QStringLiteral("require")));
+    combo.setCurrentIndex(combo.findData(QString()));
+    config.setParameter("sslmode", combo.currentData().toString());
+
+    EXPECT_TRUE(config.getParameter("sslmode").isEmpty());
+    EXPECT_EQ(config.getParameters().count("sslmode"), 0);
+    EXPECT_FALSE(config.connectionString().contains("sslmode="));
+}
+
+TEST(SslModeModelTest, InvalidIndexesAndChildRowsAreEmpty)
+{
+    SslModeModel model;
+    EXPECT_FALSE(model.data(QModelIndex()).isValid());
+    EXPECT_FALSE(model.data(model.index(model.rowCount(), 0)).isValid());
+    EXPECT_EQ(model.rowCount(model.index(0, 0)), 0);
+}