Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor authentication to prevent conflicts with plugins that determine the current user too early. #97

Merged
merged 3 commits into from
Mar 27, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion satispress.php
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
->set_url( plugin_dir_url( __FILE__ ) )
->set_container( $satispress_container )
->register_hooks( $satispress_container->get( 'hooks.activation' ) )
->register_hooks( $satispress_container->get( 'hooks.deactivation' ) );
->register_hooks( $satispress_container->get( 'hooks.deactivation' ) )
->register_hooks( $satispress_container->get( 'hooks.authentication' ) );

add_action( 'plugins_loaded', [ $satispress, 'compose' ], 5 );
117 changes: 0 additions & 117 deletions src/Authentication/AbstractServer.php

This file was deleted.

75 changes: 29 additions & 46 deletions src/Authentication/ApiKey/Server.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@

namespace SatisPress\Authentication\ApiKey;

use SatisPress\Authentication\AbstractServer;
use SatisPress\Authentication\Server as ServerInterface;
use SatisPress\Exception\AuthenticationException;
use SatisPress\HTTP\Request;
use SatisPress\WP_Error\HttpError;
use WP_Error;
use WP_Http as HTTP;

Expand All @@ -22,7 +22,7 @@
*
* @since 0.3.0
*/
class Server extends AbstractServer {
class Server implements ServerInterface {
/**
* API Key repository.
*
Expand All @@ -35,92 +35,75 @@ class Server extends AbstractServer {
*
* @since 0.3.0
*
* @param Request $request Request instance.
* @param ApiKeyRepository $repository API Key repository.
*/
public function __construct( Request $request, ApiKeyRepository $repository ) {
parent::__construct( $request );
public function __construct( ApiKeyRepository $repository ) {
$this->repository = $repository;
}

/**
* Handle authentication.
* Check if the server should handle the current request.
*
* @since 0.3.0
* @since 0.4.0
*
* @param int|bool $user_id Current user ID or false if unknown.
* @return int|bool A user on success, or false on failure.
* @param Request $request Request instance.
* @return bool
*/
public function authenticate( $user_id ) {
if ( ! empty( $user_id ) || ! $this->should_attempt ) {
return $user_id;
}

$header = $this->request->get_header( 'authorization' );
public function check_scheme( Request $request ): bool {
$header = $request->get_header( 'authorization' );

// Bail if the authorization header doesn't exist.
if ( null === $header || 0 !== stripos( $header, 'basic ' ) ) {
return $user_id;
return false;
}

// The password field isn't used for API Key authentication.
$realm = $this->request->get_header( 'PHP_AUTH_PW' );
$realm = $request->get_header( 'PHP_AUTH_PW' );

// Bail if this isn't a SatisPress authentication request.
if ( 'satispress' !== $realm ) {
return $user_id;
return false;
}

$this->should_attempt = false;
return true;
}

$api_key_id = $this->request->get_header( 'PHP_AUTH_USER' );
/**
* Handle authentication.
*
* @since 0.3.0
*
* @param Request $request Request instance.
* @throws AuthenticationException If authentication fails.
* @return int A user ID.
*/
public function authenticate( Request $request ): int {
$api_key_id = $request->get_header( 'PHP_AUTH_USER' );

// Bail if an API Key wasn't provided.
if ( null === $api_key_id ) {
$this->auth_status = HttpError::missingAuthorizationHeader();
return false;
throw AuthenticationException::forMissingAuthorizationHeader();
}

$api_key = $this->repository->find_by_token( $api_key_id );

// Bail if the API Key doesn't exist.
if ( null === $api_key ) {
$this->auth_status = HttpError::invalidCredentials();
return false;
throw AuthenticationException::forInvalidCredentials();
}

$user = $api_key->get_user();

// Bail if the user couldn't be determined.
if ( ! $this->validate_user( $user ) ) {
$this->auth_status = HttpError::invalidCredentials();
return false;
throw AuthenticationException::forInvalidCredentials();
}

$this->maybe_update_last_used_time( $api_key );
$this->auth_status = true;

return $user->ID;
}

/**
* Handle errors encountered when authenticating.
*
* @since 0.3.0
*
* @param WP_Error $error Error object.
*/
protected function handle_error( WP_Error $error ) {
$error_data = $error->get_error_data();

if ( ! empty( $error_data['status'] ) && HTTP::UNAUTHORIZED === $error_data['status'] ) {
header( 'WWW-Authenticate: Basic realm="SatisPress"' );
}

$status_code = empty( $error_data['status'] ) ? HTTP::INTERNAL_SERVER_ERROR : $error_data['status'];
wp_die( wp_kses_data( $error->get_error_message() ), absint( $status_code ) );
}

/**
* Update the last used time if it's been more than a minute.
*
Expand Down
21 changes: 12 additions & 9 deletions src/Authentication/Server.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

namespace SatisPress\Authentication;

use SatisPress\Exception\AuthenticationException;
use Satispress\HTTP\Request;
use WP_Error;

/**
Expand All @@ -20,22 +22,23 @@
*/
interface Server {
/**
* Handle authentication.
* Check if the server should handle the current request.
*
* @since 0.3.0
* @since 0.4.0
*
* @param int|bool $user_id Current user ID or false if unknown.
* @return int|bool A user on success, or false on failure.
* @param Request $request Request instance.
* @return bool
*/
public function authenticate( $user_id );
public function check_scheme( Request $request ): bool;

/**
* Report authentication errors.
* Handle authentication.
*
* @since 0.3.0
*
* @param WP_Error|mixed $value Error from another authentication handler, null if we should handle it, or another value if not.
* @return WP_Error|bool|null
* @param Request $request Request instance.
* @throws AuthenticationException If authentications fails.
* @return int A user ID.
*/
public function get_authentication_errors( $value );
public function authenticate( Request $request ): int;
}
40 changes: 14 additions & 26 deletions src/Authentication/UnauthorizedServer.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@

namespace SatisPress\Authentication;

use SatisPress\WP_Error\HttpError;
use SatisPress\Exception\AuthenticationException;
use SatisPress\HTTP\Request;
use WP_Error;
use WP_Http as HTTP;

Expand All @@ -22,41 +23,28 @@
*
* @since 0.3.0
*/
class UnauthorizedServer extends AbstractServer {
class UnauthorizedServer implements Server {
/**
* Handle authentication.
* Check if the server should handle the current request.
*
* @since 0.3.0
* @since 0.4.0
*
* @param int|bool $user_id Current user ID or false if unknown.
* @return int|bool A user ID on success, or false on failure.
* @param Request $request Request instance.
* @return bool
*/
public function authenticate( $user_id ) {
if ( ! empty( $user_id ) || ! $this->should_attempt ) {
return $user_id;
}

$this->should_attempt = false;
$this->auth_status = HttpError::authenticationRequired();

return false;
public function check_scheme( Request $request ): bool {
return true;
}

/**
* Display an error message when authentication fails.
* Handle authentication.
*
* @since 0.3.0
*
* @param WP_Error $error Error object.
* @param Request $request Request instance.
* @throws AuthenticationException If the user has not been authenticated at this point.
*/
protected function handle_error( WP_Error $error ) {
header( 'WWW-Authenticate: Basic realm="SatisPress"' );

wp_die(
wp_kses_data( $error->get_error_message() ),
esc_html__( 'Authentication Required', 'satispress' ),
// phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped
[ 'response' => HTTP::UNAUTHORIZED ]
);
public function authenticate( Request $request ): int {
throw AuthenticationException::forAuthenticationRequired();
}
}
Loading