Skip to content

Display delegated section in common settings for delegated non-admin#55511

Open
printminion-co wants to merge 1 commit into
nextcloud:masterfrom
IONOS-Productivity:fix/display_delegated_admin_settings_to_non-admins
Open

Display delegated section in common settings for delegated non-admin#55511
printminion-co wants to merge 1 commit into
nextcloud:masterfrom
IONOS-Productivity:fix/display_delegated_admin_settings_to_non-admins

Conversation

@printminion-co
Copy link
Copy Markdown
Contributor

Summary

IDeclarativeSettingsForm can't be delegated and can be shown only to admin. Lets not load IDeclarativeSettings for non admins. Otherwise we get "Access forbidden" while displaying /settings/admin and prevent to show delegated sections to non admin like:

Background jobs OCA\Settings\Settings\Admin\Server
Email server OCA\Settings\Settings\Admin\Mail

Before state

# create group1
occ group:add group1

# create user  
OC_PASS=g1user1 ./occ user:add --password-from-env --display-name g1user1 --group group1  -- g1user1


# delegate Mail setting so group1 
./occ admin-delegation:add OCA\\Settings\\Settings\\Admin\\Mail group1
Selection_20251002-004

After state

Selection_20251002-003

Todo

  • validate delegated configurations can be used by group users with delegation

Checklist

@come-nc come-nc self-assigned this Oct 7, 2025
@come-nc come-nc self-requested a review October 7, 2025 08:44
@come-nc come-nc removed their assignment Oct 7, 2025
@come-nc come-nc marked this pull request as ready for review October 7, 2025 08:50
@come-nc come-nc requested a review from a team as a code owner October 7, 2025 08:50
@come-nc come-nc requested review from provokateurin and salmart-dev and removed request for a team October 7, 2025 08:50
@come-nc come-nc marked this pull request as draft October 7, 2025 08:51
@come-nc
Copy link
Copy Markdown
Contributor

come-nc commented Oct 7, 2025

(sorry, I confused tabs, ignore my last changes here)

Copy link
Copy Markdown
Member

@provokateurin provokateurin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment thread apps/settings/lib/Controller/CommonSettingsTrait.php Outdated
@printminion-co printminion-co force-pushed the fix/display_delegated_admin_settings_to_non-admins branch from f942e16 to 189c703 Compare October 13, 2025 09:02
Comment thread apps/settings/lib/Controller/CommonSettingsTrait.php Outdated
@github-actions
Copy link
Copy Markdown
Contributor

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

@printminion-co printminion-co force-pushed the fix/display_delegated_admin_settings_to_non-admins branch 3 times, most recently from 2643ab1 to 2335869 Compare April 29, 2026 13:56
@printminion-co printminion-co marked this pull request as ready for review April 29, 2026 13:56
@printminion-co printminion-co force-pushed the fix/display_delegated_admin_settings_to_non-admins branch from 2335869 to 09204a1 Compare April 30, 2026 12:03
@printminion-co printminion-co changed the title Display delegated section in common settings for non-admin Display delegated section in common settings for delegated non-admin Apr 30, 2026
@artonge artonge requested a review from provokateurin April 30, 2026 12:26
Copy link
Copy Markdown
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello, thank you for the PR.
Could you instead use a try/catch around the call to getFormsWithValues ?
If you catch OC\AppFramework\Middleware\Security\Exceptions\NotAdminException you can simply continue with an empty array for declarative settings.

That avoid duplicating the logic for testing admin rights, and if later declarative settings support delegation it will keep working.

@printminion-co printminion-co force-pushed the fix/display_delegated_admin_settings_to_non-admins branch from 09204a1 to 416aec0 Compare May 5, 2026 14:37
…or non-admin

IDeclarativeSettingsForm can't be delegated and can be shown only to admin.
Lets not load IDeclarativeSettings for non admins.
Otherwise we get "Access forbidden" while displaying /settings/admin
and prevent to show delegated sections to non admin like:

Background jobs   OCA\Settings\Settings\Admin\Server
Email server      OCA\Settings\Settings\Admin\Mail

Signed-off-by: Misha M.-Kupriyanov <kupriyanov@strato.de>
@printminion-co printminion-co force-pushed the fix/display_delegated_admin_settings_to_non-admins branch from 416aec0 to 3d4d5fe Compare May 18, 2026 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants