From 13982be9f693686abbc500c8b8d620e2b46434f0 Mon Sep 17 00:00:00 2001 From: Deon George Date: Thu, 18 Apr 2013 18:17:33 +1000 Subject: [PATCH] Fix password reset issues --- application/classes/Auth/OSB.php | 80 ++++++++++--------- application/classes/Company.php | 2 +- application/classes/Config.php | 6 +- application/classes/Controller/Login.php | 10 +-- .../classes/Controller/TemplateDefault.php | 2 +- application/classes/HTTP/Exception/404.php | 2 +- application/classes/HTTP/Exception/501.php | 34 ++++++++ application/config/auth.php | 4 - application/config/config.php | 1 + application/views/errors/501.php | 4 + application/views/theme/yaml/page.php | 2 +- .../lnApp/classes/lnApp/Controller/Login.php | 12 +-- .../lnApp/classes/lnApp/Controller/Logout.php | 7 +- .../lnApp/Controller/TemplateDefault.php | 2 +- 14 files changed, 99 insertions(+), 69 deletions(-) create mode 100644 application/classes/HTTP/Exception/501.php create mode 100644 application/views/errors/501.php diff --git a/application/classes/Auth/OSB.php b/application/classes/Auth/OSB.php index 1b988fba..19c7e5bc 100644 --- a/application/classes/Auth/OSB.php +++ b/application/classes/Auth/OSB.php @@ -20,17 +20,21 @@ class Auth_OSB extends Auth_ORM { * @param boolean If authentication should be done for this module:method (ie: controller:action). * @return boolean */ - public function logged_in($role = NULL, $debug = NULL) { + public function logged_in($role=NULL,$debug=NULL) { + static $status = NULL; + + if (! is_null($status)) + return $status; + $status = FALSE; // Get the user from the session - $user = $this->get_user(FALSE); + $uo = $this->get_user(); // If we are not a valid user object, then we are not logged in - if (is_object($user) AND $user instanceof Model_Account AND $user->loaded()) { - - if (Config::sitemode() == Kohana::DEVELOPMENT && Kohana::$config->load('debug')->site) - SystemMessage::add(array('title'=>'Debug','type'=>'debug','body'=>Kohana::debug(array('user'=>$user->username,'r'=>$role)))); + if (is_object($uo) AND ($uo instanceof Model_Account) AND $uo->loaded()) { + if (Config::sitemode() == Kohana::DEVELOPMENT) + SystemMessage::add(array('title'=>'Debug','type'=>'debug','body'=>Debug::vars(array('user'=>$uo->username,'r'=>$role)))); if (! empty($role)) { // Get the module details @@ -67,7 +71,7 @@ class Auth_OSB extends Auth_ORM { $roles .= ($roles ? '|' : '').$gm->group->name; // $gm->group->id == 0 means all users. - if ($gm->group->id == 0 OR $user->has_any('group',$gm->group->list_childgrps(TRUE))) { + if ($gm->group->id == 0 OR $uo->has_any('group',$gm->group->list_childgrps(TRUE))) { $status = TRUE; $roles = ''; @@ -80,7 +84,7 @@ class Auth_OSB extends Auth_ORM { SystemMessage::add(array( 'title'=>'User is not authorised in Database', 'type'=>'debug', - 'body'=>sprintf('Role(s) checked: %s
User: %s
Module: %s
Method: %s',$roles,$user->username,$mo->name,$mmo->name), + 'body'=>sprintf('Role(s) checked: %s
User: %s
Module: %s
Method: %s',$roles,$uo->username,$mo->name,$mmo->name), )); } } @@ -90,8 +94,8 @@ class Auth_OSB extends Auth_ORM { SystemMessage::add(array( 'title'=>'Debug', 'type'=>'debug', - 'body'=>sprintf('A-User: %s, Module: %s, Method: %s, Role: %s, Status: %s, Data: %s', - $user->username,Request::current()->controller(),Request::current()->action(),$role,$status,$debug))); + 'body'=>sprintf('User: %s, Module: %s, Method: %s, Role: %s, Status: %s, Data: %s', + $uo->username,Request::current()->controller(),Request::current()->action(),$role,$status,$debug))); // There is no role, so the method should be allowed to run as anonymous } else { @@ -99,15 +103,15 @@ class Auth_OSB extends Auth_ORM { SystemMessage::add(array( 'title'=>'Debug', 'type'=>'debug', - 'body'=>sprintf('B-User: %s, Module: %s, Method: %s, Status: %s, Data: %s', - $user->username,Request::current()->controller(),Request::current()->action(),'No Role Default Access',$debug))); + 'body'=>sprintf('User: %s, Module: %s, Method: %s, Status: %s, Data: %s', + $uo->username,Request::current()->controller(),Request::current()->action(),'No Role Default Access',$debug))); $status = TRUE; } // Check and see if we have a token to login and run the method } elseif ((! empty($_REQUEST['token']) AND $token = $_REQUEST['token']) OR $token=Session::instance()->get('token')) { - if ($user=$this->_get_token_user($token) AND $user !== FALSE) + if (! is_null($this->_get_token_user($token))) $status = TRUE; } else { @@ -120,19 +124,19 @@ class Auth_OSB extends Auth_ORM { /** * Gets the currently logged in user from the session. - * Returns FALSE if no user is currently logged in. + * Returns NULL if no user is currently logged in. * * @param boolean Check token users too * @return mixed */ - public function get_user($tokenuser=TRUE) { - $user = parent::get_user(); + public function get_user($default=NULL,$tokenuser=TRUE) { + $uo = parent::get_user($default); // If we are not logged in, see if there is token for the user - if ($tokenuser AND $user === NULL AND $token=Session::instance()->get('token')) - $user = $this->_get_token_user($token); + if (is_null($uo) AND $tokenuser AND $token=Session::instance()->get('token')) + $uo = $this->_get_token_user($token); - return $user; + return $uo; } /** @@ -141,17 +145,16 @@ class Auth_OSB extends Auth_ORM { * This will check that the token is valid (not expired and for the request) * * @param $token The token - * @return mixed The user + * @return Model_Account|NULL The user that the token is valid for. */ private function _get_token_user($token) { // This has been implemented, as we sometimes we seem to come here twice - static $user = NULL; + static $uo = NULL; - if (! is_null($user)) - return $user; + if (! is_null($uo)) + return $uo; $mmto = ORM::factory('Module_Method_Token',array('token'=>$token)); - $user = FALSE; // Ignore the token if it doesnt exist. if ($mmto->loaded()) { @@ -195,13 +198,13 @@ class Auth_OSB extends Auth_ORM { Session::instance()->set('token',$token); - $user = ORM::factory('Account',$mmto->account_id); - $user->log(sprintf('Token %s used for method %s [%s]',$mmto->token,$mmto->module_method->name(),Request::current()->param('id'))); + $uo = ORM::factory('Account',$mmto->account_id); + $uo->log(sprintf('Token %s used for method %s [%s]',$mmto->token,$mmto->module_method->name(),Request::current()->param('id'))); } } } - return $user; + return $uo; } /** @@ -212,28 +215,28 @@ class Auth_OSB extends Auth_ORM { * @param boolean enable autologin * @return boolean */ - protected function _login($user, $password, $remember) - { - if ( ! is_object($user)) - { + protected function _login($user,$password,$remember) { + if (! is_object($user)) { $username = $user; // Load the user $user = ORM::factory('Account'); $user->where('username','=',$username)->find(); + + // If no user loaded, return + if (! $user->loaded()) + return FALSE; } + // Create a hashed password if (is_string($password)) - { - // Create a hashed password $password = $this->hash($password); - } // If the passwords match, perform a login - if ($user->status AND $user->has_any('group',ORM::factory('Group',array('name'=>'Registered Users'))->list_childgrps(TRUE)) AND $user->password === $password) - { - if ($remember === TRUE) - { + if ($user->status AND $user->has_any('group',ORM::factory('Group',array('name'=>'Registered Users'))->list_childgrps(TRUE)) AND $user->password === $password) { + + // @todo This is not currently used. + if ($remember === TRUE) { // Create a new autologin token $token = ORM::factory('User_Token'); @@ -272,7 +275,6 @@ class Auth_OSB extends Auth_ORM { * Determine if a user is authorised to view an account * * @param Model_Account Account Ojbect to validate if the current user has access - * * @return boolean TRUE if authorised, FALSE if not. */ public function authorised(Model_Account $ao) { diff --git a/application/classes/Company.php b/application/classes/Company.php index 21a1c338..42d20934 100644 --- a/application/classes/Company.php +++ b/application/classes/Company.php @@ -23,7 +23,7 @@ class Company { } public static function instance() { - return new Company(ORM::factory('Setup',array('url'=>URL::base('http')))); + return new Company(ORM::factory('Setup',array('url'=>URL::base('http')))); } public function admin() { diff --git a/application/classes/Config.php b/application/classes/Config.php index c3b2a334..45ee3119 100644 --- a/application/classes/Config.php +++ b/application/classes/Config.php @@ -163,7 +163,11 @@ class Config extends Kohana_Config { } public static function theme() { - return Kohana::$config->load('config')->theme; + // If we are using user admin pages (and login), we'll choose the admin theme. + if (! empty(URL::$method_directory[strtolower(Request::current()->directory())]) OR in_array(strtolower(Request::current()->controller()),array('login'))) + return 'theme/'.Kohana::$config->load('config')->theme_admin; + else + return 'theme/'.Kohana::$config->load('config')->theme; } public static function time($date) { diff --git a/application/classes/Controller/Login.php b/application/classes/Controller/Login.php index 3807c02f..076fb38f 100644 --- a/application/classes/Controller/Login.php +++ b/application/classes/Controller/Login.php @@ -18,10 +18,8 @@ class Controller_Login extends lnApp_Controller_Login { */ public function action_register() { // If user already signed-in - if (Auth::instance()->logged_in()!= 0) { - // Redirect to the user account + if (Auth::instance()->logged_in()) HTTP::redirect('welcome/index'); - } HTTP::redirect('login'); } @@ -34,10 +32,8 @@ class Controller_Login extends lnApp_Controller_Login { $token_expire = 15; // If user already signed-in - if (Auth::instance()->logged_in()!= 0) { - // Redirect to the user account + if (Auth::instance()->logged_in()) HTTP::redirect('welcome/index'); - } // If the user posted their details to reset their password if ($_POST) { @@ -71,7 +67,7 @@ class Controller_Login extends lnApp_Controller_Login { // Redirect to our password reset, the Auth will validate the token. } elseif (! empty($_REQUEST['token'])) { - HTTP::redirect(URL::link('user','account/resetpassword?token=%s'.$_REQUEST['token'])); + HTTP::redirect(URL::link('user','account/resetpassword?token='.$_REQUEST['token'])); } // Show our token screen even if the email was invalid. diff --git a/application/classes/Controller/TemplateDefault.php b/application/classes/Controller/TemplateDefault.php index 47f91b4a..cdea26b5 100644 --- a/application/classes/Controller/TemplateDefault.php +++ b/application/classes/Controller/TemplateDefault.php @@ -57,7 +57,7 @@ class Controller_TemplateDefault extends lnApp_Controller_TemplateDefault { $this->ao = Auth::instance()->get_user(); if (! is_null($this->ao) AND (is_string($this->ao) OR ! $this->ao->loaded())) - throw new Kohana_Exception('Account doesnt exist :account ?',array(':account'=>(is_string($this->ao) OR is_null($this->ao)) ? $this->ao : Auth::instance()->get_user()->id)); + throw HTTP_Exception::factory(501,'Account doesnt exist :account ?',array(':account'=>(is_string($this->ao) OR is_null($this->ao)) ? $this->ao : Auth::instance()->get_user()->id)); } parent::before(); diff --git a/application/classes/HTTP/Exception/404.php b/application/classes/HTTP/Exception/404.php index dd898617..14bf5100 100644 --- a/application/classes/HTTP/Exception/404.php +++ b/application/classes/HTTP/Exception/404.php @@ -4,7 +4,7 @@ * This class overrides Kohana's 404 Exception * * @package OSB - * @category Helpers + * @category Exceptions * @author Deon George * @copyright (c) 2009-2013 Open Source Billing * @license http://dev.osbill.net/license.html diff --git a/application/classes/HTTP/Exception/501.php b/application/classes/HTTP/Exception/501.php new file mode 100644 index 00000000..e79b7f1a --- /dev/null +++ b/application/classes/HTTP/Exception/501.php @@ -0,0 +1,34 @@ +status($this->_code); + + // @todo This is not working as cleanly as I would like - ie: we shouldnt need to publish the headers ourselves? + header(':', true, 501); + + if (Kohana::$config->load('debug')->show_errors) + $response->body(View::factory('errors/501')->set('message',$this->getMessage())->render()); + else + $response->body('Dang, something went wrong, tell us how we got here...'); + + echo $response->render(); + + exit (501); + } +} +?> diff --git a/application/config/auth.php b/application/config/auth.php index 4e3ed489..a691bae8 100644 --- a/application/config/auth.php +++ b/application/config/auth.php @@ -14,9 +14,5 @@ return array( 'driver' => 'OSB', 'hash_method' => 'md5', - 'hash_key' => '', - 'lifetime' => 1209600, - 'session_key' => 'auth_user', - 'forced_key' => 'auth_forced', ); ?> diff --git a/application/config/config.php b/application/config/config.php index 9598681f..da15b4bf 100644 --- a/application/config/config.php +++ b/application/config/config.php @@ -25,6 +25,7 @@ return array( 'bsb' => '633-000', // @todo This should come from the DB 'accnum' => '120 440 821', // @todo This should come from the DB 'theme' => 'yaml', // @todo This should be in the DB + 'theme_admin' => 'yaml', // @todo This should be in the DB 'tmpdir' => '/tmp', ); ?> diff --git a/application/views/errors/501.php b/application/views/errors/501.php new file mode 100644 index 00000000..866088a5 --- /dev/null +++ b/application/views/errors/501.php @@ -0,0 +1,4 @@ +Bother, something went wrong - 501. + +
+$_COOKIE,'request'=>$_REQUEST,'session'=>$_SESSION,'server'=>$_SERVER)); ?> diff --git a/application/views/theme/yaml/page.php b/application/views/theme/yaml/page.php index 669a2913..db3e723b 100644 --- a/application/views/theme/yaml/page.php +++ b/application/views/theme/yaml/page.php @@ -105,7 +105,7 @@ = Kohana::STAGING) { ?> - + diff --git a/modules/lnApp/classes/lnApp/Controller/Login.php b/modules/lnApp/classes/lnApp/Controller/Login.php index 67c8c55c..c8246f33 100644 --- a/modules/lnApp/classes/lnApp/Controller/Login.php +++ b/modules/lnApp/classes/lnApp/Controller/Login.php @@ -15,17 +15,11 @@ class lnApp_Controller_Login extends Controller_TemplateDefault { public function action_index() { // If user already signed-in - if (Auth::instance()->logged_in() != 0) { - // Redirect to the user account + if (Auth::instance()->logged_in()) HTTP::redirect(URL::link('user','welcome/index')); - } // If there is a post and $_POST is not empty if ($_POST) { - // Store our details in a session key - Session::instance()->set(Kohana::$config->load('auth')->session_key,$_POST['username']); - Session::instance()->set('password',$_POST['password']); - // If the post data validates using the rules setup in the user model if (Auth::instance()->login($_POST['username'],$_POST['password'])) { // Redirect to the user account @@ -37,10 +31,6 @@ class lnApp_Controller_Login extends Controller_TemplateDefault { HTTP::redirect(URL::link('user','welcome/index')); } else { - // We are not successful logging in, so delete our session data - Session::instance()->delete(Kohana::$config->load('auth')->session_key); - Session::instance()->delete('password'); - SystemMessage::add(array( 'title'=>_('Invalid username or password'), 'type'=>'error', diff --git a/modules/lnApp/classes/lnApp/Controller/Logout.php b/modules/lnApp/classes/lnApp/Controller/Logout.php index f9b094e7..f78c2df2 100644 --- a/modules/lnApp/classes/lnApp/Controller/Logout.php +++ b/modules/lnApp/classes/lnApp/Controller/Logout.php @@ -15,8 +15,11 @@ class lnApp_Controller_Logout extends Controller { # If user already signed-in if (Auth::instance()->logged_in()!= 0) { $ao = Auth::instance()->get_user(); - Auth::instance()->logout(); - $ao->log('Logged Out'); + + if (method_exists($ao,'log')) + $ao->log('Logged Out'); + + Auth::instance()->logout(TRUE); HTTP::redirect('login'); } diff --git a/modules/lnApp/classes/lnApp/Controller/TemplateDefault.php b/modules/lnApp/classes/lnApp/Controller/TemplateDefault.php index d6830477..3568ccf4 100644 --- a/modules/lnApp/classes/lnApp/Controller/TemplateDefault.php +++ b/modules/lnApp/classes/lnApp/Controller/TemplateDefault.php @@ -56,7 +56,7 @@ abstract class lnApp_Controller_TemplateDefault extends Controller_Template { return (($this->auth_required !== FALSE && Auth::instance()->logged_in(NULL,get_class($this).'|'.__METHOD__) === FALSE) || (is_array($this->secure_actions) && array_key_exists($this->request->action(),$this->secure_actions) && - Auth::instance()->logged_in($this->secure_actions[$this->request->action()],get_class($this).'|'.__METHOD__) === FALSE)); + is_null(Auth::instance()->logged_in($this->secure_actions[$this->request->action()],get_class($this).'|'.__METHOD__)))); } /**