Request: @request
' => array('@request' => $_SERVER['REQUEST_METHOD'] . ' ' . $_GET['q']), + 'Headers:@headers' => array('@headers' => $header), + ), WATCHDOG_WARNING); return FALSE; } @@ -2681,6 +2689,11 @@ function mollom_moderate_validate_oauth() { // Validate nonce. if (empty($header['oauth_nonce'])) { + _mollom_watchdog(array( + 'Missing authentication nonce' => array(), + '
Request: @request
' => array('@request' => $_SERVER['REQUEST_METHOD'] . ' ' . $_GET['q']), + 'Headers:@headers' => array('@headers' => $header), + ), WATCHDOG_WARNING); return FALSE; } if ($cached = cache_get('mollom_moderation_nonces', 'cache')) { @@ -2714,7 +2727,18 @@ function mollom_moderate_validate_oauth() { $key = Mollom::rawurlencode($privateKey) . '&' . ''; $signature = rawurlencode(base64_encode(hash_hmac('sha1', $base_string, $key, TRUE))); - return $signature === $sent_signature; + $valid = ($signature === $sent_signature); + if (!$valid) { + _mollom_watchdog(array( + 'Invalid authentication signature' => array(), + '
Request: @request
' => array('@request' => $_SERVER['REQUEST_METHOD'] . ' ' . $_GET['q']), + 'Headers:@headers' => array('@headers' => $header + array('oauth_signature' => $sent_signature)), + 'Base string:
@base_string' => array('@base_string' => $base_string), + 'Expected signature:
@signature' => array('@signature' => $signature), + //'Expected key:
@key' => array('@key' => $key), + ), WATCHDOG_WARNING); + } + return $valid; } /** @@ -2732,6 +2756,7 @@ function mollom_moderate_validate_oauth() { */ function mollom_moderate($data, $action) { $result = FALSE; + $messages = array(); // Verify that action is supported or respond with a 404. if (!in_array($action, array('approve', 'spam', 'delete'))) { @@ -2742,54 +2767,74 @@ function mollom_moderate($data, $action) { $form_info = mollom_form_load($data->form_id); if ($form_info) { + if (isset($form_info['entity load callback'])) { + $load_callback = $form_info['entity load callback']; + $messages[] = "May load $form_info[entity] entity via {$form_info['entity load callback']}()."; + } + elseif (function_exists($data->entity . '_load')) { + $load_callback = $data->entity . '_load'; + $messages[] = "May load $form_info[entity] entity via {$data->entity}_load()."; + } + if (isset($form_info['entity save callback'])) { + $save_callback = $form_info['entity save callback']; + $messages[] = "May save $form_info[entity] entity via {$form_info['entity save callback']}()."; + } + elseif (function_exists($data->entity . '_save')) { + $save_callback = $data->entity . '_save'; + $messages[] = "May save $form_info[entity] entity via {$data->entity}_save()."; + } + if (isset($form_info['entity delete multiple callback'])) { + $delete_callback = $form_info['entity delete multiple callback']; + $messages[] = "May delete $form_info[entity] entity via {$form_info['entity delete multiple callback']}()."; + } + elseif (function_exists($data->entity . '_delete')) { + $delete_callback = $data->entity . '_delete'; + $messages[] = "May delete $form_info[entity] entity via {$data->entity}_delete()."; + } + // Programmatically invoke publish action. if ($action == 'approve') { // @todo Abstract this. Possibly requires Entity module as dependency. if ($data->entity == 'comment') { + $messages[] = 'Invoked comment-specific code.'; if ($comment = _comment_load($data->id)) { + $messages[] = "Loaded comment {$data->id} via _comment_load()."; // comment_invoke_comment() is a mess, so we abuse the action. comment_publish_action($comment); + $messages[] = 'Invoked comment publish action.'; // comment_admin_overview_submit() additionally updates node statistics // and invokes hooks, but the action does not. _comment_update_node_statistics($comment->nid); comment_invoke_comment($comment, 'publish'); + $messages[] = 'Invoked comment publish hooks.'; $result = TRUE; } } - else { - if (isset($form_info['entity load callback'])) { - $load_callback = $form_info['entity load callback']; - } - elseif (function_exists($data->entity . '_load')) { - $load_callback = $data->entity . '_load'; - } - if (isset($form_info['entity save callback'])) { - $save_callback = $form_info['entity save callback']; - } - elseif (function_exists($data->entity . '_save')) { - $save_callback = $data->entity . '_save'; - } - } - if (isset($load_callback) && isset($save_callback) && ($entity = $load_callback($data->id))) { + elseif (isset($load_callback) && isset($save_callback) && ($entity = $load_callback($data->id))) { $entity = (object) $entity; $entity->status = 1; $save_callback($entity); + $messages[] = "Updated status of $form_info[entity] via $load_callback() and $save_callback()."; + $result = TRUE; } } // Invoke entity delete multiple callback. - elseif (!empty($form_info['entity delete multiple callback'])) { - if (function_exists($form_info['entity delete multiple callback'])) { - $function = $form_info['entity delete multiple callback']; - $function(array($data->id)); + elseif (isset($delete_callback)) { + $messages[] = "Attempt to delete $form_info[entity] via $delete_callback()."; + if (function_exists($delete_callback)) { + $delete_callback(array($data->id)); + $messages[] = "Deleted $form_info[entity] via $delete_callback()."; + // Entity delete callbacks do not return success or failure, so we can // only assume success. $result = TRUE; } } // Programmatically invoke delete confirmation form. - elseif (isset($form_info['delete form'])) { + elseif (isset($form_info['delete form']) && isset($load_callback)) { + $messages[] = "Attempt to delete $form_info[entity] via {$form_info['delete form']} form."; if (isset($form_info['delete form file'])) { $file = $form_info['delete form file'] + array( 'type' => 'inc', @@ -2797,6 +2842,7 @@ function mollom_moderate($data, $action) { 'name' => NULL, ); module_load_include($file['type'], $file['module'], $file['name']); + $messages[] = "Loaded {$file['name']}.{$file['type']} in {$file['module']}."; } // Programmatic form submissions are not able to automatically use the // primary form submit button/action, so we need to resemble @@ -2806,7 +2852,7 @@ function mollom_moderate($data, $action) { // We assume that all delete confirmation forms take the fully loaded // entity as (only) argument. $args = array($form_id, &$form_state); - $args[] = $form_info['entity loader']($data->id); + $args[] = $load_callback($data->id); $form = call_user_func_array('drupal_retrieve_form', $args); $form_state['values'] = array(); @@ -2822,11 +2868,13 @@ function mollom_moderate($data, $action) { drupal_prepare_form($form_id, $form, $form_state); drupal_process_form($form_id, $form, $form_state); - // @todo Check actual result. - $result = TRUE; + $errors = form_get_errors(); + $result = empty($errors); } } + $messages = implode("\n", $messages); + if ($result) { _mollom_watchdog(array( '%action moderation success for @entity @id' => array( @@ -2834,26 +2882,28 @@ function mollom_moderate($data, $action) { '@entity' => $data->entity, '@id' => $data->id, ), - 'Mollom data:
@data' => array('@data' => (array) $data), - 'Messages:
@messages' => array('@messages' => drupal_get_messages()), )); } else { + // 400 is not really appropriate, but comes closest. + drupal_set_header($_SERVER['SERVER_PROTOCOL'] . ' 400 Unable to process moderation request'); + _mollom_watchdog(array( '%action moderation failure for @entity @id' => array( '%action' => $action, '@entity' => $data->entity, '@id' => $data->id, ), + 'Form ID:
@form_id' => array('@form_id' => $data->form_id), + 'Actions:
@actions' => array('@actions' => $messages), 'Mollom data:
@data' => array('@data' => (array) $data), 'Mollom form:
@form' => array('@form' => $form_info), - 'Messages:
@messages' => array('@messages' => drupal_get_messages()), + 'System messages:
@messages' => array('@messages' => drupal_get_messages()), ), WATCHDOG_ERROR); } // Report back result as success status. echo (int) $result; - exit; } /** @@ -3015,7 +3065,7 @@ function comment_mollom_form_list() { 'title' => t('Comment form'), 'entity' => 'comment', //'entity delete multiple callback' => 'comment_mollom_report_delete', - 'entity loader' => '_comment_load', + 'entity load callback' => '_comment_load', 'delete form' => 'comment_confirm_delete', 'delete form file' => array( 'name' => 'comment.admin', diff --git a/tests/mollom.test b/tests/mollom.test index 4b29f05..08b3691 100644 --- a/tests/mollom.test +++ b/tests/mollom.test @@ -87,6 +87,12 @@ class MollomWebTestCase extends DrupalWebTestCase { */ protected $createKeys = TRUE; + function __construct($test_id = NULL) { + parent::__construct($test_id); + // Hide this base class in assertions. + $this->skipClasses[__CLASS__] = TRUE; + } + /** * Set up an administrative user account and testing keys. */ @@ -178,7 +184,7 @@ class MollomWebTestCase extends DrupalWebTestCase { module_load_include('inc', 'dblog', 'dblog.admin'); $this->messages = array(); - $result = db_query("SELECT * FROM {watchdog} WHERE type = 'mollom' ORDER BY timestamp ASC"); + $result = db_query("SELECT * FROM {watchdog} ORDER BY timestamp ASC"); // The comparison logic applied in this function is a bit confusing, since // the values of the watchdog severity level constants in Drupal core are @@ -199,11 +205,11 @@ class MollomWebTestCase extends DrupalWebTestCase { $this->error($output, 'User notice'); } else { - $this->pass($output, t('Watchdog')); + $this->pass($output, t('Watchdog: @type', array('@type' => $row->type))); } } else { - $this->fail($output, t('Watchdog')); + $this->fail($output, t('Watchdog: @type', array('@type' => $row->type))); } // In case a severe message is expected, non-severe messages always pass, // since we would trigger a false positive test failure otherwise. @@ -1532,7 +1538,7 @@ class MollomAccessTestCase extends MollomWebTestCase { // settings page. $this->web_user = $this->drupalCreateUser(array_diff(module_invoke_all('perm'), array('administer mollom'))); $this->drupalLogin($this->web_user); - $this->drupalGet('admin/settings/mollom'); + $this->drupalGet('admin/settings/mollom', array('watchdog' => WATCHDOG_WARNING)); $this->assertResponse(403); } } @@ -3455,8 +3461,10 @@ class MollomAnalysisTestCase extends MollomWebTestCase { $this->drupalLogin($this->admin_user); // Verify that mollom_basic_test_form cannot be configured to put posts into // moderation queue. + $this->setProtection('mollom_basic_elements_test_form'); $this->drupalGet('admin/settings/mollom/manage/mollom_basic_elements_test_form'); $this->assertNoFieldByName('mollom[discard]'); + // Configure mollom_test_form to accept bad posts. $this->setProtection('mollom_test_form', MOLLOM_MODE_ANALYSIS, NULL, array( 'mollom[checks][profanity]' => TRUE, @@ -3874,12 +3882,41 @@ class MollomModerationIntegrationTestCase extends MollomWebTestCase { $headers = array(); $this->mollom->addAuthentication($method, $endpoint, $path, $data, $headers); - // Apply any OAuth header hacks. - foreach ($oauth_hacks as $name => $value) { - $headers['Authorization'] = preg_replace('@(' . $name . ')="[^"]*"@', '$1="' . $value . '"', $headers['Authorization']); + if (!empty($oauth_hacks)) { + // Extract OAuth header. + $input = $headers['Authorization']; + preg_match_all('@([^, =]+)="([^"]*)"@', $input, $header); + $oauth = array_combine($header[1], $header[2]); + + // Apply any OAuth header hacks. + foreach ($oauth_hacks as $name => $value) { + $oauth[$name] = $value; + } + // PHPWTF: Can't access static methods on class in a property. + $class = $this->mollom; + + // Regenerate signature based on hacked protocol parameters. + // Tests do not send additional GET/POST data at this point, so we do not + // account for that possibility. + // @see Mollom::addAuthentication() + if (!isset($oauth_hacks['oauth_signature'])) { + $oauth_query = $class::httpBuildQuery($oauth); + $base_string = implode('&', array( + $method, + $class::rawurlencode($endpoint . '/' . $path), + $class::rawurlencode($oauth_query), + )); + $key = $class::rawurlencode($this->mollom->privateKey) . '&' . ''; + $oauth['oauth_signature'] = base64_encode(hash_hmac('sha1', $base_string, $key, TRUE)); + } + + foreach ($oauth as $key => $value) { + $oauth[$key] = $key . '="' . $class::rawurlencode($value) . '"'; + } + $headers['Authorization'] = 'OAuth ' . implode(', ', $oauth); } - // curlexec() expects HTTP headers as "name: value" strings. + // curlExec() expects HTTP headers as "name: value" strings. $curl_headers = array(); foreach ($headers as $name => $value) { $curl_headers[] = $name . ': ' . $value; @@ -3945,8 +3982,8 @@ class MollomModerationIntegrationTestCase extends MollomWebTestCase { )); $this->mollomRequest('POST', $path); + $this->assertMollomWatchdogMessages(WATCHDOG_WARNING); $this->assertResponse(403); - $this->assertMollomWatchdogMessages(); $record = mollom_test_load($data->id); $this->assertTrue(!empty($record) && $record['status'] == 0, 'Test post still exists.'); @@ -3956,14 +3993,20 @@ class MollomModerationIntegrationTestCase extends MollomWebTestCase { $this->drupalLogout(); $replay_attack_nonce = md5(microtime() . mt_rand()); + + // Prime the nonce cache in order to test the replay attack. + $nonces = array(); + $nonces[$replay_attack_nonce] = time(); + cache_set('mollom_moderation_nonces', $nonces, 'cache'); + $tests = array( // Test entirely missing OAuth header throws no notices or exceptions. array('class' => array('oAuthStrategy' => '')), // Test OAuth query parameters are not supported. array('class' => array('oAuthStrategy' => 'query')), // Test missing or invalid public key. - array('class' => array('publicKey' => ''), 'watchdog' => WATCHDOG_WARNING), - array('class' => array('publicKey' => md5('foo')), 'watchdog' => WATCHDOG_WARNING), + array('class' => array('publicKey' => '')), + array('class' => array('publicKey' => md5('foo'))), // Test missing or invalid private key. // Only impacts the resulting oauth_signature and there is no Mollom log // message on a signature mismatch, just a regular 403 response. @@ -3971,10 +4014,10 @@ class MollomModerationIntegrationTestCase extends MollomWebTestCase { array('class' => array('privateKey' => md5('foo'))), // Test nonce replay attack, more than once. array('hacks' => array('oauth_nonce' => $replay_attack_nonce)), - array('hacks' => array('oauth_nonce' => $replay_attack_nonce), 'watchdog' => WATCHDOG_WARNING), - array('hacks' => array('oauth_nonce' => $replay_attack_nonce), 'watchdog' => WATCHDOG_WARNING), - // Test invalid/oudated timestamp. - array('hacks' => array('oauth_timestamp' => time() - 1000), 'watchdog' => WATCHDOG_WARNING), + array('hacks' => array('oauth_nonce' => $replay_attack_nonce)), + // Test invalid/outdated timestamp. + array('hacks' => array('oauth_timestamp' => time() - 1000)), + array('hacks' => array('oauth_timestamp' => time() + 1000)), // Test invalid signature. array('hacks' => array('oauth_signature' => base64_encode(hash_hmac('sha1', 'foo', 'bar', TRUE)))), // Test unsupported signature hashing algo. @@ -3986,9 +4029,7 @@ class MollomModerationIntegrationTestCase extends MollomWebTestCase { $test += array( 'class' => array(), 'hacks' => array(), - // Watchdog log message assertion only accounts for Mollom messages, - // not for generic 403s. - 'watchdog' => WATCHDOG_NOTICE, + 'watchdog' => WATCHDOG_WARNING, ); foreach ($test['class'] as $key => $value) { $this->mollomBackup->$key = $this->mollom->$key; @@ -3996,8 +4037,8 @@ class MollomModerationIntegrationTestCase extends MollomWebTestCase { } // Execute test. $this->mollomRequest('POST', $path, $test['hacks']); - $this->assertResponse(403); $this->assertMollomWatchdogMessages($test['watchdog']); + $this->assertResponse(403); $record = mollom_test_load($data->id); $this->assertTrue(!empty($record) && $record['status'] == 0, 'Test post still exists.'); @@ -4022,13 +4063,13 @@ class MollomModerationIntegrationTestCase extends MollomWebTestCase { // Send a moderation request for the post. $path = 'mollom/moderate/' . $data->contentId . '/' . $action; $this->mollomRequest('POST', $path); - $this->assertMollomWatchdogMessages(); $record = mollom_test_load($data->id); $new_data = mollom_data_load('mollom_test', $data->id); // Verify that the post was published. if ($action == 'approve') { + $this->assertMollomWatchdogMessages(); $this->assertResponse(200); $this->assertSame('Publishing status', $record['status'], 1); $this->assertTrue($new_data, 'Mollom data for approved post still exists.'); @@ -4036,12 +4077,14 @@ class MollomModerationIntegrationTestCase extends MollomWebTestCase { } // Verify that the post was deleted. elseif ($action == 'spam' || $action == 'delete') { + $this->assertMollomWatchdogMessages(); $this->assertResponse(200); $this->assertFalse($record, 'Test post not found.'); $this->assertFalse($new_data, 'Moderated/deleted record no longer exists.'); } // Verify that post still exists after invalid/unsupported action. else { + $this->assertMollomWatchdogMessages(WATCHDOG_WARNING); $this->assertResponse(404); $this->assertTrue(!empty($record) && $record['status'] == 0, 'Test post still exists.'); $this->assertTrue($new_data->moderate, 'Post is not flagged as moderated.'); diff --git a/tests/mollom_test.module b/tests/mollom_test.module index 4689189..d0577d9 100644 --- a/tests/mollom_test.module +++ b/tests/mollom_test.module @@ -15,6 +15,12 @@ function mollom_test_menu() { 'page arguments' => array('mollom_test_form'), 'access callback' => TRUE, ); + $items['mollom-test/form/%mollom_test/delete'] = array( + 'title' => 'Delete Mollom test item', + 'page callback' => 'drupal_get_form', + 'page arguments' => array('mollom_test_delete_form', 2), + 'access callback' => TRUE, + ); return $items; } @@ -35,8 +41,8 @@ function mollom_test_mollom_form_list() { $forms['mollom_test_form'] = array( 'title' => 'Mollom test form', 'entity' => 'mollom_test', - 'entity delete multiple callback' => 'mollom_test_delete_multiple', 'moderation callback' => 'mollom_test_mollom_form_moderation', + 'delete form' => 'mollom_test_delete_form', ); // The basic test form is identical to the mollom_test_form, but only // registers minimal information (e.g., no entity or moderation callback) to @@ -48,6 +54,13 @@ function mollom_test_mollom_form_list() { $forms['mollom_basic_elements_test_form'] = array( 'title' => 'Mollom basic elements test form', ); + // Same as mollom_test_form, but supports an entity delete callback. + // @todo Test mollom/moderate endpoint with entity delete callback. + $forms['mollom_entity_test_form'] = array( + 'title' => 'Mollom entity test form', + 'entity' => 'mollom_test', + 'entity delete multiple callback' => 'mollom_test_delete_multiple', + ); return $forms; } @@ -283,3 +296,25 @@ function mollom_test_delete_multiple(array $mids) { function mollom_entity_delete($entity, $type) { mollom_data_delete('mollom_test', $entity->mid); } + +/** + * Form constructor for deleting a Mollom test item. + */ +function mollom_test_delete_form(&$form_state, $record) { + $form['mid'] = array('#type' => 'value', '#value' => $record['mid']); + return confirm_form($form, + t('Are you sure you want to delete the %title?', array('%title' => $record['title'])), + 'mollom-test/form/' . $record['mid'], + NULL, + t('Delete') + ); +} + +/** + * Form submission handler for mollom_test_delete_form(). + */ +function mollom_test_delete_form_submit($form, &$form_state) { + mollom_test_delete_multiple(array($form_state['values']['mid'])); + drupal_set_message('The record has been deleted.'); + $form_state['redirect'] = 'mollom-test/form'; +}