-
Notifications
You must be signed in to change notification settings - Fork 0
Backend stabilization: subscription enforcement and idempotency security fix #145
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
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
81f8aa3
Initial plan
Copilot e90e80b
Add subscription enforcement to token deployment endpoints
Copilot 148c8e6
Add comprehensive unit tests for token deployment subscription enforc…
Copilot 443821a
Fix critical security issue in idempotency implementation with reques…
Copilot 2f1ed90
Address code review feedback: improve logging and deployment recording
Copilot File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,8 @@ | |
| using Microsoft.AspNetCore.Mvc.Filters; | ||
| using System.Collections.Concurrent; | ||
| using System.Text.Json; | ||
| using System.Security.Cryptography; | ||
| using System.Text; | ||
|
|
||
| namespace BiatecTokensApi.Filters | ||
| { | ||
|
|
@@ -12,6 +14,10 @@ namespace BiatecTokensApi.Filters | |
| /// This filter ensures that duplicate requests with the same idempotency key return the same response. | ||
| /// It stores the response in memory for a configurable duration (default 24 hours). | ||
| /// | ||
| /// **Security:** The filter validates that cached requests match current request parameters. | ||
| /// If the same idempotency key is used with different parameters, a warning is logged and | ||
| /// the request is rejected to prevent bypassing business logic. | ||
| /// | ||
| /// Usage: | ||
| /// [IdempotencyKey] | ||
| /// [HttpPost("create")] | ||
|
|
@@ -49,28 +55,57 @@ public override async Task OnActionExecutionAsync(ActionExecutingContext context | |
| } | ||
|
|
||
| var key = idempotencyKey.ToString(); | ||
| var logger = context.HttpContext.RequestServices.GetService<ILogger<IdempotencyKeyAttribute>>(); | ||
|
|
||
| // Clean up expired entries periodically | ||
| CleanupExpiredEntries(); | ||
|
|
||
| // Compute hash of request parameters for validation | ||
| var requestHash = ComputeRequestHash(context, context.ActionArguments); | ||
|
|
||
| // Check if we've seen this key before | ||
| if (_cache.TryGetValue(key, out var record)) | ||
| { | ||
| // Check if the record has expired | ||
| if (DateTime.UtcNow - record.Timestamp < Expiration) | ||
| { | ||
| // Validate that the request parameters match | ||
| if (record.RequestHash != requestHash) | ||
| { | ||
| logger?.LogWarning( | ||
| "Idempotency key reused with different parameters. Key: {Key}, CorrelationId: {CorrelationId}", | ||
| key, context.HttpContext.TraceIdentifier); | ||
|
|
||
| context.Result = new BadRequestObjectResult(new | ||
| { | ||
| success = false, | ||
| errorCode = "IDEMPOTENCY_KEY_MISMATCH", | ||
| errorMessage = "The provided idempotency key has been used with different request parameters. Please use a unique key for this request or reuse the same parameters.", | ||
| correlationId = context.HttpContext.TraceIdentifier | ||
| }); | ||
|
Comment on lines
+79
to
+85
|
||
| return; | ||
| } | ||
|
|
||
| // Return cached response | ||
| context.Result = new ObjectResult(record.Response) | ||
| { | ||
| StatusCode = record.StatusCode | ||
| }; | ||
| context.HttpContext.Response.Headers.Add("X-Idempotency-Hit", "true"); | ||
| context.HttpContext.Response.Headers["X-Idempotency-Hit"] = "true"; | ||
|
|
||
| logger?.LogDebug( | ||
| "Idempotency cache hit. Key: {Key}, CorrelationId: {CorrelationId}", | ||
| key, context.HttpContext.TraceIdentifier); | ||
|
|
||
| return; | ||
| } | ||
| else | ||
| { | ||
| // Expired - remove it and continue | ||
| _cache.TryRemove(key, out _); | ||
| logger?.LogDebug( | ||
| "Idempotency record expired and removed. Key: {Key}", | ||
| key); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -85,11 +120,49 @@ public override async Task OnActionExecutionAsync(ActionExecutingContext context | |
| Key = key, | ||
| Response = objectResult.Value, | ||
| StatusCode = objectResult.StatusCode ?? 200, | ||
| Timestamp = DateTime.UtcNow | ||
| Timestamp = DateTime.UtcNow, | ||
| RequestHash = requestHash | ||
| }; | ||
|
|
||
| _cache.TryAdd(key, newRecord); | ||
| executedContext.HttpContext.Response.Headers.Add("X-Idempotency-Hit", "false"); | ||
| executedContext.HttpContext.Response.Headers["X-Idempotency-Hit"] = "false"; | ||
|
|
||
| logger?.LogDebug( | ||
| "Idempotency record cached. Key: {Key}, StatusCode: {StatusCode}", | ||
| key, newRecord.StatusCode); | ||
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Computes a hash of the request parameters for validation | ||
| /// </summary> | ||
| /// <param name="context">Action executing context</param> | ||
| /// <param name="arguments">Action arguments</param> | ||
| /// <returns>Hash string</returns> | ||
| private string ComputeRequestHash(ActionExecutingContext context, IDictionary<string, object?> arguments) | ||
| { | ||
| try | ||
| { | ||
| // Serialize arguments to JSON for consistent hashing | ||
| var json = JsonSerializer.Serialize(arguments, new JsonSerializerOptions | ||
| { | ||
| WriteIndented = false, | ||
| DefaultIgnoreCondition = System.Text.Json.Serialization.JsonIgnoreCondition.Never | ||
| }); | ||
|
|
||
| // Compute SHA256 hash | ||
| using var sha256 = SHA256.Create(); | ||
| var hashBytes = sha256.ComputeHash(Encoding.UTF8.GetBytes(json)); | ||
| return Convert.ToBase64String(hashBytes); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| // Log serialization failure and return empty hash | ||
| var logger = context.HttpContext?.RequestServices?.GetService<ILogger<IdempotencyKeyAttribute>>(); | ||
| logger?.LogWarning(ex, "Failed to compute request hash for idempotency check. Returning empty hash."); | ||
|
|
||
| // Return empty hash (will not match cached requests, treating as new request) | ||
| return string.Empty; | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -122,6 +195,7 @@ private class IdempotencyRecord | |
| public object? Response { get; set; } | ||
| public int StatusCode { get; set; } | ||
| public DateTime Timestamp { get; set; } | ||
| public string RequestHash { get; set; } = string.Empty; | ||
| } | ||
| } | ||
| } | ||
124 changes: 124 additions & 0 deletions
124
BiatecTokensApi/Filters/TokenDeploymentSubscriptionAttribute.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,124 @@ | ||
| using BiatecTokensApi.Helpers; | ||
| using BiatecTokensApi.Models; | ||
| using BiatecTokensApi.Services.Interface; | ||
| using Microsoft.AspNetCore.Mvc; | ||
| using Microsoft.AspNetCore.Mvc.Filters; | ||
|
|
||
| namespace BiatecTokensApi.Filters | ||
| { | ||
| /// <summary> | ||
| /// Action filter that validates subscription tier entitlements for token deployment operations | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// This filter enforces subscription-based access control by validating that users have | ||
| /// appropriate tier permissions and available deployment quota before proceeding with token deployment. | ||
| /// When validation fails, it returns a clear error response that prompts users to upgrade their subscription. | ||
| /// | ||
| /// **Enforcement Policy:** | ||
| /// - Free tier: Limited token deployments (3) | ||
| /// - Basic tier: Moderate token deployments (10) | ||
| /// - Premium tier: Generous token deployments (50) | ||
| /// - Enterprise tier: Unlimited token deployments | ||
| /// | ||
| /// **Usage:** | ||
| /// Apply this attribute to token deployment endpoints: | ||
| /// [TokenDeploymentSubscription] | ||
| /// [HttpPost("create")] | ||
| /// public async Task<IActionResult> CreateToken(...) { ... } | ||
| /// </remarks> | ||
| [AttributeUsage(AttributeTargets.Method, AllowMultiple = false, Inherited = true)] | ||
| public class TokenDeploymentSubscriptionAttribute : ActionFilterAttribute | ||
| { | ||
| /// <summary> | ||
| /// Called before the action executes to validate subscription tier and deployment quota | ||
| /// </summary> | ||
| /// <param name="context">The action executing context</param> | ||
| public override async Task OnActionExecutionAsync(ActionExecutingContext context, ActionExecutionDelegate next) | ||
| { | ||
| // Get required services from DI | ||
| var tierService = context.HttpContext.RequestServices.GetService<ISubscriptionTierService>(); | ||
| var logger = context.HttpContext.RequestServices.GetService<ILogger<TokenDeploymentSubscriptionAttribute>>(); | ||
|
|
||
| if (tierService == null || logger == null) | ||
| { | ||
| logger?.LogWarning("TokenDeploymentSubscriptionAttribute: Required services not available"); | ||
| await next(); | ||
| return; | ||
| } | ||
|
Comment on lines
+42
to
+47
|
||
|
|
||
| // Get user address from claims | ||
| var userAddress = context.HttpContext.User.Identity?.Name; | ||
| if (string.IsNullOrEmpty(userAddress)) | ||
| { | ||
| logger.LogWarning("TokenDeploymentSubscriptionAttribute: No user identity found"); | ||
| context.Result = new UnauthorizedObjectResult(new ApiErrorResponse | ||
| { | ||
| Success = false, | ||
| ErrorCode = ErrorCodes.UNAUTHORIZED, | ||
| ErrorMessage = "Authentication required", | ||
| RemediationHint = "Provide a valid ARC-0014 authentication token in the Authorization header.", | ||
| Timestamp = DateTime.UtcNow, | ||
| CorrelationId = context.HttpContext.TraceIdentifier, | ||
| Path = LoggingHelper.SanitizeLogInput(context.HttpContext.Request.Path) | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| // Sanitize user address for logging | ||
| var sanitizedUserAddress = LoggingHelper.SanitizeLogInput(userAddress); | ||
|
|
||
| // Check if user can deploy tokens | ||
| var canDeploy = await tierService.CanDeployTokenAsync(userAddress); | ||
|
|
||
| if (!canDeploy) | ||
| { | ||
| // Get tier details for error message | ||
| var currentTier = await tierService.GetUserTierAsync(userAddress); | ||
| var tierLimits = tierService.GetTierLimits(currentTier); | ||
| var currentCount = await tierService.GetTokenDeploymentCountAsync(userAddress); | ||
|
|
||
| logger.LogWarning( | ||
| "Token deployment denied for user {UserAddress}: Deployment limit reached. Current tier: {Tier}, Current count: {Count}, Max: {Max}. CorrelationId: {CorrelationId}", | ||
| sanitizedUserAddress, currentTier, currentCount, tierLimits.MaxTokenDeployments, context.HttpContext.TraceIdentifier); | ||
|
|
||
| context.Result = new ObjectResult(new ApiErrorResponse | ||
| { | ||
| Success = false, | ||
| ErrorCode = ErrorCodes.SUBSCRIPTION_LIMIT_REACHED, | ||
| ErrorMessage = $"Token deployment limit reached for {tierLimits.TierName} tier. You have deployed {currentCount} of {tierLimits.MaxTokenDeployments} allowed tokens.", | ||
| RemediationHint = $"Upgrade to a higher tier to deploy more tokens. Current tier: {tierLimits.TierName}. Visit the billing page to upgrade your subscription.", | ||
| Details = new Dictionary<string, object> | ||
| { | ||
| { "currentTier", currentTier.ToString() }, | ||
| { "currentDeployments", currentCount }, | ||
| { "maxDeployments", tierLimits.MaxTokenDeployments }, | ||
| { "tierDescription", tierLimits.Description } | ||
| }, | ||
| Timestamp = DateTime.UtcNow, | ||
| CorrelationId = context.HttpContext.TraceIdentifier, | ||
| Path = LoggingHelper.SanitizeLogInput(context.HttpContext.Request.Path) | ||
| }) | ||
| { | ||
| StatusCode = StatusCodes.Status402PaymentRequired | ||
| }; | ||
| return; | ||
| } | ||
|
|
||
| // Execute the action | ||
| var executedContext = await next(); | ||
|
|
||
| // If the action was successful (2xx status code), record the deployment | ||
| var statusCode = executedContext.HttpContext.Response.StatusCode; | ||
| if (statusCode >= 200 && statusCode < 300) | ||
| { | ||
| // Record the deployment | ||
| await tierService.RecordTokenDeploymentAsync(userAddress); | ||
|
|
||
| var newCount = await tierService.GetTokenDeploymentCountAsync(userAddress); | ||
| logger.LogInformation( | ||
| "Token deployment recorded for user {UserAddress}. New deployment count: {Count}. CorrelationId: {CorrelationId}", | ||
| sanitizedUserAddress, newCount, context.HttpContext.TraceIdentifier); | ||
| } | ||
| } | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
The error code "IDEMPOTENCY_KEY_MISMATCH" is hardcoded as a string literal instead of being defined as a constant in the ErrorCodes class. All other error codes in the codebase are defined in BiatecTokensApi/Models/ErrorCodes.cs (e.g., ErrorCodes.SUBSCRIPTION_LIMIT_REACHED, ErrorCodes.UNAUTHORIZED). This hardcoded string breaks the established pattern and makes the code harder to maintain and test. Add IDEMPOTENCY_KEY_MISMATCH as a constant in the ErrorCodes class and reference it using ErrorCodes.IDEMPOTENCY_KEY_MISMATCH.