Skip to content

Add validation for OAuth callback parameters #12

@koistya

Description

@koistya

Description

Add comprehensive validation for OAuth callback parameters to ensure security and correctness. Currently, the library accepts any parameters without validation, which could lead to security issues or unexpected behavior.

What needs to be done

  1. State parameter validation:

    • Verify state matches expected value
    • Prevent CSRF attacks
    • Handle missing state appropriately
  2. Required parameter checking:

    • Ensure either code or error is present
    • Validate parameter formats
    • Check for unexpected parameters
  3. Security validations:

    • Validate redirect URI matches expected
    • Check for parameter injection attempts
    • Validate parameter lengths
  4. Provider-specific validation:

    • Handle provider-specific parameter requirements
    • Validate known error codes

Why this matters

Parameter validation is critical for security:

  • CSRF Protection: State parameter prevents cross-site request forgery
  • Injection Prevention: Unvalidated parameters could lead to injection attacks
  • Data Integrity: Ensures the OAuth flow completes correctly
  • Error Handling: Proper validation provides better error messages

Without validation:

  • Security vulnerabilities
  • Difficult to debug issues
  • Inconsistent behavior across providers

Implementation considerations

⚠️ Note: This feature requires critical thinking during implementation. Consider:

  1. State management: How do we securely store and validate state? Should it be cryptographically signed?

  2. Backward compatibility: Adding validation might break existing integrations. Should it be opt-in initially?

  3. Alternative approach: Use a schema validation library (Zod, Yup) vs custom validation. What's the tradeoff?

  4. Security vs Flexibility: Too strict validation might break with provider updates. How do we balance this?

  5. Timing attacks: State comparison should be constant-time to prevent timing attacks

Suggested implementation

import { createHash, timingSafeEqual } from 'crypto';

interface ValidationOptions {
  expectedState?: string;
  allowedParameters?: string[];
  maxParameterLength?: number;
  validateState?: boolean;
  strictMode?: boolean; // Reject unknown parameters
}

class ParameterValidator {
  constructor(private options: ValidationOptions) {}
  
  validate(params: CallbackResult): ValidationResult {
    const errors: ValidationError[] = [];
    
    // Check for required parameters
    if (!params.code && !params.error) {
      errors.push({
        field: 'code/error',
        message: 'Either authorization code or error must be present'
      });
    }
    
    // Validate state if expected
    if (this.options.validateState && this.options.expectedState) {
      if (!params.state) {
        errors.push({
          field: 'state',
          message: 'State parameter is missing'
        });
      } else if (!this.timingSafeCompare(params.state, this.options.expectedState)) {
        errors.push({
          field: 'state',
          message: 'State parameter does not match expected value',
          severity: 'critical'
        });
      }
    }
    
    // Check parameter lengths
    for (const [key, value] of Object.entries(params)) {
      if (typeof value === 'string' && value.length > (this.options.maxParameterLength || 1024)) {
        errors.push({
          field: key,
          message: `Parameter exceeds maximum length of ${this.options.maxParameterLength}`
        });
      }
    }
    
    // Check for unexpected parameters in strict mode
    if (this.options.strictMode && this.options.allowedParameters) {
      for (const key of Object.keys(params)) {
        if (!this.options.allowedParameters.includes(key)) {
          errors.push({
            field: key,
            message: 'Unexpected parameter in callback',
            severity: 'warning'
          });
        }
      }
    }
    
    // Validate known error codes
    if (params.error) {
      const knownErrors = [
        'access_denied',
        'unauthorized_client',
        'invalid_request',
        'unsupported_response_type',
        'invalid_scope',
        'server_error',
        'temporarily_unavailable'
      ];
      
      if (!knownErrors.includes(params.error)) {
        errors.push({
          field: 'error',
          message: `Unknown error code: ${params.error}`,
          severity: 'warning'
        });
      }
    }
    
    return {
      valid: errors.filter(e => e.severity === 'critical').length === 0,
      errors,
      sanitized: this.sanitizeParameters(params)
    };
  }
  
  private timingSafeCompare(a: string, b: string): boolean {
    const bufferA = Buffer.from(a);
    const bufferB = Buffer.from(b);
    
    if (bufferA.length !== bufferB.length) {
      return false;
    }
    
    return timingSafeEqual(bufferA, bufferB);
  }
  
  private sanitizeParameters(params: CallbackResult): CallbackResult {
    const sanitized: CallbackResult = {};
    
    for (const [key, value] of Object.entries(params)) {
      if (typeof value === 'string') {
        // Remove any non-printable characters
        sanitized[key] = value.replace(/[^\x20-\x7E]/g, '');
      } else {
        sanitized[key] = value;
      }
    }
    
    return sanitized;
  }
}

// Integration with getAuthCode
export async function getAuthCode(
  input: GetAuthCodeOptions | string
): Promise<CallbackResult> {
  const options = typeof input === 'string' 
    ? { authorizationUrl: input } 
    : input;
  
  // Generate state if validation is enabled
  const state = options.validateState 
    ? options.state || crypto.randomUUID()
    : undefined;
  
  if (state) {
    // Add state to authorization URL
    const url = new URL(options.authorizationUrl);
    url.searchParams.set('state', state);
    options.authorizationUrl = url.toString();
  }
  
  // ... existing code ...
  
  const result = await server.waitForCallback(callbackPath, timeout);
  
  // Validate parameters
  if (options.validation) {
    const validator = new ParameterValidator({
      ...options.validation,
      expectedState: state
    });
    
    const validationResult = validator.validate(result);
    
    if (!validationResult.valid) {
      throw new ValidationError(
        'OAuth callback parameter validation failed',
        validationResult.errors
      );
    }
    
    return validationResult.sanitized;
  }
  
  return result;
}

Usage examples

// Basic validation with state
await getAuthCode({
  authorizationUrl: "https://github.com/login/oauth/authorize",
  validateState: true, // Auto-generates and validates state
  validation: {
    strictMode: false, // Allow unknown parameters
    maxParameterLength: 2048
  }
});

// Custom state value
await getAuthCode({
  authorizationUrl: "https://github.com/login/oauth/authorize",
  state: "my-custom-state-123",
  validateState: true,
  validation: {
    allowedParameters: ['code', 'state', 'error', 'error_description']
  }
});

// Strict validation for production
await getAuthCode({
  authorizationUrl: "https://oauth.example.com/authorize",
  validateState: true,
  validation: {
    strictMode: true,
    allowedParameters: ['code', 'state'],
    maxParameterLength: 512
  }
});

Testing requirements

  • Test CSRF protection with state validation
  • Test timing-safe string comparison
  • Test parameter length limits
  • Test sanitization of malicious input
  • Test with various OAuth providers
  • Test backward compatibility

Skills required

  • TypeScript
  • OAuth 2.0 security best practices
  • Cryptography (timing attacks, secure comparison)
  • Input validation patterns
  • Security testing

Difficulty

Easy to Medium - Critical security feature requiring careful implementation

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions