Added Masking of PII data in logs

[SuyeshBadge]

Pull Request Template

Added feature to mask PII data into logs.

Description:

• This PR introduces a new feature to mask Personally Identifiable Information (PII) fields in log entries. The MaskingFilter in the logging package has been updated to support masking of various PII fields, such as name, email, phone number, Social Security number, credit card number, date of birth, biometric data, and IP address.
• The motivation behind this change is to enhance data privacy and security by preventing sensitive user information from being logged in plaintext.
• This feature will benefit users by ensuring compliance with privacy regulations and reducing the risk of data breaches or unauthorized access to sensitive information.

Breaking Changes (if applicable):

• There are no breaking changes introduced by this PR.

Additional Information:

• The changes rely on the built-in reflect package in Go for inspecting and modifying struct fields.
• [ISSUE] https://github.com/gofr-dev/gofr/issues/459
• New test cases have been added to ensure comprehensive testing of the PII masking functionality.

Checklist:

• [x] I have formatted my code using goimport and golangci-lint.
• [x] All new code is covered by unit tests.
• [x] This PR does not decrease the overall code coverage.
• [x] I have reviewed the code comments and documentation for clarity.

Thank you for your contribution!

[SuyeshBadge]

@aryanmehrotra, @vipul-rawat, and @Umang01-hash,

I'm initiating a discussion thread to commence the implementation of the mentioned feature. Let's delve into the various potential changes and methodologies to effectively incorporate this feature.

Looking forward to your valuable insights and contributions.

[SuyeshBadge]

Hey @vipul-rawat, I've pushed the requested changes along with the formatting and linting fixes. Please have a look once you get a chance and let me know if anything is missing.

[SuyeshBadge]

Certainly, @vipul-rawat will make the changes and update you accordingly.

[SuyeshBadge]

I am still not sure if this would handle the pointers

You're right to be concerned about handling pointers. The current implementation assumes that the fields are direct values and not pointers. Will update this too.

[SuyeshBadge]

Hey @vipul-rawat , I've updated the filtered logging feature to support pointer values and added documentation on how to use it. However, one test check is failing, and I'm not sure why. Could you please take a look and provide some guidance on fixing it? Thanks for your help!

[vipul-rawat]

@SuyeshBadge This implementation requires users to be aware of the logger and define it themselves, whereas for most use cases the framework is responsible for managing the logger itself. Can we use configs reading to achieve the same instead of the user defining the filter in the main file?

Also, how can a user-defined filter be called DefaultFilter? Should be CustomFilter or equivalent.

The pipeline was failing due to an MQTT test! Probably the public server was experiencing some downtime. Rerunning should fix the same. Also, update the local with development.

[SuyeshBadge]

Hello @vipul-rawat,

Could you kindly provide further clarification regarding the utilization of configuration readings in this context? Specifically, would you prefer me to:

1. Pass the configuration explicitly to the NewLogger function?
2. Expose a setConfig method that will be invoked prior to declaring the logger?
3. Directly invoke godotenv from the logger.go file?

I require additional details to proceed effectively. Thank you.

[vipul-rawat]

@SuyeshBadge What I mean is that currently according to your implementation, the NewLogger is being made by the user in main, but what we want is that the filter gets configured automatically during the app creation (during gofr.New()), so for example if user gives the config MASKED_FIELDS=<comma separated field names> then what framework does is that it adds the filter with those names, we don't want to have the filtering of fields by default.

I think something like SetFilter similar to setConfig that is suggested in point 2 seems good.

Point 1 will not be viable as it would be a breaking change and not worth it. Point 3 also not viable as logger's responsibility is not to read configurations or load them.

@srijan-27 FYA

[SuyeshBadge]

Hey @vipul-rawat,

I've implemented the changes as per your instructions. Additionally, I addressed several linting issues that were present. Kindly review the updates at your convenience. With the latest implementation, the application now reads values from the configuration and configures the logger accordingly. Furthermore, I've revised the documentation for improved clarity and understanding.

[SuyeshBadge]

Also if possible could you please re-run the checks, some of them are failing.

[srijan-27]

@SuyeshBadge - Are you still working on this?

[SuyeshBadge]

@SuyeshBadge - Are you still working on this?

Yes @srijan-27 will update soon

[SuyeshBadge]

Hey @srijan-27 and @vipul-rawat,

The changes we discussed have been implemented. Please review them and let me know if anything else is needed.

Thanks!

[SuyeshBadge]

Hey @srijan-27 @vipul-rawat,

I see there have been some recent changes pushed on the PR. Is there anything pending on my end? If yes, please let me know. Otherwise, let's move forward with this.

Thanks!

[srijan-27]

@SuyeshBadge - Have you tested these changes by running an example on local and verifying whether the fields of struct are getting masked?

[SuyeshBadge]

Not yet @srijan-27 .

[aryanmehrotra]

@SuyeshBadge can we connect and close this ASAP? If you are stuck we can work together!

[SuyeshBadge]

Certainly, @aryanmehrotra .

I identified the issue with the filter not working and have implemented a fix. However, we can still connect if you'd like to discuss the changes in detail.

[srijan-27]

Certainly, @aryanmehrotra .

I identified the issue with the filter not working and have implemented a fix. However, we can still connect if you'd like to discuss the changes in detail.

@SuyeshBadge - If you have implemented the fix, please push the code and we can verify then.

[SuyeshBadge]

Sure folks, will push it today.

[SuyeshBadge]

Hey Folks, Pushed some latest changes please take a look and let me know if this works.

FYI @srijan-27 @aryanmehrotra

[srijan-27]

@SuyeshBadge - can you please attach the snapshots for the masked logs, if you have tested it locally on any example.

[SuyeshBadge]

Sure, @srijan-27. Attached are the snapshots of the logs, the code, and the keys that need to be masked.

• Snapshot of fields that need to be masked
  

• Snapshot of the log after masking
  

• Snapshot of logs without masking
  

• Snapshot of the code snippet
  

[SuyeshBadge]

• Snapshot of the log, if the data passed, is a pointer

[srijan-27]

@SuyeshBadge - have you tried passing the struct as args in log?

Something like this:

func helloHandler(ctx *gofr.Context) (interface{}, error) {
	user := struct {
		Name     string
		Email    string
		Password string
	}{
		"srijan", "[email protected]", "secret-123",
	}

	ctx.Logger.Infof("%v", user)

	return "hello world!", nil
}

[SuyeshBadge]

Hey @srijan-27, In this case, the output will not be masked because the message being passed to the filter method is a string that is formatted before being passed. I'm not entirely sure if this is the intended behavior. Let me know what you think and what we can do here.

[srijan-27]

@SuyeshBadge - I don't think this should be intended behaviour. As if we consider this, then we are restricting user to only mask fields in unformatted logs. We should have a behaviour such that user can mask fields irrespective of the log formatting.

[SuyeshBadge]

Hey @srijan-27, Can we discuss this over a call? Let me know if you are available.

[SuyeshBadge]

Hi @srijan-27,

The issue is fixed. We are now masking the arguments, ensuring that all types of data passed are masked and not limited to any specific case. If you encounter any more edge cases, please feel free to raise them.

Also, let me know if this solution works for you. I am attaching a snapshot of the updated use cases.

---

Snapshots

[srijan-27]

@SuyeshBadge - Although this implementation is masking the fields as required. But it is also adding a log which is not helpful and not understandable for user. See below:

Also, the request/response log is missing in the new implementation:

Please check this.