From 181454ddc5f6ebc81f0bb0bb11a75b794a7e9497 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 3 Sep 2025 10:54:29 +0200 Subject: [PATCH 01/43] more group examples --- .../api_schemas_webserver/groups.py | 18 +++++ .../src/models_library/groups.py | 73 ++++++++++--------- packages/models-library/tests/test_users.py | 24 +++++- 3 files changed, 76 insertions(+), 39 deletions(-) diff --git a/packages/models-library/src/models_library/api_schemas_webserver/groups.py b/packages/models-library/src/models_library/api_schemas_webserver/groups.py index 643c66b817a7..092dfe85c641 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/groups.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/groups.py @@ -187,6 +187,12 @@ class MyGroupsGet(OutputSchema): organizations: list[GroupGet] | None = None all: GroupGet product: GroupGet | None = None + support: Annotated[ + GroupGet | None, + Field( + description="Group ID of the app support team or None if no support is defined for this product" + ), + ] = None model_config = ConfigDict( json_schema_extra={ @@ -225,6 +231,12 @@ class MyGroupsGet(OutputSchema): "description": "Open to all users", "accessRights": {"read": True, "write": False, "delete": False}, }, + "support": { + "gid": "2", + "label": "Support Team", + "description": "The support team of the application", + "accessRights": {"read": False, "write": False, "delete": False}, + }, } } ) @@ -234,6 +246,7 @@ def from_domain_model( cls, groups_by_type: GroupsByTypeTuple, my_product_group: tuple[Group, AccessRightsDict] | None, + my_support_group: tuple[Group, AccessRightsDict] | None = None, ) -> Self: assert groups_by_type.primary # nosec assert groups_by_type.everyone # nosec @@ -249,6 +262,11 @@ def from_domain_model( if my_product_group else None ), + support=( + GroupGet.from_domain_model(*my_support_group) + if my_support_group + else None + ), ) diff --git a/packages/models-library/src/models_library/groups.py b/packages/models-library/src/models_library/groups.py index d35b1de7dcca..7e13237b78c4 100644 --- a/packages/models-library/src/models_library/groups.py +++ b/packages/models-library/src/models_library/groups.py @@ -39,41 +39,44 @@ class Group(BaseModel): @staticmethod def _update_json_schema_extra(schema: JsonDict) -> None: - schema.update( - { - "examples": [ - { - "gid": 1, - "name": "Everyone", - "type": "everyone", - "description": "all users", - "thumbnail": None, - }, - { - "gid": 2, - "name": "User", - "description": "primary group", - "type": "primary", - "thumbnail": None, - }, - { - "gid": 3, - "name": "Organization", - "description": "standard group", - "type": "standard", - "thumbnail": None, - "inclusionRules": {}, - }, - { - "gid": 4, - "name": "Product", - "description": "standard group for products", - "type": "standard", - "thumbnail": None, - }, - ] - } - ) + everyone = { + "gid": 1, + "name": "Everyone", + "type": "everyone", + "description": "all users", + "thumbnail": None, + } + user = { + "gid": 2, + "name": "User", + "description": "primary group", + "type": "primary", + "thumbnail": None, + } + organization = { + "gid": 3, + "name": "Organization", + "description": "standard group", + "type": "standard", + "thumbnail": None, + "inclusionRules": {}, + } + product = { + "gid": 4, + "name": "Product", + "description": "standard group for products", + "type": "standard", + "thumbnail": None, + } + support = { + "gid": 5, + "name": "Support", + "description": "support group", + "type": "standard", + "thumbnail": None, + } + + schema.update({"examples": [everyone, user, organization, product, support]}) model_config = ConfigDict( populate_by_name=True, json_schema_extra=_update_json_schema_extra diff --git a/packages/models-library/tests/test_users.py b/packages/models-library/tests/test_users.py index 47678a2ad7ce..598e9074c6a7 100644 --- a/packages/models-library/tests/test_users.py +++ b/packages/models-library/tests/test_users.py @@ -1,3 +1,4 @@ +import pytest from models_library.api_schemas_webserver.users import ( MyProfileRestGet, ) @@ -7,7 +8,11 @@ from pydantic import TypeAdapter -def test_adapter_from_model_to_schema(): +@pytest.mark.parametrize("with_support_group", [True, False]) +@pytest.mark.parametrize("with_standard_groups", [True, False]) +def test_adapter_from_model_to_schema( + with_support_group: bool, with_standard_groups: bool +): my_profile = MyProfile.model_validate(MyProfile.model_json_schema()["example"]) groups = TypeAdapter(list[Group]).validate_python( @@ -17,13 +22,24 @@ def test_adapter_from_model_to_schema(): ar = AccessRightsDict(read=False, write=False, delete=False) my_groups_by_type = GroupsByTypeTuple( - primary=(groups[1], ar), standard=[(groups[2], ar)], everyone=(groups[0], ar) + primary=(groups[1], ar), + standard=[(groups[2], ar)] if with_standard_groups else [], + everyone=(groups[0], ar), ) - my_product_group = groups[-1], AccessRightsDict( + my_product_group = groups[3], AccessRightsDict( read=False, write=False, delete=False ) + + my_support_group = groups[4], AccessRightsDict( + read=False, write=False, delete=False + ) + my_preferences = {"foo": Preference(default_value=3, value=1)} MyProfileRestGet.from_domain_model( - my_profile, my_groups_by_type, my_product_group, my_preferences + my_profile, + my_groups_by_type, + my_product_group, + my_preferences, + my_support_group if with_support_group else None, ) From 87170f20318fa6a9f511083c46c1fa761efe78e9 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 3 Sep 2025 11:37:06 +0200 Subject: [PATCH 02/43] adds support to groups --- .../api_schemas_webserver/groups.py | 2 +- .../api_schemas_webserver/users.py | 5 +++- .../src/models_library/groups.py | 22 ++++++++++---- .../groups/_groups_repository.py | 30 +++++++++---------- .../groups/_groups_rest.py | 26 ++++++++-------- .../groups/_groups_service.py | 21 +++++++++++-- .../simcore_service_webserver/groups/api.py | 4 ++- .../users/_controller/rest/users_rest.py | 16 ++++++++-- .../with_dbs/03/test_users_rest_profiles.py | 5 ++++ 9 files changed, 89 insertions(+), 42 deletions(-) diff --git a/packages/models-library/src/models_library/api_schemas_webserver/groups.py b/packages/models-library/src/models_library/api_schemas_webserver/groups.py index 092dfe85c641..505d06e7fb40 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/groups.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/groups.py @@ -246,7 +246,7 @@ def from_domain_model( cls, groups_by_type: GroupsByTypeTuple, my_product_group: tuple[Group, AccessRightsDict] | None, - my_support_group: tuple[Group, AccessRightsDict] | None = None, + my_support_group: tuple[Group, AccessRightsDict] | None, ) -> Self: assert groups_by_type.primary # nosec assert groups_by_type.everyone # nosec diff --git a/packages/models-library/src/models_library/api_schemas_webserver/users.py b/packages/models-library/src/models_library/api_schemas_webserver/users.py index 7b07616b73f2..f9ff1821fc7f 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/users.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/users.py @@ -132,6 +132,7 @@ def from_domain_model( my_groups_by_type: GroupsByTypeTuple, my_product_group: tuple[Group, AccessRightsDict] | None, my_preferences: AggregatedPreferences, + my_support_group: tuple[Group, AccessRightsDict] | None, ) -> Self: data = remap_keys( my_profile.model_dump( @@ -152,7 +153,9 @@ def from_domain_model( ) return cls( **data, - groups=MyGroupsGet.from_domain_model(my_groups_by_type, my_product_group), + groups=MyGroupsGet.from_domain_model( + my_groups_by_type, my_product_group, my_support_group + ), preferences=my_preferences, ) diff --git a/packages/models-library/src/models_library/groups.py b/packages/models-library/src/models_library/groups.py index 7e13237b78c4..cbd35847b549 100644 --- a/packages/models-library/src/models_library/groups.py +++ b/packages/models-library/src/models_library/groups.py @@ -39,21 +39,21 @@ class Group(BaseModel): @staticmethod def _update_json_schema_extra(schema: JsonDict) -> None: - everyone = { + everyone: JsonDict = { "gid": 1, "name": "Everyone", "type": "everyone", "description": "all users", "thumbnail": None, } - user = { + user: JsonDict = { "gid": 2, "name": "User", "description": "primary group", "type": "primary", "thumbnail": None, } - organization = { + organization: JsonDict = { "gid": 3, "name": "Organization", "description": "standard group", @@ -61,14 +61,14 @@ def _update_json_schema_extra(schema: JsonDict) -> None: "thumbnail": None, "inclusionRules": {}, } - product = { + product: JsonDict = { "gid": 4, "name": "Product", "description": "standard group for products", "type": "standard", "thumbnail": None, } - support = { + support: JsonDict = { "gid": 5, "name": "Support", "description": "support group", @@ -76,7 +76,17 @@ def _update_json_schema_extra(schema: JsonDict) -> None: "thumbnail": None, } - schema.update({"examples": [everyone, user, organization, product, support]}) + schema.update( + { + "examples": [ + everyone, + user, + organization, + product, + support, + ] + } + ) model_config = ConfigDict( populate_by_name=True, json_schema_extra=_update_json_schema_extra diff --git a/services/web/server/src/simcore_service_webserver/groups/_groups_repository.py b/services/web/server/src/simcore_service_webserver/groups/_groups_repository.py index 05bed6c18d68..eecb008b4556 100644 --- a/services/web/server/src/simcore_service_webserver/groups/_groups_repository.py +++ b/services/web/server/src/simcore_service_webserver/groups/_groups_repository.py @@ -108,7 +108,7 @@ async def _get_group_and_access_rights_or_raise( *, caller_id: UserID, group_id: GroupID, - permission: Literal["read", "write", "delete"] | None, + check_permission: Literal["read", "write", "delete"] | None, ) -> Row: result = await conn.execute( sa.select( @@ -122,8 +122,8 @@ async def _get_group_and_access_rights_or_raise( if not row: raise GroupNotFoundError(gid=group_id) - if permission: - _check_group_permissions(row, caller_id, group_id, permission) + if check_permission: + _check_group_permissions(row, caller_id, group_id, check_permission) return row @@ -274,29 +274,29 @@ async def get_user_group( """ async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: row = await _get_group_and_access_rights_or_raise( - conn, caller_id=user_id, group_id=group_id, permission="read" + conn, caller_id=user_id, group_id=group_id, check_permission="read" ) group, access_rights = _to_group_info_tuple(row) return group, access_rights -async def get_product_group_for_user( +async def get_any_group_for_user( app: web.Application, connection: AsyncConnection | None = None, *, user_id: UserID, - product_gid: GroupID, + group_gid: GroupID, ) -> tuple[Group, AccessRightsDict]: """ - Returns product's group if user belongs to it, otherwise it + Returns any group if user belongs to it (even if it has no permissions), otherwise it raises GroupNotFoundError """ async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: row = await _get_group_and_access_rights_or_raise( conn, caller_id=user_id, - group_id=product_gid, - permission=None, + group_id=group_gid, + check_permission=None, ) group, access_rights = _to_group_info_tuple(row) return group, access_rights @@ -363,7 +363,7 @@ async def update_standard_group( async with transaction_context(get_asyncpg_engine(app), connection) as conn: row = await _get_group_and_access_rights_or_raise( - conn, caller_id=user_id, group_id=group_id, permission="write" + conn, caller_id=user_id, group_id=group_id, check_permission="write" ) assert row.gid == group_id # nosec # NOTE: update does not include access-rights @@ -391,7 +391,7 @@ async def delete_standard_group( ) -> None: async with transaction_context(get_asyncpg_engine(app), connection) as conn: await _get_group_and_access_rights_or_raise( - conn, caller_id=user_id, group_id=group_id, permission="delete" + conn, caller_id=user_id, group_id=group_id, check_permission="delete" ) await conn.execute( @@ -581,7 +581,7 @@ async def get_user_in_group( async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: # first check if the group exists await _get_group_and_access_rights_or_raise( - conn, caller_id=caller_id, group_id=group_id, permission="read" + conn, caller_id=caller_id, group_id=group_id, check_permission="read" ) # get the user with its permissions @@ -611,7 +611,7 @@ async def update_user_in_group( # first check if the group exists await _get_group_and_access_rights_or_raise( - conn, caller_id=caller_id, group_id=group_id, permission="write" + conn, caller_id=caller_id, group_id=group_id, check_permission="write" ) # now check the user exists @@ -651,7 +651,7 @@ async def delete_user_from_group( async with transaction_context(get_asyncpg_engine(app), connection) as conn: # first check if the group exists await _get_group_and_access_rights_or_raise( - conn, caller_id=caller_id, group_id=group_id, permission="write" + conn, caller_id=caller_id, group_id=group_id, check_permission="write" ) # check the user exists @@ -714,7 +714,7 @@ async def add_new_user_in_group( async with transaction_context(get_asyncpg_engine(app), connection) as conn: # first check if the group exists await _get_group_and_access_rights_or_raise( - conn, caller_id=caller_id, group_id=group_id, permission="write" + conn, caller_id=caller_id, group_id=group_id, check_permission="write" ) query = sa.select(users.c.id) diff --git a/services/web/server/src/simcore_service_webserver/groups/_groups_rest.py b/services/web/server/src/simcore_service_webserver/groups/_groups_rest.py index 07a0b5026d48..806fa60b0b64 100644 --- a/services/web/server/src/simcore_service_webserver/groups/_groups_rest.py +++ b/services/web/server/src/simcore_service_webserver/groups/_groups_rest.py @@ -56,8 +56,6 @@ async def list_groups(request: web.Request): assert groups_by_type.primary assert groups_by_type.everyone - my_product_group = None - if product.group_id: with suppress(GroupNotFoundError): # Product is optional @@ -66,16 +64,20 @@ async def list_groups(request: web.Request): user_id=req_ctx.user_id, product_gid=product.group_id, ) - - my_groups = MyGroupsGet( - me=GroupGet.from_domain_model(*groups_by_type.primary), - organizations=[ - GroupGet.from_domain_model(*gi) for gi in groups_by_type.standard - ], - all=GroupGet.from_domain_model(*groups_by_type.everyone), - product=( - GroupGet.from_domain_model(*my_product_group) if my_product_group else None - ), + else: + my_product_group = None + + if product.support_standard_group_id: + my_support_group = await _groups_service.get_support_group_for_user_or_none( + app=request.app, + user_id=req_ctx.user_id, + support_gid=product.support_standard_group_id, + ) + else: + my_support_group = None + + my_groups = MyGroupsGet.from_domain_model( + groups_by_type, my_product_group, my_support_group ) return envelope_json_response(my_groups) diff --git a/services/web/server/src/simcore_service_webserver/groups/_groups_service.py b/services/web/server/src/simcore_service_webserver/groups/_groups_service.py index 67ff9d1dfafa..c28b7d9d2e34 100644 --- a/services/web/server/src/simcore_service_webserver/groups/_groups_service.py +++ b/services/web/server/src/simcore_service_webserver/groups/_groups_service.py @@ -1,3 +1,5 @@ +from contextlib import suppress + from aiohttp import web from models_library.basic_types import IDStr from models_library.emails import LowerCaseEmailStr @@ -16,7 +18,7 @@ from ..users import users_service from . import _groups_repository -from .exceptions import GroupsError +from .exceptions import GroupNotFoundError, GroupsError # # GROUPS @@ -71,11 +73,24 @@ async def get_product_group_for_user( Returns product's group if user belongs to it, otherwise it raises GroupNotFoundError """ - return await _groups_repository.get_product_group_for_user( - app, user_id=user_id, product_gid=product_gid + return await _groups_repository.get_any_group_for_user( + app, user_id=user_id, group_gid=product_gid ) +async def get_support_group_for_user_or_none( + app: web.Application, *, user_id: UserID, support_gid: GroupID +) -> tuple[Group, AccessRightsDict] | None: + """ + Returns support's group if user belongs to it, otherwise it + """ + with suppress(GroupNotFoundError): + return await _groups_repository.get_any_group_for_user( + app, user_id=user_id, group_gid=support_gid + ) + return None + + # # CRUD operations on groups linked to a user # diff --git a/services/web/server/src/simcore_service_webserver/groups/api.py b/services/web/server/src/simcore_service_webserver/groups/api.py index dfe60c5866f9..68f14c239baf 100644 --- a/services/web/server/src/simcore_service_webserver/groups/api.py +++ b/services/web/server/src/simcore_service_webserver/groups/api.py @@ -7,6 +7,7 @@ auto_add_user_to_product_group, get_group_from_gid, get_product_group_for_user, + get_support_group_for_user_or_none, is_user_by_email_in_group, list_all_user_groups_ids, list_group_members, @@ -20,10 +21,11 @@ "auto_add_user_to_product_group", "get_group_from_gid", "get_product_group_for_user", + "get_support_group_for_user_or_none", "is_user_by_email_in_group", "list_all_user_groups_ids", + "list_group_members", "list_user_groups_ids_with_read_access", "list_user_groups_with_read_access", - "list_group_members", # nopycln: file ) diff --git a/services/web/server/src/simcore_service_webserver/users/_controller/rest/users_rest.py b/services/web/server/src/simcore_service_webserver/users/_controller/rest/users_rest.py index c242fb5e1ebb..0fd831d20500 100644 --- a/services/web/server/src/simcore_service_webserver/users/_controller/rest/users_rest.py +++ b/services/web/server/src/simcore_service_webserver/users/_controller/rest/users_rest.py @@ -58,8 +58,6 @@ async def get_my_profile(request: web.Request) -> web.Response: assert groups_by_type.primary assert groups_by_type.everyone - my_product_group = None - if product.group_id: with suppress(GroupNotFoundError): # Product is optional @@ -68,13 +66,25 @@ async def get_my_profile(request: web.Request) -> web.Response: user_id=req_ctx.user_id, product_gid=product.group_id, ) + else: + my_product_group = None + + if product.support_standard_group_id: + # Support group is optional + my_support_group = await groups_service.get_support_group_for_user_or_none( + app=request.app, + user_id=req_ctx.user_id, + support_gid=product.support_standard_group_id, + ) + else: + my_support_group = None my_profile, preferences = await _users_service.get_my_profile( request.app, user_id=req_ctx.user_id, product_name=req_ctx.product_name ) profile = MyProfileRestGet.from_domain_model( - my_profile, groups_by_type, my_product_group, preferences + my_profile, groups_by_type, my_product_group, preferences, my_support_group ) return envelope_json_response(profile) diff --git a/services/web/server/tests/unit/with_dbs/03/test_users_rest_profiles.py b/services/web/server/tests/unit/with_dbs/03/test_users_rest_profiles.py index ab7c0fb6f3a5..8bc3bd716ec5 100644 --- a/services/web/server/tests/unit/with_dbs/03/test_users_rest_profiles.py +++ b/services/web/server/tests/unit/with_dbs/03/test_users_rest_profiles.py @@ -88,6 +88,7 @@ async def private_user( "first_name": partial_first_name, "last_name": "Bond", "email": f"james{partial_email}", + # Maximum privacy "privacy_hide_username": True, "privacy_hide_email": True, "privacy_hide_fullname": True, @@ -108,6 +109,7 @@ async def semi_private_user( "first_name": partial_first_name, "last_name": "Maxwell", "email": "j@maxwell.me", + # Medium privacy "privacy_hide_username": False, "privacy_hide_email": True, "privacy_hide_fullname": False, # <-- @@ -128,6 +130,7 @@ async def public_user( "first_name": "Taylor", "last_name": "Swift", "email": f"taylor{partial_email}", + # Fully public "privacy_hide_username": False, "privacy_hide_email": False, "privacy_hide_fullname": False, @@ -407,6 +410,8 @@ async def test_get_profile( "thumbnail": None, } + assert got_profile_groups["support"] is None + sorted_by_group_id = functools.partial(sorted, key=lambda d: d["gid"]) assert sorted_by_group_id( got_profile_groups["organizations"] From 37a40a16cb0885084df34f9dd311df3642b851bc Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 3 Sep 2025 11:38:27 +0200 Subject: [PATCH 03/43] updates OAS --- .../simcore_service_webserver/api/v0/openapi.yaml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml index c4bef8dbf213..c64564647d3b 100644 --- a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml +++ b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml @@ -13605,6 +13605,12 @@ components: anyOf: - $ref: '#/components/schemas/GroupGet' - type: 'null' + support: + anyOf: + - $ref: '#/components/schemas/GroupGet' + - type: 'null' + description: Group ID of the app support team or None if no support is defined + for this product type: object required: - me @@ -13643,6 +13649,14 @@ components: description: Some foundation gid: '16' label: Blue Fundation + support: + accessRights: + delete: false + read: false + write: false + description: The support team of the application + gid: '2' + label: Support Team MyPermissionGet: properties: name: From 0dda812c2d1adac4e0b2d3889ff5ccda703a1bc9 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 3 Sep 2025 11:38:29 +0200 Subject: [PATCH 04/43] =?UTF-8?q?services/webserver=20api=20version:=200.7?= =?UTF-8?q?5.0=20=E2=86=92=200.76.0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- services/web/server/VERSION | 2 +- services/web/server/setup.cfg | 2 +- .../server/src/simcore_service_webserver/api/v0/openapi.yaml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/services/web/server/VERSION b/services/web/server/VERSION index c52842c467cf..62df9f538d84 100644 --- a/services/web/server/VERSION +++ b/services/web/server/VERSION @@ -1 +1 @@ -0.75.0 +0.76.0 diff --git a/services/web/server/setup.cfg b/services/web/server/setup.cfg index 22e2a9f3d1ea..e1bef4a8a5a1 100644 --- a/services/web/server/setup.cfg +++ b/services/web/server/setup.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 0.75.0 +current_version = 0.76.0 commit = True message = services/webserver api version: {current_version} → {new_version} tag = False diff --git a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml index c64564647d3b..963b5b65693f 100644 --- a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml +++ b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml @@ -2,7 +2,7 @@ openapi: 3.1.0 info: title: simcore-service-webserver description: Main service with an interface (http-API & websockets) to the web front-end - version: 0.75.0 + version: 0.76.0 servers: - url: '' description: webserver From b77b23be6deeecad4de58f76511d818e8d025053 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 3 Sep 2025 16:51:02 +0200 Subject: [PATCH 05/43] splits tests --- .../src/pytest_simcore/faker_products_data.py | 24 +- .../pytest_simcore/helpers/faker_factories.py | 2 + .../unit/with_dbs/03/test_users_rest_phone.py | 494 ++++++++++++++++++ ...profiles.py => test_users_rest_profile.py} | 459 ---------------- 4 files changed, 515 insertions(+), 464 deletions(-) create mode 100644 services/web/server/tests/unit/with_dbs/03/test_users_rest_phone.py rename services/web/server/tests/unit/with_dbs/03/{test_users_rest_profiles.py => test_users_rest_profile.py} (59%) diff --git a/packages/pytest-simcore/src/pytest_simcore/faker_products_data.py b/packages/pytest-simcore/src/pytest_simcore/faker_products_data.py index e55c1e489f09..e91f21ca7e6b 100644 --- a/packages/pytest-simcore/src/pytest_simcore/faker_products_data.py +++ b/packages/pytest-simcore/src/pytest_simcore/faker_products_data.py @@ -4,9 +4,9 @@ # pylint: disable=unused-argument # pylint: disable=unused-variable """ - Fixtures to produce fake data for a product: - - it is self-consistent - - granular customization by overriding fixtures +Fixtures to produce fake data for a product: + - it is self-consistent + - granular customization by overriding fixtures """ from typing import Any @@ -65,11 +65,25 @@ def bcc_email(request: pytest.FixtureRequest, product_name: ProductName) -> Emai ) +@pytest.fixture +def support_standard_group_id(faker: Faker) -> int | None: + # NOTE: override to change + return None + + @pytest.fixture def product( - faker: Faker, product_name: ProductName, support_email: EmailStr + faker: Faker, + product_name: ProductName, + support_email: EmailStr, + support_standard_group_id: int | None, ) -> dict[str, Any]: - return random_product(name=product_name, support_email=support_email, fake=faker) + return random_product( + name=product_name, + support_email=support_email, + support_standard_group_id=support_standard_group_id, + fake=faker, + ) @pytest.fixture diff --git a/packages/pytest-simcore/src/pytest_simcore/helpers/faker_factories.py b/packages/pytest-simcore/src/pytest_simcore/helpers/faker_factories.py index e2ff83fc53a3..7c50df70de5c 100644 --- a/packages/pytest-simcore/src/pytest_simcore/helpers/faker_factories.py +++ b/packages/pytest-simcore/src/pytest_simcore/helpers/faker_factories.py @@ -256,6 +256,7 @@ def fake_task(**overrides) -> dict[str, Any]: def random_product( *, group_id: int | None = None, + support_standard_group_id: int | None = None, registration_email_template: str | None = None, fake: Faker = DEFAULT_FAKER, **overrides, @@ -302,6 +303,7 @@ def random_product( "priority": fake.pyint(0, 10), "max_open_studies_per_user": fake.pyint(1, 10), "group_id": group_id, + "support_standard_group_id": support_standard_group_id, } if ui := fake.random_element( diff --git a/services/web/server/tests/unit/with_dbs/03/test_users_rest_phone.py b/services/web/server/tests/unit/with_dbs/03/test_users_rest_phone.py new file mode 100644 index 000000000000..c93af68cd925 --- /dev/null +++ b/services/web/server/tests/unit/with_dbs/03/test_users_rest_phone.py @@ -0,0 +1,494 @@ +# pylint: disable=protected-access +# pylint: disable=redefined-outer-name +# pylint: disable=too-many-arguments +# pylint: disable=too-many-statements +# pylint: disable=unused-argument +# pylint: disable=unused-variable + + +from http import HTTPStatus + +import pytest +from aiohttp.test_utils import TestClient +from common_library.users_enums import UserRole +from models_library.api_schemas_webserver.users import ( + MyProfileRestGet, +) +from pytest_simcore.helpers.assert_checks import assert_status +from pytest_simcore.helpers.monkeypatch_envs import EnvVarsDict, setenvs_from_dict +from pytest_simcore.helpers.webserver_users import UserInfoDict +from servicelib.aiohttp import status +from simcore_service_webserver.models import PhoneNumberStr +from simcore_service_webserver.users._controller.rest.users_rest import ( + _REGISTRATION_CODE_VALUE_FAKE, +) + + +@pytest.fixture +def app_environment( + app_environment: EnvVarsDict, monkeypatch: pytest.MonkeyPatch +) -> EnvVarsDict: + # disables GC and DB-listener + return app_environment | setenvs_from_dict( + monkeypatch, + { + "WEBSERVER_GARBAGE_COLLECTOR": "null", + "WEBSERVER_DB_LISTENER": "0", + "WEBSERVER_DEV_FEATURES_ENABLED": "1", # NOTE: still under development + }, + ) + + +@pytest.mark.parametrize("user_role", [UserRole.USER]) +async def test_phone_registration_basic_workflow( + user_role: UserRole, + logged_user: UserInfoDict, + client: TestClient, +): + assert client.app + + # GET initial profile + url = client.app.router["get_my_profile"].url_for() + resp = await client.get(f"{url}") + data, _ = await assert_status(resp, status.HTTP_200_OK) + + initial_profile = MyProfileRestGet.model_validate(data) + initial_phone = initial_profile.phone + assert initial_phone + + # REGISTER phone number + # Change the last 3 digits of the initial phone number to '999' + new_phone = f"{initial_phone[:-3]}999" + url = client.app.router["my_phone_register"].url_for() + resp = await client.post( + f"{url}", + json={ + "phone": new_phone, + }, + ) + await assert_status(resp, status.HTTP_202_ACCEPTED) + + # CONFIRM phone registration + url = client.app.router["my_phone_confirm"].url_for() + resp = await client.post( + f"{url}", + json={ + "code": _REGISTRATION_CODE_VALUE_FAKE, + }, + ) + await assert_status(resp, status.HTTP_204_NO_CONTENT) + + # GET updated profile + url = client.app.router["get_my_profile"].url_for() + resp = await client.get(f"{url}") + data, _ = await assert_status(resp, status.HTTP_200_OK) + + updated_profile = MyProfileRestGet.model_validate(data) + + # Verify phone was updated + assert updated_profile.phone == new_phone + assert updated_profile.phone != initial_phone + + # Verify other fields remained unchanged + assert updated_profile.first_name == initial_profile.first_name + assert updated_profile.last_name == initial_profile.last_name + assert updated_profile.login == initial_profile.login + assert updated_profile.user_name == initial_profile.user_name + + +@pytest.mark.parametrize("user_role", [UserRole.USER]) +async def test_phone_registration_workflow( + user_role: UserRole, + logged_user: UserInfoDict, + client: TestClient, +): + assert client.app + + # GET initial profile + url = client.app.router["get_my_profile"].url_for() + resp = await client.get(f"{url}") + data, _ = await assert_status(resp, status.HTTP_200_OK) + + initial_profile = MyProfileRestGet.model_validate(data) + initial_phone = initial_profile.phone + assert initial_phone + + # STEP 1: REGISTER phone number + new_phone = f"{initial_phone[:-3]}999" # Change the last 3 digits to '999' + url = client.app.router["my_phone_register"].url_for() + resp = await client.post( + f"{url}", + json={ + "phone": new_phone, + }, + ) + await assert_status(resp, status.HTTP_202_ACCEPTED) + + # STEP 2: CONFIRM phone registration + url = client.app.router["my_phone_confirm"].url_for() + resp = await client.post( + f"{url}", + json={ + "code": _REGISTRATION_CODE_VALUE_FAKE, + }, + ) + await assert_status(resp, status.HTTP_204_NO_CONTENT) + + # GET updated profile + url = client.app.router["get_my_profile"].url_for() + resp = await client.get(f"{url}") + data, _ = await assert_status(resp, status.HTTP_200_OK) + + updated_profile = MyProfileRestGet.model_validate(data) + + # Verify phone was updated + assert updated_profile.phone == new_phone + assert updated_profile.phone != initial_phone + + # Verify other fields remained unchanged + assert updated_profile.first_name == initial_profile.first_name + assert updated_profile.last_name == initial_profile.last_name + assert updated_profile.login == initial_profile.login + assert updated_profile.user_name == initial_profile.user_name + + +@pytest.mark.parametrize("user_role", [UserRole.USER]) +async def test_phone_registration_with_resend( + user_role: UserRole, + logged_user: UserInfoDict, + client: TestClient, + user_phone_number: PhoneNumberStr, +): + assert client.app + + # STEP 1: REGISTER phone number + new_phone = user_phone_number + url = client.app.router["my_phone_register"].url_for() + resp = await client.post( + f"{url}", + json={ + "phone": new_phone, + }, + ) + await assert_status(resp, status.HTTP_202_ACCEPTED) + + # STEP 2: RESEND code (optional step) + url = client.app.router["my_phone_resend"].url_for() + resp = await client.post(f"{url}") + await assert_status(resp, status.HTTP_202_ACCEPTED) + + # STEP 3: CONFIRM phone registration + url = client.app.router["my_phone_confirm"].url_for() + resp = await client.post( + f"{url}", + json={ + "code": _REGISTRATION_CODE_VALUE_FAKE, + }, + ) + await assert_status(resp, status.HTTP_204_NO_CONTENT) + + # GET updated profile + url = client.app.router["get_my_profile"].url_for() + resp = await client.get(f"{url}") + data, _ = await assert_status(resp, status.HTTP_200_OK) + + updated_profile = MyProfileRestGet.model_validate(data) + + # Verify phone was updated + assert updated_profile.phone == new_phone + + +@pytest.mark.parametrize("user_role", [UserRole.USER]) +async def test_phone_registration_change_existing_phone( + user_role: UserRole, + logged_user: UserInfoDict, + client: TestClient, + user_phone_number: PhoneNumberStr, +): + assert client.app + + # Set initial phone + first_phone = user_phone_number + url = client.app.router["my_phone_register"].url_for() + resp = await client.post( + f"{url}", + json={ + "phone": first_phone, + }, + ) + await assert_status(resp, status.HTTP_202_ACCEPTED) + + url = client.app.router["my_phone_confirm"].url_for() + resp = await client.post( + f"{url}", + json={ + "code": _REGISTRATION_CODE_VALUE_FAKE, + }, + ) + await assert_status(resp, status.HTTP_204_NO_CONTENT) + + # Change to new phone + # Create a different phone number by changing the last digits + new_phone = user_phone_number[:-4] + "9999" + url = client.app.router["my_phone_register"].url_for() + resp = await client.post( + f"{url}", + json={ + "phone": new_phone, + }, + ) + await assert_status(resp, status.HTTP_202_ACCEPTED) + + url = client.app.router["my_phone_confirm"].url_for() + resp = await client.post( + f"{url}", + json={ + "code": _REGISTRATION_CODE_VALUE_FAKE, + }, + ) + await assert_status(resp, status.HTTP_204_NO_CONTENT) + + # GET updated profile + url = client.app.router["get_my_profile"].url_for() + resp = await client.get(f"{url}") + data, _ = await assert_status(resp, status.HTTP_200_OK) + + updated_profile = MyProfileRestGet.model_validate(data) + + # Verify phone was updated to new phone + assert updated_profile.phone == new_phone + assert updated_profile.phone != first_phone + + +# +# PHONE REGISTRATION FAILURE TESTS +# + + +@pytest.mark.parametrize("user_role", [UserRole.USER]) +async def test_phone_resend_without_pending_registration( + user_role: UserRole, + logged_user: UserInfoDict, + client: TestClient, +): + assert client.app + + # Try to resend code without any pending registration + url = client.app.router["my_phone_resend"].url_for() + resp = await client.post(f"{url}") + await assert_status(resp, status.HTTP_400_BAD_REQUEST) + + +@pytest.mark.parametrize("user_role", [UserRole.USER]) +async def test_phone_confirm_without_pending_registration( + user_role: UserRole, + logged_user: UserInfoDict, + client: TestClient, +): + assert client.app + + # Try to confirm code without any pending registration + url = client.app.router["my_phone_confirm"].url_for() + resp = await client.post( + f"{url}", + json={ + "code": _REGISTRATION_CODE_VALUE_FAKE, + }, + ) + await assert_status(resp, status.HTTP_400_BAD_REQUEST) + + +@pytest.mark.parametrize("user_role", [UserRole.USER]) +async def test_phone_confirm_with_wrong_code( + user_role: UserRole, + logged_user: UserInfoDict, + client: TestClient, + user_phone_number: PhoneNumberStr, +): + assert client.app + + # STEP 1: REGISTER phone number + new_phone = user_phone_number + url = client.app.router["my_phone_register"].url_for() + resp = await client.post( + f"{url}", + json={ + "phone": new_phone, + }, + ) + await assert_status(resp, status.HTTP_202_ACCEPTED) + + # STEP 2: Try to confirm with wrong code + url = client.app.router["my_phone_confirm"].url_for() + resp = await client.post( + f"{url}", + json={ + "code": "wrongcode1234", + }, + ) + await assert_status(resp, status.HTTP_400_BAD_REQUEST) + + +@pytest.mark.parametrize("user_role", [UserRole.USER]) +async def test_phone_confirm_with_invalid_code_format( + user_role: UserRole, + logged_user: UserInfoDict, + client: TestClient, + user_phone_number: PhoneNumberStr, +): + assert client.app + + # STEP 1: REGISTER phone number + new_phone = user_phone_number + url = client.app.router["my_phone_register"].url_for() + resp = await client.post( + f"{url}", + json={ + "phone": new_phone, + }, + ) + await assert_status(resp, status.HTTP_202_ACCEPTED) + + # STEP 2: Try to confirm with invalid code format (contains special characters) + url = client.app.router["my_phone_confirm"].url_for() + resp = await client.post( + f"{url}", + json={ + "code": "123-456", # Invalid format according to pattern + }, + ) + await assert_status(resp, status.HTTP_422_UNPROCESSABLE_ENTITY) + + +@pytest.mark.parametrize("user_role", [UserRole.USER]) +async def test_phone_register_with_empty_phone( + user_role: UserRole, + logged_user: UserInfoDict, + client: TestClient, +): + assert client.app + + # Try to register with empty phone number + url = client.app.router["my_phone_register"].url_for() + resp = await client.post( + f"{url}", + json={ + "phone": "", # Empty phone number + }, + ) + await assert_status(resp, status.HTTP_422_UNPROCESSABLE_ENTITY) + + # Try to register with whitespace-only phone number + url = client.app.router["my_phone_register"].url_for() + resp = await client.post( + f"{url}", + json={ + "phone": " ", # Whitespace only + }, + ) + await assert_status(resp, status.HTTP_422_UNPROCESSABLE_ENTITY) + + +@pytest.mark.parametrize("user_role", [UserRole.USER]) +async def test_phone_confirm_with_empty_code( + user_role: UserRole, + logged_user: UserInfoDict, + client: TestClient, + user_phone_number: PhoneNumberStr, +): + assert client.app + + # STEP 1: REGISTER phone number + new_phone = user_phone_number + url = client.app.router["my_phone_register"].url_for() + resp = await client.post( + f"{url}", + json={ + "phone": new_phone, + }, + ) + await assert_status(resp, status.HTTP_202_ACCEPTED) + + # STEP 2: Try to confirm with empty code + url = client.app.router["my_phone_confirm"].url_for() + resp = await client.post( + f"{url}", + json={ + "code": "", # Empty code + }, + ) + await assert_status(resp, status.HTTP_422_UNPROCESSABLE_ENTITY) + + +@pytest.mark.parametrize( + "user_role,expected", + [ + (UserRole.ANONYMOUS, status.HTTP_401_UNAUTHORIZED), + (UserRole.GUEST, status.HTTP_403_FORBIDDEN), + ], +) +async def test_phone_register_access_rights( + user_role: UserRole, + logged_user: UserInfoDict, + client: TestClient, + expected: HTTPStatus, + user_phone_number: PhoneNumberStr, +): + assert client.app + + # Try to register phone with insufficient permissions + url = client.app.router["my_phone_register"].url_for() + resp = await client.post( + f"{url}", + json={ + "phone": user_phone_number, + }, + ) + await assert_status(resp, expected) + + +@pytest.mark.parametrize( + "user_role,expected", + [ + (UserRole.ANONYMOUS, status.HTTP_401_UNAUTHORIZED), + (UserRole.GUEST, status.HTTP_403_FORBIDDEN), + ], +) +async def test_phone_resend_access_rights( + user_role: UserRole, + logged_user: UserInfoDict, + client: TestClient, + expected: HTTPStatus, +): + assert client.app + + # Try to resend code with insufficient permissions + url = client.app.router["my_phone_resend"].url_for() + resp = await client.post(f"{url}") + await assert_status(resp, expected) + + +@pytest.mark.parametrize( + "user_role,expected", + [ + (UserRole.ANONYMOUS, status.HTTP_401_UNAUTHORIZED), + (UserRole.GUEST, status.HTTP_403_FORBIDDEN), + ], +) +async def test_phone_confirm_access_rights( + user_role: UserRole, + logged_user: UserInfoDict, + client: TestClient, + expected: HTTPStatus, +): + assert client.app + + # Try to confirm code with insufficient permissions + url = client.app.router["my_phone_confirm"].url_for() + resp = await client.post( + f"{url}", + json={ + "code": _REGISTRATION_CODE_VALUE_FAKE, + }, + ) + await assert_status(resp, expected) diff --git a/services/web/server/tests/unit/with_dbs/03/test_users_rest_profiles.py b/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py similarity index 59% rename from services/web/server/tests/unit/with_dbs/03/test_users_rest_profiles.py rename to services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py index 8bc3bd716ec5..6bd3e708eb68 100644 --- a/services/web/server/tests/unit/with_dbs/03/test_users_rest_profiles.py +++ b/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py @@ -32,13 +32,9 @@ from pytest_simcore.helpers.webserver_users import NewUser, UserInfoDict from servicelib.aiohttp import status from servicelib.rest_constants import RESPONSE_MODEL_POLICY -from simcore_service_webserver.models import PhoneNumberStr from simcore_service_webserver.user_preferences._service import ( get_frontend_user_preferences_aggregation, ) -from simcore_service_webserver.users._controller.rest.users_rest import ( - _REGISTRATION_CODE_VALUE_FAKE, -) from sqlalchemy.exc import OperationalError as SQLAlchemyOperationalError from sqlalchemy.ext.asyncio import AsyncConnection @@ -600,458 +596,3 @@ async def test_get_profile_with_failing_db_connection( data, error = await assert_status(resp, expected) assert not data assert error["message"] == "Authentication service is temporary unavailable" - - -@pytest.mark.parametrize("user_role", [UserRole.USER]) -async def test_phone_registration_basic_workflow( - user_role: UserRole, - logged_user: UserInfoDict, - client: TestClient, -): - assert client.app - - # GET initial profile - url = client.app.router["get_my_profile"].url_for() - resp = await client.get(f"{url}") - data, _ = await assert_status(resp, status.HTTP_200_OK) - - initial_profile = MyProfileRestGet.model_validate(data) - initial_phone = initial_profile.phone - assert initial_phone - - # REGISTER phone number - # Change the last 3 digits of the initial phone number to '999' - new_phone = f"{initial_phone[:-3]}999" - url = client.app.router["my_phone_register"].url_for() - resp = await client.post( - f"{url}", - json={ - "phone": new_phone, - }, - ) - await assert_status(resp, status.HTTP_202_ACCEPTED) - - # CONFIRM phone registration - url = client.app.router["my_phone_confirm"].url_for() - resp = await client.post( - f"{url}", - json={ - "code": _REGISTRATION_CODE_VALUE_FAKE, - }, - ) - await assert_status(resp, status.HTTP_204_NO_CONTENT) - - # GET updated profile - url = client.app.router["get_my_profile"].url_for() - resp = await client.get(f"{url}") - data, _ = await assert_status(resp, status.HTTP_200_OK) - - updated_profile = MyProfileRestGet.model_validate(data) - - # Verify phone was updated - assert updated_profile.phone == new_phone - assert updated_profile.phone != initial_phone - - # Verify other fields remained unchanged - assert updated_profile.first_name == initial_profile.first_name - assert updated_profile.last_name == initial_profile.last_name - assert updated_profile.login == initial_profile.login - assert updated_profile.user_name == initial_profile.user_name - - -@pytest.mark.parametrize("user_role", [UserRole.USER]) -async def test_phone_registration_workflow( - user_role: UserRole, - logged_user: UserInfoDict, - client: TestClient, -): - assert client.app - - # GET initial profile - url = client.app.router["get_my_profile"].url_for() - resp = await client.get(f"{url}") - data, _ = await assert_status(resp, status.HTTP_200_OK) - - initial_profile = MyProfileRestGet.model_validate(data) - initial_phone = initial_profile.phone - assert initial_phone - - # STEP 1: REGISTER phone number - new_phone = f"{initial_phone[:-3]}999" # Change the last 3 digits to '999' - url = client.app.router["my_phone_register"].url_for() - resp = await client.post( - f"{url}", - json={ - "phone": new_phone, - }, - ) - await assert_status(resp, status.HTTP_202_ACCEPTED) - - # STEP 2: CONFIRM phone registration - url = client.app.router["my_phone_confirm"].url_for() - resp = await client.post( - f"{url}", - json={ - "code": _REGISTRATION_CODE_VALUE_FAKE, - }, - ) - await assert_status(resp, status.HTTP_204_NO_CONTENT) - - # GET updated profile - url = client.app.router["get_my_profile"].url_for() - resp = await client.get(f"{url}") - data, _ = await assert_status(resp, status.HTTP_200_OK) - - updated_profile = MyProfileRestGet.model_validate(data) - - # Verify phone was updated - assert updated_profile.phone == new_phone - assert updated_profile.phone != initial_phone - - # Verify other fields remained unchanged - assert updated_profile.first_name == initial_profile.first_name - assert updated_profile.last_name == initial_profile.last_name - assert updated_profile.login == initial_profile.login - assert updated_profile.user_name == initial_profile.user_name - - -@pytest.mark.parametrize("user_role", [UserRole.USER]) -async def test_phone_registration_with_resend( - user_role: UserRole, - logged_user: UserInfoDict, - client: TestClient, - user_phone_number: PhoneNumberStr, -): - assert client.app - - # STEP 1: REGISTER phone number - new_phone = user_phone_number - url = client.app.router["my_phone_register"].url_for() - resp = await client.post( - f"{url}", - json={ - "phone": new_phone, - }, - ) - await assert_status(resp, status.HTTP_202_ACCEPTED) - - # STEP 2: RESEND code (optional step) - url = client.app.router["my_phone_resend"].url_for() - resp = await client.post(f"{url}") - await assert_status(resp, status.HTTP_202_ACCEPTED) - - # STEP 3: CONFIRM phone registration - url = client.app.router["my_phone_confirm"].url_for() - resp = await client.post( - f"{url}", - json={ - "code": _REGISTRATION_CODE_VALUE_FAKE, - }, - ) - await assert_status(resp, status.HTTP_204_NO_CONTENT) - - # GET updated profile - url = client.app.router["get_my_profile"].url_for() - resp = await client.get(f"{url}") - data, _ = await assert_status(resp, status.HTTP_200_OK) - - updated_profile = MyProfileRestGet.model_validate(data) - - # Verify phone was updated - assert updated_profile.phone == new_phone - - -@pytest.mark.parametrize("user_role", [UserRole.USER]) -async def test_phone_registration_change_existing_phone( - user_role: UserRole, - logged_user: UserInfoDict, - client: TestClient, - user_phone_number: PhoneNumberStr, -): - assert client.app - - # Set initial phone - first_phone = user_phone_number - url = client.app.router["my_phone_register"].url_for() - resp = await client.post( - f"{url}", - json={ - "phone": first_phone, - }, - ) - await assert_status(resp, status.HTTP_202_ACCEPTED) - - url = client.app.router["my_phone_confirm"].url_for() - resp = await client.post( - f"{url}", - json={ - "code": _REGISTRATION_CODE_VALUE_FAKE, - }, - ) - await assert_status(resp, status.HTTP_204_NO_CONTENT) - - # Change to new phone - # Create a different phone number by changing the last digits - new_phone = user_phone_number[:-4] + "9999" - url = client.app.router["my_phone_register"].url_for() - resp = await client.post( - f"{url}", - json={ - "phone": new_phone, - }, - ) - await assert_status(resp, status.HTTP_202_ACCEPTED) - - url = client.app.router["my_phone_confirm"].url_for() - resp = await client.post( - f"{url}", - json={ - "code": _REGISTRATION_CODE_VALUE_FAKE, - }, - ) - await assert_status(resp, status.HTTP_204_NO_CONTENT) - - # GET updated profile - url = client.app.router["get_my_profile"].url_for() - resp = await client.get(f"{url}") - data, _ = await assert_status(resp, status.HTTP_200_OK) - - updated_profile = MyProfileRestGet.model_validate(data) - - # Verify phone was updated to new phone - assert updated_profile.phone == new_phone - assert updated_profile.phone != first_phone - - -# -# PHONE REGISTRATION FAILURE TESTS -# - - -@pytest.mark.parametrize("user_role", [UserRole.USER]) -async def test_phone_resend_without_pending_registration( - user_role: UserRole, - logged_user: UserInfoDict, - client: TestClient, -): - assert client.app - - # Try to resend code without any pending registration - url = client.app.router["my_phone_resend"].url_for() - resp = await client.post(f"{url}") - await assert_status(resp, status.HTTP_400_BAD_REQUEST) - - -@pytest.mark.parametrize("user_role", [UserRole.USER]) -async def test_phone_confirm_without_pending_registration( - user_role: UserRole, - logged_user: UserInfoDict, - client: TestClient, -): - assert client.app - - # Try to confirm code without any pending registration - url = client.app.router["my_phone_confirm"].url_for() - resp = await client.post( - f"{url}", - json={ - "code": _REGISTRATION_CODE_VALUE_FAKE, - }, - ) - await assert_status(resp, status.HTTP_400_BAD_REQUEST) - - -@pytest.mark.parametrize("user_role", [UserRole.USER]) -async def test_phone_confirm_with_wrong_code( - user_role: UserRole, - logged_user: UserInfoDict, - client: TestClient, - user_phone_number: PhoneNumberStr, -): - assert client.app - - # STEP 1: REGISTER phone number - new_phone = user_phone_number - url = client.app.router["my_phone_register"].url_for() - resp = await client.post( - f"{url}", - json={ - "phone": new_phone, - }, - ) - await assert_status(resp, status.HTTP_202_ACCEPTED) - - # STEP 2: Try to confirm with wrong code - url = client.app.router["my_phone_confirm"].url_for() - resp = await client.post( - f"{url}", - json={ - "code": "wrongcode1234", - }, - ) - await assert_status(resp, status.HTTP_400_BAD_REQUEST) - - -@pytest.mark.parametrize("user_role", [UserRole.USER]) -async def test_phone_confirm_with_invalid_code_format( - user_role: UserRole, - logged_user: UserInfoDict, - client: TestClient, - user_phone_number: PhoneNumberStr, -): - assert client.app - - # STEP 1: REGISTER phone number - new_phone = user_phone_number - url = client.app.router["my_phone_register"].url_for() - resp = await client.post( - f"{url}", - json={ - "phone": new_phone, - }, - ) - await assert_status(resp, status.HTTP_202_ACCEPTED) - - # STEP 2: Try to confirm with invalid code format (contains special characters) - url = client.app.router["my_phone_confirm"].url_for() - resp = await client.post( - f"{url}", - json={ - "code": "123-456", # Invalid format according to pattern - }, - ) - await assert_status(resp, status.HTTP_422_UNPROCESSABLE_ENTITY) - - -@pytest.mark.parametrize("user_role", [UserRole.USER]) -async def test_phone_register_with_empty_phone( - user_role: UserRole, - logged_user: UserInfoDict, - client: TestClient, -): - assert client.app - - # Try to register with empty phone number - url = client.app.router["my_phone_register"].url_for() - resp = await client.post( - f"{url}", - json={ - "phone": "", # Empty phone number - }, - ) - await assert_status(resp, status.HTTP_422_UNPROCESSABLE_ENTITY) - - # Try to register with whitespace-only phone number - url = client.app.router["my_phone_register"].url_for() - resp = await client.post( - f"{url}", - json={ - "phone": " ", # Whitespace only - }, - ) - await assert_status(resp, status.HTTP_422_UNPROCESSABLE_ENTITY) - - -@pytest.mark.parametrize("user_role", [UserRole.USER]) -async def test_phone_confirm_with_empty_code( - user_role: UserRole, - logged_user: UserInfoDict, - client: TestClient, - user_phone_number: PhoneNumberStr, -): - assert client.app - - # STEP 1: REGISTER phone number - new_phone = user_phone_number - url = client.app.router["my_phone_register"].url_for() - resp = await client.post( - f"{url}", - json={ - "phone": new_phone, - }, - ) - await assert_status(resp, status.HTTP_202_ACCEPTED) - - # STEP 2: Try to confirm with empty code - url = client.app.router["my_phone_confirm"].url_for() - resp = await client.post( - f"{url}", - json={ - "code": "", # Empty code - }, - ) - await assert_status(resp, status.HTTP_422_UNPROCESSABLE_ENTITY) - - -@pytest.mark.parametrize( - "user_role,expected", - [ - (UserRole.ANONYMOUS, status.HTTP_401_UNAUTHORIZED), - (UserRole.GUEST, status.HTTP_403_FORBIDDEN), - ], -) -async def test_phone_register_access_rights( - user_role: UserRole, - logged_user: UserInfoDict, - client: TestClient, - expected: HTTPStatus, - user_phone_number: PhoneNumberStr, -): - assert client.app - - # Try to register phone with insufficient permissions - url = client.app.router["my_phone_register"].url_for() - resp = await client.post( - f"{url}", - json={ - "phone": user_phone_number, - }, - ) - await assert_status(resp, expected) - - -@pytest.mark.parametrize( - "user_role,expected", - [ - (UserRole.ANONYMOUS, status.HTTP_401_UNAUTHORIZED), - (UserRole.GUEST, status.HTTP_403_FORBIDDEN), - ], -) -async def test_phone_resend_access_rights( - user_role: UserRole, - logged_user: UserInfoDict, - client: TestClient, - expected: HTTPStatus, -): - assert client.app - - # Try to resend code with insufficient permissions - url = client.app.router["my_phone_resend"].url_for() - resp = await client.post(f"{url}") - await assert_status(resp, expected) - - -@pytest.mark.parametrize( - "user_role,expected", - [ - (UserRole.ANONYMOUS, status.HTTP_401_UNAUTHORIZED), - (UserRole.GUEST, status.HTTP_403_FORBIDDEN), - ], -) -async def test_phone_confirm_access_rights( - user_role: UserRole, - logged_user: UserInfoDict, - client: TestClient, - expected: HTTPStatus, -): - assert client.app - - # Try to confirm code with insufficient permissions - url = client.app.router["my_phone_confirm"].url_for() - resp = await client.post( - f"{url}", - json={ - "code": _REGISTRATION_CODE_VALUE_FAKE, - }, - ) - await assert_status(resp, expected) From 4eb14759ed62b5db7ceaa454b39592fe6ebb6a5c Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 3 Sep 2025 17:01:49 +0200 Subject: [PATCH 06/43] splits tests further --- .../with_dbs/03/test_users_rest_profile.py | 278 +--------------- .../with_dbs/03/test_users_rest_search.py | 309 ++++++++++++++++++ 2 files changed, 310 insertions(+), 277 deletions(-) create mode 100644 services/web/server/tests/unit/with_dbs/03/test_users_rest_search.py diff --git a/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py b/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py index 6bd3e708eb68..90268a853a91 100644 --- a/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py +++ b/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py @@ -7,7 +7,6 @@ import functools -from collections.abc import AsyncIterable from copy import deepcopy from http import HTTPStatus from typing import Any @@ -17,19 +16,13 @@ from aiohttp.test_utils import TestClient from aiopg.sa.connection import SAConnection from common_library.users_enums import UserRole -from models_library.api_schemas_webserver.groups import GroupUserGet from models_library.api_schemas_webserver.users import ( MyProfileRestGet, - UserGet, ) from psycopg2 import OperationalError -from pydantic import TypeAdapter from pytest_simcore.helpers.assert_checks import assert_status from pytest_simcore.helpers.monkeypatch_envs import EnvVarsDict, setenvs_from_dict -from pytest_simcore.helpers.webserver_login import ( - switch_client_session_to, -) -from pytest_simcore.helpers.webserver_users import NewUser, UserInfoDict +from pytest_simcore.helpers.webserver_users import UserInfoDict from servicelib.aiohttp import status from servicelib.rest_constants import RESPONSE_MODEL_POLICY from simcore_service_webserver.user_preferences._service import ( @@ -54,275 +47,6 @@ def app_environment( ) -@pytest.fixture -def partial_first_name() -> str: - return "Jaimito" - - -@pytest.fixture -def partial_username() -> str: - return "COMMON_USERNAME" - - -@pytest.fixture -def partial_email() -> str: - return "@acme.com" - - -@pytest.fixture -async def private_user( - client: TestClient, - partial_username: str, - partial_email: str, - partial_first_name: str, -) -> AsyncIterable[UserInfoDict]: - assert client.app - async with NewUser( - app=client.app, - user_data={ - "name": f"james{partial_username}", - "first_name": partial_first_name, - "last_name": "Bond", - "email": f"james{partial_email}", - # Maximum privacy - "privacy_hide_username": True, - "privacy_hide_email": True, - "privacy_hide_fullname": True, - }, - ) as usr: - yield usr - - -@pytest.fixture -async def semi_private_user( - client: TestClient, partial_username: str, partial_first_name: str -) -> AsyncIterable[UserInfoDict]: - assert client.app - async with NewUser( - app=client.app, - user_data={ - "name": f"maxwell{partial_username}", - "first_name": partial_first_name, - "last_name": "Maxwell", - "email": "j@maxwell.me", - # Medium privacy - "privacy_hide_username": False, - "privacy_hide_email": True, - "privacy_hide_fullname": False, # <-- - }, - ) as usr: - yield usr - - -@pytest.fixture -async def public_user( - client: TestClient, partial_username: str, partial_email: str -) -> AsyncIterable[UserInfoDict]: - assert client.app - async with NewUser( - app=client.app, - user_data={ - "name": f"taylor{partial_username}", - "first_name": "Taylor", - "last_name": "Swift", - "email": f"taylor{partial_email}", - # Fully public - "privacy_hide_username": False, - "privacy_hide_email": False, - "privacy_hide_fullname": False, - }, - ) as usr: - yield usr - - -@pytest.mark.parametrize("user_role", [UserRole.USER]) -async def test_search_users_by_partial_fullname( - user_role: UserRole, - logged_user: UserInfoDict, - client: TestClient, - partial_first_name: str, - private_user: UserInfoDict, - semi_private_user: UserInfoDict, - public_user: UserInfoDict, -): - assert client.app - assert user_role == logged_user["role"] - - # logged_user has default settings - assert private_user["id"] != logged_user["id"] - assert public_user["id"] != logged_user["id"] - - # SEARCH by partial first_name - assert partial_first_name in private_user.get("first_name", "") - assert partial_first_name in semi_private_user.get("first_name", "") - assert partial_first_name not in public_user.get("first_name", "") - - url = client.app.router["search_users"].url_for() - resp = await client.post(f"{url}", json={"match": partial_first_name}) - data, _ = await assert_status(resp, status.HTTP_200_OK) - - # expected `semi_private_user` found - found = TypeAdapter(list[UserGet]).validate_python(data) - assert found - assert len(found) == 1 - assert found[0].user_name == semi_private_user["name"] - assert found[0].first_name == semi_private_user.get("first_name") - assert found[0].last_name == semi_private_user.get("last_name") - assert found[0].email is None - - -@pytest.mark.parametrize("user_role", [UserRole.USER]) -async def test_search_users_by_partial_email( - user_role: UserRole, - logged_user: UserInfoDict, - client: TestClient, - partial_email: str, - public_user: UserInfoDict, - semi_private_user: UserInfoDict, - private_user: UserInfoDict, -): - - # SEARCH by partial email - assert partial_email in private_user["email"] - assert partial_email not in semi_private_user["email"] - assert partial_email in public_user["email"] - - url = client.app.router["search_users"].url_for() - resp = await client.post(f"{url}", json={"match": partial_email}) - data, _ = await assert_status(resp, status.HTTP_200_OK) - - found = TypeAdapter(list[UserGet]).validate_python(data) - assert found - assert len(found) == 1 - - # expected `public_user` found - assert found[0].user_id == public_user["id"] - assert found[0].user_name == public_user["name"] - assert found[0].email == public_user["email"] - assert found[0].first_name == public_user.get("first_name") - assert found[0].last_name == public_user.get("last_name") - - # SEARCH user for admin (from a USER) - url = ( - client.app.router["search_user_accounts"] - .url_for() - .with_query(email=partial_email) - ) - resp = await client.get(f"{url}") - await assert_status(resp, status.HTTP_403_FORBIDDEN) - - -@pytest.mark.parametrize("user_role", [UserRole.USER]) -async def test_search_users_by_partial_username( - logged_user: UserInfoDict, - client: TestClient, - partial_username: str, - public_user: UserInfoDict, - semi_private_user: UserInfoDict, - private_user: UserInfoDict, -): - assert client.app - - # SEARCH by partial username - assert partial_username in private_user["name"] - assert partial_username in semi_private_user["name"] - assert partial_username in public_user["name"] - - url = client.app.router["search_users"].url_for() - resp = await client.post(f"{url}", json={"match": partial_username}) - data, _ = await assert_status(resp, status.HTTP_200_OK) - - found = TypeAdapter(list[UserGet]).validate_python(data) - assert found - assert len(found) == 2 - - # expected `public_user` found - index = [u.user_id for u in found].index(public_user["id"]) - assert found[index].user_name == public_user["name"] - assert found[index].email == public_user["email"] - assert found[index].first_name == public_user.get("first_name") - assert found[index].last_name == public_user.get("last_name") - - # expected `semi_private_user` found - index = (index + 1) % 2 - assert found[index].user_name == semi_private_user["name"] - assert found[index].email is None - assert found[index].first_name == semi_private_user.get("first_name") - assert found[index].last_name == semi_private_user.get("last_name") - - -async def test_search_myself( - client: TestClient, - public_user: UserInfoDict, - semi_private_user: UserInfoDict, - private_user: UserInfoDict, -): - assert client.app - for user in [public_user, semi_private_user, private_user]: - async with switch_client_session_to(client, user): - - # search me - url = client.app.router["search_users"].url_for() - resp = await client.post(f"{url}", json={"match": user["name"]}) - data, _ = await assert_status(resp, status.HTTP_200_OK) - - found = TypeAdapter(list[UserGet]).validate_python(data) - assert found - assert len(found) == 1 - - # I can see my own data - assert found[0].user_name == user["name"] - assert found[0].email == user["email"] - assert found[0].first_name == user.get("first_name") - assert found[0].last_name == user.get("last_name") - - -@pytest.mark.acceptance_test( - "https://github.com/ITISFoundation/osparc-issues/issues/1779" -) -@pytest.mark.parametrize("user_role", [UserRole.USER]) -async def test_get_user_by_group_id( - user_role: UserRole, - logged_user: UserInfoDict, - client: TestClient, - public_user: UserInfoDict, - private_user: UserInfoDict, -): - assert client.app - assert user_role == logged_user["role"] - - assert private_user["id"] != logged_user["id"] - assert public_user["id"] != logged_user["id"] - - # GET public_user by its primary gid - url = client.app.router["get_all_group_users"].url_for( - gid=f"{public_user['primary_gid']}" - ) - resp = await client.get(f"{url}") - data, _ = await assert_status(resp, status.HTTP_200_OK) - - users = TypeAdapter(list[GroupUserGet]).validate_python(data) - assert len(users) == 1 - assert users[0].id == public_user["id"] - assert users[0].user_name == public_user["name"] - assert users[0].first_name == public_user.get("first_name") - assert users[0].last_name == public_user.get("last_name") - - # GET private_user by its primary gid - url = client.app.router["get_all_group_users"].url_for( - gid=f"{private_user['primary_gid']}" - ) - resp = await client.get(f"{url}") - data, _ = await assert_status(resp, status.HTTP_200_OK) - - users = TypeAdapter(list[GroupUserGet]).validate_python(data) - assert len(users) == 1 - assert users[0].id == private_user["id"] - assert users[0].user_name is None, "It's private" - assert users[0].first_name is None, "It's private" - assert users[0].last_name is None, "It's private" - - @pytest.mark.parametrize( "user_role,expected", [ diff --git a/services/web/server/tests/unit/with_dbs/03/test_users_rest_search.py b/services/web/server/tests/unit/with_dbs/03/test_users_rest_search.py new file mode 100644 index 000000000000..7473033cbd1a --- /dev/null +++ b/services/web/server/tests/unit/with_dbs/03/test_users_rest_search.py @@ -0,0 +1,309 @@ +# pylint: disable=protected-access +# pylint: disable=redefined-outer-name +# pylint: disable=too-many-arguments +# pylint: disable=too-many-statements +# pylint: disable=unused-argument +# pylint: disable=unused-variable + + +from collections.abc import AsyncIterable + +import pytest +from aiohttp.test_utils import TestClient +from common_library.users_enums import UserRole +from models_library.api_schemas_webserver.groups import GroupUserGet +from models_library.api_schemas_webserver.users import ( + UserGet, +) +from pydantic import TypeAdapter +from pytest_simcore.helpers.assert_checks import assert_status +from pytest_simcore.helpers.monkeypatch_envs import EnvVarsDict, setenvs_from_dict +from pytest_simcore.helpers.webserver_login import ( + switch_client_session_to, +) +from pytest_simcore.helpers.webserver_users import NewUser, UserInfoDict +from servicelib.aiohttp import status + + +@pytest.fixture +def app_environment( + app_environment: EnvVarsDict, monkeypatch: pytest.MonkeyPatch +) -> EnvVarsDict: + # disables GC and DB-listener + return app_environment | setenvs_from_dict( + monkeypatch, + { + "WEBSERVER_GARBAGE_COLLECTOR": "null", + "WEBSERVER_DB_LISTENER": "0", + "WEBSERVER_DEV_FEATURES_ENABLED": "1", # NOTE: still under development + }, + ) + + +@pytest.fixture +def partial_first_name() -> str: + return "Jaimito" + + +@pytest.fixture +def partial_username() -> str: + return "COMMON_USERNAME" + + +@pytest.fixture +def partial_email() -> str: + return "@acme.com" + + +@pytest.fixture +async def private_user( + client: TestClient, + partial_username: str, + partial_email: str, + partial_first_name: str, +) -> AsyncIterable[UserInfoDict]: + assert client.app + async with NewUser( + app=client.app, + user_data={ + "name": f"james{partial_username}", + "first_name": partial_first_name, + "last_name": "Bond", + "email": f"james{partial_email}", + # Maximum privacy + "privacy_hide_username": True, + "privacy_hide_email": True, + "privacy_hide_fullname": True, + }, + ) as usr: + yield usr + + +@pytest.fixture +async def semi_private_user( + client: TestClient, partial_username: str, partial_first_name: str +) -> AsyncIterable[UserInfoDict]: + assert client.app + async with NewUser( + app=client.app, + user_data={ + "name": f"maxwell{partial_username}", + "first_name": partial_first_name, + "last_name": "Maxwell", + "email": "j@maxwell.me", + # Medium privacy + "privacy_hide_username": False, + "privacy_hide_email": True, + "privacy_hide_fullname": False, # <-- + }, + ) as usr: + yield usr + + +@pytest.fixture +async def public_user( + client: TestClient, partial_username: str, partial_email: str +) -> AsyncIterable[UserInfoDict]: + assert client.app + async with NewUser( + app=client.app, + user_data={ + "name": f"taylor{partial_username}", + "first_name": "Taylor", + "last_name": "Swift", + "email": f"taylor{partial_email}", + # Fully public + "privacy_hide_username": False, + "privacy_hide_email": False, + "privacy_hide_fullname": False, + }, + ) as usr: + yield usr + + +@pytest.mark.acceptance_test( + "https://github.com/ITISFoundation/osparc-issues/issues/1779" +) +@pytest.mark.parametrize("user_role", [UserRole.USER]) +async def test_get_user_by_group_id( + user_role: UserRole, + logged_user: UserInfoDict, + client: TestClient, + public_user: UserInfoDict, + private_user: UserInfoDict, +): + assert client.app + assert user_role == logged_user["role"] + + assert private_user["id"] != logged_user["id"] + assert public_user["id"] != logged_user["id"] + + # GET public_user by its primary gid + url = client.app.router["get_all_group_users"].url_for( + gid=f"{public_user['primary_gid']}" + ) + resp = await client.get(f"{url}") + data, _ = await assert_status(resp, status.HTTP_200_OK) + + users = TypeAdapter(list[GroupUserGet]).validate_python(data) + assert len(users) == 1 + assert users[0].id == public_user["id"] + assert users[0].user_name == public_user["name"] + assert users[0].first_name == public_user.get("first_name") + assert users[0].last_name == public_user.get("last_name") + + # GET private_user by its primary gid + url = client.app.router["get_all_group_users"].url_for( + gid=f"{private_user['primary_gid']}" + ) + resp = await client.get(f"{url}") + data, _ = await assert_status(resp, status.HTTP_200_OK) + + users = TypeAdapter(list[GroupUserGet]).validate_python(data) + assert len(users) == 1 + assert users[0].id == private_user["id"] + assert users[0].user_name is None, "It's private" + assert users[0].first_name is None, "It's private" + assert users[0].last_name is None, "It's private" + + +@pytest.mark.parametrize("user_role", [UserRole.USER]) +async def test_search_users_by_partial_fullname( + user_role: UserRole, + logged_user: UserInfoDict, + client: TestClient, + partial_first_name: str, + private_user: UserInfoDict, + semi_private_user: UserInfoDict, + public_user: UserInfoDict, +): + assert client.app + assert user_role == logged_user["role"] + + # logged_user has default settings + assert private_user["id"] != logged_user["id"] + assert public_user["id"] != logged_user["id"] + + # SEARCH by partial first_name + assert partial_first_name in private_user.get("first_name", "") + assert partial_first_name in semi_private_user.get("first_name", "") + assert partial_first_name not in public_user.get("first_name", "") + + url = client.app.router["search_users"].url_for() + resp = await client.post(f"{url}", json={"match": partial_first_name}) + data, _ = await assert_status(resp, status.HTTP_200_OK) + + # expected `semi_private_user` found + found = TypeAdapter(list[UserGet]).validate_python(data) + assert found + assert len(found) == 1 + assert found[0].user_name == semi_private_user["name"] + assert found[0].first_name == semi_private_user.get("first_name") + assert found[0].last_name == semi_private_user.get("last_name") + assert found[0].email is None + + +@pytest.mark.parametrize("user_role", [UserRole.USER]) +async def test_search_users_by_partial_email( + user_role: UserRole, + logged_user: UserInfoDict, + client: TestClient, + partial_email: str, + public_user: UserInfoDict, + semi_private_user: UserInfoDict, + private_user: UserInfoDict, +): + + # SEARCH by partial email + assert partial_email in private_user["email"] + assert partial_email not in semi_private_user["email"] + assert partial_email in public_user["email"] + + url = client.app.router["search_users"].url_for() + resp = await client.post(f"{url}", json={"match": partial_email}) + data, _ = await assert_status(resp, status.HTTP_200_OK) + + found = TypeAdapter(list[UserGet]).validate_python(data) + assert found + assert len(found) == 1 + + # expected `public_user` found + assert found[0].user_id == public_user["id"] + assert found[0].user_name == public_user["name"] + assert found[0].email == public_user["email"] + assert found[0].first_name == public_user.get("first_name") + assert found[0].last_name == public_user.get("last_name") + + # SEARCH user for admin (from a USER) + url = ( + client.app.router["search_user_accounts"] + .url_for() + .with_query(email=partial_email) + ) + resp = await client.get(f"{url}") + await assert_status(resp, status.HTTP_403_FORBIDDEN) + + +@pytest.mark.parametrize("user_role", [UserRole.USER]) +async def test_search_users_by_partial_username( + logged_user: UserInfoDict, + client: TestClient, + partial_username: str, + public_user: UserInfoDict, + semi_private_user: UserInfoDict, + private_user: UserInfoDict, +): + assert client.app + + # SEARCH by partial username + assert partial_username in private_user["name"] + assert partial_username in semi_private_user["name"] + assert partial_username in public_user["name"] + + url = client.app.router["search_users"].url_for() + resp = await client.post(f"{url}", json={"match": partial_username}) + data, _ = await assert_status(resp, status.HTTP_200_OK) + + found = TypeAdapter(list[UserGet]).validate_python(data) + assert found + assert len(found) == 2 + + # expected `public_user` found + index = [u.user_id for u in found].index(public_user["id"]) + assert found[index].user_name == public_user["name"] + assert found[index].email == public_user["email"] + assert found[index].first_name == public_user.get("first_name") + assert found[index].last_name == public_user.get("last_name") + + # expected `semi_private_user` found + index = (index + 1) % 2 + assert found[index].user_name == semi_private_user["name"] + assert found[index].email is None + assert found[index].first_name == semi_private_user.get("first_name") + assert found[index].last_name == semi_private_user.get("last_name") + + +async def test_search_myself( + client: TestClient, + public_user: UserInfoDict, + semi_private_user: UserInfoDict, + private_user: UserInfoDict, +): + assert client.app + for user in [public_user, semi_private_user, private_user]: + async with switch_client_session_to(client, user): + + # search me + url = client.app.router["search_users"].url_for() + resp = await client.post(f"{url}", json={"match": user["name"]}) + data, _ = await assert_status(resp, status.HTTP_200_OK) + + found = TypeAdapter(list[UserGet]).validate_python(data) + assert found + assert len(found) == 1 + + # I can see my own data + assert found[0].user_name == user["name"] + assert found[0].email == user["email"] + assert found[0].first_name == user.get("first_name") + assert found[0].last_name == user.get("last_name") From 5dabf47c477f49c5dffdc4fde8220a4f4ad13f64 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 3 Sep 2025 17:23:46 +0200 Subject: [PATCH 07/43] refactors profile groups --- .../groups/_groups_rest.py | 34 ++++------------- .../groups/_groups_service.py | 38 +++++++++++++++++++ .../simcore_service_webserver/groups/api.py | 2 + .../users/_controller/rest/users_rest.py | 35 ++++------------- 4 files changed, 56 insertions(+), 53 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/groups/_groups_rest.py b/services/web/server/src/simcore_service_webserver/groups/_groups_rest.py index 806fa60b0b64..23a47136b030 100644 --- a/services/web/server/src/simcore_service_webserver/groups/_groups_rest.py +++ b/services/web/server/src/simcore_service_webserver/groups/_groups_rest.py @@ -1,5 +1,4 @@ import logging -from contextlib import suppress from aiohttp import web from models_library.api_schemas_webserver.groups import ( @@ -30,7 +29,6 @@ GroupsRequestContext, GroupsUsersPathParams, ) -from .exceptions import GroupNotFoundError _logger = logging.getLogger(__name__) @@ -49,32 +47,16 @@ async def list_groups(request: web.Request): product: Product = products_web.get_current_product(request) req_ctx = GroupsRequestContext.model_validate(request) - groups_by_type = await _groups_service.list_user_groups_with_read_access( - request.app, user_id=req_ctx.user_id + ( + groups_by_type, + my_product_group, + my_support_group, + ) = await _groups_service.get_user_profile_groups( + request.app, user_id=req_ctx.user_id, product=product ) - assert groups_by_type.primary - assert groups_by_type.everyone - - if product.group_id: - with suppress(GroupNotFoundError): - # Product is optional - my_product_group = await _groups_service.get_product_group_for_user( - app=request.app, - user_id=req_ctx.user_id, - product_gid=product.group_id, - ) - else: - my_product_group = None - - if product.support_standard_group_id: - my_support_group = await _groups_service.get_support_group_for_user_or_none( - app=request.app, - user_id=req_ctx.user_id, - support_gid=product.support_standard_group_id, - ) - else: - my_support_group = None + assert groups_by_type.primary # nosec + assert groups_by_type.everyone # nosec my_groups = MyGroupsGet.from_domain_model( groups_by_type, my_product_group, my_support_group diff --git a/services/web/server/src/simcore_service_webserver/groups/_groups_service.py b/services/web/server/src/simcore_service_webserver/groups/_groups_service.py index c28b7d9d2e34..f0b05cdcc474 100644 --- a/services/web/server/src/simcore_service_webserver/groups/_groups_service.py +++ b/services/web/server/src/simcore_service_webserver/groups/_groups_service.py @@ -16,6 +16,7 @@ from models_library.users import UserID from pydantic import EmailStr +from ..products.models import Product from ..users import users_service from . import _groups_repository from .exceptions import GroupNotFoundError, GroupsError @@ -91,6 +92,43 @@ async def get_support_group_for_user_or_none( return None +async def get_user_profile_groups( + app: web.Application, *, user_id: UserID, product: Product +) -> tuple[ + GroupsByTypeTuple, + tuple[Group, AccessRightsDict] | None, + tuple[Group, AccessRightsDict] | None, +]: + """ + Get all groups needed for user profile including standard groups, + product group, and support group. + + Returns: + Tuple of (groups_by_type, my_product_group, my_support_group) + """ + groups_by_type = await list_user_groups_with_read_access(app, user_id=user_id) + + my_product_group = None + if product.group_id: # Product group is optional + with suppress(GroupNotFoundError): + my_product_group = await get_product_group_for_user( + app=app, + user_id=user_id, + product_gid=product.group_id, + ) + + my_support_group = None + if product.support_standard_group_id: # Support group is optional + # NOTE: my_support_group can be part of groups_by_type.standard! + my_support_group = await get_support_group_for_user_or_none( + app=app, + user_id=user_id, + support_gid=product.support_standard_group_id, + ) + + return groups_by_type, my_product_group, my_support_group + + # # CRUD operations on groups linked to a user # diff --git a/services/web/server/src/simcore_service_webserver/groups/api.py b/services/web/server/src/simcore_service_webserver/groups/api.py index 68f14c239baf..d72b1e923eaa 100644 --- a/services/web/server/src/simcore_service_webserver/groups/api.py +++ b/services/web/server/src/simcore_service_webserver/groups/api.py @@ -8,6 +8,7 @@ get_group_from_gid, get_product_group_for_user, get_support_group_for_user_or_none, + get_user_profile_groups, is_user_by_email_in_group, list_all_user_groups_ids, list_group_members, @@ -20,6 +21,7 @@ "auto_add_user_to_groups", "auto_add_user_to_product_group", "get_group_from_gid", + "get_user_profile_groups", "get_product_group_for_user", "get_support_group_for_user_or_none", "is_user_by_email_in_group", diff --git a/services/web/server/src/simcore_service_webserver/users/_controller/rest/users_rest.py b/services/web/server/src/simcore_service_webserver/users/_controller/rest/users_rest.py index 0fd831d20500..42d116f76ad9 100644 --- a/services/web/server/src/simcore_service_webserver/users/_controller/rest/users_rest.py +++ b/services/web/server/src/simcore_service_webserver/users/_controller/rest/users_rest.py @@ -1,5 +1,4 @@ import logging -from contextlib import suppress from aiohttp import web from models_library.api_schemas_webserver.users import ( @@ -18,7 +17,6 @@ from ...._meta import API_VTAG from ....groups import api as groups_service -from ....groups.exceptions import GroupNotFoundError from ....login.decorators import login_required from ....products import products_web from ....products.models import Product @@ -51,33 +49,16 @@ async def get_my_profile(request: web.Request) -> web.Response: product: Product = products_web.get_current_product(request) req_ctx = UsersRequestContext.model_validate(request) - groups_by_type = await groups_service.list_user_groups_with_read_access( - request.app, user_id=req_ctx.user_id + ( + groups_by_type, + my_product_group, + my_support_group, + ) = await groups_service.get_user_profile_groups( + request.app, user_id=req_ctx.user_id, product=product ) - assert groups_by_type.primary - assert groups_by_type.everyone - - if product.group_id: - with suppress(GroupNotFoundError): - # Product is optional - my_product_group = await groups_service.get_product_group_for_user( - app=request.app, - user_id=req_ctx.user_id, - product_gid=product.group_id, - ) - else: - my_product_group = None - - if product.support_standard_group_id: - # Support group is optional - my_support_group = await groups_service.get_support_group_for_user_or_none( - app=request.app, - user_id=req_ctx.user_id, - support_gid=product.support_standard_group_id, - ) - else: - my_support_group = None + assert groups_by_type.primary # nosec + assert groups_by_type.everyone # nosec my_profile, preferences = await _users_service.get_my_profile( request.app, user_id=req_ctx.user_id, product_name=req_ctx.product_name From adbdb5f06d06324b26cd55de82ab5a57ddaa0165 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 3 Sep 2025 18:02:51 +0200 Subject: [PATCH 08/43] fixes statics --- .../server/src/simcore_service_webserver/products/_models.py | 1 + .../server/src/simcore_service_webserver/statics/_events.py | 3 --- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/products/_models.py b/services/web/server/src/simcore_service_webserver/products/_models.py index cb2a08400ac4..e28c20eb2b68 100644 --- a/services/web/server/src/simcore_service_webserver/products/_models.py +++ b/services/web/server/src/simcore_service_webserver/products/_models.py @@ -332,6 +332,7 @@ def to_statics(self) -> dict[str, Any]: "support": True, "is_payment_enabled": True, "is_dynamic_services_telemetry_enabled": True, + "support_standard_group_id": True, }, exclude_none=True, exclude_unset=True, diff --git a/services/web/server/src/simcore_service_webserver/statics/_events.py b/services/web/server/src/simcore_service_webserver/statics/_events.py index 742fdaa828b4..af62a9bff253 100644 --- a/services/web/server/src/simcore_service_webserver/statics/_events.py +++ b/services/web/server/src/simcore_service_webserver/statics/_events.py @@ -147,9 +147,6 @@ async def create_and_cache_statics_json(app: web.Application) -> None: release_vtag = _get_release_notes_vtag(vtag) data["vcsReleaseUrl"] = template_url.format(vtag=release_vtag) - # Add support_standard_group_id - data["supportStandardGroupId"] = product.support_standard_group_id - data_json = json_dumps(data) _logger.debug("Front-end statics.json: %s", data_json) From 5bb292d9bf1f9d810bf0ef50ee22dccb666bc274 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 3 Sep 2025 18:32:36 +0200 Subject: [PATCH 09/43] only group --- .../groups/_groups_service.py | 21 ++------ .../with_dbs/03/test_users_rest_profile.py | 53 ++++++++++++++++++- 2 files changed, 55 insertions(+), 19 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/groups/_groups_service.py b/services/web/server/src/simcore_service_webserver/groups/_groups_service.py index f0b05cdcc474..83dd4276b40d 100644 --- a/services/web/server/src/simcore_service_webserver/groups/_groups_service.py +++ b/services/web/server/src/simcore_service_webserver/groups/_groups_service.py @@ -79,25 +79,12 @@ async def get_product_group_for_user( ) -async def get_support_group_for_user_or_none( - app: web.Application, *, user_id: UserID, support_gid: GroupID -) -> tuple[Group, AccessRightsDict] | None: - """ - Returns support's group if user belongs to it, otherwise it - """ - with suppress(GroupNotFoundError): - return await _groups_repository.get_any_group_for_user( - app, user_id=user_id, group_gid=support_gid - ) - return None - - async def get_user_profile_groups( app: web.Application, *, user_id: UserID, product: Product ) -> tuple[ GroupsByTypeTuple, tuple[Group, AccessRightsDict] | None, - tuple[Group, AccessRightsDict] | None, + Group | None, ]: """ Get all groups needed for user profile including standard groups, @@ -120,10 +107,8 @@ async def get_user_profile_groups( my_support_group = None if product.support_standard_group_id: # Support group is optional # NOTE: my_support_group can be part of groups_by_type.standard! - my_support_group = await get_support_group_for_user_or_none( - app=app, - user_id=user_id, - support_gid=product.support_standard_group_id, + my_support_group = await get_group_from_gid( + app, product.support_standard_group_id ) return groups_by_type, my_product_group, my_support_group diff --git a/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py b/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py index 90268a853a91..915ef53d6d6d 100644 --- a/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py +++ b/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py @@ -7,12 +7,14 @@ import functools +from collections.abc import Iterator from copy import deepcopy from http import HTTPStatus from typing import Any from unittest.mock import patch import pytest +import sqlalchemy as sa from aiohttp.test_utils import TestClient from aiopg.sa.connection import SAConnection from common_library.users_enums import UserRole @@ -22,6 +24,7 @@ from psycopg2 import OperationalError from pytest_simcore.helpers.assert_checks import assert_status from pytest_simcore.helpers.monkeypatch_envs import EnvVarsDict, setenvs_from_dict +from pytest_simcore.helpers.postgres_tools import sync_insert_and_get_row_lifespan from pytest_simcore.helpers.webserver_users import UserInfoDict from servicelib.aiohttp import status from servicelib.rest_constants import RESPONSE_MODEL_POLICY @@ -47,6 +50,41 @@ def app_environment( ) +@pytest.fixture(scope="module") +def support_group( + postgres_db: sa.engine.Engine, + product_name: str, +) -> Iterator[dict[str, Any]]: + """Creates a standard support group and assigns it to the current product""" + from simcore_postgres_database.models.groups import groups + from simcore_postgres_database.models.products import products + + # Create support group using direct database insertion + group_values = { + "name": "Support Group", + "description": "Support group for product", + "type": "STANDARD", + } + + with sync_insert_and_get_row_lifespan( + postgres_db, + table=groups, + values=group_values, + pk_col=groups.c.gid, + ) as group_row: + group_id = group_row["gid"] + + # Update product to set support_standard_group_id + with postgres_db.begin() as conn: + conn.execute( + sa.update(products) + .where(products.c.name == product_name) + .values(support_standard_group_id=group_id) + ) + + yield group_row + + @pytest.mark.parametrize( "user_role,expected", [ @@ -100,6 +138,7 @@ async def test_get_profile( primary_group: dict[str, Any], standard_groups: list[dict[str, Any]], all_group: dict[str, str], + support_group: dict[str, Any], ): assert client.app @@ -130,13 +169,25 @@ async def test_get_profile( "thumbnail": None, } - assert got_profile_groups["support"] is None + # support group exists + assert got_profile_groups["support"] + support_group_id = got_profile_groups["support"]["gid"] + assert support_group_id == support_group["gid"] + assert got_profile_groups["support"]["description"] == support_group["description"] + assert "accessRights" not in got_profile_groups["support"] + + # standard groups with at least read access sorted_by_group_id = functools.partial(sorted, key=lambda d: d["gid"]) assert sorted_by_group_id( got_profile_groups["organizations"] ) == sorted_by_group_id(standard_groups) + # user is NOT part of the support group + all_standard_groups_ids = {g["gid"] for g in standard_groups} + assert support_group_id not in all_standard_groups_ids + + # preferences assert profile.preferences == await get_frontend_user_preferences_aggregation( client.app, user_id=logged_user["id"], product_name="osparc" ) From 4ceaa42844f0e33b67adbd0db2dae62a2d42aabb Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 3 Sep 2025 18:40:01 +0200 Subject: [PATCH 10/43] gets product support group --- .../api_schemas_webserver/groups.py | 91 +++++++++++-------- .../api_schemas_webserver/users.py | 2 +- packages/models-library/tests/test_users.py | 4 +- .../groups/_groups_rest.py | 4 +- .../groups/_groups_service.py | 8 +- .../users/_controller/rest/users_rest.py | 4 +- 6 files changed, 62 insertions(+), 51 deletions(-) diff --git a/packages/models-library/src/models_library/api_schemas_webserver/groups.py b/packages/models-library/src/models_library/api_schemas_webserver/groups.py index 505d06e7fb40..0694283df9a8 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/groups.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/groups.py @@ -54,14 +54,53 @@ class GroupAccessRights(BaseModel): ) -class GroupGet(OutputSchema): - gid: GroupID = Field(..., description="the group ID") - label: str = Field(..., description="the group name") - description: str = Field(..., description="the group description") - thumbnail: AnyUrl | None = Field( - default=None, description="url to the group thumbnail" - ) - access_rights: GroupAccessRights = Field(..., alias="accessRights") +class BasicGroupGet(OutputSchema): + gid: Annotated[GroupID, Field(description="the group's unique ID")] + label: Annotated[str, Field(description="the group's display name")] + description: str + thumbnail: Annotated[ + AnyUrl | None, Field(description="a link to the group's thumbnail") + ] = None + + @field_validator("thumbnail", mode="before") + @classmethod + def _sanitize_legacy_data(cls, v): + if v: + # Enforces null if thumbnail is not valid URL or empty + with suppress(ValidationError): + return TypeAdapter(AnyHttpUrl).validate_python(v) + return None + + @classmethod + def _extract_basic_group_data(cls, group: Group) -> dict: + """Extract common group data for schema conversion""" + return remap_keys( + group.model_dump( + include={ + "gid", + "name", + "description", + "thumbnail", + }, + exclude={ + "inclusion_rules", # deprecated + }, + exclude_unset=True, + by_alias=False, + ), + rename={ + "name": "label", + }, + ) + + @classmethod + def from_domain_model(cls, group: Group) -> Self: + # Adapts these domain models into this schema + return cls.model_validate(cls._extract_basic_group_data(group)) + + +class GroupGet(BasicGroupGet): + access_rights: Annotated[GroupAccessRights, Field(alias="accessRights")] inclusion_rules: Annotated[ dict[str, str], @@ -77,24 +116,7 @@ def from_domain_model(cls, group: Group, access_rights: AccessRightsDict) -> Sel # Adapts these domain models into this schema return cls.model_validate( { - **remap_keys( - group.model_dump( - include={ - "gid", - "name", - "description", - "thumbnail", - }, - exclude={ - "inclusion_rules", # deprecated - }, - exclude_unset=True, - by_alias=False, - ), - rename={ - "name": "label", - }, - ), + **cls._extract_basic_group_data(group), "access_rights": access_rights, } ) @@ -136,15 +158,6 @@ def _update_json_schema_extra(schema: JsonDict) -> None: model_config = ConfigDict(json_schema_extra=_update_json_schema_extra) - @field_validator("thumbnail", mode="before") - @classmethod - def _sanitize_legacy_data(cls, v): - if v: - # Enforces null if thumbnail is not valid URL or empty - with suppress(ValidationError): - return TypeAdapter(AnyHttpUrl).validate_python(v) - return None - class GroupCreate(InputSchema): label: str @@ -188,7 +201,7 @@ class MyGroupsGet(OutputSchema): all: GroupGet product: GroupGet | None = None support: Annotated[ - GroupGet | None, + BasicGroupGet | None, Field( description="Group ID of the app support team or None if no support is defined for this product" ), @@ -235,7 +248,7 @@ class MyGroupsGet(OutputSchema): "gid": "2", "label": "Support Team", "description": "The support team of the application", - "accessRights": {"read": False, "write": False, "delete": False}, + "thumbnail": "https://placekitten.com/15/15", }, } } @@ -246,7 +259,7 @@ def from_domain_model( cls, groups_by_type: GroupsByTypeTuple, my_product_group: tuple[Group, AccessRightsDict] | None, - my_support_group: tuple[Group, AccessRightsDict] | None, + my_support_group: Group | None, ) -> Self: assert groups_by_type.primary # nosec assert groups_by_type.everyone # nosec @@ -263,7 +276,7 @@ def from_domain_model( else None ), support=( - GroupGet.from_domain_model(*my_support_group) + BasicGroupGet.from_domain_model(my_support_group) if my_support_group else None ), diff --git a/packages/models-library/src/models_library/api_schemas_webserver/users.py b/packages/models-library/src/models_library/api_schemas_webserver/users.py index f9ff1821fc7f..59bffad22880 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/users.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/users.py @@ -132,7 +132,7 @@ def from_domain_model( my_groups_by_type: GroupsByTypeTuple, my_product_group: tuple[Group, AccessRightsDict] | None, my_preferences: AggregatedPreferences, - my_support_group: tuple[Group, AccessRightsDict] | None, + my_support_group: Group | None, ) -> Self: data = remap_keys( my_profile.model_dump( diff --git a/packages/models-library/tests/test_users.py b/packages/models-library/tests/test_users.py index 598e9074c6a7..d1e5dbe4efdc 100644 --- a/packages/models-library/tests/test_users.py +++ b/packages/models-library/tests/test_users.py @@ -30,9 +30,7 @@ def test_adapter_from_model_to_schema( read=False, write=False, delete=False ) - my_support_group = groups[4], AccessRightsDict( - read=False, write=False, delete=False - ) + my_support_group = groups[4] my_preferences = {"foo": Preference(default_value=3, value=1)} diff --git a/services/web/server/src/simcore_service_webserver/groups/_groups_rest.py b/services/web/server/src/simcore_service_webserver/groups/_groups_rest.py index 23a47136b030..42fbd892d3f9 100644 --- a/services/web/server/src/simcore_service_webserver/groups/_groups_rest.py +++ b/services/web/server/src/simcore_service_webserver/groups/_groups_rest.py @@ -50,7 +50,7 @@ async def list_groups(request: web.Request): ( groups_by_type, my_product_group, - my_support_group, + product_support_group, ) = await _groups_service.get_user_profile_groups( request.app, user_id=req_ctx.user_id, product=product ) @@ -59,7 +59,7 @@ async def list_groups(request: web.Request): assert groups_by_type.everyone # nosec my_groups = MyGroupsGet.from_domain_model( - groups_by_type, my_product_group, my_support_group + groups_by_type, my_product_group, product_support_group ) return envelope_json_response(my_groups) diff --git a/services/web/server/src/simcore_service_webserver/groups/_groups_service.py b/services/web/server/src/simcore_service_webserver/groups/_groups_service.py index 83dd4276b40d..b6ca9fc23dc2 100644 --- a/services/web/server/src/simcore_service_webserver/groups/_groups_service.py +++ b/services/web/server/src/simcore_service_webserver/groups/_groups_service.py @@ -91,7 +91,7 @@ async def get_user_profile_groups( product group, and support group. Returns: - Tuple of (groups_by_type, my_product_group, my_support_group) + Tuple of (groups_by_type, my_product_group, product_support_group) """ groups_by_type = await list_user_groups_with_read_access(app, user_id=user_id) @@ -104,14 +104,14 @@ async def get_user_profile_groups( product_gid=product.group_id, ) - my_support_group = None + product_support_group = None if product.support_standard_group_id: # Support group is optional # NOTE: my_support_group can be part of groups_by_type.standard! - my_support_group = await get_group_from_gid( + product_support_group = await get_group_from_gid( app, product.support_standard_group_id ) - return groups_by_type, my_product_group, my_support_group + return groups_by_type, my_product_group, product_support_group # diff --git a/services/web/server/src/simcore_service_webserver/users/_controller/rest/users_rest.py b/services/web/server/src/simcore_service_webserver/users/_controller/rest/users_rest.py index 42d116f76ad9..f306da6aa28c 100644 --- a/services/web/server/src/simcore_service_webserver/users/_controller/rest/users_rest.py +++ b/services/web/server/src/simcore_service_webserver/users/_controller/rest/users_rest.py @@ -52,7 +52,7 @@ async def get_my_profile(request: web.Request) -> web.Response: ( groups_by_type, my_product_group, - my_support_group, + product_support_group, ) = await groups_service.get_user_profile_groups( request.app, user_id=req_ctx.user_id, product=product ) @@ -65,7 +65,7 @@ async def get_my_profile(request: web.Request) -> web.Response: ) profile = MyProfileRestGet.from_domain_model( - my_profile, groups_by_type, my_product_group, preferences, my_support_group + my_profile, groups_by_type, my_product_group, preferences, product_support_group ) return envelope_json_response(profile) From b4af7889990dd0db8f3ac055b9b97c3d698a7684 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 3 Sep 2025 18:41:28 +0200 Subject: [PATCH 11/43] cleanup --- .../web/server/src/simcore_service_webserver/groups/api.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/groups/api.py b/services/web/server/src/simcore_service_webserver/groups/api.py index d72b1e923eaa..0c22a3e6a255 100644 --- a/services/web/server/src/simcore_service_webserver/groups/api.py +++ b/services/web/server/src/simcore_service_webserver/groups/api.py @@ -7,7 +7,6 @@ auto_add_user_to_product_group, get_group_from_gid, get_product_group_for_user, - get_support_group_for_user_or_none, get_user_profile_groups, is_user_by_email_in_group, list_all_user_groups_ids, @@ -21,9 +20,8 @@ "auto_add_user_to_groups", "auto_add_user_to_product_group", "get_group_from_gid", - "get_user_profile_groups", "get_product_group_for_user", - "get_support_group_for_user_or_none", + "get_user_profile_groups", "is_user_by_email_in_group", "list_all_user_groups_ids", "list_group_members", From 1e1c2446ee15c0c08205588345e4a88864e1c300 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 3 Sep 2025 18:45:43 +0200 Subject: [PATCH 12/43] tests --- .../server/tests/unit/with_dbs/03/test_users_rest_profile.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py b/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py index 915ef53d6d6d..75552e270087 100644 --- a/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py +++ b/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py @@ -164,7 +164,7 @@ async def test_get_profile( assert got_profile_groups["product"] == { "accessRights": {"delete": False, "read": False, "write": False}, "description": "osparc product group", - "gid": 2, + "gid": 3, "label": "osparc", "thumbnail": None, } From 68fc0fc633ae3697fbf40f655043a9fccb12a7a7 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 3 Sep 2025 19:16:34 +0200 Subject: [PATCH 13/43] separate access rights in repo --- .../groups/_groups_repository.py | 31 ++++++++++++++----- .../groups/_groups_service.py | 7 +++++ 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/groups/_groups_repository.py b/services/web/server/src/simcore_service_webserver/groups/_groups_repository.py index eecb008b4556..799565e505a3 100644 --- a/services/web/server/src/simcore_service_webserver/groups/_groups_repository.py +++ b/services/web/server/src/simcore_service_webserver/groups/_groups_repository.py @@ -679,6 +679,26 @@ async def delete_user_from_group( # +async def check_group_write_access( + app: web.Application, + connection: AsyncConnection | None = None, + *, + caller_id: UserID, + group_id: GroupID, +) -> None: + """ + Checks if caller has write access to the group. + + Raises: + GroupNotFoundError: if group not found or caller has no access + UserInsufficientRightsError: if caller has no write permission + """ + async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: + await _get_group_and_access_rights_or_raise( + conn, caller_id=caller_id, group_id=group_id, check_permission="write" + ) + + async def is_user_by_email_in_group( app: web.Application, connection: AsyncConnection | None = None, @@ -701,7 +721,6 @@ async def add_new_user_in_group( app: web.Application, connection: AsyncConnection | None = None, *, - caller_id: UserID, group_id: GroupID, # either user_id or user_name new_user_id: UserID | None = None, @@ -709,14 +728,11 @@ async def add_new_user_in_group( access_rights: AccessRightsDict | None = None, ) -> None: """ - adds new_user (either by id or email) in group (with gid) owned by user_id + adds new_user (either by id or email) in group (with gid) + + Note: This function does not check permissions - caller must ensure permissions are checked separately """ async with transaction_context(get_asyncpg_engine(app), connection) as conn: - # first check if the group exists - await _get_group_and_access_rights_or_raise( - conn, caller_id=caller_id, group_id=group_id, check_permission="write" - ) - query = sa.select(users.c.id) if new_user_id is not None: query = query.where(users.c.id == new_user_id) @@ -747,7 +763,6 @@ async def add_new_user_in_group( raise UserAlreadyInGroupError( uid=new_user_id, gid=group_id, - user_id=caller_id, access_rights=access_rights, ) from exc diff --git a/services/web/server/src/simcore_service_webserver/groups/_groups_service.py b/services/web/server/src/simcore_service_webserver/groups/_groups_service.py index b6ca9fc23dc2..8d420d9686a1 100644 --- a/services/web/server/src/simcore_service_webserver/groups/_groups_service.py +++ b/services/web/server/src/simcore_service_webserver/groups/_groups_service.py @@ -297,11 +297,18 @@ async def add_user_in_group( Raises: UserInGroupNotFoundError GroupsException + GroupNotFoundError + UserInsufficientRightsError """ if not _only_one_true(new_by_user_id, new_by_user_name, new_by_user_email): msg = "Invalid method call, required one of these: user id, username or user email, none provided" raise GroupsError(msg=msg) + # First check if caller has write access to the group + await _groups_repository.check_group_write_access( + app, caller_id=user_id, group_id=group_id + ) + # get target user to add to group if new_by_user_email: user = await _groups_repository.get_user_from_email( From 3255e696559f1c3f24d6b14f9854fa23dce14f87 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 3 Sep 2025 19:22:41 +0200 Subject: [PATCH 14/43] fixing test --- .../groups/_groups_service.py | 1 - .../with_dbs/03/test_users_rest_profile.py | 102 +++++++++++++++--- 2 files changed, 90 insertions(+), 13 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/groups/_groups_service.py b/services/web/server/src/simcore_service_webserver/groups/_groups_service.py index 8d420d9686a1..1f44670dcac9 100644 --- a/services/web/server/src/simcore_service_webserver/groups/_groups_service.py +++ b/services/web/server/src/simcore_service_webserver/groups/_groups_service.py @@ -322,7 +322,6 @@ async def add_user_in_group( return await _groups_repository.add_new_user_in_group( app, - caller_id=user_id, group_id=group_id, new_user_id=new_by_user_id, new_user_name=new_by_user_name, diff --git a/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py b/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py index 75552e270087..f4e67f74ab93 100644 --- a/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py +++ b/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py @@ -7,7 +7,7 @@ import functools -from collections.abc import Iterator +from collections.abc import AsyncIterator from copy import deepcopy from http import HTTPStatus from typing import Any @@ -21,10 +21,10 @@ from models_library.api_schemas_webserver.users import ( MyProfileRestGet, ) +from models_library.groups import AccessRightsDict from psycopg2 import OperationalError from pytest_simcore.helpers.assert_checks import assert_status from pytest_simcore.helpers.monkeypatch_envs import EnvVarsDict, setenvs_from_dict -from pytest_simcore.helpers.postgres_tools import sync_insert_and_get_row_lifespan from pytest_simcore.helpers.webserver_users import UserInfoDict from servicelib.aiohttp import status from servicelib.rest_constants import RESPONSE_MODEL_POLICY @@ -32,7 +32,7 @@ get_frontend_user_preferences_aggregation, ) from sqlalchemy.exc import OperationalError as SQLAlchemyOperationalError -from sqlalchemy.ext.asyncio import AsyncConnection +from sqlalchemy.ext.asyncio import AsyncConnection, AsyncEngine @pytest.fixture @@ -50,12 +50,13 @@ def app_environment( ) -@pytest.fixture(scope="module") -def support_group( - postgres_db: sa.engine.Engine, +@pytest.fixture +async def support_group( + asyncpg_engine: AsyncEngine, product_name: str, -) -> Iterator[dict[str, Any]]: +) -> AsyncIterator[dict[str, Any]]: """Creates a standard support group and assigns it to the current product""" + from pytest_simcore.helpers.postgres_tools import insert_and_get_row_lifespan from simcore_postgres_database.models.groups import groups from simcore_postgres_database.models.products import products @@ -66,8 +67,9 @@ def support_group( "type": "STANDARD", } - with sync_insert_and_get_row_lifespan( - postgres_db, + # pylint: disable=contextmanager-generator-missing-cleanup + async with insert_and_get_row_lifespan( + asyncpg_engine, table=groups, values=group_values, pk_col=groups.c.gid, @@ -75,14 +77,15 @@ def support_group( group_id = group_row["gid"] # Update product to set support_standard_group_id - with postgres_db.begin() as conn: - conn.execute( + async with asyncpg_engine.begin() as conn: + await conn.execute( sa.update(products) .where(products.c.name == product_name) .values(support_standard_group_id=group_id) ) yield group_row + # group will be deleted after test @pytest.mark.parametrize( @@ -131,7 +134,7 @@ async def test_access_update_profile( @pytest.mark.parametrize("user_role", [UserRole.USER]) -async def test_get_profile( +async def test_get_profile_user_not_in_support_group( user_role: UserRole, logged_user: UserInfoDict, client: TestClient, @@ -193,6 +196,81 @@ async def test_get_profile( ) +@pytest.mark.test +@pytest.mark.parametrize("user_role", [UserRole.USER]) +async def test_get_profile_user_in_support_group( + user_role: UserRole, + logged_user: UserInfoDict, + client: TestClient, + primary_group: dict[str, Any], + standard_groups: list[dict[str, Any]], + all_group: dict[str, str], + support_group: dict[str, Any], +): + assert client.app + from simcore_service_webserver.groups import _groups_repository + + # Now add user to support group with read-only access + await _groups_repository.add_new_user_in_group( + client.app, + group_id=support_group["gid"], + new_user_id=logged_user["id"], + access_rights=AccessRightsDict(read=True, write=False, delete=False), + ) + + url = client.app.router["get_my_profile"].url_for() + assert url.path == "/v0/me" + + resp = await client.get(f"{url}") + data, error = await assert_status(resp, status.HTTP_200_OK) + + assert not error + profile = MyProfileRestGet.model_validate(data) + + assert profile.login == logged_user["email"] + assert profile.first_name == logged_user.get("first_name", None) + assert profile.last_name == logged_user.get("last_name", None) + assert profile.role == user_role.name + assert profile.groups + assert profile.expiration_date is None + + got_profile_groups = profile.groups.model_dump(**RESPONSE_MODEL_POLICY, mode="json") + assert got_profile_groups["me"] == primary_group + assert got_profile_groups["all"] == all_group + assert got_profile_groups["product"] == { + "accessRights": {"delete": False, "read": False, "write": False}, + "description": "osparc product group", + "gid": 3, + "label": "osparc", + "thumbnail": None, + } + + # support group exists + assert got_profile_groups["support"] + support_group_id = got_profile_groups["support"]["gid"] + + assert support_group_id == support_group["gid"] + assert got_profile_groups["support"]["description"] == support_group["description"] + assert "accessRights" not in got_profile_groups["support"] + + # When user is part of support group, it should appear in standard groups + sorted_by_group_id = functools.partial(sorted, key=lambda d: d["gid"]) + expected_standard_groups = [ + *standard_groups, + { + "gid": support_group_id, + "label": support_group["name"], + "description": support_group["description"], + "thumbnail": None, + "accessRights": {"read": True, "write": False, "delete": False}, + }, + ] + assert sorted_by_group_id( + got_profile_groups["organizations"] + ) == sorted_by_group_id(expected_standard_groups) + assert support_group_id in {g["gid"] for g in got_profile_groups["organizations"]} + + @pytest.mark.parametrize("user_role", [UserRole.USER]) async def test_update_profile( user_role: UserRole, From 9a343512f57cbccaec600559501773675ed1a340 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 3 Sep 2025 19:34:29 +0200 Subject: [PATCH 15/43] fixing tests --- .../with_dbs/03/test_users_rest_profile.py | 29 ++++++++++++------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py b/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py index f4e67f74ab93..db40aac72ed4 100644 --- a/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py +++ b/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py @@ -51,7 +51,7 @@ def app_environment( @pytest.fixture -async def support_group( +async def support_group_before_app_starts( asyncpg_engine: AsyncEngine, product_name: str, ) -> AsyncIterator[dict[str, Any]]: @@ -135,13 +135,14 @@ async def test_access_update_profile( @pytest.mark.parametrize("user_role", [UserRole.USER]) async def test_get_profile_user_not_in_support_group( + support_group_before_app_starts: dict[str, Any], + # after app starts because it modifies the product user_role: UserRole, logged_user: UserInfoDict, client: TestClient, primary_group: dict[str, Any], standard_groups: list[dict[str, Any]], all_group: dict[str, str], - support_group: dict[str, Any], ): assert client.app @@ -176,8 +177,11 @@ async def test_get_profile_user_not_in_support_group( assert got_profile_groups["support"] support_group_id = got_profile_groups["support"]["gid"] - assert support_group_id == support_group["gid"] - assert got_profile_groups["support"]["description"] == support_group["description"] + assert support_group_id == support_group_before_app_starts["gid"] + assert ( + got_profile_groups["support"]["description"] + == support_group_before_app_starts["description"] + ) assert "accessRights" not in got_profile_groups["support"] # standard groups with at least read access @@ -196,16 +200,16 @@ async def test_get_profile_user_not_in_support_group( ) -@pytest.mark.test @pytest.mark.parametrize("user_role", [UserRole.USER]) async def test_get_profile_user_in_support_group( + support_group_before_app_starts: dict[str, Any], + # after app starts because it modifies the product user_role: UserRole, logged_user: UserInfoDict, client: TestClient, primary_group: dict[str, Any], standard_groups: list[dict[str, Any]], all_group: dict[str, str], - support_group: dict[str, Any], ): assert client.app from simcore_service_webserver.groups import _groups_repository @@ -213,7 +217,7 @@ async def test_get_profile_user_in_support_group( # Now add user to support group with read-only access await _groups_repository.add_new_user_in_group( client.app, - group_id=support_group["gid"], + group_id=support_group_before_app_starts["gid"], new_user_id=logged_user["id"], access_rights=AccessRightsDict(read=True, write=False, delete=False), ) @@ -249,8 +253,11 @@ async def test_get_profile_user_in_support_group( assert got_profile_groups["support"] support_group_id = got_profile_groups["support"]["gid"] - assert support_group_id == support_group["gid"] - assert got_profile_groups["support"]["description"] == support_group["description"] + assert support_group_id == support_group_before_app_starts["gid"] + assert ( + got_profile_groups["support"]["description"] + == support_group_before_app_starts["description"] + ) assert "accessRights" not in got_profile_groups["support"] # When user is part of support group, it should appear in standard groups @@ -259,8 +266,8 @@ async def test_get_profile_user_in_support_group( *standard_groups, { "gid": support_group_id, - "label": support_group["name"], - "description": support_group["description"], + "label": support_group_before_app_starts["name"], + "description": support_group_before_app_starts["description"], "thumbnail": None, "accessRights": {"read": True, "write": False, "delete": False}, }, From 12dff6ca4171bb6bc70d8c3b0fc0f559ace34ab6 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 3 Sep 2025 19:50:58 +0200 Subject: [PATCH 16/43] fix tests --- .../server/tests/unit/isolated/products/test_products_model.py | 1 + 1 file changed, 1 insertion(+) diff --git a/services/web/server/tests/unit/isolated/products/test_products_model.py b/services/web/server/tests/unit/isolated/products/test_products_model.py index bac41a1b3330..7eff7a461992 100644 --- a/services/web/server/tests/unit/isolated/products/test_products_model.py +++ b/services/web/server/tests/unit/isolated/products/test_products_model.py @@ -61,6 +61,7 @@ def test_product_to_static(): assert product_data == { "displayName": "o²S²PARC FOO", "supportEmail": "foo@osparcf.io", + "supportStandardGroupId": 67890, "vendor": { "copyright": "© ACME correcaminos", "name": "ACME", From 7e8aa54098ec688be3234ebc88e25c70d418d2c2 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 3 Sep 2025 20:01:09 +0200 Subject: [PATCH 17/43] fixes pylint --- .../api_schemas_webserver/groups.py | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/packages/models-library/src/models_library/api_schemas_webserver/groups.py b/packages/models-library/src/models_library/api_schemas_webserver/groups.py index 0694283df9a8..09aa879b0b32 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/groups.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/groups.py @@ -72,8 +72,8 @@ def _sanitize_legacy_data(cls, v): return None @classmethod - def _extract_basic_group_data(cls, group: Group) -> dict: - """Extract common group data for schema conversion""" + def dump_basic_group_data(cls, group: Group) -> dict: + """Helper function to extract common group data for schema conversion""" return remap_keys( group.model_dump( include={ @@ -93,11 +93,6 @@ def _extract_basic_group_data(cls, group: Group) -> dict: }, ) - @classmethod - def from_domain_model(cls, group: Group) -> Self: - # Adapts these domain models into this schema - return cls.model_validate(cls._extract_basic_group_data(group)) - class GroupGet(BasicGroupGet): access_rights: Annotated[GroupAccessRights, Field(alias="accessRights")] @@ -116,7 +111,7 @@ def from_domain_model(cls, group: Group, access_rights: AccessRightsDict) -> Sel # Adapts these domain models into this schema return cls.model_validate( { - **cls._extract_basic_group_data(group), + **cls.dump_basic_group_data(group), "access_rights": access_rights, } ) @@ -259,7 +254,7 @@ def from_domain_model( cls, groups_by_type: GroupsByTypeTuple, my_product_group: tuple[Group, AccessRightsDict] | None, - my_support_group: Group | None, + product_support_group: Group | None, ) -> Self: assert groups_by_type.primary # nosec assert groups_by_type.everyone # nosec @@ -276,8 +271,10 @@ def from_domain_model( else None ), support=( - BasicGroupGet.from_domain_model(my_support_group) - if my_support_group + BasicGroupGet.model_validate( + BasicGroupGet.dump_basic_group_data(product_support_group) + ) + if product_support_group else None ), ) From eb57743d802942166d460392687fa058ea1e2ca1 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 3 Sep 2025 20:20:49 +0200 Subject: [PATCH 18/43] updates oas --- .../api/v0/openapi.yaml | 43 +++++++++++++++---- 1 file changed, 34 insertions(+), 9 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml index 963b5b65693f..8b9f42e95c35 100644 --- a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml +++ b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml @@ -9284,6 +9284,35 @@ components: - name - email title: Author + BasicGroupGet: + properties: + gid: + type: integer + exclusiveMinimum: true + title: Gid + description: the group's unique ID + minimum: 0 + label: + type: string + title: Label + description: the group's display name + description: + type: string + title: Description + thumbnail: + anyOf: + - type: string + minLength: 1 + format: uri + - type: 'null' + title: Thumbnail + description: a link to the group's thumbnail + type: object + required: + - gid + - label + - description + title: BasicGroupGet Body_service_submission: properties: file: @@ -12881,16 +12910,15 @@ components: type: integer exclusiveMinimum: true title: Gid - description: the group ID + description: the group's unique ID minimum: 0 label: type: string title: Label - description: the group name + description: the group's display name description: type: string title: Description - description: the group description thumbnail: anyOf: - type: string @@ -12898,7 +12926,7 @@ components: format: uri - type: 'null' title: Thumbnail - description: url to the group thumbnail + description: a link to the group's thumbnail accessRights: $ref: '#/components/schemas/GroupAccessRights' inclusionRules: @@ -13607,7 +13635,7 @@ components: - type: 'null' support: anyOf: - - $ref: '#/components/schemas/GroupGet' + - $ref: '#/components/schemas/BasicGroupGet' - type: 'null' description: Group ID of the app support team or None if no support is defined for this product @@ -13650,13 +13678,10 @@ components: gid: '16' label: Blue Fundation support: - accessRights: - delete: false - read: false - write: false description: The support team of the application gid: '2' label: Support Team + thumbnail: https://placekitten.com/15/15 MyPermissionGet: properties: name: From db375c7717b3f7f580dc731c3c367ccc968f8fc7 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 3 Sep 2025 10:54:29 +0200 Subject: [PATCH 19/43] more group examples --- .../api_schemas_webserver/groups.py | 18 +++++ .../src/models_library/groups.py | 73 ++++++++++--------- packages/models-library/tests/test_users.py | 24 +++++- 3 files changed, 76 insertions(+), 39 deletions(-) diff --git a/packages/models-library/src/models_library/api_schemas_webserver/groups.py b/packages/models-library/src/models_library/api_schemas_webserver/groups.py index 643c66b817a7..092dfe85c641 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/groups.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/groups.py @@ -187,6 +187,12 @@ class MyGroupsGet(OutputSchema): organizations: list[GroupGet] | None = None all: GroupGet product: GroupGet | None = None + support: Annotated[ + GroupGet | None, + Field( + description="Group ID of the app support team or None if no support is defined for this product" + ), + ] = None model_config = ConfigDict( json_schema_extra={ @@ -225,6 +231,12 @@ class MyGroupsGet(OutputSchema): "description": "Open to all users", "accessRights": {"read": True, "write": False, "delete": False}, }, + "support": { + "gid": "2", + "label": "Support Team", + "description": "The support team of the application", + "accessRights": {"read": False, "write": False, "delete": False}, + }, } } ) @@ -234,6 +246,7 @@ def from_domain_model( cls, groups_by_type: GroupsByTypeTuple, my_product_group: tuple[Group, AccessRightsDict] | None, + my_support_group: tuple[Group, AccessRightsDict] | None = None, ) -> Self: assert groups_by_type.primary # nosec assert groups_by_type.everyone # nosec @@ -249,6 +262,11 @@ def from_domain_model( if my_product_group else None ), + support=( + GroupGet.from_domain_model(*my_support_group) + if my_support_group + else None + ), ) diff --git a/packages/models-library/src/models_library/groups.py b/packages/models-library/src/models_library/groups.py index d35b1de7dcca..7e13237b78c4 100644 --- a/packages/models-library/src/models_library/groups.py +++ b/packages/models-library/src/models_library/groups.py @@ -39,41 +39,44 @@ class Group(BaseModel): @staticmethod def _update_json_schema_extra(schema: JsonDict) -> None: - schema.update( - { - "examples": [ - { - "gid": 1, - "name": "Everyone", - "type": "everyone", - "description": "all users", - "thumbnail": None, - }, - { - "gid": 2, - "name": "User", - "description": "primary group", - "type": "primary", - "thumbnail": None, - }, - { - "gid": 3, - "name": "Organization", - "description": "standard group", - "type": "standard", - "thumbnail": None, - "inclusionRules": {}, - }, - { - "gid": 4, - "name": "Product", - "description": "standard group for products", - "type": "standard", - "thumbnail": None, - }, - ] - } - ) + everyone = { + "gid": 1, + "name": "Everyone", + "type": "everyone", + "description": "all users", + "thumbnail": None, + } + user = { + "gid": 2, + "name": "User", + "description": "primary group", + "type": "primary", + "thumbnail": None, + } + organization = { + "gid": 3, + "name": "Organization", + "description": "standard group", + "type": "standard", + "thumbnail": None, + "inclusionRules": {}, + } + product = { + "gid": 4, + "name": "Product", + "description": "standard group for products", + "type": "standard", + "thumbnail": None, + } + support = { + "gid": 5, + "name": "Support", + "description": "support group", + "type": "standard", + "thumbnail": None, + } + + schema.update({"examples": [everyone, user, organization, product, support]}) model_config = ConfigDict( populate_by_name=True, json_schema_extra=_update_json_schema_extra diff --git a/packages/models-library/tests/test_users.py b/packages/models-library/tests/test_users.py index 47678a2ad7ce..598e9074c6a7 100644 --- a/packages/models-library/tests/test_users.py +++ b/packages/models-library/tests/test_users.py @@ -1,3 +1,4 @@ +import pytest from models_library.api_schemas_webserver.users import ( MyProfileRestGet, ) @@ -7,7 +8,11 @@ from pydantic import TypeAdapter -def test_adapter_from_model_to_schema(): +@pytest.mark.parametrize("with_support_group", [True, False]) +@pytest.mark.parametrize("with_standard_groups", [True, False]) +def test_adapter_from_model_to_schema( + with_support_group: bool, with_standard_groups: bool +): my_profile = MyProfile.model_validate(MyProfile.model_json_schema()["example"]) groups = TypeAdapter(list[Group]).validate_python( @@ -17,13 +22,24 @@ def test_adapter_from_model_to_schema(): ar = AccessRightsDict(read=False, write=False, delete=False) my_groups_by_type = GroupsByTypeTuple( - primary=(groups[1], ar), standard=[(groups[2], ar)], everyone=(groups[0], ar) + primary=(groups[1], ar), + standard=[(groups[2], ar)] if with_standard_groups else [], + everyone=(groups[0], ar), ) - my_product_group = groups[-1], AccessRightsDict( + my_product_group = groups[3], AccessRightsDict( read=False, write=False, delete=False ) + + my_support_group = groups[4], AccessRightsDict( + read=False, write=False, delete=False + ) + my_preferences = {"foo": Preference(default_value=3, value=1)} MyProfileRestGet.from_domain_model( - my_profile, my_groups_by_type, my_product_group, my_preferences + my_profile, + my_groups_by_type, + my_product_group, + my_preferences, + my_support_group if with_support_group else None, ) From 0b122630e334272b0b864f8f1232ba876d0c3071 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 3 Sep 2025 11:37:06 +0200 Subject: [PATCH 20/43] adds support to groups --- .../api_schemas_webserver/groups.py | 2 +- .../api_schemas_webserver/users.py | 5 +++- .../src/models_library/groups.py | 22 ++++++++++---- .../groups/_groups_repository.py | 30 +++++++++---------- .../groups/_groups_rest.py | 26 ++++++++-------- .../groups/_groups_service.py | 21 +++++++++++-- .../simcore_service_webserver/groups/api.py | 4 ++- .../users/_controller/rest/users_rest.py | 16 ++++++++-- .../with_dbs/03/test_users_rest_profiles.py | 5 ++++ 9 files changed, 89 insertions(+), 42 deletions(-) diff --git a/packages/models-library/src/models_library/api_schemas_webserver/groups.py b/packages/models-library/src/models_library/api_schemas_webserver/groups.py index 092dfe85c641..505d06e7fb40 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/groups.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/groups.py @@ -246,7 +246,7 @@ def from_domain_model( cls, groups_by_type: GroupsByTypeTuple, my_product_group: tuple[Group, AccessRightsDict] | None, - my_support_group: tuple[Group, AccessRightsDict] | None = None, + my_support_group: tuple[Group, AccessRightsDict] | None, ) -> Self: assert groups_by_type.primary # nosec assert groups_by_type.everyone # nosec diff --git a/packages/models-library/src/models_library/api_schemas_webserver/users.py b/packages/models-library/src/models_library/api_schemas_webserver/users.py index 7b07616b73f2..f9ff1821fc7f 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/users.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/users.py @@ -132,6 +132,7 @@ def from_domain_model( my_groups_by_type: GroupsByTypeTuple, my_product_group: tuple[Group, AccessRightsDict] | None, my_preferences: AggregatedPreferences, + my_support_group: tuple[Group, AccessRightsDict] | None, ) -> Self: data = remap_keys( my_profile.model_dump( @@ -152,7 +153,9 @@ def from_domain_model( ) return cls( **data, - groups=MyGroupsGet.from_domain_model(my_groups_by_type, my_product_group), + groups=MyGroupsGet.from_domain_model( + my_groups_by_type, my_product_group, my_support_group + ), preferences=my_preferences, ) diff --git a/packages/models-library/src/models_library/groups.py b/packages/models-library/src/models_library/groups.py index 7e13237b78c4..cbd35847b549 100644 --- a/packages/models-library/src/models_library/groups.py +++ b/packages/models-library/src/models_library/groups.py @@ -39,21 +39,21 @@ class Group(BaseModel): @staticmethod def _update_json_schema_extra(schema: JsonDict) -> None: - everyone = { + everyone: JsonDict = { "gid": 1, "name": "Everyone", "type": "everyone", "description": "all users", "thumbnail": None, } - user = { + user: JsonDict = { "gid": 2, "name": "User", "description": "primary group", "type": "primary", "thumbnail": None, } - organization = { + organization: JsonDict = { "gid": 3, "name": "Organization", "description": "standard group", @@ -61,14 +61,14 @@ def _update_json_schema_extra(schema: JsonDict) -> None: "thumbnail": None, "inclusionRules": {}, } - product = { + product: JsonDict = { "gid": 4, "name": "Product", "description": "standard group for products", "type": "standard", "thumbnail": None, } - support = { + support: JsonDict = { "gid": 5, "name": "Support", "description": "support group", @@ -76,7 +76,17 @@ def _update_json_schema_extra(schema: JsonDict) -> None: "thumbnail": None, } - schema.update({"examples": [everyone, user, organization, product, support]}) + schema.update( + { + "examples": [ + everyone, + user, + organization, + product, + support, + ] + } + ) model_config = ConfigDict( populate_by_name=True, json_schema_extra=_update_json_schema_extra diff --git a/services/web/server/src/simcore_service_webserver/groups/_groups_repository.py b/services/web/server/src/simcore_service_webserver/groups/_groups_repository.py index 05bed6c18d68..eecb008b4556 100644 --- a/services/web/server/src/simcore_service_webserver/groups/_groups_repository.py +++ b/services/web/server/src/simcore_service_webserver/groups/_groups_repository.py @@ -108,7 +108,7 @@ async def _get_group_and_access_rights_or_raise( *, caller_id: UserID, group_id: GroupID, - permission: Literal["read", "write", "delete"] | None, + check_permission: Literal["read", "write", "delete"] | None, ) -> Row: result = await conn.execute( sa.select( @@ -122,8 +122,8 @@ async def _get_group_and_access_rights_or_raise( if not row: raise GroupNotFoundError(gid=group_id) - if permission: - _check_group_permissions(row, caller_id, group_id, permission) + if check_permission: + _check_group_permissions(row, caller_id, group_id, check_permission) return row @@ -274,29 +274,29 @@ async def get_user_group( """ async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: row = await _get_group_and_access_rights_or_raise( - conn, caller_id=user_id, group_id=group_id, permission="read" + conn, caller_id=user_id, group_id=group_id, check_permission="read" ) group, access_rights = _to_group_info_tuple(row) return group, access_rights -async def get_product_group_for_user( +async def get_any_group_for_user( app: web.Application, connection: AsyncConnection | None = None, *, user_id: UserID, - product_gid: GroupID, + group_gid: GroupID, ) -> tuple[Group, AccessRightsDict]: """ - Returns product's group if user belongs to it, otherwise it + Returns any group if user belongs to it (even if it has no permissions), otherwise it raises GroupNotFoundError """ async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: row = await _get_group_and_access_rights_or_raise( conn, caller_id=user_id, - group_id=product_gid, - permission=None, + group_id=group_gid, + check_permission=None, ) group, access_rights = _to_group_info_tuple(row) return group, access_rights @@ -363,7 +363,7 @@ async def update_standard_group( async with transaction_context(get_asyncpg_engine(app), connection) as conn: row = await _get_group_and_access_rights_or_raise( - conn, caller_id=user_id, group_id=group_id, permission="write" + conn, caller_id=user_id, group_id=group_id, check_permission="write" ) assert row.gid == group_id # nosec # NOTE: update does not include access-rights @@ -391,7 +391,7 @@ async def delete_standard_group( ) -> None: async with transaction_context(get_asyncpg_engine(app), connection) as conn: await _get_group_and_access_rights_or_raise( - conn, caller_id=user_id, group_id=group_id, permission="delete" + conn, caller_id=user_id, group_id=group_id, check_permission="delete" ) await conn.execute( @@ -581,7 +581,7 @@ async def get_user_in_group( async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: # first check if the group exists await _get_group_and_access_rights_or_raise( - conn, caller_id=caller_id, group_id=group_id, permission="read" + conn, caller_id=caller_id, group_id=group_id, check_permission="read" ) # get the user with its permissions @@ -611,7 +611,7 @@ async def update_user_in_group( # first check if the group exists await _get_group_and_access_rights_or_raise( - conn, caller_id=caller_id, group_id=group_id, permission="write" + conn, caller_id=caller_id, group_id=group_id, check_permission="write" ) # now check the user exists @@ -651,7 +651,7 @@ async def delete_user_from_group( async with transaction_context(get_asyncpg_engine(app), connection) as conn: # first check if the group exists await _get_group_and_access_rights_or_raise( - conn, caller_id=caller_id, group_id=group_id, permission="write" + conn, caller_id=caller_id, group_id=group_id, check_permission="write" ) # check the user exists @@ -714,7 +714,7 @@ async def add_new_user_in_group( async with transaction_context(get_asyncpg_engine(app), connection) as conn: # first check if the group exists await _get_group_and_access_rights_or_raise( - conn, caller_id=caller_id, group_id=group_id, permission="write" + conn, caller_id=caller_id, group_id=group_id, check_permission="write" ) query = sa.select(users.c.id) diff --git a/services/web/server/src/simcore_service_webserver/groups/_groups_rest.py b/services/web/server/src/simcore_service_webserver/groups/_groups_rest.py index 07a0b5026d48..806fa60b0b64 100644 --- a/services/web/server/src/simcore_service_webserver/groups/_groups_rest.py +++ b/services/web/server/src/simcore_service_webserver/groups/_groups_rest.py @@ -56,8 +56,6 @@ async def list_groups(request: web.Request): assert groups_by_type.primary assert groups_by_type.everyone - my_product_group = None - if product.group_id: with suppress(GroupNotFoundError): # Product is optional @@ -66,16 +64,20 @@ async def list_groups(request: web.Request): user_id=req_ctx.user_id, product_gid=product.group_id, ) - - my_groups = MyGroupsGet( - me=GroupGet.from_domain_model(*groups_by_type.primary), - organizations=[ - GroupGet.from_domain_model(*gi) for gi in groups_by_type.standard - ], - all=GroupGet.from_domain_model(*groups_by_type.everyone), - product=( - GroupGet.from_domain_model(*my_product_group) if my_product_group else None - ), + else: + my_product_group = None + + if product.support_standard_group_id: + my_support_group = await _groups_service.get_support_group_for_user_or_none( + app=request.app, + user_id=req_ctx.user_id, + support_gid=product.support_standard_group_id, + ) + else: + my_support_group = None + + my_groups = MyGroupsGet.from_domain_model( + groups_by_type, my_product_group, my_support_group ) return envelope_json_response(my_groups) diff --git a/services/web/server/src/simcore_service_webserver/groups/_groups_service.py b/services/web/server/src/simcore_service_webserver/groups/_groups_service.py index 67ff9d1dfafa..c28b7d9d2e34 100644 --- a/services/web/server/src/simcore_service_webserver/groups/_groups_service.py +++ b/services/web/server/src/simcore_service_webserver/groups/_groups_service.py @@ -1,3 +1,5 @@ +from contextlib import suppress + from aiohttp import web from models_library.basic_types import IDStr from models_library.emails import LowerCaseEmailStr @@ -16,7 +18,7 @@ from ..users import users_service from . import _groups_repository -from .exceptions import GroupsError +from .exceptions import GroupNotFoundError, GroupsError # # GROUPS @@ -71,11 +73,24 @@ async def get_product_group_for_user( Returns product's group if user belongs to it, otherwise it raises GroupNotFoundError """ - return await _groups_repository.get_product_group_for_user( - app, user_id=user_id, product_gid=product_gid + return await _groups_repository.get_any_group_for_user( + app, user_id=user_id, group_gid=product_gid ) +async def get_support_group_for_user_or_none( + app: web.Application, *, user_id: UserID, support_gid: GroupID +) -> tuple[Group, AccessRightsDict] | None: + """ + Returns support's group if user belongs to it, otherwise it + """ + with suppress(GroupNotFoundError): + return await _groups_repository.get_any_group_for_user( + app, user_id=user_id, group_gid=support_gid + ) + return None + + # # CRUD operations on groups linked to a user # diff --git a/services/web/server/src/simcore_service_webserver/groups/api.py b/services/web/server/src/simcore_service_webserver/groups/api.py index dfe60c5866f9..68f14c239baf 100644 --- a/services/web/server/src/simcore_service_webserver/groups/api.py +++ b/services/web/server/src/simcore_service_webserver/groups/api.py @@ -7,6 +7,7 @@ auto_add_user_to_product_group, get_group_from_gid, get_product_group_for_user, + get_support_group_for_user_or_none, is_user_by_email_in_group, list_all_user_groups_ids, list_group_members, @@ -20,10 +21,11 @@ "auto_add_user_to_product_group", "get_group_from_gid", "get_product_group_for_user", + "get_support_group_for_user_or_none", "is_user_by_email_in_group", "list_all_user_groups_ids", + "list_group_members", "list_user_groups_ids_with_read_access", "list_user_groups_with_read_access", - "list_group_members", # nopycln: file ) diff --git a/services/web/server/src/simcore_service_webserver/users/_controller/rest/users_rest.py b/services/web/server/src/simcore_service_webserver/users/_controller/rest/users_rest.py index c242fb5e1ebb..0fd831d20500 100644 --- a/services/web/server/src/simcore_service_webserver/users/_controller/rest/users_rest.py +++ b/services/web/server/src/simcore_service_webserver/users/_controller/rest/users_rest.py @@ -58,8 +58,6 @@ async def get_my_profile(request: web.Request) -> web.Response: assert groups_by_type.primary assert groups_by_type.everyone - my_product_group = None - if product.group_id: with suppress(GroupNotFoundError): # Product is optional @@ -68,13 +66,25 @@ async def get_my_profile(request: web.Request) -> web.Response: user_id=req_ctx.user_id, product_gid=product.group_id, ) + else: + my_product_group = None + + if product.support_standard_group_id: + # Support group is optional + my_support_group = await groups_service.get_support_group_for_user_or_none( + app=request.app, + user_id=req_ctx.user_id, + support_gid=product.support_standard_group_id, + ) + else: + my_support_group = None my_profile, preferences = await _users_service.get_my_profile( request.app, user_id=req_ctx.user_id, product_name=req_ctx.product_name ) profile = MyProfileRestGet.from_domain_model( - my_profile, groups_by_type, my_product_group, preferences + my_profile, groups_by_type, my_product_group, preferences, my_support_group ) return envelope_json_response(profile) diff --git a/services/web/server/tests/unit/with_dbs/03/test_users_rest_profiles.py b/services/web/server/tests/unit/with_dbs/03/test_users_rest_profiles.py index ab7c0fb6f3a5..8bc3bd716ec5 100644 --- a/services/web/server/tests/unit/with_dbs/03/test_users_rest_profiles.py +++ b/services/web/server/tests/unit/with_dbs/03/test_users_rest_profiles.py @@ -88,6 +88,7 @@ async def private_user( "first_name": partial_first_name, "last_name": "Bond", "email": f"james{partial_email}", + # Maximum privacy "privacy_hide_username": True, "privacy_hide_email": True, "privacy_hide_fullname": True, @@ -108,6 +109,7 @@ async def semi_private_user( "first_name": partial_first_name, "last_name": "Maxwell", "email": "j@maxwell.me", + # Medium privacy "privacy_hide_username": False, "privacy_hide_email": True, "privacy_hide_fullname": False, # <-- @@ -128,6 +130,7 @@ async def public_user( "first_name": "Taylor", "last_name": "Swift", "email": f"taylor{partial_email}", + # Fully public "privacy_hide_username": False, "privacy_hide_email": False, "privacy_hide_fullname": False, @@ -407,6 +410,8 @@ async def test_get_profile( "thumbnail": None, } + assert got_profile_groups["support"] is None + sorted_by_group_id = functools.partial(sorted, key=lambda d: d["gid"]) assert sorted_by_group_id( got_profile_groups["organizations"] From 91874b03df42f7b236537cd0efc4377449f769e8 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 3 Sep 2025 11:38:27 +0200 Subject: [PATCH 21/43] updates OAS --- .../simcore_service_webserver/api/v0/openapi.yaml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml index c4bef8dbf213..c64564647d3b 100644 --- a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml +++ b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml @@ -13605,6 +13605,12 @@ components: anyOf: - $ref: '#/components/schemas/GroupGet' - type: 'null' + support: + anyOf: + - $ref: '#/components/schemas/GroupGet' + - type: 'null' + description: Group ID of the app support team or None if no support is defined + for this product type: object required: - me @@ -13643,6 +13649,14 @@ components: description: Some foundation gid: '16' label: Blue Fundation + support: + accessRights: + delete: false + read: false + write: false + description: The support team of the application + gid: '2' + label: Support Team MyPermissionGet: properties: name: From 6443f69b360505ff79f59599e7b3f974d531348c Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 3 Sep 2025 11:38:29 +0200 Subject: [PATCH 22/43] =?UTF-8?q?services/webserver=20api=20version:=200.7?= =?UTF-8?q?5.0=20=E2=86=92=200.76.0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- services/web/server/VERSION | 2 +- services/web/server/setup.cfg | 2 +- .../server/src/simcore_service_webserver/api/v0/openapi.yaml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/services/web/server/VERSION b/services/web/server/VERSION index c52842c467cf..62df9f538d84 100644 --- a/services/web/server/VERSION +++ b/services/web/server/VERSION @@ -1 +1 @@ -0.75.0 +0.76.0 diff --git a/services/web/server/setup.cfg b/services/web/server/setup.cfg index 22e2a9f3d1ea..e1bef4a8a5a1 100644 --- a/services/web/server/setup.cfg +++ b/services/web/server/setup.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 0.75.0 +current_version = 0.76.0 commit = True message = services/webserver api version: {current_version} → {new_version} tag = False diff --git a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml index c64564647d3b..963b5b65693f 100644 --- a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml +++ b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml @@ -2,7 +2,7 @@ openapi: 3.1.0 info: title: simcore-service-webserver description: Main service with an interface (http-API & websockets) to the web front-end - version: 0.75.0 + version: 0.76.0 servers: - url: '' description: webserver From f8fa043beb204b2f3e3235067e6b8e33e2073a9e Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 3 Sep 2025 16:51:02 +0200 Subject: [PATCH 23/43] splits tests --- .../src/pytest_simcore/faker_products_data.py | 24 +- .../pytest_simcore/helpers/faker_factories.py | 2 + .../unit/with_dbs/03/test_users_rest_phone.py | 494 ++++++++++++++++++ ...profiles.py => test_users_rest_profile.py} | 459 ---------------- 4 files changed, 515 insertions(+), 464 deletions(-) create mode 100644 services/web/server/tests/unit/with_dbs/03/test_users_rest_phone.py rename services/web/server/tests/unit/with_dbs/03/{test_users_rest_profiles.py => test_users_rest_profile.py} (59%) diff --git a/packages/pytest-simcore/src/pytest_simcore/faker_products_data.py b/packages/pytest-simcore/src/pytest_simcore/faker_products_data.py index e55c1e489f09..e91f21ca7e6b 100644 --- a/packages/pytest-simcore/src/pytest_simcore/faker_products_data.py +++ b/packages/pytest-simcore/src/pytest_simcore/faker_products_data.py @@ -4,9 +4,9 @@ # pylint: disable=unused-argument # pylint: disable=unused-variable """ - Fixtures to produce fake data for a product: - - it is self-consistent - - granular customization by overriding fixtures +Fixtures to produce fake data for a product: + - it is self-consistent + - granular customization by overriding fixtures """ from typing import Any @@ -65,11 +65,25 @@ def bcc_email(request: pytest.FixtureRequest, product_name: ProductName) -> Emai ) +@pytest.fixture +def support_standard_group_id(faker: Faker) -> int | None: + # NOTE: override to change + return None + + @pytest.fixture def product( - faker: Faker, product_name: ProductName, support_email: EmailStr + faker: Faker, + product_name: ProductName, + support_email: EmailStr, + support_standard_group_id: int | None, ) -> dict[str, Any]: - return random_product(name=product_name, support_email=support_email, fake=faker) + return random_product( + name=product_name, + support_email=support_email, + support_standard_group_id=support_standard_group_id, + fake=faker, + ) @pytest.fixture diff --git a/packages/pytest-simcore/src/pytest_simcore/helpers/faker_factories.py b/packages/pytest-simcore/src/pytest_simcore/helpers/faker_factories.py index e2ff83fc53a3..7c50df70de5c 100644 --- a/packages/pytest-simcore/src/pytest_simcore/helpers/faker_factories.py +++ b/packages/pytest-simcore/src/pytest_simcore/helpers/faker_factories.py @@ -256,6 +256,7 @@ def fake_task(**overrides) -> dict[str, Any]: def random_product( *, group_id: int | None = None, + support_standard_group_id: int | None = None, registration_email_template: str | None = None, fake: Faker = DEFAULT_FAKER, **overrides, @@ -302,6 +303,7 @@ def random_product( "priority": fake.pyint(0, 10), "max_open_studies_per_user": fake.pyint(1, 10), "group_id": group_id, + "support_standard_group_id": support_standard_group_id, } if ui := fake.random_element( diff --git a/services/web/server/tests/unit/with_dbs/03/test_users_rest_phone.py b/services/web/server/tests/unit/with_dbs/03/test_users_rest_phone.py new file mode 100644 index 000000000000..c93af68cd925 --- /dev/null +++ b/services/web/server/tests/unit/with_dbs/03/test_users_rest_phone.py @@ -0,0 +1,494 @@ +# pylint: disable=protected-access +# pylint: disable=redefined-outer-name +# pylint: disable=too-many-arguments +# pylint: disable=too-many-statements +# pylint: disable=unused-argument +# pylint: disable=unused-variable + + +from http import HTTPStatus + +import pytest +from aiohttp.test_utils import TestClient +from common_library.users_enums import UserRole +from models_library.api_schemas_webserver.users import ( + MyProfileRestGet, +) +from pytest_simcore.helpers.assert_checks import assert_status +from pytest_simcore.helpers.monkeypatch_envs import EnvVarsDict, setenvs_from_dict +from pytest_simcore.helpers.webserver_users import UserInfoDict +from servicelib.aiohttp import status +from simcore_service_webserver.models import PhoneNumberStr +from simcore_service_webserver.users._controller.rest.users_rest import ( + _REGISTRATION_CODE_VALUE_FAKE, +) + + +@pytest.fixture +def app_environment( + app_environment: EnvVarsDict, monkeypatch: pytest.MonkeyPatch +) -> EnvVarsDict: + # disables GC and DB-listener + return app_environment | setenvs_from_dict( + monkeypatch, + { + "WEBSERVER_GARBAGE_COLLECTOR": "null", + "WEBSERVER_DB_LISTENER": "0", + "WEBSERVER_DEV_FEATURES_ENABLED": "1", # NOTE: still under development + }, + ) + + +@pytest.mark.parametrize("user_role", [UserRole.USER]) +async def test_phone_registration_basic_workflow( + user_role: UserRole, + logged_user: UserInfoDict, + client: TestClient, +): + assert client.app + + # GET initial profile + url = client.app.router["get_my_profile"].url_for() + resp = await client.get(f"{url}") + data, _ = await assert_status(resp, status.HTTP_200_OK) + + initial_profile = MyProfileRestGet.model_validate(data) + initial_phone = initial_profile.phone + assert initial_phone + + # REGISTER phone number + # Change the last 3 digits of the initial phone number to '999' + new_phone = f"{initial_phone[:-3]}999" + url = client.app.router["my_phone_register"].url_for() + resp = await client.post( + f"{url}", + json={ + "phone": new_phone, + }, + ) + await assert_status(resp, status.HTTP_202_ACCEPTED) + + # CONFIRM phone registration + url = client.app.router["my_phone_confirm"].url_for() + resp = await client.post( + f"{url}", + json={ + "code": _REGISTRATION_CODE_VALUE_FAKE, + }, + ) + await assert_status(resp, status.HTTP_204_NO_CONTENT) + + # GET updated profile + url = client.app.router["get_my_profile"].url_for() + resp = await client.get(f"{url}") + data, _ = await assert_status(resp, status.HTTP_200_OK) + + updated_profile = MyProfileRestGet.model_validate(data) + + # Verify phone was updated + assert updated_profile.phone == new_phone + assert updated_profile.phone != initial_phone + + # Verify other fields remained unchanged + assert updated_profile.first_name == initial_profile.first_name + assert updated_profile.last_name == initial_profile.last_name + assert updated_profile.login == initial_profile.login + assert updated_profile.user_name == initial_profile.user_name + + +@pytest.mark.parametrize("user_role", [UserRole.USER]) +async def test_phone_registration_workflow( + user_role: UserRole, + logged_user: UserInfoDict, + client: TestClient, +): + assert client.app + + # GET initial profile + url = client.app.router["get_my_profile"].url_for() + resp = await client.get(f"{url}") + data, _ = await assert_status(resp, status.HTTP_200_OK) + + initial_profile = MyProfileRestGet.model_validate(data) + initial_phone = initial_profile.phone + assert initial_phone + + # STEP 1: REGISTER phone number + new_phone = f"{initial_phone[:-3]}999" # Change the last 3 digits to '999' + url = client.app.router["my_phone_register"].url_for() + resp = await client.post( + f"{url}", + json={ + "phone": new_phone, + }, + ) + await assert_status(resp, status.HTTP_202_ACCEPTED) + + # STEP 2: CONFIRM phone registration + url = client.app.router["my_phone_confirm"].url_for() + resp = await client.post( + f"{url}", + json={ + "code": _REGISTRATION_CODE_VALUE_FAKE, + }, + ) + await assert_status(resp, status.HTTP_204_NO_CONTENT) + + # GET updated profile + url = client.app.router["get_my_profile"].url_for() + resp = await client.get(f"{url}") + data, _ = await assert_status(resp, status.HTTP_200_OK) + + updated_profile = MyProfileRestGet.model_validate(data) + + # Verify phone was updated + assert updated_profile.phone == new_phone + assert updated_profile.phone != initial_phone + + # Verify other fields remained unchanged + assert updated_profile.first_name == initial_profile.first_name + assert updated_profile.last_name == initial_profile.last_name + assert updated_profile.login == initial_profile.login + assert updated_profile.user_name == initial_profile.user_name + + +@pytest.mark.parametrize("user_role", [UserRole.USER]) +async def test_phone_registration_with_resend( + user_role: UserRole, + logged_user: UserInfoDict, + client: TestClient, + user_phone_number: PhoneNumberStr, +): + assert client.app + + # STEP 1: REGISTER phone number + new_phone = user_phone_number + url = client.app.router["my_phone_register"].url_for() + resp = await client.post( + f"{url}", + json={ + "phone": new_phone, + }, + ) + await assert_status(resp, status.HTTP_202_ACCEPTED) + + # STEP 2: RESEND code (optional step) + url = client.app.router["my_phone_resend"].url_for() + resp = await client.post(f"{url}") + await assert_status(resp, status.HTTP_202_ACCEPTED) + + # STEP 3: CONFIRM phone registration + url = client.app.router["my_phone_confirm"].url_for() + resp = await client.post( + f"{url}", + json={ + "code": _REGISTRATION_CODE_VALUE_FAKE, + }, + ) + await assert_status(resp, status.HTTP_204_NO_CONTENT) + + # GET updated profile + url = client.app.router["get_my_profile"].url_for() + resp = await client.get(f"{url}") + data, _ = await assert_status(resp, status.HTTP_200_OK) + + updated_profile = MyProfileRestGet.model_validate(data) + + # Verify phone was updated + assert updated_profile.phone == new_phone + + +@pytest.mark.parametrize("user_role", [UserRole.USER]) +async def test_phone_registration_change_existing_phone( + user_role: UserRole, + logged_user: UserInfoDict, + client: TestClient, + user_phone_number: PhoneNumberStr, +): + assert client.app + + # Set initial phone + first_phone = user_phone_number + url = client.app.router["my_phone_register"].url_for() + resp = await client.post( + f"{url}", + json={ + "phone": first_phone, + }, + ) + await assert_status(resp, status.HTTP_202_ACCEPTED) + + url = client.app.router["my_phone_confirm"].url_for() + resp = await client.post( + f"{url}", + json={ + "code": _REGISTRATION_CODE_VALUE_FAKE, + }, + ) + await assert_status(resp, status.HTTP_204_NO_CONTENT) + + # Change to new phone + # Create a different phone number by changing the last digits + new_phone = user_phone_number[:-4] + "9999" + url = client.app.router["my_phone_register"].url_for() + resp = await client.post( + f"{url}", + json={ + "phone": new_phone, + }, + ) + await assert_status(resp, status.HTTP_202_ACCEPTED) + + url = client.app.router["my_phone_confirm"].url_for() + resp = await client.post( + f"{url}", + json={ + "code": _REGISTRATION_CODE_VALUE_FAKE, + }, + ) + await assert_status(resp, status.HTTP_204_NO_CONTENT) + + # GET updated profile + url = client.app.router["get_my_profile"].url_for() + resp = await client.get(f"{url}") + data, _ = await assert_status(resp, status.HTTP_200_OK) + + updated_profile = MyProfileRestGet.model_validate(data) + + # Verify phone was updated to new phone + assert updated_profile.phone == new_phone + assert updated_profile.phone != first_phone + + +# +# PHONE REGISTRATION FAILURE TESTS +# + + +@pytest.mark.parametrize("user_role", [UserRole.USER]) +async def test_phone_resend_without_pending_registration( + user_role: UserRole, + logged_user: UserInfoDict, + client: TestClient, +): + assert client.app + + # Try to resend code without any pending registration + url = client.app.router["my_phone_resend"].url_for() + resp = await client.post(f"{url}") + await assert_status(resp, status.HTTP_400_BAD_REQUEST) + + +@pytest.mark.parametrize("user_role", [UserRole.USER]) +async def test_phone_confirm_without_pending_registration( + user_role: UserRole, + logged_user: UserInfoDict, + client: TestClient, +): + assert client.app + + # Try to confirm code without any pending registration + url = client.app.router["my_phone_confirm"].url_for() + resp = await client.post( + f"{url}", + json={ + "code": _REGISTRATION_CODE_VALUE_FAKE, + }, + ) + await assert_status(resp, status.HTTP_400_BAD_REQUEST) + + +@pytest.mark.parametrize("user_role", [UserRole.USER]) +async def test_phone_confirm_with_wrong_code( + user_role: UserRole, + logged_user: UserInfoDict, + client: TestClient, + user_phone_number: PhoneNumberStr, +): + assert client.app + + # STEP 1: REGISTER phone number + new_phone = user_phone_number + url = client.app.router["my_phone_register"].url_for() + resp = await client.post( + f"{url}", + json={ + "phone": new_phone, + }, + ) + await assert_status(resp, status.HTTP_202_ACCEPTED) + + # STEP 2: Try to confirm with wrong code + url = client.app.router["my_phone_confirm"].url_for() + resp = await client.post( + f"{url}", + json={ + "code": "wrongcode1234", + }, + ) + await assert_status(resp, status.HTTP_400_BAD_REQUEST) + + +@pytest.mark.parametrize("user_role", [UserRole.USER]) +async def test_phone_confirm_with_invalid_code_format( + user_role: UserRole, + logged_user: UserInfoDict, + client: TestClient, + user_phone_number: PhoneNumberStr, +): + assert client.app + + # STEP 1: REGISTER phone number + new_phone = user_phone_number + url = client.app.router["my_phone_register"].url_for() + resp = await client.post( + f"{url}", + json={ + "phone": new_phone, + }, + ) + await assert_status(resp, status.HTTP_202_ACCEPTED) + + # STEP 2: Try to confirm with invalid code format (contains special characters) + url = client.app.router["my_phone_confirm"].url_for() + resp = await client.post( + f"{url}", + json={ + "code": "123-456", # Invalid format according to pattern + }, + ) + await assert_status(resp, status.HTTP_422_UNPROCESSABLE_ENTITY) + + +@pytest.mark.parametrize("user_role", [UserRole.USER]) +async def test_phone_register_with_empty_phone( + user_role: UserRole, + logged_user: UserInfoDict, + client: TestClient, +): + assert client.app + + # Try to register with empty phone number + url = client.app.router["my_phone_register"].url_for() + resp = await client.post( + f"{url}", + json={ + "phone": "", # Empty phone number + }, + ) + await assert_status(resp, status.HTTP_422_UNPROCESSABLE_ENTITY) + + # Try to register with whitespace-only phone number + url = client.app.router["my_phone_register"].url_for() + resp = await client.post( + f"{url}", + json={ + "phone": " ", # Whitespace only + }, + ) + await assert_status(resp, status.HTTP_422_UNPROCESSABLE_ENTITY) + + +@pytest.mark.parametrize("user_role", [UserRole.USER]) +async def test_phone_confirm_with_empty_code( + user_role: UserRole, + logged_user: UserInfoDict, + client: TestClient, + user_phone_number: PhoneNumberStr, +): + assert client.app + + # STEP 1: REGISTER phone number + new_phone = user_phone_number + url = client.app.router["my_phone_register"].url_for() + resp = await client.post( + f"{url}", + json={ + "phone": new_phone, + }, + ) + await assert_status(resp, status.HTTP_202_ACCEPTED) + + # STEP 2: Try to confirm with empty code + url = client.app.router["my_phone_confirm"].url_for() + resp = await client.post( + f"{url}", + json={ + "code": "", # Empty code + }, + ) + await assert_status(resp, status.HTTP_422_UNPROCESSABLE_ENTITY) + + +@pytest.mark.parametrize( + "user_role,expected", + [ + (UserRole.ANONYMOUS, status.HTTP_401_UNAUTHORIZED), + (UserRole.GUEST, status.HTTP_403_FORBIDDEN), + ], +) +async def test_phone_register_access_rights( + user_role: UserRole, + logged_user: UserInfoDict, + client: TestClient, + expected: HTTPStatus, + user_phone_number: PhoneNumberStr, +): + assert client.app + + # Try to register phone with insufficient permissions + url = client.app.router["my_phone_register"].url_for() + resp = await client.post( + f"{url}", + json={ + "phone": user_phone_number, + }, + ) + await assert_status(resp, expected) + + +@pytest.mark.parametrize( + "user_role,expected", + [ + (UserRole.ANONYMOUS, status.HTTP_401_UNAUTHORIZED), + (UserRole.GUEST, status.HTTP_403_FORBIDDEN), + ], +) +async def test_phone_resend_access_rights( + user_role: UserRole, + logged_user: UserInfoDict, + client: TestClient, + expected: HTTPStatus, +): + assert client.app + + # Try to resend code with insufficient permissions + url = client.app.router["my_phone_resend"].url_for() + resp = await client.post(f"{url}") + await assert_status(resp, expected) + + +@pytest.mark.parametrize( + "user_role,expected", + [ + (UserRole.ANONYMOUS, status.HTTP_401_UNAUTHORIZED), + (UserRole.GUEST, status.HTTP_403_FORBIDDEN), + ], +) +async def test_phone_confirm_access_rights( + user_role: UserRole, + logged_user: UserInfoDict, + client: TestClient, + expected: HTTPStatus, +): + assert client.app + + # Try to confirm code with insufficient permissions + url = client.app.router["my_phone_confirm"].url_for() + resp = await client.post( + f"{url}", + json={ + "code": _REGISTRATION_CODE_VALUE_FAKE, + }, + ) + await assert_status(resp, expected) diff --git a/services/web/server/tests/unit/with_dbs/03/test_users_rest_profiles.py b/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py similarity index 59% rename from services/web/server/tests/unit/with_dbs/03/test_users_rest_profiles.py rename to services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py index 8bc3bd716ec5..6bd3e708eb68 100644 --- a/services/web/server/tests/unit/with_dbs/03/test_users_rest_profiles.py +++ b/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py @@ -32,13 +32,9 @@ from pytest_simcore.helpers.webserver_users import NewUser, UserInfoDict from servicelib.aiohttp import status from servicelib.rest_constants import RESPONSE_MODEL_POLICY -from simcore_service_webserver.models import PhoneNumberStr from simcore_service_webserver.user_preferences._service import ( get_frontend_user_preferences_aggregation, ) -from simcore_service_webserver.users._controller.rest.users_rest import ( - _REGISTRATION_CODE_VALUE_FAKE, -) from sqlalchemy.exc import OperationalError as SQLAlchemyOperationalError from sqlalchemy.ext.asyncio import AsyncConnection @@ -600,458 +596,3 @@ async def test_get_profile_with_failing_db_connection( data, error = await assert_status(resp, expected) assert not data assert error["message"] == "Authentication service is temporary unavailable" - - -@pytest.mark.parametrize("user_role", [UserRole.USER]) -async def test_phone_registration_basic_workflow( - user_role: UserRole, - logged_user: UserInfoDict, - client: TestClient, -): - assert client.app - - # GET initial profile - url = client.app.router["get_my_profile"].url_for() - resp = await client.get(f"{url}") - data, _ = await assert_status(resp, status.HTTP_200_OK) - - initial_profile = MyProfileRestGet.model_validate(data) - initial_phone = initial_profile.phone - assert initial_phone - - # REGISTER phone number - # Change the last 3 digits of the initial phone number to '999' - new_phone = f"{initial_phone[:-3]}999" - url = client.app.router["my_phone_register"].url_for() - resp = await client.post( - f"{url}", - json={ - "phone": new_phone, - }, - ) - await assert_status(resp, status.HTTP_202_ACCEPTED) - - # CONFIRM phone registration - url = client.app.router["my_phone_confirm"].url_for() - resp = await client.post( - f"{url}", - json={ - "code": _REGISTRATION_CODE_VALUE_FAKE, - }, - ) - await assert_status(resp, status.HTTP_204_NO_CONTENT) - - # GET updated profile - url = client.app.router["get_my_profile"].url_for() - resp = await client.get(f"{url}") - data, _ = await assert_status(resp, status.HTTP_200_OK) - - updated_profile = MyProfileRestGet.model_validate(data) - - # Verify phone was updated - assert updated_profile.phone == new_phone - assert updated_profile.phone != initial_phone - - # Verify other fields remained unchanged - assert updated_profile.first_name == initial_profile.first_name - assert updated_profile.last_name == initial_profile.last_name - assert updated_profile.login == initial_profile.login - assert updated_profile.user_name == initial_profile.user_name - - -@pytest.mark.parametrize("user_role", [UserRole.USER]) -async def test_phone_registration_workflow( - user_role: UserRole, - logged_user: UserInfoDict, - client: TestClient, -): - assert client.app - - # GET initial profile - url = client.app.router["get_my_profile"].url_for() - resp = await client.get(f"{url}") - data, _ = await assert_status(resp, status.HTTP_200_OK) - - initial_profile = MyProfileRestGet.model_validate(data) - initial_phone = initial_profile.phone - assert initial_phone - - # STEP 1: REGISTER phone number - new_phone = f"{initial_phone[:-3]}999" # Change the last 3 digits to '999' - url = client.app.router["my_phone_register"].url_for() - resp = await client.post( - f"{url}", - json={ - "phone": new_phone, - }, - ) - await assert_status(resp, status.HTTP_202_ACCEPTED) - - # STEP 2: CONFIRM phone registration - url = client.app.router["my_phone_confirm"].url_for() - resp = await client.post( - f"{url}", - json={ - "code": _REGISTRATION_CODE_VALUE_FAKE, - }, - ) - await assert_status(resp, status.HTTP_204_NO_CONTENT) - - # GET updated profile - url = client.app.router["get_my_profile"].url_for() - resp = await client.get(f"{url}") - data, _ = await assert_status(resp, status.HTTP_200_OK) - - updated_profile = MyProfileRestGet.model_validate(data) - - # Verify phone was updated - assert updated_profile.phone == new_phone - assert updated_profile.phone != initial_phone - - # Verify other fields remained unchanged - assert updated_profile.first_name == initial_profile.first_name - assert updated_profile.last_name == initial_profile.last_name - assert updated_profile.login == initial_profile.login - assert updated_profile.user_name == initial_profile.user_name - - -@pytest.mark.parametrize("user_role", [UserRole.USER]) -async def test_phone_registration_with_resend( - user_role: UserRole, - logged_user: UserInfoDict, - client: TestClient, - user_phone_number: PhoneNumberStr, -): - assert client.app - - # STEP 1: REGISTER phone number - new_phone = user_phone_number - url = client.app.router["my_phone_register"].url_for() - resp = await client.post( - f"{url}", - json={ - "phone": new_phone, - }, - ) - await assert_status(resp, status.HTTP_202_ACCEPTED) - - # STEP 2: RESEND code (optional step) - url = client.app.router["my_phone_resend"].url_for() - resp = await client.post(f"{url}") - await assert_status(resp, status.HTTP_202_ACCEPTED) - - # STEP 3: CONFIRM phone registration - url = client.app.router["my_phone_confirm"].url_for() - resp = await client.post( - f"{url}", - json={ - "code": _REGISTRATION_CODE_VALUE_FAKE, - }, - ) - await assert_status(resp, status.HTTP_204_NO_CONTENT) - - # GET updated profile - url = client.app.router["get_my_profile"].url_for() - resp = await client.get(f"{url}") - data, _ = await assert_status(resp, status.HTTP_200_OK) - - updated_profile = MyProfileRestGet.model_validate(data) - - # Verify phone was updated - assert updated_profile.phone == new_phone - - -@pytest.mark.parametrize("user_role", [UserRole.USER]) -async def test_phone_registration_change_existing_phone( - user_role: UserRole, - logged_user: UserInfoDict, - client: TestClient, - user_phone_number: PhoneNumberStr, -): - assert client.app - - # Set initial phone - first_phone = user_phone_number - url = client.app.router["my_phone_register"].url_for() - resp = await client.post( - f"{url}", - json={ - "phone": first_phone, - }, - ) - await assert_status(resp, status.HTTP_202_ACCEPTED) - - url = client.app.router["my_phone_confirm"].url_for() - resp = await client.post( - f"{url}", - json={ - "code": _REGISTRATION_CODE_VALUE_FAKE, - }, - ) - await assert_status(resp, status.HTTP_204_NO_CONTENT) - - # Change to new phone - # Create a different phone number by changing the last digits - new_phone = user_phone_number[:-4] + "9999" - url = client.app.router["my_phone_register"].url_for() - resp = await client.post( - f"{url}", - json={ - "phone": new_phone, - }, - ) - await assert_status(resp, status.HTTP_202_ACCEPTED) - - url = client.app.router["my_phone_confirm"].url_for() - resp = await client.post( - f"{url}", - json={ - "code": _REGISTRATION_CODE_VALUE_FAKE, - }, - ) - await assert_status(resp, status.HTTP_204_NO_CONTENT) - - # GET updated profile - url = client.app.router["get_my_profile"].url_for() - resp = await client.get(f"{url}") - data, _ = await assert_status(resp, status.HTTP_200_OK) - - updated_profile = MyProfileRestGet.model_validate(data) - - # Verify phone was updated to new phone - assert updated_profile.phone == new_phone - assert updated_profile.phone != first_phone - - -# -# PHONE REGISTRATION FAILURE TESTS -# - - -@pytest.mark.parametrize("user_role", [UserRole.USER]) -async def test_phone_resend_without_pending_registration( - user_role: UserRole, - logged_user: UserInfoDict, - client: TestClient, -): - assert client.app - - # Try to resend code without any pending registration - url = client.app.router["my_phone_resend"].url_for() - resp = await client.post(f"{url}") - await assert_status(resp, status.HTTP_400_BAD_REQUEST) - - -@pytest.mark.parametrize("user_role", [UserRole.USER]) -async def test_phone_confirm_without_pending_registration( - user_role: UserRole, - logged_user: UserInfoDict, - client: TestClient, -): - assert client.app - - # Try to confirm code without any pending registration - url = client.app.router["my_phone_confirm"].url_for() - resp = await client.post( - f"{url}", - json={ - "code": _REGISTRATION_CODE_VALUE_FAKE, - }, - ) - await assert_status(resp, status.HTTP_400_BAD_REQUEST) - - -@pytest.mark.parametrize("user_role", [UserRole.USER]) -async def test_phone_confirm_with_wrong_code( - user_role: UserRole, - logged_user: UserInfoDict, - client: TestClient, - user_phone_number: PhoneNumberStr, -): - assert client.app - - # STEP 1: REGISTER phone number - new_phone = user_phone_number - url = client.app.router["my_phone_register"].url_for() - resp = await client.post( - f"{url}", - json={ - "phone": new_phone, - }, - ) - await assert_status(resp, status.HTTP_202_ACCEPTED) - - # STEP 2: Try to confirm with wrong code - url = client.app.router["my_phone_confirm"].url_for() - resp = await client.post( - f"{url}", - json={ - "code": "wrongcode1234", - }, - ) - await assert_status(resp, status.HTTP_400_BAD_REQUEST) - - -@pytest.mark.parametrize("user_role", [UserRole.USER]) -async def test_phone_confirm_with_invalid_code_format( - user_role: UserRole, - logged_user: UserInfoDict, - client: TestClient, - user_phone_number: PhoneNumberStr, -): - assert client.app - - # STEP 1: REGISTER phone number - new_phone = user_phone_number - url = client.app.router["my_phone_register"].url_for() - resp = await client.post( - f"{url}", - json={ - "phone": new_phone, - }, - ) - await assert_status(resp, status.HTTP_202_ACCEPTED) - - # STEP 2: Try to confirm with invalid code format (contains special characters) - url = client.app.router["my_phone_confirm"].url_for() - resp = await client.post( - f"{url}", - json={ - "code": "123-456", # Invalid format according to pattern - }, - ) - await assert_status(resp, status.HTTP_422_UNPROCESSABLE_ENTITY) - - -@pytest.mark.parametrize("user_role", [UserRole.USER]) -async def test_phone_register_with_empty_phone( - user_role: UserRole, - logged_user: UserInfoDict, - client: TestClient, -): - assert client.app - - # Try to register with empty phone number - url = client.app.router["my_phone_register"].url_for() - resp = await client.post( - f"{url}", - json={ - "phone": "", # Empty phone number - }, - ) - await assert_status(resp, status.HTTP_422_UNPROCESSABLE_ENTITY) - - # Try to register with whitespace-only phone number - url = client.app.router["my_phone_register"].url_for() - resp = await client.post( - f"{url}", - json={ - "phone": " ", # Whitespace only - }, - ) - await assert_status(resp, status.HTTP_422_UNPROCESSABLE_ENTITY) - - -@pytest.mark.parametrize("user_role", [UserRole.USER]) -async def test_phone_confirm_with_empty_code( - user_role: UserRole, - logged_user: UserInfoDict, - client: TestClient, - user_phone_number: PhoneNumberStr, -): - assert client.app - - # STEP 1: REGISTER phone number - new_phone = user_phone_number - url = client.app.router["my_phone_register"].url_for() - resp = await client.post( - f"{url}", - json={ - "phone": new_phone, - }, - ) - await assert_status(resp, status.HTTP_202_ACCEPTED) - - # STEP 2: Try to confirm with empty code - url = client.app.router["my_phone_confirm"].url_for() - resp = await client.post( - f"{url}", - json={ - "code": "", # Empty code - }, - ) - await assert_status(resp, status.HTTP_422_UNPROCESSABLE_ENTITY) - - -@pytest.mark.parametrize( - "user_role,expected", - [ - (UserRole.ANONYMOUS, status.HTTP_401_UNAUTHORIZED), - (UserRole.GUEST, status.HTTP_403_FORBIDDEN), - ], -) -async def test_phone_register_access_rights( - user_role: UserRole, - logged_user: UserInfoDict, - client: TestClient, - expected: HTTPStatus, - user_phone_number: PhoneNumberStr, -): - assert client.app - - # Try to register phone with insufficient permissions - url = client.app.router["my_phone_register"].url_for() - resp = await client.post( - f"{url}", - json={ - "phone": user_phone_number, - }, - ) - await assert_status(resp, expected) - - -@pytest.mark.parametrize( - "user_role,expected", - [ - (UserRole.ANONYMOUS, status.HTTP_401_UNAUTHORIZED), - (UserRole.GUEST, status.HTTP_403_FORBIDDEN), - ], -) -async def test_phone_resend_access_rights( - user_role: UserRole, - logged_user: UserInfoDict, - client: TestClient, - expected: HTTPStatus, -): - assert client.app - - # Try to resend code with insufficient permissions - url = client.app.router["my_phone_resend"].url_for() - resp = await client.post(f"{url}") - await assert_status(resp, expected) - - -@pytest.mark.parametrize( - "user_role,expected", - [ - (UserRole.ANONYMOUS, status.HTTP_401_UNAUTHORIZED), - (UserRole.GUEST, status.HTTP_403_FORBIDDEN), - ], -) -async def test_phone_confirm_access_rights( - user_role: UserRole, - logged_user: UserInfoDict, - client: TestClient, - expected: HTTPStatus, -): - assert client.app - - # Try to confirm code with insufficient permissions - url = client.app.router["my_phone_confirm"].url_for() - resp = await client.post( - f"{url}", - json={ - "code": _REGISTRATION_CODE_VALUE_FAKE, - }, - ) - await assert_status(resp, expected) From a2c70717a2048399c19541c3136a2deb0f9a69fc Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 3 Sep 2025 17:01:49 +0200 Subject: [PATCH 24/43] splits tests further --- .../with_dbs/03/test_users_rest_profile.py | 278 +--------------- .../with_dbs/03/test_users_rest_search.py | 309 ++++++++++++++++++ 2 files changed, 310 insertions(+), 277 deletions(-) create mode 100644 services/web/server/tests/unit/with_dbs/03/test_users_rest_search.py diff --git a/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py b/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py index 6bd3e708eb68..90268a853a91 100644 --- a/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py +++ b/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py @@ -7,7 +7,6 @@ import functools -from collections.abc import AsyncIterable from copy import deepcopy from http import HTTPStatus from typing import Any @@ -17,19 +16,13 @@ from aiohttp.test_utils import TestClient from aiopg.sa.connection import SAConnection from common_library.users_enums import UserRole -from models_library.api_schemas_webserver.groups import GroupUserGet from models_library.api_schemas_webserver.users import ( MyProfileRestGet, - UserGet, ) from psycopg2 import OperationalError -from pydantic import TypeAdapter from pytest_simcore.helpers.assert_checks import assert_status from pytest_simcore.helpers.monkeypatch_envs import EnvVarsDict, setenvs_from_dict -from pytest_simcore.helpers.webserver_login import ( - switch_client_session_to, -) -from pytest_simcore.helpers.webserver_users import NewUser, UserInfoDict +from pytest_simcore.helpers.webserver_users import UserInfoDict from servicelib.aiohttp import status from servicelib.rest_constants import RESPONSE_MODEL_POLICY from simcore_service_webserver.user_preferences._service import ( @@ -54,275 +47,6 @@ def app_environment( ) -@pytest.fixture -def partial_first_name() -> str: - return "Jaimito" - - -@pytest.fixture -def partial_username() -> str: - return "COMMON_USERNAME" - - -@pytest.fixture -def partial_email() -> str: - return "@acme.com" - - -@pytest.fixture -async def private_user( - client: TestClient, - partial_username: str, - partial_email: str, - partial_first_name: str, -) -> AsyncIterable[UserInfoDict]: - assert client.app - async with NewUser( - app=client.app, - user_data={ - "name": f"james{partial_username}", - "first_name": partial_first_name, - "last_name": "Bond", - "email": f"james{partial_email}", - # Maximum privacy - "privacy_hide_username": True, - "privacy_hide_email": True, - "privacy_hide_fullname": True, - }, - ) as usr: - yield usr - - -@pytest.fixture -async def semi_private_user( - client: TestClient, partial_username: str, partial_first_name: str -) -> AsyncIterable[UserInfoDict]: - assert client.app - async with NewUser( - app=client.app, - user_data={ - "name": f"maxwell{partial_username}", - "first_name": partial_first_name, - "last_name": "Maxwell", - "email": "j@maxwell.me", - # Medium privacy - "privacy_hide_username": False, - "privacy_hide_email": True, - "privacy_hide_fullname": False, # <-- - }, - ) as usr: - yield usr - - -@pytest.fixture -async def public_user( - client: TestClient, partial_username: str, partial_email: str -) -> AsyncIterable[UserInfoDict]: - assert client.app - async with NewUser( - app=client.app, - user_data={ - "name": f"taylor{partial_username}", - "first_name": "Taylor", - "last_name": "Swift", - "email": f"taylor{partial_email}", - # Fully public - "privacy_hide_username": False, - "privacy_hide_email": False, - "privacy_hide_fullname": False, - }, - ) as usr: - yield usr - - -@pytest.mark.parametrize("user_role", [UserRole.USER]) -async def test_search_users_by_partial_fullname( - user_role: UserRole, - logged_user: UserInfoDict, - client: TestClient, - partial_first_name: str, - private_user: UserInfoDict, - semi_private_user: UserInfoDict, - public_user: UserInfoDict, -): - assert client.app - assert user_role == logged_user["role"] - - # logged_user has default settings - assert private_user["id"] != logged_user["id"] - assert public_user["id"] != logged_user["id"] - - # SEARCH by partial first_name - assert partial_first_name in private_user.get("first_name", "") - assert partial_first_name in semi_private_user.get("first_name", "") - assert partial_first_name not in public_user.get("first_name", "") - - url = client.app.router["search_users"].url_for() - resp = await client.post(f"{url}", json={"match": partial_first_name}) - data, _ = await assert_status(resp, status.HTTP_200_OK) - - # expected `semi_private_user` found - found = TypeAdapter(list[UserGet]).validate_python(data) - assert found - assert len(found) == 1 - assert found[0].user_name == semi_private_user["name"] - assert found[0].first_name == semi_private_user.get("first_name") - assert found[0].last_name == semi_private_user.get("last_name") - assert found[0].email is None - - -@pytest.mark.parametrize("user_role", [UserRole.USER]) -async def test_search_users_by_partial_email( - user_role: UserRole, - logged_user: UserInfoDict, - client: TestClient, - partial_email: str, - public_user: UserInfoDict, - semi_private_user: UserInfoDict, - private_user: UserInfoDict, -): - - # SEARCH by partial email - assert partial_email in private_user["email"] - assert partial_email not in semi_private_user["email"] - assert partial_email in public_user["email"] - - url = client.app.router["search_users"].url_for() - resp = await client.post(f"{url}", json={"match": partial_email}) - data, _ = await assert_status(resp, status.HTTP_200_OK) - - found = TypeAdapter(list[UserGet]).validate_python(data) - assert found - assert len(found) == 1 - - # expected `public_user` found - assert found[0].user_id == public_user["id"] - assert found[0].user_name == public_user["name"] - assert found[0].email == public_user["email"] - assert found[0].first_name == public_user.get("first_name") - assert found[0].last_name == public_user.get("last_name") - - # SEARCH user for admin (from a USER) - url = ( - client.app.router["search_user_accounts"] - .url_for() - .with_query(email=partial_email) - ) - resp = await client.get(f"{url}") - await assert_status(resp, status.HTTP_403_FORBIDDEN) - - -@pytest.mark.parametrize("user_role", [UserRole.USER]) -async def test_search_users_by_partial_username( - logged_user: UserInfoDict, - client: TestClient, - partial_username: str, - public_user: UserInfoDict, - semi_private_user: UserInfoDict, - private_user: UserInfoDict, -): - assert client.app - - # SEARCH by partial username - assert partial_username in private_user["name"] - assert partial_username in semi_private_user["name"] - assert partial_username in public_user["name"] - - url = client.app.router["search_users"].url_for() - resp = await client.post(f"{url}", json={"match": partial_username}) - data, _ = await assert_status(resp, status.HTTP_200_OK) - - found = TypeAdapter(list[UserGet]).validate_python(data) - assert found - assert len(found) == 2 - - # expected `public_user` found - index = [u.user_id for u in found].index(public_user["id"]) - assert found[index].user_name == public_user["name"] - assert found[index].email == public_user["email"] - assert found[index].first_name == public_user.get("first_name") - assert found[index].last_name == public_user.get("last_name") - - # expected `semi_private_user` found - index = (index + 1) % 2 - assert found[index].user_name == semi_private_user["name"] - assert found[index].email is None - assert found[index].first_name == semi_private_user.get("first_name") - assert found[index].last_name == semi_private_user.get("last_name") - - -async def test_search_myself( - client: TestClient, - public_user: UserInfoDict, - semi_private_user: UserInfoDict, - private_user: UserInfoDict, -): - assert client.app - for user in [public_user, semi_private_user, private_user]: - async with switch_client_session_to(client, user): - - # search me - url = client.app.router["search_users"].url_for() - resp = await client.post(f"{url}", json={"match": user["name"]}) - data, _ = await assert_status(resp, status.HTTP_200_OK) - - found = TypeAdapter(list[UserGet]).validate_python(data) - assert found - assert len(found) == 1 - - # I can see my own data - assert found[0].user_name == user["name"] - assert found[0].email == user["email"] - assert found[0].first_name == user.get("first_name") - assert found[0].last_name == user.get("last_name") - - -@pytest.mark.acceptance_test( - "https://github.com/ITISFoundation/osparc-issues/issues/1779" -) -@pytest.mark.parametrize("user_role", [UserRole.USER]) -async def test_get_user_by_group_id( - user_role: UserRole, - logged_user: UserInfoDict, - client: TestClient, - public_user: UserInfoDict, - private_user: UserInfoDict, -): - assert client.app - assert user_role == logged_user["role"] - - assert private_user["id"] != logged_user["id"] - assert public_user["id"] != logged_user["id"] - - # GET public_user by its primary gid - url = client.app.router["get_all_group_users"].url_for( - gid=f"{public_user['primary_gid']}" - ) - resp = await client.get(f"{url}") - data, _ = await assert_status(resp, status.HTTP_200_OK) - - users = TypeAdapter(list[GroupUserGet]).validate_python(data) - assert len(users) == 1 - assert users[0].id == public_user["id"] - assert users[0].user_name == public_user["name"] - assert users[0].first_name == public_user.get("first_name") - assert users[0].last_name == public_user.get("last_name") - - # GET private_user by its primary gid - url = client.app.router["get_all_group_users"].url_for( - gid=f"{private_user['primary_gid']}" - ) - resp = await client.get(f"{url}") - data, _ = await assert_status(resp, status.HTTP_200_OK) - - users = TypeAdapter(list[GroupUserGet]).validate_python(data) - assert len(users) == 1 - assert users[0].id == private_user["id"] - assert users[0].user_name is None, "It's private" - assert users[0].first_name is None, "It's private" - assert users[0].last_name is None, "It's private" - - @pytest.mark.parametrize( "user_role,expected", [ diff --git a/services/web/server/tests/unit/with_dbs/03/test_users_rest_search.py b/services/web/server/tests/unit/with_dbs/03/test_users_rest_search.py new file mode 100644 index 000000000000..7473033cbd1a --- /dev/null +++ b/services/web/server/tests/unit/with_dbs/03/test_users_rest_search.py @@ -0,0 +1,309 @@ +# pylint: disable=protected-access +# pylint: disable=redefined-outer-name +# pylint: disable=too-many-arguments +# pylint: disable=too-many-statements +# pylint: disable=unused-argument +# pylint: disable=unused-variable + + +from collections.abc import AsyncIterable + +import pytest +from aiohttp.test_utils import TestClient +from common_library.users_enums import UserRole +from models_library.api_schemas_webserver.groups import GroupUserGet +from models_library.api_schemas_webserver.users import ( + UserGet, +) +from pydantic import TypeAdapter +from pytest_simcore.helpers.assert_checks import assert_status +from pytest_simcore.helpers.monkeypatch_envs import EnvVarsDict, setenvs_from_dict +from pytest_simcore.helpers.webserver_login import ( + switch_client_session_to, +) +from pytest_simcore.helpers.webserver_users import NewUser, UserInfoDict +from servicelib.aiohttp import status + + +@pytest.fixture +def app_environment( + app_environment: EnvVarsDict, monkeypatch: pytest.MonkeyPatch +) -> EnvVarsDict: + # disables GC and DB-listener + return app_environment | setenvs_from_dict( + monkeypatch, + { + "WEBSERVER_GARBAGE_COLLECTOR": "null", + "WEBSERVER_DB_LISTENER": "0", + "WEBSERVER_DEV_FEATURES_ENABLED": "1", # NOTE: still under development + }, + ) + + +@pytest.fixture +def partial_first_name() -> str: + return "Jaimito" + + +@pytest.fixture +def partial_username() -> str: + return "COMMON_USERNAME" + + +@pytest.fixture +def partial_email() -> str: + return "@acme.com" + + +@pytest.fixture +async def private_user( + client: TestClient, + partial_username: str, + partial_email: str, + partial_first_name: str, +) -> AsyncIterable[UserInfoDict]: + assert client.app + async with NewUser( + app=client.app, + user_data={ + "name": f"james{partial_username}", + "first_name": partial_first_name, + "last_name": "Bond", + "email": f"james{partial_email}", + # Maximum privacy + "privacy_hide_username": True, + "privacy_hide_email": True, + "privacy_hide_fullname": True, + }, + ) as usr: + yield usr + + +@pytest.fixture +async def semi_private_user( + client: TestClient, partial_username: str, partial_first_name: str +) -> AsyncIterable[UserInfoDict]: + assert client.app + async with NewUser( + app=client.app, + user_data={ + "name": f"maxwell{partial_username}", + "first_name": partial_first_name, + "last_name": "Maxwell", + "email": "j@maxwell.me", + # Medium privacy + "privacy_hide_username": False, + "privacy_hide_email": True, + "privacy_hide_fullname": False, # <-- + }, + ) as usr: + yield usr + + +@pytest.fixture +async def public_user( + client: TestClient, partial_username: str, partial_email: str +) -> AsyncIterable[UserInfoDict]: + assert client.app + async with NewUser( + app=client.app, + user_data={ + "name": f"taylor{partial_username}", + "first_name": "Taylor", + "last_name": "Swift", + "email": f"taylor{partial_email}", + # Fully public + "privacy_hide_username": False, + "privacy_hide_email": False, + "privacy_hide_fullname": False, + }, + ) as usr: + yield usr + + +@pytest.mark.acceptance_test( + "https://github.com/ITISFoundation/osparc-issues/issues/1779" +) +@pytest.mark.parametrize("user_role", [UserRole.USER]) +async def test_get_user_by_group_id( + user_role: UserRole, + logged_user: UserInfoDict, + client: TestClient, + public_user: UserInfoDict, + private_user: UserInfoDict, +): + assert client.app + assert user_role == logged_user["role"] + + assert private_user["id"] != logged_user["id"] + assert public_user["id"] != logged_user["id"] + + # GET public_user by its primary gid + url = client.app.router["get_all_group_users"].url_for( + gid=f"{public_user['primary_gid']}" + ) + resp = await client.get(f"{url}") + data, _ = await assert_status(resp, status.HTTP_200_OK) + + users = TypeAdapter(list[GroupUserGet]).validate_python(data) + assert len(users) == 1 + assert users[0].id == public_user["id"] + assert users[0].user_name == public_user["name"] + assert users[0].first_name == public_user.get("first_name") + assert users[0].last_name == public_user.get("last_name") + + # GET private_user by its primary gid + url = client.app.router["get_all_group_users"].url_for( + gid=f"{private_user['primary_gid']}" + ) + resp = await client.get(f"{url}") + data, _ = await assert_status(resp, status.HTTP_200_OK) + + users = TypeAdapter(list[GroupUserGet]).validate_python(data) + assert len(users) == 1 + assert users[0].id == private_user["id"] + assert users[0].user_name is None, "It's private" + assert users[0].first_name is None, "It's private" + assert users[0].last_name is None, "It's private" + + +@pytest.mark.parametrize("user_role", [UserRole.USER]) +async def test_search_users_by_partial_fullname( + user_role: UserRole, + logged_user: UserInfoDict, + client: TestClient, + partial_first_name: str, + private_user: UserInfoDict, + semi_private_user: UserInfoDict, + public_user: UserInfoDict, +): + assert client.app + assert user_role == logged_user["role"] + + # logged_user has default settings + assert private_user["id"] != logged_user["id"] + assert public_user["id"] != logged_user["id"] + + # SEARCH by partial first_name + assert partial_first_name in private_user.get("first_name", "") + assert partial_first_name in semi_private_user.get("first_name", "") + assert partial_first_name not in public_user.get("first_name", "") + + url = client.app.router["search_users"].url_for() + resp = await client.post(f"{url}", json={"match": partial_first_name}) + data, _ = await assert_status(resp, status.HTTP_200_OK) + + # expected `semi_private_user` found + found = TypeAdapter(list[UserGet]).validate_python(data) + assert found + assert len(found) == 1 + assert found[0].user_name == semi_private_user["name"] + assert found[0].first_name == semi_private_user.get("first_name") + assert found[0].last_name == semi_private_user.get("last_name") + assert found[0].email is None + + +@pytest.mark.parametrize("user_role", [UserRole.USER]) +async def test_search_users_by_partial_email( + user_role: UserRole, + logged_user: UserInfoDict, + client: TestClient, + partial_email: str, + public_user: UserInfoDict, + semi_private_user: UserInfoDict, + private_user: UserInfoDict, +): + + # SEARCH by partial email + assert partial_email in private_user["email"] + assert partial_email not in semi_private_user["email"] + assert partial_email in public_user["email"] + + url = client.app.router["search_users"].url_for() + resp = await client.post(f"{url}", json={"match": partial_email}) + data, _ = await assert_status(resp, status.HTTP_200_OK) + + found = TypeAdapter(list[UserGet]).validate_python(data) + assert found + assert len(found) == 1 + + # expected `public_user` found + assert found[0].user_id == public_user["id"] + assert found[0].user_name == public_user["name"] + assert found[0].email == public_user["email"] + assert found[0].first_name == public_user.get("first_name") + assert found[0].last_name == public_user.get("last_name") + + # SEARCH user for admin (from a USER) + url = ( + client.app.router["search_user_accounts"] + .url_for() + .with_query(email=partial_email) + ) + resp = await client.get(f"{url}") + await assert_status(resp, status.HTTP_403_FORBIDDEN) + + +@pytest.mark.parametrize("user_role", [UserRole.USER]) +async def test_search_users_by_partial_username( + logged_user: UserInfoDict, + client: TestClient, + partial_username: str, + public_user: UserInfoDict, + semi_private_user: UserInfoDict, + private_user: UserInfoDict, +): + assert client.app + + # SEARCH by partial username + assert partial_username in private_user["name"] + assert partial_username in semi_private_user["name"] + assert partial_username in public_user["name"] + + url = client.app.router["search_users"].url_for() + resp = await client.post(f"{url}", json={"match": partial_username}) + data, _ = await assert_status(resp, status.HTTP_200_OK) + + found = TypeAdapter(list[UserGet]).validate_python(data) + assert found + assert len(found) == 2 + + # expected `public_user` found + index = [u.user_id for u in found].index(public_user["id"]) + assert found[index].user_name == public_user["name"] + assert found[index].email == public_user["email"] + assert found[index].first_name == public_user.get("first_name") + assert found[index].last_name == public_user.get("last_name") + + # expected `semi_private_user` found + index = (index + 1) % 2 + assert found[index].user_name == semi_private_user["name"] + assert found[index].email is None + assert found[index].first_name == semi_private_user.get("first_name") + assert found[index].last_name == semi_private_user.get("last_name") + + +async def test_search_myself( + client: TestClient, + public_user: UserInfoDict, + semi_private_user: UserInfoDict, + private_user: UserInfoDict, +): + assert client.app + for user in [public_user, semi_private_user, private_user]: + async with switch_client_session_to(client, user): + + # search me + url = client.app.router["search_users"].url_for() + resp = await client.post(f"{url}", json={"match": user["name"]}) + data, _ = await assert_status(resp, status.HTTP_200_OK) + + found = TypeAdapter(list[UserGet]).validate_python(data) + assert found + assert len(found) == 1 + + # I can see my own data + assert found[0].user_name == user["name"] + assert found[0].email == user["email"] + assert found[0].first_name == user.get("first_name") + assert found[0].last_name == user.get("last_name") From a803a211562f64e08de836c1cc57f46423601973 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 3 Sep 2025 17:23:46 +0200 Subject: [PATCH 25/43] refactors profile groups --- .../groups/_groups_rest.py | 34 ++++------------- .../groups/_groups_service.py | 38 +++++++++++++++++++ .../simcore_service_webserver/groups/api.py | 2 + .../users/_controller/rest/users_rest.py | 35 ++++------------- 4 files changed, 56 insertions(+), 53 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/groups/_groups_rest.py b/services/web/server/src/simcore_service_webserver/groups/_groups_rest.py index 806fa60b0b64..23a47136b030 100644 --- a/services/web/server/src/simcore_service_webserver/groups/_groups_rest.py +++ b/services/web/server/src/simcore_service_webserver/groups/_groups_rest.py @@ -1,5 +1,4 @@ import logging -from contextlib import suppress from aiohttp import web from models_library.api_schemas_webserver.groups import ( @@ -30,7 +29,6 @@ GroupsRequestContext, GroupsUsersPathParams, ) -from .exceptions import GroupNotFoundError _logger = logging.getLogger(__name__) @@ -49,32 +47,16 @@ async def list_groups(request: web.Request): product: Product = products_web.get_current_product(request) req_ctx = GroupsRequestContext.model_validate(request) - groups_by_type = await _groups_service.list_user_groups_with_read_access( - request.app, user_id=req_ctx.user_id + ( + groups_by_type, + my_product_group, + my_support_group, + ) = await _groups_service.get_user_profile_groups( + request.app, user_id=req_ctx.user_id, product=product ) - assert groups_by_type.primary - assert groups_by_type.everyone - - if product.group_id: - with suppress(GroupNotFoundError): - # Product is optional - my_product_group = await _groups_service.get_product_group_for_user( - app=request.app, - user_id=req_ctx.user_id, - product_gid=product.group_id, - ) - else: - my_product_group = None - - if product.support_standard_group_id: - my_support_group = await _groups_service.get_support_group_for_user_or_none( - app=request.app, - user_id=req_ctx.user_id, - support_gid=product.support_standard_group_id, - ) - else: - my_support_group = None + assert groups_by_type.primary # nosec + assert groups_by_type.everyone # nosec my_groups = MyGroupsGet.from_domain_model( groups_by_type, my_product_group, my_support_group diff --git a/services/web/server/src/simcore_service_webserver/groups/_groups_service.py b/services/web/server/src/simcore_service_webserver/groups/_groups_service.py index c28b7d9d2e34..f0b05cdcc474 100644 --- a/services/web/server/src/simcore_service_webserver/groups/_groups_service.py +++ b/services/web/server/src/simcore_service_webserver/groups/_groups_service.py @@ -16,6 +16,7 @@ from models_library.users import UserID from pydantic import EmailStr +from ..products.models import Product from ..users import users_service from . import _groups_repository from .exceptions import GroupNotFoundError, GroupsError @@ -91,6 +92,43 @@ async def get_support_group_for_user_or_none( return None +async def get_user_profile_groups( + app: web.Application, *, user_id: UserID, product: Product +) -> tuple[ + GroupsByTypeTuple, + tuple[Group, AccessRightsDict] | None, + tuple[Group, AccessRightsDict] | None, +]: + """ + Get all groups needed for user profile including standard groups, + product group, and support group. + + Returns: + Tuple of (groups_by_type, my_product_group, my_support_group) + """ + groups_by_type = await list_user_groups_with_read_access(app, user_id=user_id) + + my_product_group = None + if product.group_id: # Product group is optional + with suppress(GroupNotFoundError): + my_product_group = await get_product_group_for_user( + app=app, + user_id=user_id, + product_gid=product.group_id, + ) + + my_support_group = None + if product.support_standard_group_id: # Support group is optional + # NOTE: my_support_group can be part of groups_by_type.standard! + my_support_group = await get_support_group_for_user_or_none( + app=app, + user_id=user_id, + support_gid=product.support_standard_group_id, + ) + + return groups_by_type, my_product_group, my_support_group + + # # CRUD operations on groups linked to a user # diff --git a/services/web/server/src/simcore_service_webserver/groups/api.py b/services/web/server/src/simcore_service_webserver/groups/api.py index 68f14c239baf..d72b1e923eaa 100644 --- a/services/web/server/src/simcore_service_webserver/groups/api.py +++ b/services/web/server/src/simcore_service_webserver/groups/api.py @@ -8,6 +8,7 @@ get_group_from_gid, get_product_group_for_user, get_support_group_for_user_or_none, + get_user_profile_groups, is_user_by_email_in_group, list_all_user_groups_ids, list_group_members, @@ -20,6 +21,7 @@ "auto_add_user_to_groups", "auto_add_user_to_product_group", "get_group_from_gid", + "get_user_profile_groups", "get_product_group_for_user", "get_support_group_for_user_or_none", "is_user_by_email_in_group", diff --git a/services/web/server/src/simcore_service_webserver/users/_controller/rest/users_rest.py b/services/web/server/src/simcore_service_webserver/users/_controller/rest/users_rest.py index 0fd831d20500..42d116f76ad9 100644 --- a/services/web/server/src/simcore_service_webserver/users/_controller/rest/users_rest.py +++ b/services/web/server/src/simcore_service_webserver/users/_controller/rest/users_rest.py @@ -1,5 +1,4 @@ import logging -from contextlib import suppress from aiohttp import web from models_library.api_schemas_webserver.users import ( @@ -18,7 +17,6 @@ from ...._meta import API_VTAG from ....groups import api as groups_service -from ....groups.exceptions import GroupNotFoundError from ....login.decorators import login_required from ....products import products_web from ....products.models import Product @@ -51,33 +49,16 @@ async def get_my_profile(request: web.Request) -> web.Response: product: Product = products_web.get_current_product(request) req_ctx = UsersRequestContext.model_validate(request) - groups_by_type = await groups_service.list_user_groups_with_read_access( - request.app, user_id=req_ctx.user_id + ( + groups_by_type, + my_product_group, + my_support_group, + ) = await groups_service.get_user_profile_groups( + request.app, user_id=req_ctx.user_id, product=product ) - assert groups_by_type.primary - assert groups_by_type.everyone - - if product.group_id: - with suppress(GroupNotFoundError): - # Product is optional - my_product_group = await groups_service.get_product_group_for_user( - app=request.app, - user_id=req_ctx.user_id, - product_gid=product.group_id, - ) - else: - my_product_group = None - - if product.support_standard_group_id: - # Support group is optional - my_support_group = await groups_service.get_support_group_for_user_or_none( - app=request.app, - user_id=req_ctx.user_id, - support_gid=product.support_standard_group_id, - ) - else: - my_support_group = None + assert groups_by_type.primary # nosec + assert groups_by_type.everyone # nosec my_profile, preferences = await _users_service.get_my_profile( request.app, user_id=req_ctx.user_id, product_name=req_ctx.product_name From 9f6e211fadd9cb297972162bdaab5edaea85487a Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 3 Sep 2025 18:02:51 +0200 Subject: [PATCH 26/43] fixes statics --- .../server/src/simcore_service_webserver/products/_models.py | 1 + .../server/src/simcore_service_webserver/statics/_events.py | 3 --- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/products/_models.py b/services/web/server/src/simcore_service_webserver/products/_models.py index cb2a08400ac4..e28c20eb2b68 100644 --- a/services/web/server/src/simcore_service_webserver/products/_models.py +++ b/services/web/server/src/simcore_service_webserver/products/_models.py @@ -332,6 +332,7 @@ def to_statics(self) -> dict[str, Any]: "support": True, "is_payment_enabled": True, "is_dynamic_services_telemetry_enabled": True, + "support_standard_group_id": True, }, exclude_none=True, exclude_unset=True, diff --git a/services/web/server/src/simcore_service_webserver/statics/_events.py b/services/web/server/src/simcore_service_webserver/statics/_events.py index 742fdaa828b4..af62a9bff253 100644 --- a/services/web/server/src/simcore_service_webserver/statics/_events.py +++ b/services/web/server/src/simcore_service_webserver/statics/_events.py @@ -147,9 +147,6 @@ async def create_and_cache_statics_json(app: web.Application) -> None: release_vtag = _get_release_notes_vtag(vtag) data["vcsReleaseUrl"] = template_url.format(vtag=release_vtag) - # Add support_standard_group_id - data["supportStandardGroupId"] = product.support_standard_group_id - data_json = json_dumps(data) _logger.debug("Front-end statics.json: %s", data_json) From b9fbb323442fb03adbe15bfc1df8036641c97434 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 3 Sep 2025 18:32:36 +0200 Subject: [PATCH 27/43] only group --- .../groups/_groups_service.py | 21 ++------ .../with_dbs/03/test_users_rest_profile.py | 53 ++++++++++++++++++- 2 files changed, 55 insertions(+), 19 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/groups/_groups_service.py b/services/web/server/src/simcore_service_webserver/groups/_groups_service.py index f0b05cdcc474..83dd4276b40d 100644 --- a/services/web/server/src/simcore_service_webserver/groups/_groups_service.py +++ b/services/web/server/src/simcore_service_webserver/groups/_groups_service.py @@ -79,25 +79,12 @@ async def get_product_group_for_user( ) -async def get_support_group_for_user_or_none( - app: web.Application, *, user_id: UserID, support_gid: GroupID -) -> tuple[Group, AccessRightsDict] | None: - """ - Returns support's group if user belongs to it, otherwise it - """ - with suppress(GroupNotFoundError): - return await _groups_repository.get_any_group_for_user( - app, user_id=user_id, group_gid=support_gid - ) - return None - - async def get_user_profile_groups( app: web.Application, *, user_id: UserID, product: Product ) -> tuple[ GroupsByTypeTuple, tuple[Group, AccessRightsDict] | None, - tuple[Group, AccessRightsDict] | None, + Group | None, ]: """ Get all groups needed for user profile including standard groups, @@ -120,10 +107,8 @@ async def get_user_profile_groups( my_support_group = None if product.support_standard_group_id: # Support group is optional # NOTE: my_support_group can be part of groups_by_type.standard! - my_support_group = await get_support_group_for_user_or_none( - app=app, - user_id=user_id, - support_gid=product.support_standard_group_id, + my_support_group = await get_group_from_gid( + app, product.support_standard_group_id ) return groups_by_type, my_product_group, my_support_group diff --git a/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py b/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py index 90268a853a91..915ef53d6d6d 100644 --- a/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py +++ b/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py @@ -7,12 +7,14 @@ import functools +from collections.abc import Iterator from copy import deepcopy from http import HTTPStatus from typing import Any from unittest.mock import patch import pytest +import sqlalchemy as sa from aiohttp.test_utils import TestClient from aiopg.sa.connection import SAConnection from common_library.users_enums import UserRole @@ -22,6 +24,7 @@ from psycopg2 import OperationalError from pytest_simcore.helpers.assert_checks import assert_status from pytest_simcore.helpers.monkeypatch_envs import EnvVarsDict, setenvs_from_dict +from pytest_simcore.helpers.postgres_tools import sync_insert_and_get_row_lifespan from pytest_simcore.helpers.webserver_users import UserInfoDict from servicelib.aiohttp import status from servicelib.rest_constants import RESPONSE_MODEL_POLICY @@ -47,6 +50,41 @@ def app_environment( ) +@pytest.fixture(scope="module") +def support_group( + postgres_db: sa.engine.Engine, + product_name: str, +) -> Iterator[dict[str, Any]]: + """Creates a standard support group and assigns it to the current product""" + from simcore_postgres_database.models.groups import groups + from simcore_postgres_database.models.products import products + + # Create support group using direct database insertion + group_values = { + "name": "Support Group", + "description": "Support group for product", + "type": "STANDARD", + } + + with sync_insert_and_get_row_lifespan( + postgres_db, + table=groups, + values=group_values, + pk_col=groups.c.gid, + ) as group_row: + group_id = group_row["gid"] + + # Update product to set support_standard_group_id + with postgres_db.begin() as conn: + conn.execute( + sa.update(products) + .where(products.c.name == product_name) + .values(support_standard_group_id=group_id) + ) + + yield group_row + + @pytest.mark.parametrize( "user_role,expected", [ @@ -100,6 +138,7 @@ async def test_get_profile( primary_group: dict[str, Any], standard_groups: list[dict[str, Any]], all_group: dict[str, str], + support_group: dict[str, Any], ): assert client.app @@ -130,13 +169,25 @@ async def test_get_profile( "thumbnail": None, } - assert got_profile_groups["support"] is None + # support group exists + assert got_profile_groups["support"] + support_group_id = got_profile_groups["support"]["gid"] + assert support_group_id == support_group["gid"] + assert got_profile_groups["support"]["description"] == support_group["description"] + assert "accessRights" not in got_profile_groups["support"] + + # standard groups with at least read access sorted_by_group_id = functools.partial(sorted, key=lambda d: d["gid"]) assert sorted_by_group_id( got_profile_groups["organizations"] ) == sorted_by_group_id(standard_groups) + # user is NOT part of the support group + all_standard_groups_ids = {g["gid"] for g in standard_groups} + assert support_group_id not in all_standard_groups_ids + + # preferences assert profile.preferences == await get_frontend_user_preferences_aggregation( client.app, user_id=logged_user["id"], product_name="osparc" ) From d472d283c99c1483e1a1168d3e504813d7099f33 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 3 Sep 2025 18:40:01 +0200 Subject: [PATCH 28/43] gets product support group --- .../api_schemas_webserver/groups.py | 91 +++++++++++-------- .../api_schemas_webserver/users.py | 2 +- packages/models-library/tests/test_users.py | 4 +- .../groups/_groups_rest.py | 4 +- .../groups/_groups_service.py | 8 +- .../users/_controller/rest/users_rest.py | 4 +- 6 files changed, 62 insertions(+), 51 deletions(-) diff --git a/packages/models-library/src/models_library/api_schemas_webserver/groups.py b/packages/models-library/src/models_library/api_schemas_webserver/groups.py index 505d06e7fb40..0694283df9a8 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/groups.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/groups.py @@ -54,14 +54,53 @@ class GroupAccessRights(BaseModel): ) -class GroupGet(OutputSchema): - gid: GroupID = Field(..., description="the group ID") - label: str = Field(..., description="the group name") - description: str = Field(..., description="the group description") - thumbnail: AnyUrl | None = Field( - default=None, description="url to the group thumbnail" - ) - access_rights: GroupAccessRights = Field(..., alias="accessRights") +class BasicGroupGet(OutputSchema): + gid: Annotated[GroupID, Field(description="the group's unique ID")] + label: Annotated[str, Field(description="the group's display name")] + description: str + thumbnail: Annotated[ + AnyUrl | None, Field(description="a link to the group's thumbnail") + ] = None + + @field_validator("thumbnail", mode="before") + @classmethod + def _sanitize_legacy_data(cls, v): + if v: + # Enforces null if thumbnail is not valid URL or empty + with suppress(ValidationError): + return TypeAdapter(AnyHttpUrl).validate_python(v) + return None + + @classmethod + def _extract_basic_group_data(cls, group: Group) -> dict: + """Extract common group data for schema conversion""" + return remap_keys( + group.model_dump( + include={ + "gid", + "name", + "description", + "thumbnail", + }, + exclude={ + "inclusion_rules", # deprecated + }, + exclude_unset=True, + by_alias=False, + ), + rename={ + "name": "label", + }, + ) + + @classmethod + def from_domain_model(cls, group: Group) -> Self: + # Adapts these domain models into this schema + return cls.model_validate(cls._extract_basic_group_data(group)) + + +class GroupGet(BasicGroupGet): + access_rights: Annotated[GroupAccessRights, Field(alias="accessRights")] inclusion_rules: Annotated[ dict[str, str], @@ -77,24 +116,7 @@ def from_domain_model(cls, group: Group, access_rights: AccessRightsDict) -> Sel # Adapts these domain models into this schema return cls.model_validate( { - **remap_keys( - group.model_dump( - include={ - "gid", - "name", - "description", - "thumbnail", - }, - exclude={ - "inclusion_rules", # deprecated - }, - exclude_unset=True, - by_alias=False, - ), - rename={ - "name": "label", - }, - ), + **cls._extract_basic_group_data(group), "access_rights": access_rights, } ) @@ -136,15 +158,6 @@ def _update_json_schema_extra(schema: JsonDict) -> None: model_config = ConfigDict(json_schema_extra=_update_json_schema_extra) - @field_validator("thumbnail", mode="before") - @classmethod - def _sanitize_legacy_data(cls, v): - if v: - # Enforces null if thumbnail is not valid URL or empty - with suppress(ValidationError): - return TypeAdapter(AnyHttpUrl).validate_python(v) - return None - class GroupCreate(InputSchema): label: str @@ -188,7 +201,7 @@ class MyGroupsGet(OutputSchema): all: GroupGet product: GroupGet | None = None support: Annotated[ - GroupGet | None, + BasicGroupGet | None, Field( description="Group ID of the app support team or None if no support is defined for this product" ), @@ -235,7 +248,7 @@ class MyGroupsGet(OutputSchema): "gid": "2", "label": "Support Team", "description": "The support team of the application", - "accessRights": {"read": False, "write": False, "delete": False}, + "thumbnail": "https://placekitten.com/15/15", }, } } @@ -246,7 +259,7 @@ def from_domain_model( cls, groups_by_type: GroupsByTypeTuple, my_product_group: tuple[Group, AccessRightsDict] | None, - my_support_group: tuple[Group, AccessRightsDict] | None, + my_support_group: Group | None, ) -> Self: assert groups_by_type.primary # nosec assert groups_by_type.everyone # nosec @@ -263,7 +276,7 @@ def from_domain_model( else None ), support=( - GroupGet.from_domain_model(*my_support_group) + BasicGroupGet.from_domain_model(my_support_group) if my_support_group else None ), diff --git a/packages/models-library/src/models_library/api_schemas_webserver/users.py b/packages/models-library/src/models_library/api_schemas_webserver/users.py index f9ff1821fc7f..59bffad22880 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/users.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/users.py @@ -132,7 +132,7 @@ def from_domain_model( my_groups_by_type: GroupsByTypeTuple, my_product_group: tuple[Group, AccessRightsDict] | None, my_preferences: AggregatedPreferences, - my_support_group: tuple[Group, AccessRightsDict] | None, + my_support_group: Group | None, ) -> Self: data = remap_keys( my_profile.model_dump( diff --git a/packages/models-library/tests/test_users.py b/packages/models-library/tests/test_users.py index 598e9074c6a7..d1e5dbe4efdc 100644 --- a/packages/models-library/tests/test_users.py +++ b/packages/models-library/tests/test_users.py @@ -30,9 +30,7 @@ def test_adapter_from_model_to_schema( read=False, write=False, delete=False ) - my_support_group = groups[4], AccessRightsDict( - read=False, write=False, delete=False - ) + my_support_group = groups[4] my_preferences = {"foo": Preference(default_value=3, value=1)} diff --git a/services/web/server/src/simcore_service_webserver/groups/_groups_rest.py b/services/web/server/src/simcore_service_webserver/groups/_groups_rest.py index 23a47136b030..42fbd892d3f9 100644 --- a/services/web/server/src/simcore_service_webserver/groups/_groups_rest.py +++ b/services/web/server/src/simcore_service_webserver/groups/_groups_rest.py @@ -50,7 +50,7 @@ async def list_groups(request: web.Request): ( groups_by_type, my_product_group, - my_support_group, + product_support_group, ) = await _groups_service.get_user_profile_groups( request.app, user_id=req_ctx.user_id, product=product ) @@ -59,7 +59,7 @@ async def list_groups(request: web.Request): assert groups_by_type.everyone # nosec my_groups = MyGroupsGet.from_domain_model( - groups_by_type, my_product_group, my_support_group + groups_by_type, my_product_group, product_support_group ) return envelope_json_response(my_groups) diff --git a/services/web/server/src/simcore_service_webserver/groups/_groups_service.py b/services/web/server/src/simcore_service_webserver/groups/_groups_service.py index 83dd4276b40d..b6ca9fc23dc2 100644 --- a/services/web/server/src/simcore_service_webserver/groups/_groups_service.py +++ b/services/web/server/src/simcore_service_webserver/groups/_groups_service.py @@ -91,7 +91,7 @@ async def get_user_profile_groups( product group, and support group. Returns: - Tuple of (groups_by_type, my_product_group, my_support_group) + Tuple of (groups_by_type, my_product_group, product_support_group) """ groups_by_type = await list_user_groups_with_read_access(app, user_id=user_id) @@ -104,14 +104,14 @@ async def get_user_profile_groups( product_gid=product.group_id, ) - my_support_group = None + product_support_group = None if product.support_standard_group_id: # Support group is optional # NOTE: my_support_group can be part of groups_by_type.standard! - my_support_group = await get_group_from_gid( + product_support_group = await get_group_from_gid( app, product.support_standard_group_id ) - return groups_by_type, my_product_group, my_support_group + return groups_by_type, my_product_group, product_support_group # diff --git a/services/web/server/src/simcore_service_webserver/users/_controller/rest/users_rest.py b/services/web/server/src/simcore_service_webserver/users/_controller/rest/users_rest.py index 42d116f76ad9..f306da6aa28c 100644 --- a/services/web/server/src/simcore_service_webserver/users/_controller/rest/users_rest.py +++ b/services/web/server/src/simcore_service_webserver/users/_controller/rest/users_rest.py @@ -52,7 +52,7 @@ async def get_my_profile(request: web.Request) -> web.Response: ( groups_by_type, my_product_group, - my_support_group, + product_support_group, ) = await groups_service.get_user_profile_groups( request.app, user_id=req_ctx.user_id, product=product ) @@ -65,7 +65,7 @@ async def get_my_profile(request: web.Request) -> web.Response: ) profile = MyProfileRestGet.from_domain_model( - my_profile, groups_by_type, my_product_group, preferences, my_support_group + my_profile, groups_by_type, my_product_group, preferences, product_support_group ) return envelope_json_response(profile) From ecac9487686bebe7d7e22d4d4abe864be32e1a6a Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 3 Sep 2025 18:41:28 +0200 Subject: [PATCH 29/43] cleanup --- .../web/server/src/simcore_service_webserver/groups/api.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/groups/api.py b/services/web/server/src/simcore_service_webserver/groups/api.py index d72b1e923eaa..0c22a3e6a255 100644 --- a/services/web/server/src/simcore_service_webserver/groups/api.py +++ b/services/web/server/src/simcore_service_webserver/groups/api.py @@ -7,7 +7,6 @@ auto_add_user_to_product_group, get_group_from_gid, get_product_group_for_user, - get_support_group_for_user_or_none, get_user_profile_groups, is_user_by_email_in_group, list_all_user_groups_ids, @@ -21,9 +20,8 @@ "auto_add_user_to_groups", "auto_add_user_to_product_group", "get_group_from_gid", - "get_user_profile_groups", "get_product_group_for_user", - "get_support_group_for_user_or_none", + "get_user_profile_groups", "is_user_by_email_in_group", "list_all_user_groups_ids", "list_group_members", From 62a20140c9b4df68a5b9922a8d0d6a8ebdd05c7d Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 3 Sep 2025 18:45:43 +0200 Subject: [PATCH 30/43] tests --- .../server/tests/unit/with_dbs/03/test_users_rest_profile.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py b/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py index 915ef53d6d6d..75552e270087 100644 --- a/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py +++ b/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py @@ -164,7 +164,7 @@ async def test_get_profile( assert got_profile_groups["product"] == { "accessRights": {"delete": False, "read": False, "write": False}, "description": "osparc product group", - "gid": 2, + "gid": 3, "label": "osparc", "thumbnail": None, } From c7cda4da967ac732a5f70d9989c6c43fb6214066 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 3 Sep 2025 19:16:34 +0200 Subject: [PATCH 31/43] separate access rights in repo --- .../groups/_groups_repository.py | 31 ++++++++++++++----- .../groups/_groups_service.py | 7 +++++ 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/groups/_groups_repository.py b/services/web/server/src/simcore_service_webserver/groups/_groups_repository.py index eecb008b4556..799565e505a3 100644 --- a/services/web/server/src/simcore_service_webserver/groups/_groups_repository.py +++ b/services/web/server/src/simcore_service_webserver/groups/_groups_repository.py @@ -679,6 +679,26 @@ async def delete_user_from_group( # +async def check_group_write_access( + app: web.Application, + connection: AsyncConnection | None = None, + *, + caller_id: UserID, + group_id: GroupID, +) -> None: + """ + Checks if caller has write access to the group. + + Raises: + GroupNotFoundError: if group not found or caller has no access + UserInsufficientRightsError: if caller has no write permission + """ + async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: + await _get_group_and_access_rights_or_raise( + conn, caller_id=caller_id, group_id=group_id, check_permission="write" + ) + + async def is_user_by_email_in_group( app: web.Application, connection: AsyncConnection | None = None, @@ -701,7 +721,6 @@ async def add_new_user_in_group( app: web.Application, connection: AsyncConnection | None = None, *, - caller_id: UserID, group_id: GroupID, # either user_id or user_name new_user_id: UserID | None = None, @@ -709,14 +728,11 @@ async def add_new_user_in_group( access_rights: AccessRightsDict | None = None, ) -> None: """ - adds new_user (either by id or email) in group (with gid) owned by user_id + adds new_user (either by id or email) in group (with gid) + + Note: This function does not check permissions - caller must ensure permissions are checked separately """ async with transaction_context(get_asyncpg_engine(app), connection) as conn: - # first check if the group exists - await _get_group_and_access_rights_or_raise( - conn, caller_id=caller_id, group_id=group_id, check_permission="write" - ) - query = sa.select(users.c.id) if new_user_id is not None: query = query.where(users.c.id == new_user_id) @@ -747,7 +763,6 @@ async def add_new_user_in_group( raise UserAlreadyInGroupError( uid=new_user_id, gid=group_id, - user_id=caller_id, access_rights=access_rights, ) from exc diff --git a/services/web/server/src/simcore_service_webserver/groups/_groups_service.py b/services/web/server/src/simcore_service_webserver/groups/_groups_service.py index b6ca9fc23dc2..8d420d9686a1 100644 --- a/services/web/server/src/simcore_service_webserver/groups/_groups_service.py +++ b/services/web/server/src/simcore_service_webserver/groups/_groups_service.py @@ -297,11 +297,18 @@ async def add_user_in_group( Raises: UserInGroupNotFoundError GroupsException + GroupNotFoundError + UserInsufficientRightsError """ if not _only_one_true(new_by_user_id, new_by_user_name, new_by_user_email): msg = "Invalid method call, required one of these: user id, username or user email, none provided" raise GroupsError(msg=msg) + # First check if caller has write access to the group + await _groups_repository.check_group_write_access( + app, caller_id=user_id, group_id=group_id + ) + # get target user to add to group if new_by_user_email: user = await _groups_repository.get_user_from_email( From 4ac32761e5cc16ff3f8b811e60d5cfb5186bfffb Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 3 Sep 2025 19:22:41 +0200 Subject: [PATCH 32/43] fixing test --- .../groups/_groups_service.py | 1 - .../with_dbs/03/test_users_rest_profile.py | 102 +++++++++++++++--- 2 files changed, 90 insertions(+), 13 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/groups/_groups_service.py b/services/web/server/src/simcore_service_webserver/groups/_groups_service.py index 8d420d9686a1..1f44670dcac9 100644 --- a/services/web/server/src/simcore_service_webserver/groups/_groups_service.py +++ b/services/web/server/src/simcore_service_webserver/groups/_groups_service.py @@ -322,7 +322,6 @@ async def add_user_in_group( return await _groups_repository.add_new_user_in_group( app, - caller_id=user_id, group_id=group_id, new_user_id=new_by_user_id, new_user_name=new_by_user_name, diff --git a/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py b/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py index 75552e270087..f4e67f74ab93 100644 --- a/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py +++ b/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py @@ -7,7 +7,7 @@ import functools -from collections.abc import Iterator +from collections.abc import AsyncIterator from copy import deepcopy from http import HTTPStatus from typing import Any @@ -21,10 +21,10 @@ from models_library.api_schemas_webserver.users import ( MyProfileRestGet, ) +from models_library.groups import AccessRightsDict from psycopg2 import OperationalError from pytest_simcore.helpers.assert_checks import assert_status from pytest_simcore.helpers.monkeypatch_envs import EnvVarsDict, setenvs_from_dict -from pytest_simcore.helpers.postgres_tools import sync_insert_and_get_row_lifespan from pytest_simcore.helpers.webserver_users import UserInfoDict from servicelib.aiohttp import status from servicelib.rest_constants import RESPONSE_MODEL_POLICY @@ -32,7 +32,7 @@ get_frontend_user_preferences_aggregation, ) from sqlalchemy.exc import OperationalError as SQLAlchemyOperationalError -from sqlalchemy.ext.asyncio import AsyncConnection +from sqlalchemy.ext.asyncio import AsyncConnection, AsyncEngine @pytest.fixture @@ -50,12 +50,13 @@ def app_environment( ) -@pytest.fixture(scope="module") -def support_group( - postgres_db: sa.engine.Engine, +@pytest.fixture +async def support_group( + asyncpg_engine: AsyncEngine, product_name: str, -) -> Iterator[dict[str, Any]]: +) -> AsyncIterator[dict[str, Any]]: """Creates a standard support group and assigns it to the current product""" + from pytest_simcore.helpers.postgres_tools import insert_and_get_row_lifespan from simcore_postgres_database.models.groups import groups from simcore_postgres_database.models.products import products @@ -66,8 +67,9 @@ def support_group( "type": "STANDARD", } - with sync_insert_and_get_row_lifespan( - postgres_db, + # pylint: disable=contextmanager-generator-missing-cleanup + async with insert_and_get_row_lifespan( + asyncpg_engine, table=groups, values=group_values, pk_col=groups.c.gid, @@ -75,14 +77,15 @@ def support_group( group_id = group_row["gid"] # Update product to set support_standard_group_id - with postgres_db.begin() as conn: - conn.execute( + async with asyncpg_engine.begin() as conn: + await conn.execute( sa.update(products) .where(products.c.name == product_name) .values(support_standard_group_id=group_id) ) yield group_row + # group will be deleted after test @pytest.mark.parametrize( @@ -131,7 +134,7 @@ async def test_access_update_profile( @pytest.mark.parametrize("user_role", [UserRole.USER]) -async def test_get_profile( +async def test_get_profile_user_not_in_support_group( user_role: UserRole, logged_user: UserInfoDict, client: TestClient, @@ -193,6 +196,81 @@ async def test_get_profile( ) +@pytest.mark.test +@pytest.mark.parametrize("user_role", [UserRole.USER]) +async def test_get_profile_user_in_support_group( + user_role: UserRole, + logged_user: UserInfoDict, + client: TestClient, + primary_group: dict[str, Any], + standard_groups: list[dict[str, Any]], + all_group: dict[str, str], + support_group: dict[str, Any], +): + assert client.app + from simcore_service_webserver.groups import _groups_repository + + # Now add user to support group with read-only access + await _groups_repository.add_new_user_in_group( + client.app, + group_id=support_group["gid"], + new_user_id=logged_user["id"], + access_rights=AccessRightsDict(read=True, write=False, delete=False), + ) + + url = client.app.router["get_my_profile"].url_for() + assert url.path == "/v0/me" + + resp = await client.get(f"{url}") + data, error = await assert_status(resp, status.HTTP_200_OK) + + assert not error + profile = MyProfileRestGet.model_validate(data) + + assert profile.login == logged_user["email"] + assert profile.first_name == logged_user.get("first_name", None) + assert profile.last_name == logged_user.get("last_name", None) + assert profile.role == user_role.name + assert profile.groups + assert profile.expiration_date is None + + got_profile_groups = profile.groups.model_dump(**RESPONSE_MODEL_POLICY, mode="json") + assert got_profile_groups["me"] == primary_group + assert got_profile_groups["all"] == all_group + assert got_profile_groups["product"] == { + "accessRights": {"delete": False, "read": False, "write": False}, + "description": "osparc product group", + "gid": 3, + "label": "osparc", + "thumbnail": None, + } + + # support group exists + assert got_profile_groups["support"] + support_group_id = got_profile_groups["support"]["gid"] + + assert support_group_id == support_group["gid"] + assert got_profile_groups["support"]["description"] == support_group["description"] + assert "accessRights" not in got_profile_groups["support"] + + # When user is part of support group, it should appear in standard groups + sorted_by_group_id = functools.partial(sorted, key=lambda d: d["gid"]) + expected_standard_groups = [ + *standard_groups, + { + "gid": support_group_id, + "label": support_group["name"], + "description": support_group["description"], + "thumbnail": None, + "accessRights": {"read": True, "write": False, "delete": False}, + }, + ] + assert sorted_by_group_id( + got_profile_groups["organizations"] + ) == sorted_by_group_id(expected_standard_groups) + assert support_group_id in {g["gid"] for g in got_profile_groups["organizations"]} + + @pytest.mark.parametrize("user_role", [UserRole.USER]) async def test_update_profile( user_role: UserRole, From eb4942712ae69a5a7b8546e7bc99a221fa7afedb Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 3 Sep 2025 19:34:29 +0200 Subject: [PATCH 33/43] fixing tests --- .../with_dbs/03/test_users_rest_profile.py | 29 ++++++++++++------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py b/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py index f4e67f74ab93..db40aac72ed4 100644 --- a/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py +++ b/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py @@ -51,7 +51,7 @@ def app_environment( @pytest.fixture -async def support_group( +async def support_group_before_app_starts( asyncpg_engine: AsyncEngine, product_name: str, ) -> AsyncIterator[dict[str, Any]]: @@ -135,13 +135,14 @@ async def test_access_update_profile( @pytest.mark.parametrize("user_role", [UserRole.USER]) async def test_get_profile_user_not_in_support_group( + support_group_before_app_starts: dict[str, Any], + # after app starts because it modifies the product user_role: UserRole, logged_user: UserInfoDict, client: TestClient, primary_group: dict[str, Any], standard_groups: list[dict[str, Any]], all_group: dict[str, str], - support_group: dict[str, Any], ): assert client.app @@ -176,8 +177,11 @@ async def test_get_profile_user_not_in_support_group( assert got_profile_groups["support"] support_group_id = got_profile_groups["support"]["gid"] - assert support_group_id == support_group["gid"] - assert got_profile_groups["support"]["description"] == support_group["description"] + assert support_group_id == support_group_before_app_starts["gid"] + assert ( + got_profile_groups["support"]["description"] + == support_group_before_app_starts["description"] + ) assert "accessRights" not in got_profile_groups["support"] # standard groups with at least read access @@ -196,16 +200,16 @@ async def test_get_profile_user_not_in_support_group( ) -@pytest.mark.test @pytest.mark.parametrize("user_role", [UserRole.USER]) async def test_get_profile_user_in_support_group( + support_group_before_app_starts: dict[str, Any], + # after app starts because it modifies the product user_role: UserRole, logged_user: UserInfoDict, client: TestClient, primary_group: dict[str, Any], standard_groups: list[dict[str, Any]], all_group: dict[str, str], - support_group: dict[str, Any], ): assert client.app from simcore_service_webserver.groups import _groups_repository @@ -213,7 +217,7 @@ async def test_get_profile_user_in_support_group( # Now add user to support group with read-only access await _groups_repository.add_new_user_in_group( client.app, - group_id=support_group["gid"], + group_id=support_group_before_app_starts["gid"], new_user_id=logged_user["id"], access_rights=AccessRightsDict(read=True, write=False, delete=False), ) @@ -249,8 +253,11 @@ async def test_get_profile_user_in_support_group( assert got_profile_groups["support"] support_group_id = got_profile_groups["support"]["gid"] - assert support_group_id == support_group["gid"] - assert got_profile_groups["support"]["description"] == support_group["description"] + assert support_group_id == support_group_before_app_starts["gid"] + assert ( + got_profile_groups["support"]["description"] + == support_group_before_app_starts["description"] + ) assert "accessRights" not in got_profile_groups["support"] # When user is part of support group, it should appear in standard groups @@ -259,8 +266,8 @@ async def test_get_profile_user_in_support_group( *standard_groups, { "gid": support_group_id, - "label": support_group["name"], - "description": support_group["description"], + "label": support_group_before_app_starts["name"], + "description": support_group_before_app_starts["description"], "thumbnail": None, "accessRights": {"read": True, "write": False, "delete": False}, }, From 6f760118b51278f6bccfadea9f0f2969cf74de80 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 3 Sep 2025 19:50:58 +0200 Subject: [PATCH 34/43] fix tests --- .../server/tests/unit/isolated/products/test_products_model.py | 1 + 1 file changed, 1 insertion(+) diff --git a/services/web/server/tests/unit/isolated/products/test_products_model.py b/services/web/server/tests/unit/isolated/products/test_products_model.py index bac41a1b3330..7eff7a461992 100644 --- a/services/web/server/tests/unit/isolated/products/test_products_model.py +++ b/services/web/server/tests/unit/isolated/products/test_products_model.py @@ -61,6 +61,7 @@ def test_product_to_static(): assert product_data == { "displayName": "o²S²PARC FOO", "supportEmail": "foo@osparcf.io", + "supportStandardGroupId": 67890, "vendor": { "copyright": "© ACME correcaminos", "name": "ACME", From bd9a2649612caba207538435bffb75c70b1e7d5a Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 3 Sep 2025 20:01:09 +0200 Subject: [PATCH 35/43] fixes pylint --- .../api_schemas_webserver/groups.py | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/packages/models-library/src/models_library/api_schemas_webserver/groups.py b/packages/models-library/src/models_library/api_schemas_webserver/groups.py index 0694283df9a8..09aa879b0b32 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/groups.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/groups.py @@ -72,8 +72,8 @@ def _sanitize_legacy_data(cls, v): return None @classmethod - def _extract_basic_group_data(cls, group: Group) -> dict: - """Extract common group data for schema conversion""" + def dump_basic_group_data(cls, group: Group) -> dict: + """Helper function to extract common group data for schema conversion""" return remap_keys( group.model_dump( include={ @@ -93,11 +93,6 @@ def _extract_basic_group_data(cls, group: Group) -> dict: }, ) - @classmethod - def from_domain_model(cls, group: Group) -> Self: - # Adapts these domain models into this schema - return cls.model_validate(cls._extract_basic_group_data(group)) - class GroupGet(BasicGroupGet): access_rights: Annotated[GroupAccessRights, Field(alias="accessRights")] @@ -116,7 +111,7 @@ def from_domain_model(cls, group: Group, access_rights: AccessRightsDict) -> Sel # Adapts these domain models into this schema return cls.model_validate( { - **cls._extract_basic_group_data(group), + **cls.dump_basic_group_data(group), "access_rights": access_rights, } ) @@ -259,7 +254,7 @@ def from_domain_model( cls, groups_by_type: GroupsByTypeTuple, my_product_group: tuple[Group, AccessRightsDict] | None, - my_support_group: Group | None, + product_support_group: Group | None, ) -> Self: assert groups_by_type.primary # nosec assert groups_by_type.everyone # nosec @@ -276,8 +271,10 @@ def from_domain_model( else None ), support=( - BasicGroupGet.from_domain_model(my_support_group) - if my_support_group + BasicGroupGet.model_validate( + BasicGroupGet.dump_basic_group_data(product_support_group) + ) + if product_support_group else None ), ) From 852c7c20b44b9906320751324527f0636c30cf1a Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Wed, 3 Sep 2025 20:20:49 +0200 Subject: [PATCH 36/43] updates oas --- .../api/v0/openapi.yaml | 43 +++++++++++++++---- 1 file changed, 34 insertions(+), 9 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml index 963b5b65693f..8b9f42e95c35 100644 --- a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml +++ b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml @@ -9284,6 +9284,35 @@ components: - name - email title: Author + BasicGroupGet: + properties: + gid: + type: integer + exclusiveMinimum: true + title: Gid + description: the group's unique ID + minimum: 0 + label: + type: string + title: Label + description: the group's display name + description: + type: string + title: Description + thumbnail: + anyOf: + - type: string + minLength: 1 + format: uri + - type: 'null' + title: Thumbnail + description: a link to the group's thumbnail + type: object + required: + - gid + - label + - description + title: BasicGroupGet Body_service_submission: properties: file: @@ -12881,16 +12910,15 @@ components: type: integer exclusiveMinimum: true title: Gid - description: the group ID + description: the group's unique ID minimum: 0 label: type: string title: Label - description: the group name + description: the group's display name description: type: string title: Description - description: the group description thumbnail: anyOf: - type: string @@ -12898,7 +12926,7 @@ components: format: uri - type: 'null' title: Thumbnail - description: url to the group thumbnail + description: a link to the group's thumbnail accessRights: $ref: '#/components/schemas/GroupAccessRights' inclusionRules: @@ -13607,7 +13635,7 @@ components: - type: 'null' support: anyOf: - - $ref: '#/components/schemas/GroupGet' + - $ref: '#/components/schemas/BasicGroupGet' - type: 'null' description: Group ID of the app support team or None if no support is defined for this product @@ -13650,13 +13678,10 @@ components: gid: '16' label: Blue Fundation support: - accessRights: - delete: false - read: false - write: false description: The support team of the application gid: '2' label: Support Team + thumbnail: https://placekitten.com/15/15 MyPermissionGet: properties: name: From e759c8a4f6a23f7b4010deda2a5ccb65e63fc53d Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 4 Sep 2025 09:30:07 +0200 Subject: [PATCH 37/43] fixes test --- .../unit/with_dbs/03/test_users_rest_profile.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py b/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py index db40aac72ed4..26c9bff70e9f 100644 --- a/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py +++ b/services/web/server/tests/unit/with_dbs/03/test_users_rest_profile.py @@ -28,6 +28,7 @@ from pytest_simcore.helpers.webserver_users import UserInfoDict from servicelib.aiohttp import status from servicelib.rest_constants import RESPONSE_MODEL_POLICY +from simcore_service_webserver.products._models import Product from simcore_service_webserver.user_preferences._service import ( get_frontend_user_preferences_aggregation, ) @@ -133,6 +134,14 @@ async def test_access_update_profile( await assert_status(resp, expected) +@pytest.fixture +def product(client: TestClient, osparc_product_name: str) -> Product: + assert client.app + from simcore_service_webserver.products import products_service + + return products_service.get_product(client.app, osparc_product_name) + + @pytest.mark.parametrize("user_role", [UserRole.USER]) async def test_get_profile_user_not_in_support_group( support_group_before_app_starts: dict[str, Any], @@ -143,6 +152,7 @@ async def test_get_profile_user_not_in_support_group( primary_group: dict[str, Any], standard_groups: list[dict[str, Any]], all_group: dict[str, str], + product: Product, ): assert client.app @@ -168,7 +178,7 @@ async def test_get_profile_user_not_in_support_group( assert got_profile_groups["product"] == { "accessRights": {"delete": False, "read": False, "write": False}, "description": "osparc product group", - "gid": 3, + "gid": product.group_id, "label": "osparc", "thumbnail": None, } @@ -210,6 +220,7 @@ async def test_get_profile_user_in_support_group( primary_group: dict[str, Any], standard_groups: list[dict[str, Any]], all_group: dict[str, str], + product: Product, ): assert client.app from simcore_service_webserver.groups import _groups_repository @@ -244,7 +255,7 @@ async def test_get_profile_user_in_support_group( assert got_profile_groups["product"] == { "accessRights": {"delete": False, "read": False, "write": False}, "description": "osparc product group", - "gid": 3, + "gid": product.group_id, "label": "osparc", "thumbnail": None, } From cec3cffcdc5ddf12980392e3488a14d287f0ad43 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 4 Sep 2025 09:30:41 +0200 Subject: [PATCH 38/43] Update packages/models-library/src/models_library/api_schemas_webserver/groups.py Co-authored-by: Sylvain <35365065+sanderegg@users.noreply.github.com> --- .../src/models_library/api_schemas_webserver/groups.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/models-library/src/models_library/api_schemas_webserver/groups.py b/packages/models-library/src/models_library/api_schemas_webserver/groups.py index 09aa879b0b32..b3963bdd6cc1 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/groups.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/groups.py @@ -54,7 +54,7 @@ class GroupAccessRights(BaseModel): ) -class BasicGroupGet(OutputSchema): +class GroupGetBase(OutputSchema): gid: Annotated[GroupID, Field(description="the group's unique ID")] label: Annotated[str, Field(description="the group's display name")] description: str From a8f12f15e25739484e7b866929db8207b8314a2d Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 4 Sep 2025 09:35:00 +0200 Subject: [PATCH 39/43] @sanderegg review: doc --- .../groups/_groups_repository.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/services/web/server/src/simcore_service_webserver/groups/_groups_repository.py b/services/web/server/src/simcore_service_webserver/groups/_groups_repository.py index 799565e505a3..ce6a7f39aa58 100644 --- a/services/web/server/src/simcore_service_webserver/groups/_groups_repository.py +++ b/services/web/server/src/simcore_service_webserver/groups/_groups_repository.py @@ -110,6 +110,21 @@ async def _get_group_and_access_rights_or_raise( group_id: GroupID, check_permission: Literal["read", "write", "delete"] | None, ) -> Row: + """Fetches a group and its access rights for a specific user (caller_id). + + Arguments: + conn -- Database connection to use for the query. + caller_id -- the user requesting the group information + group_id -- ID of the group to fetch. + check_permission -- Permission to check for the user on the group. If None, no permission check is performed. + + Raises: + GroupNotFoundError: if the group does not exist or the caller is not part of the group. + UserInsufficientRightsError: if the user lacks the specified permission on the group. + + Returns: + Row containing the group details and access rights. + """ result = await conn.execute( sa.select( *_GROUP_COLUMNS, From 3da3fdb3aa68cdb1f7ebcab0ca4cc1fe8091e3c7 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 4 Sep 2025 09:38:40 +0200 Subject: [PATCH 40/43] cleanup --- .../garbage_collector/_core_utils.py | 4 ++-- .../groups/_groups_repository.py | 10 +++++----- .../groups/_groups_service.py | 6 +++--- .../server/src/simcore_service_webserver/groups/api.py | 4 ++-- .../projects/_controller/nodes_rest.py | 6 +++--- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/garbage_collector/_core_utils.py b/services/web/server/src/simcore_service_webserver/garbage_collector/_core_utils.py index 306f90f39702..b3039394086d 100644 --- a/services/web/server/src/simcore_service_webserver/garbage_collector/_core_utils.py +++ b/services/web/server/src/simcore_service_webserver/garbage_collector/_core_utils.py @@ -7,7 +7,7 @@ from models_library.users import UserID from simcore_postgres_database.aiopg_errors import DatabaseError -from ..groups.api import get_group_from_gid +from ..groups.api import get_group_by_gid from ..projects._projects_repository_legacy import ( APP_PROJECT_DBAPI, ProjectAccessRights, @@ -83,7 +83,7 @@ async def get_new_project_owner_gid( standard_groups = {} # groups of users, multiple users can be part of this primary_groups = {} # each individual user has a unique primary group for other_gid in other_users_access_rights: - group: Group | None = await get_group_from_gid(app=app, group_id=int(other_gid)) + group: Group | None = await get_group_by_gid(app=app, group_id=int(other_gid)) # only process for users and groups with write access right if group is None: diff --git a/services/web/server/src/simcore_service_webserver/groups/_groups_repository.py b/services/web/server/src/simcore_service_webserver/groups/_groups_repository.py index ce6a7f39aa58..ae5d1cf0d34e 100644 --- a/services/web/server/src/simcore_service_webserver/groups/_groups_repository.py +++ b/services/web/server/src/simcore_service_webserver/groups/_groups_repository.py @@ -148,19 +148,19 @@ async def _get_group_and_access_rights_or_raise( # -async def get_group_from_gid( +async def get_group_by_gid( app: web.Application, connection: AsyncConnection | None = None, *, group_id: GroupID, ) -> Group | None: async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: - row = await conn.execute( + result = await conn.execute( sa.select(*_GROUP_COLUMNS).where(groups.c.gid == group_id) ) - result = row.first() - if result: - return Group.model_validate(result, from_attributes=True) + row = result.one_or_none() + if row: + return Group.model_validate(row, from_attributes=True) return None diff --git a/services/web/server/src/simcore_service_webserver/groups/_groups_service.py b/services/web/server/src/simcore_service_webserver/groups/_groups_service.py index 1f44670dcac9..860a12223a5e 100644 --- a/services/web/server/src/simcore_service_webserver/groups/_groups_service.py +++ b/services/web/server/src/simcore_service_webserver/groups/_groups_service.py @@ -26,8 +26,8 @@ # -async def get_group_from_gid(app: web.Application, group_id: GroupID) -> Group | None: - group_db = await _groups_repository.get_group_from_gid(app, group_id=group_id) +async def get_group_by_gid(app: web.Application, group_id: GroupID) -> Group | None: + group_db = await _groups_repository.get_group_by_gid(app, group_id=group_id) if group_db: return Group.model_construct(**group_db.model_dump()) @@ -107,7 +107,7 @@ async def get_user_profile_groups( product_support_group = None if product.support_standard_group_id: # Support group is optional # NOTE: my_support_group can be part of groups_by_type.standard! - product_support_group = await get_group_from_gid( + product_support_group = await get_group_by_gid( app, product.support_standard_group_id ) diff --git a/services/web/server/src/simcore_service_webserver/groups/api.py b/services/web/server/src/simcore_service_webserver/groups/api.py index 0c22a3e6a255..d660eedef16a 100644 --- a/services/web/server/src/simcore_service_webserver/groups/api.py +++ b/services/web/server/src/simcore_service_webserver/groups/api.py @@ -5,7 +5,7 @@ add_user_in_group, auto_add_user_to_groups, auto_add_user_to_product_group, - get_group_from_gid, + get_group_by_gid, get_product_group_for_user, get_user_profile_groups, is_user_by_email_in_group, @@ -19,7 +19,7 @@ "add_user_in_group", "auto_add_user_to_groups", "auto_add_user_to_product_group", - "get_group_from_gid", + "get_group_by_gid", "get_product_group_for_user", "get_user_profile_groups", "is_user_by_email_in_group", diff --git a/services/web/server/src/simcore_service_webserver/projects/_controller/nodes_rest.py b/services/web/server/src/simcore_service_webserver/projects/_controller/nodes_rest.py index 8208df41a0ed..a6b9eb0bc691 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_controller/nodes_rest.py +++ b/services/web/server/src/simcore_service_webserver/projects/_controller/nodes_rest.py @@ -57,7 +57,7 @@ from ..._meta import API_VTAG as VTAG from ...catalog import catalog_service from ...dynamic_scheduler import api as dynamic_scheduler_service -from ...groups.api import get_group_from_gid, list_all_user_groups_ids +from ...groups import api as groups_service from ...groups.exceptions import GroupNotFoundError from ...login.decorators import login_required from ...models import ClientSessionHeaderParams @@ -578,7 +578,7 @@ async def get_project_services_access_for_gid(request: web.Request) -> web.Respo groups_to_compare = {EVERYONE_GROUP_ID} # Get the group from the provided group ID - _sharing_with_group: Group | None = await get_group_from_gid( + _sharing_with_group: Group | None = await groups_service.get_group_by_gid( app=request.app, group_id=query_params.for_gid ) @@ -591,7 +591,7 @@ async def get_project_services_access_for_gid(request: web.Request) -> web.Respo _user_id = await users_service.get_user_id_from_gid( app=request.app, primary_gid=query_params.for_gid ) - user_groups_ids = await list_all_user_groups_ids( + user_groups_ids = await groups_service.list_all_user_groups_ids( app=request.app, user_id=_user_id ) groups_to_compare.update(set(user_groups_ids)) From cfece686f147e42860a11100c39abaa74cf537c2 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 4 Sep 2025 09:39:34 +0200 Subject: [PATCH 41/43] @bisgaard-itis review: rename --- .../src/models_library/api_schemas_webserver/groups.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/models-library/src/models_library/api_schemas_webserver/groups.py b/packages/models-library/src/models_library/api_schemas_webserver/groups.py index 09aa879b0b32..88ce7946ecb7 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/groups.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/groups.py @@ -64,7 +64,7 @@ class BasicGroupGet(OutputSchema): @field_validator("thumbnail", mode="before") @classmethod - def _sanitize_legacy_data(cls, v): + def _sanitize_thumbnail_input(cls, v): if v: # Enforces null if thumbnail is not valid URL or empty with suppress(ValidationError): From 08aa00c887b7be6f71ba49f1def3cb9ca70d1164 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 4 Sep 2025 09:44:57 +0200 Subject: [PATCH 42/43] @sanderegg review: rename --- .../src/models_library/api_schemas_webserver/groups.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/models-library/src/models_library/api_schemas_webserver/groups.py b/packages/models-library/src/models_library/api_schemas_webserver/groups.py index 88ce7946ecb7..5f56fbc9790e 100644 --- a/packages/models-library/src/models_library/api_schemas_webserver/groups.py +++ b/packages/models-library/src/models_library/api_schemas_webserver/groups.py @@ -54,7 +54,7 @@ class GroupAccessRights(BaseModel): ) -class BasicGroupGet(OutputSchema): +class GroupGetBase(OutputSchema): gid: Annotated[GroupID, Field(description="the group's unique ID")] label: Annotated[str, Field(description="the group's display name")] description: str @@ -94,7 +94,7 @@ def dump_basic_group_data(cls, group: Group) -> dict: ) -class GroupGet(BasicGroupGet): +class GroupGet(GroupGetBase): access_rights: Annotated[GroupAccessRights, Field(alias="accessRights")] inclusion_rules: Annotated[ @@ -196,7 +196,7 @@ class MyGroupsGet(OutputSchema): all: GroupGet product: GroupGet | None = None support: Annotated[ - BasicGroupGet | None, + GroupGetBase | None, Field( description="Group ID of the app support team or None if no support is defined for this product" ), @@ -271,8 +271,8 @@ def from_domain_model( else None ), support=( - BasicGroupGet.model_validate( - BasicGroupGet.dump_basic_group_data(product_support_group) + GroupGetBase.model_validate( + GroupGetBase.dump_basic_group_data(product_support_group) ) if product_support_group else None From b241651183f6e7f0d79021b791ac94727af207ef Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 4 Sep 2025 09:49:28 +0200 Subject: [PATCH 43/43] updates OAS --- .../api/v0/openapi.yaml | 60 +++++++++---------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml index 8b9f42e95c35..fdc59662ecd3 100644 --- a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml +++ b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml @@ -9284,35 +9284,6 @@ components: - name - email title: Author - BasicGroupGet: - properties: - gid: - type: integer - exclusiveMinimum: true - title: Gid - description: the group's unique ID - minimum: 0 - label: - type: string - title: Label - description: the group's display name - description: - type: string - title: Description - thumbnail: - anyOf: - - type: string - minLength: 1 - format: uri - - type: 'null' - title: Thumbnail - description: a link to the group's thumbnail - type: object - required: - - gid - - label - - description - title: BasicGroupGet Body_service_submission: properties: file: @@ -12942,6 +12913,35 @@ components: - description - accessRights title: GroupGet + GroupGetBase: + properties: + gid: + type: integer + exclusiveMinimum: true + title: Gid + description: the group's unique ID + minimum: 0 + label: + type: string + title: Label + description: the group's display name + description: + type: string + title: Description + thumbnail: + anyOf: + - type: string + minLength: 1 + format: uri + - type: 'null' + title: Thumbnail + description: a link to the group's thumbnail + type: object + required: + - gid + - label + - description + title: GroupGetBase GroupUpdate: properties: label: @@ -13635,7 +13635,7 @@ components: - type: 'null' support: anyOf: - - $ref: '#/components/schemas/BasicGroupGet' + - $ref: '#/components/schemas/GroupGetBase' - type: 'null' description: Group ID of the app support team or None if no support is defined for this product