Add forced() method and middleware

[CodeWithDennis]

This PR is a draft. I'm still trying to figure out how to redirect to the 2FA page.

[Baspa]

I believe I already built this in a project of us previous of building this package. I can take a look at it if you want @CodeWithDennis

[CodeWithDennis]

I believe I already built this in a project of us previous of building this package. I can take a look at it if you want @CodeWithDennis

Yeah, go for it! Just a heads-up, there are a few PHPSTAN issues. =(

[Baspa]

In the project I have this code:

class RedirectWhen2faNotEnabled
{
    /**
     * Handle an incoming request.
     *
     * @param  \Closure(\Illuminate\Http\Request): (\Symfony\Component\HttpFoundation\Response)  $next
     */
    public function handle(Request $request, Closure $next): Response
    {
        if (!$request->user()?->hasTwoFactorEnabled() && !$request->routeIs('filament.admin.pages.two-factor')) {
            return redirect()->route('filament.admin.pages.two-factor');
        }

        return $next($request);
    }
}

And in my bootstrap/app.php:

    ->withMiddleware(function (Middleware $middleware) {
        $middleware->group('panel:admin', [
            \Illuminate\Session\Middleware\StartSession::class,
            RedirectWhen2faNotEnabled::class
        ]);
    })

I needed the StartSession middleware to be able to have the user in the request I guess.

Will take a further look tonight. @CodeWithDennis

[CodeWithDennis]

You should be able to edit this PR, so feel free to make any changes! Unless you prefer to rewrite it. =)

[Baspa]

Already fixed redirecting in my lunch break :) @CodeWithDennis

[CodeWithDennis]

Already fixed redirecting in my lunch break :) @CodeWithDennis

Dedication 🔥

[CodeWithDennis]

@Baspa Does PHPStan command work for you in my branch? Seems to crash on me.

[Baspa]

@Baspa Does PHPStan command work for you in my branch? Seems to crash on me.

Seem to work for me but it was very slow, I also gave it full memory so that might be the reason why it worked for me..

[CodeWithDennis]

Already fixed redirecting in my lunch break :) @CodeWithDennis

One issue with this when using multi-tenancy:

Missing required parameter for [Route: filament.customer.pages.two-factor] [URI: customer/{tenant}/two-factor] [Missing parameter: tenant].

[Baspa]

Already fixed redirecting in my lunch break :) @CodeWithDennis

One issue with this when using multi-tenancy:

Missing required parameter for [Route: filament.customer.pages.two-factor] [URI: customer/{tenant}/two-factor] [Missing parameter: tenant].

Can you try again maybe? :)

[CodeWithDennis]

@Baspa Does PHPStan command work for you in my branch? Seems to crash on me.

Seem to work for me but it was very slow, I also gave it full memory so that might be the reason why it worked for me..

Must be something on my side.

[CodeWithDennis]

Can you try again maybe? :)

Same problem, i think this is where i got stuck because filament()->getTenant() is not available in the middleware.

Shorter:

$tenant = filament()->getTenant(); // Still same issue

TwoFactor::getUrl($tenant);

But we also need to check if the panel is multi tenancy.

[Baspa]

Can you try again maybe? :)

Same problem, i think this is where i got stuck because filament()->getTenant() is not available in the middleware.

Shorter:

$tenant = TwoFactor::getUrl(); // Still same issue

TwoFactor::getUrl($tenant);

But we also need to check if the panel is multi tenancy.

Hmmm good one.. I have to think about that

[Baspa]

Is it not available in any middleware at all? Or just after a specific middleware? Then we could maybe load that middleware before our middleware? @CodeWithDennis

[CodeWithDennis]

Is it not available in any middleware at all? Or just after a specific middleware? Then we could maybe load that middleware before our middleware? @CodeWithDennis

I mean it returns null.

[Baspa]

Maybe we could use something like this: https://filamentphp.com/docs/3.x/panels/tenancy#applying-middleware-to-tenant-aware-routes ? @CodeWithDennis

https://filamentphp.com/docs/3.x/panels/tenancy#using-tenant-aware-middleware-to-apply-global-scopes

[CodeWithDennis]

Won't this be the same problem where we can't access the tenant parameter? I think this PR kinda done if we can figure out how we can access it. (Besides the PHPStan issues) @Baspa

Edit: Perhaps session parameter?

[CodeWithDennis]

@Baspa I haven't had time to check it again yet. Did you come up with any ideas that might solve the problem?

[Baspa]

@CodeWithDennis unfortunately I haven't been able to take a look at this as well.. hopefully later this week

[Baspa]

It should be fixed right now, can you test it to be sure? @CodeWithDennis

[CodeWithDennis]

It should be fixed right now, can you test it to be sure? @CodeWithDennis

Yes it works! Couple of questions:

• Will it also work without tenancy?
• Should we display a message that its forced.

[Baspa]

It should be fixed right now, can you test it to be sure? @CodeWithDennis

Yes it works! Couple of questions:

• Will it also work without tenancy?
• Should we display a message that its forced.

Hmm, when you're not using tenancy the tenantMiddleware is not triggered. We should add another middleware when not using tenancy I guess. Displaying a message would be a idea. @CodeWithDennis

[CodeWithDennis]

It should be fixed right now, can you test it to be sure? @CodeWithDennis

Yes it works! Couple of questions:

• Will it also work without tenancy?
• Should we display a message that its forced.

Hmm, when you're not using tenancy the tenantMiddleware is not triggered. We should add another middleware when not using tenancy I guess. Displaying a message would be a idea. @CodeWithDennis

I can imagine that if users are constantly being redirected to this page, it could be confusing, and they might not understand what's happening. It would be good to inform them that the app requires 2FA before they can use it.

I don't have time right now, but if I get a chance and you haven't added it yet, I'll take a look.

[Baspa]

I can make a difference in which middleware to use now by also adding withTenancy(). Don't know if that's the best solution but it's quite clean I guess. Didn't have time to checkout the message yet. @CodeWithDennis

[CodeWithDennis]

I can make a difference in which middleware to use now by also adding withTenancy(). Don't know if that's the best solution but it's quite clean I guess. Didn't have time to checkout the message yet. @CodeWithDennis

I think you can check if the Panel uses Tenancy and grab that.

[Baspa]

I can make a difference in which middleware to use now by also adding withTenancy(). Don't know if that's the best solution but it's quite clean I guess. Didn't have time to checkout the message yet. @CodeWithDennis

I think you can check if the Panel uses Tenancy and grab that.

Unfortunately $panel->hasTenancy() returns false in the TwoFactorAuthPlugin class..

[CodeWithDennis]

Can we make this customizable?
Example:

->forced(message: __('Some message')

[Baspa]

Yes maybe it's better to let the developer choose which message to show to the user. I'm also not really satisfied with showing the message yet. Weird enough I couldn't show the message in the blade view itself using the session helper..