I've discovered that when ooyala_sync_pull_videos() is executed during cron runs it will only sync published nodes. It seems that this is being caused by node access restrictions preventing cron from finding the unpublished nodes in ooyala_load_nodes(). This is because cron is always run as the anonymous user. ie. The function drupal_cron_run() literally does this $GLOBALS['user'] = drupal_anonymous_user();
It seems to me that cron should be able to synchronize Backlot data for an unpublished node, given that a user will not necessarily want to publish a node until Backlot has finished processing it's attached videos. Currently the ooyala module's cron function will attempt to sync an unknown embed code up to 10 times before giving up completely (which probably masks this problem on sites with a large interval between cron runs), but on a site which runs cron every 5 minutes it is quite conceivable that the ooyala module will give up before a user returns to check whether their video is ready to be published.
Having thought about this, I think that the solution is to disable node access on the EntityFieldQuery in ooyala_load_nodes(). But obviously I'll defer to a maintainer's judgement on whether this presents any security issues.
I'll provide a patch which resolves the issue in this way. Let me know your thoughts.
| Comment | File | Size | Author |
|---|---|---|---|
| #1 | fix-sync-node-access-2154245-1.patch | 1.16 KB | jamesharv |
Comments
Comment #1
jamesharv commentedPatch attached
Comment #2
jamesharv commentedFYI I have also noticed that
ooyala_ooyala_sync_pull_videos()(which is also involved in the cron sync), is faulty. I'll start a separate issue for that.Comment #3
simeI wonder if this should be driven by a setting... or an alter by another module might also be the way to go (if possible).
The reason I say this is, is it setting a bad example using DANGEROUS_ACCESS_CHECK_OPT_OUT out of the box... maybe there are use-cases we don't know about, where access should be checked.
Comment #4
jamesharv commented@sime I take your point, but it seems like this function was originally designed to ignore node access. The fact that it doesn't seems to be a hangover from the way the module was ported from the D6 version.
The D6 version of the
ooyala_load_nodes()function usesdb_query()to do direct database queries when loading nids (see http://drupalcode.org/project/ooyala.git/blob/refs/heads/6.x-2.x:/ooyala...). In the D7 version this has been ported to use EntityFieldQuery, which is great, but EntityFieldQuery obeys node access which is causing this issue.Incidentally this bug also affect the auto-publish on ping feature, as that function is performed by cron as well.
I'm open to doing more work on this if required, but it seems like disabling node access for the query restores this function to its originally intended behaviour.
Comment #5
simeThat seems pretty fair, if it wasn't doing access checking before, it needn't do it now.
Comment #6
quicksketchThis patch seems legitimate to me. Considering the primary purpose of the ping feature is the auto-publishing of content when the video has finished processing, the access check preventing cron from publishing is a bug.
If there are any concerns around skipping access checks, it's going to be a problem we'll need to fix in D6. So all-told this looks good to me and I've comitted/pushed it to the 7.x-2.x branch.