RedirectIfAuthenticated middleware API support

[alvarofvr]

What in this template can be improved or added as a feature?

Hi guys, i think that the built-in RedirectIfAuthenticated middleware (https://github.com/platformsh-templates/laravel/blob/master/app/Http/Middleware/RedirectIfAuthenticated.php) not work correctly in the context of a Laravel Breeze application scaffolded with api option (php artisan breeze:install api) or in general in the context of a client-driven authentication where Laravel act as a pure Backend. Practically the middleware performs an authentication check and if it is already authenticated it is redirected to the HOME (/dashboard).

class RedirectIfAuthenticated
{
    /**
     * Handle an incoming request.
     *
     * @param  \Illuminate\Http\Request  $request
     * @param  \Closure(\Illuminate\Http\Request): (\Illuminate\Http\Response|\Illuminate\Http\RedirectResponse)  $next
     * @param  string|null  ...$guards
     * @return \Illuminate\Http\Response|\Illuminate\Http\RedirectResponse
     */
    public function handle(Request $request, Closure $next, ...$guards)
    {
        $guards = empty($guards) ? [null] : $guards;

        foreach ($guards as $guard) {
            if (Auth::guard($guard)->check()) {
                return redirect(RouteServiceProvider::HOME);
            }
        }

        return $next($request);
    }
}

If we take as an example a SPA where routing is managed on the client side this approach does not work. If the client call an API middlewared by RedirectIfAuthenticated the client will receive a bad response because it expects a JSON and not a redirect.

In my opinion the communication mode in this middleware should therefore be considered.

What exactly should be updated?

It would therefore be sufficient to add a check on the response expectation of the request with $request->expectsJson() and if the expectation is to receive a JSON then return an error message "Already authenticated." with a HTTP status code 200, the default return, the default would remain a redirect.

class RedirectIfAuthenticated
{
    /**
     * Handle an incoming request.
     *
     * @param  \Illuminate\Http\Request  $request
     * @param  \Closure(\Illuminate\Http\Request): (\Illuminate\Http\Response|\Illuminate\Http\RedirectResponse)  $next
     * @param  string|null  ...$guards
     * @return \Illuminate\Http\Response|\Illuminate\Http\RedirectResponse
     */
    public function handle(Request $request, Closure $next, ...$guards)
    {
        $guards = empty($guards) ? [null] : $guards;

        foreach ($guards as $guard) {
            if (Auth::guard($guard)->check()) {
                if ($request->expectsJson()) {
                    return response()->json(['error' => 'Already authenticated.']);
                }
                return redirect(RouteServiceProvider::HOME);
            }
        }

        return $next($request);
    }
}

How important is this feature to you?

I think this problem is not blocking for me and for most users of this middleware, but I encountered this request on StackOverflow related to this -> https://stackoverflow.com/questions/74988485/fresh-laravel-sanctum-api-redirecting-to- dashboard-route which I answered and which appears to have been viewed several times.

Additional context

No response

[shahsawoodshinwari]

Great! This should be implemented

[chiragparekh]

I also had the same issue used the same solution.

[begueradj]

This is a valid bug that should be fixed :+1:

Just a tiny suggestion: as the response is about confirming the user is authenticated, it would be better to write it this way:

if ($request->expectsJson()) {
    return response()->json(['message' => 'authenticated.'], 200);
}

[gilzow]

Given that the copy here is pulled directly from laravel/laravel repository, it would probably be better to add an issue in the source repository.

[alvarofvr]

Mantainers wan't merge the update with reason: "we need to be very careful regarding the amount of code we include" (3 lines of code).

[begueradj]

The response you got is clearly an "automatic copy/paste" one. That's actually worrying because what about if it was a security issue ...