Skip to content

Commit bf8f922

Browse files
committed
fixes get tag
1 parent e6ae619 commit bf8f922

File tree

4 files changed

+52
-30
lines changed

4 files changed

+52
-30
lines changed

packages/postgres-database/src/simcore_postgres_database/utils_tags.py

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
from .utils_repos import pass_or_acquire_connection, transaction_context
99
from .utils_tags_sql import (
10-
count_users_with_given_access_rights_stmt,
10+
count_groups_with_given_access_rights_stmt,
1111
create_tag_stmt,
1212
delete_tag_stmt,
1313
get_tag_stmt,
@@ -67,7 +67,7 @@ async def access_count(
6767
Returns >0 if it does and represents the number of groups granting this access to the user
6868
"""
6969
async with pass_or_acquire_connection(self.engine, connection) as conn:
70-
count_stmt = count_users_with_given_access_rights_stmt(
70+
count_stmt = count_groups_with_given_access_rights_stmt(
7171
user_id=user_id, tag_id=tag_id, read=read, write=write, delete=delete
7272
)
7373
permissions_count: int | None = await conn.scalar(count_stmt)
@@ -237,7 +237,7 @@ async def create_access_rights(
237237
write: bool,
238238
delete: bool,
239239
):
240-
...
240+
raise NotImplementedError
241241

242242
async def update_access_rights(
243243
self,
@@ -250,9 +250,13 @@ async def update_access_rights(
250250
write: bool,
251251
delete: bool,
252252
):
253-
...
253+
raise NotImplementedError
254254

255255
async def delete_access_rights(
256-
self, connection: AsyncConnection | None = None, *, user_id: int, tag_id: int
256+
self,
257+
connection: AsyncConnection | None = None,
258+
*,
259+
user_id: int,
260+
tag_id: int,
257261
):
258-
...
262+
raise NotImplementedError

packages/postgres-database/src/simcore_postgres_database/utils_tags_sql.py

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -54,20 +54,30 @@ def get_tag_stmt(
5454
user_id: int,
5555
tag_id: int,
5656
):
57-
return sa.select(*_TAG_COLUMNS, *_ACCESS_RIGHTS_COLUMNS).select_from(
58-
_join_user_to_given_tag(
59-
access_condition=tags_access_rights.c.read.is_(True),
60-
tag_id=tag_id, # makes it unique result or None
61-
user_id=user_id,
57+
return (
58+
sa.select(
59+
*_TAG_COLUMNS,
60+
# aggregation ensures MOST PERMISSIVE policy of access-rights
61+
sa.func.bool_or(tags_access_rights.c.read).label("read"),
62+
sa.func.bool_or(tags_access_rights.c.write).label("write"),
63+
sa.func.bool_or(tags_access_rights.c.delete).label("delete")
64+
)
65+
.select_from(
66+
_join_user_to_given_tag(
67+
access_condition=tags_access_rights.c.read.is_(True),
68+
tag_id=tag_id,
69+
user_id=user_id,
70+
)
6271
)
72+
.group_by(tags.c.id)
6373
)
6474

6575

6676
def list_tags_stmt(*, user_id: int):
6777
return (
6878
sa.select(
6979
*_TAG_COLUMNS,
70-
# MOST permisive aggregation of access-rights
80+
# aggregation ensures MOST PERMISSIVE policy of access-rights
7181
sa.func.bool_or(tags_access_rights.c.read).label("read"),
7282
sa.func.bool_or(tags_access_rights.c.write).label("write"),
7383
sa.func.bool_or(tags_access_rights.c.delete).label("delete")
@@ -87,7 +97,7 @@ def create_tag_stmt(**values):
8797
return tags.insert().values(**values).returning(*_TAG_COLUMNS)
8898

8999

90-
def count_users_with_given_access_rights_stmt(
100+
def count_groups_with_given_access_rights_stmt(
91101
*,
92102
user_id: int,
93103
tag_id: int,
@@ -96,7 +106,7 @@ def count_users_with_given_access_rights_stmt(
96106
delete: bool | None
97107
):
98108
"""
99-
How many users are given EXACTLY these access permissions
109+
How many groups (from this user_id) are given EXACTLY these access permissions
100110
"""
101111
access = []
102112
if read is not None:

packages/postgres-database/tests/test_utils_tags.py

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -437,7 +437,7 @@ async def test_tags_repo_list_and_get(
437437
)
438438

439439

440-
async def test_tags_repo_uniquely_list_shared_tags(
440+
async def test_tags_repo_uniquely_list_or_get_shared_tags(
441441
asyncpg_engine: AsyncEngine,
442442
connection: SAConnection,
443443
user: RowProxy,
@@ -446,7 +446,7 @@ async def test_tags_repo_uniquely_list_shared_tags(
446446
conn = connection
447447
tags_repo = TagsRepo(asyncpg_engine)
448448

449-
# (setup): create a tag and share with group
449+
# (1) create a tag which cannot be written
450450
expected_tag_id = await create_tag(
451451
conn,
452452
name="T1",
@@ -457,6 +457,15 @@ async def test_tags_repo_uniquely_list_shared_tags(
457457
write=False, # <-- cannot write
458458
delete=True,
459459
)
460+
461+
got = await tags_repo.get(user_id=user.id, tag_id=expected_tag_id)
462+
assert got
463+
assert got["id"] == expected_tag_id
464+
assert got["read"] is True
465+
assert got["write"] is False # <--
466+
assert got["delete"] is True
467+
468+
# (2) share with standard group
460469
await create_tag_access(
461470
conn,
462471
tag_id=expected_tag_id,
@@ -466,18 +475,17 @@ async def test_tags_repo_uniquely_list_shared_tags(
466475
delete=False,
467476
)
468477

469-
# (check) can read
478+
# checks that the agregattion is the MOST permisive
479+
# checks that user_id has now full access via its primary and its stadard group
470480
got = await tags_repo.get(user_id=user.id, tag_id=expected_tag_id)
471481
assert got
472-
assert got["write"] is False
482+
assert got["id"] == expected_tag_id
483+
assert got["read"] is True
484+
assert got["write"] is True # <--
485+
assert got["delete"] is True
473486

474487
user_tags = await tags_repo.list_all(user_id=user.id)
475-
assert len(user_tags) == 1
476-
# checks that the agregattion is the MOST permisive
477-
assert user_tags[0]["id"] == expected_tag_id
478-
assert user_tags[0]["read"] is True
479-
assert user_tags[0]["write"] is True
480-
assert user_tags[0]["delete"] is True
488+
assert user_tags == [got]
481489

482490

483491
async def test_tags_repo_update(

services/web/server/src/simcore_service_webserver/tags/_handlers.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ async def wrapper(request: web.Request) -> web.StreamResponse:
3939
raise web.HTTPNotFound(reason=f"{exc}") from exc
4040

4141
except TagOperationNotAllowedError as exc:
42-
raise web.HTTPUnauthorized(reason=f"{exc}") from exc
42+
raise web.HTTPForbidden(reason=f"{exc}") from exc
4343

4444
return wrapper
4545

@@ -110,6 +110,11 @@ async def delete_tag(request: web.Request):
110110
raise web.HTTPNoContent(content_type=MIMETYPE_APPLICATION_JSON)
111111

112112

113+
#
114+
# tags ACCESS RIGHTS is exposed as a sub-resource groups
115+
#
116+
117+
113118
@routes.get(f"/{VTAG}/tags/{{tag_id}}/groups", name="list_tag_groups")
114119
@login_required
115120
@permission_required("tag.crud.*")
@@ -123,11 +128,6 @@ async def list_tag_groups(request: web.Request):
123128
raise NotImplementedError
124129

125130

126-
#
127-
# tags ACCESS RIGHTS is exposed as a sub-resource groups
128-
#
129-
130-
131131
@routes.post(f"/{VTAG}/tags/{{tag_id}}/groups/{{group_id}}", name="create_tag_group")
132132
@login_required
133133
@permission_required("tag.crud.*")

0 commit comments

Comments
 (0)