allow an attribute to return more than one value

[blizzz]

With #18469 I have overseen that our result processing does always only accept on value per attribute. The relevant piece of code is from 2012. Ancient! Anyway, usually it is not a problem. There is just one DN, just one displayname to use and it is also ok to just one email address for instance. However with that PR we also cache the groups a user belongs to, if 'memberof' is present. And just one groups sucks :( This means that a user has maximum of one (maybe 2 with primary) group(s) shown.

Now I am fixing this, but it is not fully done yet. Open ToDos:

• SYMPTOM: username fallback instead of display name attribute value is used as display name (e.g. on users page).
• SYMPTOM: array_xx values in the user management group box (next to totally correct ones from LDAP)
• TODO: LDAP returns the dn not in an array, but in a string. I am not aware of any other attribute that works like this. However, we don't have a handling yet that compares attribute names when accessing the values. Attributes that are provided by users can very well be a DN, although it hardly makes sense in most cases. When this happens, PHP will happily just take the first letter of the DN. Since checking in any possible situation where we check for an attributes value if the attribute is 'dn' is just stupid and error prone, I will modify the resutl array from search() so that 'dn' is dealt with as with any other.
• BUG: AD Primary Group is not read.
• TODO: adjust tests

FYI @DeepDiver1975 @cmonteroluque

For the record, "changelog" about affected and relevant methods:

search 
    - uses to return different array structure dependend on there was one or more attributes given
    - if more: [ 0 => [ 'attr1' => $values[0] ], … ]
    - if one : [ 0 => $values, … ]
    - now, structure is always: [ 0 => [ 'attr1' => $values[, 'attr2' => $values[] ], … ]

searchUsers 
    · returns result from search without modification

searchGroups
    · returns result from search without modification

fetchList 
    · returns result from search without modification, if many attributes where requested
    · returns flattened array otherwise: [ 0 => [ 'attr' => $values ] ]   →  $values
    · maintaines behaviour as before, when just one attribute was provided

fetchListOfUsers
    · passes  result from search to batchApplyUserAttributes without modification
    · returns result from fetchList without modification

fetchListOfGroups
    · returns result from fetchList without modification

fetchUsersByLoginName
    · returns result from fetchList without modification    

wizard::fetchGroups
    · returns result from searchGroups

ownCloudUserNames($result)
    · passes $result to ldap2ownCloudNames

getGroupsByMember
    · returns result from fetchList

ldap2ownCloudNames($result)
    · used to expect $result to be [ 'attr1' = $value, … ], i.e. always more requested attributes
    · now adjusted to return format from fetchList

getLDAPUserByLoginName
    · return first result from fetchList (via fetchUsersByLoginName) without modification

processAttributes($record)
    · $record is one item of the whole result set
    · adjusted

fetchListOfUsers used by:
    - OK countUsersInGroup, one attribute
    - OK getUsersInPrimaryGroup 
    - OK inGroup, one attribute, 2x
    - OK usersInGroup, one attribute
    - OK getUsers
    - OK fetchUsersByLoginName

searchGroups used by:
    - OK primaryGroupID2Name, one attribute
    - OK wizard::fetchGroups

fetchListOfGroups used by:
    - OK getGroupsByMember
    - OK getGroupsChunk

wizard::fetchGroups used by:
    · OK determineGroups, return value not used

getGroupsByMember used by:
    - OK getUserGroups

fetchUsersByLoginName used by:
    - OK getLDAPUserByLoginName
    - OK wizard::testLoginName

getLDAPUserByLoginName used by:
    - OK checkPassword
    - OK loginName2UserName

[ghost]

thanks @blizzz

[blizzz]

Since #18469 was backported to stable8.1, this one is affected as well. I keep the fix as simple as possible for an easier backport.

[blizzz]

Ready for review. Since it touches the very core of the LDAP backend, there's not just one thing to test, but everything should work as it was used to. I am confident that it does… The one obvious thing this should fix is that only one (max. two) groups of an LDAP user were shown, when there should be more. Either users page or personal page.

Because the original issue was a decision from 2012 which worked pretty well until #18469 was merged, that one needs to change. It's not a terribly huge, but fundamental change, we need to have it in 8.2 And because #18469 was backported to 8.1.x, we need a backport as well. @karlitschek please confirm.

So, please test and review @MorrisJobke @PVince81 @davitol @GreenArchon or others

[DeepDiver1975]

@cmonteroluque this requires qa involvement - kind of critical respecting the time line

[ghost]

@DeepDiver1975 agreed. Today's a holiday in Spain. Pinging @rperezb for early work tomorrow

[davitol]

@blizzz Testing this PR I've found the following behaviour:
The following log is always written in the owncloud.log file each time the admin access to the Admin Page with an OK LDAP Configuration(AD in this case):

{"reqId":"MCkuN6Pc16rF1NTxzIlL","remoteAddr":"XX.XXX.XXX.X8","app":"PHP","message":"ldap_read(): Search: Invalid DN syntax at /opt/owncloud/apps/user_ldap/lib/ldap.php#257","level":3,"time":"2015-10-13T10:33:03+00:00"}
Edit: I've realized it also happened before the fix

[blizzz]

@davitol results from a diagnostic query, message is nasty, but all is good.

[ghost]

thanks @blizzz and @davitol. Confirmed that it should be good to go

[DeepDiver1975]

@karlitschek @cmonteroluque may I suggest to postpone the backport? I'd love to have @blizzz back on this once he did recover- THX

[karlitschek]

makes sense imho

[ghost]

yeah, agree

[MorrisJobke]

I cherry picked the commits from here to stable8.1 #19972 @blizzz needs to have a look at it for sure.