Skip to content

fix: secure training page mutations - #145

Open
smithynb wants to merge 1 commit into
devfrom
fix/training-auth
Open

fix: secure training page mutations#145
smithynb wants to merge 1 commit into
devfrom
fix/training-auth

Conversation

@smithynb

Copy link
Copy Markdown
Contributor

Summary

  • require role 3 or higher to create training pages
  • require role 5 to update or delete training pages
  • split creation from updates so edits use PATCH /api/training/:pageId
  • validate requests before persistence and perform multi-step writes in transactions
  • keep intended public reads available while enforcing visibility for internal training pages
  • surface rejected saves and deletes in the client without reporting success prematurely

Root cause

The training router was mounted without authentication or authorization on its write endpoints. In addition, POST /api/training doubled as an overwrite operation: it deleted an existing page by name before validating and inserting the replacement. An anonymous malformed request could therefore delete a training page even when the request ultimately returned an error.

This change makes POST create-only, moves updates to an authorized PATCH route, and wraps page/item persistence in transactions so failed writes do not leave destructive partial state.

Validation

  • focused Playwright regression spec: 1 passed
  • targeted backend and client ESLint
  • containerized client production build
  • git diff --check dev...HEAD
  • manual contract check: role-3 PATCH is rejected, DELETE still returns 201, and anonymous GET remains intentionally public

Resolves #144

Require authoritative roles for training-page creation, updates, and deletion while keeping intended public reads available. Split create and update into validated transactional flows, enforce internal-page visibility, and make the client handle rejected writes safely.
Comment thread routes/trainingPages.js Dismissed
Comment thread routes/trainingPages.js Dismissed
Comment thread routes/trainingPages.js Dismissed
Comment thread routes/trainingPages.js Dismissed
Comment thread routes/trainingPages.js Dismissed
@smithynb

Copy link
Copy Markdown
Contributor Author

Closed CodeQL warnings about rate limiting since we are not to handle it at the app level.

@smithynb
smithynb marked this pull request as ready for review July 30, 2026 18:53
Copilot AI review requested due to automatic review settings July 30, 2026 18:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unauthenticated destructive writes on /api/training/*

3 participants