Protection doesn't work with Views Bulk Operations

Marked #2277347: Protection doesn't work with Views Bulk Operations as a duplicate.

I can confirm that this bug is in the 7.x-1.x version also.

The attached patch applies user protection when executing the following Views Bulk Operation actions from within a view:

• Cancel user account
• Modify user roles

I tried to also apply protection for the action "Modify entity values", but for some reason that doesn't work yet.

Technical: even if I supply a validation handler for the vbo action form for "Modify entity values", the validation function doesn't get called. This is not the case for the other two actions.

I figured out how to protect the "Change value" action too now. The validation handler needed to move to the submit button. I also completed the checks for the "Change value" action: first a check against all edits is done, then a check for each property is done (theme, language and fields are ignored).

This is actually a pretty critical issue, so bumping to critical.

I've tested with VBO further and wrote some automated tests for this issue. So I found out that the previous patch didn't fix the whole problem. In some cases users could still be blocked or canceled with VBO.

Here are two new patches: one with tests only (to be sure they fail without the fix) and one with the test + the fix. Phew, took me a while to get everything right.

Some minor changes. I think this is ready.

Committed #7.

Someone wants to create a patch for the 6.x-1.x version?

Although there are tests for VBO now, they are not run because VBO is missing from the test environment. Adding test_dependencies[] in the info file.

Re-uploading patch, because it seems testbot didn't take the file.

Note that this patch returns green, but that doesn't mean that the VBO tests work (in fact, they don't. I get 9 fails, 1 exception), because the "test_dependencies" are resolved before the patch is applied. But by committing this, at least the fails can show up for the testbot.

Great catch, Stevel. Yes, I noticed too that tests are failing on VBO.

Previously, the tests did work, because test dependencies are declared on userprotect_test.info. Later the testbot was changed to only look at the 'main' module for test dependencies and ignore submodules.

Committed #11, with a small difference. Dependencies should be declared in the form {project}:{module} as noted on https://www.drupal.org/docs/7/creating-custom-modules/writing-module-inf....

Do you want to take care of the test failures?

This patch seems to solve the issues.

@MegaChriz: did you bypass the "waiting for branch? I could not find an option to do that. I posted about that on #2866130: "Waiting for branch to pass" can't be circumvented anymore. Could you wait committing this until @drumm or someone from the infrastructure team could take a look at that?

@Stevel: yes, I forced the test to not wait for branch tests to pass.

Strange that VBO calls the table 'views_entity_user'. Is this always the case on a VBO enabled user view or can the table have other names as well?

@Stevel
I'll wait with committing the patch.

I'm finding some "'views_entity_' . $type" constructs in both entity and views, so it might be a more general approach taken there. Also, the tests work also by leaving out the test for == 'users', but I let that in just to be sure (perhaps older versions of VBO / Views / entity). Out for today, feel free to commit :)

The 'view_entity_' . $entity_type thing was added in #1334374: Re-use generic entity views table and is present since the 7.x-3.4 release of VBO. So that means we should keep the case for 'users' if we want to keep compatibility with VBO 7.x-3.3.

I updated the patch with a comment explaining why the two tables names are checked. As the issue with the 'bypass' is solved, no need to wait any longer.

Fantastic! Committed #21.

I wonder though if we should judge this change as a security issue?