Skip to content

Commit e64d30c

Browse files
author
Andrei Neagu
committed
refactor access_rights
1 parent 8646c78 commit e64d30c

File tree

2 files changed

+89
-59
lines changed

2 files changed

+89
-59
lines changed

services/storage/src/simcore_service_storage/modules/db/access_layer.py

Lines changed: 84 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -262,19 +262,13 @@ async def get_project_access_rights(
262262
# determine user's access rights by aggregating AR of all groups
263263
return _aggregate_access_rights(row.access_rights, user_group_ids)
264264

265-
async def get_file_access_rights( # pylint:disable=too-many-return-statements # noqa: PLR0911
265+
async def _get_access_from_metadata_entry(
266266
self,
267267
*,
268-
connection: AsyncConnection | None = None,
268+
connection: AsyncConnection | None,
269269
user_id: UserID,
270270
file_id: StorageFileID,
271-
) -> AccessRights:
272-
"""
273-
Returns access-rights of user (user_id) over data file resource (file_id)
274-
275-
raises InvalidFileIdentifier
276-
"""
277-
271+
) -> AccessRights | None:
278272
#
279273
# 1. file registered in file_meta_data table
280274
#
@@ -285,67 +279,100 @@ async def get_file_access_rights( # pylint:disable=too-many-return-statements #
285279
result = await conn.execute(stmt)
286280
row = result.one_or_none()
287281

288-
if row:
289-
if int(row.user_id) == user_id:
290-
# is owner
282+
if not row:
283+
return None
284+
285+
if int(row.user_id) == user_id:
286+
# is owner
287+
return AccessRights.all()
288+
289+
if not row.project_id:
290+
# not owner and not shared via project
291+
return AccessRights.none()
292+
293+
# has associated project
294+
access_rights = await self.get_project_access_rights(
295+
user_id=user_id, project_id=row.project_id
296+
)
297+
if not access_rights:
298+
_logger.warning(
299+
"File %s references a project %s that does not exists in db. "
300+
" TIP: Audit sync between files_meta_data and projects tables",
301+
file_id,
302+
row.project_id,
303+
)
304+
return AccessRights.none()
305+
306+
return access_rights
307+
308+
async def _get_access_without_metardata_entry(
309+
self,
310+
*,
311+
user_id: UserID,
312+
file_id: StorageFileID,
313+
) -> AccessRights:
314+
#
315+
# 2. file is NOT registered in meta-data table e.g. it is about to be uploaded or it was deleted
316+
# We rely on the assumption that file_id is formatted either as
317+
#
318+
# - project's data: {project_id}/{node_id}/{filename/with/possible/folders}
319+
# - API data: api/{file_id}/{filename/with/possible/folders}
320+
# - Exporter data: exporter/{user_id}/{filename/with/possible/folders}
321+
#
322+
323+
try:
324+
parent, _, _ = file_id.split("/", maxsplit=2)
325+
326+
if parent == "api":
327+
# ownership still not defined, so we assume it is user_id
291328
return AccessRights.all()
292329

293-
if not row.project_id:
294-
# not owner and not shared via project
295-
return AccessRights.none()
330+
if parent == EXPORTS_S3_PREFIX:
331+
# ownership still not defined, so we assume it is user_id
332+
# NOTE: all permissions are required for: downloading, uploading and aborting
333+
return AccessRights.all()
296334

297-
# has associated project
335+
# otherwise assert 'parent' string corresponds to a valid UUID
298336
access_rights = await self.get_project_access_rights(
299-
user_id=user_id, project_id=row.project_id
337+
user_id=user_id, project_id=ProjectID(parent)
300338
)
301339
if not access_rights:
302340
_logger.warning(
303-
"File %s references a project %s that does not exists in db."
304-
"TIP: Audit sync between files_meta_data and projects tables",
341+
"File %s references a project that does not exists in db",
305342
file_id,
306-
row.project_id,
307343
)
308344
return AccessRights.none()
309345

310-
else:
311-
#
312-
# 2. file is NOT registered in meta-data table e.g. it is about to be uploaded or it was deleted
313-
# We rely on the assumption that file_id is formatted either as
314-
#
315-
# - project's data: {project_id}/{node_id}/{filename/with/possible/folders}
316-
# - API data: api/{file_id}/{filename/with/possible/folders}
317-
# - Exporter data: exporter/{user_id}/{filename/with/possible/folders}
318-
#
319-
try:
320-
parent, _, _ = file_id.split("/", maxsplit=2)
321-
322-
if parent == "api":
323-
# ownership still not defined, so we assume it is user_id
324-
return AccessRights.all()
325-
326-
if parent == EXPORTS_S3_PREFIX:
327-
# ownership still not defined, so we assume it is user_id
328-
# NOTE: all permissions are required for: downloading, uploading and aborting
329-
return AccessRights.all()
330-
331-
# otherwise assert 'parent' string corresponds to a valid UUID
332-
access_rights = await self.get_project_access_rights(
333-
user_id=user_id, project_id=ProjectID(parent)
334-
)
335-
if not access_rights:
336-
_logger.warning(
337-
"File %s references a project that does not exists in db",
338-
file_id,
339-
)
340-
return AccessRights.none()
346+
return access_rights
341347

342-
except (ValueError, AttributeError) as err:
343-
raise InvalidFileIdentifierError(
344-
identifier=file_id,
345-
details=str(err),
346-
) from err
348+
except (ValueError, AttributeError) as err:
349+
raise InvalidFileIdentifierError(
350+
identifier=file_id,
351+
details=str(err),
352+
) from err
347353

348-
return access_rights
354+
async def get_file_access_rights(
355+
self,
356+
*,
357+
connection: AsyncConnection | None = None,
358+
user_id: UserID,
359+
file_id: StorageFileID,
360+
) -> AccessRights:
361+
"""
362+
Returns access-rights of user (user_id) over data file resource (file_id)
363+
364+
Raises InvalidFileIdentifierError
365+
"""
366+
access_rights = await self._get_access_from_metadata_entry(
367+
connection=connection, user_id=user_id, file_id=file_id
368+
)
369+
370+
if access_rights is not None:
371+
return access_rights
372+
373+
return await self._get_access_without_metardata_entry(
374+
user_id=user_id, file_id=file_id
375+
)
349376

350377
async def get_readable_project_ids(
351378
self, *, connection: AsyncConnection | None = None, user_id: UserID

services/storage/tests/unit/test_async_jobs.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
# pylint: disable=unused-argument
33

44
from dataclasses import dataclass
5+
from datetime import timedelta
56
from typing import Any
67
from uuid import UUID
78

@@ -28,7 +29,7 @@
2829
from pytest_mock import MockerFixture
2930
from servicelib.rabbitmq import RabbitMQRPCClient
3031
from servicelib.rabbitmq.rpc_interfaces.async_jobs import async_jobs
31-
from simcore_service_storage.modules.celery._task import _wrap_exception
32+
from simcore_service_storage.modules.celery._task import _error_handling
3233
from simcore_service_storage.modules.celery.client import TaskUUID
3334
from simcore_service_storage.modules.celery.models import TaskState, TaskStatus
3435

@@ -69,7 +70,9 @@ async def get_task_result(self, *args, **kwargs) -> Any:
6970
_ = kwargs
7071
assert self.get_task_result_object is not None
7172
if isinstance(self.get_task_result_object, Exception):
72-
return _wrap_exception(self.get_task_result_object)
73+
return _error_handling(1, timedelta(seconds=1), tuple())(
74+
self.get_task_result_object
75+
)
7376
return self.get_task_result_object
7477

7578
async def get_task_uuids(self, *args, **kwargs) -> set[TaskUUID]:

0 commit comments

Comments
 (0)