Allow Admin to Skip e-mail confirmation for Export/Anonymization

[xkon]

We thought that it might be good to allow admins the option to avoid a user's e-mail confirmation.

We've talked a bit about it with @allendav and @mikejolley and 1 way was adding a drop down menu for the Admin to select if they want to skip it or not.

[xkon]

This first patch gives a try on skipping confirmation e-mails completely.

As far as my tests go on the Admin side everything works as supposed to but do take a good look please as I'm not sure if there's anything else hooked up that I missed :D .

Patch 43890.diff​ :

• Adds a dropdown menu with 2 options With Confirmation / Without Confirmation
• Add a new post_status of request-skipped - Confirmation Skipped
• Adds new buttons on the Next Steps column that allow you to download / erase data from scratch
• Avoids the request-confirmed update after every action to keep the same status

as seen in 43890_preview.gif​

[azaozz]

I'm thinking this would be good for v2 perhaps? Also, why add a "user request" at all, admins can export any data at any time and as many times as they want. We can add the entry forms for the email address on the Tools => Privacy screen (that was the original intention anyway).

[xkon]

Yes v2 sounds good as it still needs stuff fixing and a full check at this point will be not worth the time.

On the Tools -> Privacy screen you mention. That page was named 'Tools' when it was hosting everything under it, there's nothing in there at the moment except the Privacy Page settings (see #43894 suggestion) but I think that keeping them in the actual Export/Erasure lists makes more sense as that's what they are about and you can easily find it (+ you can keep the logs and such as well in your list since they are kept there for the time being).

[TZ Media]

For our clients, we'll need the possibility to skip the confirmation completely. They have data in multiple systems and have confirmed the request before a request is entered into WordPress. So they need a way to export all user data for a given email address without any confirmation emails.

[xkon]

Well if we decide 'were to put' (I still think that under the respective tools is better) these 2 actions all the base code from the patch above works as I've tested. We can skip adding them to the tables as @azaozz mentions and just use the functions solely for an instant export/anonymization.

If you think it will absolutely help for v1 ( note that nobody says how soon a next version will be - that's what I gathered from the last core chat ) I can squeeze some time and maybe rework on it today if we can catch the deadline as well of course.

[TZ Media]

If I remember correctly I can still hook into the confirmation request email before it is sent and change the email address to prevent it from being sent to the user.

I would definitely like to see it in 4.9.6, but it is not the highest priority for me.

Instant export would also be great.

BTW: I'd also need an "instant erase" without confirmation email (again because confirmation happened already outside of WordPress. Should this go into a separate ticket? I can use the same workaround as above for now with our clients, though.

[iandunn]

Related: #44066

[desrosj]

Moving to the new Privacy component.

[iprg]

#44066 was marked as a duplicate.

[allendav]

I also think we should also consider giving a box for the admin to give a reason for skipping confirmation - that will be useful in the logs eventually

[allendav]

Idea: instead of a dropdown, perhaps the no-confirmation flow could be started with a link next to the "primary" confirmation-flow button

[desrosj]

Removing the GDPR keyword. This has been replaced by the new Privacy component and privacy focuses in Trac.

[wesselvandenberg]

Hey, we at Wordcamp Nijmegen saw that it needs design. We got a few questions; Why does this need a email conformation and why do we need an admin for this?

The main question here is:
What risk is mitigated with the double conformation?

Is it an option to let users export their personal data trough a button on their profile page?

[xkon]

refresh

[xkon]

43890.2.diff​ is simply a refresh on the previous patch.

We would still like a feedback on ui and design of course whenever possible ( cc @melchoyce ) just to have it fully ready for a future release :) .

[karmatosed]

This seems to have a design, so I am going to remove that keyword and make sure it has the feedback one.

[xkon]

refresh

[xkon]

@garrett-eclipse , could you take a look at 43890.3.diff​ as well? I don't think I've missed something and if all good we can mark this for commit.

[birgire]

Thanks for the refresh @xkon

I noticed one thing in 43890.3.diff​, it changes the signature of the wp_create_user_request() function from:

wp_create_user_request( $email_address = '', $action_name = '', $request_data = array() ) {

to:

wp_create_user_request( $email_address = '', $action_name = '', $request_confirmation = '', $request_data = array() ) {

To preserve backward support, I would rather suggest:

wp_create_user_request( $email_address = '', $action_name = '', $request_data = array(), $request_confirmation = '' ) {

[birgire]

Few random thoughts:

It looks like $request_confirmation is binary, what about using true/false instead of 'yes' vs 'no' or '' strings ?

Personally, I like the former better:

wp_create_user_request( ..., true );

wp_create_user_request( ..., 'yes' );

In that case one might use:

$request_confirmation      = ( 'yes' === $_POST['request_confirmation'] );

instead of

$request_confirmation      = sanitize_text_field( $_POST['request_confirmation'] );

Another thing, how should we handle missing $_POST['request_confirmation'] ?

Should it trigger an invalid action error?

Since this is a binary option, would a checkbox be suitable here, instead of a dropdown?

It looks like unit tests will need an update.

Cheers

[karmatosed]

I tried to test this but couldn't get it working, would it be possible to either have some steps (just to check I am not missing anything) or a screencast?

[davidbaumwald]

This ticket still needs some work. I'm adding a 2nd-opinion tag to hopefully have some more input and movement toward inclusion in a Future Release.

[estelaris]

As discussed in the design meeting, we would like to request a new screenshot to be able to provide design feedback

[paaljoachim]

It would be helpful with a screenshot so we can see where it is at right now.
@davidbaumwald can you add a screenshot?

[xkon]

43890.4.diff​ is a refresh of the previous versions that applies cleanly now & adjusts code taking into account the tickets comments.

It adds an extra Checkbox (checked by default) instead of a dropdown to allow the Admins to skip the confirmation e-mails while a request is added. If the confirmation is skipped the requests are added as Completed directly.

The new form takes a bit of space now pushing the list table further down the page, but I chose to use the same setup as we do in Settings pages. I'll wait for a design feedback on this in case we can make it a bit ore compact :) .

Since the wp_create_user_request() has an extra param now we'll have to check phpunit tests also as @birgire mentioned in a previous reply. I'll work on those on a later update.

43890.4-before.jpg​ & 43890.4-after.jpg​ are a comparison of the UI change of this patch.

Marking this for 5.7 as well as I still believe that it would be a helpful addition for cases that emails don't need to be communicated.

[xkon]

phpunit tests

[xkon]

43890.5.diff​ continues on previous patch and adds unit tests for each $send_confirmation_email case.

[xkon]

Removing design-feedback as discussed in #design & adding needs-testing for a final check before marking for commit.

To test:

• Apply the latest patch: 43890.5.diff​
• Navigate to Tools -> Export Personal Data
• Input your username and request an export with the Confirmation option checked. Result you should receive the email and the request should be added with a pending status.
• Remove your username.
• Add your username again with the Confirmation un-checked. A request should be added with a completed status and no emails should be sent to the user (all confirmations are skipped).

In a similar way the same steps can be tested under Tools -> Erase Personal Data.

[paaljoachim]

Hey @xkon

I just tested the patch. Going to Tools -> Export Personal Data screen.
It think it is a very good idea to add a checkbox option here.

I am following the testing instructions above.

Test 1:
In the Username or email address I added the default localhost:8889 admin username. Checkbox is checked. Then clicked the Send Request button.

Result is a "Confirmation request initiated successfully."
Below I see a Pending message.

Hovered over the Requester e-mail address: test@… and clicked "Complete request"
Then under Next steps: clicked Remove request.

---

Test 2.
Again added the default username: admin.
Unchecked the checkbox.
Clicked Sent Request.

Result is that it skipped a "Confirmation request initiated successfully." and went straight to "Completed".

The patch works well in the Export Personal Data screen.

---

I also did the same test within the Erase Personal Data screen, and it looks to work identical.

---

In localhost:8889 I made a new profile with my hotmail address. I am not sure if I will get an e-mail or not. If/when I get an e-mail I can update this trac ticket.

[xkon]

Thanks a lot for testing @paaljoachim ! Marking for commit :) .

[paaljoachim]

Happy to help, @xkon :)

[antpb]

In 50159:

Privacy: Allow Admin to Skip e-mail confirmation for Export.

This adds a form option to skip the admin email alert when exporting personal data.

Props xkon, azaozz, TZ-Media, iandunn, desrosj, iprg, allendav, wesselvandenberg, karmatosed, birgire, davidbaumwald, estelaris, paaljoachim, hellofromTonya.
Fixes #43890.

[antpb]

In 50160:

Coding Standards: Fix spacing in test_pending_status_with_false_send_confirmation_email test.

Follow-up to [50159] adjusts alignment of the $request_data value.

See #43890.

[SergeyBiryukov]

In 50165:

Docs: Update documentation for wp_create_user_request() per the documentation standards.

Add a @since note for the $send_confirmation_email parameter.

Follow-up to [50159].

See #43890.

[SergeyBiryukov]

Follow-up: #52430

[SergeyBiryukov]

In 50230:

Privacy: Rename the $send_confirmation_email parameter of wp_create_user_request() to $status, for clarity.

Follow-up to [50159], [50165].

Props xkon, TimothyBlynJacobs.
Fixes #52430. See #43890.

[monikarao]

I have Tested the Patch and it's working as expected:

Case 1- Input your username and request an export with the Confirmation option checked. Result you should receive the email and the request should be added with a pending status.

Result: Success message "Confirmation request initiated successfully." but I didn't receive any email in my inbox.
​https://a.cl.ly/mXu15LZg

Case 2 - Add your username again with the Confirmation un-checked. A request should be added with a completed status and no emails should be sent to the user (all confirmations are skipped).

Result : ​https://a.cl.ly/Jru1qJLy

Note: I see one issue that Box automatically gets checked after creating a new request.

Please check this video - ​https://a.cl.ly/2NuE0Lq0

[paaljoachim]

Hey @johnbillion

I tested the patch here: https://core.trac.wordpress.org/ticket/43890#comment:40
It sounds like it needs another round of testing.

[hellofromTonya]

Retested "Export Personal Data" using Email Log plugin to capture email. Works as expected.

[hellofromTonya]

Retested functionality for "Export Personal Data". Used the Email Log plugin to capture the email.

Results: Works as expected.

• Email is captured in Email Log when the "Confirmation email" checkbox is checked
• Email is not sent when the "Confirmation email" checkbox is not checked

Marking testing as done. And reclosing the ticket.

What about the checkbox state on reload?

@monikarao noted that the checkbox rechecks after pressing "Send Request":

Note: I see one issue that Box automatically gets checked after creating a new request.

This happens as the page is sent and then reloaded after processing the request. The default state of the checkbox is enabled. It does not "remember" the last state.

@xkon is the intent to have the state of the checkbox persist?

[xkon]

A request required a confirmation up to now, so I left the checkbox to be reset and not keep it's state intentionally.

I didn't want to change the logic that requests where working "by default" and create further unwanted confusion since the option is also new.

The current behavior of the commit is working as expected and in future time if there are tickets/requests in Trac to also keep the state we can look into that as well of course, IMHO though it should always "reset" as it does now.