fix: add environment variable substitution in configuration management

Author: akshay5995

CLAUDE.md

@@ -486,6 +486,15 @@ oauth_providers:
     client_secret: "${GOOGLE_CLIENT_SECRET}"
     scopes: ["openid", "email", "profile"]
   
+  # For Okta (alternative to Google):
+  # okta:
+  #   client_id: "${OKTA_CLIENT_ID}"
+  #   client_secret: "${OKTA_CLIENT_SECRET}"
+  #   scopes: ["openid", "email", "profile"]
+  #   authorization_url: "https://your-domain.okta.com/oauth2/default/v1/authorize"
+  #   token_url: "https://your-domain.okta.com/oauth2/default/v1/token"
+  #   userinfo_url: "https://your-domain.okta.com/oauth2/default/v1/userinfo"
+  
   # For custom providers:
   # custom:
   #   authorization_url: "https://provider.com/oauth/authorize"

src/config/config.py

@@ -1,6 +1,7 @@
 """Configuration management for MCP OAuth Gateway."""
 
 import os
+import re
 from dataclasses import dataclass, field
 from pathlib import Path
 from typing import Any, Dict, List, Optional
@@ -165,6 +166,36 @@ def _find_config_file(self) -> str:
 
         return "config.yaml"  # Default
 
+    def _substitute_env_vars(self, content: str) -> str:
+        """Substitute environment variables in configuration content.
+
+        Supports formats:
+        - ${VAR_NAME} - Required environment variable (raises error if not set)
+        - ${VAR_NAME:-default_value} - Optional with default value
+        """
+
+        def replace_env_var(match):
+            var_expr = match.group(1)
+
+            if ":-" in var_expr:
+                # Format: ${VAR_NAME:-default_value}
+                var_name, default_value = var_expr.split(":-", 1)
+                return os.getenv(var_name, default_value)
+            else:
+                # Format: ${VAR_NAME}
+                var_name = var_expr
+                value = os.getenv(var_name)
+                if value is None:
+                    raise ValueError(
+                        f"Environment variable '{var_name}' is required but not set. "
+                        f"Please set {var_name} or use ${{{var_name}:-default}} format for optional variables."
+                    )
+                return value
+
+        # Pattern to match ${VAR_NAME} and ${VAR_NAME:-default}
+        pattern = r"\$\{([^}]+)\}"
+        return re.sub(pattern, replace_env_var, content)
+
     def load_config(self) -> GatewayConfig:
         """Load configuration from file."""
         config_file = Path(self.config_path)
@@ -176,7 +207,13 @@ def load_config(self) -> GatewayConfig:
             return self.config
 
         with open(config_file) as f:
-            data = yaml.safe_load(f) or {}
+            raw_content = f.read()
+
+        # Substitute environment variables in the raw content
+        substituted_content = self._substitute_env_vars(raw_content)
+
+        # Parse the substituted YAML content
+        data = yaml.safe_load(substituted_content) or {}
 
         # Parse OAuth providers with single provider validation
         oauth_providers = {}
@@ -258,11 +295,11 @@ def load_config(self) -> GatewayConfig:
         redis_data = storage_data.get("redis", {})
         redis_config = RedisStorageConfig(
             host=redis_data.get("host", "localhost"),
-            port=redis_data.get("port", 6379),
+            port=int(redis_data.get("port", 6379)),
             password=redis_data.get("password"),
-            db=redis_data.get("db", 0),
+            db=int(redis_data.get("db", 0)),
             ssl=redis_data.get("ssl", False),
-            max_connections=redis_data.get("max_connections", 10),
+            max_connections=int(redis_data.get("max_connections", 10)),
         )
 
         # Parse Vault configuration

tests/config/test_config.py

@@ -4,6 +4,8 @@
 import tempfile
 from unittest.mock import patch
 
+import pytest
+
 from src.config.config import (
     ConfigManager,
     CorsConfig,
@@ -365,6 +367,197 @@ def test_get_provider_not_exists(self):
         provider = config_manager.get_provider("nonexistent_provider")
         assert provider is None
 
+    @patch.dict(
+        os.environ,
+        {
+            "GITHUB_CLIENT_ID": "github_test_id",
+            "GITHUB_CLIENT_SECRET": "github_test_secret",
+            "REDIS_HOST": "redis.test.com",
+            "REDIS_PORT": "6380",
+            "VAULT_TOKEN": "vault_test_token",
+        },
+    )
+    def test_env_var_substitution_required_vars(self):
+        """Test environment variable substitution for required variables."""
+        yaml_content = """
+host: "127.0.0.1"
+port: 8080
+issuer: "http://localhost:8080"
+session_secret: "test-secret"
+
+storage:
+  type: "redis"
+  redis:
+    host: "${REDIS_HOST}"
+    port: "${REDIS_PORT}"
+  vault:
+    token: "${VAULT_TOKEN}"
+
+oauth_providers:
+  github:
+    client_id: "${GITHUB_CLIENT_ID}"
+    client_secret: "${GITHUB_CLIENT_SECRET}"
+    scopes: ["user:email"]
+
+mcp_services:
+  calculator:
+    name: "Calculator Service"
+    url: "http://localhost:3001/mcp"
+    oauth_provider: "github"
+    auth_required: true
+"""
+
+        with tempfile.NamedTemporaryFile(mode="w", suffix=".yaml", delete=False) as f:
+            f.write(yaml_content)
+            config_path = f.name
+
+        try:
+            config_manager = ConfigManager(config_path)
+            config = config_manager.load_config()
+
+            # Check that environment variables were substituted
+            github_provider = config.oauth_providers["github"]
+            assert github_provider.client_id == "github_test_id"
+            assert github_provider.client_secret == "github_test_secret"
+
+            # Check storage config
+            assert config.storage.redis.host == "redis.test.com"
+            assert config.storage.redis.port == 6380  # Should be converted to int
+            assert config.storage.vault.token == "vault_test_token"
+
+        finally:
+            os.unlink(config_path)
+
+    @patch.dict(os.environ, {"REDIS_HOST": "redis.prod.com"}, clear=False)
+    def test_env_var_substitution_with_defaults(self):
+        """Test environment variable substitution with default values."""
+        yaml_content = """
+host: "127.0.0.1"
+port: 8080
+
+storage:
+  type: "redis"
+  redis:
+    host: "${REDIS_HOST:-localhost}"
+    port: "${REDIS_PORT:-6379}"
+    password: "${REDIS_PASSWORD:-}"
+    db: "${REDIS_DB:-0}"
+
+oauth_providers:
+  github:
+    client_id: "${GITHUB_CLIENT_ID:-default_client_id}"
+    client_secret: "${GITHUB_CLIENT_SECRET:-default_secret}"
+"""
+
+        with tempfile.NamedTemporaryFile(mode="w", suffix=".yaml", delete=False) as f:
+            f.write(yaml_content)
+            config_path = f.name
+
+        try:
+            config_manager = ConfigManager(config_path)
+            config = config_manager.load_config()
+
+            # Check that environment variables were used
+            assert config.storage.redis.host == "redis.prod.com"  # From env
+            assert config.storage.redis.port == 6379  # Default value, converted to int
+            assert config.storage.redis.password == ""  # Default empty string
+            assert config.storage.redis.db == 0  # Default value
+
+            # Check OAuth provider defaults
+            github_provider = config.oauth_providers["github"]
+            assert github_provider.client_id == "default_client_id"
+            assert github_provider.client_secret == "default_secret"
+
+        finally:
+            os.unlink(config_path)
+
+    def test_env_var_substitution_missing_required(self):
+        """Test that missing required environment variables raise an error."""
+        yaml_content = """
+oauth_providers:
+  github:
+    client_id: "${MISSING_CLIENT_ID}"
+    client_secret: "static_secret"
+"""
+
+        with tempfile.NamedTemporaryFile(mode="w", suffix=".yaml", delete=False) as f:
+            f.write(yaml_content)
+            config_path = f.name
+
+        try:
+            config_manager = ConfigManager(config_path)
+
+            with pytest.raises(
+                ValueError,
+                match="Environment variable 'MISSING_CLIENT_ID' is required but not set",
+            ):
+                config_manager.load_config()
+
+        finally:
+            os.unlink(config_path)
+
+    def test_env_var_substitution_mixed_formats(self):
+        """Test mixed environment variable formats in the same config."""
+        yaml_content = """
+host: "${HOST:-0.0.0.0}"
+port: "${PORT}"
+issuer: "${ISSUER:-http://localhost:8080}"
+session_secret: "${SESSION_SECRET}"
+debug: "${DEBUG:-false}"
+"""
+
+        with patch.dict(os.environ, {"PORT": "9000", "SESSION_SECRET": "secret123"}):
+            with tempfile.NamedTemporaryFile(
+                mode="w", suffix=".yaml", delete=False
+            ) as f:
+                f.write(yaml_content)
+                config_path = f.name
+
+            try:
+                config_manager = ConfigManager(config_path)
+                config = config_manager.load_config()
+
+                # Check mixed substitution results
+                assert config.host == "0.0.0.0"  # Default value
+                assert config.port == 9000  # From environment
+                assert config.issuer == "http://localhost:8080"  # Default value
+                assert config.session_secret == "secret123"  # From environment
+                assert config.debug is False  # Default value converted to boolean
+
+            finally:
+                os.unlink(config_path)
+
+    def test_substitute_env_vars_method_direct(self):
+        """Test the _substitute_env_vars method directly."""
+        config_manager = ConfigManager()
+
+        # Test required variable
+        with patch.dict(os.environ, {"TEST_VAR": "test_value"}):
+            result = config_manager._substitute_env_vars("value: ${TEST_VAR}")
+            assert result == "value: test_value"
+
+        # Test variable with default
+        result = config_manager._substitute_env_vars("value: ${MISSING_VAR:-default}")
+        assert result == "value: default"
+
+        # Test missing required variable
+        with pytest.raises(
+            ValueError,
+            match="Environment variable 'MISSING_REQUIRED' is required but not set",
+        ):
+            config_manager._substitute_env_vars("value: ${MISSING_REQUIRED}")
+
+        # Test multiple variables
+        with patch.dict(os.environ, {"VAR1": "value1", "VAR2": "value2"}):
+            result = config_manager._substitute_env_vars("${VAR1} and ${VAR2}")
+            assert result == "value1 and value2"
+
+        # Test variables with complex defaults
+        result = config_manager._substitute_env_vars(
+            "${VAR:-default with spaces and : colons}"
+        )
+        assert result == "default with spaces and : colons"
+
 
 class TestDataClasses:
     """Test cases for configuration data classes."""