Support OAuth protected registry server #1509
Conversation
JasonYeMSFT
requested review from
JonathanCrd,
chidozieononiwu,
Copilot,
g2vinay,
jongio,
joshfree,
msalaman and
sandeep-sen
There was a problem hiding this comment.
Pull request overview
This PR extends the RegistryServerProvider to support OAuth-protected registry servers by adding authentication capabilities for MCP servers with HTTP transport. The implementation allows registry servers to specify OAuth scopes in registry.json, which triggers the creation of custom HttpClient instances with access token handlers that automatically fetch and attach bearer tokens to outgoing requests.
Changes:
- Added OAuth scope configuration support to
RegistryServerInfomodel - Implemented
CreateClientWithAccessTokenmethod inHttpClientServicewith anAccessTokenHandlerdelegating handler - Updated
RegistryServerProviderandRegistryDiscoveryStrategyto inject and use authentication services - Modified unit tests to accommodate new constructor dependencies with proper mocking
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
core/Azure.Mcp.Core/src/Services/Http/IHttpClientService.cs |
Added interface method for creating HttpClient with access token provider |
core/Azure.Mcp.Core/src/Services/Http/HttpClientService.cs |
Implemented access token-enabled HttpClient creation with custom delegating handler |
core/Azure.Mcp.Core/src/Areas/Server/Models/RegistryServerInfo.cs |
Added OAuthScopes property to registry server configuration model |
core/Azure.Mcp.Core/src/Areas/Server/Commands/Discovery/RegistryServerProvider.cs |
Integrated OAuth support into client creation logic for HTTP transport |
core/Azure.Mcp.Core/src/Areas/Server/Commands/Discovery/RegistryDiscoveryStrategy.cs |
Added dependency injection for authentication services |
core/Azure.Mcp.Core/src/Areas/Server/Resources/registry.json |
Added example "arm" server configuration with OAuth scopes |
core/Azure.Mcp.Core/tests/Azure.Mcp.Core.UnitTests/Areas/Server/Commands/Discovery/RegistryServerProviderTests.cs |
Updated tests to mock new dependencies |
core/Azure.Mcp.Core/tests/Azure.Mcp.Core.UnitTests/Areas/Server/Commands/Discovery/RegistryDiscoveryStrategyTests.cs |
Updated tests to mock new dependencies |
core/Azure.Mcp.Core/tests/Azure.Mcp.Core.UnitTests/Areas/Server/Commands/ServiceCollectionExtensionsTests.cs |
Added token credential provider registration to test setup |
core/Azure.Mcp.Core/tests/Azure.Mcp.Core.UnitTests/Areas/Server/Commands/ToolLoading/SingleProxyToolLoaderTests.cs |
Refactored to use helper method for creating RegistryDiscoveryStrategy with dependencies |
core/Azure.Mcp.Core/tests/Azure.Mcp.Core.UnitTests/Areas/Server/Commands/ToolLoading/ServerToolLoaderTests.cs |
Refactored to use helper method for creating RegistryDiscoveryStrategy with dependencies |
core/Azure.Mcp.Core/src/Areas/Server/Commands/Discovery/RegistryServerProvider.cs
Outdated
Show resolved
Hide resolved
core/Azure.Mcp.Core/src/Areas/Server/Commands/Discovery/RegistryServerProvider.cs
Outdated
Show resolved
Hide resolved
...ests/Azure.Mcp.Core.UnitTests/Areas/Server/Commands/Discovery/RegistryServerProviderTests.cs
Outdated
Show resolved
Hide resolved
vukelich
left a comment
vukelich
left a comment
There was a problem hiding this comment.
Chatted offline. Jason was already mid-change for switching to IHttpClientFactory. Will return for review when that's complete.
core/Azure.Mcp.Core/src/Areas/Server/Commands/Discovery/RegistryDiscoveryStrategy.cs
Outdated
Show resolved
Hide resolved
core/Azure.Mcp.Core/src/Areas/Server/Commands/Discovery/RegistryServerProvider.cs
Outdated
Show resolved
Hide resolved
github-project-automation
bot
moved this from Untriaged
to In Progress
in Azure MCP Server
core/Azure.Mcp.Core/src/Areas/Server/Models/RegistryServerInfo.cs
Outdated
Show resolved
Hide resolved
core/Azure.Mcp.Core/src/Areas/Server/Models/RegistryServerInfo.cs
Outdated
Show resolved
Hide resolved
| /// <summary> | ||
| /// Add HttpClient for each registry server with OAuthScopes that knows how to fetch its access token. | ||
| /// </summary> | ||
| private static IServiceCollection AddHttpClientForMcpRegistry(this IServiceCollection services) |
There was a problem hiding this comment.
For clean code organization, please put this code somewhere close to adding the registry tool loader and discovery strategy into the DI container. This code is not relevant for the TenantService.
| private static IServiceCollection AddHttpClientForMcpRegistry(this IServiceCollection services) | ||
| { | ||
| var assembly = Assembly.GetExecutingAssembly(); | ||
| var resourceName = assembly |
There was a problem hiding this comment.
I'd strongly recommend leaving a comment as a warning to other maintainers both here and in RegistryDiscoveryStrategy that we have two different files reading and acting on registry.json. As an example, if one of them changes the name of the file, then the other will break.
There was a problem hiding this comment.
The logic for registring RegistryServer related DI depdendencies have been moved to RegistryServerServiceCollectionExtension.cs.
| var resourceName = assembly | ||
| .GetManifestResourceNames() | ||
| .FirstOrDefault(n => n.EndsWith("registry.json", StringComparison.OrdinalIgnoreCase)); | ||
| if (resourceName is null) |
There was a problem hiding this comment.
As an alternative to opening and parsing this file within this method and tossing out the results, we could load the results into an object and add it into the DI container such as adding RegistryRoot as a type into the DI for RegistryDiscoveryStrategy to be constructed with.
There was a problem hiding this comment.
This allows the application to have a "read once" rather than multiple times in separate spots.
There was a problem hiding this comment.
Now azmcp loads the registry.json once and registers the parsed object as a DI singleton.
core/Azure.Mcp.Core/src/Areas/Server/Commands/Discovery/RegistryDiscoveryStrategy.cs
Outdated
Show resolved
Hide resolved
| services.AddHttpClient(RegistryServerHelper.GetRegistryServerHttpClientName(serverName)) | ||
| .AddHttpMessageHandler((services) => | ||
| { | ||
| var fetchAccessToken = async (CancellationToken cancellationToken) => |
There was a problem hiding this comment.
Rather than passing around function callbacks, can we please construct AccessTokenHandler with the OAuth scopes and a credential provider, and the internal logic of AccessTokenHandler has this logic?
There was a problem hiding this comment.
This is very TypeScript/JavaScript-esque rather than OOB
There was a problem hiding this comment.
It's modified to constructor AccessTokenHandler with an IAzureTokenCredentialProvider and oauthScopes.
| // Licensed under the MIT License. | ||
|
|
||
| using System.Net; | ||
| using System.Net.Http.Headers; |
Use DI to read server registry once and load at multiple places Style and documentation fixes per PR feedback
3 participants
What does this PR do?
[Provide a clear, concise description of the changes]This PR extends the RegistryServerProvider to support adding registry servers protected by OAuth for all its MCP accesses.
[Any additional context, screenshots, or information that helps reviewers]registry.jsonnow let's MCP servers with http transport to optionally declare anoauthScopesparameter. When this property is defined, the RegistryServerProvider will instantiate the corresponding McpClient object with a custom HttpClient. This custom HttpClient object has a delegate handler that fetches an access token and sets it in the Authorization header in each request.Text scenarios
The server may apply additional restrictions that limit access, such as the tenant restriction.
GitHub issue number?
Pre-merge Checklist
servers/Azure.Mcp.Server/CHANGELOG.mdand/orservers/Fabric.Mcp.Server/CHANGELOG.mdfor product changes (features, bug fixes, UI/UX, updated dependencies)servers/Azure.Mcp.Server/README.mdand/orservers/Fabric.Mcp.Server/README.mddocumentationeng/scripts/Process-PackageReadMe.ps1. See Package README/servers/Azure.Mcp.Server/docs/azmcp-commands.mdand/or/docs/fabric-commands.md.\eng\scripts\Update-AzCommandsMetadata.ps1to update tool metadata in azmcp-commands.md (required for CI)ToolDescriptionEvaluatorand obtained a score of0.4or more and a top 3 ranking for all related test promptsconsolidated-tools.json/servers/Azure.Mcp.Server/docs/e2eTestPrompts.mdcrypto mining, spam, data exfiltration, etc.)/azp run mcp - pullrequest - liveto run Live Test Pipeline