Introducing user config provisionning api

[skjnldsv]

This is something that we miss but we need to add proper security checks
This is a great addition and can help devs to properly set up ajax edits of their config without creating controllers every time.

What we need to check:

• How do we handle the access:
  • all user should be able to edit their own config
  • but some configs are sensitive and managed by the admin or core (login/lastLogin, settings/email.... etc)

Please discuss and help :)

[skjnldsv]

Here some ideas:

1. Add a verification that checks if the app key requested matches an installed and enabled app this will ensure only proper keys can be defined.
2. Add a 'capability' or some sort to properly allow keys to be user-defined with proper permissions on apps.

[rullzer]

While I'm not against this. Or something that easily allows this. However this does not allow for server side sanity checks.

Add a 'capability' or some sort to properly allow keys to be user-defined with proper permissions on apps.

If we are going that way I would suggest to make a 'basic' Controller (ConfigController) that handles all the basics. With some helper functions in there to make creating it very easy (and standarized).

Else we end up with a huge thing again where apps have to regsiter types of settings that can be set. Possible values etc.

[skjnldsv]

Else we end up with a huge thing again where apps have to regsiter types of settings that can be set. Possible values etc.

Which is already the case if apps have to register ajax entry point for their user config :)
Maybe we could simplify that?

[rullzer]

Well register the endpoint should be the least of the trouble.

Let me think if I can come up with a simple extendable controller that should do the trick

[skjnldsv]

Well register the endpoint should be the least of the trouble.

yes, it's not that big of a trouble, I agree, but having something like a simple function to have in the app declaration is simpler than setting up a full controller, routes and stuff :)

[codecov]

Codecov Report

Merging #9856 into master will decrease coverage by 0.17%.
The diff coverage is 0%.

@@             Coverage Diff              @@
##             master    #9856      +/-   ##
============================================
- Coverage     52.07%   51.89%   -0.18%     
+ Complexity    25946    25779     -167     
============================================
  Files          1645     1634      -11     
  Lines         95839    95471     -368     
  Branches       1290     1309      +19     
============================================
- Hits          49907    49547     -360     
+ Misses        45932    45924       -8

Impacted Files	Coverage Δ	Complexity Δ
...isioning_api/composer/composer/autoload_static.php	0% <ø> (ø)	1 <0> (ø)	⬇️
apps/provisioning_api/appinfo/routes.php	0% <0%> (ø)	0 <0> (ø)	⬇️
...ioning_api/composer/composer/autoload_classmap.php	0% <0%> (ø)	0 <0> (ø)	⬇️
...ioning_api/lib/Controller/UserConfigController.php	0% <0%> (ø)	12 <12> (?)
apps/oauth2/lib/Settings/Admin.php	92.3% <0%> (-7.7%)	4% <0%> (+1%)
apps/theming/lib/ThemingDefaults.php	90% <0%> (-7.3%)	56% <0%> (+4%)
lib/private/Authentication/Token/DefaultToken.php	85.24% <0%> (-6.69%)	20% <0%> (ø)
apps/oauth2/lib/Controller/SettingsController.php	72.41% <0%> (-5.85%)	3% <0%> (-2%)
...B/QueryBuilder/FunctionBuilder/FunctionBuilder.php	77.77% <0%> (-2.23%)	9% <0%> (-1%)
lib/private/User/Database.php	79.47% <0%> (-1.9%)	43% <0%> (+1%)
... and 60 more

[rullzer]

@skjnldsv I had a quick idea for a controller here. To standardize this a bit. basically I prefere to have a controller per app instead of 1 generic thing. As then we can still have server side validation. We could add default validators (int, bool etc). Or custom ones.

Then sure app devs still need to code this. And add the route. But the basics are very simple. And having it standardized helps.

Right now it is far from finished (or being functional). But I think this makes more sense than 1 endpoint where all the magic happens. (also if we want to upgrade something we can just introduce a new controller and deprecate the old one etc).

[skjnldsv]

So my edit in the api is not needed anymore, right?
It's too bad though, It would have been far easier for javascript stuff :)

[rullzer]

@skjnldsv correct.

@skjnldsv well I think this is nicer as you can still do server side checks and validations. We probably still need to properly fix the new controller as I'm not happy with it currently.

Main idea here of having a fully standardized controller. Is that we could write some basic javascript library that devs can then use. Then in the JS the only thing that is needed is the url to the endpoint. Maybe we could even propose a default endpoint for that. (So then it is just the app name).

[MorrisJobke]

Most likely nothing for 14 -> moving to 15.

[skjnldsv]

@MorrisJobke yes, it's on discussion with @rullzer and @blizzz :)

[MorrisJobke]

I guess we need to move this to 16 😢

[skjnldsv]

@MorrisJobke yes, it's on discussion with @rullzer and @blizzz :)

😁

[rullzer]

Removed my extra commit. While I still would like some externable control where user input can be validated. Lets get this stuff in for now. I can then have a look at d401422 at a later time

[ChristophWurst]

How can we prevent users from setting arbitrary config values with this? I'm not sure it's save to have the ability to manipulate all user settings.

I'm not sure I like this approach. This might make it possible to circumvent some permission/security checks that apps handle via user config. Or state changes of the config values that are not expected.

[rullzer]

@ChristophWurst yeah that was my initial tought as well. That is why I started with d401422 but never finished.

[ChristophWurst]

What shall we do? Even if we say this is not the right solution, merging this now will allow some people to use it and it might be very hard to get rid of it again.

I think we should close this and discuss a decent mechanism and implement that.

[MorrisJobke]

Let's close this for now as it seems to be the wrong (or too limited) approach for now. It was a nice idea but had some problems.