diff --git a/build-petitions.make b/build-petitions.make index 0ff8891d3..ec1c6a330 100644 --- a/build-petitions.make +++ b/build-petitions.make @@ -19,5 +19,5 @@ projects[drupal][patch][] = https://drupal.org/files/issues/drupal-insertassert_ ; Petitions installation profile ; ------------------------------- projects[petitions][type] = profile -projects[petitions][download][tag] = 7.x-2.0-rc24 +projects[petitions][download][tag] = 7.x-2.0-rc26 projects[petitions][download][type] = git diff --git a/drupal-org.make b/drupal-org.make index 584e43e83..2aebdc7bf 100644 --- a/drupal-org.make +++ b/drupal-org.make @@ -72,7 +72,7 @@ projects[pathauto][version] = 1.2 projects[petitionssignatureform][type] = module projects[petitionssignatureform][download][type] = git projects[petitionssignatureform][download][url] = http://git.drupal.org/sandbox/whitehouse/2283727.git -projects[petitionssignatureform][download][revision] = 212fb88e21a0deabd7a6831be6a8913b3d8fc554 +projects[petitionssignatureform][download][revision] = d855b200d2c9fadfb2bf22f901d679f1c3724d50 projects[profile2][version] = 1.3 diff --git a/modules/custom/petitions_api/signatures_queue/includes/preprocess_signatures.inc b/modules/custom/petitions_api/signatures_queue/includes/preprocess_signatures.inc index a06baf8ee..2046039c1 100644 --- a/modules/custom/petitions_api/signatures_queue/includes/preprocess_signatures.inc +++ b/modules/custom/petitions_api/signatures_queue/includes/preprocess_signatures.inc @@ -198,6 +198,8 @@ function _signatures_queue_preprocess_signatures_safe_insert_batch_into_table($b // against data in the database. $new_signatures = _signatures_queue_preprocess_signatures_dedupe_signatures($table_name, $batch); + $sanitized_signatures = _signatures_queue_preprocess_signatures_sanitize_batch($table_name, $new_signatures); + // Build insert query. $fields = _signatures_queue_preprocess_signatures_insert_batch_fields($table_name); signatures_queue_set_db(); @@ -205,12 +207,7 @@ function _signatures_queue_preprocess_signatures_safe_insert_batch_into_table($b $query->fields(array_values($fields)); // Add values. - foreach ($new_signatures as $item) { - // Typecast the "signup" value for database type compatibility. - if (array_key_exists('signup', $item->data)) { - $item->data['signup'] = (int) $item->data['signup']; - } - + foreach ($sanitized_signatures as $item) { // Restrict values to those that correspond to database fields. $values = array_intersect_key((array) $item->data, $fields); @@ -237,6 +234,67 @@ function _signatures_queue_preprocess_signatures_safe_insert_batch_into_table($b return TRUE; } +/** + * Sanitizes a batch of items for safe database insertion. + * + * Prevents database errors from type, length, and character encoding issues. + * + * @param string $table_name + * The name of the table the batch will be inserted into. + * @param array $batch + * A batch of items as returned by + * _signatures_queue_preprocess_signatures_claim_batch(). + * + * @throws Exception + * Throws an exception in case of an invalid table name argument. + * + * @return array + * Returns the sanitized batch of items. + */ +function _signatures_queue_preprocess_signatures_sanitize_batch($table_name, $batch) { + // Get the schema for the table the batch will be inserted into. + module_load_install('signatures_queue'); + if ($table_name == 'signatures_pending_validation') { + $schema = _signatures_queue_get_signatures_schema(); + } + elseif ($table_name = 'validations') { + $schema = _signatures_queue_get_validations_schema(); + } + else { + throw new Exception(t('Invalid table name.')); + } + + // Loop through batch items. + foreach ($batch as $item) { + // Loop through item values. + foreach ($item->data as $key => $value) { + if (isset($schema['fields'][$key]['type'])) { + + // Sanitize VARCHAR fields. + if ($schema['fields'][$key]['type'] == 'varchar') { + // Prevent PDO exceptions from unsupported characters like this guy: + // http://www.fileformat.info/info/unicode/char/1F600/index.htm. (Not + // kidding.) + $plain_value = utf8_encode($value); + + // Truncate value to the maximum length of its target database column. + $trimmed_value = drupal_substr($plain_value, 0, $schema['fields'][$key]['length']); + + $item->data[$key] = $trimmed_value; + } + + // Sanitize INT fields. + elseif ($schema['fields'][$key]['type'] == 'int') { + // Make sure INTs come through as actual INTs! + $item->data[$key] = (int) $item->data[$key]; + } + } + } + } + + return $batch; +} + /** * Returns an array of fields to be used for inserting batch into local db. * diff --git a/modules/custom/petitions_api/signatures_queue/includes/process_signatures.inc b/modules/custom/petitions_api/signatures_queue/includes/process_signatures.inc index ec35681fb..1b4b19919 100644 --- a/modules/custom/petitions_api/signatures_queue/includes/process_signatures.inc +++ b/modules/custom/petitions_api/signatures_queue/includes/process_signatures.inc @@ -35,6 +35,7 @@ * @see signatures_queue_invoke_workflow() */ function _signatures_queue_process_signatures($job_id, $server_name, $worker_name, $options) { + $watchdog_suffix = _signatures_queue_watchdog_suffix('process_signatures', $job_id, $server_name, $worker_name); $limit = signatures_queue_get_queue_batch_size('process_signatures'); // Set the active database to the signatures_processing db. @@ -62,10 +63,10 @@ function _signatures_queue_process_signatures($job_id, $server_name, $worker_nam // Process the batch. while ($item = $result->fetchAssoc()) { // Make sure timestamps are valid and petition IDs match. - if (!_signatures_queue_process_signatures_assert_legitimate($item)) { + if (!_signatures_queue_process_signatures_assert_legitimate($item) || !_signatures_queue_process_signatures_valid_petition($item['petition_id'])) { // Skip processing illegitimate item. $item['signature_id'] = 'illegitimate'; - _signatures_queue_process_signatures_move_to_processed($item); + _signatures_queue_process_signatures_move_to_processed($item, $watchdog_suffix); // Keep track of how many invalid matches are skipped and considered // "processed". $count_illegitimate++; @@ -87,7 +88,6 @@ function _signatures_queue_process_signatures($job_id, $server_name, $worker_nam } else { // Log failure. - $watchdog_suffix = _signatures_queue_watchdog_suffix('process_signatures', $job_id, $server_name, $worker_name); watchdog('signatures_queue', 'New signature could not be created. secret_validation_key: @secret_validation_key, vid: @vid, petition_id: @petition_id, timestamp_validated: @timestamp_validated. @suffix', array( '@secret_validation_key' => $item['secret_validation_key'], '@vid' => $item['vid'], @@ -104,10 +104,10 @@ function _signatures_queue_process_signatures($job_id, $server_name, $worker_nam // module_invoke_all('save_signature', $item); // Add signature ID and API key to signatures_validations table. - _signatures_queue_process_signatures_add_to_signatures_validations($item); + _signatures_queue_process_signatures_add_to_signatures_validations($item, $watchdog_suffix); // Move item to processed tables and delete from pre-process tables. - _signatures_queue_process_signatures_move_to_processed($item); + _signatures_queue_process_signatures_move_to_processed($item, $watchdog_suffix); // Subscribe to list if requested. if ((bool) $item['signup']) { @@ -186,6 +186,34 @@ function _signatures_queue_process_signatures_assert_legitimate($item) { return $legitimate; } +/** + * Determine whether a given petition ID is valid. + * + * If petition IDs in the two records from validations and + * signatures_pending_validation do NOT match, this is suspicious behavior + * indicating someone potentially gaming the system. Alert and notify people. + * + * @param string $petition_id + * The petition ID. + * + * @return bool + * Returns TRUE if petition is valid, FALSE if not. + */ +function _signatures_queue_process_signatures_valid_petition($petition_id) { + // @todo This should really use the logic in Petition::isSignable(). And + // Petition::isSignable should really be updated to honor the signatures + // queue grace period if passed in as a param. But getting that to work + // will take some refactoring. For now, just try to load the petition. If + // you can't load it, you shouldn't be able to sign it. It was likely + // been removed/unpublished by an administrator because it violates terms of + // service. + $petition = petitions_data_get_petition($petition_id); + if (!$petition) { + return FALSE; + } + return TRUE; +} + /** * If user with this email does not exist, create one. * @@ -204,7 +232,7 @@ function _signatures_queue_process_signatures_create_user($item) { $user = (object) array( 'is_new' => TRUE, 'mail' => trim($item['email']), - 'name' => preg_replace("/[^\x80-\xF7 [:alnum:]@]/", '_', trim($item['email'])), + 'name' => _signatures_queue_process_signatures_get_unique_username($item['email']), 'status' => 1, ); $new_user = user_save($user); @@ -279,27 +307,38 @@ function _signatures_queue_process_signatures_create_record($item, $user) { * * @param string $item * The item from the database query. + * @param string $watchdog_suffix + * A string of job details as created by _signatures_queue_watchdog_suffix(). * * @return bool * Returns TRUE on success. */ -function _signatures_queue_process_signatures_add_to_signatures_validations($item) { +function _signatures_queue_process_signatures_add_to_signatures_validations($item, $watchdog_suffix) { // Save to database. - $id = db_insert('signature_validations') - ->fields(array( - 'secret_validation_key' => $item['secret_validation_key'], - 'signature_id' => $item['signature_id'], - 'petition_id' => $item['petition_id'], - 'signature_source_api_key' => $item['signature_source_api_key'], - 'email' => $item['email'], - 'timestamp_processed' => time(), - )) - ->execute(); + try { + db_insert('signature_validations') + ->fields(array( + 'secret_validation_key' => $item['secret_validation_key'], + 'signature_id' => $item['signature_id'], + 'petition_id' => $item['petition_id'], + 'signature_source_api_key' => $item['signature_source_api_key'], + 'email' => $item['email'], + 'timestamp_processed' => time(), + )) + ->execute(); + } + catch (PDOException $exception) { + // @todo Abuse detection. + watchdog('signatures_queue', 'An item could not be added due to a database error: item: !item, exception: !exception. @suffix', array( + '!item' => signatures_queue_format_for_logging($item), + '!exception' => signatures_queue_format_for_logging($exception), + '@suffix' => $watchdog_suffix, + ), WATCHDOG_CRITICAL); + } return TRUE; } - /** * Move items to processed tables. * @@ -308,49 +347,102 @@ function _signatures_queue_process_signatures_add_to_signatures_validations($ite * * @param string $item * The item from the database query. + * @param string $watchdog_suffix + * A string of job details as created by _signatures_queue_watchdog_suffix(). * * @return bool * Returns TRUE on success. */ -function _signatures_queue_process_signatures_move_to_processed($item) { +function _signatures_queue_process_signatures_move_to_processed($item, $watchdog_suffix) { // Set the active database to the signatures_processing db. signatures_queue_set_db(); // Add to processed tables. - $id = db_insert('signatures_processed') - ->fields(array( - 'signature_source_api_key' => $item['signature_source_api_key'], - 'timestamp_petition_close' => $item['timestamp_petition_close'], - 'timestamp_validation_close' => $item['timestamp_validation_close'], - 'petition_id' => $item['petition_id'], - 'first_name' => $item['first_name'], - 'last_name' => $item['last_name'], - 'zip' => $item['zip'], - 'email' => $item['email'], - 'signup' => $item['signup'], - 'timestamp_validation_email_sent' => $item['timestamp_validation_email_sent'], - 'timestamp_submitted' => $item['timestamp_submitted'], - 'secret_validation_key' => $item['secret_validation_key'], - )) - ->execute(); - $id = db_insert('validations_processed') - ->fields(array( - 'secret_validation_key' => $item['secret_validation_key'], - 'timestamp_validated' => $item['timestamp_validated'], - 'timestamp_validation_close' => $item['timestamp_validation_close'], - 'client_ip' => $item['client_ip'], - 'petition_id' => $item['petition_id'], - )) - ->execute(); + $erred = FALSE; + $exception = NULL; + try { + db_insert('signatures_processed') + ->fields(array( + 'signature_source_api_key' => $item['signature_source_api_key'], + 'timestamp_petition_close' => $item['timestamp_petition_close'], + 'timestamp_validation_close' => $item['timestamp_validation_close'], + 'petition_id' => $item['petition_id'], + 'first_name' => $item['first_name'], + 'last_name' => $item['last_name'], + 'zip' => $item['zip'], + 'email' => $item['email'], + 'signup' => $item['signup'], + 'timestamp_validation_email_sent' => $item['timestamp_validation_email_sent'], + 'timestamp_submitted' => $item['timestamp_submitted'], + 'secret_validation_key' => $item['secret_validation_key'], + )) + ->execute(); + } + catch (PDOException $exception) { + $erred = TRUE; + } + try { + db_insert('validations_processed') + ->fields(array( + 'secret_validation_key' => $item['secret_validation_key'], + 'timestamp_validated' => $item['timestamp_validated'], + 'timestamp_validation_close' => $item['timestamp_validation_close'], + 'client_ip' => $item['client_ip'], + 'petition_id' => $item['petition_id'], + )) + ->execute(); + } + catch (PDOException $exception) { + $erred = TRUE; + } + + // Log errors. + if ($erred) { + // @todo Abuse detection. + watchdog('signatures_queue', 'An item could not be moved to processed due to a database error: item: !item, exception: !exception. @suffix', array( + '!item' => signatures_queue_format_for_logging($item), + '!exception' => signatures_queue_format_for_logging($exception), + '@suffix' => $watchdog_suffix, + ), WATCHDOG_CRITICAL); + } // Delete from pre-process tables. - $deleted = db_delete('signatures_pending_validation') + db_delete('signatures_pending_validation') ->condition('sid', $item['sid']) ->execute(); - $deleted = db_delete('validations') + db_delete('validations') ->condition('vid', $item['vid']) ->execute(); // Set the active database back to default. db_set_active(); } + +/** + * Get a unique username to correspond with a given email address. + * + * @param string $email + * The email address. + * + * @return string + * A unique username. + */ +function _signatures_queue_process_signatures_get_unique_username($email) { + // uniqid() gets a prefixed unique identifier based on the current time in + // microseconds, so it should never create a duplicate, even in a + // multithreaded processing scenario. + $prefix = ''; + $more_entropy = TRUE; + $unique_id = uniqid($prefix, $more_entropy); + $unique_id_length = strlen($unique_id); + + // The user "name" column in the database cannot exceed 60 characters, so the + // "safe email" value is truncated accordingly. + // @see user_schema() + $sanitized_email = preg_replace("/[^\x80-\xF7 [:alnum:]@]/", '_', trim($email)); + $max_username_length = 60; + $max_email_length = $max_username_length - $unique_id_length; + $safe_email = substr($sanitized_email, 0, $max_email_length); + + return "{$safe_email}{$unique_id}"; +} diff --git a/modules/custom/petitions_api/signatures_queue/includes/receive_new_signatures.inc b/modules/custom/petitions_api/signatures_queue/includes/receive_new_signatures.inc index 935cc357c..95615cd87 100644 --- a/modules/custom/petitions_api/signatures_queue/includes/receive_new_signatures.inc +++ b/modules/custom/petitions_api/signatures_queue/includes/receive_new_signatures.inc @@ -134,10 +134,12 @@ function _signatures_queue_build_new_queue_item(array $signature) { // Add application data to the item. $petition = petitions_data_get_petition($signature['petition_id']); - $validation_grace_period = (int) variable_get('signatures_queue_validation_grace_period', 0); + $validation_grace_period = (int) variable_get('signatures_queue_validation_grace_period', 2); $item['timestamp_submitted'] = time(); $item['timestamp_petition_close'] = $petition['deadline']; - $item['timestamp_validation_close'] = $petition['deadline'] + $validation_grace_period; + // The validation link is valid for today plus grace period (converted from + // days to unix time). + $item['timestamp_validation_close'] = time() + $validation_grace_period * 24 * 60 * 60; $item['secret_validation_key'] = ''; return $item; diff --git a/modules/custom/petitions_api/signatures_queue/signatures_queue.admin.inc b/modules/custom/petitions_api/signatures_queue/signatures_queue.admin.inc index 0393431db..fc0458f08 100644 --- a/modules/custom/petitions_api/signatures_queue/signatures_queue.admin.inc +++ b/modules/custom/petitions_api/signatures_queue/signatures_queue.admin.inc @@ -116,7 +116,7 @@ function signatures_queue_configure() { '#default_value' => variable_get('signatures_queue_validation_grace_period', 0), '#size' => 12, '#maxlength' => 12, - '#description' => t('Number of days after we stop accepting new signature submissions that a signature can still be validated.'), + '#description' => t('Number of days a signature validation link is valid.'), '#required' => TRUE, ); diff --git a/modules/custom/petitions_api/signatures_queue/signatures_queue.install b/modules/custom/petitions_api/signatures_queue/signatures_queue.install index 55fbca796..da2a9af31 100644 --- a/modules/custom/petitions_api/signatures_queue/signatures_queue.install +++ b/modules/custom/petitions_api/signatures_queue/signatures_queue.install @@ -43,6 +43,9 @@ function signatures_queue_install() { // Warn users about two week limit on archiving. drupal_set_message($t('IMPORTANT! The archiving workflow assumes you are processing signatures regularly. The default assumption is that all queues will be cleared within any given two week period. If you intend to NOT process signatures regularly, make the necessary adjustments to $queues_last_emptied in includes/archive_signatures.inc.'), 'warning'); + + // Start with a validation grace period of 2 days (48 hours). + signatures_queue_update_7001(); } /** @@ -428,3 +431,11 @@ function signatures_queue_uninstall() { ->execute(); cache_clear_all('variables', 'cache_bootstrap'); } + +/** + * Set default grace period to 2 days (48 hours). + */ +function signatures_queue_update_7001() { + variable_set('signatures_queue_validation_grace_period', 2); + drupal_set_message(t('Setting default signatures_queue_validation_grace_period to 2 days (48 hours).')); +} diff --git a/modules/custom/petitions_data/classes/Petition.inc b/modules/custom/petitions_data/classes/Petition.inc index 43bb06ff9..45be0b7da 100644 --- a/modules/custom/petitions_data/classes/Petition.inc +++ b/modules/custom/petitions_data/classes/Petition.inc @@ -600,6 +600,14 @@ class Petition { throw new Exception("Can't determine if petition is signable if petition status is not set."); } + // If the signature threshold has been passed and the petition has not + // received a response. + $signatures_passed_threshold = $this->getSignaturesNeeded() == 0; + $pending_response = $this->getStatus() == WH_PETITION_STATUS_UNDER_REVIEW; + if ($signatures_passed_threshold && $pending_response) { + return TRUE; + } + // Has the petition passed its deadline for accepting signatures? if ($this->deadline < time()) { return FALSE; diff --git a/modules/custom/wh_petitions/wh_petitions.module b/modules/custom/wh_petitions/wh_petitions.module index 6872b5297..9afbeeb70 100644 --- a/modules/custom/wh_petitions/wh_petitions.module +++ b/modules/custom/wh_petitions/wh_petitions.module @@ -1933,10 +1933,21 @@ function wh_petitions_get_signature_id($petition_id, $uid) { * ID of created signature or FALSE on failure. */ function wh_petitions_create_signature($petition, $user, $signature_collection_connection, $ip_address = '', $user_agent = '') { - // Can only sign petitions that are open - private, public, under review. - if ($petition['petition_status'] != WH_PETITION_STATUS_PRIVATE && $petition['petition_status'] != WH_PETITION_STATUS_PUBLIC && $petition['petition_status'] != WH_PETITION_STATUS_UNDER_REVIEW) { - watchdog('wh_petitions', 'unable to create signature on @petition because of invliad petition status (@status)', array('@petition' => $petition['_id'], '@status' => $petition['petition_status'])); - return FALSE; + // If petition is being signed through simplified signing, skip validation + // checks confirming petition is signable, defer to signatures_queue module's + // preprocess and process checks. + $active_signing_method = variable_get('petitions_signing_method', NULL); + if ($active_signing_method != WH_PETITIONS_SIGNING_METHOD_SIMPLIFIED_SIGNING) { + // Simplified signing is not enabled. Perform legacy validation checks + // before counting signature. Can only sign petitions that are open--that + // is, private, public, or under review. + if ($petition['petition_status'] != WH_PETITION_STATUS_PRIVATE && $petition['petition_status'] != WH_PETITION_STATUS_PUBLIC && $petition['petition_status'] != WH_PETITION_STATUS_UNDER_REVIEW) { + watchdog('wh_petitions', 'Unable to create signature on @petition due to invalid petition status (@status).', array( + '@petition' => $petition['_id'], + '@status' => $petition['petition_status'], + )); + return FALSE; + } } // Check if signature_id exists.