From a3b85c64caf8f3b1cb33f643319497f507abf9f7 Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Sat, 26 Apr 2025 23:31:04 -0700 Subject: [PATCH] Fixhancement: check more permissions for status consumer messages (#9804) --- .../app/data/websocket-progress-message.ts | 2 + .../services/websocket-status.service.spec.ts | 44 ++++++++++++++++++ .../app/services/websocket-status.service.ts | 23 ++++++++-- src/documents/consumer.py | 4 ++ src/paperless/consumers.py | 18 +++++--- src/paperless/tests/test_websockets.py | 46 +++++++++++++++++++ 6 files changed, 125 insertions(+), 12 deletions(-) diff --git a/src-ui/src/app/data/websocket-progress-message.ts b/src-ui/src/app/data/websocket-progress-message.ts index c8e37e232..0c540e55d 100644 --- a/src-ui/src/app/data/websocket-progress-message.ts +++ b/src-ui/src/app/data/websocket-progress-message.ts @@ -7,4 +7,6 @@ export interface WebsocketProgressMessage { message?: string document_id: number owner_id?: number + users_can_view?: number[] + groups_can_view?: number[] } diff --git a/src-ui/src/app/services/websocket-status.service.spec.ts b/src-ui/src/app/services/websocket-status.service.spec.ts index d3bf71f7e..3a4115fe6 100644 --- a/src-ui/src/app/services/websocket-status.service.spec.ts +++ b/src-ui/src/app/services/websocket-status.service.spec.ts @@ -355,6 +355,50 @@ describe('ConsumerStatusService', () => { ) }) + it('should notify user if user can view or is in group', () => { + settingsService.currentUser = { + id: 1, + username: 'testuser', + is_superuser: false, + groups: [1], + } + websocketStatusService.connect() + server.send({ + type: WebsocketStatusType.STATUS_UPDATE, + data: { + task_id: '1234', + filename: 'file1.pdf', + current_progress: 50, + max_progress: 100, + docuement_id: 12, + owner_id: 2, + status: 'WORKING', + users_can_view: [1], + groups_can_view: [], + }, + }) + expect(websocketStatusService.getConsumerStatusNotCompleted()).toHaveLength( + 1 + ) + server.send({ + type: WebsocketStatusType.STATUS_UPDATE, + data: { + task_id: '5678', + filename: 'file2.pdf', + current_progress: 50, + max_progress: 100, + docuement_id: 13, + owner_id: 2, + status: 'WORKING', + users_can_view: [], + groups_can_view: [1], + }, + }) + expect(websocketStatusService.getConsumerStatusNotCompleted()).toHaveLength( + 2 + ) + }) + it('should trigger deleted subject on document deleted', () => { let deleted = false websocketStatusService.onDocumentDeleted().subscribe(() => { diff --git a/src-ui/src/app/services/websocket-status.service.ts b/src-ui/src/app/services/websocket-status.service.ts index 13f82412f..676d36b42 100644 --- a/src-ui/src/app/services/websocket-status.service.ts +++ b/src-ui/src/app/services/websocket-status.service.ts @@ -1,6 +1,7 @@ import { Injectable } from '@angular/core' import { Subject } from 'rxjs' import { environment } from 'src/environments/environment' +import { User } from '../data/user' import { WebsocketDocumentsDeletedMessage } from '../data/websocket-documents-deleted-message' import { WebsocketProgressMessage } from '../data/websocket-progress-message' import { SettingsService } from './settings.service' @@ -173,13 +174,25 @@ export class WebsocketStatusService { } } + private canViewMessage(messageData: WebsocketProgressMessage): boolean { + // see paperless.consumers.StatusConsumer._can_view + const user: User = this.settingsService.currentUser + return ( + !messageData.owner_id || + user.is_superuser || + (messageData.owner_id && messageData.owner_id === user.id) || + (messageData.users_can_view && + messageData.users_can_view.includes(user.id)) || + (messageData.groups_can_view && + messageData.groups_can_view.some((groupId) => + user.groups?.includes(groupId) + )) + ) + } + handleProgressUpdate(messageData: WebsocketProgressMessage) { // fallback if backend didn't restrict message - if ( - messageData.owner_id && - messageData.owner_id !== this.settingsService.currentUser?.id && - !this.settingsService.currentUser?.is_superuser - ) { + if (!this.canViewMessage(messageData)) { return } diff --git a/src/documents/consumer.py b/src/documents/consumer.py index 04ba588d4..c78c21d37 100644 --- a/src/documents/consumer.py +++ b/src/documents/consumer.py @@ -137,6 +137,10 @@ class ConsumerPlugin( extra_args={ "document_id": document_id, "owner_id": self.metadata.owner_id if self.metadata.owner_id else None, + "users_can_view": (self.metadata.view_users or []) + + (self.metadata.change_users or []), + "groups_can_view": (self.metadata.view_groups or []) + + (self.metadata.change_groups or []), }, ) diff --git a/src/paperless/consumers.py b/src/paperless/consumers.py index c72b58aa7..3d045f444 100644 --- a/src/paperless/consumers.py +++ b/src/paperless/consumers.py @@ -10,14 +10,18 @@ class StatusConsumer(WebsocketConsumer): def _authenticated(self): return "user" in self.scope and self.scope["user"].is_authenticated - def _is_owner_or_unowned(self, data): + def _can_view(self, data): + user = self.scope.get("user") if self.scope.get("user") else None + owner_id = data.get("owner_id") + users_can_view = data.get("users_can_view", []) + groups_can_view = data.get("groups_can_view", []) return ( - ( - self.scope["user"].is_superuser - or self.scope["user"].id == data["owner_id"] + user.is_superuser + or user.id == owner_id + or user.id in users_can_view + or any( + user.groups.filter(pk=group_id).exists() for group_id in groups_can_view ) - if "owner_id" in data and "user" in self.scope - else True ) def connect(self): @@ -40,7 +44,7 @@ class StatusConsumer(WebsocketConsumer): if not self._authenticated(): self.close() else: - if self._is_owner_or_unowned(event["data"]): + if self._can_view(event["data"]): self.send(json.dumps(event)) def documents_deleted(self, event): diff --git a/src/paperless/tests/test_websockets.py b/src/paperless/tests/test_websockets.py index 5ba909d1c..08eec2b35 100644 --- a/src/paperless/tests/test_websockets.py +++ b/src/paperless/tests/test_websockets.py @@ -90,6 +90,52 @@ class TestWebSockets(TestCase): await communicator.disconnect() + async def test_status_update_check_perms(self): + communicator = WebsocketCommunicator(application, "/ws/status/") + + communicator.scope["user"] = mock.Mock() + communicator.scope["user"].is_authenticated = True + communicator.scope["user"].is_superuser = False + communicator.scope["user"].id = 1 + + connected, subprotocol = await communicator.connect() + self.assertTrue(connected) + + # Test as owner + message = {"type": "status_update", "data": {"task_id": "test", "owner_id": 1}} + channel_layer = get_channel_layer() + await channel_layer.group_send( + "status_updates", + message, + ) + response = await communicator.receive_json_from() + self.assertEqual(response, message) + + # Test with a group that the user belongs to + communicator.scope["user"].groups.filter.return_value.exists.return_value = True + message = { + "type": "status_update", + "data": {"task_id": "test", "owner_id": 2, "groups_can_view": [1]}, + } + channel_layer = get_channel_layer() + await channel_layer.group_send( + "status_updates", + message, + ) + response = await communicator.receive_json_from() + self.assertEqual(response, message) + + # Test with a different owner_id + message = {"type": "status_update", "data": {"task_id": "test", "owner_id": 2}} + channel_layer = get_channel_layer() + await channel_layer.group_send( + "status_updates", + message, + ) + response = await communicator.receive_nothing() + self.assertNotEqual(response, message) + await communicator.disconnect() + @mock.patch("paperless.consumers.StatusConsumer._authenticated") async def test_receive_documents_deleted(self, _authenticated): _authenticated.return_value = True