[BUGFIX] Encrypt api_key before DataHandler records the history diff - #30
Merged
Conversation
CHANGELOG documents the fix and the one-off cleanup for history entries written before it. Dropping the whole row rather than editing history_data keeps that a single portable statement: the payload comes in two shapes (flat for ACTION_ADD, oldRecord/newRecord for ACTION_MODIFY) and would otherwise need JSON manipulation in four SQL dialects. sys_log.log_data references the history id, but only as a soft reference, and the backend history view queries sys_history by tablename/recuid.
Member
|
@julianammann Thank you for the fix! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
On updates, DataHandler captures the sys_history diff in
compareFieldArrayWithCurrentAndUnset()before it callsprocessDatamap_postProcessFieldArray(). Encrypting only there left the plaintext key in the record history, readable in the record's history view and in the database, although the column itself was encrypted. Inserts were not affected.EncryptApiKeynow also encrypts inprocessDatamap_preProcessFieldArray, so only the ciphertext reaches the history. The post-process hook stays for the "empty means keep the stored key" handling;encrypt()is idempotent, so the value is not encrypted twice.The new functional test
EncryptApiKeyHistoryTestreproduces it through the real DataHandler.Existing sys_history rows are not rewritten. Installations that stored keys before this fix should purge them once.