Fixhancement: check more permissions for status consumer messages (#9804)

This commit is contained in:
shamoon 2025-04-26 23:31:04 -07:00 committed by GitHub
parent 0cefdc8475
commit a3b85c64ca
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 125 additions and 12 deletions

View File

@ -7,4 +7,6 @@ export interface WebsocketProgressMessage {
message?: string
document_id: number
owner_id?: number
users_can_view?: number[]
groups_can_view?: number[]
}

View File

@ -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(() => {

View File

@ -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
}

View File

@ -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 []),
},
)

View File

@ -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):

View File

@ -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