Fix: OpenEMR logs sensitive information such as payment details #7341
Conversation
Signed-off-by: Jubitta John <[email protected]>
Signed-off-by: Jubitta John <[email protected]>
| $field_value = $matches[2]; | ||
| $masked_value = str_repeat('X', strlen($field_value)); | ||
| return "$field_name = '$masked_value'"; | ||
| }, $comments); |
There was a problem hiding this comment.
@stephenwaite is there something else that needs to be updated to the PR?
|
@stephenwaite Thank you for the approval. :) |
| * For any insert operation, if the current table contains any field containing sensitive information, | ||
| * the value for that field is masked to match its length. | ||
| */ | ||
| foreach (self::SENSITIVE_RECORDS as $table => $fields) { |
There was a problem hiding this comment.
would put the querytype == insert prior to the foreach (no reason to loop through the sensitive records array if the querytype does not equal insert)
There was a problem hiding this comment.
@bradymiller I believe this is now resolved? Good for us to bring this in?
Signed-off-by: Jubitta John <[email protected]>
|
hi @jubittajohn , Code looks good and bringing this in. thanks for the improvement! |
…emr#7341) * Adding log scrubbing template to mask sensitive payment details in log Signed-off-by: Jubitta John <[email protected]> * Fixed the php styling issue Signed-off-by: Jubitta John <[email protected]> * Insert operation check is moved to beginning Signed-off-by: Jubitta John <[email protected]> --------- Signed-off-by: Jubitta John <[email protected]> Co-authored-by: Jubitta John <[email protected]> (cherry picked from commit 9be12b8)
|
@bradymiller @stephenwaite @adunsulag Thank you for looking into the Pull Request, the review comments, and the approvals. |
|
hi @jubittajohn , turns out that modifying entries to the log is not something we should be doing to ensure OpenEMR remains compliant with ONC certification, so we will need to revert this code. Your code was well done and sorry we need to do this. |
…details (openemr#7341)" This reverts commit 9be12b8.
…details (openemr#7341)" (openemr#7396) This reverts commit 9be12b8.
…_last * 'master' of https://github.com/openemr/openemr: Hide dashboard card 2 (openemr#7423) fix: ccda zip import and php warnings and deprecations (openemr#7416) Fee sheet and Codes revenue code (openemr#7415) Refactor UB04 837i X12 feature to make work again. (openemr#7412) feat: support loop 2420E (openemr#7405) Fix: Revert "Fix: OpenEMR logs sensitive information such as payment details (openemr#7341)" (openemr#7396) Weno better error handling from fetches (openemr#7408)
Fixes #7340
Short description of what this resolves:
This PR adds a log scrubbing template to mask sensitive details in the log(which is currently exposed in logs). Currently, this template only masks the field "reference" in the table "ar_session" to conceal sensitive payment details in the log. However, if approved, this template can be used to extend the masking of other sensitive details to other tables and fields too. Masking of sensitive information in logs is essential to prevent the stealing of information. (ASVS 7.1.1).
After update:
Changes proposed in this pull request: