feat: support the RFC 8707 resource indicator in authorize() - #54
Merged
Conversation
getToken() already forwarded `resource`, but authorize() had no way to pass one, so a browser client could not obtain a resource-bound token at all — and Authorizer's own MCP endpoint accepts nothing else. `resource` is now sent on BOTH halves of the code flow. Both are required: the /authorize call is what binds the audience to the authorization code, and the exchange must echo the same value because the token endpoint rejects a code exchange whose resource does not match the authorization request. Sending it in only one place yields either an unbound token or a rejected exchange. Omitted entirely when the caller does not ask for one, so every existing integration keeps the client as its audience — an empty `resource` would be rejected as an invalid target. Not sent on refresh: the server carries the binding across rotation itself, and supplying a stale or guessed value turns a working refresh into invalid_target. Tests mock window.fetch rather than cross-fetch (getFetcher resolves to window.fetch whenever a window exists, so mocking cross-fetch would have left the real fetch in place and asserted an empty call list), run under jsdom with WebCrypto and TextEncoder polyfilled (authorize() is browser-only and derives the PKCE challenge with crypto.subtle, so under the default node environment both tests would have passed vacuously), and were verified to fail when the change is reverted. Full suite: 11 suites, 103 tests passing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds the RFC 8707
resourceindicator toauthorize(), so a browser client can obtain a resource-bound token.This is the hard blocker for using Authorizer's remote MCP server (authorizerdev/authorizer#757) from the browser:
/mcpaccepts only tokens whoseaudis<authorizerURL>/mcp, and there was previously no way to ask for one.getToken()already forwardedresource;authorize()did not.Both halves, or neither works
resourceis sent on the/authorizerequest and on the code exchange. That is not belt-and-braces:/authorizecall is what binds the audience to the authorization coderesourcedoes not match the authorization requestSending it in only one place yields either an unbound token or a rejected exchange.
Backward compatibility
Omitted entirely when the caller does not pass one, so every existing integration keeps the client as its audience. This matters: an empty
resourceis rejected by the server as an invalid target, so emitting the parameter unconditionally would break all of them. There is a test for exactly that.Not sent on refresh. The server carries the binding across rotation itself, and supplying a stale or guessed value turns a working refresh into
invalid_target.Note for the server-side release
Server behaviour changed underneath this: a refresh request naming a different resource than the grant was bound to is now rejected with
invalid_targetrather than silently ignored. That was never a working configuration, but it did not error before.Testing
Two tests, both verified to fail when the change is reverted. Full suite green: 11 suites, 103 tests.
The test fixtures took four passes to make honest, and each failure mode is worth knowing about because they all passed vacuously at first:
authorize()is browser-only and returns early viahasWindow(), so under the default node environment both tests asserted nothing.code_challengederivation needs both; without them the flow throws before building a URL.window.fetchmocked, notcross-fetch—getFetcher()resolves towindow.fetchwhenever a window exists, so mockingcross-fetchleft the real fetch in place and the token-request assertion read an empty call list.Each is commented in the test file so the next person does not rediscover them.