Skip to content

Fix OAuthProxyProviderClient validation #620

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

izqui
Copy link

@izqui izqui commented Jun 12, 2025

This PR fixes OAuth proxy provider functionality by adding configurable validation controls and resolving overly permissive client validation that was breaking tests and compromising security.

Motivation and Context

The core issue was that for stateless proxy setups, you want to:

  • Return undefined from getClient since you don't store client information locally
  • Skip local client validation since the upstream server handles it
  • Skip local PKCE validation since the upstream server handles it

But this wasn't properly configurable and was affecting non-proxy scenarios.

How Has This Been Tested?

  • All existing tests now pass (previously 4 tests were failing)
  • Added comprehensive test coverage for both strict and permissive validation modes
  • Tested proxy provider with both stateless (getClient returns undefined) and stateful configurations
  • Verified that non-proxy OAuth providers maintain strict validation by default
  • Tested token, authorization, and revocation handlers with various client validation scenarios

Test scenarios covered:

  • Standard OAuth providers with strict client validation (default behavior)
  • Proxy providers with stateless operation (skipLocalClientValidation: true)
  • Proxy providers with mixed validation (local client validation, upstream PKCE validation)
  • Invalid client scenarios properly rejected when validation is enabled
  • Fallback client creation working correctly when validation is disabled

Breaking Changes

No breaking changes - this is fully backward compatible:

  • Existing ProxyOAuthServerProvider instances continue to work exactly as before (defaults to skipLocalClientValidation: true and skipLocalPkceValidation: true)
  • Standard OAuth providers maintain strict validation by default
  • All existing APIs remain unchanged

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

Key changes made:

  1. Added configurable validation properties to OAuthServerProvider interface:

    skipLocalPkceValidation?: boolean;
    skipLocalClientValidation?: boolean;
  2. Enhanced ProxyOptions to accept these configuration flags:

    export type ProxyOptions = {
      // ... existing properties
      skipLocalPkceValidation?: boolean;
      skipLocalClientValidation?: boolean;
    };
  3. Updated client authentication middleware to respect the allowFallbackClient flag, which is derived from the provider's skipLocalClientValidation setting.

  4. Fixed authorize handler to only create fallback clients when explicitly configured to do so.

  5. Added comprehensive documentation showing how to configure stateless proxy setups:

    const statelessProxyProvider = new ProxyOAuthServerProvider({
      // ... endpoints
      getClient: async (client_id) => undefined, // Stateless operation
      skipLocalClientValidation: true,
      skipLocalPkceValidation: true
    });

Design decisions:

  • Security by default: Validation is strict unless explicitly disabled
  • Explicit configuration: The intent to skip validation must be clearly stated
  • Granular control: PKCE and client validation can be configured independently
  • Backward compatibility: Proxy providers default to permissive behavior as they did before

This resolves the tension between supporting stateless proxy scenarios while maintaining security for standard OAuth implementations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant