Skip to content

Commit 66900fd

Browse files
authored
Fix follow-up MCP token review feedback (#1044)
1 parent 79022be commit 66900fd

5 files changed

Lines changed: 49 additions & 14 deletions

File tree

EssentialCSharp.Web.Tests/McpApiTokenServiceTests.cs

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,30 @@ public async Task CreateTokenAsync_WithExpiryBeyondSixMonths_Throws()
4545

4646
using var scope = factory.Services.CreateScope();
4747
var tokenService = scope.ServiceProvider.GetRequiredService<McpApiTokenService>();
48-
DateTime requestedExpiry = McpApiTokenService.GetDefaultExpirationUtc(DateTime.UtcNow).AddDays(1);
48+
DateTime requestedExpiry = McpApiTokenService.GetDefaultExpirationUtc(DateTime.UtcNow).AddDays(2);
4949

5050
await Assert.That(() => tokenService.CreateTokenAsync(userId, "too-long", requestedExpiry))
5151
.Throws<ArgumentOutOfRangeException>()
52-
.WithMessageContaining("6 months");
52+
.WithMessageContaining(McpApiTokenService.MaxExpiryValidationMessage);
53+
}
54+
55+
[Test]
56+
public async Task CreateTokenAsync_WithExplicitCreatedAt_UsesReferenceTimeForDefaultExpiry()
57+
{
58+
string userId = await McpTestHelper.CreateUserAsync(factory, "mcp-explicit-created-at");
59+
60+
using var scope = factory.Services.CreateScope();
61+
var tokenService = scope.ServiceProvider.GetRequiredService<McpApiTokenService>();
62+
DateTime createdAtUtc = new(2026, 4, 30, 23, 59, 59, DateTimeKind.Utc);
63+
64+
(_, var entity) = await tokenService.CreateTokenAsync(
65+
userId,
66+
"explicit-created-at",
67+
createdAtUtc: createdAtUtc);
68+
69+
await Assert.That(entity.CreatedAt).IsEqualTo(createdAtUtc);
70+
await Assert.That(entity.ExpiresAt).IsNotNull();
71+
await Assert.That(entity.ExpiresAt!.Value)
72+
.IsEqualTo(McpApiTokenService.GetDefaultExpirationUtc(createdAtUtc));
5373
}
5474
}

EssentialCSharp.Web/Areas/Identity/Pages/Account/Manage/McpAccess.cshtml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
@page
22
@model McpAccessModel
33
@using EssentialCSharp.Web.Models
4+
@using EssentialCSharp.Web.Services
45
@{
56
ViewData["Title"] = "MCP Access";
67
ViewData["ActivePage"] = ManageNavPages.McpAccess;
@@ -61,7 +62,7 @@
6162
<label asp-for="ExpiresOn" class="form-label">Expiry Date</label>
6263
<input asp-for="ExpiresOn" type="date" class="form-control" max="@Model.MaxExpiresOn.ToString("yyyy-MM-dd")" />
6364
<span asp-validation-for="ExpiresOn" class="text-danger"></span>
64-
<div class="form-text">MCP tokens default to 6 months and cannot exceed 6 months from today. The token expires at end of day (23:59:59) UTC on the selected date.</div>
65+
<div class="form-text">MCP tokens expire after @McpApiTokenService.DefaultLifetimeMonths months by default, with a maximum lifetime of @McpApiTokenService.DefaultLifetimeMonths months from today. The token expires at end of day (UTC) on the selected date.</div>
6566
</div>
6667
<button type="submit" class="btn btn-primary">Create Token</button>
6768
</form>

EssentialCSharp.Web/Areas/Identity/Pages/Account/Manage/McpAccess.cshtml.cs

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@ public class McpAccessModel(
3333
public async Task<IActionResult> OnGetAsync()
3434
{
3535
DisableCaching();
36-
InitializeExpiryBounds();
36+
DateTime nowUtc = DateTime.UtcNow;
37+
InitializeExpiryBounds(nowUtc);
3738
ApplyDefaultExpiryIfMissing();
3839
string? userId = userManager.GetUserId(User);
3940
if (userId is null) return Challenge();
@@ -44,7 +45,9 @@ public async Task<IActionResult> OnGetAsync()
4445
public async Task<IActionResult> OnPostCreateAsync()
4546
{
4647
DisableCaching();
47-
InitializeExpiryBounds();
48+
DateTime nowUtc = DateTime.UtcNow;
49+
DateOnly todayUtc = DateOnly.FromDateTime(nowUtc);
50+
InitializeExpiryBounds(nowUtc);
4851
ApplyDefaultExpiryIfMissing();
4952
string? userId = userManager.GetUserId(User);
5053
if (userId is null) return Challenge();
@@ -54,7 +57,7 @@ public async Task<IActionResult> OnPostCreateAsync()
5457

5558
if (ExpiresOn.HasValue)
5659
{
57-
if (ExpiresOn.Value < DateOnly.FromDateTime(DateTime.UtcNow))
60+
if (ExpiresOn.Value < todayUtc)
5861
ModelState.AddModelError(nameof(ExpiresOn), "Expiry date must be today or in the future.");
5962
if (ExpiresOn.Value > MaxExpiresOn)
6063
ModelState.AddModelError(nameof(ExpiresOn), McpApiTokenService.MaxExpiryValidationMessage);
@@ -69,7 +72,11 @@ public async Task<IActionResult> OnPostCreateAsync()
6972
// Convert date-only boundary to end-of-day UTC instant before persisting
7073
DateTime? expiresAt = ExpiresOn?.ToDateTime(TimeOnly.MaxValue, DateTimeKind.Utc);
7174

72-
var (rawToken, entity) = await tokenService.CreateTokenAsync(userId, TokenName.Trim(), expiresAt);
75+
var (rawToken, entity) = await tokenService.CreateTokenAsync(
76+
userId,
77+
TokenName.Trim(),
78+
expiresAt,
79+
createdAtUtc: nowUtc);
7380
GeneratedToken = rawToken;
7481
GeneratedTokenEntity = entity;
7582
UserTokens = await tokenService.GetUserTokensAsync(userId);
@@ -97,9 +104,9 @@ private void DisableCaching()
97104
Response.Headers.Expires = "0";
98105
}
99106

100-
private void InitializeExpiryBounds()
107+
private void InitializeExpiryBounds(DateTime nowUtc)
101108
{
102-
MaxExpiresOn = McpApiTokenService.GetDefaultExpiryDate();
109+
MaxExpiresOn = McpApiTokenService.GetDefaultExpiryDate(nowUtc);
103110
}
104111

105112
private void ApplyDefaultExpiryIfMissing()

EssentialCSharp.Web/Controllers/McpTokenController.cs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,13 @@ public async Task<IActionResult> CreateToken(
2626
if (name.Length > 256)
2727
return BadRequest(new { Error = "Token name must be 256 characters or fewer." });
2828

29+
DateTime nowUtc = DateTime.UtcNow;
30+
DateOnly todayUtc = DateOnly.FromDateTime(nowUtc);
2931
DateTime? expiresAt = null;
30-
DateOnly maxExpiresOn = McpApiTokenService.GetDefaultExpiryDate();
32+
DateOnly maxExpiresOn = McpApiTokenService.GetDefaultExpiryDate(nowUtc);
3133
if (request?.ExpiresOn is DateOnly expiresOn)
3234
{
33-
if (expiresOn < DateOnly.FromDateTime(DateTime.UtcNow))
35+
if (expiresOn < todayUtc)
3436
return BadRequest(new { Error = "ExpiresOn must be today or in the future." });
3537
if (expiresOn > maxExpiresOn)
3638
return BadRequest(new { Error = McpApiTokenService.MaxExpiryValidationMessage });
@@ -39,7 +41,11 @@ public async Task<IActionResult> CreateToken(
3941
}
4042

4143
var (rawToken, entity) = await tokenService.CreateTokenAsync(
42-
userId, name, expiresAt, cancellationToken);
44+
userId,
45+
name,
46+
expiresAt,
47+
createdAtUtc: nowUtc,
48+
cancellationToken: cancellationToken);
4349

4450
return Ok(new
4551
{

EssentialCSharp.Web/Services/McpApiTokenService.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ namespace EssentialCSharp.Web.Services;
1010
public class McpApiTokenService(EssentialCSharpWebContext db)
1111
{
1212
public const int DefaultLifetimeMonths = 6;
13-
public const string MaxExpiryValidationMessage = "MCP tokens can expire at most 6 months from today.";
13+
public static readonly string MaxExpiryValidationMessage = $"MCP tokens can expire at most {DefaultLifetimeMonths} months from today.";
1414

1515
public sealed record ResolvedMcpApiToken(Guid TokenId, string UserId);
1616

@@ -36,10 +36,11 @@ public static string GenerateRawToken()
3636
string userId,
3737
string name,
3838
DateTime? expiresAt = null,
39+
DateTime? createdAtUtc = null,
3940
CancellationToken cancellationToken = default)
4041
{
4142
string raw = GenerateRawToken();
42-
DateTime createdAt = DateTime.UtcNow;
43+
DateTime createdAt = createdAtUtc ?? DateTime.UtcNow;
4344
DateTime effectiveExpiration = ResolveExpiration(expiresAt, createdAt);
4445

4546
var entity = new McpApiToken

0 commit comments

Comments
 (0)