Split Auth functionality out to new file

This commit is contained in:
Kelly Dwan 2019-11-15 12:47:20 -05:00
parent 03949905c0
commit e0e8fae44b
No known key found for this signature in database
GPG key ID: 8BA5575F3D11575D
6 changed files with 165 additions and 154 deletions

View file

@ -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;
}

View file

@ -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;
}

View file

@ -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,

View file

@ -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(

View file

@ -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';

View file

@ -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;