Skip to content

Commit 6768cd0

Browse files
ifahimrezaclaude
andcommitted
fix(oauth): advertise offline_access, and treat it as asking for nothing
ChatGPT checks discovery metadata for offline_access before trusting refresh-token renewal. Advertise it in both documents, and strip it at authorize and refresh before the scope decision, so a client sending only that word is neither pinned to read nor narrowed on renewal. Closes #159 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ApYKEKcBnpuz8w6BX2agfH
1 parent fd1c77b commit 6768cd0

6 files changed

Lines changed: 138 additions & 5 deletions

includes/oauth/class-saddle-oauth-discovery.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,7 @@ public static function protected_resource_metadata() {
330330
$document = array(
331331
'resource' => Saddle_OAuth::resource_id(),
332332
'authorization_servers' => array( Saddle_OAuth::issuer() ),
333-
'scopes_supported' => Saddle_OAuth::SCOPES,
333+
'scopes_supported' => Saddle_OAuth::advertised_scopes(),
334334
'bearer_methods_supported' => array( 'header' ),
335335
'resource_name' => Saddle_MCP::server_name(),
336336
'resource_documentation' => 'https://wordpress.org/plugins/saddle/',
@@ -358,7 +358,9 @@ public static function authorization_server_metadata() {
358358
'authorization_endpoint' => Saddle_OAuth::endpoint( 'authorize' ),
359359
'token_endpoint' => Saddle_OAuth::endpoint( 'token' ),
360360
'revocation_endpoint' => Saddle_OAuth::endpoint( 'revoke' ),
361-
'scopes_supported' => Saddle_OAuth::SCOPES,
361+
// Includes `offline_access`: ChatGPT checks this list before trusting
362+
// refresh-token renewal (#159). Advertised, never granted as access.
363+
'scopes_supported' => Saddle_OAuth::advertised_scopes(),
362364
'response_types_supported' => array( 'code' ),
363365
'response_modes_supported' => array( 'query' ),
364366
'grant_types_supported' => array( 'authorization_code', 'refresh_token' ),

includes/oauth/class-saddle-oauth-endpoints.php

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,10 @@ public static function authorize( WP_REST_Request $request ) {
154154
return self::bounce( $redirect_uri, 'server_error', $request_id->get_error_message(), $state );
155155
}
156156

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

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

301304
// A refresh may narrow the scope but never widen it — otherwise a
302305
// read-only grant could quietly promote itself on renewal.
303-
if ( isset( $params['scope'] ) && '' !== $params['scope'] ) {
304-
$requested = Saddle_OAuth::normalize_scope( (string) $params['scope'] );
306+
//
307+
// `offline_access` is stripped first, same as at authorize: a refresh
308+
// that names ONLY that word has not asked to narrow anything, and
309+
// normalizing it alone would fall through to read — silently shrinking
310+
// an admin grant on renewal, which is the opposite failure.
311+
$asked = isset( $params['scope'] ) ? Saddle_OAuth::strip_refresh_scope( (string) $params['scope'] ) : '';
312+
if ( '' !== $asked ) {
313+
$requested = Saddle_OAuth::normalize_scope( $asked );
305314
if ( ! self::scope_is_subset( $requested, $scope ) ) {
306315
return Saddle_OAuth::error_response( 'invalid_scope', __( 'A refresh cannot ask for more access than was originally granted.', 'saddle' ) );
307316
}

includes/oauth/class-saddle-oauth.php

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,22 @@ class Saddle_OAuth {
4848
*/
4949
const SCOPES = array( 'saddle:read', 'saddle:write', 'saddle:admin' );
5050

51+
/**
52+
* The OpenID Connect scope a client asks for to be issued a refresh token.
53+
*
54+
* Saddle issues refresh tokens unconditionally, so this scope changes
55+
* nothing about what a grant can do — it is NOT a tier and never reaches
56+
* {@see self::scope_to_tier()}. It exists because ChatGPT reads a provider's
57+
* discovery metadata for it before trusting refresh-token renewal (OpenAI,
58+
* "Developer mode and MCP apps in ChatGPT", 2026-08): a server that never
59+
* advertises it may be treated as one that never renews, and the connector
60+
* dies at ACCESS_TTL with a reconnect as the only remedy. See #159.
61+
*
62+
* Kept out of SCOPES on purpose: that constant is the tier-clamp set and
63+
* must stay exactly three.
64+
*/
65+
const REFRESH_SCOPE = 'offline_access';
66+
5167
/**
5268
* The scope challenged on an unauthenticated request. Least privilege: a
5369
* client that knows nothing else asks for read, and steps up if it needs to.
@@ -326,6 +342,39 @@ public static function authorize_capability() {
326342
return (string) apply_filters( 'saddle_oauth_authorize_capability', 'manage_options' );
327343
}
328344

345+
/**
346+
* Every scope the discovery documents list: the three tier scopes plus the
347+
* refresh scope. Discovery-only — {@see self::normalize_scope()} still grants
348+
* from SCOPES alone, so advertising the extra word cannot widen a token.
349+
*
350+
* @return string[]
351+
*/
352+
public static function advertised_scopes() {
353+
return array_merge( self::SCOPES, array( self::REFRESH_SCOPE ) );
354+
}
355+
356+
/**
357+
* Drop the refresh scope from a requested scope string, returning the rest.
358+
*
359+
* The authorize endpoint decides "did this client express a preference?" by
360+
* whether `scope` is empty, and a client that asked for nothing is offered
361+
* the site's own level. `offline_access` is not a preference about access:
362+
* a client that sends ONLY that word has still asked for nothing, and must
363+
* not be pinned to read-only for it (that is precisely the pre-#98 outcome
364+
* for ChatGPT). Strip it BEFORE that decision, not inside normalize_scope(),
365+
* whose "unrecognized words are a preference" rule is right for `openid
366+
* profile` and wrong for this one word.
367+
*
368+
* @param string $requested Space-delimited scope string.
369+
* @return string The same string without the refresh scope, trimmed.
370+
*/
371+
public static function strip_refresh_scope( $requested ) {
372+
$asked = preg_split( '/\s+/', trim( (string) $requested ), -1, PREG_SPLIT_NO_EMPTY );
373+
$asked = is_array( $asked ) ? $asked : array();
374+
375+
return implode( ' ', array_values( array_diff( $asked, array( self::REFRESH_SCOPE ) ) ) );
376+
}
377+
329378
/**
330379
* Normalize a requested scope string to the scopes Saddle actually grants.
331380
*

tests/oauth-discovery-test.php

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,24 @@ public function test_protected_resource_names_the_mcp_endpoint_and_its_server()
6262
$this->assertNotEmpty( $doc['scopes_supported'] );
6363
}
6464

65+
/**
66+
* ChatGPT reads `scopes_supported` for `offline_access` before trusting
67+
* refresh-token renewal; a server that never lists it may be treated as one
68+
* that never renews, and the connector dies at the access-token TTL (#159).
69+
* Both documents carry it — and the tier-clamp set does not.
70+
*/
71+
public function test_offline_access_is_advertised_without_becoming_a_tier() {
72+
$resource = Saddle_OAuth_Discovery::protected_resource_metadata();
73+
$server = Saddle_OAuth_Discovery::authorization_server_metadata();
74+
75+
$this->assertContains( 'offline_access', $resource['scopes_supported'] );
76+
$this->assertContains( 'offline_access', $server['scopes_supported'] );
77+
$this->assertContains( 'refresh_token', $server['grant_types_supported'], 'Advertising the scope only makes sense alongside the grant.' );
78+
79+
$this->assertSame( array( 'saddle:read', 'saddle:write', 'saddle:admin' ), Saddle_OAuth::SCOPES, 'SCOPES is the tier clamp and must stay exactly three.' );
80+
$this->assertSame( 'read', Saddle_OAuth::scope_to_tier( 'offline_access' ), 'The refresh scope must never resolve above the least tier.' );
81+
}
82+
6583
public function test_the_resource_id_has_no_trailing_slash() {
6684
// RFC 8707 canonical form. A client sends this back verbatim as
6785
// `resource=`, and the audience check is a string comparison.

tests/oauth-flow-test.php

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -482,6 +482,35 @@ public function test_refresh_cannot_widen_scope() {
482482
$this->assertSame( 'invalid_scope', $response->get_data()['error'] );
483483
}
484484

485+
/**
486+
* The refresh-side half of #159. A refresh naming only `offline_access`
487+
* has not asked to narrow anything; before the strip it normalized to read
488+
* and silently shrank the grant on renewal.
489+
*/
490+
public function test_refresh_asking_only_for_offline_access_keeps_the_granted_scope() {
491+
Saddle_Capabilities::set_tier( 'admin' );
492+
493+
$client = $this->register_client();
494+
$verifier = $this->verifier();
495+
$code = $this->authorize( $client['client_id'], $this->challenge_for( $verifier ) );
496+
$tokens = $this->exchange( $client['client_id'], $code, $verifier )->get_data();
497+
498+
$request = new WP_REST_Request( 'POST', '/saddle/v1/oauth/token' );
499+
$request->set_body_params(
500+
array(
501+
'grant_type' => 'refresh_token',
502+
'refresh_token' => $tokens['refresh_token'],
503+
'client_id' => $client['client_id'],
504+
'scope' => 'offline_access',
505+
)
506+
);
507+
508+
$response = rest_get_server()->dispatch( $request );
509+
510+
$this->assertSame( 200, $response->get_status() );
511+
$this->assertSame( $tokens['scope'], $response->get_data()['scope'], 'The refresh scope alone must neither widen nor narrow the grant.' );
512+
}
513+
485514
public function test_unsupported_grant_type_is_named_as_such() {
486515
$request = new WP_REST_Request( 'POST', '/saddle/v1/oauth/token' );
487516
$request->set_body_params( array( 'grant_type' => 'password' ) );

tests/oauth-scope-test.php

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,32 @@ public function test_scopes_saddle_does_not_grant_are_dropped_without_widening()
279279
);
280280
}
281281

282+
/**
283+
* `offline_access` is the one unrecognized word that is NOT a preference
284+
* about access: it asks for a refresh token, which every grant gets. Now
285+
* that discovery advertises it (#159), a client that sends only that word
286+
* has still asked for nothing and must reach consent proposing the site's
287+
* own level — not be pinned to read the way `openid profile` is.
288+
*/
289+
public function test_asking_only_for_offline_access_is_asking_for_nothing() {
290+
Saddle_Capabilities::set_tier( 'admin' );
291+
292+
$this->assertSame(
293+
'saddle:read saddle:write saddle:admin',
294+
$this->pending_scope( array( 'scope' => 'offline_access' ) )
295+
);
296+
}
297+
298+
public function test_offline_access_never_widens_an_explicit_request() {
299+
Saddle_Capabilities::set_tier( 'admin' );
300+
301+
$this->assertSame(
302+
'saddle:read',
303+
$this->pending_scope( array( 'scope' => 'saddle:read offline_access' ) ),
304+
'The refresh scope rides along; it must neither widen the request nor survive into the grant.'
305+
);
306+
}
307+
282308
/* ------------------------------------------------------------------
283309
* What the consent screen grants
284310
* --------------------------------------------------------------- */

0 commit comments

Comments
 (0)