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

[pfefferle]

A review of the FedCM implementation (#299) turned up one functional blocker and several security/spec issues. Every fix was developed test-first; the failing test reproducing each issue was written before the fix.

Fixes

FedCM endpoints saw logged-in users as logged out (functional blocker)

accounts and assertion rely on is_user_logged_in(), but core's rest_cookie_check_errors() treats any cookie-authenticated REST request without a _wpnonce/X-WP-Nonce as unauthenticated. The browser's FedCM machinery can never send a REST nonce, so in a real browser the accounts endpoint always answered 401 and the flow dead-ended. (PHPUnit couldn't catch it: wp_set_current_user() doesn't set $wp_rest_auth_cookie, so the nonce path never ran in tests.)

The fix is a rest_authentication_errors filter that exempts requests bearing Sec-Fetch-Dest: webidentity — scoped to the plugin's fedcm/ routes only. Sec--prefixed headers are forbidden for cross-site JavaScript, so this does not open a CSRF hole; non-FedCM routes keep core's strict behavior (covered by tests).

RPs could obtain arbitrary-scope tokens without scope consent

The assertion endpoint accepted whatever params.scope the RP sent and the token endpoint then issued a real access token for it — but the FedCM browser dialog never displays scopes, so a one-click sign-in could silently become a create update token. Requested scopes are now clamped to profile/email; the indieauth_fedcm_scope filter still allows deliberate opt-up.

Code redemption never verified client_id/redirect_uri

verify_local_authorization_code() only checked PKCE — a code could be exchanged with a mismatched client_id or redirect_uri. Both are now enforced at the token endpoint. FedCM codes are issued without a redirect, so they are redeemable without a redirect_uri at both the token and authorization endpoints (keyed off the server-set fedcm marker), and the unused urn:ietf:wg:oauth:2.0:oob placeholder is no longer stored.

Smaller fixes

• Origin validation: a missing port now counts as the scheme default, so a client_id of https://app.example.com:443/ matches the browser's Origin: https://app.example.com.
• Error shape: assertion errors follow the FedCM Error API — {"error": {"code": "invalid_request" | "unauthorized_client" | "access_denied"}}.
• CORS: the public config.json and client_metadata endpoints send Access-Control-Allow-Origin: * without credentials; reflected credentialed CORS remains only on accounts/assertion where the spec requires it.

Testing instructions

1. npm run test:wp-env — 108 tests, includes new coverage for the nonce exemption (positive + two negative cases), scope clamping, code binding (mismatch rejection + FedCM redemption without redirect_uri), default-port origins, error shape, and public CORS.
2. Manual check of the critical fix on a wp-env site: log in, then
   curl -b <cookies> -H "Sec-Fetch-Dest: webidentity" <site>/wp-json/indieauth/1.0/fedcm/accounts
   returns the account list without a nonce; the same request without the header is still rejected, and core routes (e.g. /wp/v2/users/me) still 401 without a nonce even with the header.

Follow-ups (not in this PR)

• IdentityProvider.register() still fires on every dashboard load; it should probably move behind an explicit button on the settings page.
• The two code-redemption paths (authorization + token endpoint) still duplicate the verification mechanics; worth unifying into one shared verifier in a separate refactor.
• Worth one manual Chrome run to confirm the auth cookie (default SameSite=Lax) is attached to FedCM fetches; if not, FedCM-enabled sites need SameSite=None.

[Copilot]

Pull request overview

This PR hardens and unblocks the FedCM (Federated Credential Management) flow in the IndieAuth WordPress plugin by ensuring cookie-authenticated FedCM REST requests work without a REST nonce, clamping scopes to identity-safe values, and binding authorization codes to their original client parameters during redemption.

Changes:

• Exempt FedCM REST requests (scoped to the plugin’s FedCM routes) from WordPress REST cookie nonce enforcement.
• Clamp RP-requested scopes for FedCM assertion to profile/email, and align assertion error responses with the FedCM Error API shape.
• Enforce client_id and conditional redirect_uri binding during authorization code redemption (token + authorization endpoints), with corresponding PHPUnit coverage.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
tests/phpunit/tests/includes/rest/class-test-token-controller.php	Adds tests for client_id/redirect_uri mismatch rejection and FedCM redemption without redirect_uri at the token endpoint.
tests/phpunit/tests/includes/rest/class-test-fedcm-controller.php	Adds tests for public CORS behavior, FedCM error shape, default-port origin handling, scope clamping, and REST nonce exemption behavior.
tests/phpunit/tests/includes/rest/class-test-authorization-controller.php	Adds a test ensuring FedCM codes can be redeemed without redirect_uri at the authorization endpoint.
includes/rest/class-token-controller.php	Makes redirect_uri optional at request-parse time and enforces client_id/redirect_uri binding during code verification (with FedCM exception).
includes/rest/class-fedcm-controller.php	Implements REST nonce exemption for FedCM routes, default-port origin equality, public CORS for non-user-data endpoints, FedCM Error API response shape, and scope clamping.
includes/rest/class-authorization-controller.php	Makes redirect_uri conditionally required based on whether the stored code is FedCM-marked.
includes/class-indieauth.php	Registers the FedCM REST-auth exemption filter.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (2)

includes/rest/class-token-controller.php:390

• For non-FedCM authorization codes, omitting redirect_uri currently falls into the "redirect_uri mismatch" invalid_grant branch and destroys the code. Previously redirect_uri was treated as a required parameter at the token endpoint (invalid_request) and the code could be retried; this change can turn a client bug into a one-shot failure. Consider returning an invalid_request when redirect_uri is missing for non-FedCM codes, and only destroying the code on an actual mismatch.

		// FedCM codes are issued without a redirect_uri; every other code is bound to one.
		if ( empty( $return['fedcm'] ) && ( ! isset( $args['redirect_uri'] ) || ! isset( $return['redirect_uri'] ) || $return['redirect_uri'] !== $args['redirect_uri'] ) ) {
			$codes->destroy( $args['code'] );
			return new OAuth_Response( 'invalid_grant', \__( 'The redirect_uri does not match the authorization request', 'indieauth' ), 400 );
		}

tests/phpunit/tests/includes/rest/class-test-fedcm-controller.php:640

• The data provider PHPDoc says each case returns 3 values, but the provider actually returns 4 (Sec-Fetch-Dest, rest_route, auth_cookie, exempt). Update the @return description so it matches the provider signature and the test method parameters.

	/**
	 * Data provider for the REST nonce exemption checks.
	 *
	 * @return array[] Sec-Fetch-Dest header (null to omit), request URI, whether the cookie user is kept.
	 */

[pfefferle]

Picked up the two suppressed comments from the last review round in 6f2dd9a.

The redirect_uri one is a real regression I introduced here. Making the parameter optional at parse time meant a missing redirect_uri fell through into the mismatch branch and destroyed the code, where before it was rejected as invalid_request by the required-parameter check and the code survived. Split the two cases now: missing is invalid_request and the code stays usable for a retry, an actual mismatch is still invalid_grant and still destroys the code. That also matches RFC 6749 5.2. A client that omits the parameter learns nothing about the stored value, so this does not open a guessing oracle. client_id does not need the same treatment, it is already checked before the code is looked up.

Also fixed the stale @return on the data provider.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (1)

includes/rest/class-fedcm-controller.php:402

• Origin validation compares scheme and host with strict string equality. URL scheme and host are case-insensitive, so a registered client_id like HTTPS://App.Example.com/ would incorrectly fail against the browser’s Origin: https://app.example.com. Normalizing scheme/host to lowercase before comparing (and before default-port lookup) avoids false negatives.

		// Scheme and host must match.
		if ( $origin_scheme !== $client_id_scheme || $origin_host !== $client_id_host ) {
			return false;
		}

[pfefferle]

Fixed the origin casing in 7236f02, scheme and host are lowercased before the comparison and before the default port lookup. Added a test with a HTTPS://App.Example.com/ client_id against a lowercase Origin, which fails without the change.

Same bug as the one in same_origin() on #311, the two functions are near-duplicates. Worth folding the FedCM one into same_origin() once both of these land.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Suppressed comments (1)

includes/rest/class-fedcm-controller.php:661

• Scope clamping can preserve duplicates because array_intersect() keeps every occurrence from $requested. A request like profile profile email would store profile profile email, which is non-canonical and may confuse downstream scope parsing/comparisons. Consider de-duplicating (and optionally normalizing whitespace) before implode().

			// The FedCM browser UI never displays scopes, so only identity
			// scopes may be granted without a real consent screen.
			$requested = array_filter( explode( ' ', \sanitize_text_field( $params['scope'] ) ) );
			$granted   = array_values( array_intersect( $requested, array( 'profile', 'email' ) ) );
			if ( $granted ) {
				$scope = implode( ' ', $granted );
			}

[pfefferle]

Took the scope one in c9330a5. array_intersect() keeps every occurrence, so profile email profile was stored as-is. Deduplicated now, with a test.

[pfefferle]

Ran a full review over this branch and it turned up more than the bot rounds did. Fixed in 98946aa:

The missing redirect_uri change I made earlier was wrong and I reverted it. Not destroying the code made the endpoint a repeatable liveness oracle: client_id is the client's public URL, so anyone holding a harvested code could poll until they had what they needed, and the code survived every probe. A binding failure destroys the code again, missing parameter included. A single request can still tell a live code from one that was never issued, same as any other binding failure, but it costs the attacker the code. There is a test for that specifically.

The authorization endpoint had the same gap from the other side. It accepts the same codes but never revoked on a mismatch, so the destroy at the token endpoint did not actually hold, you could just probe the other endpoint instead. It revokes now too.

client_id and redirect_uri are compared with a new same_url() that ignores a missing trailing slash, scheme and host case, and an explicit default port. With a raw !== a client sending https://app.example.com against a stored https://app.example.com/ had its code destroyed and could not retry, which trunk never did.

The CORS work in this branch did not reach the browser. Core's rest_send_cors_headers() runs on rest_pre_serve_request, after the response headers have gone out, and header() replaces, so it overwrote the * with the echoed Origin plus credentials. Core's handler is removed for FedCM routes now. The assertion error responses never had CORS headers at all, so the browser dropped the body and the Error API codes never surfaced.

Scope: an empty or write-only scope request fell back to profile and minted a bearer token the client never asked for. It grants what survives the clamp and nothing else now, so those requests get an identity response with no token.

The nonce exemption no longer returns true. That ended the whole filter chain, which would skip any REST hardening plugin's policy on FedCM routes, not just the nonce check. It removes core's nonce callback instead.

One thing I did not touch: the nonce exemption may not be reachable at all. Browsers only send SameSite=None cookies on FedCM requests and WordPress auth cookies carry no SameSite attribute, so they are treated as Lax and never arrive. If that is right the flow fails on a stock install and the tests only pass because they set $wp_rest_auth_cookie by hand. I have not tried it in a browser, so I would rather you look before I change how the auth cookie is sent.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Suppressed comments (1)

includes/rest/class-authorization-controller.php:477

• This binding-failure path will now destroy the code (good), but the binding check above still uses array_diff_assoc( $params, $token ), which treats cosmetic-but-equivalent URLs as mismatches (e.g. https://app.example.com vs https://app.example.com/, explicit default ports). That can unnecessarily burn valid codes and also makes this endpoint stricter than the token endpoint (which now uses same_url()), so the binding check should use same_url() for client_id (and redirect_uri when present).

		// 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 );