-
Notifications
You must be signed in to change notification settings - Fork 8
Add 6 month default and max MCP token expiry #1043
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| using EssentialCSharp.Web.Services; | ||
| using Microsoft.AspNetCore.Mvc.Testing; | ||
| using Microsoft.Extensions.DependencyInjection; | ||
|
|
||
| namespace EssentialCSharp.Web.Tests; | ||
|
|
||
| [NotInParallel("McpTests")] | ||
| [ClassDataSource<WebApplicationFactory>(Shared = SharedType.PerClass)] | ||
| public class McpApiTokenServiceTests(WebApplicationFactory factory) | ||
| { | ||
| [Test] | ||
| public async Task CreateTokenAsync_WithoutExpiry_UsesSixMonthDefault() | ||
| { | ||
| string userId = await McpTestHelper.CreateUserAsync(factory, "mcp-default-expiry"); | ||
|
|
||
| using var scope = factory.Services.CreateScope(); | ||
| var tokenService = scope.ServiceProvider.GetRequiredService<McpApiTokenService>(); | ||
|
|
||
| (_, var entity) = await tokenService.CreateTokenAsync(userId, "default-expiry"); | ||
|
|
||
| await Assert.That(entity.ExpiresAt).IsNotNull(); | ||
| await Assert.That(entity.ExpiresAt!.Value) | ||
| .IsEqualTo(McpApiTokenService.GetDefaultExpirationUtc(entity.CreatedAt)); | ||
| } | ||
|
|
||
| [Test] | ||
| public async Task CreateTokenAsync_WithExpiryWithinSixMonths_UsesRequestedExpiry() | ||
| { | ||
| string userId = await McpTestHelper.CreateUserAsync(factory, "mcp-custom-expiry"); | ||
|
|
||
| using var scope = factory.Services.CreateScope(); | ||
| var tokenService = scope.ServiceProvider.GetRequiredService<McpApiTokenService>(); | ||
| DateTime requestedExpiry = DateTime.UtcNow.AddMonths(3); | ||
|
|
||
| (_, var entity) = await tokenService.CreateTokenAsync(userId, "custom-expiry", requestedExpiry); | ||
|
|
||
| await Assert.That(entity.ExpiresAt).IsNotNull(); | ||
| await Assert.That(entity.ExpiresAt!.Value).IsEqualTo(requestedExpiry); | ||
| } | ||
|
|
||
| [Test] | ||
| public async Task CreateTokenAsync_WithExpiryBeyondSixMonths_Throws() | ||
| { | ||
| string userId = await McpTestHelper.CreateUserAsync(factory, "mcp-max-expiry"); | ||
|
|
||
| using var scope = factory.Services.CreateScope(); | ||
| var tokenService = scope.ServiceProvider.GetRequiredService<McpApiTokenService>(); | ||
| DateTime requestedExpiry = McpApiTokenService.GetDefaultExpirationUtc(DateTime.UtcNow).AddDays(1); | ||
|
|
||
| await Assert.That(() => tokenService.CreateTokenAsync(userId, "too-long", requestedExpiry)) | ||
| .Throws<ArgumentOutOfRangeException>() | ||
| .WithMessageContaining("6 months"); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -58,10 +58,10 @@ | |||||
| <span asp-validation-for="TokenName" class="text-danger"></span> | ||||||
| </div> | ||||||
| <div class="mb-3"> | ||||||
| <label asp-for="ExpiresOn" class="form-label">Expiry Date <span class="text-muted">(optional)</span></label> | ||||||
| <input asp-for="ExpiresOn" type="date" class="form-control" /> | ||||||
| <label asp-for="ExpiresOn" class="form-label">Expiry Date</label> | ||||||
| <input asp-for="ExpiresOn" type="date" class="form-control" max="@Model.MaxExpiresOn.ToString("yyyy-MM-dd")" /> | ||||||
| <span asp-validation-for="ExpiresOn" class="text-danger"></span> | ||||||
| <div class="form-text">Leave blank for a non-expiring token. The token expires at end of day (23:59:59) UTC on the selected date.</div> | ||||||
| <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> | ||||||
|
||||||
| <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> | |
| <div class="form-text">MCP tokens default to 6 months and cannot exceed 6 months from today. The token expires at end of day (UTC) on the selected date.</div> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,8 @@ public class McpAccessModel( | |
|
|
||
| public List<McpApiToken> UserTokens { get; private set; } = []; | ||
|
|
||
| public DateOnly MaxExpiresOn { get; private set; } | ||
|
|
||
| [BindProperty] | ||
| [StringLength(256, ErrorMessage = "Token name must be 256 characters or fewer.")] | ||
| public string TokenName { get; set; } = "My Token"; | ||
|
|
@@ -31,6 +33,8 @@ public class McpAccessModel( | |
| public async Task<IActionResult> OnGetAsync() | ||
| { | ||
| DisableCaching(); | ||
| InitializeExpiryBounds(); | ||
| ApplyDefaultExpiryIfMissing(); | ||
| string? userId = userManager.GetUserId(User); | ||
| if (userId is null) return Challenge(); | ||
| UserTokens = await tokenService.GetUserTokensAsync(userId); | ||
|
|
@@ -40,14 +44,21 @@ public async Task<IActionResult> OnGetAsync() | |
| public async Task<IActionResult> OnPostCreateAsync() | ||
| { | ||
| DisableCaching(); | ||
| InitializeExpiryBounds(); | ||
| ApplyDefaultExpiryIfMissing(); | ||
| string? userId = userManager.GetUserId(User); | ||
| if (userId is null) return Challenge(); | ||
|
|
||
| if (string.IsNullOrWhiteSpace(TokenName)) | ||
| ModelState.AddModelError(nameof(TokenName), "Token name is required."); | ||
|
|
||
| if (ExpiresOn.HasValue && ExpiresOn.Value < DateOnly.FromDateTime(DateTime.UtcNow)) | ||
| ModelState.AddModelError(nameof(ExpiresOn), "Expiry date must be today or in the future."); | ||
| if (ExpiresOn.HasValue) | ||
| { | ||
| if (ExpiresOn.Value < DateOnly.FromDateTime(DateTime.UtcNow)) | ||
| ModelState.AddModelError(nameof(ExpiresOn), "Expiry date must be today or in the future."); | ||
| if (ExpiresOn.Value > MaxExpiresOn) | ||
| ModelState.AddModelError(nameof(ExpiresOn), McpApiTokenService.MaxExpiryValidationMessage); | ||
|
Comment on lines
46
to
+60
|
||
| } | ||
|
|
||
| if (!ModelState.IsValid) | ||
| { | ||
|
|
@@ -85,4 +96,14 @@ private void DisableCaching() | |
| Response.Headers.Pragma = "no-cache"; | ||
| Response.Headers.Expires = "0"; | ||
| } | ||
|
|
||
| private void InitializeExpiryBounds() | ||
| { | ||
| MaxExpiresOn = McpApiTokenService.GetDefaultExpiryDate(); | ||
| } | ||
|
|
||
| private void ApplyDefaultExpiryIfMissing() | ||
| { | ||
| ExpiresOn ??= MaxExpiresOn; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -27,10 +27,13 @@ public async Task<IActionResult> CreateToken( | |||||||||||||||||||||||||||
| return BadRequest(new { Error = "Token name must be 256 characters or fewer." }); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| DateTime? expiresAt = null; | ||||||||||||||||||||||||||||
| DateOnly maxExpiresOn = McpApiTokenService.GetDefaultExpiryDate(); | ||||||||||||||||||||||||||||
| if (request?.ExpiresOn is DateOnly expiresOn) | ||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||
| if (expiresOn < DateOnly.FromDateTime(DateTime.UtcNow)) | ||||||||||||||||||||||||||||
|
Comment on lines
29
to
33
|
||||||||||||||||||||||||||||
| DateTime? expiresAt = null; | |
| DateOnly maxExpiresOn = McpApiTokenService.GetDefaultExpiryDate(); | |
| if (request?.ExpiresOn is DateOnly expiresOn) | |
| { | |
| if (expiresOn < DateOnly.FromDateTime(DateTime.UtcNow)) | |
| DateTime nowUtc = DateTime.UtcNow; | |
| DateOnly todayUtc = DateOnly.FromDateTime(nowUtc); | |
| DateTime? expiresAt = null; | |
| DateOnly maxExpiresOn = McpApiTokenService.GetDefaultExpiryDate(nowUtc); | |
| if (request?.ExpiresOn is DateOnly expiresOn) | |
| { | |
| if (expiresOn < todayUtc) |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -9,8 +9,17 @@ namespace EssentialCSharp.Web.Services; | |||||
|
|
||||||
| public class McpApiTokenService(EssentialCSharpWebContext db) | ||||||
| { | ||||||
| public const int DefaultLifetimeMonths = 6; | ||||||
| public const string MaxExpiryValidationMessage = "MCP tokens can expire at most 6 months from today."; | ||||||
|
||||||
| public const string MaxExpiryValidationMessage = "MCP tokens can expire at most 6 months from today."; | |
| public static string MaxExpiryValidationMessage => $"MCP tokens can expire at most {DefaultLifetimeMonths} months from today."; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test can be flaky around a UTC midnight boundary:
requestedExpiryis computed fromDateTime.UtcNow, butCreateTokenAsynccomputescreatedAtfrom a separateUtcNow. If the date rolls over between those calls, the service's computed max can advance by a day and therequestedExpirymay no longer be beyond the max, causing intermittent failures. Consider makingrequestedExpiryunambiguously beyond any possible max (e.g., more than 1 day past), or refactoring token creation to accept/inject a reference time so tests can be deterministic.