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

fix(masking): Various performance, functional, and readability fixes for JSON-based masking #2051

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

jamfor352
Copy link
Contributor

@jamfor352 jamfor352 commented Jan 25, 2025

Several fixes here:

  1. I realised after further usage and some extensive testing in more complex scenarios that JSON Arrays weren't handled correctly - they were always either masked at the whole field level, or left unmasked. Now, the same settings that apply now also apply to nested objects inside arrays, or if the array contains primitives, the higher level array will apply. So for example, given:
address.firstLine

this will now apply to both:

{
  "address": {
      "firstLine": "value"
  }
}

and now also to all the firstLine values in an array type, too (this was not working correctly previously):

{
  "address": [
    {
      "firstLine": "value"
    },
    {
      "firstLine": "someOtherValue"
    }
  ]
}

Primitives will be handled as expected as well, where a filter like:

address.values

would now result that the following object:

{
    "address": {
      "values": [ "one", "two", "three" ]
    }
}

will correctly look like:

{
    "address": {
      "values": [ "xxxx", "xxxx", "xxxx" ]
    }
}

instead of the current:

{
    "address": {
      "values": "xxxx"
    }
}

which is currently losing the format, as well as non-sensitive information such as array length.

  1. Some readability improvements & fixes, also updating tests to be uniform, so the exact same test code is not repeated. Now the JsonMaskerTest interface defines the core tests the input and output, with the ShowByDefault and MaskByDefault tests implementing the interface (with their own backing application.yml which makes them fit the test criteria) and have their own specific tests written in the class only

  2. Small performance improvements, including defining the topic -> filtering keys in the constructor rather than constantly re-evaluating

  3. Remove the phrase "Please contact akhq administrator" which is likely to cause confusion and bring people here, when the issue isn't AKHQ but that a record has not fulfilled the criteria for masking

@jamfor352 jamfor352 changed the title fix(masking): Various performance and readability fixes fix(masking): Various performance, functional, and readability fixes for JSON-based masking Jan 25, 2025
@jamfor352
Copy link
Contributor Author

cc: @AlexisSouquiere since you have familiarity reviewing this code beforehand :) A few things changed as explained in the PR. Makes it more reliable and functional and should improve performance (a bit) too

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

Successfully merging this pull request may close these issues.

1 participant