bpocallaghan / laravel-admin-starter

A Laravel Admin Starter project with Page Builder, Roles, Impersonation, Analytics, Blog, News, Banners, FAQ, Testimonials and more
http://bpocallaghan.co.za
337 stars 92 forks source link

CSRF token exception #10

Closed cagcak closed 6 years ago

cagcak commented 6 years ago

Hello again. I forgot to mention about following problem I have encountered on some pages:

The page has expired due to inactivity.

Please refresh and try again.

specifically it has triggered on banners page when user hits the save button which redirect to /admin/general/banners, I just took the screenshot below:

2017-11-28_16-52-20

extra : client console error message:

Failed to load resource: the server responded with a status of 419 (unknown status)

It might be CSRF token exception, I'm not sure.

Best regards

bpocallaghan commented 6 years ago

Hi @cagcak Yes - I've added the 'your session expired' for TokenMismatchException.

It should throw the error on local environment, think also when you have multiple form pages open.

I still want to add it so that when it fails - it generates a new one so the user does not have to refresh and can just submit again with new token. Everything in due time :)

But please let me know if happens 'randomly' for you. I've did some big-ish updates a few weeks ago and did not have much time to do any upgrades lately - might've broken a few things.

Thanks for looking so deep into it - always helps to get more eyes and opinions on it. :)

Hope this helps...

bpocallaghan commented 6 years ago

@cagcak Forgot to mention - In the Admin / Resource Controllers - On the index method I save the url to the session and then on store/edit I redirect to that url saved in session. This can be an issue if you have multiple pages open - it will redirect to the recently opened url.

Save url

Redirect

It might be an issue if you want to open multiple pages - due to it only being in the admin I figured its oky - does make 'changing' url easier (don't have to update the redirect url)

But yeah - probably personal preference..., unless you can see a major flaw in it?

cagcak commented 6 years ago

Hello again @bpocallaghan . That makes perfectly sense. I dont leave multiple pages open while artisan serving generally. CSRF token in middleware is needed for protection. But I think it must be activeted in only production. That is my opinion, so I might be wrong :) . You know the best. Anyway I have changed the APP_ENV as production to use the same token in my localhost. But you might be consider modifying App\Http\Middleware\VerifyCsrfToken for local and development modes as follows:

public function handle($request, \Closure $next)
{
    if (in_array(env('APP_ENV'), ['local', 'dev'])) {
        return $next($request);
    }

    return parent::handle($request, $next);
}

I'm not good enough to contribute at backend development but if I will encounter find an issue in frontend I'll try to fix it.

Best regards

bpocallaghan commented 6 years ago

Hi @cagcak I do agree with you - makes total sense. Just to add - only personal preference really. On the one side - you disable CSRF on local - which is 100%. Just a note - might be nice to see the CSRF on a form / ajax that you 'forgot' to add the CRSF field, but again - why will this happens if you've setup ajax correctly + forms - just thought I share.

Luckily - like most cases in Laravel , only takes a few lines to make it work for you :)

Thanks - Happy coding...