Revert "Revert "preLogin hook should use string UID not IUser""

[PVince81]

Had a chat with @tomneedham and the actual workflow of this emitter is as follows:

1. the first time it had string UID
2. later on, it got changed to IUser by mistake when moving to Symfony. This got released in some version.
3. then @tomneedham noticed that and provided this PR to revert this back to stage 1)

When looking at this I wasn't aware that there was a time where the event was correct in the past.
Considering this, I think we can accept this change again as it will bring it back to its original intended state.

@sharidas can you do some research to identify what versions the stages 1) and 2) were in to confirm ?

[codecov]

Codecov Report

Merging #33071 into master will not change coverage.
The diff coverage is 100%.

@@            Coverage Diff            @@
##             master   #33071   +/-   ##
=========================================
  Coverage     64.14%   64.14%           
  Complexity    18724    18724           
=========================================
  Files          1185     1185           
  Lines         70460    70460           
  Branches       1270     1270           
=========================================
  Hits          45198    45198           
  Misses        24892    24892           
  Partials        370      370

Flag	Coverage Δ	Complexity Δ
#javascript	52.89% <ø> (ø)	0 <ø> (ø)	⬇️
#phpunit	65.42% <100%> (ø)	18724 <0> (ø)	⬇️

Impacted Files	Coverage Δ	Complexity Δ
lib/private/User/Session.php	79.47% <100%> (ø)	154 <0> (ø)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7ecc170...b38a3f7. Read the comment docs.

[sharidas]

My initial investigation ( with git log lib/private/User/Session.php on stable10 branch ) are as follows:

1. d35a58d was the commit which brought the change ->
   private function loginUser($user, $password) {
   This had $this->manager->emit('\OC\User', 'preLogin', [$user, $password]);. This change is before symfony changes. And as per the function signature $user is of type IUser
2. 74f8be8 was the commit which added symfony event for the change ->
   private function loginUser($user, $password)
   This change moved the code moves the code in the method inside closure. i.e,
   return $this->emittingCall(function () use (&$user, &$password) ....
   The $user is still instance of IUser
3. 1d4282b was the commit which changed the access specifier for the method
   protected function loginUser($user, $password)
   Nothing else was touched here...
4. 354377d was the commit which added the failed login for the method. But this never touched the $user
5. 621a647 was the commit which made small modification to the funcation arg
   protected function loginUser(IUser $user = null, $password)
   It did added $uid = $user === null ? '' : $user->getUID(); as the first line in the method

I will have a re-look to make sure which commit brought uid as first change and then shift to IUser. Based on my above observation, it does not look so. From the first time the method was introduced $user was an instance of IUser.

[sharidas]

1. f10d105 is the commit which had
   private function loginWithPassword($uid, $password) { $this->manager->emit('\OC\User', 'preLogin', [$uid, $password]);
   This is uid.
2. d35a58d is the commit which had
   private function loginWithPassword($uid, $password) { $this->manager->emit('\OC\User', 'preLogin', [$uid, $password]);
   The uid is used here... and
   private function loginWithToken($token) { ... $this->manager->emit('\OC\User', 'preLogin', [$uid, $password]);
   The uid is here, and
   private function loginUser($user, $password) { ... $this->manager->emit('\OC\User', 'preLogin', [$user, $password]);
   3.74f8be86d4bbba9b3e6137089aa2fea1fde4dfe7 is the commit which brought the symfony events to Session. But the preLogin has the same 3 change here too

The commit which brought IUser for preLogin event instead of uid is d35a58d.

And from then on, this change was there with the new method added private function loginUser.
So the change with IUser is there from v10.0.0 version.

[sharidas]

From my observations shared #33071 (comment) this change LGTM.

[sharidas]

So this changed from uid to IUser with ownCloud 10.0.0.

Yes

[PVince81]

@sharidas please backport

[sharidas]

Backport to stable10 -> #33185