fix: read OIDC Prompt from the Prompt key, not the Ports key (#12557)

changelog/unreleased/12557.md

@@ -0,0 +1,9 @@
+Bugfix: Read OIDC Prompt from the correct system configuration key
+
+The OpenID Connect Prompt value provided through the system configuration was
+read from the Ports key instead of the Prompt key. As a result the
+administrator-provided Prompt was ignored and replaced with the raw Ports
+value, which is not a valid OIDC prompt parameter. The Prompt is now read from
+the Prompt key.
+
+https://github.com/owncloud/client/issues/12557

src/libsync/config/appconfig.cpp

@@ -19,20 +19,34 @@ namespace chrono = std::chrono;
 
 AppConfig::AppConfig()
 {
-    _serverUrl = Theme::instance()->overrideServerUrlV2();
-    // If a theme provides a hardcoded URL, do not allow for URL change.
-    _allowServerURLChange = Theme::instance()->overrideServerUrlV2().isEmpty();
-    _skipUpdateCheck = false;
-    _openIdConfig = loadOpenIdConfigFromTheme();
+    loadThemeDefaults();
 
     if (!Theme::instance()->allowSystemConfigOverrides())
         return;
 
-    // Load all overrides
-
+    // Load all overrides from the system config path
     auto format = Utility::isWindows() ? QSettings::NativeFormat : QSettings::IniFormat;
     const QSettings system(configPath(QOperatingSystemVersion::currentType(), *Theme::instance()), format);
+    applySystemConfig(system);
+}
+
+AppConfig::AppConfig(const QSettings &system)
+{
+    loadThemeDefaults();
+    applySystemConfig(system);
+}
 
+void AppConfig::loadThemeDefaults()
+{
+    _serverUrl = Theme::instance()->overrideServerUrlV2();
+    // If a theme provides a hardcoded URL, do not allow for URL change.
+    _allowServerURLChange = Theme::instance()->overrideServerUrlV2().isEmpty();
+    _skipUpdateCheck = false;
+    _openIdConfig = loadOpenIdConfigFromTheme();
+}
+
+void AppConfig::applySystemConfig(const QSettings &system)
+{
     _serverUrl = system.value(SetupServerUrlKey, QString()).toString();
     if (system.contains(SetupAllowServerUrlChangeKey)) {
         _allowServerURLChange = system.value(SetupAllowServerUrlChangeKey).toBool();
@@ -68,7 +82,7 @@ OpenIdConfig AppConfig::loadOpenIdConfigFromSystemConfig(const QSettings &system
     QString clientId = system.value(OidcClientIdKey, QString()).toString();
     QString clientSecret = system.value(OidcClientSecretKey, QString()).toString();
     QString scopes = system.value(OidcScopesKey, QString()).toString();
-    QString prompt = system.value(OidcPortsKey, QString()).toString();
+    QString prompt = system.value(OidcPromptKey, QString()).toString();
 
     QVector<quint16> ports;
     QVariant portsVar = system.value(OidcPortsKey, QString()).toString();

src/libsync/config/appconfig.h

@@ -67,6 +67,16 @@ class OWNCLOUDSYNC_EXPORT AppConfig
 {
 public:
     explicit AppConfig();
+
+    /**
+     * Construct an AppConfig from an explicitly provided system configuration.
+     * The theme defaults are loaded first and then overridden with the values from @p system,
+     * mirroring the behavior of the default constructor without reading the on-disk/registry path.
+     * @param system the system configuration settings to apply on top of the theme defaults
+     * @internal kept public for testing purposes
+     */
+    explicit AppConfig(const QSettings &system);
+
     /**
      * Determine if changing the server URL is allowed based on system configuration.
      * This value is only relevant if SystemConfig::serverUrl() returns a non-empty string.
@@ -104,6 +114,16 @@ class OWNCLOUDSYNC_EXPORT AppConfig
     static QString configPath(const QOperatingSystemVersion::OSType& os, const Theme& theme);
 
 private:
+    /**
+     * Initialize all settings with the defaults provided by the current theme.
+     */
+    void loadThemeDefaults();
+
+    /**
+     * Apply the overrides from the given system configuration on top of the already loaded theme defaults.
+     */
+    void applySystemConfig(const QSettings &system);
+
     static OpenIdConfig loadOpenIdConfigFromTheme();
     static OpenIdConfig loadOpenIdConfigFromSystemConfig(const QSettings &system);
 

test/testappconfig.cpp

@@ -5,6 +5,9 @@
 #include "libsync/owncloudtheme.h"
 #include "libsync/config/appconfig.h"
 
+#include <QSettings>
+#include <QTemporaryDir>
+
 class TestAppConfig : public QObject
 {
     Q_OBJECT
@@ -17,6 +20,34 @@ private Q_SLOTS:
         QCOMPARE(OCC::AppConfig::configPath(QOperatingSystemVersion::MacOS, t), QString("/Library/Preferences/com.owncloud.desktopclient/ownCloud.ini"));
         QCOMPARE(OCC::AppConfig::configPath(QOperatingSystemVersion::Unknown, t), QString("/etc/ownCloud/ownCloud.ini"));
     }
+
+    void testLoadOpenIdConfig()
+    {
+        QTemporaryDir dir;
+        QVERIFY(dir.isValid());
+        const QString path = dir.filePath(QStringLiteral("system.ini"));
+
+        {
+            QSettings settings(path, QSettings::IniFormat);
+            settings.setValue(QStringLiteral("OpenIDConnect/ClientId"), QStringLiteral("my-client-id"));
+            settings.setValue(QStringLiteral("OpenIDConnect/ClientSecret"), QStringLiteral("my-client-secret"));
+            settings.setValue(QStringLiteral("OpenIDConnect/Ports"), QStringLiteral("8080,8443"));
+            settings.setValue(QStringLiteral("OpenIDConnect/Scopes"), QStringLiteral("openid email"));
+            settings.setValue(QStringLiteral("OpenIDConnect/Prompt"), QStringLiteral("select_account consent"));
+            settings.sync();
+        }
+
+        // Drive a real AppConfig instance through its public interface, exactly as production code does.
+        const QSettings settings(path, QSettings::IniFormat);
+        const OCC::OpenIdConfig cfg = OCC::AppConfig(settings).openIdConfig();
+
+        // Prompt must be read from the Prompt key, not the Ports key (regression test for #12557)
+        QCOMPARE(cfg.prompt(), QStringLiteral("select_account consent"));
+        QCOMPARE(cfg.ports(), (QList<quint16>{8080, 8443}));
+        QCOMPARE(cfg.scopes(), QStringLiteral("openid email"));
+        QCOMPARE(cfg.clientId(), QStringLiteral("my-client-id"));
+        QCOMPARE(cfg.clientSecret(), QStringLiteral("my-client-secret"));
+    }
 };
 
 QTEST_GUILESS_MAIN(TestAppConfig)