1. This site uses cookies. By continuing to use this site, you are agreeing to our use of cookies. Learn More.

My add-on causes a security error :(

Discussion in 'XenForo Development Discussions' started by Liam W, Apr 27, 2013.

  1. Liam W

    Liam W Well-Known Member

    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
     
  2. Chris D

    Chris D XenForo Developer Staff Member

    How long is the script taking? o_O

    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?
     
  3. Jake Bunce

    Jake Bunce XenForo Moderator Staff Member

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

    Liam W Well-Known Member

    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.

    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.
     
  5. Jake Bunce

    Jake Bunce XenForo Moderator Staff Member

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

    Code:
    	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.
     
  6. Liam W

    Liam W Well-Known Member

    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.
     
  7. Jake Bunce

    Jake Bunce XenForo Moderator Staff Member

    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.
     
    tenants likes this.
  8. Liam W

    Liam W Well-Known Member

    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!
     
  9. tenants

    tenants Well-Known Member

    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:

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

    Code:
    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)
     
  10. tenants

    tenants Well-Known Member

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

    Code:
    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)
     
    Liam W and Jake Bunce like this.

Share This Page