[DC-135] feat: introduce system based configuration aka group policy by DeepDiver1975 · Pull Request #12367 · owncloud/client

DeepDiver1975 · 2025-10-09T15:32:12Z

Background

When the client is installed in a bigger organization via tools like Microsoft SCCM (now known as MECM) or similar tools it can be required to specify some pre-configured values.
Platform specific configuration storage shall be used (e.g. registry on MS)

Solution

Two configuration values are now supported:

Setup/ServerUrl: allows to specify the server url in the account setup
Setup/AllowServerUrlChange: allows to specify if the url can be changed - if false the given Setup/ServerUrl is the only allowed server to be connected to
OpenIDConnect/*: adds a bunch of paremeters to configure OpenIdConnect

Configuration will be read from:

linux: /etc/ownCloud/ownCloud.ini
mac: /Library/Preferences/com.owncloud.desktopclient/ownCloud.ini
windows: HKEY_LOCAL_MACHINE\Software\ownCloud\ownCloud

Further Configuration Values

OpenID Connect client id and client secret
including the capability to use PKCE without a secret
[ ]

update-docs · 2025-10-09T15:32:17Z

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

…n mechanisms

…onfig + add OpenIdConnect configuration

…d settings from a system location. skipUpdateCheck is using SystemConfig already. Proxy settings will follow

src/libsync/config/systemconfig.h

modSpike · 2026-01-27T11:02:19Z

src/gui/creds/credentials.cpp

    // parent with nam to ensure we reset when the nam is reset
    // todo: #22 - the parenting here is highly questionable, as is the use of the shared account ptr
-    _oAuthJob = new AccountBasedOAuth(_account, this);
+    SystemConfig systemConfig;


I think either the SystemConfig or the openIdConfig should be a member of credentials, then we don't need to load the config over and over again.

modSpike · 2026-01-27T11:05:00Z

src/gui/creds/oauth.cpp

 const QString wellKnownPathC = QStringLiteral("/.well-known/openid-configuration");

-auto defaultOauthPromptValue()
+auto defaultOauthPromptValue(const OpenIdConfig& config)


I'm not fond of auto's when the return type is not obvious

modSpike · 2026-01-27T11:10:15Z

src/gui/newaccountwizard/urlpagecontroller.cpp

 */
 #include "urlpagecontroller.h"

+#include "../../libsync/config/systemconfig.h"


that doesn't look right - see the same file included in oauthpagecontroller. cpp. I think you should only need config/systemconfig.h

modSpike · 2026-01-27T11:20:09Z

src/libsync/config/systemconfig.cpp

+
+namespace {
+    // Setup related keys
+    static constexpr auto KEY_SETUP_ALLOW_SERVER_URL_CHANGE = "Setup/AllowServerUrlChange";


we need to make a group decision about how to declare const strings for settings keys going forward. I don't like the anonymous namespace as you have to know where to find it. and "auto" is not cool imo for decl'd values or function returns.

IMO we should prefer (private whenever possible) static const members a la:

inline static const QString KEY_SETUP_ALLOW_SERVER_URL_CHANGE = "Setup/AllowServerUrlChange";

then we have a clear view into the possible keys as they are clearly outlined right in the class decl

settings keys in a private section in the class definition, inline static const QString SomeConfigKey = QStringLiteral("the/key"); This documents the fact which config keys this class uses, plus it allows documentation for each key or group of keys.

modSpike · 2026-01-27T11:28:31Z

src/libsync/config/systemconfig.cpp

+    if (os == QOperatingSystemVersion::Windows) {
+        // we use HKEY_LOCAL_MACHINE\Software\Policies since this is the location whe GPO operates
+        return QString("HKEY_LOCAL_MACHINE\\Software\\Policies\\%1\\%2").arg(theme.vendor(), theme.appNameGUI());
+    }


I don't understand why we alternately use appNameGUI vs appName?

casing - "owncloud" vs "ownCloud"

In registry paths: "HKEY_LOCAL_MACHINE\Software\Policies\ownCloud\ownCloud
On linux: /etc/owncloud/owncloud.ini

Added comments explaining where we use what and why.

modSpike · 2026-01-27T11:28:45Z

src/libsync/config/systemconfig.cpp

+    auto format = Utility::isWindows() ? QSettings::NativeFormat : QSettings::IniFormat;
+    QSettings system(configPath(QOperatingSystemVersion::currentType(), *Theme::instance()), format);
+
+    _allowServerURLChange = system.value(KEY_SETUP_ALLOW_SERVER_URL_CHANGE, true).toBool();


is this right? don't we have the case where there is no value in system config for allow url change BUT theme defines url (kw for sure). In that case allow url change should be false, right?

Fixed in the getter

modSpike · 2026-01-27T11:31:17Z

src/libsync/config/systemconfig.cpp

+    _allowServerURLChange = system.value(KEY_SETUP_ALLOW_SERVER_URL_CHANGE, true).toBool();
+    _serverUrl = system.value(KEY_SETUP_SERVER_URL, QString()).toString();
+    _skipUpdateCheck = system.value(KEY_UPDATER_SKIP_UPDATE_CHECK, false).toBool();
+    _moveToTrash = system.value(KEY_SETUP_MOVE_TO_TRASH, false).toBool();


we also have a user setting = move to trash. How is this system setting different and does it take precedence over the user setting, in which case maybe we need to hide that option in the settings gui or ?

modSpike · 2026-01-27T11:38:04Z

src/libsync/configfile.cpp

+    if (_systemConfig.moveToTrash()) {
+        return true;
+    }
+


yeah per above this imo won't work. Imagine: user has switched move to trash off. does it automatically turn back on in the gui, and either way, why is it still moving to trash? confusion ensues.

I also have doubts about user settings doing this "resolve" behavior since the system config has precedence I would expect the resolution to happen there

Removed the move-to-trash bits, to many open questions. See https://kiteworks.atlassian.net/browse/DC-225?focusedCommentId=1959239

modSpike · 2026-01-27T11:40:52Z

src/libsync/configfile.cpp

-    if (connection.isEmpty())
-        con = defaultConnection();
+    if (_systemConfig.skipUpdateCheck()) {
+        return true;


similar to comment for moveToTrash I think this is very confusing and goes against the idea that system config takes precedence and does the resolving

modSpike · 2026-01-27T11:42:46Z

src/libsync/configfile.cpp

-    fallback = getValue(skipUpdateCheckC(), QString(), fallback);
-
-    QVariant value = getPolicySetting(skipUpdateCheckC(), fallback);
+    QVariant value = getValue(skipUpdateCheckC(), QString(), fallback);


I do not understand this at all. please add a comment to explain it

DeepDiver1975 · 2026-02-03T15:22:35Z

docs have been added: owncloud/docs-client-desktop#681

erikjv · 2026-02-04T14:00:45Z

What's left is the skipUpdateCheck, which has the same problem as the moveToTrash. I propose we remove it for now, so we can close this PR, and do further work in another PR.

About which config value has a higher priority, and which class resolves the: I think there are no settings left that come from the config and the system config. So I propose to fix this in another PR too.

DeepDiver1975 · 2026-02-04T14:31:00Z

What's left is the skipUpdateCheck, which has the same problem as the moveToTrash. I propose we remove it for now, so we can close this PR, and do further work in another PR.

We need to keep this as a functional parity to the currently documented mechanism - see https://doc.owncloud.com/desktop/6.0/automatic_updater.html#preventing-automatic-updates

For the specific case the current implementation is valid

DeepDiver1975 force-pushed the feat/global-configuration branch from cbff648 to f445526 Compare October 10, 2025 14:48

DeepDiver1975 marked this pull request as draft October 10, 2025 14:48

DeepDiver1975 mentioned this pull request Jan 15, 2026

OIDC can't be set up for the hardcoded clients owncloud/ocis#9389

Closed

DeepDiver1975 added 2 commits January 15, 2026 17:09

feat: allow account setup configuration via system based configuratio…

8183ceb

…n mechanisms

feat: introduce class SystemConfig

c3b3e93

DeepDiver1975 changed the title ~~feat: allow account setup configuration via system based configuratio…~~ [DC-135] feat: allow account setup configuration via system based configuratio… Jan 16, 2026

feat: add ConfigResolver as unique access point for Theme and SystemC…

e40cda6

…onfig + add OpenIdConnect configuration

DeepDiver1975 force-pushed the feat/global-configuration branch from f445526 to e40cda6 Compare January 16, 2026 14:08

DeepDiver1975 added 2 commits January 16, 2026 16:26

fix: include path

bbdc9f1

feat: remove Windows specific getPolicySetting and the attempt to rea…

ef47893

…d settings from a system location. skipUpdateCheck is using SystemConfig already. Proxy settings will follow

DeepDiver1975 changed the title ~~[DC-135] feat: allow account setup configuration via system based configuratio…~~ [DC-135] feat: introduce system based configuration aka group policy Jan 16, 2026

Merge branch 'master' into feat/global-configuration

305cd6b

DeepDiver1975 marked this pull request as ready for review January 16, 2026 16:06

DeepDiver1975 added 3 commits January 16, 2026 17:54

fix: TestSystemConfig

b189bee

docs: comment new config classes

988297e

fix: extract config keys as constants

1ba8919

erikjv reviewed Jan 22, 2026

View reviewed changes

src/libsync/config/systemconfig.h Show resolved Hide resolved

DeepDiver1975 added 3 commits January 22, 2026 14:52

fix: OpenID Connect scopes and prompt example config

54fdf07

Merge branch 'master' into feat/global-configuration

00e27be

fix: merge SystemConfig and ConfigResolver

2fe4be6

DeepDiver1975 mentioned this pull request Jan 27, 2026

[7.x] Changes required owncloud/docs-client-desktop#672

Open

13 tasks

modSpike reviewed Jan 27, 2026

View reviewed changes

erikjv self-assigned this Feb 2, 2026

erikjv added 2 commits February 2, 2026 16:55

Incorporate feedback

20b6f5a

Merge branch 'master' into feat/global-configuration

3bd43e9

mmattel mentioned this pull request Feb 3, 2026

Introduce system based configuration owncloud/docs-client-desktop#678

Closed

More feedback fixes

931b6c7

Conversation

DeepDiver1975 commented Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Background

Solution

Further Configuration Values

Uh oh!

update-docs bot commented Oct 9, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DeepDiver1975 commented Feb 3, 2026

Uh oh!

erikjv commented Feb 4, 2026

Uh oh!

DeepDiver1975 commented Feb 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

DeepDiver1975 commented Oct 9, 2025 •

edited

Loading