[Bug]: Wrong return type on login

[CodeWithDennis]

What happened?

Login as user

Vormkracht10\TwoFactorAuth\Http\Livewire\Auth\Login::loginWithFortify(): Return value must be of type Vormkracht10\TwoFactorAuth\Http\Responses\LoginResponse|Livewire\Features\SupportRedirects\Redirector|null, Illuminate\Http\Response returned

How to reproduce the bug

Try logging in.

This bug was introduced in v1.3.2.

Package Version

v1.3.2

PHP Version

8.2

Laravel Version

11

Which operating systems does with happen with?

macOS

Notes

This bug goes all the way to latest version, but was introduced in v1.3.2.

[Baspa]

Hmm does this happen since version 1.3.2? I only changed missing prefix when retrieving config values.

[CodeWithDennis]

Hmm does this happen since version 1.3.2? I only changed missing prefix when retrieving config values.

Yea, all versions after 1.3.2 (including 1.3.2). It seems like it loading my UserStatusCheck middleware and breaks.

<?php

namespace App\Http\Middleware;

use Closure;

use Illuminate\Http\Request;

use Illuminate\Support\Facades\Auth;

use Symfony\Component\HttpFoundation\Response;

class UserStatusCheck

{

    public function handle(Request $request, Closure $next): Response

    {

        if (Auth::check() && ! Auth::user()->is_active) {

            Auth::logout();

            abort(403);

        }

        return $next($request);

    }

}

[Baspa]

Is it supposed to load the userStatusCheck middleware? Seems like the middleware is included in the pipeline that Fortify uses? If that's the case I guess it would be ok to add the Illuminate\Http\Response as return type as well

[CodeWithDennis]

I don’t believe the middleware is the problem. Even when I comment it out, the same error occurs, but the issue shifts to:

vendor/vormkracht10/filament-2fa/src/Http/Livewire/Auth/Login.php:134

Vormkracht10\TwoFactorAuth\Http\Livewire\Auth\Login::loginWithFortify(): Return value must be of type Vormkracht10\TwoFactorAuth\Http\Responses\LoginResponse|Livewire\Features\SupportRedirects\Redirector|null, Illuminate\Http\Response returned

[CodeWithDennis]

Version v1.3.1 works perfectly fine.

[CodeWithDennis]

There isn't much difference between these two versions, and changing the code back manually doesn’t seem to fix the problem. I'm confused.

[EdgarsK]

+1, same error, v1.4.0

[Baspa]

Hmm alright, will take a look at it this evening @EdgarsK / @CodeWithDennis

[Baspa]

I don't get this error when using v1.4.0, using the default config and logging in without 2FA enabled. Can you provide me a reproduction repo maybe? @EdgarsK / @CodeWithDennis

[CodeWithDennis]

I can't with this project, but what you can try is to add a custom middleware in Laravel and see if that gives the error (i assume it will).

[Baspa]

I can't with this project, but what you can try is to add a custom middleware in Laravel and see if that gives the error (i assume it will).

Alright. Did you also add it to the fortify pipeline? Or just create the middleware?

[CodeWithDennis]

Just create and add the middleware to the $middlewareGroups (web) in the kernel.

[Baspa]

I guess the issue should be fixed by now. Tested it and I don't get the error anymore.

[CodeWithDennis]

I will let you know first thing in the morning =)

[CodeWithDennis]

The return type error is fixed, but since it now returns a different response, the login validation doesn't show up anymore, and I can't seem to log in. (Reverting back to 1.3.1 makes everything work perfectly again)

[Baspa]

Can you show me an example of the middleware? I didn't get any errors trying to login.

[CodeWithDennis]

<?php

namespace App\Http\Middleware;

use Closure;

use Illuminate\Http\Request;

use Illuminate\Support\Facades\Auth;

use Symfony\Component\HttpFoundation\Response;

class UserStatusCheck

{

    public function handle(Request $request, Closure $next): Response

    {

        if (Auth::check() && ! Auth::user()->is_active) {

            Auth::logout();

            abort(403);

        }

        return $next($request);

    }

}

Maybe its in combination with multi-tenancy?

[CodeWithDennis]

I'll create a test repository today and try to replicate the behavior.

[CodeWithDennis]

While creating a reproduction repo, I ran into various issues that my colleague also encountered. I made the repo to replicate the problem.

https://github.com/vormkracht10/filament-2fa/issues/40

[Baspa]

While creating a reproduction repo, I ran into various issues that my colleague also encountered. I made the repo to replicate the problem.

https://github.com/vormkracht10/filament-2fa/issues/40

Thanks! Will check it tonight :)

[CodeWithDennis]

While creating a reproduction repo, I ran into various issues that my colleague also encountered. I made the repo to replicate the problem.

40

Thanks! Will check it tonight :)

This is unrelated to this issue tho!

[CodeWithDennis]

I've been going through the code and ended up at SendTwoFactorCodeListener.php, specifically at line 27:

$user->notify(app(config('filament-two-factor-auth.send_otp_class', SendOTP::class)));

It seems like this is why nothing happens when I submit the form—it just falls back to the default Filament form without showing any messages.

I'm going to keep looking into it. Also, just to clarify, this is on Laravel 10, not 11 like I mentioned earlier.

Edit: Maybe I messed up somewhere with the event listeners?

Edit 2: Seems that's not the case because i'm getting this dump after submitting the login.

public function handle(TwoFactorAuthenticationChallenged | TwoFactorAuthenticationEnabled $event): void
{
    /** @var mixed $user */
    $user = $event->user;

    dd($user);
    $user->notify(app(config('filament-two-factor-auth.send_otp_class', SendOTP::class)));
}

[CodeWithDennis]

@Baspa So... after updating to the latest version and then checking for the bug, it looks like everything is working now. It might have been something related to the middleware versus tenantMiddleware. I guess I should have started by updating to the latest version first.

[CodeWithDennis]

@Baspa On a different note, since we're already talking, the user menu item only shows up when multi-tenancy is disabled. Is there a particular reason for that?

if (! config('filament-two-factor-auth.enabled_features.multi_tenancy')) {
    $panel->userMenuItems([
        'two-factor-authentication' => MenuItem::make()
            ->icon('heroicon-o-lock-closed')
            ->label(__('Two-Factor Authentication'))
            ->url(fn (): string => TwoFactor::getUrl()),
    ]);
}

[Baspa]

@Baspa On a different note, since we're already talking, the user menu item only shows up when multi-tenancy is disabled. Is there a particular reason for that?

if (! config('filament-two-factor-auth.enabled_features.multi_tenancy')) {
    $panel->userMenuItems([
        'two-factor-authentication' => MenuItem::make()
            ->icon('heroicon-o-lock-closed')
            ->label(__('Two-Factor Authentication'))
            ->url(fn (): string => TwoFactor::getUrl()),
    ]);
}

The reason for that is that I couldn't find a good way to add the tenant_id of the user in the menu item in de panel provider. To clarify this a bit I added this to the docs: https://github.com/vormkracht10/filament-2fa?tab=readme-ov-file#multi-tenant-setup.

I'm still not satisfied with how this works right now but couldn't find another solution right now..

[CodeWithDennis]

@Baspa I use multi-tenancy and when i manually remove that check it seems to work perfectly fine.

[Baspa]

Hmm, I will test it this weekend. I believe I got some errors while not checking on multi tenancy but if that's the case then this could be simplified. @CodeWithDennis

[CodeWithDennis]

I could just copy the code and add it to my panel, but if this will be resolved on plugin level i will wait. =)

[CodeWithDennis]

Closing this since its resolved.