[Client] Support completing OAuth authorization in a later request by bbdu3 · Pull Request #660 · modelcontextprotocol/php-sdk

bbdu3 · 2026-11-04T08:39:52Z

Summary

Support deferred OAuth authorization for web applications, allowing the authorization code flow to resume through a subsequent callback request, including across different PHP workers.

Adds RedirectAuthorizationHandler, AuthorizationPendingException, and OAuthAuthenticator::finishAuthorization(), with persistent, single-use authorization state and regression tests.

Fixes #630.

Motivation and Context

AuthorizationHandlerInterface::authorize() currently expects callback parameters within the same call, which does not fit web applications where authorization redirects and callbacks are handled by separate HTTP requests.

This change implements the approach :

The handler throws AuthorizationPendingException with the authorization URL.
The authenticator persists the pending flow before propagating the exception.
The callback route calls finishAuthorization() to validate the response, exchange the authorization code, and store the tokens.
The application can then retry the original MCP operation.

Test

Unit tests

All 54 OAuthAuthenticator tests pass, covering cross-instance completion, PKCE context preservation, state validation and replay prevention, issuer validation, user storage isolation, and offline_access fallback.

Storage tests cover interleaved writes, atomic consumption across instances, issuer-specific cleanup, and malformed records.

HTTP integration tests

All 10 OAuth integration tests pass against local authorization and MCP server fixtures.

The cross-process regression test verifies that:

Authorization can be completed in separate PHP processes.
Only one competing callback succeeds.
The persisted token can access a protected MCP tool.
Replayed callbacks are rejected.

Broader checks

Full unit suite: 1,742 tests, with the same 5 pre-existing Windows-related failures and 1 skipped test as the baseline. i confirmed these failures are not regression introduced in this PR.
PHPStan: same 5 pre-existing errors, no new errors.
PHP CS Fixer passes for changed PHP files.
git diff --check passes.

Breaking Changes

None. Existing interfaces and synchronous authorization remain compatible.

CredentialStorageInterface is unchanged. Deferred authorization requires:

Persistent storage implementing PendingAuthorizationStorageInterface.
Catching AuthorizationPendingException and redirecting to its authorization URL.
Recreating the authenticator in the callback route with the same configuration and session-scoped storage.
Calling finishAuthorization() before retrying the MCP operation.

Types of Changes

Bug fix (non-breaking change)
New feature (non-breaking change)
Documentation update

chr-hertel · 2026-11-10T08:17:35Z

Thanks @bbdu3 for working on this! The client auth stack moved quite a bit underneath you - #663, #664, #666, #668 and #669 are on main now. So I ported your commit onto main, fixed what broke on the way and retargeted the PR - your commit stays first, my fixes are on top.

adapted to main: OAuthErrorException instead of matching messages, tokens keyed by audience, state checked before the error
Client::connect() wrapped the exception in a ConnectionException, so the documented catch (AuthorizationPendingException) never fired - it's rethrown as it is now, and HttpTransport doesn't log it as an error anymore
the pending authorization kept only part of the metadata - without code_challenge_methods_supported a restarted flow gets refused
it also stored the whole client, incl. a configured client_secret - a configured client is now resolved again on the callback, only a dynamic registration is kept, so another flow re-registering doesn't break the callback
finishAuthorization() checks the expected issuer and forgets a registration the server answered invalid_client for, like the normal flow
audience always equals the resource on main, so I dropped it
OAuth::build() refuses RedirectAuthorizationHandler without setCredentialStorage(), since the default in-memory storage loses the pending state with the request
PHPStan errors in the tests and the fixture, TestDox on the new tests, docs rewritten and the changelog entry moved to 0.10.0

One more thing: the commit's author email reads gamil.com and the sign-off says unknown - want to fix that?

Copilot

🟡 Changes recommended

The documentation must require session initialization on both routes before deriving the credential path, otherwise browsers can share a credential file and reuse another user’s access token.

1 open issue

Initialize the session before deriving the credential file path · New

What changed in this PR

Adds deferred OAuth authorization to the PHP client's authentication flow, allowing web applications to complete authorization in a later callback request, including on another PHP worker.

Changes:

Adds redirect-based authorization and finishAuthorization() while preserving synchronous handlers.
Persists expiring, single-use authorization state and propagates pending authorization exceptions.
Documents the web callback flow and adds unit and cross-process integration coverage.

File	Description
`tests/Unit/Client/Transport/HttpTransportTest.php`	Tests pending authorization propagation without retries or error logging.
`tests/Unit/Client/Auth/OAuthTest.php`	Tests the redirect handler's storage requirement.
`tests/Unit/Client/Auth/OAuthAuthenticatorTest.php`	Covers deferred completion, validation, replay prevention, and fallback.
`tests/Unit/Client/Auth/FileCredentialStorageTest.php`	Tests pending state persistence, consumption, and cleanup.
`tests/Unit/Client/Auth/AuthorizationServerMetadataTest.php`	Tests metadata serialization round trips.
`tests/Integration/Fixture/oauth/finish-authorization.php`	Completes authorization in a separate process.
`tests/Integration/ClientOAuthTest.php`	Tests cross-process completion and competing callbacks.
`src/Exception/AuthorizationPendingException.php`	Adds an exception carrying the authorization URL.
`src/Client/Transport/HttpTransport.php`	Propagates pending authorization without error handling.
`src/Client/Auth/RedirectAuthorizationHandler.php`	Defers authorization through the new exception.
`src/Client/Auth/PendingAuthorizationStorageInterface.php`	Defines pending state storage and atomic consumption.
`src/Client/Auth/PendingAuthorization.php`	Represents and serializes pending authorization context.
`src/Client/Auth/OAuthAuthenticator.php`	Saves pending flows and completes callbacks.
`src/Client/Auth/OAuth.php`	Requires explicit storage for redirect authorization.
`src/Client/Auth/InMemoryCredentialStorage.php`	Adds in-memory pending state management.
`src/Client/Auth/FileCredentialStorage.php`	Persists and atomically consumes pending state.
`src/Client/Auth/AuthorizationServerMetadata.php`	Adds metadata serialization for persisted flows.
`src/Client/Auth/AuthorizationHandlerInterface.php`	Documents the pending authorization exception.
`src/Client.php`	Preserves pending authorization exceptions during connection.
`docs/client/authorization.md`	Documents session-scoped storage and callback handling.
`CHANGELOG.md`	Records deferred OAuth authorization support.

_{Review effort: Balanced}

Copilot

🔵 Needs a closer look

The server-side OAuth example omits setApplicationType('web'), leaving the native default and risking rejection of its HTTPS callback by authorization servers enforcing native-app redirect restrictions.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Set the OAuth application type to web

docs/client/authorization.md:105

Please add setApplicationType('web') to this server-side example. OAuth defaults to native, and neither the redirect URI nor the handler changes that setting, so the example registers as a native application; authorization servers enforcing native-app redirect restrictions can reject this HTTPS callback.

1 resolved since last review

Initialize the session before deriving the credential file path

_{Review effort: Balanced}

Copilot

🔵 Needs a closer look

Human review is needed to assess an unpublished concern about test cleanup leaving hidden lock files in the system temporary directory.

No open issues 🎉

_{Review effort: Balanced}

Copilot

🔵 Needs a closer look

Human assessment is needed because issuer-wide client re-registration can invalidate another pending OAuth flow, causing a valid callback to fail after its state has been consumed.

No open issues 🎉

_{Review effort: Balanced}

Copilot

🔵 Needs a closer look

Human assessment is needed because a critical concurrency race can allow a rejected stale callback to delete another worker’s newer registration, tokens, and pending authorizations.

No open issues 🎉

_{Review effort: Balanced}

Signed-off-by: unknown <[email protected]>

…t doubles

…horization

…l flow Resolve the client again instead of storing it, so a configured secret stays out of the storage. Check the expected issuer, forget a registration the server refused, and drop the audience field, which always equals the resource.

…rage

… them with TestDox

…ocs as web

…ation

…deferred callback

Copilot

🟢 Approval recommended

The deferred OAuth authorization change has no active review issues.

No open issues 🎉

_{Review effort: Balanced}

chr-hertel

Thanks @bbdu3!

bbdu3 requested review from CodeWithKyrian, Nyholm, chr-hertel and soyuka as code owners November 4, 2026 08:39

bbdu3 changed the base branch from main to feat-client-auth November 4, 2026 09:39

chr-hertel added this to the 1.0.0 milestone Nov 4, 2026

chr-hertel added Client Issues & PRs related to the Client component auth Issues and PRs related to Authentication / OAuth labels Nov 4, 2026

chr-hertel mentioned this pull request Nov 4, 2026

[Client] Add OAuth authorization with the authorization code flow #666

Merged

chr-hertel modified the milestones: 1.0.0, 0.10.0 Nov 8, 2026

chr-hertel force-pushed the fix/web-oauth-authorization branch from f9f0073 to 4134ed6 Compare November 10, 2026 08:17

chr-hertel changed the base branch from feat-client-auth to main November 10, 2026 08:17

chr-hertel requested a balanced review from Copilot November 10, 2026 08:18

Copilot started reviewing on behalf of chr-hertel November 10, 2026 08:18

View session

Copilot AI reviewed Nov 10, 2026

View reviewed changes

Comment thread docs/client/authorization.md Resolved

chr-hertel requested a balanced review from Copilot November 10, 2026 08:25

Copilot started reviewing on behalf of chr-hertel November 10, 2026 08:25

View session

Copilot AI reviewed Nov 10, 2026

View reviewed changes

chr-hertel requested a balanced review from Copilot November 10, 2026 08:34

Copilot started reviewing on behalf of chr-hertel November 10, 2026 08:34

View session

Copilot AI reviewed Nov 10, 2026

View reviewed changes

chr-hertel requested a balanced review from Copilot November 10, 2026 09:14

Copilot started reviewing on behalf of chr-hertel November 10, 2026 09:15

View session

Copilot AI reviewed Nov 10, 2026

View reviewed changes

chr-hertel requested a balanced review from Copilot November 10, 2026 09:35

Copilot started reviewing on behalf of chr-hertel November 10, 2026 09:36

View session

Copilot AI reviewed Nov 10, 2026

View reviewed changes

chr-hertel mentioned this pull request Nov 10, 2026

[Client] Keep a client another worker registered when dropping a refused one #725

Merged

bbdu3 and others added 13 commits November 10, 2026 10:58

fix: support completing OAuth authorization across requests

bf56e5b

Signed-off-by: unknown <[email protected]>

[Client] Pass synchronize() through in the deferred authorization tes…

62f5729

…t doubles

[Client] Keep the full authorization server metadata in a pending aut…

5312a66

…horization

[Client] Refuse RedirectAuthorizationHandler without a credential sto…

1d951eb

…rage

[Client] Don't log a pending authorization as a transport error

7d1b0b3

[Client] Fix PHPStan in the deferred authorization tests and describe…

b75379d

… them with TestDox

[Client] Document finishing the authorization in a later request

033c008

[Client] Require a started session in the deferred authorization docs

6460aed

[Client] Register the web application in the deferred authorization d…

8a725a3

…ocs as web

[Client] Remove the hidden credential lock files after the OAuth tests

92788f2

[Client] Finish a deferred authorization with its own dynamic registr…

dcd83ba

…ation

[Client] Drop a refused registration with forgetRefusedClient() on a …

849aa6d

…deferred callback

chr-hertel force-pushed the fix/web-oauth-authorization branch from 3acec6a to 849aa6d Compare November 10, 2026 10:10

chr-hertel requested a balanced review from Copilot November 10, 2026 10:13

Copilot started reviewing on behalf of chr-hertel November 10, 2026 10:14

View session

Copilot AI reviewed Nov 10, 2026

View reviewed changes

chr-hertel approved these changes Nov 10, 2026

View reviewed changes

chr-hertel merged commit da5d0d7 into modelcontextprotocol:main Nov 10, 2026
30 checks passed

chr-hertel mentioned this pull request Nov 10, 2026

[Client] Close the remaining OAuth and redirect gaps before 0.10.0 #727

Merged

Navigation Menu

Repository navigation

[Client] Support completing OAuth authorization in a later request - #660

[Client] Support completing OAuth authorization in a later request#660
chr-hertel merged 13 commits into
modelcontextprotocol:mainfrom
bbdu3:fix/web-oauth-authorization

bbdu3 commented Nov 4, 2026 •

edited

Loading

Uh oh!

chr-hertel commented Nov 10, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

chr-hertel left a comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

bbdu3 commented Nov 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Motivation and Context

Test

Breaking Changes

Types of Changes

Uh oh!

chr-hertel commented Nov 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

chr-hertel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bbdu3 commented Nov 4, 2026 •

edited

Loading

chr-hertel commented Nov 10, 2026 •

edited

Loading