diff --git a/.env.example b/.env.example index f39b0e1..2077acc 100644 --- a/.env.example +++ b/.env.example @@ -22,3 +22,22 @@ ALLOWED_ORIGINS=["http://localhost:8000","http://localhost:6274"] # Defaults are 120 requests per 1 minute # RATE_LIMIT_MAX_REQUESTS=120 # RATE_LIMIT_WINDOW_MINUTES=1 + +# Optional: keep OAuth state (client registrations, users' tokens) in Redis so +# sign-ins survive container recreation, scale-to-zero and multiple replicas. +# Unset = encrypted file store inside the container (lost on recreation). +# rediss:// = TLS (required by Azure Redis); the path is the DB index. +# REDIS_URL contains a credential — keep it in a secret, not in source control. +# Enabling it (or setting JWT_SIGNING_KEY) signs every user out once. +# Docs: https://gofastmcp.com/servers/auth/oauth-proxy +# REDIS_URL=redis://localhost:6379/1 + +# Fixed token-signing secret (32+ characters), so rotating CLIENT_SECRET doesn't +# sign everyone out. Same value on every replica. Default: derived from +# CLIENT_SECRET. Left empty, it counts as unset. +# JWT_SIGNING_KEY= + +# Fernet key for the Redis store. Requires REDIS_URL. Default: derived like +# FastMCP's own store key — set it explicitly in production. Generate: +# python -c "from cryptography.fernet import Fernet; print(Fernet.generate_key().decode())" +# STORAGE_ENCRYPTION_KEY= diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 1ec6a0b..d80b9ff 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -18,7 +18,7 @@ MCP server for Microsoft Planner built with FastMCP and Microsoft Graph SDK. Aut - `src/` — application package - `src/server.py` — FastMCP app, middleware, route mounting - `src/config.py` — `Settings` via `pydantic_settings.BaseSettings` - - `src/auth_provider.py` — Azure OAuth provider + - `src/auth_provider.py` — Azure OAuth provider; optional encrypted Redis store for OAuth state (`REDIS_URL`) - `src/deps.py` — shared dependency helpers (e.g. `get_optional_context()`) - `src/graph_client_manager.py` — singleton `GraphClientManager` with per-user OBO clients - `src/types.py` — shared structural types (e.g. `CollectionRequestBuilder` Protocol) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 53fada6..b10cdd8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -13,6 +13,24 @@ jobs: build-and-test: runs-on: ubuntu-24.04 + # Real Redis for tests/test_redis_storage_integration.py, which skips + # without REDIS_TEST_URL. A mock cannot show OAuth state surviving a + # restart or Redis applying FastMCP's TTLs. + # Docs: https://docs.github.com/en/actions/use-cases-and-examples/using-containerized-services/creating-redis-service-containers + services: + redis: + image: redis:7-alpine + ports: + - 6379:6379 + options: >- + --health-cmd "redis-cli ping" + --health-interval 5s + --health-timeout 3s + --health-retries 10 + + env: + REDIS_TEST_URL: redis://localhost:6379/15 + steps: - uses: actions/checkout@v4 diff --git a/README.md b/README.md index 2795e21..ff80f12 100644 --- a/README.md +++ b/README.md @@ -152,12 +152,33 @@ ALLOWED_ORIGINS=["http://localhost:8000","http://localhost:6274"] # Defaults are 120 requests per 1 minute # RATE_LIMIT_MAX_REQUESTS=120 # RATE_LIMIT_WINDOW_MINUTES=1 + +# Optional: keep sign-ins in Redis so they survive restarts (see below) +# REDIS_URL=rediss://:@.redis.cache.windows.net:6380/0 +# JWT_SIGNING_KEY= +# STORAGE_ENCRYPTION_KEY= ``` `REQUIRE_AUTHORIZATION_CONSENT` controls whether FastMCP prompts users to explicitly approve each new MCP client. Keep this `true` (default) in production to prevent confused-deputy attacks; set it to `false` only during local development with throwaway clients. `RATE_LIMIT_MAX_REQUESTS` and `RATE_LIMIT_WINDOW_MINUTES` are optional and should stay commented out unless you need to override the defaults for your environment. +### Keeping sign-ins across restarts (Redis) + +By default the server keeps OAuth state — client registrations and users' Entra tokens — in an encrypted file store inside the container. That state is lost when the container is recreated or an Azure Container App scales to zero, and each replica has its own copy. Clients then fail to refresh with `401 invalid_client` and users must reconnect. + +Set `REDIS_URL` when you run more than one replica, scale to zero, or recreate containers: + +| Variable | Default | Purpose | +|---|---|---| +| `REDIS_URL` | unset (file store) | `redis://` or `rediss://` (TLS — required by Azure Redis). The path is the DB index (e.g. `/1`). Percent-encode the password if it contains `/` (Azure access keys can): `python -c "import urllib.parse; print(urllib.parse.quote('', safe=''))"`. A non-clustered Redis is required (Redis Cluster supports only DB 0 and is not supported). **Contains a credential: keep it in a secret.** | +| `JWT_SIGNING_KEY` | derived from `CLIENT_SECRET` | Fixed secret (at least 32 characters) for signing the server's tokens. Set it so rotating the Entra client secret no longer signs every user out. Use the same value on every replica. | +| `STORAGE_ENCRYPTION_KEY` | derived like FastMCP's default | Fernet key encrypting the Redis store (generate: `python -c "from cryptography.fernet import Fernet; print(Fernet.generate_key().decode())"`). Requires `REDIS_URL` — the server refuses to start without it. **Set it explicitly in production**: the derived default follows FastMCP's internal key scheme, so it is only as stable as that scheme. | + +Everything written to Redis is Fernet-encrypted. Tokens and sign-in transactions expire with FastMCP's lifetimes (refresh tokens 30 days, transactions minutes), but **client registrations never expire** — one is added per MCP client registration, and entries written under an old key stay behind as unreadable garbage. Give the store its own DB index (or its own Redis) rather than sharing a DB with a cache, and use an eviction policy that does not drop it silently (`noeviction`, or `volatile-*` with enough memory). An invalid `REDIS_URL`, `JWT_SIGNING_KEY` or `STORAGE_ENCRYPTION_KEY` fails at startup; an empty value (`JWT_SIGNING_KEY=`) counts as unset. If Redis is unreachable at runtime, OAuth requests and tool calls fail with `500` within about 4 seconds and recover on their own once Redis is back. + +> **Enabling these settings signs everyone out once.** Turning on `REDIS_URL`, setting `JWT_SIGNING_KEY`, or changing either key later invalidates existing sign-ins: connected clients get a `401` on their next call or refresh and must reconnect once. Existing file-store state is not migrated. Plan the switch. + ## Running the Server ### Local @@ -187,6 +208,7 @@ docker run --rm -i \ This starts the MCP server on port 8000. To override rate limits, also pass `-e RATE_LIMIT_MAX_REQUESTS=` and `-e RATE_LIMIT_WINDOW_MINUTES=` (defaults: `120` and `1`). +To keep sign-ins across container recreation, also pass `-e REDIS_URL=...` (see [Keeping sign-ins across restarts](#keeping-sign-ins-across-restarts-redis)). ### Running with Docker compose @@ -469,7 +491,7 @@ This section covers building, testing, and contributing to the project. src/ ├── server.py # FastMCP app, middleware, route mounting ├── config.py # Settings via pydantic-settings -├── auth_provider.py # Azure OAuth provider (OBO flow) +├── auth_provider.py # Azure OAuth provider (OBO flow) + optional Redis state store ├── deps.py # Shared dependency helpers ├── graph_client_manager.py # Singleton GraphClientManager with per-user OBO clients ├── telemetry.py # OpenTelemetry setup @@ -481,14 +503,19 @@ src/ ├── groups.py # list_my_groups ├── plans.py # plan tools + list_plan_categories ├── tasks.py # task tools + list_task_fields - └── buckets.py # bucket tools + ├── buckets.py # bucket tools + └── users.py # list_users tests/ ├── conftest.py +├── test_auth_provider.py ├── test_buckets_tool.py +├── test_config.py ├── test_groups_tool.py ├── test_planner_service.py ├── test_plans_tool.py -└── test_tasks_tool.py +├── test_redis_storage_integration.py # needs a real Redis (REDIS_TEST_URL) +├── test_tasks_tool.py +└── test_users_tool.py ``` ### Installing Dev Dependencies @@ -509,13 +536,20 @@ With verbose output: uv run pytest -v ``` +`tests/test_redis_storage_integration.py` runs against a real Redis and is skipped unless `REDIS_TEST_URL` is set (CI provides a `redis:7-alpine` service). Point it at a scratch DB — the tests write and delete their own keys: + +```bash +docker run -d --rm --name redis-test -p 6379:6379 redis:7-alpine +REDIS_TEST_URL=redis://localhost:6379/15 uv run pytest tests/test_redis_storage_integration.py -v +``` + ### Architecture The server is built with [FastMCP](https://gofastmcp.com) and uses these key patterns: **Server composition** — Tools are split into five domain routers (`me`, `groups`, `plans`, `tasks`, `buckets`), each a standalone `FastMCP` instance [mounted](https://gofastmcp.com/servers/composition) on the main app. This keeps each domain's tools, imports, and tests isolated. -**Authentication** — The server uses FastMCP's [`OAuthProxy`](https://gofastmcp.com/servers/auth/oauth-proxy) pattern via a custom `AzureProvider`. Azure Entra ID does not support Dynamic Client Registration (DCR), so the provider acts as a DCR-compliant proxy facing MCP clients while using the pre-registered app credentials with Azure. When a tool call arrives, the server exchanges the MCP session token for a Microsoft Graph token via the [On-Behalf-Of flow](https://learn.microsoft.com/en-us/entra/identity-platform/v2-oauth2-on-behalf-of-flow), scoped to `Tasks.ReadWrite` and `User.Read`. +**Authentication** — The server uses FastMCP's [`OAuthProxy`](https://gofastmcp.com/servers/auth/oauth-proxy) pattern via a custom `AzureProvider`. Azure Entra ID does not support Dynamic Client Registration (DCR), so the provider acts as a DCR-compliant proxy facing MCP clients while using the pre-registered app credentials with Azure. When a tool call arrives, the server exchanges the MCP session token for a Microsoft Graph token via the [On-Behalf-Of flow](https://learn.microsoft.com/en-us/entra/identity-platform/v2-oauth2-on-behalf-of-flow), scoped to `Tasks.ReadWrite`, `User.Read` and `User.ReadBasic.All`. The proxy's state (client registrations, users' Entra tokens) lives in FastMCP's encrypted file store by default, or in an encrypted Redis store when `REDIS_URL` is set — see [Keeping sign-ins across restarts](#keeping-sign-ins-across-restarts-redis). **Middleware** — Five [built-in middleware](https://gofastmcp.com/servers/middleware#built-in-middleware) layers are stacked on the server (outermost first): diff --git a/pyproject.toml b/pyproject.toml index 3015ed7..3fd70e8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,14 +1,17 @@ [project] name = "microsoft-planner-mcp" -version = "0.3.3" +version = "0.4.0" description = "MCP server for Microsoft Planner via Microsoft Graph" readme = "README.md" license = "MIT" requires-python = ">=3.12" dependencies = [ - "fastmcp[apps]>=3.2.3", + # <4: src/auth_provider.py mirrors FastMCP's store-key derivation (salts, + # KDF); a major release could change it and sign every Redis user out. + "fastmcp[apps]>=3.2.3,<4", "msgraph-sdk>=1.55.0", "odata-v4-query>=0.4.1", + "py-key-value-aio[redis]>=0.4.4,<0.5.0", "pydantic-settings>=2.13.1", ] @@ -40,4 +43,6 @@ filterwarnings = [ "ignore::DeprecationWarning:kiota_abstractions", # msgraph-sdk generated request-configuration subclasses fire warn() in class bodies (SDK bug) "ignore::DeprecationWarning:msgraph", + # py-key-value-aio's RedisStore calls redis-py's deprecated setex() (library bug) + "ignore:Call to deprecated setex:DeprecationWarning:key_value", ] diff --git a/src/auth_provider.py b/src/auth_provider.py index 1699809..da62004 100644 --- a/src/auth_provider.py +++ b/src/auth_provider.py @@ -6,10 +6,81 @@ # The resulting session token is validated by FastMCP on every tool call, so # no tool executes without a verified Entra ID identity. # Docs: https://gofastmcp.com/servers/auth/authentication +from cryptography.fernet import Fernet +from fastmcp.server.auth.jwt_issuer import derive_jwt_key from fastmcp.server.auth.providers.azure import AzureProvider +from key_value.aio.stores.redis import RedisStore +from key_value.aio.wrappers.encryption import FernetEncryptionWrapper +from redis.asyncio import Redis +from redis.asyncio.retry import Retry +from redis.backoff import ExponentialWithJitterBackoff from .config import settings + +_JWT_SALT = "fastmcp-jwt-signing-key" + +# Derived once and handed to FastMCP as bytes, which it uses verbatim +# (oauth_proxy/proxy.py). Passing the string would make FastMCP re-run the +# same 1M-iteration PBKDF2 at import — slower cold starts on scale-to-zero. +_jwt_signing_key: bytes | None = ( + derive_jwt_key( + low_entropy_material=settings.JWT_SIGNING_KEY.get_secret_value(), salt=_JWT_SALT + ) + if settings.JWT_SIGNING_KEY is not None + else None +) + + +def _redis_client_storage() -> FernetEncryptionWrapper | None: + """Encrypted Redis store for OAuth proxy state, or None for FastMCP's default. + + The default is a file store inside the container: container recreation, + scale-to-zero or a second replica loses every client registration and + token, and refreshes fail with 401 invalid_client. The store holds users' + Entra refresh tokens, so it is always Fernet-encrypted. + Docs: https://gofastmcp.com/servers/auth/oauth-proxy + """ + if settings.REDIS_URL is None: + return None + + if settings.STORAGE_ENCRYPTION_KEY is not None: + key = settings.STORAGE_ENCRYPTION_KEY.get_secret_value().encode() + else: + # Mirror FastMCP's default-store key derivation (oauth_proxy/proxy.py): + # storage key from the JWT signing key, itself from JWT_SIGNING_KEY or + # CLIENT_SECRET. Without this, no key scheme would match FastMCP's. + jwt_key = _jwt_signing_key or derive_jwt_key( + high_entropy_material=settings.CLIENT_SECRET, salt=_JWT_SALT + ) + key = derive_jwt_key( + high_entropy_material=jwt_key.decode(), salt="fastmcp-storage-encryption-key" + ) + + return FernetEncryptionWrapper( + # RedisStore(url=...) rebuilds the client from host/port/db/password and + # drops TLS, so a rediss:// URL (Azure Redis) would connect in plaintext + # and fail. redis-py's from_url honours the scheme. + # Explicit outage budget: every tool call reads token state from Redis, + # so a hung Redis must fail fast (500, the client retries) — measured + # ~4 s per request with this budget. One retry still covers a stale + # pooled connection (e.g. Azure dropping idle sockets). + key_value=RedisStore( + client=Redis.from_url( + str(settings.REDIS_URL), + decode_responses=True, + socket_connect_timeout=2, + socket_timeout=2, + retry=Retry(ExponentialWithJitterBackoff(base=0.1, cap=0.5), retries=1), + ) + ), + fernet=Fernet(key), + # A rotated key turns old entries into cache misses (users reconnect) + # instead of crashing the token endpoint — FastMCP's default behaviour. + raise_on_decryption_error=False, + ) + + auth = AzureProvider( client_id=settings.CLIENT_ID, client_secret=settings.CLIENT_SECRET, @@ -40,4 +111,7 @@ # to disable it. Docs: # https://gofastmcp.com/servers/auth/oauth-proxy#param-require-authorization-consent require_authorization_consent=settings.REQUIRE_AUTHORIZATION_CONSENT, + # Both None keeps FastMCP's defaults (file store, key from CLIENT_SECRET). + client_storage=_redis_client_storage(), + jwt_signing_key=_jwt_signing_key, ) diff --git a/src/config.py b/src/config.py index 77b9ce5..a64165f 100644 --- a/src/config.py +++ b/src/config.py @@ -1,6 +1,14 @@ from __future__ import annotations -from pydantic import PositiveInt +from cryptography.fernet import Fernet +from pydantic import ( + Field, + PositiveInt, + RedisDsn, + SecretStr, + field_validator, + model_validator, +) from pydantic_settings import BaseSettings, SettingsConfigDict @@ -9,7 +17,20 @@ class Settings(BaseSettings): # environment variables. Without env_file every required value must be # exported in the shell, which is impractical for local development. # Docs: https://docs.pydantic.dev/latest/concepts/pydantic_settings/#dotenv-env-support - model_config = SettingsConfigDict(env_file=".env", env_file_encoding="utf-8") + # hide_input_in_errors: REDIS_URL carries the Redis password, and pydantic + # echoes the raw input in validation errors by default — a malformed URL + # would print the credential to the startup log. + # Docs: https://docs.pydantic.dev/latest/api/config/#pydantic.config.ConfigDict.hide_input_in_errors + # env_ignore_empty: `JWT_SIGNING_KEY=` (an uncommented .env.example line or + # an unset compose ${VAR}) must mean "unset", not SecretStr('') — an empty + # signing key would derive a publicly computable Redis encryption key. + # Docs: https://docs.pydantic.dev/latest/concepts/pydantic_settings/#parsing-environment-variable-values + model_config = SettingsConfigDict( + env_file=".env", + env_file_encoding="utf-8", + hide_input_in_errors=True, + env_ignore_empty=True, + ) # Azure Entra ID app registration credentials. Used by AzureProvider # (auth_provider.py) to drive the OAuth authorization code flow and by @@ -47,6 +68,62 @@ class Settings(BaseSettings): RATE_LIMIT_MAX_REQUESTS: PositiveInt = 120 RATE_LIMIT_WINDOW_MINUTES: PositiveInt = 1 + # Where the OAuth proxy keeps client registrations and user tokens. Unset, + # FastMCP uses an encrypted file store inside the container, which is lost + # on container recreation / scale-to-zero and not shared between replicas + # — every client then fails token refresh with 401 invalid_client. + # redis:// or rediss:// (TLS, required by Azure Redis); the path selects + # the DB index so the store can share a Redis with other services. + # Docs: https://gofastmcp.com/servers/auth/oauth-proxy + # repr=False: the URL carries the Redis password; keep it out of reprs/logs. + REDIS_URL: RedisDsn | None = Field(default=None, repr=False) + + # Fixed secret for signing FastMCP's tokens. Unset, it is derived from + # CLIENT_SECRET, so rotating the Entra secret logs every user out. Also the + # root of the derived Redis encryption key, so a short one is refused. + JWT_SIGNING_KEY: SecretStr | None = Field(default=None, min_length=32) + + # Fernet key encrypting the Redis store. Unset, it is derived the way + # FastMCP derives its default store key. Only meaningful with REDIS_URL. + # Docs: https://cryptography.io/en/latest/fernet/ + STORAGE_ENCRYPTION_KEY: SecretStr | None = None + + @field_validator("REDIS_URL") + @classmethod + def _numeric_db_path(cls, value: RedisDsn | None) -> RedisDsn | None: + # redis-py ignores a non-numeric path and silently uses DB 0. + if value is not None and value.path not in (None, "", "/"): + if not value.path.lstrip("/").isdigit(): + raise ValueError("REDIS_URL path must be a DB index, e.g. /1") + return value + + @field_validator("JWT_SIGNING_KEY") + @classmethod + def _non_blank_signing_key(cls, value: SecretStr | None) -> SecretStr | None: + # Whitespace passes min_length but is as predictable as an empty key. + if value is not None and not value.get_secret_value().strip(): + raise ValueError("JWT_SIGNING_KEY must not be blank") + return value + + @field_validator("STORAGE_ENCRYPTION_KEY") + @classmethod + def _valid_fernet_key(cls, value: SecretStr | None) -> SecretStr | None: + if value is not None: + try: + Fernet(value.get_secret_value()) + except ValueError as exc: + raise ValueError( + "STORAGE_ENCRYPTION_KEY must be a Fernet key " + "(Fernet.generate_key(): 32 url-safe base64-encoded bytes)" + ) from exc + return value + + @model_validator(mode="after") + def _encryption_key_needs_redis(self) -> Settings: + if self.STORAGE_ENCRYPTION_KEY is not None and self.REDIS_URL is None: + raise ValueError("STORAGE_ENCRYPTION_KEY requires REDIS_URL") + return self + # pydantic-settings reads CLIENT_ID, CLIENT_SECRET, and TENANT_ID from the # environment / .env at runtime, so there are no missing arguments even though diff --git a/src/server.py b/src/server.py index 7b5aba8..6d6707b 100644 --- a/src/server.py +++ b/src/server.py @@ -44,7 +44,7 @@ "Authenticate with your Microsoft account to read and manage Planner tasks." ), mask_error_details=True, - version="0.3.3", + version="0.4.0", # Forward ctx.info() / ctx.debug() log messages to MCP clients at INFO # level and above. Without this, tool-level ctx.info() calls are silently # suppressed even though the tools emit them. Clients (Claude, MCP diff --git a/tests/test_auth_provider.py b/tests/test_auth_provider.py index bfabd64..909185e 100644 --- a/tests/test_auth_provider.py +++ b/tests/test_auth_provider.py @@ -6,6 +6,12 @@ import sys from unittest.mock import patch +import pytest +from cryptography.fernet import Fernet +from fastmcp.server.auth.jwt_issuer import derive_jwt_key +from key_value.aio.stores.redis import RedisStore +from key_value.aio.wrappers.encryption import FernetEncryptionWrapper + from src import config as config_module MODULE = "src.auth_provider" @@ -14,6 +20,39 @@ # already-created provider and miss whether new settings are forwarded. # Docs: https://docs.python.org/3/library/importlib.html#importlib.reload +FERNET_KEY = "fXpQ0Ul6ZJ8fKk5q8D0v0b7n7cK4l9sQ3m2a1b0c9d8=" +STORAGE_ENV = ("REDIS_URL", "JWT_SIGNING_KEY", "STORAGE_ENCRYPTION_KEY") + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def make_provider_kwargs(monkeypatch, **env): + """Reload src.auth_provider under the given env; return AzureProvider's kwargs.""" + for name in STORAGE_ENV: + monkeypatch.delenv(name, raising=False) + monkeypatch.delenv(name.lower(), raising=False) + for name, value in env.items(): + monkeypatch.setenv(name, value) + + settings = config_module.Settings(_env_file=None) + original_settings = config_module.settings + # Import BEFORE patching: a first import_module inside the patch would + # execute the module, and the reload execute it again — two calls. + module = importlib.import_module(MODULE) + try: + with patch.object(config_module, "settings", settings), patch( + "fastmcp.server.auth.providers.azure.AzureProvider" + ) as mock_provider: + importlib.reload(module) + mock_provider.assert_called_once() + return mock_provider.call_args.kwargs + finally: + config_module.settings = original_settings + importlib.reload(sys.modules[MODULE]) + # --------------------------------------------------------------------------- # Tests: settings @@ -54,24 +93,111 @@ def test_settings_reads_lowercase_require_authorization_consent_env(monkeypatch) def test_auth_provider_passes_configured_require_authorization_consent(monkeypatch): monkeypatch.delenv("REQUIRE_AUTHORIZATION_CONSENT", raising=False) - monkeypatch.setenv("require_authorization_consent", "false") - settings = config_module.Settings(_env_file=None) - original_settings = config_module.settings - existing_module = sys.modules.get(MODULE) + kwargs = make_provider_kwargs(monkeypatch, require_authorization_consent="false") - try: - with patch.object(config_module, "settings", settings), patch( - "fastmcp.server.auth.providers.azure.AzureProvider" - ) as mock_provider: - if existing_module is None: - importlib.import_module(MODULE) - else: - importlib.reload(existing_module) + assert kwargs["require_authorization_consent"] is False - mock_provider.assert_called_once() - assert mock_provider.call_args.kwargs["require_authorization_consent"] is False - finally: - config_module.settings = original_settings - if MODULE in sys.modules: - importlib.reload(sys.modules[MODULE]) + +# --------------------------------------------------------------------------- +# Tests: OAuth proxy state storage wiring +# --------------------------------------------------------------------------- + + +def test_no_redis_url_keeps_fastmcp_default_store(monkeypatch): + # client_storage=None / jwt_signing_key=None is exactly today's build: + # FastMCP's encrypted file store and a key derived from CLIENT_SECRET. + kwargs = make_provider_kwargs(monkeypatch) + + assert kwargs["client_storage"] is None + assert kwargs["jwt_signing_key"] is None + + +@pytest.mark.parametrize( + "url,connection_class,db", + [ + ("redis://localhost:6379/1", "Connection", 1), + ("rediss://:pw@example.redis.cache.windows.net:6380/2", "SSLConnection", 2), + ], + ids=["redis-plain", "rediss-tls"], +) +def test_redis_url_wires_encrypted_redis_store(monkeypatch, url, connection_class, db): + # RedisStore(url=...) rebuilds the client from host/port/db/password and + # drops TLS, so a rediss:// URL must still yield an SSLConnection. + kwargs = make_provider_kwargs(monkeypatch, REDIS_URL=url) + + storage = kwargs["client_storage"] + assert isinstance(storage, FernetEncryptionWrapper) + assert storage.raise_on_decryption_error is False + assert isinstance(storage.key_value, RedisStore) + pool = storage.key_value._client.connection_pool + assert pool.connection_class.__name__ == connection_class + assert pool.connection_kwargs["db"] == db + + +def test_redis_client_bounds_outage_latency(monkeypatch): + # Every tool call reads the token mapping from Redis. Left to redis-py's + # implicit defaults (5 s timeouts, up to 10 retries), a black-holed Redis + # would stall every request; pin a short, explicit budget instead. + kwargs = make_provider_kwargs(monkeypatch, REDIS_URL="redis://localhost:6379/1") + + pool = kwargs["client_storage"].key_value._client.connection_pool + assert pool.connection_kwargs["socket_connect_timeout"] == 2 + assert pool.connection_kwargs["socket_timeout"] == 2 + assert pool.connection_kwargs["retry"].get_retries() == 1 + + +def test_jwt_signing_key_is_forwarded_derived_once(monkeypatch): + # Bytes are used verbatim by FastMCP; a string would make it re-run the + # same 1M-iteration PBKDF2. Same derivation, so the signing key is unchanged. + kwargs = make_provider_kwargs( + monkeypatch, JWT_SIGNING_KEY="a-fixed-signing-secret-of-at-least-32-chars" + ) + + assert kwargs["jwt_signing_key"] == derive_jwt_key( + low_entropy_material="a-fixed-signing-secret-of-at-least-32-chars", + salt="fastmcp-jwt-signing-key", + ) + + +def derived_storage_key(jwt_key: bytes) -> bytes: + # FastMCP's default store key: oauth_proxy/proxy.py derives it from the + # (already derived) JWT signing key with this salt. + return derive_jwt_key( + high_entropy_material=jwt_key.decode(), salt="fastmcp-storage-encryption-key" + ) + + +@pytest.mark.parametrize( + "env,expected_key", + [ + ({"STORAGE_ENCRYPTION_KEY": FERNET_KEY}, FERNET_KEY.encode()), + ( + {}, + derived_storage_key( + derive_jwt_key( + high_entropy_material="test-client-secret", salt="fastmcp-jwt-signing-key" + ) + ), + ), + ( + {"JWT_SIGNING_KEY": "a-fixed-signing-secret-of-at-least-32-chars"}, + derived_storage_key( + derive_jwt_key( + low_entropy_material="a-fixed-signing-secret-of-at-least-32-chars", + salt="fastmcp-jwt-signing-key", + ) + ), + ), + ], + ids=["explicit-key", "derived-from-client-secret", "derived-from-jwt-signing-key"], +) +def test_redis_store_encryption_key(monkeypatch, env, expected_key): + # Unset, the key must be derived exactly as FastMCP derives its default + # store key, so no new key scheme is invented and JWT_SIGNING_KEY alone + # already decouples storage from CLIENT_SECRET. + monkeypatch.setenv("CLIENT_SECRET", "test-client-secret") + with patch("cryptography.fernet.Fernet", wraps=Fernet) as fernet: + make_provider_kwargs(monkeypatch, REDIS_URL="redis://localhost:6379/1", **env) + + assert fernet.call_args.args[0] == expected_key diff --git a/tests/test_config.py b/tests/test_config.py index e34c9cc..b7c9140 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -88,3 +88,132 @@ def test_rate_limit_rejects_non_positive(monkeypatch, env_var, env_value): with pytest.raises(ValidationError): config_module.Settings(_env_file=None) + + +# --------------------------------------------------------------------------- +# Tests: OAuth proxy state storage settings +# --------------------------------------------------------------------------- + +# A valid Fernet key (32 url-safe base64 bytes). Fernet rejects anything else, +# so tests that need a key must use a real one. +# Docs: https://cryptography.io/en/latest/fernet/#cryptography.fernet.Fernet +FERNET_KEY = "fXpQ0Ul6ZJ8fKk5q8D0v0b7n7cK4l9sQ3m2a1b0c9d8=" +STORAGE_ENV = ("REDIS_URL", "JWT_SIGNING_KEY", "STORAGE_ENCRYPTION_KEY") + + +def clear_storage_env(monkeypatch): + for name in STORAGE_ENV: + monkeypatch.delenv(name, raising=False) + monkeypatch.delenv(name.lower(), raising=False) + + +def test_storage_settings_default_to_none(monkeypatch): + clear_storage_env(monkeypatch) + + settings = config_module.Settings(_env_file=None) + + assert (settings.REDIS_URL, settings.JWT_SIGNING_KEY, settings.STORAGE_ENCRYPTION_KEY) == ( + None, + None, + None, + ) + + +@pytest.mark.parametrize( + "url", + ["redis://localhost:6379/1", "rediss://:pw@example.redis.cache.windows.net:6380/0"], + ids=["redis-plain", "rediss-tls"], +) +def test_redis_url_accepts_redis_and_rediss(monkeypatch, url): + clear_storage_env(monkeypatch) + monkeypatch.setenv("REDIS_URL", url) + + settings = config_module.Settings(_env_file=None) + + assert str(settings.REDIS_URL) == url + + +@pytest.mark.parametrize( + "url", + ["http://:s3cret-pw@redis:6379", "s3cret-pw@redis:6379"], + ids=["wrong-scheme", "no-scheme"], +) +def test_invalid_redis_url_fails_without_leaking_it(monkeypatch, url): + # REDIS_URL carries the Redis password. pydantic echoes input_value in + # errors by default, which would print the credential to startup logs. + clear_storage_env(monkeypatch) + monkeypatch.setenv("REDIS_URL", url) + + with pytest.raises(ValidationError) as exc: + config_module.Settings(_env_file=None) + + assert "REDIS_URL" in str(exc.value) + assert "s3cret-pw" not in str(exc.value) + + +def test_storage_encryption_key_without_redis_url_fails(monkeypatch): + # The key only ever applies to the Redis store; set alone it would be a + # silent no-op, so startup refuses it. + clear_storage_env(monkeypatch) + monkeypatch.setenv("STORAGE_ENCRYPTION_KEY", FERNET_KEY) + + with pytest.raises(ValidationError, match="STORAGE_ENCRYPTION_KEY requires REDIS_URL"): + config_module.Settings(_env_file=None) + + +def test_invalid_storage_encryption_key_fails(monkeypatch): + clear_storage_env(monkeypatch) + monkeypatch.setenv("REDIS_URL", "redis://localhost:6379/1") + monkeypatch.setenv("STORAGE_ENCRYPTION_KEY", "not-a-fernet-key") + + with pytest.raises(ValidationError, match="STORAGE_ENCRYPTION_KEY"): + config_module.Settings(_env_file=None) + + +@pytest.mark.parametrize("name", STORAGE_ENV, ids=["redis-url", "jwt-key", "encryption-key"]) +def test_empty_storage_setting_means_unset(monkeypatch, name): + # `JWT_SIGNING_KEY=` (an uncommented .env.example line, or an unset + # compose ${VAR}) must not become SecretStr(''): an empty signing key + # would derive a publicly computable key for the Redis store. + clear_storage_env(monkeypatch) + monkeypatch.setenv(name, "") + + settings = config_module.Settings(_env_file=None) + + assert getattr(settings, name) is None + + +@pytest.mark.parametrize( + "value", ["too-short", " " * 40], ids=["too-short", "whitespace-only"] +) +def test_weak_jwt_signing_key_fails(monkeypatch, value): + # Whitespace passes min_length but is as predictable as an empty key. + clear_storage_env(monkeypatch) + monkeypatch.setenv("JWT_SIGNING_KEY", value) + + with pytest.raises(ValidationError, match="JWT_SIGNING_KEY"): + config_module.Settings(_env_file=None) + + +@pytest.mark.parametrize( + "url", + ["redis://localhost:6379/abc", "redis://localhost:6379/1/extra"], + ids=["non-numeric-db", "nested-path"], +) +def test_redis_url_with_invalid_db_path_fails(monkeypatch, url): + # redis-py ignores a non-numeric path and silently uses DB 0 — the DB + # another service on a shared Redis is most likely to be using. + clear_storage_env(monkeypatch) + monkeypatch.setenv("REDIS_URL", url) + + with pytest.raises(ValidationError, match="REDIS_URL"): + config_module.Settings(_env_file=None) + + +def test_settings_repr_hides_redis_password(monkeypatch): + clear_storage_env(monkeypatch) + monkeypatch.setenv("REDIS_URL", "rediss://:s3cret-pw@example.redis.cache.windows.net:6380/0") + + settings = config_module.Settings(_env_file=None) + + assert "s3cret-pw" not in repr(settings) diff --git a/tests/test_redis_storage_integration.py b/tests/test_redis_storage_integration.py new file mode 100644 index 0000000..bf9ee12 --- /dev/null +++ b/tests/test_redis_storage_integration.py @@ -0,0 +1,188 @@ +"""Integration tests for the Redis OAuth proxy store against a real Redis.""" + +from __future__ import annotations + +import os +import time +import uuid +from unittest.mock import AsyncMock, MagicMock, patch + +import fastmcp +import pytest +from cryptography.fernet import Fernet +from fastmcp.server.auth.oauth_proxy.models import ClientCode +from fastmcp.server.auth.providers.azure import AzureProvider +from mcp.shared.auth import OAuthClientInformationFull +from redis.asyncio import Redis + +from src import auth_provider as auth_module +from src import config as config_module + +# A mocked store cannot show that state written by one server process is read +# back by the next, or that Redis applies FastMCP's TTLs — only a real Redis +# can. CI provides one as a service container; locally, point this at any +# scratch DB (e.g. redis://localhost:6379/15). Without it the tests skip. +# Docs: https://docs.github.com/en/actions/use-cases-and-examples/using-containerized-services/creating-redis-service-containers +REDIS_TEST_URL = os.environ.get("REDIS_TEST_URL") +pytestmark = pytest.mark.skipif( + not REDIS_TEST_URL, reason="set REDIS_TEST_URL to run against a real Redis" +) + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def make_storage(monkeypatch): + """The client storage a freshly started server process would build.""" + monkeypatch.setenv("REDIS_URL", REDIS_TEST_URL) + settings = config_module.Settings(_env_file=None) + with patch.object(auth_module, "settings", settings): + return auth_module._redis_client_storage() + + +def make_provider(monkeypatch) -> AzureProvider: + """A provider as one server process builds it.""" + provider = AzureProvider( + client_id="test-client-id", + client_secret="test-client-secret", + tenant_id="test-tenant-id", + base_url="http://localhost:8000", + required_scopes=["mcp-access"], + client_storage=make_storage(monkeypatch), + ) + # The server calls get_routes() at startup; it initialises the JWT issuer + # that token issuance and refresh need. + provider.get_routes() + return provider + + +def make_client(client_id: str) -> OAuthClientInformationFull: + return OAuthClientInformationFull( + client_id=client_id, + redirect_uris=["http://localhost:59999/callback"], + grant_types=["authorization_code", "refresh_token"], + token_endpoint_auth_method="none", + ) + + +# --------------------------------------------------------------------------- +# Tests: state survives a server restart +# --------------------------------------------------------------------------- + + +async def test_client_registered_before_restart_is_known_after(monkeypatch): + # Losing this registration is the reported failure: every refresh after a + # container recreation returned 401 invalid_client. + client_id = f"test-{uuid.uuid4()}" + await make_provider(monkeypatch).register_client(make_client(client_id)) + + restarted = make_provider(monkeypatch) + + found = await restarted.get_client(client_id) + # FastMCP's default file store would also survive an in-process "restart", + # so pin that the registration really lives in Redis. + raw = Redis.from_url(REDIS_TEST_URL, decode_responses=True) + try: + in_redis = await raw.exists(f"mcp-oauth-proxy-clients::{client_id}") + finally: + await raw.delete(f"mcp-oauth-proxy-clients::{client_id}") + await raw.aclose() + assert found is not None and found.client_id == client_id + assert in_redis == 1 + + +async def test_refresh_token_issued_before_restart_refreshes_after(monkeypatch, tmp_path): + # The acceptance criterion end to end through FastMCP's real token code: + # tokens issued by one process, redeemed by a fresh one on the same Redis. + # Each "process" gets its own empty FastMCP home (= a recreated container), + # so nothing can survive via the default file store. Only the upstream + # Entra token endpoint is stubbed. + client = make_client(f"test-{uuid.uuid4()}") + monkeypatch.setattr(fastmcp.settings, "home", tmp_path / "before") + before = make_provider(monkeypatch) + await before.register_client(client) + # FastMCP stores this ClientCode in its browser callback after the Entra + # sign-in; seeding it directly stands in for the interactive login. + code = f"test-{uuid.uuid4()}" + await before._code_store.put( + key=code, + value=ClientCode( + code=code, + client_id=client.client_id, + redirect_uri="http://localhost:59999/callback", + code_challenge=None, + code_challenge_method="S256", + scopes=["mcp-access"], + idp_tokens={"access_token": "entra-at", "refresh_token": "entra-rt", "expires_in": 3600}, + expires_at=time.time() + 300, + created_at=time.time(), + ), + ) + issued = await before.exchange_authorization_code( + client, await before.load_authorization_code(client, code) + ) + + monkeypatch.setattr(fastmcp.settings, "home", tmp_path / "restarted") + restarted = make_provider(monkeypatch) + upstream = MagicMock() + upstream.refresh_token = AsyncMock( + return_value={"access_token": "entra-at-2", "refresh_token": "entra-rt-2", "expires_in": 3600} + ) + with patch.object(restarted, "_create_upstream_oauth_client", return_value=upstream): + known_client = await restarted.get_client(client.client_id) + loaded = await restarted.load_refresh_token(known_client, issued.refresh_token) + assert loaded is not None, "refresh token issued before the restart was lost" + refreshed = await restarted.exchange_refresh_token(known_client, loaded, loaded.scopes) + + raw = Redis.from_url(REDIS_TEST_URL, decode_responses=True) + await raw.delete(f"mcp-oauth-proxy-clients::{client.client_id}") + await raw.aclose() + # The restarted process decrypted the upstream refresh token from Redis. + assert upstream.refresh_token.await_args.kwargs["refresh_token"] == "entra-rt" + assert refreshed.access_token and refreshed.access_token != issued.access_token + + +# --------------------------------------------------------------------------- +# Tests: what lands in Redis +# --------------------------------------------------------------------------- + + +async def test_entries_are_encrypted_and_honour_ttl(monkeypatch): + # The store holds users' Entra refresh tokens: never in plaintext, and + # short-lived entries must expire (FastMCP passes ttl= on every put). + key = f"test-{uuid.uuid4()}" + await make_storage(monkeypatch).put( + key=key, value={"refresh_token": "entra-rt-plaintext"}, collection="mcp-upstream-tokens", ttl=60 + ) + + raw = Redis.from_url(REDIS_TEST_URL, decode_responses=True) + try: + stored = await raw.get(f"mcp-upstream-tokens::{key}") + ttl = await raw.ttl(f"mcp-upstream-tokens::{key}") + finally: + await raw.delete(f"mcp-upstream-tokens::{key}") + await raw.aclose() + + assert "__encrypted_data__" in stored + assert "entra-rt-plaintext" not in stored + assert 0 < ttl <= 60 + + +async def test_rotated_encryption_key_reads_as_a_miss(monkeypatch): + # After a key change, old entries must read as "not found" (the client + # gets 401 and reconnects) rather than raise and turn /token into a 500. + key = f"test-{uuid.uuid4()}" + monkeypatch.setenv("STORAGE_ENCRYPTION_KEY", Fernet.generate_key().decode()) + await make_storage(monkeypatch).put(key=key, value={"v": 1}, collection="mcp-test") + + monkeypatch.setenv("STORAGE_ENCRYPTION_KEY", Fernet.generate_key().decode()) + try: + found = await make_storage(monkeypatch).get(key=key, collection="mcp-test") + finally: + raw = Redis.from_url(REDIS_TEST_URL, decode_responses=True) + await raw.delete(f"mcp-test::{key}") + await raw.aclose() + + assert found is None diff --git a/uv.lock b/uv.lock index 5d9ef33..88b129f 100644 --- a/uv.lock +++ b/uv.lock @@ -1142,12 +1142,13 @@ wheels = [ [[package]] name = "microsoft-planner-mcp" -version = "0.3.3" +version = "0.4.0" source = { virtual = "." } dependencies = [ { name = "fastmcp", extra = ["apps"] }, { name = "msgraph-sdk" }, { name = "odata-v4-query" }, + { name = "py-key-value-aio", extra = ["redis"] }, { name = "pydantic-settings" }, ] @@ -1164,9 +1165,10 @@ otel = [ [package.metadata] requires-dist = [ - { name = "fastmcp", extras = ["apps"], specifier = ">=3.2.3" }, + { name = "fastmcp", extras = ["apps"], specifier = ">=3.2.3,<4" }, { name = "msgraph-sdk", specifier = ">=1.55.0" }, { name = "odata-v4-query", specifier = ">=0.4.1" }, + { name = "py-key-value-aio", extras = ["redis"], specifier = ">=0.4.4,<0.5.0" }, { name = "pydantic-settings", specifier = ">=2.13.1" }, ] @@ -1623,6 +1625,9 @@ keyring = [ memory = [ { name = "cachetools" }, ] +redis = [ + { name = "redis" }, +] [[package]] name = "pycparser" @@ -1902,6 +1907,15 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/f1/12/de94a39c2ef588c7e6455cfbe7343d3b2dc9d6b6b2f40c4c6565744c873d/pyyaml-6.0.3-cp314-cp314t-win_arm64.whl", hash = "sha256:ebc55a14a21cb14062aa4162f906cd962b28e2e9ea38f9b4391244cd8de4ae0b", size = 149341, upload-time = "2025-09-25T21:32:56.828Z" }, ] +[[package]] +name = "redis" +version = "8.1.0" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/a8/99/604f0b666d4c616d891cf77ebb9db6bb21601344c051aebf1b72b9ff915f/redis-8.1.0.tar.gz", hash = "sha256:6e1a19beef9225c83efd689c7e6b7da2d5215b1f42cd13b7fc3714d0a09c7b25", size = 5254356, upload-time = "2026-07-30T08:51:00.269Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/66/9d/c5731f6e3608663d4d3656fd8d3aecee8b509c3082818f5a13eae925baea/redis-8.1.0-py3-none-any.whl", hash = "sha256:a4fe1aac3d3b3cc791d4b3d5931c5a956045dc951ee74d1c913ee3ac4d2ee9fb", size = 560618, upload-time = "2026-07-30T08:50:58.497Z" }, +] + [[package]] name = "referencing" version = "0.37.0"