Add config flag for GDPR compliance. Remove last octet from ips in request-logs accordingly.#1585
Add config flag for GDPR compliance. Remove last octet from ips in request-logs accordingly.#1585aashah merged 1 commit intocloudfoundry:masterfrom FloThinksPi-Forks:flo-marc-gdpr
Conversation
|
We have created an issue in Pivotal Tracker to manage this: https://www.pivotaltracker.com/story/show/171974450 The labels on this github issue will be updated when the story is started. |
| request = ActionDispatch::Request.new(env) | ||
|
|
||
| # Remove last octet of ip if EU GDPR compliance is needed | ||
| ip = VCAP::CloudController::Config.config.get(:logging, :gdpr_compliant) ? request.ip.split('.')[0...-1].join('.') + '.0' : request.ip |
There was a problem hiding this comment.
This looks like it'll only work with ipv4 - maybe worth looking into a gem that also supports ipv6 to do this as many folks are probably anonymizing IP addresses in ruby for gdpr compliance. (I found https://github.com/ankane/ip_anonymizer after a brief search)
There was a problem hiding this comment.
I would probably also favor this construct over a ternary:
ip = if VCAP::CloudController::Config.config.get(:logging, :gdpr_compliant)
<redact ip>
else
<unredacted-ip>There was a problem hiding this comment.
@selzoc Thank you for the hint. We changed the implementation quite a bit to allow for v6 adresses. We ditched third party libs to not introduce new dependencies as the task is quite simple to adress with the stdlib of ruby. Also using you recommendation for the variable assignment : ).
If we find we need this in other parts of the CC we will think of pulling the code to anonymize the ip out, somwhere else to make it reusable. For now i think we are good keeping it in request_logs.
There was a problem hiding this comment.
I still think it would be clearer to pull it out into a function. But yeah I guess no need for a gem!
There was a problem hiding this comment.
@selzoc as i am still not that familiar with the codebase, where would such a function be placed in the coding ? Maybe lib/utils ?
There was a problem hiding this comment.
I just meant extracting lines 15-24 into a function within the same class, just to clean up the call function a bit.
IP Adresses now get their last octet cut of and replaced by 0 if the new config option logging.gdpr_compliant is true. E.g 192.168.1.80 will be logged as 192.168.1.0 which provides sufficent anonymization to comply with european data protection regulations. If the flag is false or not present, CC will have the old behaviour which means logging the full ips. Signed-off-by: Marc Misoch <47423110+MMisoch@users.noreply.github.com> Add unittest for ip anonymization in request logs Signed-off-by: Marc Misoch <47423110+MMisoch@users.noreply.github.com> Fix rubocop issues for request_logs(_specs).rb Signed-off-by: Marc Misoch <47423110+MMisoch@users.noreply.github.com> Make ip anonymization ipv6 capable and use stdlib to validate adresses We ditched any additional libary as the stdlib is sufficient and we dont want to introduce new dependencies as this is for now just needed once. Signed-off-by: Marc Misoch <47423110+MMisoch@users.noreply.github.com> Add more test for ipv6 behaviour and print ipv6 canonicaly by default Signed-off-by: Marc Misoch <47423110+MMisoch@users.noreply.github.com> Renamed config flag to anonymize_ips to reflect correct behaviour Signed-off-by: Marc Misoch <47423110+MMisoch@users.noreply.github.com> Pull out anonymize ip logic in own function Signed-off-by: Marc Misoch <47423110+MMisoch@users.noreply.github.com>
|
@selzoc can you review again, also squashed all changes into one commit so history is more clean once merged. |
|
Thanks for the PR! |
4 participants
A short explanation of the proposed change:
IP Adresses now get their last octet cut of and replaced by 0 if the
new config option logging.gdpr_compliant is true. E.g 192.168.1.80
will be logged as 192.168.1.0 which provides sufficent anonymization
to comply with european data protection regulations.
If the flag is false or not present, CC will have the old behaviour
which means logging the full ips.
This flag can also be used in other places that one may find needs
adoptions to comply to EU GDPR.
An explanation of the use cases your change solves
It provides a way to comply to the european data protection regulations.
All EU Customers of Pivotal that save their logs would benefit from this change.
The IP is specifically called as Personal information: https://gdpr.eu/recital-30-online-identifiers-for-profiling-and-identification/
So either one has to:
This change enables customers to use the third option as the first ones are not practicable.
Links to any other associated PRs
Followup of: #1575
Issue: #1568
Slack discussion: https://cloudfoundry.slack.com/archives/C07C04W4Q/p1584635872138900
I have reviewed the contributing guide
I have viewed, signed, and submitted the Contributor License Agreement
I have made this pull request to the
masterbranchI have run all the unit tests using
bundle exec rakeI have run CF Acceptance Tests
Could not get CATS to run(having no bosh-lite atm). Would be thankful if some pivot can run it against one of you bosh-lites.