[Bug]: Group cache is not invalidated correctly, leading to shares not being accepted automatically

[sirkrypt0]

⚠️ This issue respects the following points: ⚠️

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

Bug description

When a plugin such as user_oidc or nextcloud-oidc-login is used and groups are synchronizing via them, folder shares that are shared with a new group of that user are not accepted automatically. Instead, users have to accept them manually by checking the pending shares (https://nextcloud.example.com/apps/files/pendingshares), even though the settings to accept the shares automatically are all correct. This is described in issues in the respective plugins, namely nextcloud/user_oidc#827 and pulsejet/nextcloud-oidc-login#256.
As described in the issues, this was working with Nextcloud 27.

Tracing the problem

However, those issues are only a symptom to an underlying problem with the caching of the user groups in the GroupManager. Consider the following (pseudoish) code:

// Get groups of the user and do something with them, e.g. check which groups a user does not belong to yet.
$userGroups = $groupManager->getUserGroups($user);
// User didn't belong to group, so add them.
$someGroup->addUser($user);

This is what happens in both plugins and seems perfectly logic. Except, this leads to the shares not being accepted automatically.

The call to getUserGroups ultimately leads to getUserIdGroupIds of the manager:

server/lib/private/Group/Manager.php

Lines 374 to 386 in 6b85a3a

	private function getUserIdGroupIds(string $uid): array {
	if (!isset($this->cachedUserGroups[$uid])) {
	$groups = [];
	foreach ($this->backends as $backend) {
	if ($groupIds = $backend->getUserGroups($uid)) {
	$groups = array_merge($groups, $groupIds);
	}
	}
	$this->cachedUserGroups[$uid] = $groups;
	}
	return $this->cachedUserGroups[$uid];
	}

If the user hasn't been cached yet, their groups are loaded and stored in the cache.

Now, we add the user to a new group through a call to addUser:

server/lib/private/Group/Group.php

Lines 149 to 172 in 6b85a3a

	public function addUser(IUser $user): void {
	if ($this->inGroup($user)) {
	return;
	}
	$this->dispatcher->dispatchTyped(new BeforeUserAddedEvent($this, $user));
	if ($this->emitter) {
	$this->emitter->emit('\OC\Group', 'preAddUser', [$this, $user]);
	}
	foreach ($this->backends as $backend) {
	if ($backend->implementsActions(\OC\Group\Backend::ADD_TO_GROUP)) {
	$backend->addToGroup($user->getUID(), $this->gid);
	$this->users[$user->getUID()] = $user;
	$this->dispatcher->dispatchTyped(new UserAddedEvent($this, $user));
	if ($this->emitter) {
	$this->emitter->emit('\OC\Group', 'postAddUser', [$this, $user]);
	}
	return;
	}
	}
	}

This adds the user to the group and notifies others through events before and after that.
The UserAddedEvent is handled by the UserAddedToGroupListener from the files_sharing app and gets all shares the user has access to from the share manager, to automatically accept them:

server/apps/files_sharing/lib/Listener/UserAddedToGroupListener.php

Lines 47 to 57 in 6b85a3a

	$shares = $this->shareManager->getSharedWith($user->getUID(), IShare::TYPE_GROUP, null, -1);
	foreach ($shares as $share) {
	// If this is not the new group we can skip it
	if ($share->getSharedWith() !== $group->getGID()) {
	continue;
	}
	// Accept the share if needed
	$this->shareManager->acceptShare($share, $user->getUID());
	}

The share manager asks its providers and the call ends up in the DefaultShareProvider, which asks the group manager for the users groups again.

server/lib/private/Share20/DefaultShareProvider.php

Line 897 in 6b85a3a

$allGroups = $this->groupManager->getUserGroupIds($user);

And at this point, the user is already in the user group cache of the group manager, but without the new group, so shares belonging to the new group are not returned yet and thus can also not be automatically accepted by the UserAddedToGroupListener.

Only when the postAddUser event is fired in the group->addUser function will the group manager clear the cache, but it's too late already.

server/lib/private/Group/Manager.php

Lines 72 to 74 in 6b85a3a

	$this->listen('\OC\Group', 'postAddUser', function (IGroup $group): void {
	$this->cachedUserGroups = [];
	});

Potential solutions

To fix this, the cachedUserGroups must be cleared correctly. Here are some options I though about.

Before the user is added to the group

This is easy to implement by adding the following listener to the group manager:

$this->listen('\OC\Group', 'preAddUser', function ($group) use (&$cachedUserGroups) {
        /**
          * @var \OC\Group\Group $group
          */
        $cachedUserGroups = [];
});

I can confirm that this works and remediates the issue for now. Nevertheless, I think it could still be that some other class also listens to the preAddUser event and is executed after the preAddUser listener of the group manager. If that class would then call the group manager again to populate the cache (which doesn't seem to be the case right now, but might happen unknowingly in the future), we are back to the original problem.

After the user is added to the group

This is how it's handled right now via the postAddUser event, but this is handled after the type UserAddedEvent and leads to the current issue.

I think ideally, we would clear the cache right after the call to $backend->addToGroup to avoid that new calls to the group manager "poison" the cache before it can pick up the new group. But since the cache is an implementation detail of the concrete Manager class and not part of the IGroupManager, we can't simply call something like $manager->invalidateCache and have to rely on the event instead.

Therefore, we either need to ensure that the group manager handles the UserAddedEvent first or add another event which for now is only listened to by the group manager to clean up the cache. The caching side effects should then be documented to ensure that users listen to the UserAddedEvent if they need a consistent view on the groups.

Extending the IGroupManager interface with a new function

We could somewhat expose the caching detail of the group manager into the interface and add a new function either to flush the cache or to get the groups without caching logic (which could default to calling the usual function if the concrete implementation doesn't support caching).

Steps to reproduce

1. Setup a Nextcloud 29 (others report the issue with 28 as well)
2. Configure either the user_oidc or the nextcloud-oidc-login plugin with your IdP and enable group provisioning.
3. Log in to the Nextcloud via the IdP. Make sure that the user has some groups assigned. Log out.
4. Log in as the admin user, create a folder and share it with one of the groups the user has assigned.
5. In the Nextcloud user overview, remove the group you shared the folder with from the users groups. The group will be assigned to the user again in the next login.
6. Log back in as the normal user. Observe that the shared folder needs to be accepted manually.

Alternatively, if you don't want to setup an OIDC plugin, you can modify the code of your Nextcloud to replicate the behavior by adding $this->groupManager->getUserGroupIds($targetUser); before this call to addUser:

server/apps/provisioning_api/lib/Controller/UsersController.php

Lines 1433 to 1434 in 6b85a3a

	// Add user to group
	$group->addUser($targetUser);

Then, adding users to groups via the Nextcloud UI won't accept the shares automatically as well. Removing the line will make it work again.

Expected behavior

Shares should be accepted automatically if configured so.

But precisely, calling getUserGroupIds or similar before adding a user to a new group should not have unexpected side-effects like caches not being flushed correctly and later calls to get the users groups returning stale data.

Nextcloud Server version

29

Operating system

None

PHP engine version

None

Web server

None

Database engine version

None

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

Fresh Nextcloud Server install

Are you using the Nextcloud Server Encryption module?

None

What user-backends are you using?

• Default user-backend (database)
• LDAP/ Active Directory
• SSO - SAML
• Other

Configuration report

{
    "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
        },
        "upgrade.disable-web": true,
        "instanceid": "***REMOVED SENSITIVE VALUE***",
        "passwordsalt": "***REMOVED SENSITIVE VALUE***",
        "secret": "***REMOVED SENSITIVE VALUE***",
        "trusted_domains": [
            "localhost:8080"
        ],
        "datadirectory": "***REMOVED SENSITIVE VALUE***",
        "dbtype": "sqlite3",
        "version": "29.0.5.1",
        "overwrite.cli.url": "http:\/\/localhost:8080",
        "installed": true,
        "oidc_login_disable_registration": false,
        "oidc_create_groups": true,
        "oidc_login_client_id": "nextcloud",
        "oidc_login_client_secret": "redacted",
        "oidc_login_provider_url": "https:\/\/127.0.0.1\/realms\/playground",
        "oidc_login_logout_url": false,
        "oidc_login_redir_fallback": true,
        "oidc_login_attributes": {
            "id": "sub",
            "mail": "email",
            "groups": "groups"
        },
        "oidc_login_hide_password_form": false,
        "oidc_login_tls_verify": true,
        "oidc_login_webdav_enabled": true
    }
}

List of activated Apps

Enabled:
  - activity: 2.21.1
  - circles: 29.0.0-dev
  - cloud_federation_api: 1.12.0
  - comments: 1.19.0
  - contactsinteraction: 1.10.0
  - dashboard: 7.9.0
  - dav: 1.30.1
  - federatedfilesharing: 1.19.0
  - federation: 1.19.0
  - files: 2.1.1
  - files_downloadlimit: 2.0.0
  - files_pdfviewer: 2.10.0
  - files_reminders: 1.2.0
  - files_sharing: 1.21.0
  - files_trashbin: 1.19.0
  - files_versions: 1.22.0
  - firstrunwizard: 2.18.0
  - logreader: 2.14.0
  - lookup_server_connector: 1.17.0
  - nextcloud_announcements: 1.18.0
  - notifications: 2.17.0
  - oauth2: 1.17.0
  - oidc_login: 3.1.1
  - password_policy: 1.19.0
  - photos: 2.5.0
  - privacy: 1.13.0
  - provisioning_api: 1.19.0
  - recommendations: 2.1.0
  - related_resources: 1.4.0
  - serverinfo: 1.19.0
  - settings: 1.12.0
  - sharebymail: 1.19.0
  - support: 1.12.0
  - survey_client: 1.17.0
  - systemtags: 1.19.0
  - text: 3.10.1
  - theming: 2.4.0
  - twofactor_backupcodes: 1.18.0
  - updatenotification: 1.19.1
  - user_status: 1.9.0
  - viewer: 2.3.0
  - weather_status: 1.9.0
  - workflowengine: 2.11.0
Disabled:
  - admin_audit: 1.19.0
  - bruteforcesettings: 2.9.0
  - encryption: 2.17.0
  - files_external: 1.21.0
  - suspicious_login: 7.0.0
  - twofactor_totp: 11.0.0-dev
  - user_ldap: 1.20.0

Nextcloud Signing status

No errors have been found.

Nextcloud Logs

No response

Additional info

No response

[kesselb]

cc @nextcloud/server-backend

[joshtrichards]

Related: #45036 & #45034