diff --git a/WebApp/ChapterSettings.cs b/WebApp/ChapterSettings.cs index 99f20cc..6e9b497 100644 --- a/WebApp/ChapterSettings.cs +++ b/WebApp/ChapterSettings.cs @@ -6,7 +6,7 @@ namespace WebApp { private int[]? _scheduledTeams; - public int[] UserId + public int[] ScheduledTeams { get => _scheduledTeams ?? []; set diff --git a/WebApp/Components/App.razor b/WebApp/Components/App.razor index d03f3ab..5870269 100644 --- a/WebApp/Components/App.razor +++ b/WebApp/Components/App.razor @@ -13,7 +13,9 @@ - + + + diff --git a/WebApp/Components/Login.razor b/WebApp/Components/Login.razor index a6804e1..64b0a6a 100644 --- a/WebApp/Components/Login.razor +++ b/WebApp/Components/Login.razor @@ -98,8 +98,18 @@ private class LoginModel { + [Required(ErrorMessage = "Email is required")] + [EmailAddress(ErrorMessage = "Invalid email format")] + [MaxLength(100, ErrorMessage = "Email must be less than 100 characters")] + [RegularExpression(@"^[^@\s]+@[^@\s]+\.[^@\s]+$", ErrorMessage = "Please enter a valid email address")] public string Email { get; set; } = string.Empty; + + [Required(ErrorMessage = "Password is required")] + [MinLength(8, ErrorMessage = "Password must be at least 8 characters")] + [MaxLength(100, ErrorMessage = "Password must be less than 100 characters")] + [DataType(DataType.Password)] public string Password { get; set; } = string.Empty; + public bool RememberMe { get; set; } } diff --git a/WebApp/Components/Shared/AppErrorBoundary.razor b/WebApp/Components/Shared/AppErrorBoundary.razor new file mode 100644 index 0000000..ca6e3f4 --- /dev/null +++ b/WebApp/Components/Shared/AppErrorBoundary.razor @@ -0,0 +1,55 @@ +@using Microsoft.AspNetCore.Components.Web + + + + @ChildContent + + + + + + + + An Error Occurred + + @if (ShowDetails) + { + + Error: @ex.Message + + + Stack Trace: + 
@ex.StackTrace
+ + } + else + { + + Something went wrong. Please try refreshing the page or contact support if the problem persists. + + } + + + Return to Home + + + + + + +@code { + [Parameter] public RenderFragment? ChildContent { get; set; } + [Inject] private IWebHostEnvironment Environment { get; set; } = default!; + [Inject] private NavigationManager Navigation { get; set; } = default!; + [Inject] private ILogger Logger { get; set; } = default!; + + private bool ShowDetails => Environment.IsDevelopment(); + + protected override void OnParametersSet() + { + Logger.LogError("Error boundary triggered"); + } +} diff --git a/WebApp/IMPROVEMENTS.md b/WebApp/IMPROVEMENTS.md new file mode 100644 index 0000000..cb7712b --- /dev/null +++ b/WebApp/IMPROVEMENTS.md @@ -0,0 +1,1104 @@ +# WebApp Improvement Plan + +## 🔴 CRITICAL - Security Issues (Fix Immediately) + +### 1. Fix Login Credential Exposure ⚠️ URGENT +**Status**: Not Started +**Priority**: P0 +**Estimated Time**: 30 minutes + +**Problem**: Login.razor (lines 129-133) sends credentials via GET request in URL parameters. + +**Security Risks**: +- Credentials appear in server logs +- Credentials stored in browser history +- Credentials may leak via referrer headers +- Violates OWASP security guidelines + +**Current Code**: +```csharp +// Login.razor - INSECURE +var uri = $"/Auth/CookieLogin?email={Uri.EscapeDataString(_loginModel.Email)}" + + $"&password={Uri.EscapeDataString(_loginModel.Password)}" + + $"&rememberMe={_loginModel.RememberMe}"; +Navigation.NavigateTo(uri, forceLoad: true); +``` + +**Fix**: +```csharp +// Login.razor - Add HttpClient injection +[Inject] private HttpClient Http { get; set; } = default!; + +private async Task HandleLogin() +{ + _isSubmitting = true; + _errorMessage = null; + + try + { + // Get client IP address + var ipAddress = HttpContextAccessor.HttpContext?.Connection.RemoteIpAddress?.ToString() + ?? "unknown"; + + // Check rate limiting + if (RateLimitService.IsLockedOut(ipAddress)) + { + var remaining = RateLimitService.GetRemainingLockoutTime(ipAddress); + _errorMessage = $"Too many failed attempts. Please try again in {remaining?.Minutes ?? 15} minutes."; + return; + } + + // Create form data + var formData = new Dictionary + { + ["email"] = _loginModel.Email, + ["password"] = _loginModel.Password, + ["rememberMe"] = _loginModel.RememberMe.ToString() + }; + + // POST credentials securely + var response = await Http.PostAsync("/Auth/CookieLogin", + new FormUrlEncodedContent(formData)); + + if (response.IsSuccessStatusCode) + { + Navigation.NavigateTo("/", forceLoad: true); + } + else + { + _errorMessage = "Invalid email or password."; + } + } + catch (Exception ex) + { + _errorMessage = "An error occurred during login. Please try again."; + Console.WriteLine($"Login error: {ex}"); + } + finally + { + _isSubmitting = false; + } +} +``` + +```csharp +// AuthController.cs - Remove [HttpGet] attribute +[HttpPost] // Remove [HttpGet] support +[AllowAnonymous] +public async Task CookieLogin( + [FromForm] string email, + [FromForm] string password, + [FromForm] bool rememberMe = false) +{ + // ... existing implementation +} +``` + +**Files to Modify**: +- `WebApp/Components/Login.razor` +- `WebApp/Authentication/AuthController.cs` + +--- + +### 2. Fix StateContainer Duplicate Registration ⚠️ URGENT +**Status**: Not Started +**Priority**: P0 +**Estimated Time**: 5 minutes + +**Problem**: Program.cs registers StateContainer twice with different lifetimes (lines 41-42). + +**Current Code**: +```csharp +builder.Services.AddScoped(); // Server-side +builder.Services.AddSingleton();//Client-side +``` + +**Issue**: This creates two separate service registrations, leading to unpredictable behavior. + +**Fix**: +```csharp +// For Blazor Server (recommended) +builder.Services.AddScoped(); + +// OR if you need singleton (shared across all users - less common) +// builder.Services.AddSingleton(); +``` + +**Recommendation**: Use `Scoped` for Blazor Server to maintain state per user connection. + +**Files to Modify**: +- `WebApp/Program.cs` (line 41-42) + +--- + +### 3. Add CSRF Protection to Forms +**Status**: Not Started +**Priority**: P0 +**Estimated Time**: 15 minutes + +**Problem**: Forms don't explicitly include antiforgery tokens. + +**Fix**: +```razor + + + + + + + +``` + +**Apply to all forms in**: +- `WebApp/Components/Login.razor` +- `WebApp/Components/Pages/StudentPages/*.razor` +- `WebApp/Components/Pages/TeamPages/*.razor` +- `WebApp/Components/Pages/EventDefinitionPages/*.razor` + +--- + +## 🟠 HIGH Priority - Code Quality + +### 4. Fix Misleading Property Names +**Status**: Not Started +**Priority**: P1 +**Estimated Time**: 15 minutes + +**Problem**: StateContainer has property named `UserId` that actually stores `scheduledTeams`. + +**Current Code**: +```csharp +public class StateContainer +{ + private int[]? _scheduledTeams; + + public int[] UserId // Misleading name! + { + get => _scheduledTeams ?? []; + set { _scheduledTeams = value; NotifyStateChanged(); } + } +} +``` + +**Fix**: +```csharp +public class StateContainer +{ + private int[]? _scheduledTeams; + + public int[] ScheduledTeams // Correct, descriptive name + { + get => _scheduledTeams ?? []; + set + { + _scheduledTeams = value; + NotifyStateChanged(); + } + } + + public event Action? OnChange; + + private void NotifyStateChanged() => OnChange?.Invoke(); +} +``` + +**Files to Modify**: +- `WebApp/ChapterSettings.cs` +- All files that use `StateContainer.UserId` (search and replace) + +--- + +### 5. Add Structured Logging +**Status**: Not Started +**Priority**: P1 +**Estimated Time**: 1 hour + +**Add Serilog for better logging**: + +```bash +dotnet add package Serilog.AspNetCore +dotnet add package Serilog.Sinks.Console +dotnet add package Serilog.Sinks.File +``` + +```csharp +// Program.cs - Add at the top +using Serilog; + +// Replace builder creation +var builder = WebApplication.CreateBuilder(args); + +// Add Serilog +builder.Host.UseSerilog((context, configuration) => + configuration + .ReadFrom.Configuration(context.Configuration) + .Enrich.FromLogContext() + .Enrich.WithMachineName() + .Enrich.WithThreadId() + .WriteTo.Console( + outputTemplate: "[{Timestamp:HH:mm:ss} {Level:u3}] {Message:lj} {Properties:j}{NewLine}{Exception}") + .WriteTo.File( + path: "logs/webapp-.txt", + rollingInterval: RollingInterval.Day, + retainedFileCountLimit: 30, + outputTemplate: "{Timestamp:yyyy-MM-dd HH:mm:ss.fff zzz} [{Level:u3}] {Message:lj} {Properties:j}{NewLine}{Exception}")); +``` + +```json +// appsettings.json - Add Serilog configuration +{ + "Serilog": { + "MinimumLevel": { + "Default": "Information", + "Override": { + "Microsoft": "Warning", + "Microsoft.AspNetCore": "Warning", + "System": "Warning" + } + } + } +} +``` + +**Benefits**: +- Structured log output for parsing/analysis +- File rotation with retention policies +- Console and file output +- Performance optimizations + +**Files to Modify**: +- `WebApp/Program.cs` +- `WebApp/appsettings.json` +- `WebApp/appsettings.Development.json` + +--- + +### 6. Add Global Error Handling +**Status**: Not Started +**Priority**: P1 +**Estimated Time**: 30 minutes + +**Create ErrorBoundary Component**: + +```razor + +@using Microsoft.AspNetCore.Components.Web + + + + @ChildContent + + + + + + + + An Error Occurred + + @if (ShowDetails) + { + + Error: @ex.Message + + + Stack Trace: + 
@ex.StackTrace
+ + } + else + { + + Something went wrong. Please try refreshing the page or contact support if the problem persists. + + } + + + Return to Home + + + + + + +@code { + [Parameter] public RenderFragment? ChildContent { get; set; } + [Inject] private IWebHostEnvironment Environment { get; set; } = default!; + [Inject] private NavigationManager Navigation { get; set; } = default!; + [Inject] private ILogger Logger { get; set; } = default!; + + private bool ShowDetails => Environment.IsDevelopment(); + + protected override void OnParametersSet() + { + Logger.LogError("Error boundary triggered"); + } +} +``` + +**Wrap App.razor**: +```razor + + + + +``` + +**Files to Create**: +- `WebApp/Components/Shared/AppErrorBoundary.razor` + +**Files to Modify**: +- `WebApp/Components/App.razor` + +--- + +### 7. Enhance Input Validation +**Status**: Not Started +**Priority**: P1 +**Estimated Time**: 20 minutes + +**Strengthen validation attributes**: + +```csharp +// Login.razor +private class LoginModel +{ + [Required(ErrorMessage = "Email is required")] + [EmailAddress(ErrorMessage = "Invalid email format")] + [MaxLength(100, ErrorMessage = "Email must be less than 100 characters")] + [RegularExpression(@"^[^@\s]+@[^@\s]+\.[^@\s]+$", ErrorMessage = "Please enter a valid email address")] + public string Email { get; set; } = string.Empty; + + [Required(ErrorMessage = "Password is required")] + [MinLength(8, ErrorMessage = "Password must be at least 8 characters")] + [MaxLength(100, ErrorMessage = "Password must be less than 100 characters")] + [DataType(DataType.Password)] + public string Password { get; set; } = string.Empty; + + public bool RememberMe { get; set; } +} +``` + +**Apply similar validation to**: +- Student creation/edit forms +- Team creation/edit forms +- Event definition forms + +**Files to Modify**: +- `WebApp/Components/Login.razor` +- `WebApp/Components/Pages/StudentPages/Create.razor` +- `WebApp/Components/Pages/StudentPages/Edit.razor` +- `WebApp/Components/Pages/TeamPages/Create.razor` +- `WebApp/Components/Pages/TeamPages/Edit.razor` + +--- + +## 🟡 MEDIUM Priority - Architecture + +### 8. Add Health Checks +**Status**: Not Started +**Priority**: P2 +**Estimated Time**: 45 minutes + +**Add health check endpoints**: + +```bash +dotnet add package Microsoft.Extensions.Diagnostics.HealthChecks.EntityFrameworkCore +``` + +```csharp +// Program.cs +builder.Services.AddHealthChecks() + .AddDbContextCheck( + name: "Database", + failureStatus: HealthStatus.Unhealthy, + tags: new[] { "db", "sql", "sqlite" }) + .AddCheck("Authentication Service", () => + { + // Check if auth service is configured + var authSettings = builder.Configuration.GetSection("Authentication"); + return authSettings.Exists() + ? HealthCheckResult.Healthy("Authentication configured") + : HealthCheckResult.Degraded("Authentication may not be fully configured"); + }, tags: new[] { "auth" }) + .AddCheck("Rate Limiter", () => + { + // Check if rate limiter is working + return HealthCheckResult.Healthy("Rate limiter active"); + }, tags: new[] { "security" }); + +// After app.Build() +app.MapHealthChecks("/health", new HealthCheckOptions +{ + ResponseWriter = async (context, report) => + { + context.Response.ContentType = "application/json"; + var result = JsonSerializer.Serialize(new + { + status = report.Status.ToString(), + checks = report.Entries.Select(e => new + { + name = e.Key, + status = e.Value.Status.ToString(), + description = e.Value.Description, + duration = e.Value.Duration.ToString() + }), + totalDuration = report.TotalDuration.ToString() + }); + await context.Response.WriteAsync(result); + } +}); + +// Simpler endpoint for Kubernetes/Docker +app.MapHealthChecks("/health/ready"); +app.MapHealthChecks("/health/live"); +``` + +**Endpoints**: +- `/health` - Full health check with JSON response +- `/health/ready` - Readiness probe (200 if ready) +- `/health/live` - Liveness probe (200 if alive) + +**Files to Modify**: +- `WebApp/Program.cs` + +--- + +### 9. Add Response Caching +**Status**: Not Started +**Priority**: P2 +**Estimated Time**: 30 minutes + +```csharp +// Program.cs +builder.Services.AddResponseCaching(); +builder.Services.AddOutputCache(options => +{ + options.AddBasePolicy(builder => builder.Cache()); + + // Cache event list for 5 minutes + options.AddPolicy("EventList", builder => + builder + .Expire(TimeSpan.FromMinutes(5)) + .Tag("events")); + + // Cache student list for 2 minutes + options.AddPolicy("StudentList", builder => + builder + .Expire(TimeSpan.FromMinutes(2)) + .Tag("students")); +}); + +// After UseRouting() +app.UseResponseCaching(); +app.UseOutputCache(); +``` + +**Apply to pages**: +```razor +@* EventDefinitionPages/Index.razor *@ +@attribute [OutputCache(PolicyName = "EventList")] +``` + +**Files to Modify**: +- `WebApp/Program.cs` +- `WebApp/Components/Pages/EventDefinitionPages/Index.razor` +- `WebApp/Components/Pages/StudentPages/Index.razor` +- `WebApp/Components/Pages/TeamPages/Index.razor` + +--- + +### 10. Implement Repository Pattern +**Status**: Not Started +**Priority**: P2 +**Estimated Time**: 4 hours + +**Create repository interfaces and implementations**: + +```csharp +// Repositories/IRepository.cs +public interface IRepository where T : class +{ + Task> GetAllAsync(); + Task GetByIdAsync(int id); + Task CreateAsync(T entity); + Task UpdateAsync(T entity); + Task DeleteAsync(int id); + Task ExistsAsync(int id); +} + +// Repositories/IStudentRepository.cs +public interface IStudentRepository : IRepository +{ + Task> GetStudentsWithTeamsAsync(); + Task> GetStudentsWithRankingsAsync(); + Task GetStudentDetailsAsync(int id); + Task> SearchByNameAsync(string searchTerm); +} + +// Repositories/StudentRepository.cs +public class StudentRepository : IStudentRepository +{ + private readonly AppDbContext _context; + private readonly ILogger _logger; + + public StudentRepository(AppDbContext context, ILogger logger) + { + _context = context; + _logger = logger; + } + + public async Task> GetAllAsync() + { + return await _context.Students + .AsNoTracking() + .OrderBy(s => s.LastName) + .ThenBy(s => s.FirstName) + .ToListAsync(); + } + + public async Task GetByIdAsync(int id) + { + return await _context.Students.FindAsync(id); + } + + public async Task> GetStudentsWithTeamsAsync() + { + return await _context.Students + .Include(s => s.Teams) + .ThenInclude(t => t.Event) + .AsNoTracking() + .ToListAsync(); + } + + public async Task> GetStudentsWithRankingsAsync() + { + return await _context.Students + .Include(s => s.EventRankings) + .ThenInclude(er => er.EventDefinition) + .AsNoTracking() + .ToListAsync(); + } + + public async Task GetStudentDetailsAsync(int id) + { + return await _context.Students + .Include(s => s.Teams) + .ThenInclude(t => t.Event) + .Include(s => s.EventRankings) + .ThenInclude(er => er.EventDefinition) + .FirstOrDefaultAsync(s => s.Id == id); + } + + public async Task CreateAsync(Student student) + { + _context.Students.Add(student); + await _context.SaveChangesAsync(); + _logger.LogInformation("Created student {StudentId}: {Name}", + student.Id, student.Name); + return student; + } + + public async Task UpdateAsync(Student student) + { + _context.Entry(student).State = EntityState.Modified; + await _context.SaveChangesAsync(); + _logger.LogInformation("Updated student {StudentId}: {Name}", + student.Id, student.Name); + } + + public async Task DeleteAsync(int id) + { + var student = await _context.Students.FindAsync(id); + if (student != null) + { + _context.Students.Remove(student); + await _context.SaveChangesAsync(); + _logger.LogInformation("Deleted student {StudentId}", id); + } + } + + public async Task ExistsAsync(int id) + { + return await _context.Students.AnyAsync(s => s.Id == id); + } + + public async Task> SearchByNameAsync(string searchTerm) + { + return await _context.Students + .Where(s => s.FirstName.Contains(searchTerm) || + s.LastName.Contains(searchTerm)) + .AsNoTracking() + .ToListAsync(); + } +} +``` + +**Register in Program.cs**: +```csharp +builder.Services.AddScoped(); +builder.Services.AddScoped(); +builder.Services.AddScoped(); +``` + +**Files to Create**: +- `WebApp/Repositories/IRepository.cs` +- `WebApp/Repositories/IStudentRepository.cs` +- `WebApp/Repositories/StudentRepository.cs` +- `WebApp/Repositories/IEventDefinitionRepository.cs` +- `WebApp/Repositories/EventDefinitionRepository.cs` +- `WebApp/Repositories/ITeamRepository.cs` +- `WebApp/Repositories/TeamRepository.cs` + +**Files to Modify**: +- `WebApp/Program.cs` +- All page files that currently use `AppDbContext` directly + +--- + +### 11. Add API Versioning +**Status**: Not Started +**Priority**: P2 +**Estimated Time**: 1 hour + +```bash +dotnet add package Asp.Versioning.Mvc +``` + +```csharp +// Program.cs +builder.Services.AddApiVersioning(options => +{ + options.DefaultApiVersion = new ApiVersion(1, 0); + options.AssumeDefaultVersionWhenUnspecified = true; + options.ReportApiVersions = true; + options.ApiVersionReader = new UrlSegmentApiVersionReader(); +}).AddMvc(); +``` + +**Files to Modify**: +- `WebApp/Program.cs` +- Any API controllers you create + +--- + +## 🟢 LOW Priority - Nice to Have + +### 12. Add Swagger/OpenAPI Documentation +**Status**: Not Started +**Priority**: P3 +**Estimated Time**: 30 minutes + +```bash +dotnet add package Swashbuckle.AspNetCore +``` + +```csharp +// Program.cs +builder.Services.AddEndpointsApiExplorer(); +builder.Services.AddSwaggerGen(c => +{ + c.SwaggerDoc("v1", new OpenApiInfo + { + Title = "TSA Chapter Organizer API", + Version = "v1", + Description = "API for managing TSA chapter events, students, and teams", + Contact = new OpenApiContact + { + Name = "TSA Chapter Organizer", + Email = "support@example.com" + } + }); + + // Include XML comments if available + var xmlFile = $"{Assembly.GetExecutingAssembly().GetName().Name}.xml"; + var xmlPath = Path.Combine(AppContext.BaseDirectory, xmlFile); + if (File.Exists(xmlPath)) + { + c.IncludeXmlComments(xmlPath); + } + + // Add authentication + c.AddSecurityDefinition("Cookie", new OpenApiSecurityScheme + { + Type = SecuritySchemeType.ApiKey, + In = ParameterLocation.Cookie, + Name = "TSA.Auth" + }); +}); + +// Only in development +if (app.Environment.IsDevelopment()) +{ + app.UseSwagger(); + app.UseSwaggerUI(c => + { + c.SwaggerEndpoint("/swagger/v1/swagger.json", "TSA Chapter Organizer API v1"); + c.RoutePrefix = "api-docs"; + }); +} +``` + +**Access**: Navigate to `/api-docs` in development + +**Files to Modify**: +- `WebApp/Program.cs` +- `WebApp/WebApp.csproj` (enable XML documentation) + +--- + +### 13. Add Request/Response Compression +**Status**: Not Started +**Priority**: P3 +**Estimated Time**: 15 minutes + +```csharp +// Program.cs +builder.Services.AddResponseCompression(options => +{ + options.EnableForHttps = true; + options.Providers.Add(); + options.Providers.Add(); + options.MimeTypes = ResponseCompressionDefaults.MimeTypes.Concat( + new[] { "application/json", "text/html", "text/css", "application/javascript" }); +}); + +builder.Services.Configure(options => +{ + options.Level = CompressionLevel.Fastest; +}); + +builder.Services.Configure(options => +{ + options.Level = CompressionLevel.Optimal; +}); + +// Add early in middleware pipeline +app.UseResponseCompression(); +``` + +**Files to Modify**: +- `WebApp/Program.cs` + +--- + +### 14. Add Database Migration Check on Startup +**Status**: Not Started +**Priority**: P3 +**Estimated Time**: 20 minutes + +```csharp +// Program.cs - After app.Build(), before app.Run() +using (var scope = app.Services.CreateScope()) +{ + var services = scope.ServiceProvider; + var logger = services.GetRequiredService>(); + + try + { + var context = services.GetRequiredService(); + + if (app.Environment.IsDevelopment()) + { + // Auto-apply migrations in development + logger.LogInformation("Applying database migrations (Development)..."); + await context.Database.MigrateAsync(); + logger.LogInformation("Database migrations applied successfully"); + } + else + { + // Check if migrations are needed in production + var pendingMigrations = await context.Database.GetPendingMigrationsAsync(); + if (pendingMigrations.Any()) + { + logger.LogError( + "Database has pending migrations: {Migrations}", + string.Join(", ", pendingMigrations)); + throw new InvalidOperationException( + "Database migrations are pending. Run migrations before starting the application."); + } + logger.LogInformation("Database is up to date"); + } + } + catch (Exception ex) + { + logger.LogError(ex, "An error occurred while checking database migrations"); + if (!app.Environment.IsDevelopment()) + { + throw; // Fail fast in production + } + } +} +``` + +**Files to Modify**: +- `WebApp/Program.cs` + +--- + +### 15. Add Rate Limiting Middleware +**Status**: Not Started +**Priority**: P3 +**Estimated Time**: 30 minutes + +**.NET 7+ has built-in rate limiting**: + +```csharp +// Program.cs +builder.Services.AddRateLimiter(options => +{ + // General API rate limit + options.AddFixedWindowLimiter("api", limiterOptions => + { + limiterOptions.PermitLimit = 100; + limiterOptions.Window = TimeSpan.FromMinutes(1); + limiterOptions.QueueProcessingOrder = QueueProcessingOrder.OldestFirst; + limiterOptions.QueueLimit = 10; + }); + + // Stricter limit for authentication endpoints + options.AddSlidingWindowLimiter("auth", limiterOptions => + { + limiterOptions.PermitLimit = 10; + limiterOptions.Window = TimeSpan.FromMinutes(1); + limiterOptions.SegmentsPerWindow = 2; + limiterOptions.QueueProcessingOrder = QueueProcessingOrder.OldestFirst; + limiterOptions.QueueLimit = 2; + }); + + options.OnRejected = async (context, token) => + { + context.HttpContext.Response.StatusCode = StatusCodes.Status429TooManyRequests; + + if (context.Lease.TryGetMetadata(MetadataName.RetryAfter, out var retryAfter)) + { + await context.HttpContext.Response.WriteAsync( + $"Too many requests. Please try again after {retryAfter.TotalSeconds} seconds.", + token); + } + else + { + await context.HttpContext.Response.WriteAsync( + "Too many requests. Please try again later.", + token); + } + }; +}); + +// After UseRouting() +app.UseRateLimiter(); +``` + +**Apply to controllers**: +```csharp +[EnableRateLimiting("auth")] +public class AuthController : Controller +{ + // ... +} +``` + +**Files to Modify**: +- `WebApp/Program.cs` +- `WebApp/Authentication/AuthController.cs` + +--- + +### 16. Improve Component Organization +**Status**: Not Started +**Priority**: P3 +**Estimated Time**: 2 hours + +**Reorganize into feature folders**: + +``` +WebApp/Components/ +├── Features/ +│ ├── Students/ +│ │ ├── StudentList.razor +│ │ ├── StudentForm.razor +│ │ ├── StudentDetails.razor +│ │ ├── StudentEventRanking.razor +│ │ └── StudentSelector.razor +│ ├── Teams/ +│ │ ├── TeamList.razor +│ │ ├── TeamForm.razor +│ │ ├── TeamDetails.razor +│ │ ├── TeamScheduler.razor +│ │ ├── TeamSelector.razor +│ │ └── TeamPrintout.razor +│ ├── Events/ +│ │ ├── EventList.razor +│ │ ├── EventForm.razor +│ │ ├── EventDetails.razor +│ │ ├── EventAttributes.razor +│ │ └── EventPrintout.razor +│ ├── MeetingSchedule/ +│ │ ├── MeetingScheduleIndex.razor +│ │ ├── ScheduledTeamsList.razor +│ │ └── UnscheduledStudentsList.razor +│ └── Authentication/ +│ └── Login.razor +├── Shared/ +│ ├── Components/ +│ │ ├── CrudActions.razor +│ │ └── ErrorBoundary.razor +│ └── Layout/ +│ ├── MainLayout.razor +│ ├── EmptyLayout.razor +│ └── NavMenu.razor +└── Pages/ + ├── Home.razor + ├── Import.razor + ├── Legend.razor + └── Error.razor +``` + +**Benefits**: +- Better organization by feature +- Easier to find related components +- Clearer responsibility boundaries +- Easier onboarding for new developers + +**This is a large refactoring - consider doing incrementally** + +--- + +### 17. Add Client-Side Validation Feedback +**Status**: Not Started +**Priority**: P3 +**Estimated Time**: 30 minutes + +**Improve MudBlazor validation UX**: + +```razor + + + +``` + +**Add custom validators**: +```csharp +public class StrongPasswordAttribute : ValidationAttribute +{ + protected override ValidationResult? IsValid(object? value, ValidationContext validationContext) + { + var password = value as string; + if (string.IsNullOrEmpty(password)) + return ValidationResult.Success; + + var hasUpperCase = password.Any(char.IsUpper); + var hasLowerCase = password.Any(char.IsLower); + var hasDigit = password.Any(char.IsDigit); + + if (!hasUpperCase || !hasLowerCase || !hasDigit) + { + return new ValidationResult( + "Password must contain at least one uppercase letter, one lowercase letter, and one digit"); + } + + return ValidationResult.Success; + } +} +``` + +**Files to Modify**: +- All form components +- Add `Validators/StrongPasswordAttribute.cs` + +--- + +## 📊 Implementation Roadmap + +### Week 1: Critical Security (P0) +- [ ] Day 1: Fix login credential exposure (#1) +- [ ] Day 1: Fix StateContainer duplicate registration (#2) +- [ ] Day 2: Add CSRF protection (#3) +- [ ] Day 2-3: Fix misleading property names (#4) +- [ ] Day 4-5: Add structured logging (#5) + +### Week 2: High Priority Quality (P1) +- [ ] Day 1: Add global error handling (#6) +- [ ] Day 2: Enhance input validation (#7) +- [ ] Day 3-4: Add health checks (#8) +- [ ] Day 5: Add response caching (#9) + +### Week 3-4: Architecture Improvements (P2) +- [ ] Days 1-3: Implement repository pattern (#10) +- [ ] Day 4: Add API versioning (#11) + +### Future Iterations: Nice to Have (P3) +- [ ] Add Swagger/OpenAPI (#12) +- [ ] Add compression (#13) +- [ ] Database migration checks (#14) +- [ ] Rate limiting middleware (#15) +- [ ] Component reorganization (#16) +- [ ] Client-side validation (#17) + +--- + +## 🎯 Success Metrics + +### Security +- ✅ No credentials in logs or browser history +- ✅ All forms protected with CSRF tokens +- ✅ Rate limiting prevents brute force attacks + +### Performance +- ⏱️ Page load times reduced by 30% (caching) +- ⏱️ Reduced database queries (repository pattern) +- ⏱️ Smaller payload sizes (compression) + +### Code Quality +- 📊 No misleading variable names +- 📊 Structured logging for debugging +- 📊 Clear error messages for users +- 📊 Consistent validation across forms + +### Maintainability +- 🔧 Repository pattern isolates data access +- 🔧 Error boundaries prevent cascading failures +- 🔧 Health checks enable monitoring +- 🔧 Organized component structure + +--- + +## 📝 Notes + +- All changes should be tested in development before deploying to production +- Consider creating feature branches for larger changes (repository pattern, reorganization) +- Update documentation as features are implemented +- Add unit tests for new repository classes +- Monitor logs after implementing structured logging +- Review health check endpoints with DevOps team + +--- + +**Document Version**: 1.0 +**Last Updated**: 2025-12-02 +**Next Review**: After P0 items are completed diff --git a/WebApp/Program.cs b/WebApp/Program.cs index 8635a45..976ae7c 100644 --- a/WebApp/Program.cs +++ b/WebApp/Program.cs @@ -1,12 +1,26 @@ using Data; using Microsoft.EntityFrameworkCore; using MudBlazor.Services; +using Serilog; using WebApp; using WebApp.Authentication; using WebApp.Components; var builder = WebApplication.CreateBuilder(args); +// Configure Serilog +builder.Host.UseSerilog((context, configuration) => + configuration + .ReadFrom.Configuration(context.Configuration) + .Enrich.FromLogContext() + .WriteTo.Console( + outputTemplate: "[{Timestamp:HH:mm:ss} {Level:u3}] {Message:lj} {Properties:j}{NewLine}{Exception}") + .WriteTo.File( + path: "logs/webapp-.txt", + rollingInterval: RollingInterval.Day, + retainedFileCountLimit: 30, + outputTemplate: "{Timestamp:yyyy-MM-dd HH:mm:ss.fff zzz} [{Level:u3}] {Message:lj} {Properties:j}{NewLine}{Exception}")); + // Configure authentication secrets for production (Docker, etc.) if (builder.Environment.IsProduction()) { diff --git a/WebApp/WebApp.csproj b/WebApp/WebApp.csproj index b0a87eb..08638d1 100644 --- a/WebApp/WebApp.csproj +++ b/WebApp/WebApp.csproj @@ -28,6 +28,9 @@ + + + diff --git a/WebApp/appsettings.Development.json b/WebApp/appsettings.Development.json index 0c208ae..921e0ef 100644 --- a/WebApp/appsettings.Development.json +++ b/WebApp/appsettings.Development.json @@ -4,5 +4,15 @@ "Default": "Information", "Microsoft.AspNetCore": "Warning" } + }, + "Serilog": { + "MinimumLevel": { + "Default": "Debug", + "Override": { + "Microsoft": "Warning", + "Microsoft.AspNetCore": "Warning", + "System": "Warning" + } + } } } diff --git a/WebApp/appsettings.json b/WebApp/appsettings.json index dfe8d50..96730d9 100644 --- a/WebApp/appsettings.json +++ b/WebApp/appsettings.json @@ -5,6 +5,16 @@ "Microsoft.AspNetCore": "Warning" } }, + "Serilog": { + "MinimumLevel": { + "Default": "Information", + "Override": { + "Microsoft": "Warning", + "Microsoft.AspNetCore": "Warning", + "System": "Warning" + } + } + }, "AllowedHosts": "*", "ConnectionStrings": { "SQLiteDefault": "Data Source=data.db" diff --git a/WebApp/logs/webapp-20251203.txt b/WebApp/logs/webapp-20251203.txt new file mode 100644 index 0000000..172a101 --- /dev/null +++ b/WebApp/logs/webapp-20251203.txt @@ -0,0 +1,7 @@ +2025-12-03 14:07:50.779 -05:00 [INF] Login rate limiting service started {"SourceContext":"WebApp.Authentication.LoginRateLimitService"} +2025-12-03 14:07:53.133 -05:00 [ERR] Error boundary triggered {"SourceContext":"WebApp.Components.Shared.AppErrorBoundary","RequestId":"0HNHIJT0PE9UJ:00000001","RequestPath":"/login","ConnectionId":"0HNHIJT0PE9UJ"} +2025-12-03 14:08:08.603 -05:00 [DBG] Successful credential validation for admin@test.com {"SourceContext":"WebApp.Authentication.AuthenticationService","ActionId":"fab60e78-16b9-40cd-897c-fed8413ce202","ActionName":"WebApp.Authentication.AuthController.CookieLogin (tsa-chapter-organizer)","RequestId":"0HNHIJT0PE9UK:00000001","RequestPath":"/Auth/CookieLogin","ConnectionId":"0HNHIJT0PE9UK"} +2025-12-03 14:08:08.626 -05:00 [INF] Successful login for admin@test.com (Administrator) from ::1 {"SourceContext":"WebApp.Authentication.AuthController","ActionId":"fab60e78-16b9-40cd-897c-fed8413ce202","ActionName":"WebApp.Authentication.AuthController.CookieLogin (tsa-chapter-organizer)","RequestId":"0HNHIJT0PE9UK:00000001","RequestPath":"/Auth/CookieLogin","ConnectionId":"0HNHIJT0PE9UK"} +2025-12-03 14:08:08.666 -05:00 [ERR] Error boundary triggered {"SourceContext":"WebApp.Components.Shared.AppErrorBoundary","RequestId":"0HNHIJT0PE9UK:00000003","RequestPath":"/","ConnectionId":"0HNHIJT0PE9UK"} +2025-12-03 14:08:13.026 -05:00 [WRN] Compiling a query which loads related collections for more than one collection navigation, either via 'Include' or through projection, but no 'QuerySplittingBehavior' has been configured. By default, Entity Framework will use 'QuerySplittingBehavior.SingleQuery', which can potentially result in slow query performance. See https://go.microsoft.com/fwlink/?linkid=2134277 for more information. To identify the query that's triggering this warning call 'ConfigureWarnings(w => w.Throw(RelationalEventId.MultipleCollectionIncludeWarning))'. {"EventId":{"Id":20504,"Name":"Microsoft.EntityFrameworkCore.Query.MultipleCollectionIncludeWarning"},"SourceContext":"Microsoft.EntityFrameworkCore.Query","TransportConnectionId":"u30JsHVho23GyKv2giOOPQ","RequestId":"0HNHIJT0PE9UK:00000011","RequestPath":"/_blazor","ConnectionId":"0HNHIJT0PE9UK"} +2025-12-03 14:08:14.140 -05:00 [WRN] Compiling a query which loads related collections for more than one collection navigation, either via 'Include' or through projection, but no 'QuerySplittingBehavior' has been configured. By default, Entity Framework will use 'QuerySplittingBehavior.SingleQuery', which can potentially result in slow query performance. See https://go.microsoft.com/fwlink/?linkid=2134277 for more information. To identify the query that's triggering this warning call 'ConfigureWarnings(w => w.Throw(RelationalEventId.MultipleCollectionIncludeWarning))'. {"EventId":{"Id":20504,"Name":"Microsoft.EntityFrameworkCore.Query.MultipleCollectionIncludeWarning"},"SourceContext":"Microsoft.EntityFrameworkCore.Query","TransportConnectionId":"u30JsHVho23GyKv2giOOPQ","RequestId":"0HNHIJT0PE9UK:00000011","RequestPath":"/_blazor","ConnectionId":"0HNHIJT0PE9UK"}