Skip to content

Commit 9bb5158

Browse files
committed
[fix] Addressed review comments
- Replaced the assert authorizing websocket connections with an explicit conditional, which python -O cannot strip - Rechecked permissions and organization scope on every websocket request and pushed update, closing the socket when access was revoked - Accepted binary frames in BatchCommandConsumer.receive() and ignored them - Revalidated batch ownership when creating child commands and before executing each one, skipping devices transferred to another organization - Made the batch status write conditional on the status the calculation started from, so a stale result cannot overwrite a newer one - Evaluated the confirm page device ids once, so the reviewed set and the executed set cannot diverge - Rotated the wizard token when the target set changes, dropping the exclusions of the previous set - Sent the active filters with the reconnect request, applied them before pagination and reconciled the table against the filtered snapshot - Moved the batch command signal receivers to connection/handlers.py - Moved skipped device counting, filtering and row building to the model - Broadcast canonical status and ISO timestamps, rendering them in the browser like the single command page does - Derived the device admin URLs from the swapped model - Restored the wizard when the confirm page Back link is used - Returned the union of the schemas available to superusers - Made the password masking of the input summary case insensitive - Restored the punctuation of the command validation messages - Pluralized the device count of the confirm page - Documented mass commands as asynchronous with a concurrency which depends on the celery workers - Removed the leftovers of the dropped Django 4.2 support
1 parent 73b2315 commit 9bb5158

21 files changed

Lines changed: 1632 additions & 466 deletions

File tree

docs/user/shell-commands.rst

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -241,9 +241,11 @@ superusers).
241241

242242
.. note::
243243

244-
Commands are executed in the background, one device at a time, so a
245-
mass command sent to many devices keeps updating for a while after the
246-
page is opened.
244+
Commands are executed asynchronously in the background: each device is
245+
handled by an independent background task, so how many devices are
246+
contacted at the same time depends on the concurrency of the Celery
247+
workers. A mass command sent to many devices keeps updating for a
248+
while after the page is opened.
247249

248250
Finding Past Mass Commands
249251
~~~~~~~~~~~~~~~~~~~~~~~~~~

docs/user/websocket-api.rst

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -204,14 +204,24 @@ When the mass command itself changes, for example when it moves from
204204
"type": "batch_status",
205205
"id": "<uuid>", // Mass command identifier
206206
"label": "<string>", // Label given when it was sent
207+
"notes": "<string>", // Notes given when it was sent
208+
"input": { /* ... */ }, // Command input, masked for "change_password"
209+
"organization": "<uuid>", // Organization, null when system wide
210+
"group": "<uuid>", // Device group target, null when not used
211+
"location": "<uuid>", // Location target, null when not used
207212
"status": "<string>", // "idle", "in-progress", "success" or "failed"
208-
"status_display": "<string>", // Status as shown in the user interface
213+
"created": "<string>", // ISO 8601 timestamp
214+
"modified": "<string>", // ISO 8601 timestamp
209215
"affected_devices": <integer>, // Number of devices the command runs on
210216
"skipped_count": <integer>, // Number of devices which were skipped
211217
"skipped_preview": [ /* ... */], // First and last skipped devices, with the reason
212218
"total_rows": <integer> // Affected plus skipped devices
213219
}
214220
221+
The status and the timestamps are sent as they are stored, without
222+
translation or formatting, so that each client can render them with its
223+
own language and time zone.
224+
215225
When the command of one device changes:
216226

217227
.. code-block:: javascript
@@ -221,11 +231,14 @@ When the command of one device changes:
221231
"id": "<uuid>", // Command identifier
222232
"device": "<uuid>", // Device identifier
223233
"device_name": "<string>", // Device name
234+
"connection": "<uuid>", // Device connection used, may be null
235+
"batch_command": "<uuid>", // Mass command this command belongs to
224236
"status": "<string>", // "in-progress", "success" or "failed"
225-
"status_display": "<string>", // Status as shown in the user interface
226237
"output": "<string>", // Output collected so far
227-
"modified": "<string>", // Last modification, formatted for display
238+
"created": "<string>", // ISO 8601 timestamp
239+
"modified": "<string>", // ISO 8601 timestamp
228240
"index": <integer>, // Position of the row, sent only for new commands
241+
"affected_devices": <integer>, // Commands created so far, sent with "index"
229242
"total_rows": <integer> // Affected plus skipped devices, sent with "index"
230243
}
231244
@@ -239,16 +252,34 @@ for the results it missed:
239252
240253
{
241254
"type": "request_current_state",
242-
"page": 1 // Page of results, 20 rows per page
255+
"page": 1, // Page of results, 20 rows per page
256+
"filters": { // Optional, the filters the page is showing
257+
"q": "<string>", // Search term matched against the device name
258+
"status": "<string>", // Command status, or "skipped"
259+
"location_id": "<uuid>", // Location of the device
260+
"group_id": "<uuid>", // Device group
261+
"organization_id": "<uuid>" // Organization of the device
262+
}
243263
}
244264
265+
Every filter is optional and an empty string means the filter is not
266+
active. The filters are applied before the results are paginated, so a
267+
client which is showing a filtered table receives the same rows it would
268+
get by reloading the page.
269+
245270
The server replies with one message holding that page:
246271

247272
.. code-block:: javascript
248273
249274
{
250275
"type": "batch_state",
251-
"batch_status": { /* ... */ }, // Same fields as the "batch_status" message
252-
"commands": [ /* ... */ ], // Rows of the requested page
253-
"total_rows": <integer> // Affected plus skipped devices
276+
"batch_status": { /* ... */ }, // Fields of the "batch_status" message,
277+
// except "type" and "total_rows"
278+
"commands": [ /* ... */ ], // Rows of the requested page, in the shape
279+
// of the "command_update" message
280+
"total_rows": <integer> // Rows matching the filters, for the paginator
254281
}
282+
283+
Rows of devices which were skipped are included in ``commands`` with
284+
``is_skipped`` set to ``true``, a ``status`` of ``skipped`` and the reason
285+
in ``output``.

openwisp_controller/config/base/channels_consumer.py

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@ class BaseDeviceConsumer(WebsocketConsumer):
1010
channel_layer_group = "config.device"
1111

1212
def _is_user_authenticated(self):
13-
return self.scope["user"].is_authenticated is True
13+
user = self.scope.get("user")
14+
return user is not None and user.is_authenticated
1415

1516
def is_user_authorized(self):
1617
user = self.scope["user"]
@@ -32,16 +33,18 @@ def _user_has_permissions(self, add=True, change=True, delete=True):
3233
return self.scope["user"].has_perms(permissions)
3334

3435
def connect(self):
36+
if not self._is_user_authenticated() or not self.is_user_authorized():
37+
self.close()
38+
return
3539
try:
36-
assert self._is_user_authenticated() and self.is_user_authorized()
3740
self.pk_ = self.scope["url_route"]["kwargs"]["pk"]
38-
except (AssertionError, KeyError):
41+
except KeyError:
3942
self.close()
40-
else:
41-
async_to_sync(self.channel_layer.group_add)(
42-
f"{self.channel_layer_group}-{self.pk_}", self.channel_name
43-
)
44-
self.accept()
43+
return
44+
async_to_sync(self.channel_layer.group_add)(
45+
f"{self.channel_layer_group}-{self.pk_}", self.channel_name
46+
)
47+
self.accept()
4548

4649
def disconnect(self, close_code):
4750
try:

0 commit comments

Comments
 (0)