log password reset email errors to log and do not reveal to user #17896
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #17091
Description:
The lines changed may have already achieved this previously. I could not reproduce seeing any revealing details in the errors I provoked. However, it is now ensured that no detail of the exception is logged to the user and they just get told to contact the admin. The exception is logged to the system logs with the injected logger instance. Currently it is only using the implied
__toString()
method of the exception, which should provide adequate detail to admins. However, there might be better detail that could be logged here. Maybe we want to log the message with$ex->getMessage()
instead like was previously fed back to the user.Also, I noticed that
Piwik::translate
was used instead of using an injectedTranslator
instance. @tsteur mentioned that we prefer to use DI, but there are a few instances ofPiwik::translate
in this file, so I left it for now, also because there it uses the injected class anyway and it might be easier to understand for a significant amount of users/developers.Review