From e0e8fae44bdbf0734e30233cd491f9f526beb3b3 Mon Sep 17 00:00:00 2001
From: Kelly Dwan <ryelle@users.noreply.github.com>
Date: Fri, 15 Nov 2019 12:47:20 -0500
Subject: [PATCH] Split Auth functionality out to new file

---
 .../wporg-5ftf/includes/authentication.php    | 151 ++++++++++++++++++
 plugins/wporg-5ftf/includes/email.php         | 149 +----------------
 plugins/wporg-5ftf/includes/pledge-form.php   |   8 +-
 plugins/wporg-5ftf/includes/pledge.php        |   4 +-
 plugins/wporg-5ftf/index.php                  |   1 +
 .../tests/{test-email.php => test-auth.php}   |   6 +-
 6 files changed, 165 insertions(+), 154 deletions(-)
 create mode 100644 plugins/wporg-5ftf/includes/authentication.php
 rename plugins/wporg-5ftf/tests/{test-email.php => test-auth.php} (97%)

diff --git a/plugins/wporg-5ftf/includes/authentication.php b/plugins/wporg-5ftf/includes/authentication.php
new file mode 100644
index 0000000..fd3227d
--- /dev/null
+++ b/plugins/wporg-5ftf/includes/authentication.php
@@ -0,0 +1,151 @@
+<?php
+/**
+ * Helper functions creating & storing authentication tokens.
+ *
+ * We don't want pledges connected to individual w.org accounts, because that would encourage people to create
+ * "company" accounts instead of having their contributions show up as real human beings; or because that
+ * individual will likely eventually leave the company, and "ownership" of the pledge would be orphaned; or
+ * because we'd have to tie multiple accounts to the pledge (and all the extra time/UX costs associated with that),
+ * and would still have problems with orphaned ownership, etc.
+ *
+ * So instead, we just ask companies to create pledges using a group email (e.g., support@wordcamp.org), and
+ * we email them time-restricted, once-time-use auth tokens when they want to "log in".
+ *
+ * WP "nonces" aren't ideal for this purpose from a security perspective, because they're less secure. They're
+ * reusable, last up to 24 hours, and have a much smaller search space in brute force attacks. They're only
+ * intended to prevent CSRF, and should not be used for authentication or authorization.
+ *
+ * They also create an inconsistent UX, because a nonce could be valid for 24 hours, or for 1 second, due to their
+ * stateless nature -- see `wp_nonce_tick()`. That would lead to some situations where a nonce had already expired
+ * by the time the contributor opened the email and clicked on the link.
+ *
+ * So instead, true stateful CSPRN authentication tokens are generated; see `get_authentication_url()` and
+ * `is_valid_authentication_token()` for details.
+ *
+ * For additional background:
+ * - https://stackoverflow.com/a/35715087/450127 (which is better security advice than ircmarxell's 2010 answer).
+ */
+
+namespace WordPressDotOrg\FiveForTheFuture\Auth;
+use WordPressDotOrg\FiveForTheFuture;
+
+defined( 'WPINC' ) || die();
+
+const TOKEN_PREFIX = '5ftf_auth_token_';
+
+// Longer than `get_password_reset_key()` just to be safe.
+// See https://core.trac.wordpress.org/ticket/43546#comment:34.
+const TOKEN_LENGTH = 32;
+
+add_action( 'wp_head', __NAMESPACE__ . '\prevent_caching_auth_tokens', 99 );
+
+/**
+ * Prevent caching mechanisms from caching authentication tokens.
+ *
+ * Search engines would often be too slow to index tokens before they expire, but other mechanisms like Varnish,
+ * etc could create situations where they're leaked to others.
+ */
+function prevent_caching_auth_tokens() {
+	// phpcs:ignore WordPress.Security.NonceVerification.Missing -- Nonce not required, not processing form data.
+	if ( isset( $_GET['auth_token'] ) || isset( $_POST['auth_token'] ) ) {
+		nocache_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
+ *                               confirmation request.
+ * @param bool   $use_once       Whether or not the token should be deleted after the first use. Only pass `false`
+ *                               when the action requires several steps in a flow, rather than a single step. For
+ *                               instance, be able to 1) view a private pledge; 2) make changes and save them; and
+ *                               3) reload the private pledge with the new changes displayed.
+ *
+ * @return string
+ */
+function get_authentication_url( $pledge_id, $action, $action_page_id, $use_once = true ) {
+	$auth_token = array(
+		// This will create a CSPRN and is similar to how `get_password_reset_key()` and
+		// `generate_recovery_mode_token()` work.
+		'value'      => wp_generate_password( TOKEN_LENGTH, false ),
+		// todo Ideally should encrypt at rest, see https://core.trac.wordpress.org/ticket/24783.
+		'expiration' => time() + ( 2 * HOUR_IN_SECONDS ),
+		'use_once'   => $use_once,
+	);
+
+	/*
+	 * 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 cannot be reused[1], 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.
+ *
+ * [1] In some cases, tokens can be reused, when that is explicitly required for their flow. For an example, see
+ * the documentation in `get_authentication_url()`.
+ *
+ * @param int    $pledge_id
+ * @param string $action
+ * @param string $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 ( ! is_string( $unverified_token ) || TOKEN_LENGTH !== strlen( $unverified_token ) ) {
+		return false;
+	}
+
+	if ( $valid_token && $valid_token['expiration'] > time() && hash_equals( $valid_token['value'], $unverified_token ) ) {
+		$verified = true;
+
+		// Tokens should not be reusable -- to increase security -- unless explicitly required to fulfill their purpose.
+		if ( false !== $valid_token['use_once'] ) {
+			delete_post_meta( $pledge_id, TOKEN_PREFIX . $action );
+		}
+	}
+
+	return $verified;
+}
diff --git a/plugins/wporg-5ftf/includes/email.php b/plugins/wporg-5ftf/includes/email.php
index 9a4c1d9..9874017 100644
--- a/plugins/wporg-5ftf/includes/email.php
+++ b/plugins/wporg-5ftf/includes/email.php
@@ -1,58 +1,14 @@
 <?php
-
 /**
- * Helper functions for sending emails, including authentication tokens.
- *
- * We don't want pledges connected to individual w.org accounts, because that would encourage people to create
- * "company" accounts instead of having their contributions show up as real human beings; or because that
- * individual will likely eventually leave the company, and "ownership" of the pledge would be orphaned; or
- * because we'd have to tie multiple accounts to the pledge (and all the extra time/UX costs associated with that),
- * and would still have problems with orphaned ownership, etc.
- *
- * So instead, we just ask companies to create pledges using a group email (e.g., support@wordcamp.org), and
- * we email them time-restricted, once-time-use auth tokens when they want to "log in".
- *
- * WP "nonces" aren't ideal for this purpose from a security perspective, because they're less secure. They're
- * reusable, last up to 24 hours, and have a much smaller search space in brute force attacks. They're only
- * intended to prevent CSRF, and should not be used for authentication or authorization.
- *
- * They also create an inconsistent UX, because a nonce could be valid for 24 hours, or for 1 second, due to their
- * stateless nature -- see `wp_nonce_tick()`. That would lead to some situations where a nonce had already expired
- * by the time the contributor opened the email and clicked on the link.
- *
- * So instead, true stateful CSPRN authentication tokens are generated; see `get_authentication_url()` and
- * `is_valid_authentication_token()` for details.
- *
- * For additional background:
- * - https://stackoverflow.com/a/35715087/450127 (which is better security advice than ircmarxell's 2010 answer).
+ * Helper functions for sending emails.
  */
 
 namespace WordPressDotOrg\FiveForTheFuture\Email;
-use WordPressDotOrg\FiveForTheFuture;
+
+use const WordPressDotOrg\FiveForTheFuture\PREFIX;
 
 defined( 'WPINC' ) || die();
 
-const TOKEN_PREFIX = '5ftf_auth_token_';
-
-// Longer than `get_password_reset_key()` just to be safe.
-// See https://core.trac.wordpress.org/ticket/43546#comment:34.
-const TOKEN_LENGTH = 32;
-
-add_action( 'wp_head', __NAMESPACE__ . '\prevent_caching_auth_tokens', 99 );
-
-/**
- * Prevent caching mechanisms from caching authentication tokens.
- *
- * Search engines would often be too slow to index tokens before they expire, but other mechanisms like Varnish,
- * etc could create situations where they're leaked to others.
- */
-function prevent_caching_auth_tokens() {
-	// phpcs:ignore WordPress.Security.NonceVerification.Missing -- Nonce not required, not processing form data.
-	if ( isset( $_GET['auth_token'] ) || isset( $_POST['auth_token'] ) ) {
-		nocache_headers();
-	}
-}
-
 /**
  * Wrap `wp_mail()` with shared functionality.
  *
@@ -81,105 +37,8 @@ function send_email( $to, $subject, $message, $pledge_id ) {
 	 * @param bool   $result
 	 * @param int    $pledge_id
 	 */
-	do_action( FiveForTheFuture\PREFIX . '_email_result', $to, $subject, $message, $headers, $result, $pledge_id );
+	do_action( PREFIX . '_email_result', $to, $subject, $message, $headers, $result, $pledge_id );
 
 	return $result;
 }
 
-/**
- * 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
- *                               confirmation request.
- * @param bool   $use_once       Whether or not the token should be deleted after the first use. Only pass `false`
- *                               when the action requires several steps in a flow, rather than a single step. For
- *                               instance, be able to 1) view a private pledge; 2) make changes and save them; and
- *                               3) reload the private pledge with the new changes displayed.
- *
- * @return string
- */
-function get_authentication_url( $pledge_id, $action, $action_page_id, $use_once = true ) {
-	$auth_token = array(
-		// This will create a CSPRN and is similar to how `get_password_reset_key()` and
-		// `generate_recovery_mode_token()` work.
-		'value'      => wp_generate_password( TOKEN_LENGTH, false ),
-		// todo Ideally should encrypt at rest, see https://core.trac.wordpress.org/ticket/24783.
-		'expiration' => time() + ( 2 * HOUR_IN_SECONDS ),
-		'use_once'   => $use_once,
-	);
-
-	/*
-	 * 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 cannot be reused[1], 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.
- *
- * [1] In some cases, tokens can be reused, when that is explicitly required for their flow. For an example, see
- * the documentation in `get_authentication_url()`.
- *
- * @param int    $pledge_id
- * @param string $action
- * @param string $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 ( ! is_string( $unverified_token ) || TOKEN_LENGTH !== strlen( $unverified_token ) ) {
-		return false;
-	}
-
-	if ( $valid_token && $valid_token['expiration'] > time() && hash_equals( $valid_token['value'], $unverified_token ) ) {
-		$verified = true;
-
-		// Tokens should not be reusable -- to increase security -- unless explicitly required to fulfill their purpose.
-		if ( false !== $valid_token['use_once'] ) {
-			delete_post_meta( $pledge_id, TOKEN_PREFIX . $action );
-		}
-	}
-
-	return $verified;
-}
diff --git a/plugins/wporg-5ftf/includes/pledge-form.php b/plugins/wporg-5ftf/includes/pledge-form.php
index fc6422e..337efec 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, Email };
+use WordPressDotOrg\FiveForTheFuture\{ Auth, Contributor, Email, Pledge, PledgeMeta };
 use WP_Error, WP_User;
 
 defined( 'WPINC' ) || die();
@@ -130,7 +130,7 @@ function process_pledge_confirmation_email( $pledge_id, $action, $unverified_tok
 		return true;
 	}
 
-	$email_confirmed = Email\is_valid_authentication_token( $pledge_id, $action, $unverified_token );
+	$email_confirmed = Auth\is_valid_authentication_token( $pledge_id, $action, $unverified_token );
 
 	if ( $email_confirmed ) {
 		update_post_meta( $pledge_id, $meta_key, true );
@@ -170,7 +170,7 @@ function send_contributor_confirmation_emails( $pledge_id, $contributor_id = nul
 		$name = $user->first_name ? $user->first_name : '@' . $user->user_nicename;
 
 		/*
-		 * This uses w.org login accounts instead of `Email\get_authentication_url()`, because the reasons for using
+		 * This uses w.org login accounts instead of `Auth\get_authentication_url()`, because the reasons for using
 		 * tokens for pledges don't apply to contributors, accounts are more secure, and they provide a better UX
 		 * because there's no expiration.
 		 */
@@ -300,7 +300,7 @@ function send_manage_pledge_link( $pledge_id ) {
 	$message =
 		'Howdy, please open this link to update your pledge:' . "\n\n" .
 
-		Email\get_authentication_url(
+		Auth\get_authentication_url(
 			$pledge_id,
 			'manage_pledge',
 			get_page_by_path( 'manage-pledge' )->ID,
diff --git a/plugins/wporg-5ftf/includes/pledge.php b/plugins/wporg-5ftf/includes/pledge.php
index 750a33e..40c8a7d 100755
--- a/plugins/wporg-5ftf/includes/pledge.php
+++ b/plugins/wporg-5ftf/includes/pledge.php
@@ -7,7 +7,7 @@
 namespace WordPressDotOrg\FiveForTheFuture\Pledge;
 
 use WordPressDotOrg\FiveForTheFuture;
-use WordPressDotOrg\FiveForTheFuture\{ Contributor, Email };
+use WordPressDotOrg\FiveForTheFuture\{ Auth, Contributor, Email };
 use WP_Error, WP_Query;
 
 use const WordPressDotOrg\FiveForTheFuture\PledgeMeta\META_PREFIX;
@@ -209,7 +209,7 @@ function send_pledge_confirmation_email( $pledge_id, $action_page_id ) {
 
 	$message = sprintf(
 		"Thanks for pledging your organization's time to contribute to the WordPress open source project! Please confirm this email address in order to publish your pledge:\n\n%s",
-		Email\get_authentication_url( $pledge_id, 'confirm_pledge_email', $action_page_id )
+		Auth\get_authentication_url( $pledge_id, 'confirm_pledge_email', $action_page_id )
 	);
 
 	return Email\send_email(
diff --git a/plugins/wporg-5ftf/index.php b/plugins/wporg-5ftf/index.php
index a69b2da..070ed41 100755
--- a/plugins/wporg-5ftf/index.php
+++ b/plugins/wporg-5ftf/index.php
@@ -25,6 +25,7 @@ add_action( 'plugins_loaded', __NAMESPACE__ . '\load' );
 function load() {
 	$running_unit_tests = isset( $_SERVER['_'] ) && false !== strpos( $_SERVER['_'], 'phpunit' );
 
+	require_once get_includes_path() . 'authentication.php';
 	require_once get_includes_path() . 'contributor.php';
 	require_once get_includes_path() . 'email.php';
 	require_once get_includes_path() . 'pledge.php';
diff --git a/plugins/wporg-5ftf/tests/test-email.php b/plugins/wporg-5ftf/tests/test-auth.php
similarity index 97%
rename from plugins/wporg-5ftf/tests/test-email.php
rename to plugins/wporg-5ftf/tests/test-auth.php
index b64a265..5d99dec 100644
--- a/plugins/wporg-5ftf/tests/test-email.php
+++ b/plugins/wporg-5ftf/tests/test-auth.php
@@ -1,12 +1,12 @@
 <?php
 
-use function WordPressDotOrg\FiveForTheFuture\Email\{ get_authentication_url, is_valid_authentication_token };
-use const WordPressDotOrg\FiveForTheFuture\Email\{ TOKEN_PREFIX };
+use function WordPressDotOrg\FiveForTheFuture\Auth\{ get_authentication_url, is_valid_authentication_token };
+use const WordPressDotOrg\FiveForTheFuture\Auth\{ TOKEN_PREFIX };
 use const WordPressDotOrg\FiveForTheFuture\Pledge\CPT_ID as PLEDGE_POST_TYPE;
 
 defined( 'WPINC' ) || die();
 
-class Test_Email extends WP_UnitTestCase {
+class Test_Auth extends WP_UnitTestCase {
 	// phpcs:ignore PSR2.Classes.PropertyDeclaration.Multiple
 	protected static $pledge, $action, $page, $action_url, $token;