Security hardening: MCP server auth#448
Conversation
064e68d to
8c100a7
Compare
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds security controls for MCP server hosting and MCP client URL handling to address scan findings around unauthenticated server mounting and SSRF via arbitrary URLs.
Changes:
- Introduces
McpPluginOptions.RequireAuthand wires an auth-gating middleware for/mcp, plus a newAddTeamsMcp(Action<McpPluginOptions>)overload. - Adds MCP client-side URL validation (scheme allowlist + block private networks by default) with
AllowPrivateNetworkandValidateUrloverrides. - Adds unit tests covering URL validation behavior and DNS resolution scenarios.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| Tests/Microsoft.Teams.Plugins.External.McpClient.Tests/UrlValidationTests.cs | Adds unit tests for scheme filtering, DNS resolution, private IP blocking, and override behavior. |
| Libraries/Microsoft.Teams.Plugins/Microsoft.Teams.Plugins.External/Microsoft.Teams.Plugins.External.McpClient/UrlValidation.cs | Implements the URL validation gate and private-address detection logic, plus a DNS test seam. |
| Libraries/Microsoft.Teams.Plugins/Microsoft.Teams.Plugins.External/Microsoft.Teams.Plugins.External.McpClient/McpClientPluginParams.cs | Adds AllowPrivateNetwork and ValidateUrl parameters to control SSRF validation behavior. |
| Libraries/Microsoft.Teams.Plugins/Microsoft.Teams.Plugins.External/Microsoft.Teams.Plugins.External.McpClient/McpClientPlugin.cs | Enforces URL validation before creating transport in tool fetch and tool invocation paths. |
| Libraries/Microsoft.Teams.Plugins/Microsoft.Teams.Plugins.External/Microsoft.Teams.Plugins.External.Mcp/McpPluginOptions.cs | Adds new server plugin options container with RequireAuth delegate hook. |
| Libraries/Microsoft.Teams.Plugins/Microsoft.Teams.Plugins.External/Microsoft.Teams.Plugins.External.Mcp/McpPlugin.cs | Adds middleware to enforce RequireAuth for /mcp and logs a startup warning if unset. |
| Libraries/Microsoft.Teams.Plugins/Microsoft.Teams.Plugins.External/Microsoft.Teams.Plugins.External.Mcp/Extensions/HostApplicationBuilder.cs | Adds a new builder extension overload for configuring McpPluginOptions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
7e1bd72 to
3077474
Compare
3077474 to
bf02a45
Compare
Per team decision, the MCP client SSRF guard adds friction for legitimate on-prem MCP customers and the underlying risk is low-likelihood. Reverting the client-side URL validation work; the McpPlugin server auth (#14), including the null-safe Logger fix, is kept. Removes: - UrlValidation.cs (and tests) - McpClientPluginParams.AllowPrivateNetwork + ValidateUrl - ValidateMcpServerUrlAsync calls in McpClientPlugin
- McpPlugin ctor: validate options is non-null up front instead of letting later null deref in Configure/OnStart surface as NRE. - AddTeamsMcp(builder, configure): null-check the configure callback before invoking; clearer failure mode. - Auth-gating middleware: re-throw OperationCanceledException when the request was aborted by the client, instead of swallowing it and writing a spurious 401. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
c775b41 to
77fbe36
Compare
## Summary Brings `main` into `releases/v2` for the 2.0.7 Libraries release, with quoted-replies (PR #389) excluded. Scope is **Legacy / Libraries only** — `core/` is not in scope for this release. Single squashed commit. ## What's in this release - All commits merged to main since 2.0.6 (the previous Libraries release), notably: - **Reactions GA** (PR #509) — `[Experimental("ExperimentalTeamsReactions")]` removed across Libraries - Prompt Preview Support (PR #433) - MCP server auth security hardening (PR #448) - Various dependency bumps and bug fixes ## What's NOT in this release **Quoted-replies feature (PR #389) is excluded:** - `MessageActivity.AddQuote()`, `PrependQuote()` builder methods — absent - `Context.Reply()` quote-aware behavior — reverted to legacy `replyToId` + blockquote - `Samples/Samples.Quoting` — removed - QR-related tests — removed **Kept intentionally:** - `QuotedReplyEntity` and `QuotedReplyData` types stay in `Microsoft.Teams.Api.Entities` (still annotated `[Experimental("ExperimentalTeamsQuotedReplies")]`) so inbound activities carrying `quotedReply` entities still parse - `MessageActivity.AddTargetedMessageInfo` runtime strip-QR logic — preserves prompt-preview cleanup - Entity.cs serializer cases for `quotedReply` (with existing `#pragma warning disable ExperimentalTeamsQuotedReplies`) ## Version `version.json`: `2.0.6` → `2.0.7` ## Scope **Libraries only.** `core/` (the `releases/core` track) is not affected by this PR — when this prep merges to `releases/v2`, the `core/` tree may appear in the branch as part of the merge, but the Legacy publish pipeline only builds and ships from `Libraries/`, `Samples/`, and `Tests/`. ## Test plan - [x] `dotnet build Microsoft.Teams.sln --configuration Release` — 0 errors, 5 pre-existing warnings - [x] `dotnet test Microsoft.Teams.sln --configuration Release` — all suites pass across net8.0 and net10.0 (Api 407+407, Apps 136+136, Common 63+63, AI 17+17, McpClient 9+9, Graph 3+3, AspNetCore 25+25 with 1 pre-existing skip) - [x] `dotnet pack` — nupkgs produced; consumer install verified (fresh console project, `dotnet add package Microsoft.Teams.Apps --prerelease`, `dotnet run` confirms `QuotedReplyEntity` ships, `AddQuote`/`PrependQuote` absent, `AddTargetedMessageInfo` present, `ReactionClient.AddAsync`/`DeleteAsync` present) - [ ] Teams.NET-ESRP pipeline with `packageSet=Legacy`, `publishType=Public` (post-merge) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
## Summary Adds a Trust Model page under Essentials → App Configuration documenting the layered authentication model the SDK uses for inbound JSON Web Tokens. ## Why A scanner flagged the SDK's `JsonWebToken` accessor class because it does not perform signature verification at the point of token parsing. A cross-SDK audit confirmed this is intentional architecture: signature verification runs at the HTTP boundary (via the activity pipeline's token validator), and the accessor class is a typed view over already-validated payloads. Every consumer of decoded claims is downstream of a validator pass. This page makes the architectural invariant explicit for SDK consumers, reviewers, and future contributors. ## What's covered - What the SDK validates automatically on `/api/messages` (signature, issuer, audience, lifetime, algorithm) - What downstream code can rely on (typed accessors over validated tokens) - How to add custom authentication to non-default surfaces (MCP, callbacks, webhooks) — references the `requireAuth` hook recently added in microsoft/teams.ts#540 / microsoft/teams.net#448 - Anti-patterns to avoid (constructing the accessor from untrusted input as a basis for authorization decisions) ## Build verified `npm run build` clean. ## Related PRs - TypeScript: microsoft/teams.ts#586 - Python: microsoft/teams.py#432 - .NET: microsoft/teams.net#517 --------- Co-authored-by: Copilot Autofix powered by AI <[email protected]>
Summary
Addresses security scan finding MCP server mounted without auth. .NET half of a 2-SDK PR set (TypeScript PR is on the same branch name).
MCP server auth
New
McpPluginOptionsclass with an opt-inRequireAuthdelegate. New overload on the existing extension method:Delegate signature:
Func<HttpContext, Task<bool>>.false/throw → 401 via an ASP.NET Core middleware path-filtered to/mcp. Parameterless.AddTeamsMcp()overload is unchanged (backward compatible). WhenRequireAuthis unset, a one-time startup warning fires.Design doc
design/mcp-server-auth-options.mdE2E verified
/mcpreturns 401 without/wrong auth, 400 ("Mcp-Session-Id header is required") on correct bearer (past middleware)/v3/conversations/devtools/activities);/devtoolsUI unaffected (200)RequireAuthis unset; silent when set