-
Notifications
You must be signed in to change notification settings - Fork 9.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
BUG FIX - Uncaught exception when accessing admin with none existent use... #2
Conversation
…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.
@@ -384,7 +384,11 @@ public function reload() | |||
*/ | |||
public function loadByUsername($username) | |||
{ | |||
$this->setData($this->getResource()->loadByUsername($username)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you like to practice implementing an integration test for this method?
Supposedly dev/tests/integration/testsuite/Mage/Admin/Model/UserTest.php -- it doesn't exist yet. But implementing just loadByUsernameTest() would be enough.
I recommend using @magentoDataFixture to create a test admin account fixture. The fixture will be reverted automatically after test execution.
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.
I have now added the requested tests for this method and included fixtures. |
Thank you, Alistair. The changes were incorporated into Magento 2 and pushed to Github. Closing this pull request. |
…an 0. Can't upload specific image
…an 0. Can't upload specific image
…an 0. Can't upload specific image
…an 0. Can't upload specific image
…an 0. Can't upload specific image
…an 0. Can't upload specific image
…an 0. Can't upload specific image
…an 0. Can't upload specific image
…an 0. Can't upload specific image
…an 0. Can't upload specific image
…an 0. Can't upload specific image
…an 0. Can't upload specific image
[Exception] Deprecated Functionality: explode(): Passing null to parameter magento#2 ($string) of type string is deprecated in /var/www/html/lib/internal/Magento/Framewo rk/Validator/EmailAddress.php on line 68
...r.
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.