Merge branch 'sharing-improvements' into 'main'

Sharing improvements

See merge request reportcreator/reportcreator!828

CHANGELOG.md

@@ -3,6 +3,11 @@
 ## Upcoming
 * Do not automatically log in (e.g. via OIDC) after logout
 * OIDC: use preferred_username as login_hint for re-authentication
+* Accept username from env variable in createorupdateuser command
+* Render markdown preview in web worker
+* Show tooltips on list actions for missing permissions
+* Project Design selection improvements: order by usage count
+* Improvements of sharing notes and reports
 
 
 ## v2025.4 - 2025-01-14

api/src/reportcreator_api/conf/settings.py

@@ -609,6 +609,7 @@ def __bool__(self):
 DISABLE_SHARING = config('DISABLE_SHARING', cast=bool, default=False)
 SHARING_PASSWORD_REQUIRED = config('SHARING_PASSWORD_REQUIRED', cast=bool, default=False)
 SHARING_READONLY_REQUIRED = config('SHARING_READONLY_REQUIRED', cast=bool, default=False)
+SHARING_MAX_FAILED_PASSWORD_ATTEMPTS = 100
 
 
 ENABLE_PRIVATE_DESIGNS = config('ENABLE_PRIVATE_DESIGNS', cast=bool, default=False)

api/src/reportcreator_api/pentests/admin.py

@@ -18,6 +18,7 @@
     ProjectNotebookPage,
     ProjectType,
     ReportSection,
+    ShareInfo,
     UploadedAsset,
     UploadedImage,
     UploadedProjectFile,
@@ -75,60 +76,58 @@ def link_notes(self, obj):
     def link_uploaded_files(self, obj):
         return admin_changelist_url('Uploaded files', 'pentests', 'uploadedprojectfile', {'linked_object_id': obj.id})
 
+    def link_shared_notes(self, obj):
+        return admin_changelist_url('Shared notes', 'pentests', 'shareinfo', {'note__project_id': obj.id})
+
 
 @admin.register(PentestFinding)
 class PentestFindingAdmin(SimpleHistoryAdmin, BaseAdmin):
     list_display = ['id', 'title', 'project', 'created', 'status']
-
-    def link_project(self, obj):
-        return admin_change_url(obj.project.name, 'pentests', 'pentestproject', obj.project.id)
+    readonly_fields = ['project']
 
 
 @admin.register(ReportSection)
 class ReportSectionAdmin(SimpleHistoryAdmin, BaseAdmin):
     list_display = ['id', 'section_label', 'project', 'status']
     list_select_related = ['project__project_type']
-
-    def link_project(self, obj):
-        return admin_change_url(obj.project.name, 'pentests', 'pentestproject', obj.project.id)
+    readonly_fields = ['project']
 
 
 @admin.register(Comment)
 class CommentAdmin(BaseAdmin):
     list_display = ['id', 'text', 'path', 'status', 'created', 'user']
+    readonly_fields = ['finding', 'section', 'user']
 
     def link_answers(self, obj):
         return admin_changelist_url('Answers', 'pentests', 'commentanswer', {'comment_id': obj.id})
 
+    def link_project(self, obj):
+        return admin_change_url(obj.project.name, 'pentests', 'pentestproject', obj.project.id)
+
 
 @admin.register(CommentAnswer)
 class CommentAnswerAdmin(BaseAdmin):
     list_display = ['id', 'text', 'created', 'user']
-
-    def link_comment(self, obj):
-        return admin_change_url('Comment', 'pentests', 'comment', obj.comment.id)
+    readonly_fields = ['comment']
 
 
 @admin.register(ProjectNotebookPage)
 class ProjectNotebookPageAdmin(SimpleHistoryAdmin, BaseAdmin):
     list_display = ['id', 'title', 'project', 'created', 'checked']
-
-    def link_project(self, obj):
-        return admin_change_url(obj.project.name, 'pentests', 'pentestproject', obj.project.id)
+    readonly_fields = ['project']
 
 
 @admin.register(UserNotebookPage)
 class UserNotebookPageAdmin(BaseAdmin):
     list_display = ['id', 'title', 'user', 'created', 'checked']
-
-    def link_user(self, obj):
-        return admin_change_url(obj.user.name, 'users', 'pentestuser', obj.user.id)
+    readonly_fields = ['user']
 
 
 @admin.register(FindingTemplate)
 class FindingTemplateAdmin(SimpleHistoryAdmin, BaseAdmin):
     list_display = ['id', 'main_translation', 'created', 'tags']
     list_select_related = ['main_translation']
+    readonly_fields = ['main_translation']
 
     def link_translations(self, obj):
         return admin_changelist_url('Translations', 'pentests', 'findingtemplatetranslation', {'template_id': obj.id})
@@ -140,57 +139,48 @@ def link_images(self, obj):
 @admin.register(FindingTemplateTranslation)
 class FindingTemplateTranslationAdmin(SimpleHistoryAdmin, BaseAdmin):
     list_display = ['id', 'title', 'language', 'created', 'status']
-
-    def link_template(self, obj):
-        return admin_change_url('Template', 'pentests', 'findingtemplate', obj.template_id)
+    readonly_fields = ['template']
 
 
 class FileAdminBase(SimpleHistoryAdmin, BaseAdmin):
     list_display = ['id', 'created', 'linked_object', 'name']
+    readonly_fields = ['linked_object']
 
 
 @admin.register(UploadedImage)
 class UploadedImageAdmin(FileAdminBase):
-    def link_project(self, obj):
-        return admin_change_url(obj.linked_object.name, 'pentests', 'pentestproject', obj.linked_object.id)
+    pass
 
 
 @admin.register(UploadedAsset)
 class UploadedAssetAdmin(FileAdminBase):
-    def link_project_type(self, obj):
-        return admin_change_url(obj.linked_object.name, 'pentests', 'projecttype', obj.linked_object.id)
+    pass
 
 
 @admin.register(UploadedProjectFile)
 class UploadedProjectFileAdmin(FileAdminBase):
-    def link_project(self, obj):
-        return admin_change_url(obj.linked_object.name, 'pentests', 'pentestproject', obj.linked_object.id)
+    pass
 
 
 @admin.register(UploadedUserNotebookImage)
 class UploadedUserNotebookImageAdmin(FileAdminBase):
-    def link_user(self, obj):
-        return admin_change_url(obj.linked_object.name, 'users', 'pentestuser', obj.linked_object.id)
+    pass
 
 
 @admin.register(UploadedTemplateImage)
 class UploadedTemplateImageAdmin(FileAdminBase):
-    def link_template(self, obj):
-        return admin_change_url(obj.linked_object.name, 'pentests', 'findingtemplate', obj.linked_object.id)
+    pass
 
 
 @admin.register(UploadedUserNotebookFile)
 class UploadedUserNotebookFileAdmin(FileAdminBase):
-    def link_user(self, obj):
-        return admin_change_url(obj.linked_object.name, 'users', 'pentestuser', obj.linked_object.id)
+    pass
 
 
 @admin.register(UserPublicKey)
 class UserPublicKeyAdmin(BaseAdmin):
     list_display = ['id', 'user', 'name', 'created', 'enabled']
-
-    def link_user(self, obj):
-        return admin_change_url(obj.user.username, 'users', 'pentestuser', obj.user.id)
+    readonly_fields = ['user']
 
     def link_encrypted_key_parts(self, obj):
         return admin_changelist_url('ArchivedProjectPublicKeyEncryptedKeyPart encrypted with this public key', 'pentests', 'archivedprojectpublickeyencryptedkeypart', {'public_key_id': obj.id})
@@ -208,24 +198,15 @@ def link_key_parts(self, obj):
 class ArchivedProjectKeyPartAdmin(BaseAdmin):
     list_display = ['id', 'archived_project', 'user', 'is_decrypted', 'decrypted_at']
     list_select_related = ['archived_project', 'user']
-
-    def link_user(self, obj):
-        return admin_change_url(obj.user.username, 'users', 'pentestuser', obj.user.id)
-
-    def link_archive(self, obj):
-        return admin_change_url(obj.archived_project.name, 'pentests', 'archivedproject', obj.archived_project.id)
+    readonly_fields = ['archived_project', 'user']
 
     def link_encrypted_key_parts(self, obj):
         return admin_changelist_url('Encrypted key part data', 'pentests', 'archivedprojectpublickeyencryptedkeypart', {'key_part_id': obj.id})
 
 
 @admin.register(ArchivedProjectPublicKeyEncryptedKeyPart)
 class ArchivedProjectPublicKeyEncryptedKeyPartAdmin(BaseAdmin):
-    def link_key_part(self, obj):
-        return admin_change_url('Archive key part', 'pentests', 'archivedprojectkeypart', obj.key_part.id)
-
-    def link_public_key(self, obj):
-        return admin_change_url(obj.public_key.name, 'pentests', 'userpublickey', obj.public_key.id)
+    readonly_fields = ['key_part', 'public_key', 'encrypted_data']
 
 
 @admin.register(CollabEvent)
@@ -236,4 +217,20 @@ class CollabEventAdmin(BaseAdmin):
 @admin.register(CollabClientInfo)
 class CollabClientInfoAdmin(BaseAdmin):
     list_display = ['id', 'created', 'user', 'client_id', 'path']
+    readonly_fields = ['user']
+
+
+@admin.register(ShareInfo)
+class ShareInfoAdmin(BaseAdmin):
+    list_display = ['id', 'created', 'project_name', 'note_title']
+    readonly_fields = ['note', 'shared_by']
+    list_select_related = ['note__project']
 
+    def project_name(self, obj):
+        return obj.note.project.name
+
+    def note_title(self, obj):
+        return obj.note.title
+
+    def link_project(self, obj):
+        return admin_change_url(obj.note.project.name, 'pentests', 'pentestproject', obj.note.project.id)

api/src/reportcreator_api/pentests/migrations/0061_shareinfo_failed_password_attempts.py

@@ -0,0 +1,18 @@
+# Generated by Django 5.1.4 on 2025-01-13 09:31
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('pentests', '0060_projecttype_usage_count'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='shareinfo',
+            name='failed_password_attempts',
+            field=models.PositiveIntegerField(default=0),
+        ),
+    ]

api/src/reportcreator_api/pentests/models/notes.py

@@ -59,6 +59,8 @@ class ShareInfo(BaseModel):
     shared_by = models.ForeignKey(to=PentestUser, on_delete=models.SET_NULL, null=True, blank=True)
     comment = models.TextField(null=True, blank=True)
 
+    failed_password_attempts = models.PositiveIntegerField(default=0)
+
     objects = querysets.ShareInfoManager()
 
     @property

api/src/reportcreator_api/pentests/models/project.py

@@ -511,6 +511,14 @@ def project_id(self):
             return self.section.project_id
         return None
 
+    @property
+    def project(self):
+        if self.finding_id:
+            return self.finding.project
+        elif self.section_id:
+            return self.section.project
+        return None
+
 
 class CommentAnswer(BaseModel):
     comment = models.ForeignKey(to=Comment, on_delete=models.CASCADE, related_name="answers")

api/src/reportcreator_api/pentests/querysets.py

@@ -514,6 +514,9 @@ def only_active(self):
             .filter(expire_date__gte=timezone.now().date()) \
             .filter(is_revoked=False)
 
+    def increment_failed_password_attempts(self):
+        return self.update(failed_password_attempts=models.F('failed_password_attempts') + 1)
+
 
 class ShareInfoManager(models.Manager.from_queryset(ShareInfoQuerySet)):
     pass

api/src/reportcreator_api/pentests/serializers/notes.py

@@ -266,6 +266,13 @@ def validate_password(self, value):
         if not self.instance.password:
             raise serializers.ValidationError('No password set')
         if not constant_time_compare(value, self.instance.password):
+            # Increment failed PW counter
+            ShareInfo.objects.filter(pk=self.instance.pk).increment_failed_password_attempts()
+            self.instance.refresh_from_db()
+            # Revoke share if too many failed attempts => brute force protection
+            if self.instance.failed_password_attempts >= settings.SHARING_MAX_FAILED_PASSWORD_ATTEMPTS:
+                self.instance.is_revoked = True
+                self.instance.save()
             raise serializers.ValidationError('Invalid password')
         return value
 

api/src/reportcreator_api/pentests/signals.py

@@ -87,6 +87,12 @@ def project_set_readonly_history(sender, instance, *args, **kwargs):
     sysreptor_signals.post_finish.send(sender=instance.__class__, instance=instance)
 
 
+@receiver(signals.pre_save, sender=ShareInfo)
+def share_info_reset_failed_password_attempts(sender, instance, *args, **kwargs):
+    if not instance.is_revoked and 'is_revoked' in instance.changed_fields:
+        instance.failed_password_attempts = 0
+
+
 def update_project_report_sections_structure(projects, project_type, patch_previous_project_history=False):
     # Update structure of all reports using that project_type
     sections_to_create = []

api/src/reportcreator_api/tests/test_sharing.py

@@ -1,10 +1,12 @@
 import pytest
+from django.test import override_settings
 from django.urls import reverse
 
+from reportcreator_api.pentests.models.notes import ShareInfo
 from reportcreator_api.tests.mock import api_client, create_project, create_projectnotebookpage, create_shareinfo
 
 
-@pytest.mark.django_db()
+@pytest.mark.django_db
 class TestSharedPermissions:
     @pytest.fixture(autouse=True)
     def setUp(self):
@@ -92,7 +94,7 @@ def test_access_images(self, filename, expected):
         assert res.status_code == (200 if expected else 403)
 
 
-@pytest.mark.django_db()
+@pytest.mark.django_db
 class TestSharePasswordAuth:
     @pytest.fixture(autouse=True)
     def setUp(self):
@@ -128,3 +130,26 @@ def test_password_valid(self):
         share_info_other = create_shareinfo(note=self.note, password=self.password + 'other')
         res = self.client.get(reverse('sharednote-detail', kwargs={'shareinfo_pk': share_info_other.id, 'id': self.note.note_id}))
         assert res.status_code == 403
+
+    def test_password_brute_force_protection(self):
+        with override_settings(SHARING_MAX_FAILED_PASSWORD_ATTEMPTS=1):
+            res = self.client.post(reverse('publicshareinfo-auth', kwargs={'pk': self.share_info.id}), data={'password': 'invalid'})
+            assert res.status_code == 400
+
+            # Locked
+            self.share_info.refresh_from_db()
+            assert self.share_info.failed_password_attempts == 1
+            assert self.share_info.is_revoked
+            assert not self.share_info.is_active
+
+            res = self.client.post(reverse('publicshareinfo-auth', kwargs={'pk': self.share_info.id}), data={'password': self.password})
+            assert res.status_code == 404
+
+            # Unlock
+            self.share_info = ShareInfo.objects.get(id=self.share_info.id)
+            self.share_info.is_revoked = False
+            self.share_info.save()
+
+            assert self.share_info.failed_password_attempts == 0
+            res = self.client.post(reverse('publicshareinfo-auth', kwargs={'pk': self.share_info.id}), data={'password': self.password})
+            assert res.status_code == 200

api/src/reportcreator_api/utils/admin.py

@@ -12,7 +12,7 @@ class BaseAdmin(admin.ModelAdmin):
 
     def get_readonly_fields(self, request, obj):
         readonly_fields = super().get_readonly_fields(request, obj)
-        return readonly_fields + tuple(set([f for f in dir(self) if f.startswith('link_')]).difference(readonly_fields))
+        return tuple(readonly_fields) + tuple(set([f for f in dir(self) if f.startswith('link_')]).difference(readonly_fields))
 
 
 def admin_url(label, app_name, model_name, type_name, params=None, *args, **kwargs):

packages/frontend/src/components/Notes/ShareDialog.vue

@@ -69,6 +69,7 @@
                 v-model="createShareInfoForm.data"
                 :disabled="createShareInfoForm.saveInProgress || !canShare"
                 :error="createShareInfoForm.error"
+                :hidden-fields="['is_revoked']"
               />
               <btn-confirm
                 :action="createShareInfo"