Skip to content

Security hardening: MCP server auth#448

Merged
corinagum merged 7 commits into
mainfrom
cg/mcp-auth-hardening
May 11, 2026
Merged

Security hardening: MCP server auth#448
corinagum merged 7 commits into
mainfrom
cg/mcp-auth-hardening

Conversation

@corinagum

@corinagum corinagum commented Apr 23, 2026

Copy link
Copy Markdown
Collaborator

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).

Note: Earlier revisions of this branch also addressed finding MCP client URL validation. That has been dropped — the SSRF risk against MCP client URLs is low-likelihood and the default private-network filter created friction for legitimate on-prem MCP customers. Reverted in commit 7e1bd722. The null-safe Logger fix introduced alongside the original work is preserved.

MCP server auth

New McpPluginOptions class with an opt-in RequireAuth delegate. New overload on the existing extension method:

builder.AddTeamsMcp(options =>
{
    options.RequireAuth = ctx =>
        Task.FromResult(ctx.Request.Headers.Authorization.ToString() == "Bearer ...");
});

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). When RequireAuth is unset, a one-time startup warning fires.

Design doc

design/mcp-server-auth-options.md

E2E verified

  • Middleware gating: /mcp returns 401 without/wrong auth, 400 ("Mcp-Session-Id header is required") on correct bearer (past middleware)
  • Activity path: DevTools chat message accepted (201 on /v3/conversations/devtools/activities); /devtools UI unaffected (200)
  • Startup warning fires when RequireAuth is unset; silent when set

@corinagum
corinagum marked this pull request as ready for review April 29, 2026 19:25
Copilot AI review requested due to automatic review settings April 29, 2026 19:25
@corinagum
corinagum force-pushed the cg/mcp-auth-hardening branch from 064e68d to 8c100a7 Compare April 29, 2026 19:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.RequireAuth and wires an auth-gating middleware for /mcp, plus a new AddTeamsMcp(Action<McpPluginOptions>) overload.
  • Adds MCP client-side URL validation (scheme allowlist + block private networks by default) with AllowPrivateNetwork and ValidateUrl overrides.
  • 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.

Copilot AI review requested due to automatic review settings April 29, 2026 22:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot AI review requested due to automatic review settings May 5, 2026 22:53
@corinagum corinagum changed the title Security hardening: MCP server auth + client URL validation Security hardening: MCP server auth May 5, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@corinagum
corinagum force-pushed the cg/mcp-auth-hardening branch from 7e1bd72 to 3077474 Compare May 6, 2026 22:09
Copilot AI review requested due to automatic review settings May 6, 2026 23:14
@corinagum
corinagum force-pushed the cg/mcp-auth-hardening branch from 3077474 to bf02a45 Compare May 6, 2026 23:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

corinagum and others added 7 commits May 11, 2026 14:17
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]>
Copilot AI review requested due to automatic review settings May 11, 2026 21:17
@corinagum
corinagum force-pushed the cg/mcp-auth-hardening branch from c775b41 to 77fbe36 Compare May 11, 2026 21:17
@corinagum
corinagum enabled auto-merge (squash) May 11, 2026 21:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

@corinagum
corinagum merged commit 7ac5c8d into main May 11, 2026
10 checks passed
@corinagum
corinagum deleted the cg/mcp-auth-hardening branch May 11, 2026 21:46
corinagum added a commit that referenced this pull request May 15, 2026
## 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]>
corinagum added a commit to microsoft/teams-sdk that referenced this pull request May 27, 2026
## 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]>
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.

3 participants