Don't attempt to translate login names to uids when uids are provided

[blizzz]

It's a legacy of #1142 which was a downstream of a poor fix, that resolved password reset of local users, when the login name provided did not match the user name. The fix in place was to pipe every request to the user manager through the loginName2UserName even if a user id was provided. With other user backends (cough LDAP cough) it might not cost a bit of performance, but also may return false positives. Unfortunately this happens, so a backport is necessary.

While resolving login names to user names is not terribly rock solid, back then we introduced this mechanism for federated shares, since end users do not know their user id. This mechanism can be triggered by throwing a hook on demand. Which is what is happening now.

fixes#7445#6986

[rullzer]

@blizzz how to test this?

Like how to test the case where it causes issues?

[blizzz]

Was tested successfully #6986 (comment)

[rullzer]

In that case 🐘

[codecov]

Codecov Report

Merging #7611 into master will decrease coverage by 20.45%.
The diff coverage is 100%.

@@ Coverage Diff @@## master #7611 +/- ##
=============================================
- Coverage 51.2% 30.74% -20.46% + Complexity 24893 24890 -3 
=============================================
Files 1602 1602 Lines 94803 94801 -2 Branches 1372 1372 =============================================
- Hits 48543 29146 -19397 - Misses 46260 65655 +19395

Impacted Files	Coverage Δ	Complexity Δ
lib/private/User/Manager.php	26.8% <ø> (-58.2%)	75 <0> (-3)
core/Controller/LostController.php	79.73% <100%> (+0.54%)	32 <0> (ø)	⬇️
lib/public/Comments/CommentsEvent.php	0% <0%> (-100%)	3% <0%> (ø)
apps/user_ldap/lib/Migration/UUIDFix.php	0% <0%> (-100%)	5% <0%> (ø)
lib/public/Files/ForbiddenException.php	0% <0%> (-100%)	2% <0%> (ø)
...rivate/Authentication/Token/DefaultTokenMapper.php	0% <0%> (-100%)	11% <0%> (ø)
apps/user_ldap/lib/LDAPUtility.php	0% <0%> (-100%)	1% <0%> (ø)
apps/user_ldap/lib/BackendUtility.php	0% <0%> (-100%)	1% <0%> (ø)
apps/admin_audit/composer/autoload.php	0% <0%> (-100%)	0% <0%> (ø)
apps/user_ldap/lib/Mapping/GroupMapping.php	0% <0%> (-100%)	1% <0%> (ø)
... and 374 more

[blizzz]

Like how to test the case where it causes issues?

What I did here (more a theoretical scenario) was to have two LDAP users a different cased givenName ("martha" and "MARTHA") and used this as internal username attribute, as well as login attribute.

Without the change only the lowercase user would show up on the users page, with it both are shown. Also, on occ files:scan --all would only scan the lowercase-user (twice) while with this fix it is run for all users.

[MorrisJobke]

@blizzz There are 30 failures in unit tests.

[blizzz]

Boy, already the very first one I look into tests the undesirable behaviour. Was downstreamed #1915. 🙈

[blizzz]

Oh, and then we have the fact that the local user backend sort of accepts user ids in any case. I am impressed this did not blow up yet, since the lower casing against the database happens only against the users table, not others, like preferences iirc.

I am kllling FilesystemTest::testUserNameCasing, because initMountPoints($userid) should in no way be called with a wrongly cased user id.

• verify that all initMountPoints($userid) usages provide a safe useridinitMountPoints($userid) checks the UID itself and uses the real one, plus adds debug output
• evaluate to make Database::userExists case sensitive.

[blizzz]

evaluate to make Database::userExists case sensitive.

Would extend Database::loadUser with another parameter that would leave out the LOWER ops in the DB Query. Or copy a lot of code. Since we intent to touch it in 14 anyway, let's keep it for now.

[blizzz]

The background of #7611 (comment) is that this is another downstream "fix" that was introduced, because a prior downstream "fix" inserted some horrible behaviour this PR is ironing out.

Tests passing now, would like to have @MorrisJobke 's OK, too, however.

[MorrisJobke]

CI failure is due to a hiccup and can be ignored.

[MorrisJobke]

Code makes sense 👍

[blizzz]

Backports in #7694 and #7695