Skip to content
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

Exceptions on login are not logged #2066

Closed
adragus-inviqa opened this issue Oct 9, 2015 · 5 comments
Closed

Exceptions on login are not logged #2066

adragus-inviqa opened this issue Oct 9, 2015 · 5 comments

Comments

@adragus-inviqa
Copy link
Contributor

Is there a reason for not logging them? Because I think they should definitely be logged.

$this->messageManager->addError(
__('Something went wrong while validating the login and password.')
);

@slavvka
Copy link
Member

slavvka commented Oct 27, 2015

@adragus-inviqa, thank you for your notification. Please feel free to contribute it with your pull request. Thank you!

@daim2k5
Copy link
Contributor

daim2k5 commented Nov 3, 2015

@adragus-inviqa do you like provide a PR?

@therouv
Copy link
Contributor

therouv commented Nov 4, 2015

I think the reason for not logging the exceptions here is because of a possible PA DSS validation as in the exception message can contain sensitive customer data.

M1 does not log the exceptions here as well, see https://github.com/OpenMage/magento-mirror/blob/magento-1.9/app/code/core/Mage/Customer/controllers/AccountController.php#L177

@adragus-inviqa
Copy link
Contributor Author

Then why is this "up for grabs"? That's why I ask first.

@davidalger
Copy link
Member

@therouv is correct… adding an exception log call here would be a PA-DSS violation - it is because the username/password are passed directly as arguments to the authenticate() method vs being passed as an object. An exception backtrace would contain the username and password in plaintext within the backtrace.

I've opened a PR to correct the error message displayed to the user and also to re-add the note which was formerly there in M1. Closing this issue page. Please reopen if you disagree with this assessment.

magento-engcom-team pushed a commit that referenced this issue Feb 12, 2018
Fixed issue:
MAGETWO-86656: app:config:dump doesn't work if run this command twice
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants