Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 76 additions & 0 deletions .github/skills/run-conformance-from-branch/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
---
name: run-conformance-from-branch
description: Run MCP conformance tests in the C# SDK against a conformance branch (including forks) instead of the published npm version, then restore pinned dependencies.
compatibility: Requires npm, node, and dotnet SDK. Uses the csharp-sdk repo package.json/package-lock.json and tests/ModelContextProtocol.AspNetCore.Tests.
---

# Run Conformance From Branch

Run C# SDK conformance tests against an unpublished `modelcontextprotocol/conformance` branch (including branches in forks).

## Use Cases

- Validate a conformance PR before it is published to npm
- Validate C# SDK behavior against a fork with custom scenario changes
- Reproduce failures caused by conformance changes

## Safety / Repo Hygiene

1. Start from a clean git state.
2. Commit or stash local changes first.
3. Restore pinned dependencies when done (`git checkout -- package.json package-lock.json` + `npm ci`).

## Inputs

- **Source type**: `upstream-branch` or `fork-branch`
- **Source locator**:
- Upstream branch: `modelcontextprotocol/conformance#<branch>`
- Fork branch: `<owner>/conformance#<branch>`
- **Scenario** (optional): e.g. `auth/scope-step-up`

## Workflows

### A) Install directly from GitHub branch (upstream or fork)

From `csharp-sdk` root:

```bash
npm install --no-save @modelcontextprotocol/conformance@github:<owner>/conformance#<branch>
```

Examples:

```bash
npm install --no-save @modelcontextprotocol/conformance@github:modelcontextprotocol/conformance#main
npm install --no-save @modelcontextprotocol/conformance@github:myuser/conformance#sep-2350-check
```

## Run Tests

### Run client conformance tests with dotnet test filter:

```bash
dotnet test tests/ModelContextProtocol.AspNetCore.Tests/ModelContextProtocol.AspNetCore.Tests.csproj -f net10.0 --filter "FullyQualifiedName~ClientConformanceTests"
```

### Run server conformance tests with dotnet test filter:

```bash
dotnet test tests/ModelContextProtocol.AspNetCore.Tests/ModelContextProtocol.AspNetCore.Tests.csproj -f net10.0 --filter "FullyQualifiedName~ServerConformanceTests"
```

## Reporting

Always report:

1. Installed conformance source (`npm ls @modelcontextprotocol/conformance --depth=0`)
2. Scenario results (pass/fail/warnings)
3. Any new check IDs observed (for traceability)

## Cleanup / Restore

Return repo to pinned dependency state:

```bash
npm ci
```
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ internal sealed partial class ClientOAuthProvider : McpHttpClient
private string? _tokenEndpointAuthMethod;
private ITokenCache _tokenCache;
private AuthorizationServerMetadata? _authServerMetadata;
private readonly HashSet<string> _previouslyRequestedScopes = new(StringComparer.Ordinal);
private readonly object _scopeAccumulatorLock = new();

/// <summary>
/// Initializes a new instance of the <see cref="ClientOAuthProvider"/> class using the specified options.
Expand Down Expand Up @@ -714,16 +716,45 @@ private async Task PerformDynamicClientRegistrationAsync(

private string? GetScopeParameter(ProtectedResourceMetadata protectedResourceMetadata)
{
// Determine the scopes for the current operation from the challenge or metadata.
string? currentOperationScopes;
if (!string.IsNullOrEmpty(protectedResourceMetadata.WwwAuthenticateScope))
{
return protectedResourceMetadata.WwwAuthenticateScope;
currentOperationScopes = protectedResourceMetadata.WwwAuthenticateScope;
}
else if (protectedResourceMetadata.ScopesSupported.Count > 0)
{
return string.Join(" ", protectedResourceMetadata.ScopesSupported);
currentOperationScopes = string.Join(" ", protectedResourceMetadata.ScopesSupported);
}
else
{
currentOperationScopes = _configuredScopes;
}

return _configuredScopes;
if (string.IsNullOrEmpty(currentOperationScopes))
{
lock (_scopeAccumulatorLock)
{
// If we have previously requested scopes but nothing new, return the accumulated set.
return _previouslyRequestedScopes.Count > 0
? string.Join(" ", _previouslyRequestedScopes.OrderBy(s => s, StringComparer.Ordinal))
: null;
}
}

// Per SEP-2350: Compute the union of previously requested scopes and newly challenged scopes
// to avoid losing permissions needed for other operations during step-up authorization.
var newScopes = currentOperationScopes!.Split(new[] { ' ' }, StringSplitOptions.RemoveEmptyEntries);
lock (_scopeAccumulatorLock)
{
foreach (var scope in newScopes)
{
_previouslyRequestedScopes.Add(scope);
}

// Sort scopes for stable, deterministic output (scopes are unordered per RFC 6749 §3.3).
return string.Join(" ", _previouslyRequestedScopes.OrderBy(s => s, StringComparer.Ordinal));
}
Comment on lines +752 to +757
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@copilot are scopes ordered? Is this a correctness concern or just a concern about possible flaky tests that don't treat scopes as unordered?

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.

OAuth scopes are not ordered — per RFC 6749 §3.3 they're a space-delimited set, so the server treats them as a set regardless of order. This is not a correctness concern for the OAuth protocol.

It is, however, a real flaky test concern: the AuthorizationFlow_UsesScopeFromProtectedResourceMetadata test was asserting an exact scope string ("mcp:tools files:read") that could fail nondeterministically since HashSet enumeration order is not guaranteed.

Fixed in 84b5585 by:

  • Sorting scopes with OrderBy(s => s, StringComparer.Ordinal) before joining in GetScopeParameter, so the emitted scope parameter is stable and deterministic.
  • Updating the one test that used an exact string assertion to use an order-independent HashSet comparison (consistent with the pattern already used in the other scope tests).

}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,8 @@ public sealed class ProtectedResourceMetadata
/// The scopes included in the WWW-Authenticate challenge MAY match scopes_supported, be a subset or superset of it,
/// or an alternative collection that is neither a strict subset nor superset. Clients MUST NOT assume any particular
/// set relationship between the challenged scope set and scopes_supported. Clients MUST treat the scopes provided
/// in the challenge as authoritative for satisfying the current request.
/// in the challenge as authoritative for the current operation. When re-authorizing, clients SHOULD include these
/// scopes alongside any previously granted scopes to avoid losing permissions needed for other operations (SEP-2350).
///
/// https://modelcontextprotocol.io/specification/2025-11-25/basic/authorization#protected-resource-metadata-discovery-requirements
/// </summary>
Expand Down
141 changes: 134 additions & 7 deletions tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,9 @@ public async Task AuthorizationFlow_UsesScopeFromProtectedResourceMetadata()
await using var client = await McpClient.CreateAsync(
transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken);

Assert.Equal("mcp:tools files:read", requestedScope);
var requestedScopeSet = new HashSet<string>(requestedScope!.Split(' '));
Assert.Contains("mcp:tools", requestedScopeSet);
Assert.Contains("files:read", requestedScopeSet);
}

[Fact]
Expand Down Expand Up @@ -473,9 +475,13 @@ public async Task AuthorizationFlow_UsesScopeFromForbiddenHeader()
McpServerTool.Create([McpServerTool(Name = "admin-tool")]
(ClaimsPrincipal user) =>
{
// Tool now just checks if user has the required scopes
// If they don't, it shouldn't get here due to middleware
Assert.True(user.HasClaim("scope", adminScopes), "User should have admin scopes when tool executes");
// Verify the user's scope claim contains all required admin scopes.
// With scope accumulation (SEP-2350), the token scope will be the union
// of previously granted and newly challenged scopes.
var scopeClaim = user.FindFirst("scope")?.Value ?? "";
var scopeSet = new HashSet<string>(scopeClaim.Split(' '));
Assert.Contains("admin:read", scopeSet);
Assert.Contains("admin:write", scopeSet);
return "Admin tool executed.";
}),
]);
Expand Down Expand Up @@ -510,9 +516,11 @@ public async Task AuthorizationFlow_UsesScopeFromForbiddenHeader()

if (toolCallParams?.Name == "admin-tool")
{
// Check if user has required scopes
// Check if user has required scopes (scope claim contains all admin scopes)
var user = context.User;
if (!user.HasClaim("scope", adminScopes))
var scopeClaim = user.FindFirst("scope")?.Value ?? "";
var scopeSet = new HashSet<string>(scopeClaim.Split(' '));
if (!scopeSet.Contains("admin:read") || !scopeSet.Contains("admin:write"))
{
// User lacks required scopes, return 403 before MapMcp processes the request
context.Response.StatusCode = StatusCodes.Status403Forbidden;
Expand Down Expand Up @@ -554,7 +562,126 @@ public async Task AuthorizationFlow_UsesScopeFromForbiddenHeader()
var adminResult = await client.CallToolAsync("admin-tool", cancellationToken: TestContext.Current.CancellationToken);
Assert.Equal("Admin tool executed.", adminResult.Content[0].ToString());

Assert.Equal(adminScopes, requestedScope);
// SEP-2350: Verify that the step-up authorization request includes the union
// of previously requested scopes (mcp:tools) and newly challenged scopes (admin:read admin:write).
var requestedScopeSet = new HashSet<string>(requestedScope!.Split(' '));
Assert.Contains("mcp:tools", requestedScopeSet);
Assert.Contains("admin:read", requestedScopeSet);
Assert.Contains("admin:write", requestedScopeSet);
}

[Fact]
public async Task AuthorizationFlow_AccumulatesScopesAcrossMultipleStepUps()
{
// SEP-2350: Verify scope accumulation across multiple step-up authorization challenges.
// First call requires "files:read", second call requires "files:write".
// The second authorization request should include both "mcp:tools files:read files:write".

Builder.Services.AddMcpServer()
.WithTools([
McpServerTool.Create([McpServerTool(Name = "read-tool")]
(ClaimsPrincipal user) =>
{
return "Read tool executed.";
}),
McpServerTool.Create([McpServerTool(Name = "write-tool")]
(ClaimsPrincipal user) =>
{
return "Write tool executed.";
}),
]);

List<string?> requestedScopes = [];

await using var app = await StartMcpServerAsync(configureMiddleware: app =>
{
app.Use(async (context, next) =>
{
if (context.Request.Method == HttpMethods.Post && context.Request.Path == "/")
{
context.Request.EnableBuffering();

var message = await JsonSerializer.DeserializeAsync(
context.Request.Body,
McpJsonUtilities.DefaultOptions.GetTypeInfo(typeof(JsonRpcMessage)),
context.RequestAborted) as JsonRpcMessage;

context.Request.Body.Position = 0;

if (message is JsonRpcRequest request && request.Method == "tools/call")
{
var toolCallParams = JsonSerializer.Deserialize(
request.Params,
McpJsonUtilities.DefaultOptions.GetTypeInfo(typeof(CallToolRequestParams))) as CallToolRequestParams;

var user = context.User;
var scopeClaim = user.FindFirst("scope")?.Value ?? "";
var scopeSet = new HashSet<string>(scopeClaim.Split(' '));

if (toolCallParams?.Name == "read-tool" && !scopeSet.Contains("files:read"))
{
context.Response.StatusCode = StatusCodes.Status403Forbidden;
context.Response.Headers.WWWAuthenticate = $"Bearer error=\"insufficient_scope\", resource_metadata=\"{McpServerUrl}/.well-known/oauth-protected-resource\", scope=\"files:read\"";
await context.Response.StartAsync(context.RequestAborted);
await context.Response.Body.FlushAsync(context.RequestAborted);
return;
}

if (toolCallParams?.Name == "write-tool" && !scopeSet.Contains("files:write"))
{
context.Response.StatusCode = StatusCodes.Status403Forbidden;
context.Response.Headers.WWWAuthenticate = $"Bearer error=\"insufficient_scope\", resource_metadata=\"{McpServerUrl}/.well-known/oauth-protected-resource\", scope=\"files:write\"";
await context.Response.StartAsync(context.RequestAborted);
await context.Response.Body.FlushAsync(context.RequestAborted);
return;
}
}
}

await next(context);
});
});

await using var transport = new HttpClientTransport(new()
{
Endpoint = new(McpServerUrl),
OAuth = new()
{
ClientId = "demo-client",
ClientSecret = "demo-secret",
RedirectUri = new Uri("http://localhost:1179/callback"),
AuthorizationRedirectDelegate = (uri, redirect, ct) =>
{
var query = QueryHelpers.ParseQuery(uri.Query);
requestedScopes.Add(query["scope"].ToString());
return HandleAuthorizationUrlAsync(uri, redirect, ct);
},
},
}, HttpClient, LoggerFactory);

await using var client = await McpClient.CreateAsync(
transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken);

// Initial auth gets "mcp:tools" from protected resource metadata
Assert.Single(requestedScopes);
Assert.Equal("mcp:tools", requestedScopes[0]);

// First step-up: read-tool requires "files:read"
var readResult = await client.CallToolAsync("read-tool", cancellationToken: TestContext.Current.CancellationToken);
Assert.Equal("Read tool executed.", readResult.Content[0].ToString());
Assert.Equal(2, requestedScopes.Count);
var secondScopeSet = new HashSet<string>(requestedScopes[1]!.Split(' '));
Assert.Contains("mcp:tools", secondScopeSet);
Assert.Contains("files:read", secondScopeSet);

// Second step-up: write-tool requires "files:write"
var writeResult = await client.CallToolAsync("write-tool", cancellationToken: TestContext.Current.CancellationToken);
Assert.Equal("Write tool executed.", writeResult.Content[0].ToString());
Assert.Equal(3, requestedScopes.Count);
var thirdScopeSet = new HashSet<string>(requestedScopes[2]!.Split(' '));
Assert.Contains("mcp:tools", thirdScopeSet);
Assert.Contains("files:read", thirdScopeSet);
Assert.Contains("files:write", thirdScopeSet);
}

[Fact]
Expand Down