[Bug]: Can't add an admin user through saml anymore

dgarlans commented 8 months ago

⚠️ This issue respects the following points: ⚠️

[X] This is a bug, not a question or a configuration/webserver/proxy issue.
[X] This issue is not already reported on Github OR Nextcloud Community Forum (I've searched it).
[X] Nextcloud Server is up to date. See Maintenance and Release Schedule for supported versions.
[X] I agree to follow Nextcloud's Code of Conduct.

Bug description

According to documentation, and our use of nextcloud over the last several years, when we have users login to nextcloud via saml using duo, if we want a user to be an admin on the nextcloud instance, we just had to make sure they were a part of the group passed through saml as "admin".

Now, however, we've been setting up some new nextcloud instances and it's not working anymore. We see that the user is a member of a group called "admin", but if we look in the command line through occ user info, we see the user actually gets added to "SAML_admin", instead of the actual "admin" group.

This is happening on 28.0.2 and 28.0.3. It worked on 28.0.1, but after upgrading to 28.0.2 or fresh installing 28.02 or 28.0.3 the incorrect behavior is occurring.

Steps to reproduce

Create new nextcloud instance via docker
Setup SAML login for nextcloud via Duo, include "admin" as one of the "groups"
User logs in, and is created as expected.
User is shown as a member of "admin" group, but is not an admin.
Run occ user:info, and see that group membership is "SAML_admin".
If user is manually added to admin group via a direct-login user, user is shown as being a member of "admin" and "saml_admin"

Expected behavior

User should be added to actual "admin" group and become an administrator. This worked correctly throughout many previous versions going back to at least 17-18.

Installation method

Community Docker image

Nextcloud Server version

28

Operating system

Debian/Ubuntu

PHP engine version

PHP 8.2

Web server

Apache (supported)

Database engine version

PostgreSQL

Is this bug present after an update or on a fresh install?

Fresh Nextcloud Server install

Are you using the Nextcloud Server Encryption module?

Encryption is Disabled

What user-backends are you using?

[ ] Default user-backend (database)
[ ] LDAP/ Active Directory
[X] SSO - SAML
[ ] Other

Configuration report

www-data@4a3f4229e3c5:~/html$ php occ config:list system
{
    "system": {
        "htaccess.RewriteBase": "\/",
        "memcache.local": "\\OC\\Memcache\\APCu",
        "apps_paths": [
            {
                "path": "\/var\/www\/html\/apps",
                "url": "\/apps",
                "writable": false
            },
            {
                "path": "\/var\/www\/html\/custom_apps",
                "url": "\/custom_apps",
                "writable": true
            }
        ],
        "memcache.distributed": "\\OC\\Memcache\\Redis",
        "memcache.locking": "\\OC\\Memcache\\Redis",
        "redis": {
            "host": "***REMOVED SENSITIVE VALUE***",
            "password": "***REMOVED SENSITIVE VALUE***",
            "port": 6379
        },
        "overwriteprotocol": "https",
        "overwrite.cli.url": "http:\/\/nextcloud.testexample.com",
        "trusted_proxies": "***REMOVED SENSITIVE VALUE***",
        "upgrade.disable-web": true,
        "passwordsalt": "***REMOVED SENSITIVE VALUE***",
        "secret": "***REMOVED SENSITIVE VALUE***",
        "trusted_domains": [
            "localhost",
            "vault.cygnusaero.com"
        ],
        "datadirectory": "***REMOVED SENSITIVE VALUE***",
        "dbtype": "pgsql",
        "version": "28.0.3.2",
        "dbname": "***REMOVED SENSITIVE VALUE***",
        "dbhost": "***REMOVED SENSITIVE VALUE***",
        "dbport": "",
        "dbtableprefix": "oc_",
        "dbuser": "***REMOVED SENSITIVE VALUE***",
        "dbpassword": "***REMOVED SENSITIVE VALUE***",
        "installed": true,
        "instanceid": "***REMOVED SENSITIVE VALUE***",
        "loglevel": 2,
        "maintenance": false
    }
}

List of activated Apps

www-data@4a3f4229e3c5:~/html$ php occ app:list
Enabled:
  - activity: 2.20.0
  - circles: 28.0.0-dev
  - cloud_federation_api: 1.11.0
  - comments: 1.18.0
  - contactsinteraction: 1.9.0
  - dashboard: 7.8.0
  - dav: 1.29.1
  - federatedfilesharing: 1.18.0
  - federation: 1.18.0
  - files: 2.0.0
  - files_pdfviewer: 2.9.0
  - files_reminders: 1.1.0
  - files_sharing: 1.20.0
  - files_trashbin: 1.18.0
  - files_versions: 1.21.0
  - firstrunwizard: 2.17.0
  - logreader: 2.13.0
  - lookup_server_connector: 1.16.0
  - nextcloud_announcements: 1.17.0
  - notifications: 2.16.0
  - oauth2: 1.16.3
  - onlyoffice: 9.0.0
  - password_policy: 1.18.0
  - photos: 2.4.0
  - privacy: 1.12.0
  - provisioning_api: 1.18.0
  - recommendations: 2.0.0
  - related_resources: 1.3.0
  - serverinfo: 1.18.0
  - settings: 1.10.1
  - sharebymail: 1.18.0
  - support: 1.11.0
  - survey_client: 1.16.0
  - systemtags: 1.18.0
  - text: 3.9.1
  - theming: 2.3.0
  - twofactor_backupcodes: 1.17.0
  - updatenotification: 1.18.0
  - user_saml: 6.1.1
  - user_status: 1.8.1
  - viewer: 2.2.0
  - weather_status: 1.8.0
  - workflowengine: 2.10.0
Disabled:
  - admin_audit: 1.18.0
  - bruteforcesettings: 2.8.0
  - encryption: 2.16.0
  - files_external: 1.20.0
  - suspicious_login: 6.0.0
  - twofactor_totp: 10.0.0-beta.2
  - user_ldap: 1.19.0

Nextcloud Signing status

No errors have been found.

Nextcloud Logs

No response

Additional info

My initial examination of the relevant code makes it seem like this was a deliberate change, specifically on this line of code: https://github.com/nextcloud/user_saml/blob/181ecd34e1cc24c72108e29e291024a63442d6a6/lib/GroupManager.php#L305

dgarlans commented 8 months ago

https://github.com/nextcloud/user_saml/commit/69c5f8f4211fa14a945399e0cfaefc4307d8256e

It looks like this was done deliberately. "- allow local group modifications in some cases

must not be about admin group"

We use SAML to centrally manage the multifactor authentication for our users and administrators through single sign on. Was this a change implemented specifically to disallow the assignment of administrators via SAML?

blizzz commented 8 months ago

Thanks for reporting. Closing it as a duplicate of https://github.com/nextcloud/user_saml/issues/813

dgarlans commented 8 months ago

Thanks for reporting. Closing it as a duplicate of #813

Some how i missed this one! thanks

nextcloud / user_saml