Framework: [5.3] Can we include which guards were attempted when throwing AuthenticationException?

Created on 1 Sep 2016  路  3Comments  路  Source: laravel/framework

The two guards I have (web and customer) have two different login paths. I could not find a way to determine which _guard_ was being used when AuthenticationException was thrown.

By default, unauthenticated() redirects to guest('login') which is totally fine in most cases. However, I think it would be beneficial to include the $guards from the Authenticate middleware when throwing the Exception.

For instance, I'm doing this...

// App\Exceptions\Handler.php

protected function unauthenticated($request, AuthenticationException $e)
{
    if ($request->expectsJson()) {
        return response()->json(['error' => 'Unauthenticated.'], 401);
    } else {
        // If unauthenticated trying to access admin, redirect to Admin login.
        if(str_contains('admin', $request->path())) {
            return redirect()->to('auth/admin/login');
        }
        // Redirect to Customer login.
        return redirect()->guest('login');
    }
}

...which is pretty ugly considering a guest might want to access a page called "badminton" behind the customer auth guard.

If we're able to see which guards were attempted, we could leave it up to the developer to determine the best way to handle the presence of multiple guards (i.e. which guard gets preference).

Otherwise, we're flying blind when handling AuthenticationExceptions (whereas previously this did not live in the framework/src and could easily be adapted to particular use cases).

OR

How do we access which guard was attempted in the Handler? If there's a more straightforward way, do tell!

Most helpful comment

Using request->is() still feels clunky.

For instance, let's say customer/*, account/*, and foo/bar/* all had the customer guard. But account/* has the account guard as well.

Doing this in unauthenticated()...

if($exception->guards->contains('account') {
    // redirect to register page.
}

if($exception->guards->contains('customer') {
    // redirect to login page.
}

...is way more intuitive than...

if ($request->is('customer/*') {
    // redirect to login page.
}
if ($request->is('account/*') {
    // redirect to register page.
}
if ($request->is('foo/bar/*') {
    // redirect to login page.
}

...right?

It reads better, it provides appropriate context for throwing the Exception, and it doesn't require leaking the routing structure.

In other words, _I don't really care which room in my house you're trying to get into, I just want to know which outside door you're knocking on so I can point you to the right place_.

Anyway, I'm ok with the workaround but I think this could be valuable.

All 3 comments

Checking the url is correct but use the below method.

_The is method allows you to verify that the incoming request URI matches a given pattern. You may use the * character as a wildcard when utilizing this method:_

if ($request->is('customer/*')) {
    //
}

Using request->is() still feels clunky.

For instance, let's say customer/*, account/*, and foo/bar/* all had the customer guard. But account/* has the account guard as well.

Doing this in unauthenticated()...

if($exception->guards->contains('account') {
    // redirect to register page.
}

if($exception->guards->contains('customer') {
    // redirect to login page.
}

...is way more intuitive than...

if ($request->is('customer/*') {
    // redirect to login page.
}
if ($request->is('account/*') {
    // redirect to register page.
}
if ($request->is('foo/bar/*') {
    // redirect to login page.
}

...right?

It reads better, it provides appropriate context for throwing the Exception, and it doesn't require leaking the routing structure.

In other words, _I don't really care which room in my house you're trying to get into, I just want to know which outside door you're knocking on so I can point you to the right place_.

Anyway, I'm ok with the workaround but I think this could be valuable.

I agree. I feel that with this new AuthenticationException we've lost something.

I don't see it as a big deal to simply add some params to the exception so we know which guard was used during the failed login attempt:

protected function authenticate(array $guards)
{
    if (empty($guards)) {
        return $this->auth->authenticate();
    }

    foreach ($guards as $guard) {
        if ($this->auth->guard($guard)->check()) {
            return $this->auth->shouldUse($guard);
        }
    }

    throw new AuthenticationException($guards);
}

https://github.com/laravel/framework/pull/15494

Was this page helpful?
0 / 5 - 0 ratings

Related issues

iivanov2 picture iivanov2  路  3Comments

gabriellimo picture gabriellimo  路  3Comments

ghost picture ghost  路  3Comments

SachinAgarwal1337 picture SachinAgarwal1337  路  3Comments

JamborJan picture JamborJan  路  3Comments