Add instructor rating feature with enrollment-gated reviews

[Copilot]

Implements a rating system allowing enrolled students to review instructors on a 1-5 scale with optional comments.

Core Components

• Entity: InstructorRating with unique constraint on (StudentId, CourseId) to prevent duplicate ratings
• Repository: IInstructorRatingRepository with queries by course, student, and instructor name
• Service: Business logic enforces enrollment requirement before rating; aggregates statistics across instructor's courses
• API: 8 RESTful endpoints for CRUD operations, statistics, and filtered queries

Business Rules

// Students must be enrolled to rate
var isEnrolled = await _unitOfWork.Registrations
    .IsStudentRegisteredForCourseAsync(studentId, courseId);
if (!isEnrolled)
    throw new InvalidOperationException("Student must be enrolled in the course to submit a rating.");

// One rating per student per course (DB constraint)
entity.HasIndex(ir => new { ir.StudentId, ir.CourseId }).IsUnique();

API Surface

• POST /api/instructorratings - Create rating (validates enrollment)
• GET /api/instructorratings/instructor/{name}/stats - Aggregate statistics with distribution breakdown
• GET /api/instructorratings/course/{courseId} - All ratings for a course
• Standard CRUD endpoints for ratings by ID, student, instructor

DTOs

• CreateInstructorRatingDto - Rating (1-5), CourseId, StudentId, optional Comment
• InstructorRatingStatsDto - AverageRating, TotalRatings, star distribution (1-5), CourseIds

Statistics are calculated in-memory from repository queries; no precomputed aggregates.

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

In general, to fix log-forging issues when logging user input, normalize or encode the input before logging. For plain-text logs, remove newline and carriage return characters (and optionally other control characters). For HTML logs, HTML-encode the value. The aim is to ensure a malicious user cannot inject extra log lines or formatting.

In this case, the best fix is to sanitize instructorName just before it is passed to the logger, by removing newline (\n) and carriage return (\r) characters. This keeps all existing functionality intact (the service call still receives the original, unsanitized instructorName for correct lookups) while ensuring that the logged version cannot create forged entries. We only need to change the body of GetRatingsByInstructor in api/CourseRegistration.API/Controllers/InstructorRatingsController.cs: introduce a local variable that holds a sanitized copy for logging and use that in _logger.LogInformation(...). No new imports are required; we can rely on string.Replace which is in System and already available.

To fix this, we should sanitize the user-provided instructorName before it is logged, ensuring that any newline or carriage-return characters (and optionally other control characters) are removed. This prevents a malicious user from injecting extra log lines or confusing content into plain text logs while preserving the semantic value of the log entry.

The most direct, low-impact fix is to introduce a local, sanitized variable inside GetInstructorStats (and, for consistency and to eliminate a similar pattern, in GetRatingsByInstructor as well) and use that variable in the _logger.LogInformation calls. For example, we can call Replace("\r", "").Replace("\n", "") on instructorName. This keeps the method signature and its behavior unchanged for the rest of the system while ensuring that what is written to the logs cannot contain line breaks. No new imports are required; we use string’s built-in methods.

Concretely:

• In GetRatingsByInstructor, before logging, define var sanitizedInstructorName = instructorName.Replace("\r", string.Empty).Replace("\n", string.Empty); and use that in the _logger.LogInformation call.
• In GetInstructorStats (line 130+), likewise define a sanitizedInstructorName variable and use it in the log call on line 132, leaving the rest of the logic intact.

In general, sensitive controller actions should be protected by explicit authorization checks. For ASP.NET Core Web API, this is most commonly done using the [Authorize] attribute (optionally with roles/policies) so that only authenticated and authorized users can call these endpoints. If finer-grained control is required, additional checks can be done in the action method body (e.g., verifying that the current user is the owner of the resource or has a specific role).

The single best fix here, without changing existing functionality of the service layer, is to decorate the DeleteRating action (and, for consistency, the similarly sensitive UpdateRating action) with an [Authorize] attribute so that only authenticated users can update or delete ratings. This introduces a clear, declarative access control requirement on these methods while preserving their existing logic and return types. To implement this, we need to import Microsoft.AspNetCore.Authorization at the top of the file and add [Authorize] above the DeleteRating method. Given the similar risk profile, adding [Authorize] above UpdateRating in the same file is also recommended and consistent with the reported issue type.

Concretely in api/CourseRegistration.API/Controllers/InstructorRatingsController.cs:

• Add using Microsoft.AspNetCore.Authorization; alongside the existing using statements.
• Add [Authorize] above the UpdateRating method.
• Add [Authorize] above the DeleteRating method.

No other code changes or new methods are required.

In general, to fix IDOR issues you must ensure that operations on an object identified by a client‑supplied ID are only allowed if the current user is authorized to access that specific object. In ASP.NET Core this is typically done by (a) retrieving the resource, (b) running an authorization check against the current User (e.g., via IAuthorizationService or a policy), and only then (c) performing the update/delete if authorized, otherwise returning Forbid() or Unauthorized().

For this controller, the least disruptive fix that keeps existing functionality is:

• Inject IAuthorizationService into InstructorRatingsController.
• Before calling the service methods that update or delete a rating (UpdateRating and DeleteRating), fetch the rating by ID from the service.
• If the rating does not exist, keep returning 404 as today.
• If it exists, call _authorizationService.AuthorizeAsync(User, rating, "InstructorRatingOwnerPolicy") (or similar) to ensure the current user is allowed to act on that rating.
  • On success, proceed with update/delete as currently.
  • On failure, return Forbid() wrapped in the same ApiResponseDto<object>/ApiResponseDto<InstructorRatingDto> style used elsewhere, to avoid changing the API contract.

Because we only see part of the file, the code changes must stay inside the shown controller. Concretely:

• Add a using Microsoft.AspNetCore.Authorization; import at the top so we can use IAuthorizationService.
• Add a private readonly field _authorizationService and update the controller constructor (in the shown region) to accept and assign it.
• In UpdateRating, first call something like var existingRating = await _ratingService.GetRatingByIdAsync(id); (assuming such a method exists; if not, use whatever retrieval method is available in the shown snippet) and:
  • If existingRating == null, return 404 as today.
  • Otherwise, call _authorizationService.AuthorizeAsync(User, existingRating, "InstructorRatingOwnerPolicy").
  • If not authorized, return Forbid(ApiResponseDto<InstructorRatingDto>.ErrorResponse("Not authorized to update this rating")).
  • Only then call _ratingService.UpdateRatingAsync(id, updateRatingDto) and return success.
• In DeleteRating, do the same pattern: retrieve rating, check authorization, then call _ratingService.DeleteRatingAsync(id).

Since we must not assume new service methods that aren’t shown, you should adapt the retrieval to whatever method exists in the real file (e.g., GetRatingByIdAsync). In the replacement blocks below I’ll illustrate using a plausible GetRatingByIdAsync that would be present in the same service, but you may need to align the exact method name/signature with the rest of your code.