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

Log details of mailing error and don't display details to end user #21173

Merged
merged 1 commit into from
Aug 18, 2021

Conversation

mattwire
Copy link
Contributor

Overview

Found when using the sparkpost extension and an error from sparkpost causes a failure to send the email. The CiviCRM mail library logs the message to the user session and it is then displayed - this is problematic because there could be sensitive information returned in the error message. Additionally there is no indication that an error occurred in the CiviCRM log files.

Before

Error message from mailer is displayed to end-user who is trying to unsubscribe:

image

After

Simple error message displayed to end-user with no sensitive information. Detail recorded as error in CiviCRM log:

image

Technical Details

CRM_Core_Session::setStatus() will display the message to any user who has an active session (eg. when following an unsubscribe link in an email). When certain mailing libraries are using this results in far too much detailed information being displayed to the end-user and not recorded in the logs.

Comments

@mlutfy @seamuslee001

@civibot
Copy link

civibot bot commented Aug 18, 2021

(Standard links)

@eileenmcnaughton
Copy link
Contributor

@mattwire this seems fine to me - test error relates but it's just a string to update. We could show more to users with debug permission - but I'm happy to only add that if it ever comes up

@seamuslee001 seamuslee001 merged commit 2213b34 into civicrm:master Aug 18, 2021
@agileware-justin
Copy link
Contributor

Good work @mattwire

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants