diff --git a/plugins/wporg-5ftf/includes/email.php b/plugins/wporg-5ftf/includes/email.php new file mode 100644 index 0000000..522ea88 --- /dev/null +++ b/plugins/wporg-5ftf/includes/email.php @@ -0,0 +1,143 @@ +', + 'Reply-To: support@wordcamp.org', + // todo update address when new one is created + ); + + return wp_mail( $to, $subject, $message, $headers ); +} + +/** + * Generate an action URL with a secure, unique authentication token. + * + * @param int $pledge_id + * @param string $action + * @param int $action_page_id The ID of the page that the user will be taken back to, in order to process their + * verification request. + * + * @return string + */ +function get_authentication_url( $pledge_id, $action, $action_page_id ) { + $auth_token = array( + // This will create a CSPRN and is similar to how `get_password_reset_key()` works. + 'value' => wp_generate_password( TOKEN_LENGTH, false ), + // todo should encrypt at rest? core doesn't but others do + 'expiration' => time() + ( 2 * HOUR_IN_SECONDS ), + ); + + /* + * Tying the token to a specific pledge is important for security, otherwise companies could get a valid token + * for their pledge, and use it to edit other company's pledges. + * + * Similarly, tying it to specific actions is also important, to protect against CSRF attacks. + * + * This function intentionally requires the caller to pass in a pledge ID and action, so that it can guarantee + * that each token will be unique across pledges and actions. + */ + update_post_meta( $pledge_id, TOKEN_PREFIX . $action, $auth_token ); + + $auth_url = add_query_arg( + array( + 'action' => $action, + 'pledge_id' => $pledge_id, + 'auth_token' => $auth_token['value'], + ), + get_permalink( $action_page_id ) + ); + + // todo include a "this lnk will expire in 10 hours and after its used once" message too? + // probably, but what's the best way to do that DRYly? + + return $auth_url; +} + +/** + * Verify whether or not a given authentication token is valid. + * + * These tokens are more secure than WordPress' imitation nonces, because they can only be used once, and expire + * in a shorter timeframe. Like WP nonces, though, they must be tied to a specific action and post object in order + * to prevent misuse. + * + * @param $pledge_id + * @param $action + * @param $unverified_token + * + * @return bool + */ +function is_valid_authentication_token( $pledge_id, $action, $unverified_token ) { + $verified = false; + $valid_token = get_post_meta( $pledge_id, TOKEN_PREFIX . $action, true ); + + /* + * Later on we'll compare the value to user input, and the user could input null/false/etc, so let's guarantee + * that the thing we're comparing against is really what we expect it to be. + */ + if ( ! is_array( $valid_token ) || ! array_key_exists( 'value', $valid_token ) || ! array_key_exists( 'expiration', $valid_token ) ) { + return false; + } + + if ( ! is_string( $valid_token['value'] ) || TOKEN_LENGTH !== strlen( $valid_token['value'] ) ) { + return false; + } + + if ( $valid_token && $valid_token['expiration'] > time() && $unverified_token === $valid_token['value'] ) { + $verified = true; + + // Tokens should not be reusable, to increase security. + delete_post_meta( $pledge_id, TOKEN_PREFIX . $action ); + // todo when used to manage pledge, token will probably get deleted when viewing, and then they won't be able to save + // fix that when create the manage process, though. for now this works for confirming email address. + // maye pass a `context` param to this function, either 'view' or 'update', and only delete if context is 'update' ? + // make sure view and update functions checks to make sure have valid token, not create though + } + + return $verified; +} diff --git a/plugins/wporg-5ftf/includes/pledge-form.php b/plugins/wporg-5ftf/includes/pledge-form.php index ef32532..950a970 100755 --- a/plugins/wporg-5ftf/includes/pledge-form.php +++ b/plugins/wporg-5ftf/includes/pledge-form.php @@ -6,7 +6,7 @@ namespace WordPressDotOrg\FiveForTheFuture\PledgeForm; use WordPressDotOrg\FiveForTheFuture; -use WordPressDotOrg\FiveForTheFuture\{ Pledge, PledgeMeta, Contributor }; +use WordPressDotOrg\FiveForTheFuture\{ Pledge, PledgeMeta, Contributor, Email }; use WP_Error, WP_User; defined( 'WPINC' ) || die(); @@ -21,11 +21,12 @@ add_shortcode( '5ftf_pledge_form_manage', __NAMESPACE__ . '\render_form_manage' * @return false|string */ function render_form_new() { - $action = filter_input( INPUT_POST, 'action' ); + $action = isset( $_GET['action'] ) ? filter_input( INPUT_GET, 'action' ) : filter_input( INPUT_POST, 'action' ); $data = get_form_submission(); $messages = []; $complete = false; $directory_url = get_permalink( get_page_by_path( 'pledges' ) ); + $view = 'form-pledge-new.php'; if ( 'Submit Pledge' === $action ) { $processed = process_form_new(); @@ -35,11 +36,16 @@ function render_form_new() { } elseif ( 'success' === $processed ) { $complete = true; } + } else if ( 'confirm_pledge_email' === $action ) { + $view = 'form-pledge-confirm-email.php'; + $pledge_id = filter_input( INPUT_GET, 'pledge_id', FILTER_VALIDATE_INT ); + $unverified_token = filter_input( INPUT_GET, 'auth_token', FILTER_SANITIZE_STRING ); + $email_confirmed = process_email_confirmation( $pledge_id, $action, $unverified_token ); } ob_start(); $readonly = false; - require FiveForTheFuture\PATH . 'views/form-pledge-new.php'; + require FiveForTheFuture\get_views_path() . $view; return ob_get_clean(); } @@ -111,6 +117,40 @@ function process_form_new() { return 'success'; } +/** + * Process a request to confirm a company's email address. + * + * @param int $pledge_id + * @param string $action + * @param array $unverified_token + * + * @return bool + */ +function process_email_confirmation( $pledge_id, $action, $unverified_token ) { + $meta_key = PledgeMeta\META_PREFIX . 'pledge-email-confirmed'; + $already_confirmed = get_post( $pledge_id )->$meta_key; + + if ( $already_confirmed ) { + /* + * If they refresh the page after confirming, they'd otherwise get an error because the token had been + * used, and might be confused and think that the address wasn't confirmed. + * + * This leaks the fact that the address is confirmed, because it will return true even if the token is + * invalid, but there aren't any security/privacy implications of that. + */ + return true; + } + + $email_confirmed = Email\is_valid_authentication_token( $pledge_id, $action, $unverified_token ); + + if ( $email_confirmed ) { + update_post_meta( $pledge_id, $meta_key, true ); + wp_update_post( array( 'ID' => $pledge_id, 'post_status' => 'publish' ) ); + } + + return $email_confirmed; +} + /** * Render the form(s) for managing existing pledges. * @@ -179,6 +219,12 @@ function process_form_manage() { __( 'A pledge already exists for this domain.', 'wporg' ) ); } + + // todo email any new contributors for confirmation + // notify any removed contributors? + // ask them to update their profiles? + // automatically update contributor profiles? + // anything else? } /** diff --git a/plugins/wporg-5ftf/includes/pledge-meta.php b/plugins/wporg-5ftf/includes/pledge-meta.php index 2d690ad..703938c 100755 --- a/plugins/wporg-5ftf/includes/pledge-meta.php +++ b/plugins/wporg-5ftf/includes/pledge-meta.php @@ -184,10 +184,19 @@ function render_meta_boxes( $pledge, $box ) { * @param WP_Post $pledge */ function save_pledge( $pledge_id, $pledge ) { - $action = filter_input( INPUT_GET, 'action' ); + $get_action = filter_input( INPUT_GET, 'action' ); + $post_action = filter_input( INPUT_POST, 'action' ); $ignored_actions = array( 'trash', 'untrash', 'restore' ); - if ( $action && in_array( $action, $ignored_actions, true ) ) { + /* + * This is only intended to run when the front end form and wp-admin forms are submitted, not when posts are + * programmatically updated. + */ + if ( 'Submit Pledge' !== $post_action && 'editpost' !== $get_action ) { + return; + } + + if ( $get_action && in_array( $get_action, $ignored_actions, true ) ) { return; } diff --git a/plugins/wporg-5ftf/includes/pledge.php b/plugins/wporg-5ftf/includes/pledge.php index 52aa0f8..9a05b8b 100755 --- a/plugins/wporg-5ftf/includes/pledge.php +++ b/plugins/wporg-5ftf/includes/pledge.php @@ -5,6 +5,7 @@ */ namespace WordPressDotOrg\FiveForTheFuture\Pledge; +use WordPressDotOrg\FiveForTheFuture\Email; use WordPressDotOrg\FiveForTheFuture; use WP_Error; @@ -127,5 +128,42 @@ function create_new_pledge( $name ) { 'post_status' => 'draft', ); - return wp_insert_post( $args, true ); + + $pledge_id = wp_insert_post( $args, true ); + // The pledge's meta data is saved at this point via `save_pledge_meta()`, which is a `save_post` callback. + + if ( ! is_wp_error( $pledge_id ) ) { + send_pledge_verification_email( $pledge_id, get_post()->ID ); + } + + return $pledge_id; +} + +/** + * Email pledge manager to confirm their email address. + * + * @param int $pledge_id The ID of the pledge. + * @param int $action_page_id The ID of the page that the user will be taken back to, in order to process their + * verification request. + * + * @return bool + */ +function send_pledge_verification_email( $pledge_id, $action_page_id ) { + $pledge = get_post( $pledge_id ); + + $message = + 'Thanks for committing to help keep WordPress sustainable! Please confirm this email address ' . + 'so that we can accept your pledge:' . "\n\n" . + Email\get_authentication_url( $pledge_id, 'confirm_pledge_email', $action_page_id ) + ; + + // todo include a notice that the link will expire in X hours, so they know what to expect + // need to make that value DRY across all emails with links + // should probably say that on the front end form success message as well, so they know to go check their email now instead of after lunch. + + return Email\send_email( + $pledge->{'5ftf_org-pledge-email'}, + 'Please confirm your email address', + $message + ); } diff --git a/plugins/wporg-5ftf/index.php b/plugins/wporg-5ftf/index.php index 69a239e..ea7a329 100755 --- a/plugins/wporg-5ftf/index.php +++ b/plugins/wporg-5ftf/index.php @@ -24,6 +24,7 @@ add_action( 'plugins_loaded', __NAMESPACE__ . '\load' ); */ function load() { require_once get_includes_path() . 'contributor.php'; + require_once get_includes_path() . 'email.php'; require_once get_includes_path() . 'pledge.php'; require_once get_includes_path() . 'pledge-meta.php'; require_once get_includes_path() . 'pledge-form.php'; diff --git a/plugins/wporg-5ftf/tests/test-email.php b/plugins/wporg-5ftf/tests/test-email.php new file mode 100644 index 0000000..8fe2df0 --- /dev/null +++ b/plugins/wporg-5ftf/tests/test-email.php @@ -0,0 +1,207 @@ + PLEDGE_POST_TYPE, + 'post_title' => 'Valid Pledge', + 'post_status' => 'publish', + ); + + $valid_action_page_params = array( + 'post_type' => 'page', + 'post_title' => 'For Organizers', + 'post_status' => 'publish', + ); + + $valid_pledge_id = self::factory()->post->create( $valid_pledge_params ); + + self::$valid_pledge = get_post( $valid_pledge_id ); + + + $valid_action_page_id = self::factory()->post->create( $valid_action_page_params ); + self::$valid_action_page = get_post( $valid_action_page_id ); + + self::$valid_action = 'confirm_pledge_email'; + // todo better example action to use, like contributor verifying participation? or is this one just as good? + // should probably use the manage one once that's implemented, b/c should test the `view` context and the `update` context. + // use this one for now, though. + + self::verify_before_class_fixtures(); + } + + /** + * Verify whether or not the fixtures were setup correctly. + * + * @return bool + */ + protected static function verify_before_class_fixtures() { + self::assertSame( 'object', gettype( self::$valid_action_page ) ); + self::assertSame( 'For Organizers', self::$valid_action_page->post_title ); + self::assertSame( 'object', gettype( self::$valid_pledge ) ); + self::assertSame( 'Valid Pledge', self::$valid_pledge->post_title ); + self::assertSame( PLEDGE_POST_TYPE, self::$valid_pledge->post_type ); + } + + /** + * Setup fixtures that are unique for each test. + */ + public function setUp() { + parent::setUp(); + + /* + * `get_authentication_url()` should create a valid token in the database. + * + * This must be called before every test, because the process of verifying a valid token will delete it. + */ + self::$valid_action_url = get_authentication_url( self::$valid_pledge->ID, self::$valid_action, self::$valid_action_page->ID ); + self::$valid_token = get_post_meta( self::$valid_pledge->ID, TOKEN_PREFIX . self::$valid_action, true ); + + // Verify that the fixtures are setup correctly. + $action_url_args = wp_parse_args( wp_parse_url( self::$valid_action_url, PHP_URL_QUERY ) ); + + $this->assertSame( 'array', gettype( self::$valid_token ) ); + $this->assertSame( $action_url_args['action'], self::$valid_action ); + $this->assertSame( $action_url_args['auth_token'], self::$valid_token['value'] ); + } + + /** + * @covers ::is_valid_authentication_token + */ + public function test_valid_token_accepted() { + $verified = is_valid_authentication_token( self::$valid_pledge->ID, self::$valid_action, self::$valid_token['value'] ); + + $this->assertTrue( $verified ); + + // todo test that `view` and `update` contexts work as well, when those are added + // maybe need to test some failures for that too + } + + /** + * @covers ::is_valid_authentication_token + * + * @dataProvider data_invalid_token_rejected + */ + public function test_invalid_token_rejected( $invalid_token ) { + /* + * It's expected that some of the values passed in won't have a `value` item, so fallback to the item + * itself in those cases, to avoid PHPUnit throwing an exception. + */ + $invalid_token_value = $invalid_token['value'] ?? $invalid_token; + $verified = is_valid_authentication_token( self::$valid_pledge->ID, self::$valid_action, $invalid_token_value ); + + $this->assertSame( false, $verified ); + } + + /** + * Test that invalid tokens are rejected. + * + * Note that data providers can't access fixtures. + * See https://phpunit.readthedocs.io/en/7.4/writing-tests-for-phpunit.html#data-providers. + * + * @covers ::is_valid_authentication_token + */ + public function data_invalid_token_rejected() { + return array( + 'non-existent-token' => array( false ), // Simulates `get_post_meta()` return value. + 'wrong-data-type' => array( 'this string is not an array' ), + 'wrong-array-items' => array( 'this' => "doesn't have `value` and `expiration` items" ), + + 'invalid-value' => array( + array( + 'value' => 'Valid tokens will never contain special characters like !@#$%^&*()', + 'expiration' => time() + HOUR_IN_SECONDS, + ) + ), + ); + } + + /** + * @covers ::is_valid_authentication_token + */ + public function test_expired_token_rejected() { + $expired_token = self::$valid_token; + $expired_token['expiration'] = time() - 1; + + update_post_meta( self::$valid_pledge->ID, TOKEN_PREFIX . self::$valid_action, $expired_token ); + + $verified = is_valid_authentication_token( self::$valid_pledge->ID, self::$valid_action, self::$valid_token['value'] ); + + $this->assertSame( false, $verified ); + } + + /** + * @covers ::is_valid_authentication_token + */ + public function test_used_token_rejected() { + // The token should be deleted once it's used/verified for the first time. + $first_verification = is_valid_authentication_token( self::$valid_pledge->ID, self::$valid_action, self::$valid_token['value'] ); + $second_verification = is_valid_authentication_token( self::$valid_pledge->ID, self::$valid_action, self::$valid_token['value'] ); + + $this->assertSame( true, $first_verification ); + $this->assertSame( false, $second_verification ); + } + + /** + * @covers ::is_valid_authentication_token + */ + public function test_valid_token_rejected_for_other_pages() { + $verified = is_valid_authentication_token( self::$valid_action_page->ID, self::$valid_action, self::$valid_token['value'] ); + + $this->assertSame( false, $verified ); + } + + /** + * @covers ::is_valid_authentication_token + */ + public function test_valid_token_rejected_for_other_actions() { + // Setup another valid token for the other action. + $other_valid_action = 'confirm_contributor_participation'; + // todo update this when the action for that step is created, so that they match and show that valid actions + $other_valid_action_url = get_authentication_url( self::$valid_pledge->ID, $other_valid_action, self::$valid_action_page->ID ); + + // Intentionally mismatch the token and action. + $verified = is_valid_authentication_token( self::$valid_pledge->ID, $other_valid_action, self::$valid_token['value'] ); + + $this->assertSame( false, $verified ); + } + + /** + * @covers ::is_valid_authentication_token + */ + public function test_valid_token_rejected_for_other_pledge() { + $other_valid_pledge_params = array( + 'post_type' => PLEDGE_POST_TYPE, + 'post_title' => 'Other Valid Pledge', + 'post_status' => 'publish', + ); + + $other_valid_pledge_id = self::factory()->post->create( $other_valid_pledge_params ); + $other_valid_pledge = get_post( $other_valid_pledge_id ); + + // Create a valid token for the other pledge. + get_authentication_url( $other_valid_pledge->ID, self::$valid_action, self::$valid_action_page->ID ); + + $other_valid_token = get_post_meta( $other_valid_pledge->ID, TOKEN_PREFIX . self::$valid_action, true ); + + // Intentionally mismatch the pledge and token. + $verified = is_valid_authentication_token( $other_valid_pledge_id, self::$valid_action, self::$valid_token['value'] ); + + $this->assertSame( 'Other Valid Pledge', $other_valid_pledge->post_title ); + $this->assertSame( 'array', gettype( $other_valid_token ) ); + $this->assertArrayHasKey( 'value', $other_valid_token ); + $this->assertNotSame( $other_valid_token['value'], self::$valid_token['value'] ); + $this->assertSame( false, $verified ); + } +} diff --git a/plugins/wporg-5ftf/views/form-pledge-confirm-email.php b/plugins/wporg-5ftf/views/form-pledge-confirm-email.php new file mode 100755 index 0000000..345f8c8 --- /dev/null +++ b/plugins/wporg-5ftf/views/form-pledge-confirm-email.php @@ -0,0 +1,42 @@ + + + + +
+

+ Thank you for confirming your address! We've emailed confirmation links to your contributors, and your pledge will show up in the directory once one of them confirms their participation. +

+
+ + + +
+

+ + Your confirmation link has expired, please obtain a new one: +

+ +

+ +

+
+ + diff --git a/plugins/wporg-5ftf/views/form-pledge-new.php b/plugins/wporg-5ftf/views/form-pledge-new.php index 83b6d84..1075c04 100755 --- a/plugins/wporg-5ftf/views/form-pledge-new.php +++ b/plugins/wporg-5ftf/views/form-pledge-new.php @@ -30,7 +30,7 @@ use function WordPressDotOrg\FiveForTheFuture\get_views_path;
-

+

Post a job listing on jobs.wordpress.net.', 'wporg' ) + // todo ask mel about moving this outside the `notice-success`, since it's not really part of the success notification, and distracts from it. + // many users have notification fatigue and no longer trust them or pay attention to them, because they're so often misused for non-critical information, + // and the jobs thing is more of an "ad" in this context than something directly related to the process the user wants to complete ); ?>