fix(usergroups): clear cached user groups on pre hooks #45036
Conversation
Signed-off-by: iLinaza <i.linaza@gmail.com>
solracsf
changed the title
|
I am not sure which side effects this would have. Scary change ;) Would it not be better to emit additional Events in user_saml? |
|
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. |
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):
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