Skip to content

Fix reader file upload#15133

Open
samiat4911 wants to merge 1 commit into
DefectDojo:devfrom
samiat4911:fix/reader-file-upload
Open

Fix reader file upload#15133
samiat4911 wants to merge 1 commit into
DefectDojo:devfrom
samiat4911:fix/reader-file-upload

Conversation

@samiat4911

@samiat4911 samiat4911 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Description

This PR allows users with file add access to upload product tracking files on Engagement, Test, and Finding objects without requiring full edit access to those objects.

The previous behavior reused generic related-object permissions where file upload POST requests were treated like object edits. That blocked reader-style users from attaching supporting evidence files. The fix separates file permissions from generic related-object permissions and uses the existing Product_Tracking_Files_* permission enum for API file endpoints.

class UserHasFindingFilePermission(BaseRelatedObjectPermission):
    permission_map = {
        "get_permission": Permissions.Product_Tracking_Files_View,
        "put_permission": Permissions.Product_Tracking_Files_Edit,
        "delete_permission": Permissions.Product_Tracking_Files_Delete,
        "post_permission": Permissions.Product_Tracking_Files_Add,
    }

The Engagement, Test, and Finding file actions now use these file-specific permission classes instead of the broader related-object permission classes.

@action(
    detail=True, methods=["get", "post"], parser_classes=(MultiPartParser,), permission_classes=(IsAuthenticated, permissions.UserHasFindingFilePermission),
)
def files(self, request, pk=None):

For the classic UI manage_files page, the PR keeps edit users on the existing full manage formset, but gives add-only users a separate formset that can upload new files without exposing existing-file delete controls.

has_file_add_permission = user_has_permission(request.user, obj, Permissions.Product_Tracking_Files_Add)
has_file_edit_permission = user_has_permission(request.user, obj, Permissions.Product_Tracking_Files_Edit)
if not (has_file_add_permission or has_file_edit_permission):
    raise PermissionDenied

formset_class = ManageFileFormSet if has_file_edit_permission else AddOnlyManageFileFormSet
files_queryset = obj.files.all() if has_file_edit_permission else FileUpload.objects.none()
AddOnlyManageFileFormSet = modelformset_factory(FileUpload, extra=3, max_num=10, fields=["title", "file"], can_delete=False, formset=BaseManageFileFormSet)
```**Test results**

Added and updated unit coverage in `unittests/test_permissions_audit.py` for:

- Engagement file upload by a member/reader-style user.
- Finding file upload by a member/reader-style user.
- Test file upload by a member/reader-style user.
- UI `manage_files` upload to a Finding by a member/reader-style user.

Validation run:

```bash
ruff check --isolated dojo/forms.py dojo/views.py dojo/authorization/api_permissions.py dojo/engagement/api/views.py dojo/finding/api/views.py dojo/test/api/views.py unittests/test_permissions_audit.py
python -m compileall dojo/forms.py dojo/views.py dojo/authorization/api_permissions.py dojo/engagement/api/views.py dojo/finding/api/views.py dojo/test/api/views.py unittests/test_permissions_audit.py
docker compose -f docker-compose.yml -f docker-compose.override.unit_tests.yml run --rm --entrypoint 'python manage.py check' uwsgi
docker compose -f docker-compose.yml -f docker-compose.override.unit_tests.yml run --rm --entrypoint 'python manage.py test unittests.test_permissions_audit.TestRelatedObjectPermissions --keepdb -v 2' uwsgi
docker compose -f docker-compose.yml -f docker-compose.override.unit_tests.yml run --rm --entrypoint 'python manage.py test unittests.test_permissions_audit.TestRelatedObjectPermissions.test_manage_files_member_can_upload_to_finding --keepdb -v 2' uwsgi

Results:

  • Ruff passed.
  • Python compile check passed.
  • Django system check passed with only the existing staticfiles.W004 warning for /app/components/node_modules.
  • TestRelatedObjectPermissions passed: 39 tests, 17 skipped.
  • The focused UI regression test passed.

Documentation

Please update any documentation when needed in the documentation folder)

Checklist

This checklist is for your information.

  • Make sure to rebase your PR against the very latest dev.
  • Features/Changes should be submitted against the dev.
  • Bugfixes should be submitted against the bugfix branch.
  • Give a meaningful name to your PR, as it may end up being used in the release notes.
  • Your code is Ruff compliant (see ruff.toml).
  • Your code is python 3.13 compliant.
  • If this is a new feature and not a bug fix, you've included the proper documentation in the docs at https://github.com/DefectDojo/django-DefectDojo/tree/dev/docs as part of this PR.
  • Model changes must include the necessary migrations in the dojo/db_migrations folder.
  • Add applicable tests to the unit tests.
  • Add the proper label to categorize your PR.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant