Skip to content

Commit 0fd6093

Browse files
239573049claude
239573049
and
claude
committed
🐛 fix(mcp): validate Google ID token signature in OAuth callback
Previously the MCP OAuth callback read Google ID token claims without verifying the token signature, allowing a crafted JWT to bypass authentication. Now validates the token against Google's JWKS public keys with issuer, audience, and lifetime checks. Also updates test constructors to match new MessageRouter and EmbedService signatures. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 0ff5336 commit 0fd6093

File tree

5 files changed

+68
-26
lines changed

5 files changed

+68
-26
lines changed

‎src/OpenDeepWiki/MCP/McpOAuthServer.cs‎

Lines changed: 43 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@
55
using System.Text.Json;
66
using Microsoft.EntityFrameworkCore;
77
using Microsoft.Extensions.Options;
8+
using Microsoft.IdentityModel.Protocols;
9+
using Microsoft.IdentityModel.Protocols.OpenIdConnect;
10+
using Microsoft.IdentityModel.Tokens;
811
using OpenDeepWiki.EFCore;
912
using OpenDeepWiki.Entities;
1013
using OpenDeepWiki.Services.Auth;
@@ -24,6 +27,7 @@ public class McpOAuthServer
2427
private readonly ILogger<McpOAuthServer> _logger;
2528
private readonly string _googleClientId;
2629
private readonly string _googleClientSecret;
30+
private readonly ConfigurationManager<OpenIdConnectConfiguration> _googleOidcConfigManager;
2731

2832
// In-memory stores for OAuth state (short-lived, lost on restart = clients re-auth)
2933
private static readonly ConcurrentDictionary<string, PendingAuthorization> PendingAuthorizations = new();
@@ -57,6 +61,12 @@ public McpOAuthServer(
5761
?? configuration["Google:ClientSecret"]
5862
?? throw new InvalidOperationException(
5963
"GOOGLE_CLIENT_SECRET is required for MCP OAuth authorization server");
64+
65+
// Initialize Google OIDC configuration manager for ID token signature validation
66+
_googleOidcConfigManager = new ConfigurationManager<OpenIdConnectConfiguration>(
67+
"https://accounts.google.com/.well-known/openid-configuration",
68+
new OpenIdConnectConfigurationRetriever(),
69+
new HttpDocumentRetriever(httpClientFactory.CreateClient()));
6070
}
6171

6272
/// <summary>
@@ -211,18 +221,47 @@ public async Task<IResult> HandleCallback(HttpContext context)
211221
var tokenJson = await tokenResponse.Content.ReadAsStringAsync();
212222
var tokenData = JsonSerializer.Deserialize<JsonElement>(tokenJson);
213223

214-
// Extract user info from the ID token
224+
// Validate and extract user info from the Google ID token
215225
var idToken = tokenData.GetProperty("id_token").GetString();
216226
if (string.IsNullOrEmpty(idToken))
217227
{
218228
_logger.LogError("MCP OAuth callback: no id_token in Google response");
219229
return Results.BadRequest(new { error = "server_error", error_description = "No ID token from Google" });
220230
}
221231

232+
// Validate Google ID token signature using Google's JWKS public keys
222233
var handler = new JwtSecurityTokenHandler();
223-
var jwt = handler.ReadJwtToken(idToken);
224-
var email = jwt.Claims.FirstOrDefault(c => c.Type == "email")?.Value;
225-
var name = jwt.Claims.FirstOrDefault(c => c.Type == "name")?.Value ?? email;
234+
var oidcConfig = await _googleOidcConfigManager.GetConfigurationAsync(CancellationToken.None);
235+
var validationParameters = new TokenValidationParameters
236+
{
237+
ValidateIssuer = true,
238+
ValidIssuers = new[] { "https://accounts.google.com", "accounts.google.com" },
239+
ValidateAudience = true,
240+
ValidAudience = _googleClientId,
241+
ValidateLifetime = true,
242+
ValidateIssuerSigningKey = true,
243+
IssuerSigningKeys = oidcConfig.SigningKeys,
244+
};
245+
246+
TokenValidationResult validationResult;
247+
try
248+
{
249+
validationResult = await handler.ValidateTokenAsync(idToken, validationParameters);
250+
}
251+
catch (Exception ex)
252+
{
253+
_logger.LogError(ex, "MCP OAuth callback: Google ID token validation failed");
254+
return Results.BadRequest(new { error = "server_error", error_description = "ID token validation failed" });
255+
}
256+
257+
if (!validationResult.IsValid)
258+
{
259+
_logger.LogError("MCP OAuth callback: Google ID token is invalid: {Exception}", validationResult.Exception?.Message);
260+
return Results.BadRequest(new { error = "server_error", error_description = "ID token validation failed" });
261+
}
262+
263+
var email = validationResult.ClaimsIdentity.FindFirst("email")?.Value;
264+
var name = validationResult.ClaimsIdentity.FindFirst("name")?.Value ?? email;
226265

227266
if (string.IsNullOrEmpty(email))
228267
{

‎tests/OpenDeepWiki.Tests/Chat/Providers/ProviderEnableDisablePropertyTests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ public Property DisabledProvider_ShouldNotProcessMessagesViaRouter()
112112
count =>
113113
{
114114
var logger = NullLogger<MessageRouter>.Instance;
115-
var router = new MessageRouter(logger);
115+
var router = new MessageRouter(logger, null!);
116116
var platforms = Enumerable.Range(1, count)
117117
.Select(i => $"platform_{i}")
118118
.ToList();

‎tests/OpenDeepWiki.Tests/Chat/Routing/ProviderRoutingPropertyTests.cs‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
using FsCheck.Fluent;
33
using FsCheck.Xunit;
44
using Microsoft.AspNetCore.Http;
5+
using Microsoft.Extensions.DependencyInjection;
56
using Microsoft.Extensions.Logging;
67
using Microsoft.Extensions.Logging.Abstractions;
78
using OpenDeepWiki.Chat.Abstractions;
@@ -59,7 +60,7 @@ public Property MessageShouldBeRoutedToMatchingProvider()
5960
GeneratePlatformCount().ToArbitrary(),
6061
count =>
6162
{
62-
var router = new MessageRouter(_logger);
63+
var router = new MessageRouter(_logger, null!);
6364
var platforms = Enumerable.Range(1, count)
6465
.Select(i => $"platform_{i}")
6566
.ToList();
@@ -94,7 +95,7 @@ public Property RegisteredProviderCountShouldMatch()
9495
GeneratePlatformCount().ToArbitrary(),
9596
count =>
9697
{
97-
var router = new MessageRouter(_logger);
98+
var router = new MessageRouter(_logger, null!);
9899
var platforms = Enumerable.Range(1, count)
99100
.Select(i => $"platform_{i}")
100101
.ToList();
@@ -122,7 +123,7 @@ public Property UnregisteredPlatformShouldReturnNull()
122123
GenerateUniquePlatformId().ToArbitrary(),
123124
(count, unregisteredPlatform) =>
124125
{
125-
var router = new MessageRouter(_logger);
126+
var router = new MessageRouter(_logger, null!);
126127
var platforms = Enumerable.Range(1, count)
127128
.Select(i => $"registered_{i}")
128129
.ToList();
@@ -154,7 +155,7 @@ public Property HasProviderShouldBeConsistentWithGetProvider()
154155
GenerateValidPlatform().ToArbitrary(),
155156
(count, queryPlatform) =>
156157
{
157-
var router = new MessageRouter(_logger);
158+
var router = new MessageRouter(_logger, null!);
158159
var platforms = Enumerable.Range(1, count)
159160
.Select(i => $"platform_{i}")
160161
.ToList();
@@ -183,7 +184,7 @@ public Property UnregisteredProviderShouldNotBeAccessible()
183184
GeneratePlatformCount().ToArbitrary(),
184185
count =>
185186
{
186-
var router = new MessageRouter(_logger);
187+
var router = new MessageRouter(_logger, null!);
187188
var platforms = Enumerable.Range(1, count)
188189
.Select(i => $"platform_{i}")
189190
.ToList();
@@ -217,7 +218,7 @@ public Property DuplicateRegistrationShouldUpdateProvider()
217218
GenerateUniquePlatformId().ToArbitrary(),
218219
platformId =>
219220
{
220-
var router = new MessageRouter(_logger);
221+
var router = new MessageRouter(_logger, null!);
221222

222223
// 注册第一个 Provider
223224
var provider1 = CreateTestProvider(platformId, "Provider 1");
@@ -242,7 +243,7 @@ public Property DuplicateRegistrationShouldUpdateProvider()
242243
[Fact]
243244
public void PlatformIdMatching_ShouldBeCaseInsensitive()
244245
{
245-
var router = new MessageRouter(_logger);
246+
var router = new MessageRouter(_logger, null!);
246247

247248
// 注册小写平台
248249
var provider = CreateTestProvider("feishu", "Feishu Provider");
@@ -270,7 +271,7 @@ public void PlatformIdMatching_ShouldBeCaseInsensitive()
270271
[InlineData(" ")]
271272
public void EmptyPlatformId_ShouldReturnNull(string? platformId)
272273
{
273-
var router = new MessageRouter(_logger);
274+
var router = new MessageRouter(_logger, null!);
274275

275276
var result = router.GetProvider(platformId!);
276277

‎tests/OpenDeepWiki.Tests/Services/Chat/EmbedServiceAppConfigPropertyTests.cs‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using Microsoft.Extensions.Logging;
66
using Microsoft.Extensions.Logging.Abstractions;
77
using Microsoft.Extensions.Options;
8+
using OpenDeepWiki.Agents;
89
using OpenDeepWiki.EFCore;
910
using OpenDeepWiki.Entities;
1011
using OpenDeepWiki.Services.Chat;
@@ -55,7 +56,7 @@ public Property GetAppConfig_ShouldReturnCorrectAppName()
5556
var statsService = new AppStatisticsService(context, StatsLogger);
5657
var logService = new ChatLogService(context, LogLogger);
5758
var embedService = new EmbedService(
58-
context, chatAppService, statsService, logService, null!, RepoOptions, EmbedLogger);
59+
context, null!, chatAppService, statsService, logService, null!, RepoOptions, EmbedLogger);
5960

6061
// Create app
6162
var app = chatAppService.CreateAppAsync("user1", new CreateChatAppDto
@@ -94,7 +95,7 @@ public Property GetAppConfig_ShouldReturnCorrectAvailableModels()
9495
var statsService = new AppStatisticsService(context, StatsLogger);
9596
var logService = new ChatLogService(context, LogLogger);
9697
var embedService = new EmbedService(
97-
context, chatAppService, statsService, logService, null!, RepoOptions, EmbedLogger);
98+
context, null!, chatAppService, statsService, logService, null!, RepoOptions, EmbedLogger);
9899

99100
// Create app with specific models
100101
var app = chatAppService.CreateAppAsync("user1", new CreateChatAppDto
@@ -137,7 +138,7 @@ public Property GetAppConfig_ShouldReturnCorrectDefaultModel()
137138
var statsService = new AppStatisticsService(context, StatsLogger);
138139
var logService = new ChatLogService(context, LogLogger);
139140
var embedService = new EmbedService(
140-
context, chatAppService, statsService, logService, null!, RepoOptions, EmbedLogger);
141+
context, null!, chatAppService, statsService, logService, null!, RepoOptions, EmbedLogger);
141142

142143
// Create app with specific default model
143144
var app = chatAppService.CreateAppAsync("user1", new CreateChatAppDto
@@ -177,7 +178,7 @@ public Property GetAppConfig_ShouldReturnCorrectIconUrl()
177178
var statsService = new AppStatisticsService(context, StatsLogger);
178179
var logService = new ChatLogService(context, LogLogger);
179180
var embedService = new EmbedService(
180-
context, chatAppService, statsService, logService, null!, RepoOptions, EmbedLogger);
181+
context, null!, chatAppService, statsService, logService, null!, RepoOptions, EmbedLogger);
181182

182183
// Create app with icon
183184
var app = chatAppService.CreateAppAsync("user1", new CreateChatAppDto
@@ -255,7 +256,7 @@ public Property UpdateApp_ShouldReflectChanges()
255256
var statsService = new AppStatisticsService(context, StatsLogger);
256257
var logService = new ChatLogService(context, LogLogger);
257258
var embedService = new EmbedService(
258-
context, chatAppService, statsService, logService, null!, RepoOptions, EmbedLogger);
259+
context, null!, chatAppService, statsService, logService, null!, RepoOptions, EmbedLogger);
259260

260261
// Create app
261262
var app = chatAppService.CreateAppAsync("user1", new CreateChatAppDto
@@ -301,7 +302,7 @@ public Property GetAppConfig_WithDomainValidation_ShouldValidateCorrectly()
301302
var statsService = new AppStatisticsService(context, StatsLogger);
302303
var logService = new ChatLogService(context, LogLogger);
303304
var embedService = new EmbedService(
304-
context, chatAppService, statsService, logService, null!, RepoOptions, EmbedLogger);
305+
context, null!, chatAppService, statsService, logService, null!, RepoOptions, EmbedLogger);
305306

306307
// Create app with domain validation
307308
var app = chatAppService.CreateAppAsync("user1", new CreateChatAppDto

‎tests/OpenDeepWiki.Tests/Services/Chat/EmbedServiceSecurityPropertyTests.cs‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
using OpenDeepWiki.Services.Chat;
1111
using OpenDeepWiki.Services.Repositories;
1212

13+
1314
namespace OpenDeepWiki.Tests.Services.Chat;
1415

1516
/// <summary>
@@ -54,7 +55,7 @@ public Property InvalidAppId_ShouldBeRejected()
5455
var statsService = new AppStatisticsService(context, StatsLogger);
5556
var logService = new ChatLogService(context, LogLogger);
5657
var embedService = new EmbedService(
57-
context, chatAppService, statsService, logService, null!, RepoOptions, EmbedLogger);
58+
context, null!, chatAppService, statsService, logService, null!, RepoOptions, EmbedLogger);
5859

5960
var (isValid, errorCode, _) = embedService.ValidateAppAsync(invalidAppId)
6061
.GetAwaiter().GetResult();
@@ -81,7 +82,7 @@ public Property ValidAppId_ShouldBeAccepted()
8182
var statsService = new AppStatisticsService(context, StatsLogger);
8283
var logService = new ChatLogService(context, LogLogger);
8384
var embedService = new EmbedService(
84-
context, chatAppService, statsService, logService, null!, RepoOptions, EmbedLogger);
85+
context, null!, chatAppService, statsService, logService, null!, RepoOptions, EmbedLogger);
8586

8687
// Create a valid app
8788
var app = chatAppService.CreateAppAsync("user1", new CreateChatAppDto
@@ -118,7 +119,7 @@ public Property InactiveApp_ShouldBeRejected()
118119
var statsService = new AppStatisticsService(context, StatsLogger);
119120
var logService = new ChatLogService(context, LogLogger);
120121
var embedService = new EmbedService(
121-
context, chatAppService, statsService, logService, null!, RepoOptions, EmbedLogger);
122+
context, null!, chatAppService, statsService, logService, null!, RepoOptions, EmbedLogger);
122123

123124
// Create and then deactivate the app
124125
var app = chatAppService.CreateAppAsync("user1", new CreateChatAppDto
@@ -160,7 +161,7 @@ public Property AppWithoutApiKey_ShouldBeRejected()
160161
var statsService = new AppStatisticsService(context, StatsLogger);
161162
var logService = new ChatLogService(context, LogLogger);
162163
var embedService = new EmbedService(
163-
context, chatAppService, statsService, logService, null!, RepoOptions, EmbedLogger);
164+
context, null!, chatAppService, statsService, logService, null!, RepoOptions, EmbedLogger);
164165

165166
// Create app without API key
166167
var app = chatAppService.CreateAppAsync("user1", new CreateChatAppDto
@@ -205,7 +206,7 @@ public Property DomainValidationEnabled_NonAllowedDomain_ShouldBeRejected()
205206
var statsService = new AppStatisticsService(context, StatsLogger);
206207
var logService = new ChatLogService(context, LogLogger);
207208
var embedService = new EmbedService(
208-
context, chatAppService, statsService, logService, null!, RepoOptions, EmbedLogger);
209+
context, null!, chatAppService, statsService, logService, null!, RepoOptions, EmbedLogger);
209210

210211
// Create app with domain validation enabled
211212
var app = chatAppService.CreateAppAsync("user1", new CreateChatAppDto
@@ -244,7 +245,7 @@ public Property DomainValidationEnabled_AllowedDomain_ShouldBeAccepted()
244245
var statsService = new AppStatisticsService(context, StatsLogger);
245246
var logService = new ChatLogService(context, LogLogger);
246247
var embedService = new EmbedService(
247-
context, chatAppService, statsService, logService, null!, RepoOptions, EmbedLogger);
248+
context, null!, chatAppService, statsService, logService, null!, RepoOptions, EmbedLogger);
248249

249250
// Create app with domain validation enabled
250251
var app = chatAppService.CreateAppAsync("user1", new CreateChatAppDto
@@ -283,7 +284,7 @@ public Property DomainValidationDisabled_AnyDomain_ShouldBeAccepted()
283284
var statsService = new AppStatisticsService(context, StatsLogger);
284285
var logService = new ChatLogService(context, LogLogger);
285286
var embedService = new EmbedService(
286-
context, chatAppService, statsService, logService, null!, RepoOptions, EmbedLogger);
287+
context, null!, chatAppService, statsService, logService, null!, RepoOptions, EmbedLogger);
287288

288289
// Create app with domain validation disabled
289290
var app = chatAppService.CreateAppAsync("user1", new CreateChatAppDto

0 commit comments

Comments
 (0)