Skip to content

Conversation

@printminion-co
Copy link

@printminion-co printminion-co commented Sep 19, 2025

Summary

Use delegation to hide from admins settings allowed only for "super-admin"

# turn on delegation
occ config:system:set --type boolean --value true -- settings.only-delegated-settings

turn off delegation

occ config:system:set --type boolean --value false -- settings.only-delegated-settings

script to delegate all settings to admin group

delegate_all.sh
#!/bin/bash

# Script to add or remove delegation to admin group for all admin sections
# Executes occ commands directly (runs inside container)
# Usage: ./delegate_all.sh [add|remove]
#
# Note: Pre-check logic implemented to workaround bug where groups can be added multiple times
# Bug reference: [Bug]: occ admin-delegation:add allows adding the same delegation multiple times #46609
# https://github.com/nextcloud/server/issues/46609

# Global variable for the group name
GROUP_NAME="admin"

# Check if operation parameter is provided
if [ $# -eq 0 ]; then
    echo "Usage: $0 [add|remove]"
    echo "  add    - Add delegation for all admin sections"
    echo "  remove - Remove delegation for all admin sections"
    exit 1
fi

OPERATION="$1"

# Validate operation parameter
if [ "$OPERATION" != "add" ] && [ "$OPERATION" != "remove" ]; then
    echo "Error: Invalid operation '$OPERATION'. Use 'add' or 'remove'."
    exit 1
fi

if [ "$OPERATION" == "add" ]; then
    echo "Starting delegation process for group '$GROUP_NAME'..."
    ACTION_VERB="Delegating"
    SUCCESS_MSG="Successfully delegated"
    FAIL_MSG="Failed to delegate"
    SKIP_MSG="Skipping (already delegated)"
    OCC_COMMAND="admin-delegation:add"
else
    echo "Starting delegation removal process for group '$GROUP_NAME'..."
    ACTION_VERB="Removing delegation"
    SUCCESS_MSG="Successfully removed delegation"
    FAIL_MSG="Failed to remove delegation"
    SKIP_MSG="Skipping (not delegated)"
    OCC_COMMAND="admin-delegation:remove"
fi

# Get admin delegation info
echo "Fetching admin delegation information..."
CONTAINER_OUTPUT=$(php occ admin-delegation:show --output=json_pretty)

# Check if the command was successful
if [ $? -ne 0 ]; then
    echo "Error: Failed to execute occ command"
    exit 1
fi

echo "Processing delegation data..."

# Parse JSON and process each setting
echo "$CONTAINER_OUTPUT" | jq -r '.[] | .settings[] | "\(.className)|\(.delegatedGroups | join(","))"' | while IFS='|' read -r className delegatedGroups; do
    if [ -n "$className" ]; then
        # Check if the group is already in the delegated groups
        group_already_delegated=false
        if [[ ",$delegatedGroups," == *",$GROUP_NAME,"* ]]; then
            group_already_delegated=true
        fi

        if [ "$OPERATION" == "add" ]; then
            if [ "$group_already_delegated" == "true" ]; then
                echo "$SKIP_MSG: $className"
                continue
            fi
            echo "$ACTION_VERB: $className"
        else
            if [ "$group_already_delegated" == "false" ]; then
                echo "$SKIP_MSG: $className"
                continue
            fi
            echo "$ACTION_VERB: $className"
        fi

        # Execute the delegation command
        php occ "$OCC_COMMAND" "$className" "$GROUP_NAME"

        # Check if delegation was successful
        if [ $? -eq 0 ]; then
            echo "$SUCCESS_MSG: $className"
        else
            echo "$FAIL_MSG: $className"
        fi
    fi
done

if [ "$OPERATION" == "add" ]; then
    echo "Delegation process completed!"
else
    echo "Delegation removal process completed!"
fi

# Show final status
echo ""
echo "Final delegation status:"
php occ admin-delegation:show --output=json_pretty

To run tests use the

tests/phpunit-autotest-settings.xml
<?xml version="1.0" encoding="utf-8" ?>
<!--
 - SPDX-FileCopyrightText: 2014-2016 ownCloud, Inc.
 - SPDX-License-Identifier: AGPL-3.0-only
-->
<phpunit bootstrap="bootstrap.php"
		 verbose="true"
		 timeoutForSmallTests="900"
		 timeoutForMediumTests="900"
		 timeoutForLargeTests="900"
>
	<testsuite name='ownCloud settings external'>
		<directory suffix=".php">../apps/settings/tests</directory>
	</testsuite>
	<!-- filters for code coverage -->
	<filter>
		<whitelist>
			<directory suffix=".php">../apps/settings</directory>
			<exclude>
				<directory suffix=".php">../apps/settings/l10n</directory>
				<directory suffix=".php">../apps/settings/tests</directory>
			</exclude>
		</whitelist>
	</filter>
</phpunit>
phpunit --configuration tests/phpunit-autotest-settings.xml

TODO

  • ...

Checklist

@printminion-co printminion-co changed the title IONOS(admin-delegation): add output option for show command to suppor… IONOS(admin-delegation): add output option for show command to support JSON formats Sep 22, 2025
@printminion-co printminion-co force-pushed the mk/dev/admin_delegation branch 4 times, most recently from 6354f42 to 1085e55 Compare September 29, 2025 12:20
@printminion-co printminion-co changed the title IONOS(admin-delegation): add output option for show command to support JSON formats extend admin-delegation Sep 29, 2025
@printminion-co printminion-co force-pushed the mk/dev/admin_delegation branch 3 times, most recently from 2e4472d to 075bd0a Compare September 30, 2025 14:20
@printminion-co printminion-co marked this pull request as ready for review September 30, 2025 14:39
@bromiesTM bromiesTM requested a review from Copilot October 8, 2025 06:24
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR extends the admin delegation functionality to hide admin settings from regular admins when only delegated settings are enabled. The feature introduces a system configuration that allows super-admins to restrict access to admin settings, making them visible only through delegation mechanisms.

  • Adds support for settings.only-delegated-settings system configuration to control delegation behavior
  • Modifies navigation to show unified Settings entry instead of separate personal/admin entries when delegation is enabled
  • Updates admin settings filtering logic to respect the delegation configuration

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/lib/Settings/ManagerTest.php Adds test coverage for delegation-aware admin settings filtering
tests/lib/NavigationManagerTest.php Tests navigation behavior with delegated settings enabled/disabled
lib/private/Settings/Manager.php Implements delegation logic in getAllowedAdminSettings method
lib/private/NavigationManager.php Modifies navigation entries based on delegation configuration
apps/settings/tests/Settings/Admin/DelegationTest.php New test file for Delegation settings class
apps/settings/lib/Settings/Admin/Delegation.php Updates Delegation class to implement IDelegatedSettings
apps/settings/lib/Command/AdminDelegation/Show.php Enhances show command with structured output formats

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@printminion-co printminion-co force-pushed the mk/dev/admin_delegation branch from 075bd0a to 7747522 Compare October 9, 2025 12:34
@printminion-co printminion-co requested a review from Copilot October 9, 2025 15:13
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +109 to +114
->withConsecutive(
['available-settings', []],
['available-groups', [['displayName' => 'Users', 'gid' => 'users']]],
['authorized-groups', []],
['authorized-settings-doc-link', 'https://docs.example.com/admin-delegation']
);
Copy link

Copilot AI Oct 10, 2025

Choose a reason for hiding this comment

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

The withConsecutive() method is deprecated in newer PHPUnit versions. Consider using separate with() expectations or the willReturnCallback() method to verify the calls individually.

Copilot uses AI. Check for mistakes.
@printminion-co printminion-co force-pushed the mk/dev/admin_delegation branch from 9166bdb to df877da Compare October 10, 2025 14:32
…edGroupService

Signed-off-by: Misha M.-Kupriyanov <kupriyanov@strato.de>
Signed-off-by: Misha M.-Kupriyanov <kupriyanov@strato.de>
…tedSettings

Signed-off-by: Misha M.-Kupriyanov <kupriyanov@strato.de>
…r and NavigationManager

Signed-off-by: Misha M.-Kupriyanov <kupriyanov@strato.de>
…tructor

Prevent initSettingState to reload already filtered delegated states of settingManager for current user.

Fixes rendering of delegated sections in apps/settings/templates/settings/frame.php
While browsing to /settings/admin/admindelegation

Signed-off-by: Misha M.-Kupriyanov <kupriyanov@strato.de>
Signed-off-by: Misha M.-Kupriyanov <kupriyanov@strato.de>
…ings for 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>
…ttings for non-admin users

Signed-off-by: Misha M.-Kupriyanov <kupriyanov@strato.de>
@printminion-co printminion-co force-pushed the mk/dev/admin_delegation branch from df877da to f13355f Compare October 10, 2025 16:26
@printminion-co printminion-co merged commit 423d91a into ionos-dev Oct 13, 2025
35 of 42 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants