Fix FedCM authentication, scope consent, and code-binding issues

includes/class-indieauth.php

@@ -209,6 +209,8 @@ private function register_rest_routes() {
 
 		// FedCM Controller hooks.
 		\add_action( 'rest_api_init', array( $this->fedcm, 'register_routes' ) );
+		\add_filter( 'rest_authentication_errors', array( $this->fedcm, 'rest_authentication_errors' ) );
+		\add_filter( 'rest_pre_dispatch', array( $this->fedcm, 'rest_pre_dispatch' ), 10, 3 );
 		\add_filter( 'indieauth_metadata', array( $this->fedcm, 'metadata' ) );
 		\add_filter( 'rest_index_indieauth_endpoints', array( $this->fedcm, 'rest_index' ) );
 		\add_action( 'set_logged_in_cookie', array( $this->fedcm, 'set_login_status_logged_in' ) );

includes/functions.php

@@ -387,6 +387,39 @@ function build_url( $parsed_url ) {
 	}
 }
 
+if ( ! function_exists( 'IndieAuth\code_binding_failure' ) ) {
+	/**
+	 * Check an authorization code against the parameters it was issued for.
+	 *
+	 * The authorization endpoint and the token endpoint both redeem the same
+	 * codes, so they must agree exactly on what counts as a match. This is the
+	 * one place that decides it.
+	 *
+	 * URLs are compared with same_url(), because a caller is not lying about its
+	 * identity by omitting a trailing slash, and a failure here destroys the code.
+	 *
+	 * @param array $token  The stored authorization code data.
+	 * @param array $params The parameters supplied at redemption.
+	 * @return string|null The name of the parameter that failed, or null if the code is bound correctly.
+	 */
+	function code_binding_failure( $token, $params ) {
+		$bound = array( 'client_id' );
+
+		// FedCM codes are issued without a redirect_uri; every other code is bound to one.
+		if ( empty( $token['fedcm'] ) ) {
+			$bound[] = 'redirect_uri';
+		}
+
+		foreach ( $bound as $key ) {
+			if ( ! isset( $params[ $key ], $token[ $key ] ) || ! same_url( $token[ $key ], $params[ $key ] ) ) {
+				return $key;
+			}
+		}
+
+		return null;
+	}
+}
+
 if ( ! function_exists( 'IndieAuth\normalize_url' ) ) {
 	/**
 	 * Normalize a URL by adding slash if no path and converting hostname to lowercase.
@@ -421,6 +454,61 @@ function normalize_url( $url, $force_ssl = false ) {
 	}
 }
 
+if ( ! function_exists( 'IndieAuth\same_url' ) ) {
+	/**
+	 * Compare two URLs, ignoring differences that do not change what they point at.
+	 *
+	 * Used to bind an authorization code to the client_id and redirect_uri it was
+	 * issued for. A client that registers "https://app.example.com" and redeems
+	 * with "https://app.example.com/" means the same URL, and the binding check
+	 * must not destroy the code over that.
+	 *
+	 * @param string $url1 First URL.
+	 * @param string $url2 Second URL.
+	 * @return bool True if both URLs are equivalent.
+	 */
+	function same_url( $url1, $url2 ) {
+		if ( ! is_string( $url1 ) || ! is_string( $url2 ) ) {
+			return false;
+		}
+
+		if ( $url1 === $url2 ) {
+			return true;
+		}
+
+		$defaults  = array(
+			'http'  => 80,
+			'https' => 443,
+		);
+		$canonical = array();
+
+		foreach ( array( $url1, $url2 ) as $url ) {
+			$parts = \wp_parse_url( $url );
+			if ( ! is_array( $parts ) || ! isset( $parts['scheme'], $parts['host'] ) ) {
+				return false;
+			}
+
+			// Scheme and host are case-insensitive.
+			$parts['scheme'] = strtolower( $parts['scheme'] );
+			$parts['host']   = strtolower( $parts['host'] );
+
+			// No path means the root path.
+			if ( empty( $parts['path'] ) ) {
+				$parts['path'] = '/';
+			}
+
+			// An explicit default port is the same as none.
+			if ( isset( $parts['port'], $defaults[ $parts['scheme'] ] ) && (int) $parts['port'] === $defaults[ $parts['scheme'] ] ) {
+				unset( $parts['port'] );
+			}
+
+			$canonical[] = build_url( $parts );
+		}
+
+		return $canonical[0] === $canonical[1];
+	}
+}
+
 /**
  * Get Scope.
  *

includes/rest/class-authorization-controller.php

@@ -18,6 +18,8 @@
 use function IndieAuth\pkce_verifier;
 use function IndieAuth\add_query_params_to_url;
 use function IndieAuth\get_url_from_user;
+use function IndieAuth\same_url;
+use function IndieAuth\code_binding_failure;
 
 /**
  * IndieAuth Authorization Controller class.
@@ -409,7 +411,8 @@ public function post( $request ) {
 	 * @return array|OAuth_Response Response to return to the REST Server.
 	 */
 	public function authorization_code( $params ) {
-		$required = array( 'redirect_uri', 'client_id', 'code', 'grant_type' );
+		// redirect_uri is required conditionally below; FedCM codes are issued without one.
+		$required = array( 'client_id', 'code', 'grant_type' );
 		foreach ( $required as $require ) {
 			if ( ! isset( $params[ $require ] ) ) {
 				// translators: Name of missing parameter.
@@ -419,14 +422,24 @@ public function authorization_code( $params ) {
 
 		$code          = $params['code'];
 		$code_verifier = isset( $params['code_verifier'] ) ? $params['code_verifier'] : null;
-		$params        = \wp_array_slice_assoc( $params, array( 'client_id', 'redirect_uri' ) );
 		$token         = $this->get_code( $code );
-		$scopes        = isset( $token['scope'] ) ? array_filter( explode( ' ', $token['scope'] ) ) : array();
 
 		if ( ! $token ) {
 			return new OAuth_Response( 'invalid_grant', \__( 'Invalid authorization code', 'indieauth' ), 400 );
 		}
-		$user = \get_user_by( 'id', $token['user'] );
+
+		$scopes = isset( $token['scope'] ) ? array_filter( explode( ' ', $token['scope'] ) ) : array();
+
+		$bound_params = array( 'client_id' );
+		if ( empty( $token['fedcm'] ) ) {
+			if ( ! isset( $params['redirect_uri'] ) ) {
+				// translators: Name of missing parameter.
+				return new OAuth_Response( 'parameter_absent', sprintf( \__( 'Missing Parameter: %1$s', 'indieauth' ), 'redirect_uri' ), 400 );
+			}
+			$bound_params[] = 'redirect_uri';
+		}
+		$params = \wp_array_slice_assoc( $params, $bound_params );
+		$user   = \get_user_by( 'id', $token['user'] );
 		if ( $token['exp'] <= time() ) {
 			$this->delete_code( $code, $token['user'] );
 			return new OAuth_Response( 'invalid_grant', \__( 'The authorization code expired', 'indieauth' ), 400 );
@@ -446,7 +459,9 @@ public function authorization_code( $params ) {
 			unset( $token['code_challenge_method'] );
 		}
 
-		if ( array() === array_diff_assoc( $params, $token ) ) {
+		// Same check as the token endpoint, from the same function, so the two
+		// endpoints can never drift apart on what a valid redemption looks like.
+		if ( null === code_binding_failure( $token, $params ) ) {
 			$this->delete_code( $code, $token['user'] );
 
 			$return = array( 'me' => $token['me'] );
@@ -457,6 +472,11 @@ public function authorization_code( $params ) {
 
 			return $return;
 		}
+
+		// The token endpoint destroys a code that fails this same binding check.
+		// This endpoint accepts the same codes, so it has to do the same, or the
+		// code could simply be probed here instead.
+		$this->delete_code( $code, $token['user'] );
 		return new OAuth_Response( 'invalid_grant', \__( 'There was an error verifying the authorization code. Check that the client_id and redirect_uri match the original request.', 'indieauth' ), 400 );
 	}