Problem/Motivation

I'm working on a Commerce site and would like to use drush sql:sanitize to remove PII.

Proposed resolution

I was wondering if people thought it might make sense to sanitize any address fields directly attached to user or profile entities (similar to how sql:sanitize by default randomizes all text fields on a user entity)? I realize this might be considered out of scope for the address module, but by the same token I thought it might be able to catch a whole host of potential PII issues in one fell swoop, so thought I'd throw it out there (:

Remaining tasks

TBD

CommentFileSizeAuthor
#30 address-sanitize-3189074-30.patch10.17 KBhungdo
#18 Screenshot 2023-11-14 at 17.37.48.png22.31 KBthe_g_bomb
#10 address-3189074-10.patch10.13 KBeheller
#9 address-3189074-9.patch9.93 KBeheller
#8 address-3189074-8-do-not-test.patch12.04 KBandyf
#7 address-3189074-7.patch10.13 KBandyf
#5 address_3189074-5.patch6.78 KBandyf

Issue fork address-3189074

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

AndyF created an issue. See original summary.

AndyF opened merge request !1

andyf’s picture

andyf commented

I've added a simple plugin that just overwrites the following fields with [Sanitized] if they're not empty:

  • given_name
  • additional_name
  • family_name
  • address_line1
  • address_line2
  • postal_code

It respects the allowlist-fields option for cases where folk want to opt out individual fields.

I appreciate it's very primitive, open to feedback (:

Thanks!

andyf’s picture

andyf commented
Status: Active » Needs review
andyf’s picture

andyf commented
StatusFileSize
new address_3189074-5.patch6.78 KB

I'm just uploading a patch for composer.json against the MR at c999754a9a010fcc78bd558b02ee2325e9b00e0d. Thanks!

andyf’s picture

andyf commented
Title: Handle drush sql:sanitize? » Sanitize address fields on drush sql:sanitize
andyf’s picture

andyf commented
StatusFileSize
new address-3189074-7.patch10.13 KB

I'm just uploading a patch for composer.json against the MR at 1ff4c3985fd1e079f10eafa66d3a37a6794ba026. Thanks!

andyf’s picture

andyf commented
StatusFileSize
new address-3189074-8-do-not-test.patch12.04 KB

I'm just uploading a patch for composer.json against the MR at a49a04c176d7fba76b3eba3647dd59aae50e6f6b. Thanks!

eheller’s picture

eheller commented
StatusFileSize
new address-3189074-9.patch9.93 KB

@AndyF This patch is great. I made a few changes in this version:

  • Add a few additional fields to FIELD_COLUMNS
  • Remove the table restrictions to update all addresses in the database
  • Special handling for country_code field. A nice-to-have would be to determine the field length dynamically, rather than hard-coding this field specifically.

I'm also using this as the basis for a sanitization plugin for telephone numbers: https://www.drupal.org/project/drupal/issues/3205630

eheller’s picture

eheller commented
StatusFileSize
new address-3189074-10.patch10.13 KB

Patch #9 was showing this error on individual order admin pages on the Drupal Commerce site I've been using this on: Notice: Undefined index: [] in Drupal\address\Plugin\Field\FieldFormatter\AddressDefaultFormatter->viewElement() (line 157 of modules/contrib/address/src/Plugin/Field/FieldFormatter/AddressDefaultFormatter.php). The new approach sets default values for country, administrative area and postal code that fit the expected formatting for those fields.

kerasai’s picture

kerasai commented

Not a "formal" review, but noting that I've been running the patch in #10 without issue for several months.

Primsi made their first commit to this issue’s fork.

primsi’s picture

primsi commented

Updated the version requirement for drush to a more up to date version. Also rebased - I've never done this on a d.o merge request though ... I hope that's ok.

  • bojanz committed ce37622d on 8.x-1.x authored by AndyF 
    Issue #3189074: Handle drush sql:sanitize?

  • bojanz committed 2b74bc20 on 2.0.x authored by AndyF 
    Issue #3189074: Handle drush sql:sanitize?
bojanz’s picture

bojanz commented
Status: Needs review » Fixed

Merged (and added address_line3 for the 2.0.x commit). Thanks, everyone!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

the_g_bomb’s picture

the_g_bomb commented
StatusFileSize
new Screenshot 2023-11-14 at 17.37.48.png22.31 KB

Is this supposed to also sanitize addresses that are not related to user details, such as public office addresses?
Screen shot showing addresses sanitized

bojanz’s picture

bojanz commented

Yes, it doesn't (and can't) differentiate.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

You can use the confusingly named allowlist-fields option, which in this case means that the fields are allowed to *not* be sanitized. Note that the setting is shared with other sanitize steps like user fields, so if you have an address field that you don't want to sanitize and a user field of the same name that you *do* want to sanitize you're going to run into a bit of a conflict.

I think it would be nice if that setting supported something like node.field_address, but that would be a new feature at this point.

the_g_bomb’s picture

the_g_bomb commented

I wonder if this could be a setting that could be switched on or off. If you have no user related data, you don't need to sanitise the addresses.

A stage environment may look strange with all the satellite office snow having redacted addresses on the contact forms.

Perhaps it could have been a submodule that doesn't need to be enabled.

the_g_bomb’s picture

the_g_bomb commented

Also what if US addresses are not valid in the settings, you are replacing all sanitized addresses with a US postcode and country code.

The OP requested any address fields directly attached to user or profile entities. This does more than that.

bojanz’s picture

bojanz commented
Status: Closed (fixed) » Needs review

Let's gather some more feedback.

This feature has not made it into a release so I would be fine with reverting it if it's controversial.

  • bojanz committed 5abba0c9 on 8.x-1.x 
    Revert "Issue #3189074: Handle drush sql:sanitize?"

This reverts commit...

  • bojanz committed 9bc40511 on 2.0.x 
    Revert "Issue #3189074: Handle drush sql:sanitize?"

This reverts commit...
bojanz’s picture

bojanz commented
Category: Feature request » Support request
Status: Needs review » Fixed

Upon further consideration, I have decided to revert this feature.

This module has almost 100 000 installs, which calls for some conservatism, so I'd rather err on the side of not having a feature than introducing a feature that goes too far. Especially since the original request called for a smaller scope and could be done in Commerce directly.

Please create a contrib module with this plugin, and I will happily link to it from the project page.

Apologies to everyone disappointed by this.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

c_archer’s picture

c_archer commented

The latest patch fails against the 2.0.1 version of the module

khiminrm’s picture

khiminrm commented

The fixes were improved to just sanitize address fields in profiles and merged into the new project commerce_sql_sanitize https://www.drupal.org/project/commerce_sql_sanitize/issues/3424078

hungdo’s picture

hungdo commented
StatusFileSize
new address-sanitize-3189074-30.patch10.17 KB

Reroll the patch file to make it works with 2.0.1 version.