Skip to content
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
6 changes: 4 additions & 2 deletions includes/oauth/class-saddle-oauth-discovery.php
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ public static function protected_resource_metadata() {
$document = array(
'resource' => Saddle_OAuth::resource_id(),
'authorization_servers' => array( Saddle_OAuth::issuer() ),
'scopes_supported' => Saddle_OAuth::SCOPES,
'scopes_supported' => Saddle_OAuth::advertised_scopes(),
'bearer_methods_supported' => array( 'header' ),
'resource_name' => Saddle_MCP::server_name(),
'resource_documentation' => 'https://wordpress.org/plugins/saddle/',
Expand Down Expand Up @@ -358,7 +358,9 @@ public static function authorization_server_metadata() {
'authorization_endpoint' => Saddle_OAuth::endpoint( 'authorize' ),
'token_endpoint' => Saddle_OAuth::endpoint( 'token' ),
'revocation_endpoint' => Saddle_OAuth::endpoint( 'revoke' ),
'scopes_supported' => Saddle_OAuth::SCOPES,
// Includes `offline_access`: ChatGPT checks this list before trusting
// refresh-token renewal (#159). Advertised, never granted as access.
'scopes_supported' => Saddle_OAuth::advertised_scopes(),
'response_types_supported' => array( 'code' ),
'response_modes_supported' => array( 'query' ),
'grant_types_supported' => array( 'authorization_code', 'refresh_token' ),
Expand Down
15 changes: 12 additions & 3 deletions includes/oauth/class-saddle-oauth-endpoints.php
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,10 @@ public static function authorize( WP_REST_Request $request ) {
return self::bounce( $redirect_uri, 'server_error', $request_id->get_error_message(), $state );
}

$asked = trim( (string) $request->get_param( 'scope' ) );
// `offline_access` is a request for a refresh token, which every grant
// gets anyway — it says nothing about access, so it must not count as
// having "named a scope" below. See Saddle_OAuth::strip_refresh_scope().
$asked = Saddle_OAuth::strip_refresh_scope( $request->get_param( 'scope' ) );

// A client that named its scopes is taken at its word — widening a
// deliberate `saddle:read` request would be both a spec violation and a
Expand Down Expand Up @@ -300,8 +303,14 @@ private static function grant_refresh_token( array $params ) {

// A refresh may narrow the scope but never widen it — otherwise a
// read-only grant could quietly promote itself on renewal.
if ( isset( $params['scope'] ) && '' !== $params['scope'] ) {
$requested = Saddle_OAuth::normalize_scope( (string) $params['scope'] );
//
// `offline_access` is stripped first, same as at authorize: a refresh
// that names ONLY that word has not asked to narrow anything, and
// normalizing it alone would fall through to read — silently shrinking
// an admin grant on renewal, which is the opposite failure.
$asked = isset( $params['scope'] ) ? Saddle_OAuth::strip_refresh_scope( (string) $params['scope'] ) : '';
if ( '' !== $asked ) {
$requested = Saddle_OAuth::normalize_scope( $asked );
if ( ! self::scope_is_subset( $requested, $scope ) ) {
return Saddle_OAuth::error_response( 'invalid_scope', __( 'A refresh cannot ask for more access than was originally granted.', 'saddle' ) );
}
Expand Down
49 changes: 49 additions & 0 deletions includes/oauth/class-saddle-oauth.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,22 @@ class Saddle_OAuth {
*/
const SCOPES = array( 'saddle:read', 'saddle:write', 'saddle:admin' );

/**
* The OpenID Connect scope a client asks for to be issued a refresh token.
*
* Saddle issues refresh tokens unconditionally, so this scope changes
* nothing about what a grant can do — it is NOT a tier and never reaches
* {@see self::scope_to_tier()}. It exists because ChatGPT reads a provider's
* discovery metadata for it before trusting refresh-token renewal (OpenAI,
* "Developer mode and MCP apps in ChatGPT", 2026-08): a server that never
* advertises it may be treated as one that never renews, and the connector
* dies at ACCESS_TTL with a reconnect as the only remedy. See #159.
*
* Kept out of SCOPES on purpose: that constant is the tier-clamp set and
* must stay exactly three.
*/
const REFRESH_SCOPE = 'offline_access';

/**
* The scope challenged on an unauthenticated request. Least privilege: a
* client that knows nothing else asks for read, and steps up if it needs to.
Expand Down Expand Up @@ -326,6 +342,39 @@ public static function authorize_capability() {
return (string) apply_filters( 'saddle_oauth_authorize_capability', 'manage_options' );
}

/**
* Every scope the discovery documents list: the three tier scopes plus the
* refresh scope. Discovery-only — {@see self::normalize_scope()} still grants
* from SCOPES alone, so advertising the extra word cannot widen a token.
*
* @return string[]
*/
public static function advertised_scopes() {
return array_merge( self::SCOPES, array( self::REFRESH_SCOPE ) );
}

/**
* Drop the refresh scope from a requested scope string, returning the rest.
*
* The authorize endpoint decides "did this client express a preference?" by
* whether `scope` is empty, and a client that asked for nothing is offered
* the site's own level. `offline_access` is not a preference about access:
* a client that sends ONLY that word has still asked for nothing, and must
* not be pinned to read-only for it (that is precisely the pre-#98 outcome
* for ChatGPT). Strip it BEFORE that decision, not inside normalize_scope(),
* whose "unrecognized words are a preference" rule is right for `openid
* profile` and wrong for this one word.
*
* @param string $requested Space-delimited scope string.
* @return string The same string without the refresh scope, trimmed.
*/
public static function strip_refresh_scope( $requested ) {
$asked = preg_split( '/\s+/', trim( (string) $requested ), -1, PREG_SPLIT_NO_EMPTY );
$asked = is_array( $asked ) ? $asked : array();

return implode( ' ', array_values( array_diff( $asked, array( self::REFRESH_SCOPE ) ) ) );
}

/**
* Normalize a requested scope string to the scopes Saddle actually grants.
*
Expand Down
18 changes: 18 additions & 0 deletions tests/oauth-discovery-test.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,24 @@ public function test_protected_resource_names_the_mcp_endpoint_and_its_server()
$this->assertNotEmpty( $doc['scopes_supported'] );
}

/**
* ChatGPT reads `scopes_supported` for `offline_access` before trusting
* refresh-token renewal; a server that never lists it may be treated as one
* that never renews, and the connector dies at the access-token TTL (#159).
* Both documents carry it — and the tier-clamp set does not.
*/
public function test_offline_access_is_advertised_without_becoming_a_tier() {
$resource = Saddle_OAuth_Discovery::protected_resource_metadata();
$server = Saddle_OAuth_Discovery::authorization_server_metadata();

$this->assertContains( 'offline_access', $resource['scopes_supported'] );
$this->assertContains( 'offline_access', $server['scopes_supported'] );
$this->assertContains( 'refresh_token', $server['grant_types_supported'], 'Advertising the scope only makes sense alongside the grant.' );

$this->assertSame( array( 'saddle:read', 'saddle:write', 'saddle:admin' ), Saddle_OAuth::SCOPES, 'SCOPES is the tier clamp and must stay exactly three.' );
$this->assertSame( 'read', Saddle_OAuth::scope_to_tier( 'offline_access' ), 'The refresh scope must never resolve above the least tier.' );
}

public function test_the_resource_id_has_no_trailing_slash() {
// RFC 8707 canonical form. A client sends this back verbatim as
// `resource=`, and the audience check is a string comparison.
Expand Down
29 changes: 29 additions & 0 deletions tests/oauth-flow-test.php
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,35 @@ public function test_refresh_cannot_widen_scope() {
$this->assertSame( 'invalid_scope', $response->get_data()['error'] );
}

/**
* The refresh-side half of #159. A refresh naming only `offline_access`
* has not asked to narrow anything; before the strip it normalized to read
* and silently shrank the grant on renewal.
*/
public function test_refresh_asking_only_for_offline_access_keeps_the_granted_scope() {
Saddle_Capabilities::set_tier( 'admin' );

$client = $this->register_client();
$verifier = $this->verifier();
$code = $this->authorize( $client['client_id'], $this->challenge_for( $verifier ) );
$tokens = $this->exchange( $client['client_id'], $code, $verifier )->get_data();

$request = new WP_REST_Request( 'POST', '/saddle/v1/oauth/token' );
$request->set_body_params(
array(
'grant_type' => 'refresh_token',
'refresh_token' => $tokens['refresh_token'],
'client_id' => $client['client_id'],
'scope' => 'offline_access',
)
);

$response = rest_get_server()->dispatch( $request );

$this->assertSame( 200, $response->get_status() );
$this->assertSame( $tokens['scope'], $response->get_data()['scope'], 'The refresh scope alone must neither widen nor narrow the grant.' );
}

public function test_unsupported_grant_type_is_named_as_such() {
$request = new WP_REST_Request( 'POST', '/saddle/v1/oauth/token' );
$request->set_body_params( array( 'grant_type' => 'password' ) );
Expand Down
26 changes: 26 additions & 0 deletions tests/oauth-scope-test.php
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,32 @@ public function test_scopes_saddle_does_not_grant_are_dropped_without_widening()
);
}

/**
* `offline_access` is the one unrecognized word that is NOT a preference
* about access: it asks for a refresh token, which every grant gets. Now
* that discovery advertises it (#159), a client that sends only that word
* has still asked for nothing and must reach consent proposing the site's
* own level — not be pinned to read the way `openid profile` is.
*/
public function test_asking_only_for_offline_access_is_asking_for_nothing() {
Saddle_Capabilities::set_tier( 'admin' );

$this->assertSame(
'saddle:read saddle:write saddle:admin',
$this->pending_scope( array( 'scope' => 'offline_access' ) )
);
}

public function test_offline_access_never_widens_an_explicit_request() {
Saddle_Capabilities::set_tier( 'admin' );

$this->assertSame(
'saddle:read',
$this->pending_scope( array( 'scope' => 'saddle:read offline_access' ) ),
'The refresh scope rides along; it must neither widen the request nor survive into the grant.'
);
}

/* ------------------------------------------------------------------
* What the consent screen grants
* --------------------------------------------------------------- */
Expand Down
Loading