[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

See DC-228.

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

src/gui/creds/credentials.cpp

src/gui/creds/oauth.cpp

src/gui/newaccountwizard/urlpagecontroller.cpp

src/libsync/config/systemconfig.cpp

src/libsync/configfile.cpp

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

From the comments in the main PR, if this is going to stay I would like the value resolution to move to the SystemConfig so we don't have cross dependencies, and there is no chance of getting the wrong one (eg the systemconfig value vs the configfile value, which may be different)

It is going to stay, so the evaluation and availability of skipupdatecheck value needs to move to system config impl

I think we should also consider a different name for that class since it's more like a collection of fixed app values, which may or may not come from an actual system config, so the current name is misleading.

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

this actually needs to move to the system config so maybe it just goes away, hope hope!

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

modSpike · 2026-02-06T10:27:14Z

src/libsync/config/systemconfig.cpp

+
+bool SystemConfig::allowServerUrlChange() const
+{
+    // If a theme provides a hardcoded URL, do not allow for URL change.


is this correct? My understanding was: if the system config defines the url we should take that value AND the allow change value.

else the theme is in play and any theme url = absolutely you can't change it

this seems to mix them?

I wanted this to behave exactly as described. If a URL is hardcoded in the theme that is it.

then please update the description to make that abundantly clear. this detail should also go in the jira story for this pr

along with an explanation of how this jives with PMFR-499

Per group discussion we decided that indeed, if a system config override is allowed, and a URL preset is defined in the system config, this always takes priority over any other setting.

modSpike · 2026-02-06T10:28:21Z

src/libsync/config/systemconfig.cpp

+    // a theme can provide a hardcoded url which is not subject of change by definition
+    auto serverUrl = Theme::instance()->overrideServerUrlV2();
+    if (!serverUrl.isEmpty()) {
+        return serverUrl;


I think this is wrong, too. The idea of the system config is it can "override" anything else. if we take the theme url by default, Kiteworks distributions can never override it with this impl

see above - same explanation

per latest above: I am not sure why this should be a system config setting at all.

If org provides any url (via config or theme) the url should not be editable. If it's not that simple I need to know why.

per discussion we do need this value and it will be evaluated according to scheme described in DC-228

modSpike · 2026-02-06T10:35:21Z

src/libsync/config/systemconfig.cpp

+    return _skipUpdateCheck;
+}
+
+OpenIdConfig SystemConfig::openIdConfig() const


openIdConfig should have an isEmpty/isValid (or similar) impl so we don't have to magically guess which values can be undefined

in fact all possible values need to be defined to pass an "isValid" check. We need this to validate system config then fall back to defaults if the config is mis-defined

also I would fill the member with values from the theme if that is where they are coming from so it doesn't need to be reevaluated on subsequent gets

modSpike · 2026-02-06T10:41:53Z

src/libsync/configfile.cpp

 #include "common/asserts.h"
 #include "common/utility.h"
 #include "common/version.h"
+


stinky!

see my previous comments about avoiding circular deps between the settings classes. I think we have decided any values "in common" should be resolved in the systemConfig from here out

src/libsync/config/systemconfig.cpp

modSpike · 2026-02-06T13:09:53Z

meet results = we have a theme switch that determines whether sysconfig can override other app settings. If this is true, we always take system config value as highest precendence. If no value is provided in the config, we fall back to previous methods of determining the value.

this pr should be updated to refer to DC-228 since it has full details of the reqs.

modSpike · 2026-02-24T15:53:31Z

src/libsync/config/systemconfig.cpp

+    auto theme = Theme::instance();
+
+    if (theme->allowSystemConfigOverrides()) {
+        if (system.contains(OidcClientIdKey) || system.contains(OidcClientSecretKey) || system.contains(OidcPortsKey) || system.contains(OidcScopesKey)


I don't understand the bulk check using ||
Just because one of those exists in the system config doesn't mean any of the others will be there. I don't understand the purpose.

I think for this one in particular, since we really want ALL values to be filled even with empty strings, I would just do system.value(key, QString()).toString on each of these, do the ports thing separate, then load it all into an openIdConfig and check isValid.

if not load from theme.

I would also break this down into loadOpenIdConfigFromConfig and loadOpenIdConfigFromTheme as this is pretty difficult to read.

openIdConfig fromSystem = loadOpenIdConfigFromConfig
if fromSystem.isValid
_openIdConfig = fromSystem
else
_openIdConfig = loadOpenIdConfigFromTheme

is a lot simpler to understand

modSpike · 2026-02-24T15:58:43Z

src/libsync/config/systemconfig.cpp

        // ini is used on macOS in contrary to plist because they are easier to maintain.
        // Note: rev-domain notation and lowercase is typically used.
-        return QString("/Library/Preferences/%1/%2.ini").arg(theme.orgDomainName(), theme.appName());
+        return QStringLiteral("/Library/Preferences/%1/%2.ini").arg(theme.orgDomainName(), theme.appName());


what's with putting the QStringLiterals back?

modSpike · 2026-02-24T16:10:03Z

src/libsync/config/systemconfig.cpp

 SystemConfig::SystemConfig()
 {
+    const bool allowSystemConfigOverrides = Theme::instance()->allowSystemConfigOverrides();
    auto format = Utility::isWindows() ? QSettings::NativeFormat : QSettings::IniFormat;


why load the QSettings if allowSystemConfigOverrides is false?
Later thought: I think the most readable impl here would be:

load the values from theme (or wherever default should come from). then

if allowOverrides:
load the settings.
if settings.contains(key) replace what came from the theme with the override

this eliminates some really ugly if/else evals AND makes the override concept really clear.

modSpike · 2026-02-24T16:15:09Z

src/libsync/config/systemconfig.cpp

+        // If a theme provides a hardcoded URL, do not allow for URL change.
+        _allowServerURLChange = Theme::instance()->overrideServerUrlV2().isEmpty();
+    }
    _skipUpdateCheck = system.value(UpdaterSkipUpdateCheckKey, false).toBool();


eh - is it really safe to default skip update check this way? meaning if we are allowing the config to set this, if the value doesn't exist is it really just as simple as "false" (after all the rigamarole around this I really am not sure anymore). It also needs some explicit value anyway in the case that allowOverrides is false, in which case the settings should not even be checked

modSpike · 2026-02-24T16:15:50Z

src/gui/creds/credentials.cpp

 #include "accessmanager.h"
 #include "account.h"
+#include "config/systemconfig.h"
 #include "configfile.h"


can this go?

modSpike · 2026-02-24T16:53:05Z

src/libsync/configfile.cpp

 }

-bool ConfigFile::skipUpdateCheck() const
-{


point me to where the old config value is scrubbed from the user file please - I haven't seen it

modSpike · 2026-02-24T16:54:32Z

src/libsync/configfile.h

    void setValue(const QString &key, const QVariant &value);

 private:
-    SystemConfig _systemConfig;


YES! thank you

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

Closed

13 tasks

modSpike reviewed Jan 27, 2026

View reviewed changes

src/gui/creds/credentials.cpp Outdated Show resolved Hide resolved

modSpike reviewed Jan 27, 2026

View reviewed changes

src/gui/creds/oauth.cpp Outdated Show resolved Hide resolved

modSpike reviewed Jan 27, 2026

View reviewed changes

src/gui/newaccountwizard/urlpagecontroller.cpp Outdated Show resolved Hide resolved

modSpike reviewed Jan 27, 2026

View reviewed changes

src/libsync/config/systemconfig.cpp Outdated Show resolved Hide resolved

modSpike reviewed Jan 27, 2026

View reviewed changes

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

modSpike reviewed Jan 27, 2026

View reviewed changes

src/libsync/config/systemconfig.cpp Outdated Show resolved Hide resolved

modSpike reviewed Jan 27, 2026

View reviewed changes

src/libsync/config/systemconfig.cpp Outdated Show resolved Hide resolved

modSpike reviewed Jan 27, 2026

View reviewed changes

src/libsync/configfile.cpp Outdated Show resolved Hide resolved

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

modSpike reviewed Feb 6, 2026

View reviewed changes

src/libsync/config/systemconfig.cpp Outdated Show resolved Hide resolved

modSpike and others added 2 commits February 20, 2026 15:30

Merge branch 'master' into feat/global-configuration

7bb3be4

Change system config handling to the new spec

cd43ad3

modSpike reviewed Feb 24, 2026

View reviewed changes

Comments

Conversation

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

Uh oh!

Background

Solution

Uh oh!

update-docs bot commented Oct 9, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

DeepDiver1975 commented Feb 3, 2026

Uh oh!

erikjv commented Feb 4, 2026

Uh oh!

DeepDiver1975 commented Feb 4, 2026

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!

modSpike Feb 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

modSpike Feb 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Uh oh!

modSpike commented Feb 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

modSpike Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

modSpike Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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 Oct 9, 2025 •

edited by erikjv

Loading

modSpike Feb 8, 2026 •

edited

Loading

modSpike Feb 6, 2026 •

edited

Loading

modSpike commented Feb 6, 2026 •

edited

Loading

modSpike Feb 24, 2026 •

edited

Loading

modSpike Feb 24, 2026 •

edited

Loading