[8.x] Adds attempt with callbacks

[DarkGhostHunter]

This PR expands on the discussion here about adding callbacks the attempt() authentication mechanisms.

Problem

There is no way to add additional checks at runtime when using attempt(). Any addition forces the developer to create a new Guard, or use nonchalantly validate() and getLastAttempted(), and fire Attempting manually.

if (Auth::validate($credentials) && $user = Auth::getLastAttempted()) {
    if ($user->isAwesomeAndCool()) {
        Auth::login($user, $request->filled('remember'));
        Event::fire(Attempting('session', $credentials, $request->filled('remember')));
        return $this->redirect();
    }
}


return $this->sendAttemptFailed();

Solution

This PR allows the SessionGuard::attempt() method to the receive a third argument that receives one or multiple callbacks. Each of these receive the validated User, and if one returns false, the whole attempt fails.

$result = Auth::attempt($credentials, false, function (Authenticatable $user) {
    return $user->isAwesomeAndCool();
});

return $result 
    ? $this->redirect() 
    : $this->sendAttemptFailed();

Since there are callables, the developer can add its own checks, or use checks made by external packages, all while controlling manually the flow and order of these.

How it works

The change adds an internal function shouldLogin() that executes each callback and returns false if one of them "fails". This is called after the user is properly validated:

if ($this->hasValidCredentials($user, $credentials) && $this->shouldLogin($callbacks, $user)) {
    $this->login($user, $remember);

     return true;
}

Breaking changes: extending the Session Guard

---

The above change is simple and non-breaking, since it doesn't changes the interface signature, but as a precaution I'm pushing it into the next version... unless it gets approved for 8.x. And this also includes an additional test.

[DarkGhostHunter]

Yeah. If someone is extending the class it will be terrible. 9.x it is.

[DarkGhostHunter]

Seems ok to me.

[DarkGhostHunter]

Since it's for 9.x, why just change the contract? This would mean interoperability between all guards (included and custom ones).

[taylorotwell]

It is in theory possible to solve this on 8.x by passing a reserved key in the $credentials array... something like __callbacks. An outside user can't submit a Closure through a form of course so we would simply spin the array of callbacks and invoke them if they are an instance of Closure. It would need to introduce a new method to make this easier like:

Auth::attemptSomeMethodName(array $arrayOfCallbacks, array $credentials, etc...)
{
    $credentials['__callbacks'] = $arrayOfCallbacks;
    return $this->attempt($credentials, etc...);
}

[DarkGhostHunter]

I really would prefer using callables because I can just reference an static method from somewhere.

I think that reserving a key it's kind of hassle because there is no guarantee the developer is not using it somewhere (I've seen some sh*t).

It could be that just adding a new method that accepts an array of callbacks could work for 8.x.

Auth::attemptAnd($credentials, fn($user) => $user->isCool());

This could work only for the Session Guard, but on 9.x, a contract could force adding the new method to allow interoperability, but I wouldn't mind if everyone voted against.

[taylorotwell]

If you add a new method, I'm not sure you have a way to get those callbacks called at the correct time during the authentication process without a breaking change? Unless I'm missing something?

[DarkGhostHunter]

If you add a new method, I'm not sure you have a way to get those callbacks called at the correct time during the authentication process without a breaking change? Unless I'm missing something?

I'm also missing something. Why it wouldn't be called at the correct time? The idea is to work on the validated user itself. Wrapping it up:

1. The dev calls attempt() (or whatever name is) with the callbacks.
2. The guard gets the user for these credentials.
3. The guard checks if the credentials are valid.
4. This is where the callbacks run for further checks, because it has now the Authenticatable instance to work with.

This is intended to work like an "AfterValidated" hook because, before validation, its pretty much like a black box.

[taylorotwell]

Well - if you think you can add a new method in such a way that is not breaking feel free to adjust this PR to do that and I'll take a look.

[DarkGhostHunter]

Im on it. ~~‘attemptAnd()’~~ will be my choice. Italo Baeza C.

…

El 22-04-2021, a la(s) 09:48, Taylor Otwell ***@***.***> escribió:  Well - if you think you can add a new method in such a way that is not breaking feel free to adjust this PR to do that and I'll take a look. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

[DarkGhostHunter]

I think it has more "semantic sense" to use attemptWith(). I used that and added the guard instance into the callback just for show.

You can now do this:

Auth::attemptWith($credentials, $remember, function ($user, $guard) {
  return $user->isCool() && $guard instanceof SessionGuard::class;
});

No BC, 8.x compatible.

[taylorotwell]

Can you resubmit this to 8.x?

[DarkGhostHunter]

Can you resubmit this to 8.x?

Lol, of course.

[DarkGhostHunter]

#37090