Remove the handle_error() method from authentication servers.

This creates a new AuthenticationException exception to transport authentication errors, including headers and status code, from the authenticate() method instead of using a handle_error() method. If the current request is a REST request, the Authentication provider will convert the exception to an error instead of allowing the exception to bubble up.
cedaro · Mar 27, 2019 · 870d8ab · 870d8ab
1 parent c0822c0
commit 870d8ab
Show file tree

Hide file tree

Showing 7 changed files with 172 additions and 123 deletions.
diff --git a/src/Authentication/ApiKey/Server.php b/src/Authentication/ApiKey/Server.php
@@ -12,7 +12,7 @@
 namespace SatisPress\Authentication\ApiKey;
 
 use SatisPress\Authentication\Server as ServerInterface;
-use SatisPress\Exception\HttpException;
+use SatisPress\Exception\AuthenticationException;
 use SatisPress\HTTP\Request;
 use WP_Error;
 use WP_Http as HTTP;
@@ -74,51 +74,36 @@ public function check_scheme( Request $request ): bool {
 	 * @since 0.3.0
 	 *
 	 * @param Request $request Request instance.
-	 * @throws HttpException If authentication fails.
+	 * @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 ) {
-			throw HttpException::forMissingAuthorizationHeader();
+			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 ) {
-			throw HttpException::forInvalidCredentials();
+			throw AuthenticationException::forInvalidCredentials();
 		}
 
 		$user = $api_key->get_user();
 
 		// Bail if the user couldn't be determined.
 		if ( ! $this->validate_user( $user ) ) {
-			throw HttpException::forInvalidCredentials();
+			throw AuthenticationException::forInvalidCredentials();
 		}
 
 		$this->maybe_update_last_used_time( $api_key );
 
 		return $user->ID;
 	}
 
-	/**
-	 * Handle errors encountered when authenticating.
-	 *
-	 * @since 0.3.0
-	 *
-	 * @param HttpException $e HTTP exception.
-	 */
-	public function handle_error( HttpException $e ): WP_Error {
-		if ( HTTP::UNAUTHORIZED === $e->getStatusCode() ) {
-			header( 'WWW-Authenticate: Basic realm="SatisPress"' );
-		}
-
-		wp_die( wp_kses_data( $e->getMessage() ), absint( $e->getStatusCode() ) );
-	}
-
 	/**
 	 * Update the last used time if it's been more than a minute.
 	 *

diff --git a/src/Authentication/Server.php b/src/Authentication/Server.php
@@ -11,7 +11,7 @@
 
 namespace SatisPress\Authentication;
 
-use SatisPress\Exception\HttpException;
+use SatisPress\Exception\AuthenticationException;
 use Satispress\HTTP\Request;
 use WP_Error;
 
@@ -37,18 +37,8 @@ public function check_scheme( Request $request ): bool;
 	 * @since 0.3.0
 	 *
 	 * @param Request $request Request instance.
-	 * @throws HttpException If authentications fails.
+	 * @throws AuthenticationException If authentications fails.
 	 * @return int A user ID.
 	 */
 	public function authenticate( Request $request ): int;
-
-	/**
-	 * Handle errors encountered when authenticating.
-	 *
-	 * @since 0.4.0
-	 *
-	 * @param HttpException $e HTTP exception.
-	 * @return WP_Error
-	 */
-	public function handle_error( HttpException $e ): WP_Error;
 }
diff --git a/src/Authentication/UnauthorizedServer.php b/src/Authentication/UnauthorizedServer.php
@@ -13,7 +13,7 @@
 
 namespace SatisPress\Authentication;
 
-use SatisPress\Exception\HttpException;
+use SatisPress\Exception\AuthenticationException;
 use SatisPress\HTTP\Request;
 use WP_Error;
 use WP_Http as HTTP;
@@ -42,27 +42,9 @@ public function check_scheme( Request $request ): bool {
 	 * @since 0.3.0
 	 *
 	 * @param Request $request Request instance.
-	 * @throws HttpException If the user has not been authenticated at this point.
+	 * @throws AuthenticationException If the user has not been authenticated at this point.
 	 */
 	public function authenticate( Request $request ): int {
-		throw HttpException::forAuthenticationRequired();
-	}
-
-	/**
-	 * Display an error message when authentication fails.
-	 *
-	 * @since 0.3.0
-	 *
-	 * @param HttpException $e HTTP exception.
-	 */
-	public function handle_error( HttpException $e ): WP_Error {
-		header( 'WWW-Authenticate: Basic realm="SatisPress"' );
-
-		wp_die(
-			wp_kses_data( $e->getMessage() ),
-			esc_html__( 'Authentication Required', 'satispress' ),
-			// phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped
-			[ 'response' => HTTP::UNAUTHORIZED ]
-		);
+		throw AuthenticationException::forAuthenticationRequired();
 	}
 }
diff --git a/src/Exception/AuthenticationException.php b/src/Exception/AuthenticationException.php
@@ -0,0 +1,136 @@
+<?php
+/**
+ * Authentication exception.
+ *
+ * @package SatisPress
+ * @license GPL-2.0-or-later
+ * @since 0.4.0
+ */
+
+declare ( strict_types = 1 );
+
+namespace SatisPress\Exception;
+
+use SatisPress\Package;
+use SatisPress\Release;
+use Throwable;
+use WP_Http as HTTP;
+
+/**
+ * Authentication exception class.
+ *
+ * @since 0.4.0
+ */
+class AuthenticationException extends HttpException {
+	/**
+	 * Error code.
+	 *
+	 * @var string
+	 */
+	protected $code = '';
+
+	/**
+	 * Response headers.
+	 *
+	 * @var array
+	 */
+	protected $headers;
+
+	/**
+	 * Constructor.
+	 *
+	 * @since 0.4.0
+	 *
+	 * @param string    $code        Exception code.
+	 * @param string    $message     Message.
+	 * @param int       $status_code Optional. HTTP status code. Defaults to 500.
+	 * @param array     $headers     Optional. Response headers.
+	 * @param Throwable $previous    Optional. Previous exception.
+	 */
+	public function __construct(
+		string $code,
+		string $message,
+		int $status_code = HTTP::INTERNAL_SERVER_ERROR,
+		array $headers = [],
+		Throwable $previous = null
+	) {
+		$this->code    = $code;
+		$this->headers = $headers;
+
+		parent::__construct( $message, $status_code, 0, $previous );
+	}
+
+	/**
+	 * Create an exception for requests that require authentication.
+	 *
+	 * @since 0.4.0.
+	 *
+	 * @param array     $headers  Response headers.
+	 * @param string    $code     Optional. The Exception code.
+	 * @param Throwable $previous Optional. The previous throwable used for the exception chaining.
+	 * @return HTTPException
+	 */
+	public static function forAuthenticationRequired(
+		array $headers = [],
+		string $code = 'invalid_request',
+		Throwable $previous = null
+	): HttpException {
+		$headers = $headers ?: [ 'WWW-Authenticate' => 'Basic realm="SatisPress"' ];
+		$message = 'Authentication is required for this resource.';
+
+		return new static( $code, $message, HTTP::UNAUTHORIZED, $headers, $previous );
+	}
+
+	/**
+	 * Create an exception for invalid credentials.
+	 *
+	 * @since 0.4.0.
+	 *
+	 * @param array     $headers  Response headers.
+	 * @param string    $code     Optional. The Exception code.
+	 * @param Throwable $previous Optional. The previous throwable used for the exception chaining.
+	 * @return HTTPException
+	 */
+	public static function forInvalidCredentials(
+		array $headers = [],
+		string $code = 'invalid_credentials',
+		Throwable $previous = null
+	): HttpException {
+		$headers = $headers ?: [ 'WWW-Authenticate' => 'Basic realm="SatisPress"' ];
+		$message = 'Invalid credentials.';
+
+		return new static( $code, $message, HTTP::UNAUTHORIZED, $headers, $previous );
+	}
+
+	/**
+	 * Create an exception for a missing authorization header.
+	 *
+	 * @since 0.4.0.
+	 *
+	 * @param array     $headers  Response headers.
+	 * @param string    $code     Optional. The Exception code.
+	 * @param Throwable $previous Optional. The previous throwable used for the exception chaining.
+	 * @return HTTPException
+	 */
+	public static function forMissingAuthorizationHeader(
+		array $headers = [],
+		string $code = 'invalid_credentials',
+		Throwable $previous = null
+	): HttpException {
+		$headers = $headers ?: [ 'WWW-Authenticate' => 'Basic realm="SatisPress"' ];
+		$message = 'Missing authorization header.';
+
+		return new static( $code, $message, HTTP::UNAUTHORIZED, $headers, $previous );
+	}
+
+	/**
+	 * Retrieve the response headers.
+	 *
+	 * @since 0.4.0
+	 *
+	 * @return array Map of header name to header value.
+	 */
+	public function getHeaders(): array {
+		return $this->headers;
+	}
+}
diff --git a/src/Exception/HttpException.php b/src/Exception/HttpException.php
@@ -51,60 +51,6 @@ public function __construct(
 		parent::__construct( $message, $code, $previous );
 	}
 
-	/**
-	 * Create an exception for requests that require authentication.
-	 *
-	 * @since 0.4.0.
-	 *
-	 * @param int       $code     Optional. The Exception code.
-	 * @param Throwable $previous Optional. The previous throwable used for the exception chaining.
-	 * @return HTTPException
-	 */
-	public static function forAuthenticationRequired(
-		int $code = 0,
-		Throwable $previous = null
-	): HttpException {
-		$message = 'Authentication is required for this resource.';
-
-		return new static( $message, HTTP::UNAUTHORIZED, $code, $previous );
-	}
-
-	/**
-	 * Create an exception for invalid credentials.
-	 *
-	 * @since 0.4.0.
-	 *
-	 * @param int       $code     Optional. The Exception code.
-	 * @param Throwable $previous Optional. The previous throwable used for the exception chaining.
-	 * @return HTTPException
-	 */
-	public static function forInvalidCredentials(
-		int $code = 0,
-		Throwable $previous = null
-	): HttpException {
-		$message = 'Invalid credentials.';
-
-		return new static( $message, HTTP::UNAUTHORIZED, $code, $previous );
-	}
-
-	/**
-	 * Create an exception for a missing authorization header.
-	 *
-	 * @since 0.4.0.
-	 *
-	 * @param int       $code     Optional. The Exception code.
-	 * @param Throwable $previous Optional. The previous throwable used for the exception chaining.
-	 * @return HTTPException
-	 */
-	public static function forMissingAuthorizationHeader(
-		int $code = 0,
-		Throwable $previous = null
-	): HttpException {
-		$message = 'Missing authorization header.';
-
-		return new static( $message, HTTP::UNAUTHORIZED, $code, $previous );
-	}
-
 	/**
 	 * Create an exception for a forbidden resource request.
 	 *

diff --git a/src/HTTP/Response.php b/src/HTTP/Response.php
@@ -11,6 +11,7 @@
 
 namespace SatisPress\HTTP;
 
+use SatisPress\Exception\AuthenticationException;
 use SatisPress\Exception\HttpException;
 use SatisPress\HTTP\ResponseBody\ErrorBody;
 use SatisPress\HTTP\ResponseBody\FileBody;
@@ -159,6 +160,7 @@ public static function for_file( string $filename ): Response {
 	 */
 	public static function from_exception( \Exception $e ): Response {
 		$status_code = 500;
+		$headers     = [];
 
 		if ( $e instanceof HttpException ) {
 			$status_code = $e->getStatusCode();
@@ -171,9 +173,15 @@ public static function from_exception( \Exception $e ): Response {
 			$message = 'Sorry, you cannot view this resource.';
 		}
 
+		if ( $e instanceof AuthenticationException ) {
+			$headers = $e->getHeaders();
+			$message = $e->getMessage();
+		}
+
 		return new static(
 			new ErrorBody( $message, $status_code ),
-			$status_code
+			$status_code,
+			$headers
 		);
 	}