The scheduled transition is a nice feature and a must have for at least 2 of my clients projects.
But I believe that there is a flow in its current implementation.
The scheduled transition as to be executed by the cron job. But the cron user is the user '0' . Therefore, as the permissions are checked before you actually execute the transition, the anonymous user must be able to execute it, which, very often is not a desired feature.
I propose the following patch (against 1.2, I can reroll it against the CVS if required) :
We check that
$_SERVER['REQUEST_URI'] == base_path() . 'cron.php';
then we don't check the permission and we execute the transition (as if the user 1).
I don't see any security issue for that (we don't take the user 1 permissions).
It should work on any plateforms (Drupal emulate REQUEST_URI for the servers, other than apache, that don't provide it).
but has only been tested on Apache. Also not tested if drupal is installed in a subdirectory.
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | workflow_cron_patch.patch | 1012 bytes | 1sp |
| #5 | workflow-243015-5.patch | 1.89 KB | jmoy |
| #2 | workflow.HEAD_.cron_.r2.patch | 1.92 KB | gnassar |
| #1 | workflow.HEAD_.cron_.patch | 1.66 KB | gnassar |
| workflow_cron.patch | 985 bytes | cirotix |
Comments
Comment #1
gnassar commented+1. Giving the anonymous user transition rights just to get scheduled transitions working is 1) unintuitive and 2) insecure.
I take a different tack -- since the current dev version now has a force parameter, that can be used to allow the transition when it comes from cron. No server string lookups.
Of course, this means that permissions have to be checked for scheduled transitions at the point they are submitted, not later when they are executed. But shouldn't this be the case anyway? A user trying to schedule a transition they are not allowed to perform should get an immediate failure.
Patch enclosed. Setting to "code needs work" pending clarification on what branch to roll patches against.
Comment #2
gnassar commentedCouple of bugs fixed in the patch. Still holding off for final determination on the proper branch.
Comment #3
dgorton commented+1 for this functionality. Testing patch from #2 on one of our sites.
Comment #4
hillaryneaf commentedWhat are chances a patch like this could be rolled out for version 5.x-2.3? We really need this same functionality!
Comment #5
jmoy commentedBumping priority to critical, since the scheduling feature is effectively useless unless this bug is patch.
Including patch based on patch in #2 above. The only change is that user 1 is allowed to schedule any transition.
Comment #6
elgreg commented+1 for this patch - definitely critical.
--- UPDATE ---
I tried using this patch, but it appears that the latest verions (rc3 and .dev) do not have a $force parameter for the workflow_execute_transition function, so the patch does nothing - did the $force parameter get dropped and is there another solution that has been put into the module?
If I add a $force = FALSE to the workflow_execute_transition function, the patch seems to work on rc3.
Comment #7
1sp commentedHi All,
Just got the quick hack working to fix the problem.
workflow_execute_transition function seems to take a $force parameter but its not defined/documented anywhere
I am attaching the patch for review
Comment #8
xamountsubscribing
Comment #9
jvandyk commentedCommitted using the $force approach. Permissions checking should already be taken care of at scheduling time by workflow_field_choices(). Thanks, everyone.
Comment #10
jvandyk commentedAlso committed to DRUPAL-5 branch.