[Tests] Pin that protected resource metadata advertises no scope of its own - #440
Conversation
…ts own SEP-2207 says a resource server must not advertise `offline_access` as a required scope. Nothing in src/ writes it - `scopesSupported` is entirely operator-supplied - so there was nothing to fix, but also nothing stopping a future default from quietly starting to advertise one.
There was a problem hiding this comment.
Pull request overview
Adds regression coverage to ensure the server-side Protected Resource Metadata (RFC 9728) emitted by the PHP SDK does not start advertising offline_access as a required/supported scope (per SEP-2207 / issue #364). This is a test-only PR intended to prevent future defaults from quietly introducing offline_access into scopes_supported.
Changes:
- Added a unit test asserting
scopes_supportedreflects only operator-supplied scopes and does not includeoffline_access. - Added a unit test asserting that omitting scopes results in no
scopes_supportedmember being serialized.
Suppressed comments (1)
tests/Unit/Server/Transport/Http/OAuth/ProtectedResourceMetadataTest.php:92
- This comment says “advertising one”, which is ambiguous (it’s referring to the offline_access scope). Rewording makes the intent clearer.
// SEP-2207: `offline_access` is a refresh-token scope, not something a
// resource requires. Nothing in the SDK injects it — this pins that, so
// a future default cannot quietly start advertising one.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| new ProtectedResourceMetadata([]); | ||
| } | ||
|
|
||
| #[TestDox('the SDK advertises exactly the scopes it was given, and never adds offline_access')] |
There was a problem hiding this comment.
Not changing this — the test's input has no whitespace/empties/duplicates, so normalize() is a no-op here; the TestDox describes this test's specific SEP-2207 guard, not a general preservation guarantee.
| $data = $metadata->jsonSerialize(); | ||
|
|
||
| $this->assertSame(['mcp:read', 'mcp:write'], $data['scopes_supported']); | ||
| $this->assertNotContains('offline_access', $data['scopes_supported']); |
There was a problem hiding this comment.
Not adding assertArrayHasKey here — line 101 already dereferences the same key and would fail first with a clear message if it were missing; jsonSerialize() also guarantees the key is set whenever scopesSupported is non-null.
chr-hertel
added
the
2026-07-28
SEP-2207 (#364) says a resource server must not advertise
offline_accessas a required scope.There is nothing to fix:
ProtectedResourceMetadata::$scopesSupportedis entirely operator-supplied, and nothing insrc/writesoffline_accessanywhere. But there was also nothing stopping a future default from quietly starting to advertise one, so this pins the behaviour with a regression test.Test only — no production code changes.