Validate OAuth authorization state

[halter73]

Summary

• generate a fresh 256-bit, base64url-encoded state value for each OAuth authorization-code transaction
• include state in authorization requests and require an exact callback match before code exchange
• extend AuthorizationResult and callback implementations to return redirect state
• update the default callback to parse the full redirect URL
• preserve the obsolete code-only callback during its compatibility window while documenting that it cannot validate response state
• add coverage for state presence, matching, missing and mismatched responses, uniqueness, token-exchange blocking, and reserved parameter handling

This is a follow-up to tarekgh's review comment on #1605, which identified the missing authorization-response binding.

[Copilot]

Pull request overview

This PR hardens the OAuth authorization-code flow in ModelContextProtocol.Core by generating a per-transaction state value, sending it in the authorization request, and requiring an exact match in the authorization response before exchanging the code for tokens. It also extends the callback/result plumbing (including samples and tests) to propagate state alongside code and iss, while keeping the obsolete code-only callback path (without state validation) during its compatibility window.

Changes:

• Add per-authorization state generation, request inclusion, and exact-match validation prior to token exchange.
• Extend AuthorizationResult plus default/manual and test callback handlers to capture/return state.
• Expand test coverage for state presence/mismatch and ensure additional auth parameters cannot override reserved parameters (including state).

Show a summary per file

File	Description
tests/ModelContextProtocol.TestOAuthServer/Program.cs	Adds a counter for authorization-code token exchanges to assert code-exchange blocking behavior in tests.
tests/ModelContextProtocol.ConformanceClient/Program.cs	Updates redirect parsing to extract and return state in AuthorizationResult.
tests/ModelContextProtocol.AspNetCore.Tests/OAuth/OAuthTestBase.cs	Updates test callback handler to return state from redirects.
tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs	Adds state validation tests, uniqueness coverage, and expands reserved-parameter override coverage to include state.
src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs	Generates state, includes it in auth URL, validates response state, and updates default/manual redirect handler to parse full redirect URL (including state).
src/ModelContextProtocol.Core/Authentication/ClientOAuthOptions.cs	Updates docs to require returning state (and optionally iss) from the callback handler; clarifies reserved params include state.
src/ModelContextProtocol.Core/Authentication/AuthorizationResult.cs	Adds State property and updates docs/remarks to describe its security role and requirement.
samples/ProtectedMcpClient/Program.cs	Updates sample callback implementation to read/return state.

Review details

Comments suppressed due to low confidence (1)

src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs:722

• Same as above: this throw hard-codes AuthorizationCallbackHandler in the message, which can be misleading when the obsolete redirect delegate is configured (it is wrapped and still flows through here). Consider a handler-agnostic message.

        if (string.IsNullOrEmpty(authResult.Code))
        {
            ThrowFailedToHandleUnauthorizedResponse($"The {nameof(ClientOAuthOptions.AuthorizationCallbackHandler)} returned a null or empty authorization code.");
        }

• Files reviewed: 8/8 changed files
• Comments generated: 2
• Review effort level: Low

[tarekgh]

A few notes from reviewing the state-validation change. The core security logic (256-bit per-transaction state, exact match before code exchange, reserved-parameter protection) looks correct and is well covered by the new tests. The items below are about app-compat and diagnostics.

[tarekgh]

Enabling state validation for AuthorizationCallbackHandler is a behavioral breaking change. Existing handlers that return an AuthorizationResult with Code and Iss but no State will now fail with 'did not include the required state parameter'. AuthorizationCallbackHandler shipped recently (#1605) so the blast radius is small, but this is worth calling out in the release notes and applying a breaking-change label, so consumers know to populate State from the redirect's state query parameter.

[tarekgh]

Nice to see state noted here. For consistency, the MCP9007 entry in docs/list-of-diagnostics.md still describes the obsolete delegate as only lacking the RFC 9207 issuer. Consider updating it to mention that authorization-response state validation is also skipped, to match these remarks.