inGroup and getUserGroups have different cache keys and implementations

[PVince81]

I'd expect Group_LDAP::inGroup to simply use Group_LDAP::getUserGroups or at least use the same cache keys.

This means that it could happen that the cache contains slightly different values, leading to bugs like owncloud/core#26683 where one API which uses getUserGroups() sees that the user is member of a group, but inGroup says the user isn't.

@jvillafanez @owncloud/ldap

[PVince81]

From what I see the cache would be memcache

[PVince81]

I tried to force clear partially the cache and found out that inGroup also called _groupMembers which itself stores the same information in yet another cache key "_groupMembers" . $dnGroup

[PVince81]

and "inGroup-members" ...

[PVince81]

Here are steps to reproduce, it is a bit difficult because it needs a special sharing scenario: owncloud/core#26683 (comment)

It is likely that we can find other easier code path that would trigger inGroup calls.

[PVince81]

Also observed on a real env where groupExists would return false but getUserGroups still returns the membership... Broke some stuff with sharing.

[PVince81]

We need to rewrite this caching thing to make more sense and only cache a single info once.

[PVince81]

@IljaN I'd like you to have a look at this.

You can setup a test LDAP docker using these scripts: https://github.com/owncloud/administration/tree/master/ldap-testing (start.sh then run the zombie spawner scripts).

See inside the scripts for the LDAP credentials.

You'll also need to setup a memcache in ownCloud.
See owncloud/core#26683 (comment) for steps to reproduce.

Let me know if you need more info about the setup, how to setup LDAP or details about this issue.
Thanks!

[IljaN]

Postponed until owncloud/core#23558 is done. Memcache usage will be obsoleted by the introduction of the central user+group account table"

[PVince81]

Due to the nature of how caching is done here using hashing keys, it is not possible to cache everything only in a single location because we can't look up / search hashed values. (ex: find all groups in which user1 is in)

[PVince81]

The user account table is in 10.0 but we'll also need a group table to be able to resolve this.

[PVince81]

@jvillafanez is the new adjusted LDAP impl still using memcache for anything ?