From fd0fdd5f878654528bff88b3ced054702298dcd5 Mon Sep 17 00:00:00 2001 From: Alistair Stead Date: Sat, 10 Dec 2011 21:20:25 +0000 Subject: [PATCH 1/3] BUG FIX - Uncaught exception when accessing admin with none existent user. Summary: When attempting to login to the Magento admin. If the user does not exist the resource method loadByUsername returns false. This result is passed to setData that will then throw an exception. Fix: Added a test to confirm $user is returned before calling setData. This allows the execution to correctly fail through to the standard exception handler and display an appropriately secure exception message. --- app/code/core/Mage/Admin/Model/User.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/code/core/Mage/Admin/Model/User.php b/app/code/core/Mage/Admin/Model/User.php index 5d5107adb3575..62895ace4eee4 100644 --- a/app/code/core/Mage/Admin/Model/User.php +++ b/app/code/core/Mage/Admin/Model/User.php @@ -384,7 +384,11 @@ public function reload() */ public function loadByUsername($username) { - $this->setData($this->getResource()->loadByUsername($username)); + $user = $this->getResource()->loadByUsername($username); + if ($user) { + $this->setData($user); + } + return $this; } From 035a235c397cfaa56415d6f288cb003ba307a8ae Mon Sep 17 00:00:00 2001 From: Alistair Stead Date: Fri, 20 Jan 2012 23:18:02 +0000 Subject: [PATCH 2/3] Removed incorrect whitespace --- app/code/core/Mage/Admin/Model/User.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/core/Mage/Admin/Model/User.php b/app/code/core/Mage/Admin/Model/User.php index 62895ace4eee4..69394d7769940 100644 --- a/app/code/core/Mage/Admin/Model/User.php +++ b/app/code/core/Mage/Admin/Model/User.php @@ -388,7 +388,7 @@ public function loadByUsername($username) if ($user) { $this->setData($user); } - + return $this; } From e50c9afaa1764dfcf6d8e8605c6da6baf57fc089 Mon Sep 17 00:00:00 2001 From: Alistair Stead Date: Fri, 20 Jan 2012 23:18:41 +0000 Subject: [PATCH 3/3] Closes #2 Response to @antonmakarenko following comments on the pull request Summary: Created new testcase for Mage_Admin_Model_User Added fixture file to create an Admin user for testing. --- .../testsuite/Mage/Admin/Model/UserTest.php | 56 +++++++++++++++++++ .../testsuite/Mage/Admin/_files/user.php | 35 ++++++++++++ 2 files changed, 91 insertions(+) create mode 100644 dev/tests/integration/testsuite/Mage/Admin/Model/UserTest.php create mode 100644 dev/tests/integration/testsuite/Mage/Admin/_files/user.php diff --git a/dev/tests/integration/testsuite/Mage/Admin/Model/UserTest.php b/dev/tests/integration/testsuite/Mage/Admin/Model/UserTest.php new file mode 100644 index 0000000000000..f8fdb303d8a0d --- /dev/null +++ b/dev/tests/integration/testsuite/Mage/Admin/Model/UserTest.php @@ -0,0 +1,56 @@ +_model = new Mage_Admin_Model_User; + } + + /** + * Ensure that an exception is not thrown if the user does not exist + */ + public function testloadByUsername() { + $this->_model->loadByUsername('non_exiting_user'); + $this->assertNull($this->_model->getId(), 'The admin user has an unexpected ID'); + $this->_model->loadByUsername('adminuser'); + $this->assertTrue(!is_null($this->_model->getId()), 'The admin user should have been loaded'); + } +} diff --git a/dev/tests/integration/testsuite/Mage/Admin/_files/user.php b/dev/tests/integration/testsuite/Mage/Admin/_files/user.php new file mode 100644 index 0000000000000..363ab0871a5d3 --- /dev/null +++ b/dev/tests/integration/testsuite/Mage/Admin/_files/user.php @@ -0,0 +1,35 @@ +setFirstname('Admin') + ->setLastname('User') + ->setEmail('test@magento.com') + ->setUsername('adminuser') + ->setPassword('1234567890') + ->setIsActive(1) + ->save();