My add-on causes a security error :(

[Liam W]

I have an add-on that overrides the admin actionLogin function, however once I've done everything I want to do, and return something it gives a security error.

I think this could be because the script is taking too long and the token is expiring? How would I stop the security error from occurring? Can I regenerate the token?

Liam

 

[Chris D]

How long is the script taking?

If you're able to remove some steps from your script (as a test) so the script runs a lot quicker (i.e. almost instant) does the issue go away? Or do you still get the security error?

 

[Jake Bunce]

Override how? Are you extending the existing controller using the same action name? The 'login' action is exempt from the CSRF check.

 

[Liam W]

Override how? Are you extending the existing action using the same action name? The 'login' action is exempt from the CSRF check.

Pretty much that yes (I don't know how else to describe it). And I thought that, but it definitely ends in a CSfF error.

How long is the script taking?

If you're able to remove some steps from your script (as a test) so the script runs a lot quicker (i.e. almost instant) does the issue go away? Or do you still get the security error?

I'm not entirely sure. It first happened when I first made it, and didn't have much going on, but I moved a few pieces of code around and the error went away.

So it really is very strange!

Also, it doesn't appear to take long at all - the error comes up rather quickly. The annoying thing is, I can't figure out which line triggers the error to occur, however it does appear to be after the newest piece has run.

 

[Jake Bunce]

The 'login' action is exempt from the CSRF check:

	protected function _checkCsrf($action)
	{
		if (strtolower($action) == 'login')
		{
			return;
		}

		return parent::_checkCsrf($action);
	}

You must be overriding this function in your extended controller or using a different action name.

 

[Liam W]

The 'login' action is exempt from the CSRF check:

protected function _checkCsrf($action)
{
if (strtolower($action) == 'login')
{
return;
}
 
return parent::_checkCsrf($action);
}

You must be overriding this function in your extended controller or using a different action name.

I know it is, but I am not overriding that function at all. Something is checking the CSRF with a different action, and it could be anything really.

 

[Jake Bunce]

I need context. How are you overriding what?

All POST and JSON requests are checked unless excepted. You may wish to make an exception for your action by extending the _checkCsrf function.

 

[Liam W]

I need context. How are you overriding what?

All POST and JSON requests are checked unless excepted. You may wish to make an exception for your action by extending the _checkCsrf function.

Basically, I'm using a class listener and then extending my own class to the admin login class, then using a actionLogin function inside my custom class.

I haven't added anything else, and I only added code to that function, so I have no idea why it gives security error's if it's supposed to be excluded.

Also, I tried adding my own _checkCsrf function but it didn't work, it still gave the error!

 

[tenants]

Have a look at the source for login user locks, it's very simple, and does this for the public area which is going to be similar to the admin area:

Login User Locks: http://xenforo.com/community/resources/loginuserlocks-prevent-brute-force-security.1347/
or
IP Based Security: http://xenforo.com/community/resources/xenloginsecurity-ip-address-account-login-security.1194/
(uses the admin login)

loginuserlocks listener:

class LoginUserLocks_Listener
{
    public static function listenController($class, array &$extend)
    {
        if ($class == 'XenForo_ControllerPublic_Login')
        {
            $extend[] = 'LoginUserLocks_ControllerPublic_Login';
        }
    }
}

loginuserlocks controller public:

class LoginUserLocks_ControllerPublic_Login extends XFCP_LoginUserLocks_ControllerPublic_Login
{
    public function actionLogin()
    {
        // do stuff
        if($somecondition)
        {
            return $this->responseView('XenForo_ViewPublic_Login', 'my_template', $viewParams);
        }
        else
        {
            return parent::actionLogin();
        }
    }
}

If you can show us a snippet of your listener and ControllerAdmin_Login, we might be able to tell you what the issue is
I suspect it's an oversight (something like not using XFCP to extend, capitalisation of the class name... or some mistake like that)

 

[tenants]

Making use of the parent (and not duplicating the actionLogin code) resolved this

public function actionLogin()
{
    $responseView = parent::actionLogin();
 
    // make sure the view type is correct
    if ($responseView instanceof XenForo_ControllerAdmin_View)
    {
        // then throughout your actionLogin, make use of the parent response
        $parentParams = $responseView->params;
        $heresThePasswordParam =$parentParams['password'];
        // so now you don't need to duplicate everything
    }
    // if everything is normal and you want to return the parent, then:
    return $responseView;
}

It's good practice to call the parent where possible (and even pass the parent variable through your plug-in where applicable).

• This avoids duplicate code
• Avoids clashing with other plugins (very important)
• Makes sure the parent acts as it should
• Will often prevent things that should not be done twice (creating sessions / attempting to delete things that have already been deleted, etc)