fix/IDOR vulnerability

[atharv02-git]

Description

• This PR resolves a security vulnerability (IDOR – Insecure Direct Object Reference) in the OnTrack system, where non-privileged users (students) were able to retrieve sensitive staff details (e.g., main_convenor_id, tutor_id, email, full_name) through the /api/units/:id endpoint.
• Note: For a detailed technical breakdown and documentation for validation steps:

Root Cause

• The data exposure occurred in the serializer layer, particularly in the Entities::UnitEntity class.
• These entity class defines how backend models are converted into API JSON responses, and they were exposing sensitive fields without checking user roles.

Fixes # (issue)

Fix Summary

• Data Exposure Happens in the Serializer:
  • The UnitEntity class is responsible for deciding what fields of the Unit model are exposed. Fields such as :staff, :main_convenor_id, and :overseer_image_id were conditionally exposed but lacked proper role validation.

Validation:

• Though I have mentioned on how to validate the fix in the technical documentation, sharing the before validation and after validation screenshots.

Before

• The below Postman response tells that, students were able to access insecure data by hitting the endpoint.
  

After:

• The After Screenshot tells that, after implementing Role based filtering student's are unable to see staff data anymore.
  

• However I would like to point out that, the code changes which I made earlier to implement Role Based filtering, the logic was breaking the actual functionality of the OnTrack, so after this change, the staff are able to access the privileged data as before but students are unable to.

• Postman API response when logged in by Convenor:
  

Type of change

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)

How Has This Been Tested?

• Verified API responses via Postman for student, tutor, and admin users.
• Confirmed that unauthorized fields like main_convenor_id, tutor_id, email, and full_name are no longer visible to student users.
• Manual testing on the /api/units/:id and /api/unit_roles endpoints.
• Verified login session handling with updated RBAC behavior.

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation if appropriate
• My changes generate no new warnings

[DarrylO21]

Hi @atharv02-git, I have tested your fix for the IDOR vulnerability and can confirm that it has been resolved. The sensitive staff information (main_convenor_id, tutor_id, email, full_name) is no longer accessible to non-privileged users such as students. The access control now correctly restricts this data based on user role, and the fix appears to be working as intended. Well done on implementing a clear and maintainable fix for this issue.

Best regards,
Darryl
PT and SCR Senior Lead, AppAttack

[lachlan-robinson]

Hi @atharv02-git, This is a well-executed fix that resolves the IDOR vulnerability with as few changes to the codebase as possible.

• The unit entity now restricts staff data from being exposed to non-staff members through the API.
• I was able to confirm this using Postman.
• The app loads without new errors and appears functional after the changes.

[ibi420]

Hello, following a thorough review of the documentation, I confirmed the vulnerability and verified that the fix effectively mitigates this vulnerability. Reconfirming with Postman, the backend now restricts access based on roles. I initially followed your prior documentation and ended up implementing a fix that broke communication with the front end, leading me to believe that was still the issue. Following our conversation, I have pulled the most recent commits as is and confirmed that isn't the case. Thank you for the correction and the opportunity to review your work.

[aNebula]

Please open an upstream PR against 9.x branch on doubtfire-lms/doubtfire-api. Remember to include a description, and may be even a link to the documentation.

[atharv02-git]

Please open an upstream PR against 9.x branch on doubtfire-lms/doubtfire-api. Remember to include a description, and may be even a link to the documentation.

Hi @aNebula,
I have opened the upstream Pull Request to the doubtfire-lms/doubtfire-api repository (9.x branch) from my alternate GitHub account, as my primary one already has an existing fork.
Just confirming that I’ve included all relevant references in the PR description - including the original PR, peer reviews, and the documentation link - as instructed.
Thanks again for your guidance throughout the trimester!