benedmunds / CodeIgniter-Ion-Auth

Simple and Lightweight Auth System for CodeIgniter
http://benedmunds.com/ion_auth/
MIT License
2.34k stars 1.14k forks source link

CI error on login #653

Closed sparky672 closed 9 years ago

sparky672 commented 9 years ago

The last couple of times I tried logging into my system, I got a pretty generic CI error... something like "cannot perform that action" and when I looked that up, it had to do with the CSRF token.

Apparently, if I go to the login page and just leave it open in my browser for a certain number of hours, I'll get the error when I finally attempt to login. A refresh of the login page solves this issue and then I can login without an issue.

My guess is that when leaving the login page open for too long, the CSRF token expires, and that subsequently causes the error message.

I realize this is a really rare case. Most people would go to the login page and immediately login without any problem. However, I'd feel much better if I could work out a nicer way to deal with this. At the very least a redirect back to a fresh login page rather than an obscure CI error page.

Ideas? Thanks!

avenirer commented 9 years ago

The CSRF token has nothing to do with Ion Auth. If you want you can stop using it on the login page by simply replacing the <?php echo form_open("auth/login");?> (which, if CSRF is enabled on the application, will add a CSRF token) with html form tag <form ...>. Also, in version 3 of Codeigniter you can find alot of other methods regarding CSRF removal: http://www.codeigniter.com/userguide3/libraries/security.html?highlight=csrf

sparky672 commented 9 years ago

Yes, I never thought or meant to imply this issue was caused by Ion Auth. I just came here looking for general help & advice since I'm also using Ion Auth.

CSRF was just a guess and I honestly did not know what exactly was causing this error.

I can reproduce it by the following...

  1. Load a fresh login page.
  2. Delete the browser cookies (CSRF) for my domain.

Then an immediate attempted login gives this exact error on the default CI formatted error page...

"The action you have requested is not allowed."

So apparently my theory is correct. By leaving the login page open for too many hours, the CSRF cookie expires and this error message will trigger upon a login attempt.

I don't necessarily want to stop the error or disable any CI security. I just want to be able to better control what happens in this scenario, like a redirect my own error page or a refresh back to the login page.

If you have any suggestions or ideas, it would be very much appreciated. Thanks.

sparky672 commented 9 years ago

Here's what I ended up doing (CI version 2.2.0)

I extended the Security Class so I could control what happens when this CSRF error occurs. I simply use a PHP header() to force the current page to refresh itself, which restores the cookie that allows a subsequent login. Just to be safe, I also sanitized the URI with htmlspecialchars().

A PHP header() must be used for this redirect because none of the CI functions for redirecting or loading views are yet available at this hook point.

I also set the HTTP status code to "200 OK" since we're simply reloading the same page, however, I'm open to anyone's thoughts on this practice.

In my application/core...

<?php
class MY_Security extends CI_Security {

    public function __construct()
    {
        parent::__construct();
    }

    public function csrf_show_error()
    {
        // show_error('The action you have requested is not allowed.');  // default code

        // force page "refresh" - redirect back to itself with sanitized URI for security
        // a page refresh restores the CSRF cookie to allow a subsequent login
        header('Location: ' . htmlspecialchars($_SERVER['REQUEST_URI']), TRUE, 200);
    }
}
avenirer commented 9 years ago

Looks nice. But it would be nicer if you could explain the user why he has to write his credentials again. Maybe with a flash message?

sparky672 commented 9 years ago

"But it would be nicer if you could explain the user why he has to write his credentials again. Maybe with a flash message?"

I agree that would be great. However, just like redirect(), set_flashdata() is not available at this hook point. In fact, this whole problem is caused by an expired CSRF cookie, which means that there's also a good chance the session itself has expired.

If you know of a way to get a message onto the screen when I don't have access to the session, flashdata, template loading, or the views, I'd be very interested.

I suppose I should either create a standalone static error page that tells the user to login again (overkill?), or another separate version of the login page (with a message built into it) and use one of those within Location instead of the current URI. Perhaps the former.

Actually, I never put a large amount of thought into creating a standalone error page, because it should be a rare occurrence that anyone leaves their login page on the screen for several hours. However, the downside to redirecting them to a separate error page is that if they don't pay attention to the error message and simply hit the browser's back button, they will not get a refresh and after typing their login again, get the error page again, repeat until they realize they need to hard refresh or reload the login page.

sparky672 commented 9 years ago

Here's what I came up with... pretty simple actually.

Instead of directing Location: back to the current page, I send it to a new controller function within Ion Auth called csrf_redirect...

header('Location: /auth/csrf_redirect', TRUE, 302);

Then the csrf_redirect function handles the flash message and a redirect back to login...

function csrf_redirect()
{
    $flash = 'Session cookie automatically reset due to expired browser session.&nbsp; Please try again.';
    $this->session->set_flashdata('message', $flash);
    redirect('/login', 'location');
}

The minor downside to this method is that you are always redirected back to the login page rather than a refresh of whatever page/form you're trying to submit. However, that's a moot point, since the session cookie expires at nearly the same time as the CSRF cookie, you'd be redirected back to the same login page regardless.

avenirer commented 9 years ago

Never tried this, but can't you just use the Codeigniter assets with get_instance()? $CI =& get_instance(); $CI->load->helper('url'); $CI->load->library('session');

sparky672 commented 9 years ago

_"Never tried this, but can't you just use the Codeigniter assets with get_instance()?"_

No, you cannot use that here. That's the same as what I've been trying to tell you... the get_instance() object has not yet been created within CI so that only generates a nasty "non-object" error.

See the accepted answer here: http://stackoverflow.com/q/8447565/594235