SLEEP-1439 Add permission for offline setup zip download #1569
Conversation
|
@hakifran we need a review here i think please ! |
I'm reviewing it |
| @@ -92,7 +92,10 @@ export const usersTableColumns = ({ | |||
| } | |||
| /> | |||
| )} | |||
| {currentUser.is_superuser && ( | |||
| {userHasPermission( | |||
| Permission.MOBILE_APP_OFFLINE_SETUP, | |||
There was a problem hiding this comment.
This can be achieved by using
<DisplayIfUserHasPerm permissions={[Permission.MOBILE_APP_OFFLINE_SETUP]} > <ExportMobileAppSetupDialog selectedUser={settings.row.original} titleMessage={MESSAGES.exportMobileAppTitle} params={params} onCreateExport={exportMobileSetup} /> </DisplayIfUserHasPerm>
import { DisplayIfUserHasPerm } from '../../components/DisplayIfUserHasPerm.tsx';
iaso/api/tasks/__init__.py
Outdated
| @@ -72,7 +72,7 @@ class TaskSourceViewSet(ModelViewSet): | |||
| PATCH /api/tasks/<id> | |||
| """ | |||
|
|
|||
| permission_classes = [permissions.IsAuthenticated, HasPermission(permission.DATA_TASKS)] # type: ignore | |||
| permission_classes = [permissions.IsAuthenticated, HasPermission(permission.DATA_TASKS, permission.MOBILE_APP_OFFLINE_SETUP)] # type: ignore | |||
There was a problem hiding this comment.
@bramj I don't understand why this permission is checked here in TaskSourceVIewSet as this is about:
"""Task API
GET /api/tasks/
GET /api/tasks/<id>
PATCH /api/tasks/<id>
"""
There was a problem hiding this comment.
This was needed to enable me to display a progress bar on the zip creation (which polled the GET /api/tasks/:id that was just created).
However, you're right that it is not clear, and too permissive, so I've changed the behavior of only the GET /api/tasks/<id> endpoint.
Basically: keep the tasks endpoint permissions as is, but allow a user to query the state of a task launched by themselves, even if they don't have the DATA_TASKS permission.
IMO it makes sense to allow this, but only on the retrieve endpoint and only for tasks created by the user themself.
@hakifran what do you think?
There was a problem hiding this comment.
@bramj I do think that the user who launch the task should have the DATA_TASKS, if not don't let him launch the mobile app offline setup. Because on my point of view this can lead to a confusion about permissions. @beygorghor or @madewulf can give their point of view on this. thank you
Also: Enable task api in case of access to offline setup file. Without this, the user will see an error when the frontend tries to render the progress bar.
such as for the generation of the mobile app setup zip. Basically: keep the tasks endpoint permissions as is, but allow a user to query the state of a task launched by themselves, even if they don't have the "DATA_TASKS" permission. E.g. For the download of the mobile app setup zip, we display a progress bar. To display this, we fetch the task that was just created. But if the user doesn't have the DATA_TASKS permission, this doesn't work. It makes sense to allow this, but only on the `retrieve` endpoint and only for tasks created by the user themself.
Also: Enable task api in case of access to offline setup file.
Without this, the user will see an error when the frontend tries to render the progress bar.
Related JIRA tickets : SLEEP-1439 (already in Trypelim)
Print screen / video
How to test