Merge branch 'sharing-ui-bugfixes' into 'main'

Sharing ui bugfixes See merge request reportcreator/reportcreator!721
Syslifters · Sep 24, 2024 · f9d17d6 · f9d17d6
2 parents 79b6db9 + 379251c
commit f9d17d6
Show file tree

Hide file tree

Showing 7 changed files with 46 additions and 22 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,7 @@
 * Add more granular file storage settings
 * Add trusted types configuration to Content Security Policy
 * Show markdown snippet in markdown image preview dialogs
+* Bugfixes in note sharing
 
 
 ## v2024.74 - 2024-09-24

diff --git a/api/src/reportcreator_api/pentests/consumers.py b/api/src/reportcreator_api/pentests/consumers.py
@@ -157,6 +157,7 @@ def has_permission(self, action=None, **kwargs):
     def get_notes_queryset(self):
         return ProjectNotebookPage.objects \
             .filter(project_id=self.related_id) \
+            .annotate_is_shared() \
             .select_related('parent', 'assignee')
 
 
@@ -199,7 +200,9 @@ def get_notes_queryset(self):
         share_info = self.get_share_info()
         if not share_info:
             return ProjectNotebookPage.objects.none()
-        return ProjectNotebookPage.objects.child_notes_of(share_info.note)
+        return ProjectNotebookPage.objects \
+            .child_notes_of(share_info.note) \
+            .annotate(is_shared=models.Q(id=share_info.note.id))
 
     async def filter_events(self, events):
         @database_sync_to_async

diff --git a/api/src/reportcreator_api/pentests/serializers/notes.py b/api/src/reportcreator_api/pentests/serializers/notes.py
@@ -46,9 +46,9 @@ class Meta(NotebookPageSerializerBase.Meta):
         fields = NotebookPageSerializerBase.Meta.fields + ['assignee', 'is_shared']
 
     def get_is_shared(self, obj) -> bool:
-        if (is_mfa_enabled := getattr(obj, 'is_mfa_enabled', None)) is not None:
-            return is_mfa_enabled
-        return obj.shareinfos.all().exists()
+        if (is_shared := getattr(obj, 'is_shared', None)) is not None:
+            return is_shared
+        return obj.shareinfos.only_active().exists()
 
 
 class UserNotebookPageSerializer(NotebookPageSerializerBase, serializers.ModelSerializer):

diff --git a/api/src/reportcreator_api/pentests/signals.py b/api/src/reportcreator_api/pentests/signals.py
@@ -32,6 +32,7 @@
     ProjectNotebookPage,
     ProjectType,
     ReportSection,
+    ShareInfo,
     SourceEnum,
     UploadedAsset,
     UploadedImage,
@@ -411,6 +412,7 @@ def uploaded_file_deleted(sender, instance, *args, **kwargs):
 @receiver(signals.post_save, sender=PentestProject)
 @receiver(signals.post_save, sender=Comment)
 @receiver(signals.post_save, sender=CommentAnswer)
+@receiver(signals.post_save, sender=ShareInfo)
 @disable_for_loaddata
 def collab_updated(sender, instance, created=False, *args, **kwargs):
     from reportcreator_api.pentests.consumers import send_collab_event_project, send_collab_event_user
@@ -443,7 +445,18 @@ def collab_updated(sender, instance, created=False, *args, **kwargs):
                     data={'value': getattr(instance, k)},
                 ))
         return
-    if isinstance(instance, CommentAnswer):
+    elif isinstance(instance, ShareInfo):
+        if created:
+            send_collab_event_project(CollabEvent.objects.create(
+                related_id=instance.note.project_id,
+                type=CollabEventType.UPDATE_KEY,
+                path=f'notes.{instance.note.note_id}.is_shared',
+                created=instance.note.updated,
+                version=instance.note.updated.timestamp(),
+                data={'value': True},
+            ))
+        return
+    elif isinstance(instance, CommentAnswer):
         instance = instance.comment
         sender = Comment
         created = False

diff --git a/api/src/reportcreator_api/tests/test_collab.py b/api/src/reportcreator_api/tests/test_collab.py
@@ -594,6 +594,14 @@ def import_notes():
             await self.assert_event({'type': CollabEventType.CREATE, 'path': f'notes.{n["id"]}', 'value': n, 'client_id': None})
         await self.assert_event({'type': CollabEventType.SORT, 'path': 'notes', 'client_id': None})
 
+    async def test_shareinfo_sync(self):
+        res = await sync_to_async(self.api_client1.post)(
+            path=reverse('shareinfo-list', kwargs={'project_pk': self.project.id, 'note_id': self.note.note_id}),
+            data={'expire_date': (timezone.now() + timedelta(days=10)).date()},
+        )
+        assert res.status_code == 201, res.data
+        await self.assert_event({'type': CollabEventType.UPDATE_KEY, 'path': self.note_path_prefix + '.is_shared', 'value': True, 'client_id': None})
+
     async def test_member_removed_write(self):
         await ProjectMemberInfo.objects.filter(project=self.project, user=self.user1).adelete()
 

diff --git a/api/src/reportcreator_api/users/serializers.py b/api/src/reportcreator_api/users/serializers.py
@@ -41,7 +41,6 @@ def get_is_mfa_enabled(self, obj) -> bool:
 
     def get_extra_kwargs(self):
         user = self.context['request'].user
-        # TODO: who can set project_admin permissions: user_manager vs admin
         read_only = not (getattr(user, 'is_user_manager', False) or getattr(user, 'is_admin', False))
         return super().get_extra_kwargs() | {
             'is_superuser': {'read_only': not getattr(user, 'is_admin', False)},

diff --git a/frontend/src/components/Notes/ShareDialog.vue b/frontend/src/components/Notes/ShareDialog.vue
@@ -49,34 +49,34 @@
         </template>
         <template #default>
           <v-container fluid>
-            <div v-if="createShareInfoForm">
+            <div v-if="currentShareInfo">
               <notes-share-info-form
-                v-model="createShareInfoForm.data"
-                :disabled="createShareInfoForm.saveInProgress || !canShare"
-                :error="createShareInfoForm.error"
+                v-model="currentShareInfo"
+                :disabled="!canShare"
               />
               <btn-confirm
-                :action="createShareInfo"
-                :disabled="!canShare"
-                :loading="createShareInfoForm.saveInProgress"
+                :action="() => updateShareInfo(currentShareInfo!)"
+                :disabled="!canShare || (isEqual(currentShareInfo, shareInfos.find(si => si.id === currentShareInfo?.id)))"
                 :confirm="false"
-                button-text="Share"
-                button-icon="mdi-share-variant"
+                button-text="Update"
+                button-icon="mdi-content-save"
                 button-color="primary"
                 class="mt-4"
               />
             </div>
-            <div v-else-if="currentShareInfo">
+            <div v-else-if="createShareInfoForm">
               <notes-share-info-form
-                v-model="currentShareInfo"
-                :disabled="!canShare"
+                v-model="createShareInfoForm.data"
+                :disabled="createShareInfoForm.saveInProgress || !canShare"
+                :error="createShareInfoForm.error"
               />
               <btn-confirm
-                :action="() => updateShareInfo(currentShareInfo!)"
-                :disabled="!canShare || (isEqual(currentShareInfo, shareInfos.find(si => si.id === currentShareInfo?.id)))"
+                :action="createShareInfo"
+                :disabled="!canShare"
+                :loading="createShareInfoForm.saveInProgress"
                 :confirm="false"
-                button-text="Update"
-                button-icon="mdi-content-save"
+                button-text="Share"
+                button-icon="mdi-share-variant"
                 button-color="primary"
                 class="mt-4"
               />