Implement Admin Delegation

[CarlSchwan]

fix #11687

Current status

• settings can be marked as "allow-delegation" in the app info.xml file

• Once marked as allow-delegation they are available in a new setting category in the admin category. In it an admin can select some groups and give them access to the specified setting

• Once a normal user get access, they can see the new setting in their setting navigation bar and access it

• Controller using the new annotation in their action @AuthorizedAdminSetting(OCA/<app>/Settings/AdminSettings.php) can be used by the user (who has the authorization for the admin)

• sorting of settings in UI

• English name Admin Right Permissions/Admin Right Priviledges/Access Control?

[juliusknorr]

Just skimmed it quickly, seems good from the general approach

[juliusknorr]

When the app is disabled, the authorization should disappears too, not so sure how I want to do that yet unfortunately

Any specific reason you went for a app that just exposes the delegation settings? I think that could probably also just be part of settings and we can have a configuration option then to turn it off.

[CarlSchwan]

So my next big problem to solve is how to handle the setAppValue and the http://localhost/ocs/v2.php/apps/provisioning_api/api/v1/config/apps/core/<key> API. Currently the bigger problem is that an appvalue is not mapped to any setting, so if a user has authorization for a setting page, we can't know which appValue they are authorized to edit.

I believe adding a simple mapping ['key1" => AdminSetting::class, "key2" => ShareAdminSetting::class] somewhere and use that to check if the user has could work but that sounds like a big change, unfortunately. Does anyone have a better idea? :)

[PVince81]

when setting up from scratch I get this on the CLI:

PHP Fatal error:  Cannot declare class OCA\AdminRightSubgranting\Migration\Version23000Date20210721100600, because the name is already in use in /srv/www/htdocs/server/core/Migrations/Version23000Date20210721100600.php on line 32

[PVince81]

@CarlSchwan do I understand correctly that this allows only a whole page of settings or a section, not specific setting strings ?

[juliusknorr]

So my next big problem to solve is how to handle the setAppValue and the http://localhost/ocs/v2.php/apps/provisioning_api/api/v1/config/apps/core/<key> API. Currently the bigger problem is that an appvalue is not mapped to any setting, so if a user has authorization for a setting page, we can't know which appValue they are authorized to edit.

I believe adding a simple mapping ['key1" => AdminSetting::class, "key2" => ShareAdminSetting::class] somewhere and use that to check if the user has could work but that sounds like a big change, unfortunately. Does anyone have a better idea? :)

I don't have a good idea right away, but that is somewhat related to the idea from #18465 where with a declarative definition of app config values we could indicate delegation of certain config keys. So maybe some kind of mapping could be an intermediate step to allow delegation of settings for now where it is required.

[CarlSchwan]

@CarlSchwan do I understand correctly that this allows only a whole page of settings or a section, not specific setting strings ?

@PVince81 Yes it allows a while page of settings and will display a section if at least one of the setting inside of it is authorized. For the specific setting strings, it's needed for the ISettings class to declare which setting string they can edit so that it's possible to modify these settings.

I don't have a good idea right away, but that is somewhat related to the idea from #18465 where with a declarative definition of app config values we could indicate delegation of certain config keys. So maybe some kind of mapping could be an intermediate step to allow delegation of settings for now where it is required.

@juliushaertl The linked issue looks interesting. If we decide that this is something we want, it would probably be better to do it before finishing this feature otherwise we will have two mappings :(

[skjnldsv]

Added a few comments that are no blockers imho.
Let's fix the CI first.

[CarlSchwan]

/compile amend /

[PVince81]

👍 code looks good

[danxuliu]

@CarlSchwan There is a change that was going to be reverted, but in the end it was not:
https://github.com/nextcloud/server/pull/28189/files#diff-50b846648bbc8a7ca74bae8cf096ee0297423bc236f56ba05c5f8a727519f615L109-R130
Was that just an oversight or is there a reason?

Due to that change if an app specifies a class that does not exist in the <admin-section> field of info.xml the settings will not load. Of course in that case the app should be fixed :-) but until that happens it would be better if the rest of the settings still work and the error is simply logged.

[CarlSchwan]

@CarlSchwan There is a change that was going to be reverted, but in the end it was not: https://github.com/nextcloud/server/pull/28189/files#diff-50b846648bbc8a7ca74bae8cf096ee0297423bc236f56ba05c5f8a727519f615L109-R130 Was that just an oversight or is there a reason?

Due to that change if an app specifies a class that does not exist in the <admin-section> field of info.xml the settings will not load. Of course in that case the app should be fixed :-) but until that happens it would be better if the rest of the settings still work and the error is simply logged.

totally an oversight, I created #32655

[danxuliu]

@CarlSchwan There is a change that was going to be reverted, but in the end it was not: https://github.com/nextcloud/server/pull/28189/files#diff-50b846648bbc8a7ca74bae8cf096ee0297423bc236f56ba05c5f8a727519f615L109-R130 Was that just an oversight or is there a reason?
Due to that change if an app specifies a class that does not exist in the <admin-section> field of info.xml the settings will not load. Of course in that case the app should be fixed :-) but until that happens it would be better if the rest of the settings still work and the error is simply logged.

totally an oversight, I created #32655

Perfect, thanks!