WIP: Introduced handlers for backend architecture

[stasel]

No description provided.

[Copilot]

Pull request overview

This PR refactors the server-side HTTP architecture by introducing a dedicated handlers/ layer (Express request/response concerns) and converting controllers into “pure” services that return values and throw typed HttpErrors.

Changes:

• Added handlers/* classes and a shared sendError helper to translate controller errors into HTTP responses.
• Refactored routers to depend on handlers instead of controllers, and updated index.ts wiring accordingly.
• Refactored multiple controllers to remove Express dependencies and to throw HttpError subclasses for error cases.

Reviewed changes

Copilot reviewed 39 out of 39 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
server/src/routes/UserRouter.ts	Switches user routes to call UserHandler methods.
server/src/routes/TraineesRouter.ts	Switches trainee-related routes to handler instances across subdomains.
server/src/routes/SearchRouter.ts	Uses SearchHandler instead of controller.
server/src/routes/GeographyRouter.ts	Uses GeographyHandler instead of controller.
server/src/routes/DashboardRouter.ts	Uses DashboardHandler instead of controller.
server/src/routes/CohortsRouter.ts	Uses CohortsHandler instead of controller.
server/src/routes/AuthenticationRouter.ts	Routes login/logout/session via AuthenticationHandler.
server/src/middlewares/AuthMiddleware.ts	Updates token cookie constant import to handler layer.
server/src/index.ts	Rewires dependency graph: services → repositories → pure controllers → HTTP handlers → routers.
server/src/handlers/index.ts	Barrel exports for handler layer.
server/src/handlers/UserHandler.ts	New HTTP handler for users with zod body parsing + error translation.
server/src/handlers/TraineeHandler.ts	New HTTP handler for base trainee CRUD endpoints.
server/src/handlers/TestHandler.ts	New HTTP handler for trainee tests endpoints.
server/src/handlers/StrikeHandler.ts	New HTTP handler for trainee strikes endpoints.
server/src/handlers/SearchHandler.ts	New HTTP handler that wraps controller results in the existing search response shape.
server/src/handlers/ProfilePictureHandler.ts	New HTTP handler managing upload + calling controller for storage updates.
server/src/handlers/LetterHandler.ts	New HTTP handler streaming generated letters to the client.
server/src/handlers/InteractionHandler.ts	New HTTP handler for trainee interactions endpoints.
server/src/handlers/Handler.ts	Adds sendError helper to map HttpError → structured HTTP responses.
server/src/handlers/GeographyHandler.ts	New HTTP handler for countries/cities search.
server/src/handlers/EmploymentHistoryHandler.ts	New HTTP handler for employment-history endpoints.
server/src/handlers/DashboardHandler.ts	New HTTP handler for dashboard response.
server/src/handlers/CohortsHandler.ts	New HTTP handler for cohorts endpoint.
server/src/handlers/AuthenticationHandler.ts	New HTTP handler for login (sets cookie), logout, and session.
server/src/errors/index.ts	Adds barrel export for error types.
server/src/errors/HttpError.ts	Introduces HttpError + common HTTP subclasses.
server/src/controllers/UserController.ts	Converts to pure controller API returning values and throwing HttpError.
server/src/controllers/Trainee/TraineeController.ts	Converts to pure controller API (CRUD returning values / throwing HttpError).
server/src/controllers/Trainee/TestController.ts	Converts to pure controller API and centralizes validation + HttpError throwing.
server/src/controllers/Trainee/StrikeController.ts	Converts to pure controller API and centralizes validation + HttpError throwing.
server/src/controllers/Trainee/ProfilePictureController.ts	Converts to pure controller API; returns URLs and cleans up temp files in finally.
server/src/controllers/Trainee/LetterController.ts	Converts to pure controller API; returns stream + metadata.
server/src/controllers/Trainee/InteractionController.ts	Converts to pure controller API and centralizes validation + HttpError throwing.
server/src/controllers/Trainee/EmploymentHistoryController.ts	Converts to pure controller API and centralizes validation + HttpError throwing.
server/src/controllers/SearchController.ts	Converts to pure controller API returning results only (handler formats response).
server/src/controllers/GeographyController.ts	Converts to pure controller API returning arrays only.
server/src/controllers/DashboardController.ts	Converts to pure controller API returning typed dashboard payload.
server/src/controllers/CohortsController.ts	Converts to pure controller API returning cohorts data.
server/src/controllers/AuthenticationController.ts	Converts to pure controller API returning { user, token } and throwing HttpError.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[dojo-security-review]

🛑 Security Review — 3 issue(s) found

The PR is a clean architectural refactor (splitting Express handlers from business-logic controllers). Two cross-cutting patterns introduced security issues: reporter IDs accepted from the request body, and an unsanitized filename in a response header.

View all findings (3 issues)

File	Line	Severity	OWASP	Issue
server/src/handlers/InteractionHandler.ts	33, 52	🟠 High	A01	Reporter ID accepted from req.body — any authenticated user can falsely attribute interactions to another user
server/src/handlers/StrikeHandler.ts	32, 50	🟠 High	A01	Same reporter ID spoofing in strikes — authorship of strikes can be falsified
server/src/handlers/LetterHandler.ts	22	🟡 Medium	A03	Unsanitized fileName interpolated into Content-Disposition header — potential HTTP response splitting via CRLF in trainee name

Generated by PR Security Review for issue #333 · ● 541.7K · ◷

[dojo-security-review]

Generated by PR Security Review for issue #333 · ● 541.7K

[dojo-security-review]

🟠 Security Issue: Reporter ID Spoofing / Broken Access Control

OWASP: A01 – Broken Access Control
Severity: High

Issue: The reporterID is accepted directly from req.body, allowing any authenticated user to attribute an interaction to a different user. An attacker can submit { "reporterID": "<victim-user-id>", ... } to falsely record that another staff member created the interaction. The same pattern appears again in updateInteraction (line 52), meaning an attacker can also rewrite the reporter on existing records.

Recommendation: Ignore the client-supplied reporterID entirely and always derive it from the authenticated session:

// addInteraction
const interaction: InteractionWithReporterID = {
  date,
  type,
  title,
  details,
  reporterID: user.id,  // always use the authenticated user — never trust req.body
} as InteractionWithReporterID;

Apply the same fix to updateInteraction (line 52).

[dojo-security-review]

🟠 Security Issue: Reporter ID Spoofing / Broken Access Control

OWASP: A01 – Broken Access Control
Severity: High

Issue: Same pattern as InteractionHandler — reporterID is accepted from req.body, so any authenticated user can record a strike and falsely attribute it to another staff member. The same flaw exists in updateStrike (line 50), allowing retroactive reassignment of authorship on existing strikes.

Recommendation: Derive reporterID exclusively from the authenticated session; never accept it from the request body:

const strike: StrikeWithReporterID = {
  reason,
  date,
  comments,
  reporterID: user.id,  // ignore req.body?.reporterID
} as StrikeWithReporterID;

Apply the same fix to updateStrike (line 50).

[dojo-security-review]

🟡 Security Issue: HTTP Response Header Injection via Unsanitized Filename

OWASP: A03 – Injection
Severity: Medium

Issue: fileName is interpolated directly into the Content-Disposition header without sanitisation. fileName originates from trainee data (display name + letter type) which is ultimately user-controlled. If a trainee's name contains " characters, the filename parameter in the header is broken out of its quotes. More critically, if the name contains \r\n (CRLF), an attacker could inject arbitrary HTTP response headers (HTTP Response Splitting), enabling cache poisoning or header-based attacks.

Recommendation: Strip or encode unsafe characters from the filename before embedding it in the header. The simplest approach is to use encodeURIComponent and the filename* RFC 5987 format, or at minimum remove " and CRLF from the string:

const safeFileName = fileName.replace(/["\r\n]/g, '_');
res.status(200).header({
  'Content-Type': contentType,
  'Content-Disposition': `attachment; filename="\$\{safeFileName}"`,
});