Skip to content

fix: restrict teacher course deletion to owned courses - #42

Merged
udaycodespace merged 1 commit into
udaycodespace:mainfrom
Jidnyasa-P:fix/40-course-deletion-ownership
Aug 5, 2026
Merged

fix: restrict teacher course deletion to owned courses#42
udaycodespace merged 1 commit into
udaycodespace:mainfrom
Jidnyasa-P:fix/40-course-deletion-ownership

Conversation

@Jidnyasa-P

Copy link
Copy Markdown
Contributor

Description

Restricts teacher course deletion to courses owned by the authenticated teacher while preserving admin access to delete any course.

Fixes #40

ECSoC26

This contribution is submitted under ECSoC26.

Problem

The course deletion route was protected by authentication and role middleware, but the controller deleted courses using only the supplied course ID.

A teacher who knew another course ID could therefore attempt to delete a course owned by a different teacher.

Implementation

Authenticated ownership

Ownership is derived only from the verified authentication context:

req.user._id

or:

req.user.id

The controller does not trust req.body.userId.

Teacher access

Teachers may delete only courses whose stored userId matches their authenticated user ID.

The actual deletion query remains ownership-scoped:

{
  _id: courseid,
  userId: authenticatedUserId,
}

Admin access

Admins may delete any course using:

{
  _id: courseid,
}

Controlled errors

The controller now returns:

  • 400 for malformed MongoDB course IDs
  • 403 when a teacher does not own the course
  • 404 when the course does not exist
  • 500 for unexpected database or server failures

Local video cleanup

After successful database deletion, referenced local videos are removed safely.

The cleanup utility:

  • Restricts deletion to the backend uploads directory
  • Uses basename-only filenames
  • Prevents path traversal
  • Removes duplicate filenames only once
  • Ignores already-missing files
  • Logs cleanup failures without exposing filesystem paths
  • Reports only deleted and failed file counts

Tests

Added tests for:

  • Teacher deleting their own course
  • Teacher blocked from deleting another teacher’s course
  • Admin deleting any course
  • Ownership derived from authenticated identity
  • Request-body user ID spoofing prevention
  • Invalid course IDs
  • Missing courses
  • Cleanup failures
  • Duplicate video filenames
  • Missing local video files
  • Safe upload-path resolution

Acceptance criteria

  • A teacher can delete their own course
  • A teacher cannot delete another teacher’s course
  • An admin can delete any course
  • Ownership comes from authenticated identity
  • Request-body ownership spoofing is ignored
  • Invalid IDs return 400
  • Missing courses return 404
  • Local video files are handled safely
  • Backend tests cover ownership and admin access
  • No frontend changes are included

Verification

Tests

cd backend
npm install
npm test

Result:

tests 10
pass 10
fail 0

Syntax checks

node --check controllers/courseDeletionController.js
node --check utils/courseFileCleanup.js
node --check routers/userRoutes.js

Relevant code checks

grep -nE "req\\.user|ownerId|deleteFilter|findOneAndDelete" controllers/courseDeletionController.js
grep -nE "Invalid course ID|Course not found|only delete courses you own" controllers/courseDeletionController.js
grep -nE "path\\.basename|resolveSafeUploadPath|ENOENT|unlink" utils/courseFileCleanup.js
grep -nE "courseDeletionController|deletecourse" routers/userRoutes.js
grep -nE "own course|another teacher|admin|invalid course ID|missing course" tests/course-ownership.test.js

Test evidence

image image

@udaycodespace
udaycodespace self-requested a review August 5, 2026 17:13
@udaycodespace udaycodespace added ECSoC26 Required label for a PR to be eligible for Sentinel scoring good-pr PA-awarded bonus for an exceptionally executed PR — +15 XP good-backend PA-awarded bonus for outstanding backend work — +50 XP and removed documentation backend configuration fullstack tests labels Aug 5, 2026
@udaycodespace

Copy link
Copy Markdown
Owner

Description

Restricts teacher course deletion to courses owned by the authenticated teacher while preserving admin access to delete any course.

Fixes #40

ECSoC26

This contribution is submitted under ECSoC26.

Problem

The course deletion route was protected by authentication and role middleware, but the controller deleted courses using only the supplied course ID.

A teacher who knew another course ID could therefore attempt to delete a course owned by a different teacher.

Implementation

Authenticated ownership

Ownership is derived only from the verified authentication context:

req.user._id

or:

req.user.id

The controller does not trust req.body.userId.

Teacher access

Teachers may delete only courses whose stored userId matches their authenticated user ID.

The actual deletion query remains ownership-scoped:

{
  _id: courseid,
  userId: authenticatedUserId,
}

Admin access

Admins may delete any course using:

{
  _id: courseid,
}

Controlled errors

The controller now returns:

  • 400 for malformed MongoDB course IDs
  • 403 when a teacher does not own the course
  • 404 when the course does not exist
  • 500 for unexpected database or server failures

Local video cleanup

After successful database deletion, referenced local videos are removed safely.

The cleanup utility:

  • Restricts deletion to the backend uploads directory
  • Uses basename-only filenames
  • Prevents path traversal
  • Removes duplicate filenames only once
  • Ignores already-missing files
  • Logs cleanup failures without exposing filesystem paths
  • Reports only deleted and failed file counts

Tests

Added tests for:

  • Teacher deleting their own course
  • Teacher blocked from deleting another teacher’s course
  • Admin deleting any course
  • Ownership derived from authenticated identity
  • Request-body user ID spoofing prevention
  • Invalid course IDs
  • Missing courses
  • Cleanup failures
  • Duplicate video filenames
  • Missing local video files
  • Safe upload-path resolution

Acceptance criteria

  • A teacher can delete their own course
  • A teacher cannot delete another teacher’s course
  • An admin can delete any course
  • Ownership comes from authenticated identity
  • Request-body ownership spoofing is ignored
  • Invalid IDs return 400
  • Missing courses return 404
  • Local video files are handled safely
  • Backend tests cover ownership and admin access
  • No frontend changes are included

Verification

Tests

cd backend
npm install
npm test

Result:

tests 10
pass 10
fail 0

Syntax checks

node --check controllers/courseDeletionController.js
node --check utils/courseFileCleanup.js
node --check routers/userRoutes.js

Relevant code checks

grep -nE "req\\.user|ownerId|deleteFilter|findOneAndDelete" controllers/courseDeletionController.js
grep -nE "Invalid course ID|Course not found|only delete courses you own" controllers/courseDeletionController.js
grep -nE "path\\.basename|resolveSafeUploadPath|ENOENT|unlink" utils/courseFileCleanup.js
grep -nE "courseDeletionController|deletecourse" routers/userRoutes.js
grep -nE "own course|another teacher|admin|invalid course ID|missing course" tests/course-ownership.test.js

Test evidence

image image

Thanks for the contribution!
The implementation is clean, covers the required edge cases, and includes solid test coverage.
Really appreciate the attention to security and overall reliability.

@udaycodespace
udaycodespace merged commit 4dd7e5f into udaycodespace:main Aug 5, 2026
2 of 13 checks passed
@ecsoc-sentinel ecsoc-sentinel Bot added the ECSoC26-L3 Difficult, auto-assigned by Sentinel — 15 points label Aug 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ECSoC26-L3 Difficult, auto-assigned by Sentinel — 15 points ECSoC26 Required label for a PR to be eligible for Sentinel scoring good-backend PA-awarded bonus for outstanding backend work — +50 XP good-pr PA-awarded bonus for an exceptionally executed PR — +15 XP

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Security]: Restrict teacher course deletion to owned courses

2 participants