Skip to content

Commit 4ce7126

Browse files
authored
Merge pull request #4776 from Discookie/relax_task_permissions
Relax permissions requirements for task management
2 parents 52ebc29 + 4f70fbd commit 4ce7126

File tree

3 files changed

+61
-38
lines changed

3 files changed

+61
-38
lines changed

docs/web/background_tasks.md

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ The most important property of a _Task_ is its **status**, which can be:
1818
* _`RUNNING`_: The task is currently being executed.
1919
* _`COMPLETED`_: The task's execution finished successfully.
2020
* _`FAILED`_: The task's execution "structurally" failed due to an "inside" property of the execution. An uncaught `Exception` would have escaped the executor's _"main"_ method.
21-
* _`CANCELLED`_: An administrator (**`SUPERUSER`**, see [the Permission system](permissions.md)) cancelled the execution of the task, and the task gracefully terminated itself.
21+
* _`CANCELLED`_: The task's owner or an administrator (**`PRODUCT_ADMIN`** or **`SUPERUSER`**, see [the Permission system](permissions.md)) cancelled the execution of the task, and the task gracefully terminated itself.
2222
* _`DROPPED`_: External influence resulted in the executing server's shutdown, and the task did not complete in a graceful way.
2323

2424
Task lifecycle
@@ -75,8 +75,7 @@ class MyThriftEndpointHandler:
7575
# The task's "User": the name of the user who is the actor which
7676
# caused the execution of the task.
7777
# The status of the task may only be queried by the relevant actor,
78-
# a PRODUCT_ADMIN (if the task is associated with a product) or
79-
# SUPERUSERs.
78+
# people with access to the product, or SUPERUSERs.
8079
"user",
8180

8281
# If the task is associated with a product, pass the ORM `Product`

web/server/codechecker_server/api/tasks.py

Lines changed: 58 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -132,18 +132,14 @@ def getTaskInfo(self, token: str) -> TaskInfo:
132132
Returns the `TaskInfo` for the task identified by `token`.
133133
"""
134134
with DBSession(self._config_db) as session:
135-
db_task: Optional[DBTask] = session.get(DBTask, token)
136-
if not db_task:
137-
raise RequestFailed(ErrorCode.GENERAL,
138-
f"Task '{token}' does not exist!")
139-
140135
has_right_to_query_status: bool = False
141136
should_set_consumed_flag: bool = False
137+
db_task: Optional[DBTask] = session.get(DBTask, token)
142138

143-
if db_task.username == self._get_username():
139+
if db_task and db_task.username == self._get_username():
144140
has_right_to_query_status = True
145141
should_set_consumed_flag = db_task.is_in_terminated_state
146-
elif db_task.product_id is not None:
142+
elif db_task and db_task.product_id is not None:
147143
associated_product: Optional[Product] = \
148144
session.get(Product, db_task.product_id)
149145
if not associated_product:
@@ -153,12 +149,11 @@ def getTaskInfo(self, token: str) -> TaskInfo:
153149
else:
154150
has_right_to_query_status = \
155151
permissions.require_permission(
156-
permissions.PRODUCT_ADMIN,
152+
permissions.PRODUCT_ACCESS,
157153
{"config_db_session": session,
158154
"productID": associated_product.id},
159155
self._auth_session)
160-
161-
if not has_right_to_query_status:
156+
else:
162157
has_right_to_query_status = permissions.require_permission(
163158
permissions.SUPERUSER,
164159
{"config_db_session": session},
@@ -167,9 +162,14 @@ def getTaskInfo(self, token: str) -> TaskInfo:
167162
if not has_right_to_query_status:
168163
raise RequestFailed(
169164
ErrorCode.UNAUTHORIZED,
170-
"Only the task's submitter, a PRODUCT_ADMIN (of the "
171-
"product the task is associated with), or a SUPERUSER "
172-
"can getTaskInfo()!")
165+
"The task does not exist, or you are not authorized to "
166+
"view the task. (Only the task's submitter, people with "
167+
"access to the task's product, or a SUPERUSER can view "
168+
"the task.)")
169+
if not db_task:
170+
raise RequestFailed(
171+
ErrorCode.GENERAL,
172+
"The task does not exist.")
173173

174174
info = _make_task_info(db_task)
175175

@@ -194,22 +194,22 @@ def getTasks(self, filters: TaskFilter) -> List[AdministratorTaskInfo]:
194194
"Querying service tasks (not associated with a "
195195
"product) requires SUPERUSER privileges!")
196196
if filters.productIDs:
197-
no_admin_products = [
197+
no_access_products = [
198198
prod_id for prod_id in filters.productIDs
199199
if not permissions.require_permission(
200-
permissions.PRODUCT_ADMIN,
200+
permissions.PRODUCT_ACCESS,
201201
{"config_db_session": session, "productID": prod_id},
202202
self._auth_session)]
203-
if no_admin_products:
204-
no_admin_products = [session.get(Product, product_id)
205-
.endpoint
206-
for product_id in no_admin_products]
203+
if no_access_products:
204+
no_access_products = [session.get(Product, product_id)
205+
.endpoint
206+
for product_id in no_access_products]
207207
# pylint: disable=consider-using-f-string
208208
raise RequestFailed(ErrorCode.UNAUTHORIZED,
209209
"Querying product tasks requires "
210-
"PRODUCT_ADMIN rights, but it is "
210+
"PRODUCT_ACCESS rights, but it is "
211211
"missing from product(s): '%s'!"
212-
% ("', '".join(no_admin_products)))
212+
% ("', '".join(no_access_products)))
213213

214214
AND = []
215215
if filters.tokens:
@@ -290,7 +290,7 @@ def getTasks(self, filters: TaskFilter) -> List[AdministratorTaskInfo]:
290290

291291
ret: List[AdministratorTaskInfo] = []
292292
has_superuser: Optional[bool] = None
293-
product_admin_rights: Dict[int, bool] = {}
293+
product_access_rights: Dict[int, bool] = {}
294294
for db_task in session.query(DBTask).filter(and_(*AND)).all():
295295
if not db_task.product_id:
296296
# Tasks associated with the server, and not a specific
@@ -306,16 +306,16 @@ def getTasks(self, filters: TaskFilter) -> List[AdministratorTaskInfo]:
306306
# Tasks associated with a product should only be visible
307307
# to PRODUCT_ADMINs of that product.
308308
try:
309-
if not product_admin_rights[db_task.product_id]:
309+
if not product_access_rights[db_task.product_id]:
310310
continue
311311
except KeyError:
312-
product_admin_rights[db_task.product_id] = \
312+
product_access_rights[db_task.product_id] = \
313313
permissions.require_permission(
314-
permissions.PRODUCT_ADMIN,
314+
permissions.PRODUCT_ACCESS,
315315
{"config_db_session": session,
316316
"productID": db_task.product_id},
317317
self._auth_session)
318-
if not product_admin_rights[db_task.product_id]:
318+
if not product_access_rights[db_task.product_id]:
319319
continue
320320

321321
ret.append(_make_admin_task_info(db_task))
@@ -333,23 +333,47 @@ def cancelTask(self, token: str) -> bool:
333333
There are no guarantees that tasks will respect this!
334334
"""
335335
with DBSession(self._config_db) as session:
336-
if not permissions.require_permission(
336+
has_right_to_cancel: bool = False
337+
db_task: Optional[DBTask] = session.get(DBTask, token)
338+
339+
if db_task and db_task.username == self._get_username():
340+
has_right_to_cancel = True
341+
elif db_task and db_task.product_id is not None:
342+
associated_product: Optional[Product] = \
343+
session.get(Product, db_task.product_id)
344+
if not associated_product:
345+
LOG.error("No product with ID '%d', but a task is "
346+
"associated with it.",
347+
db_task.product_id)
348+
else:
349+
has_right_to_cancel = \
350+
permissions.require_permission(
351+
permissions.PRODUCT_ADMIN,
352+
{"config_db_session": session,
353+
"productID": associated_product.id},
354+
self._auth_session)
355+
else:
356+
has_right_to_cancel = permissions.require_permission(
337357
permissions.SUPERUSER,
338358
{"config_db_session": session},
339-
self._auth_session):
359+
self._auth_session)
360+
361+
if not has_right_to_cancel:
340362
raise RequestFailed(
341363
ErrorCode.UNAUTHORIZED,
342-
"cancelTask() requires server-level SUPERUSER rights.")
343-
344-
db_task: Optional[DBTask] = session.get(DBTask, token)
364+
"The task does not exist, or you are not authorized to "
365+
"cancel the task. (Only the task's submitter, people with "
366+
"PRODUCT_ADMIN on the task's product, or a SUPERUSER can "
367+
"cancel the task.)")
345368
if not db_task:
346-
raise RequestFailed(ErrorCode.GENERAL,
347-
f"Task '{token}' does not exist!")
369+
raise RequestFailed(
370+
ErrorCode.GENERAL,
371+
"The task does not exist.")
348372

349373
if not db_task.can_be_cancelled:
350374
return False
351375

352-
db_task.add_comment("SUPERUSER requested cancellation.",
376+
db_task.add_comment("User requested cancellation.",
353377
self._get_username())
354378
db_task.cancel_flag = True
355379
session.commit()

web/tests/functional/tasks/test_task_management.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ def test_task_3_cancel(self):
196196
TaskStatus._NAMES_TO_VALUES["CANCELLED"])
197197
self.assertEqual(task_info.cancelFlagSet, True)
198198
self.assertIn("root", task_info.comments)
199-
self.assertIn("SUPERUSER requested cancellation.", task_info.comments)
199+
self.assertIn("User requested cancellation.", task_info.comments)
200200
self.assertIn("CANCEL!\nCancel request of admin honoured by task.",
201201
task_info.comments)
202202
self.assertIsNotNone(task_info.enqueuedAtEpoch)

0 commit comments

Comments
 (0)