flightphp / core

An extensible micro-framework for PHP
https://docs.flightphp.com
MIT License
2.63k stars 407 forks source link

route view rendered after redirect #484

Closed schmeic closed 9 months ago

schmeic commented 1 year ago

I have a filter like this: Flight::before('start', function() { ... that checks if the user is logged in, and if not, redirects to the login page and returns false.

However, the protected route view is still being rendered before the redirect happens and I got errors because there were missing variables used in the view. I was expecting that the view would not be rendered, and as a workaround I had to create empty variables.

Am I doing something wrong? How can I prevent a view from being rendered when the filter returns false?

mjisoton commented 1 year ago

Hey,

I'm not sure how other people do it, but I usually have a middleware (in the form of a 'route') to take care of checking and validating an user session. Is basically a route that matches all requests.

Flight::route('*', 'validateSession');

I put that before any other route declaration, and if there's a valid session, then I let the code proceed to the next matched route with a

return true;

If there's no session, then I do a redirect...

Flight::redirect($domain . '/login');

The 'validateSession' is exactly what you might expect: a function that gets a certain cookie from the request. That cookie contains a session token.

function validateSession(){
     $coo = Flight->request()->cookies->getData();
     if(isValid($coo['cookie_name'])){
          return true;
     } else {
          Flight::redirect($domain . '/login');
     }
}

Obviously, there's the problem of infinite redirects. To solve that, I configure a few URLs as 'available without session' (like /login, /recover-password, /change-password, /create-account, etc). In FlightPHP, there are probably many other ways to do it.

mario-nexa commented 1 year ago

Have you tried using exit; on the line immediately after the redirect() ? This will make sure that any code below does not get executed when we redirect.

schmeic commented 1 year ago

Seems like that's something Flight should be doing when redirect() is called.

magikstm commented 1 year ago

It doesn't. But, it's ok as-is. You may want to handle it differently by possibly adding some logging or the like. It leaves room for the developer to handle it as he wishes.

Ref: https://github.com/mikecao/flight/blob/2df64f37ea76ce98b97ed327c6b1f5ea2a5df2b0/flight/Engine.php#L547

schmeic commented 1 year ago

I guess exit is not the best solution, but I'm suggesting that Flight should not be rendering a view after a redirect is called. This is unexpected behavior, and I actually had to create empty data objects for my view because I was object undefined errors.

So, I don't think it's ok as-is.

n0nag0n commented 9 months ago

This is one thing I would like to address with flight is the ability to "group" together routes and then assign a middleware, similar to how SlimPHP handles it. It's really great at communicating context of what the routes should be doing without scouring a bunch of files.

n0nag0n commented 9 months ago

So flight 3.0.2 handles groups now. Hopefully that fixes what you were after?