fix(usergroups): clear cached user groups on pre hooks

[iLinaza]

• Resolves: [Bug]: Shared folders don’t show up for new users created by SAML #45034

Summary

Groups Manager stores the cache of which groups the users belong to. This can be inconsistent at certain times as it is cleared late (in post hooks).

When a user is added to a group the following steps are taken (in OC\Group\Group, addUser method):

1. The preAddUser hook is called.
2. The backend is called to actually add the user to the group.
3. The UserAddedEvent event is dispatched.
4. The postAddUser hook is called.

The cache is cleared in the postAddUser hook, in the Manager. When someone performs operations on the UserAddedEvent event and is checking which groups the user belongs to, the Manager may be returning invalid data since the cache may be in an inconsistent state. The user has already been added to the group but the cache is cleared later, in the postAddUser hook, and the manager returns outdated group list.

Before the cache is cleared, the file_sharing app is doing the sharing of folders to the newly added user (in OCA\Files_Sharing\Listener\UserAddedToGroupListener), in the UserAddedEvent event. But, at this point, when the user is created by SAML, the cache is in a incosistent state because it has not been cleared yet and is being retrieved from cache. It will be cleared later, in the postAddUser hook.

Checklist

• Code is properly formatted
• Sign-off message is added to all commits
• Tests (unit, integration, api and/or acceptance) are included
• Screenshots before/after for front-end changes
• Documentation (manuals or wiki) has been updated or is not required
• Backports requested where applicable (ex: critical bugfixes)

[blizzz]

I am not sure which side effects this would have. Scary change ;) Would it not be better to emit additional Events in user_saml?

[iLinaza]

In my case, am clearing the cache in the pre event, using reflectivity from a custom App, and it is working fine, but I don't know if it can have consequences in other Apps that we don't use.

Clearing the cache of groups to which users belong must be the task immediately before or after the user is added to a group. I understand that, if we could ensure that, doing it in the pre-hook is the last thing to be executed it should be working fine, but I don't think priority can be added to hook subscriptions, as it is possible with Events.

Otherwise, as you say, a new event or Hook could be added, but in the long run the same problem (execution order) could occur if more applications use that event? Alternatively, we could use the already existing event, UserAddedEvent, and add the subscription to clear the cache with the highest possible priority value, instead of using the post- hooks.

[hrenard]

@blizzz, would something like hrenard@0aefd68 be safer ?