Implemented all 3 critical fixes identified in Day 6 Architecture Gap Analysis:
**Fix 1: UpdateUserRole Feature (RESTful PUT endpoint)**
- Created UpdateUserRoleCommand and UpdateUserRoleCommandHandler
- Added PUT /api/tenants/{tenantId}/users/{userId}/role endpoint
- Implements self-demotion prevention (cannot demote self from TenantOwner)
- Implements last owner protection (cannot remove last TenantOwner)
- Returns UserWithRoleDto with updated role information
- Follows RESTful best practices (PUT for updates)
**Fix 2: Last TenantOwner Deletion Prevention (Security)**
- Verified CountByTenantAndRoleAsync repository method exists
- Verified IsLastTenantOwnerAsync validation in RemoveUserFromTenantCommandHandler
- UpdateUserRoleCommandHandler now prevents:
* Self-demotion from TenantOwner role
* Removing the last TenantOwner from tenant
- SECURITY: Prevents tenant from becoming ownerless (critical vulnerability fix)
**Fix 3: Database-Backed Rate Limiting (Security & Reliability)**
- Created EmailRateLimit entity with proper domain logic
- Added EmailRateLimitConfiguration for EF Core
- Implemented DatabaseEmailRateLimiter service (replaces MemoryRateLimitService)
- Updated DependencyInjection to use database-backed implementation
- Created database migration: AddEmailRateLimitsTable
- Added composite unique index on (email, tenant_id, operation_type)
- SECURITY: Rate limit state persists across server restarts (prevents email bombing)
- Implements cleanup logic for expired rate limit records
**Testing:**
- Added 9 comprehensive integration tests in Day8GapFixesTests.cs
- Fix 1: 3 tests (valid update, self-demote prevention, idempotency)
- Fix 2: 3 tests (remove last owner fails, update last owner fails, remove 2nd-to-last succeeds)
- Fix 3: 3 tests (persists across requests, expiry after window, prevents bulk emails)
- 6 tests passing, 3 skipped (long-running/environment-specific tests)
**Files Changed:**
- 6 new files created
- 6 existing files modified
- 1 database migration added
- All existing tests still pass (no regressions)
**Verification:**
- Build succeeds with no errors
- All critical business logic tests pass
- Database migration generated successfully
- Security vulnerabilities addressed
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
168 lines
6.2 KiB
C#
168 lines
6.2 KiB
C#
using ColaFlow.Modules.Identity.Application.Services;
|
|
using ColaFlow.Modules.Identity.Domain.Entities;
|
|
using ColaFlow.Modules.Identity.Infrastructure.Persistence;
|
|
using Microsoft.EntityFrameworkCore;
|
|
using Microsoft.Extensions.Logging;
|
|
|
|
namespace ColaFlow.Modules.Identity.Infrastructure.Services;
|
|
|
|
/// <summary>
|
|
/// Database-backed rate limiting service implementation.
|
|
/// Persists rate limit state in PostgreSQL to survive server restarts.
|
|
/// Prevents email bombing attacks even after application restart.
|
|
/// </summary>
|
|
public class DatabaseEmailRateLimiter : IRateLimitService
|
|
{
|
|
private readonly IdentityDbContext _context;
|
|
private readonly ILogger<DatabaseEmailRateLimiter> _logger;
|
|
|
|
public DatabaseEmailRateLimiter(
|
|
IdentityDbContext context,
|
|
ILogger<DatabaseEmailRateLimiter> logger)
|
|
{
|
|
_context = context;
|
|
_logger = logger;
|
|
}
|
|
|
|
public async Task<bool> IsAllowedAsync(
|
|
string key,
|
|
int maxAttempts,
|
|
TimeSpan window,
|
|
CancellationToken cancellationToken = default)
|
|
{
|
|
// Parse key format: "operation:email:tenantId"
|
|
// Examples:
|
|
// - "forgot-password:user@example.com:tenant-guid"
|
|
// - "verification:user@example.com:tenant-guid"
|
|
// - "invitation:user@example.com:tenant-guid"
|
|
|
|
var parts = key.Split(':');
|
|
if (parts.Length != 3)
|
|
{
|
|
_logger.LogWarning("Invalid rate limit key format: {Key}. Expected format: 'operation:email:tenantId'", key);
|
|
return true; // Fail open (allow request) if key format is invalid
|
|
}
|
|
|
|
var operationType = parts[0];
|
|
var email = parts[1].ToLower();
|
|
var tenantIdStr = parts[2];
|
|
|
|
if (!Guid.TryParse(tenantIdStr, out var tenantId))
|
|
{
|
|
_logger.LogWarning("Invalid tenant ID in rate limit key: {Key}", key);
|
|
return true; // Fail open
|
|
}
|
|
|
|
// Find existing rate limit record
|
|
var rateLimit = await _context.EmailRateLimits
|
|
.FirstOrDefaultAsync(
|
|
r => r.Email == email &&
|
|
r.TenantId == tenantId &&
|
|
r.OperationType == operationType,
|
|
cancellationToken);
|
|
|
|
// No existing record - create new one and allow
|
|
if (rateLimit == null)
|
|
{
|
|
var newRateLimit = EmailRateLimit.Create(email, tenantId, operationType);
|
|
_context.EmailRateLimits.Add(newRateLimit);
|
|
|
|
try
|
|
{
|
|
await _context.SaveChangesAsync(cancellationToken);
|
|
_logger.LogInformation(
|
|
"Rate limit record created for {Email} - {Operation} (Attempt 1/{MaxAttempts})",
|
|
email, operationType, maxAttempts);
|
|
}
|
|
catch (DbUpdateException ex)
|
|
{
|
|
// Handle race condition: another request created the record simultaneously
|
|
_logger.LogWarning(ex,
|
|
"Race condition detected while creating rate limit record for {Key}. Retrying...", key);
|
|
|
|
// Re-fetch the record created by the concurrent request
|
|
rateLimit = await _context.EmailRateLimits
|
|
.FirstOrDefaultAsync(
|
|
r => r.Email == email &&
|
|
r.TenantId == tenantId &&
|
|
r.OperationType == operationType,
|
|
cancellationToken);
|
|
|
|
if (rateLimit == null)
|
|
{
|
|
_logger.LogError("Failed to fetch rate limit record after race condition for {Key}", key);
|
|
return true; // Fail open
|
|
}
|
|
|
|
// Fall through to existing record logic below
|
|
}
|
|
|
|
if (rateLimit == null)
|
|
return true; // Record was successfully created, allow the request
|
|
}
|
|
|
|
// Check if time window has expired
|
|
if (rateLimit.IsWindowExpired(window))
|
|
{
|
|
// Window expired - reset counter and allow
|
|
rateLimit.ResetAttempts();
|
|
_context.EmailRateLimits.Update(rateLimit);
|
|
await _context.SaveChangesAsync(cancellationToken);
|
|
|
|
_logger.LogInformation(
|
|
"Rate limit window expired for {Email} - {Operation}. Counter reset (Attempt 1/{MaxAttempts})",
|
|
email, operationType, maxAttempts);
|
|
|
|
return true;
|
|
}
|
|
|
|
// Window still active - check attempt count
|
|
if (rateLimit.AttemptsCount >= maxAttempts)
|
|
{
|
|
// Rate limit exceeded
|
|
var remainingTime = window - (DateTime.UtcNow - rateLimit.LastSentAt);
|
|
|
|
_logger.LogWarning(
|
|
"Rate limit EXCEEDED for {Email} - {Operation}: {Attempts}/{MaxAttempts} attempts. " +
|
|
"Retry after {RemainingSeconds} seconds",
|
|
email, operationType, rateLimit.AttemptsCount, maxAttempts,
|
|
(int)remainingTime.TotalSeconds);
|
|
|
|
return false;
|
|
}
|
|
|
|
// Still within limit - increment counter and allow
|
|
rateLimit.RecordAttempt();
|
|
_context.EmailRateLimits.Update(rateLimit);
|
|
await _context.SaveChangesAsync(cancellationToken);
|
|
|
|
_logger.LogInformation(
|
|
"Rate limit check passed for {Email} - {Operation} (Attempt {Attempts}/{MaxAttempts})",
|
|
email, operationType, rateLimit.AttemptsCount, maxAttempts);
|
|
|
|
return true;
|
|
}
|
|
|
|
/// <summary>
|
|
/// Cleanup expired rate limit records (call this from a background job)
|
|
/// </summary>
|
|
public async Task CleanupExpiredRecordsAsync(TimeSpan retentionPeriod, CancellationToken cancellationToken = default)
|
|
{
|
|
var cutoffDate = DateTime.UtcNow - retentionPeriod;
|
|
|
|
var expiredRecords = await _context.EmailRateLimits
|
|
.Where(r => r.LastSentAt < cutoffDate)
|
|
.ToListAsync(cancellationToken);
|
|
|
|
if (expiredRecords.Any())
|
|
{
|
|
_context.EmailRateLimits.RemoveRange(expiredRecords);
|
|
await _context.SaveChangesAsync(cancellationToken);
|
|
|
|
_logger.LogInformation(
|
|
"Cleaned up {Count} expired rate limit records older than {CutoffDate}",
|
|
expiredRecords.Count, cutoffDate);
|
|
}
|
|
}
|
|
}
|