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 1528404fdb47..69a61e8c7fdd 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 @@ -1,7 +1,7 @@ import re from datetime import date, datetime from enum import Enum -from typing import Annotated, Any, Literal, Self +from typing import Annotated, Any, Literal, Self, TypeAlias import annotated_types from common_library.basic_types import DEFAULT_FACTORY @@ -18,6 +18,7 @@ StringConstraints, ValidationInfo, field_validator, + model_validator, ) from pydantic.config import JsonDict @@ -83,7 +84,14 @@ class MyProfileRestGet(OutputSchemaWithoutCamelCase): login: LowerCaseEmailStr phone: str | None = None - role: Literal["ANONYMOUS", "GUEST", "USER", "TESTER", "PRODUCT_OWNER", "ADMIN"] + role: Literal[ + "ANONYMOUS", + "GUEST", + "USER", + "TESTER", + "PRODUCT_OWNER", + "ADMIN", + ] groups: MyGroupsGet | None = None gravatar_id: Annotated[str | None, Field(deprecated=True)] = None @@ -306,15 +314,41 @@ class UserAccountReject(InputSchema): email: EmailStr +GlobString: TypeAlias = Annotated[ + str, + StringConstraints( + min_length=3, max_length=200, strip_whitespace=True, pattern=r"^[^%]*$" + ), +] + + class UserAccountSearchQueryParams(RequestParameters): email: Annotated[ - str, + GlobString | None, Field( - min_length=3, - max_length=200, description="complete or glob pattern for an email", ), - ] + ] = None + primary_group_id: Annotated[ + GroupID | None, + Field( + description="Filter by primary group ID", + ), + ] = None + user_name: Annotated[ + GlobString | None, + Field( + description="complete or glob pattern for a username", + ), + ] = None + + @model_validator(mode="after") + def _validate_at_least_one_filter(self) -> Self: + field_names = list(self.__class__.model_fields) + if not any(getattr(self, field_name, None) for field_name in field_names): + msg = f"At least one filter {field_names} must be provided" + raise ValueError(msg) + return self class UserAccountGet(OutputSchema): @@ -340,9 +374,9 @@ class UserAccountGet(OutputSchema): # pre-registration NOTE: that some users have no pre-registartion and therefore all options here can be none pre_registration_id: int | None pre_registration_created: datetime | None - invited_by: str | None = None + invited_by: UserNameID | None = None account_request_status: AccountRequestStatus | None - account_request_reviewed_by: UserID | None = None + account_request_reviewed_by: UserNameID | None = None account_request_reviewed_at: datetime | None = None # user status diff --git a/packages/models-library/src/models_library/users.py b/packages/models-library/src/models_library/users.py index 49848ebae590..eba810e7df9d 100644 --- a/packages/models-library/src/models_library/users.py +++ b/packages/models-library/src/models_library/users.py @@ -2,7 +2,6 @@ from typing import Annotated, TypeAlias from common_library.users_enums import UserRole -from models_library.basic_types import IDStr from pydantic import BaseModel, ConfigDict, Field, PositiveInt, StringConstraints from pydantic.config import JsonDict from typing_extensions import ( # https://docs.pydantic.dev/latest/api/standard_library_types/#typeddict @@ -12,8 +11,9 @@ from .emails import LowerCaseEmailStr UserID: TypeAlias = PositiveInt -UserNameID: TypeAlias = IDStr - +UserNameID: TypeAlias = Annotated[ + str, StringConstraints(strip_whitespace=True, min_length=1, max_length=100) +] FirstNameStr: TypeAlias = Annotated[ str, StringConstraints(strip_whitespace=True, max_length=255) diff --git a/services/api-server/setup.cfg b/services/api-server/setup.cfg index e263a7a4602e..30f46cc2b6fe 100644 --- a/services/api-server/setup.cfg +++ b/services/api-server/setup.cfg @@ -11,12 +11,12 @@ commit_args = --no-verify asyncio_mode = auto asyncio_default_fixture_loop_scope = function addopts = --strict-markers -markers = +markers = slow: marks tests as slow (deselect with '-m "not slow"') acceptance_test: "marks tests as 'acceptance tests' i.e. does the system do what the user expects? Typically those are workflows." testit: "marks test to run during development" [mypy] -plugins = +plugins = pydantic.mypy sqlalchemy.ext.mypy.plugin diff --git a/services/api-server/src/simcore_service_api_server/models/schemas/profiles.py b/services/api-server/src/simcore_service_api_server/models/schemas/profiles.py index 76b283aa4a99..e70ad1c262d1 100644 --- a/services/api-server/src/simcore_service_api_server/models/schemas/profiles.py +++ b/services/api-server/src/simcore_service_api_server/models/schemas/profiles.py @@ -13,8 +13,7 @@ class ProfileCommon(BaseModel): last_name: LastNameStr | None = Field(None, examples=["Maxwell"]) -class ProfileUpdate(ProfileCommon): - ... +class ProfileUpdate(ProfileCommon): ... class UserRoleEnum(StrAutoEnum): diff --git a/services/web/server/VERSION b/services/web/server/VERSION index 4a7076db09af..5d3aa5778624 100644 --- a/services/web/server/VERSION +++ b/services/web/server/VERSION @@ -1 +1 @@ -0.77.0 +0.77.2 diff --git a/services/web/server/setup.cfg b/services/web/server/setup.cfg index 5ee0c6c434ce..76a3da7ee2b0 100644 --- a/services/web/server/setup.cfg +++ b/services/web/server/setup.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 0.77.0 +current_version = 0.77.2 commit = True message = services/webserver api version: {current_version} → {new_version} tag = False @@ -13,13 +13,13 @@ commit_args = --no-verify addopts = --strict-markers asyncio_mode = auto asyncio_default_fixture_loop_scope = function -markers = +markers = slow: marks tests as slow (deselect with '-m "not slow"') acceptance_test: "marks tests as 'acceptance tests' i.e. does the system do what the user expects? Typically those are workflows." testit: "marks test to run during development" heavy_load: "mark tests that require large amount of data" [mypy] -plugins = +plugins = pydantic.mypy sqlalchemy.ext.mypy.plugin 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 8557fa526c92..bb23bd92f0ee 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.77.0 + version: 0.77.2 servers: - url: '' description: webserver @@ -1798,12 +1798,36 @@ paths: parameters: - name: email in: query - required: true + required: false schema: - type: string - minLength: 3 - maxLength: 200 + anyOf: + - type: string + minLength: 3 + maxLength: 200 + pattern: ^[^%]*$ + - type: 'null' title: Email + - name: primary_group_id + in: query + required: false + schema: + anyOf: + - type: integer + exclusiveMinimum: true + minimum: 0 + - type: 'null' + title: Primary Group Id + - name: user_name + in: query + required: false + schema: + anyOf: + - type: string + minLength: 3 + maxLength: 200 + pattern: ^[^%]*$ + - type: 'null' + title: User Name responses: '200': description: Successful Response @@ -18361,6 +18385,8 @@ components: invitedBy: anyOf: - type: string + maxLength: 100 + minLength: 1 - type: 'null' title: Invitedby accountRequestStatus: @@ -18369,9 +18395,9 @@ components: - type: 'null' accountRequestReviewedBy: anyOf: - - type: integer - exclusiveMinimum: true - minimum: 0 + - type: string + maxLength: 100 + minLength: 1 - type: 'null' title: Accountrequestreviewedby accountRequestReviewedAt: 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 ae5d1cf0d34e..ede591b76589 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 @@ -6,7 +6,6 @@ from aiohttp import web from common_library.groups_enums import GroupType from common_library.users_enums import UserRole -from models_library.basic_types import IDStr from models_library.groups import ( AccessRightsDict, Group, @@ -17,7 +16,7 @@ StandardGroupCreate, StandardGroupUpdate, ) -from models_library.users import UserID +from models_library.users import UserID, UserNameID from simcore_postgres_database.aiopg_errors import UniqueViolation from simcore_postgres_database.models.users import users from simcore_postgres_database.utils_products import get_or_create_product_group @@ -732,6 +731,22 @@ async def is_user_by_email_in_group( return user_id is not None +async def is_user_in_group( + app: web.Application, + connection: AsyncConnection | None = None, + *, + user_id: UserID, + group_id: GroupID, +) -> bool: + async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: + result = await conn.scalar( + sa.select(user_to_groups.c.uid).where( + (user_to_groups.c.uid == user_id) & (user_to_groups.c.gid == group_id) + ) + ) + return result is not None + + async def add_new_user_in_group( app: web.Application, connection: AsyncConnection | None = None, @@ -739,7 +754,7 @@ async def add_new_user_in_group( group_id: GroupID, # either user_id or user_name new_user_id: UserID | None = None, - new_user_name: IDStr | None = None, + new_user_name: UserNameID | None = None, access_rights: AccessRightsDict | None = None, ) -> 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 860a12223a5e..501adea3df06 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,7 +1,6 @@ from contextlib import suppress from aiohttp import web -from models_library.basic_types import IDStr from models_library.emails import LowerCaseEmailStr from models_library.groups import ( AccessRightsDict, @@ -13,7 +12,7 @@ StandardGroupUpdate, ) from models_library.products import ProductName -from models_library.users import UserID +from models_library.users import UserID, UserNameID from pydantic import EmailStr from ..products.models import Product @@ -262,6 +261,14 @@ async def is_user_by_email_in_group( ) +async def is_user_in_group( + app: web.Application, *, user_id: UserID, group_id: GroupID +) -> bool: + return await _groups_repository.is_user_in_group( + app, user_id=user_id, group_id=group_id + ) + + async def auto_add_user_to_groups(app: web.Application, user_id: UserID) -> None: user: dict = await users_service.get_user(app, user_id) return await _groups_repository.auto_add_user_to_groups(app, user=user) @@ -288,7 +295,7 @@ async def add_user_in_group( *, # identifies new_by_user_id: UserID | None = None, - new_by_user_name: IDStr | None = None, + new_by_user_name: UserNameID | None = None, new_by_user_email: EmailStr | None = None, access_rights: AccessRightsDict | None = None, ) -> None: 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 d660eedef16a..99d962a37eb4 100644 --- a/services/web/server/src/simcore_service_webserver/groups/api.py +++ b/services/web/server/src/simcore_service_webserver/groups/api.py @@ -9,6 +9,7 @@ get_product_group_for_user, get_user_profile_groups, is_user_by_email_in_group, + is_user_in_group, list_all_user_groups_ids, list_group_members, list_user_groups_ids_with_read_access, @@ -23,6 +24,7 @@ "get_product_group_for_user", "get_user_profile_groups", "is_user_by_email_in_group", + "is_user_in_group", "list_all_user_groups_ids", "list_group_members", "list_user_groups_ids_with_read_access", diff --git a/services/web/server/src/simcore_service_webserver/products/_controller/rest.py b/services/web/server/src/simcore_service_webserver/products/_controller/rest.py index 77c72afe3b0c..f82b1f4b1ace 100644 --- a/services/web/server/src/simcore_service_webserver/products/_controller/rest.py +++ b/services/web/server/src/simcore_service_webserver/products/_controller/rest.py @@ -10,7 +10,10 @@ from ..._meta import API_VTAG as VTAG from ...login.decorators import login_required -from ...security.decorators import permission_required +from ...security.decorators import ( + group_or_role_permission_required, + permission_required, +) from ...utils_aiohttp import envelope_json_response from .. import _service, products_web from .._repository import ProductRepository @@ -46,7 +49,7 @@ async def _get_current_product_price(request: web.Request): @routes.get(f"/{VTAG}/products/{{product_name}}", name="get_product") @login_required -@permission_required("product.details.*") +@group_or_role_permission_required("product.details.*") @handle_rest_requests_exceptions async def _get_product(request: web.Request): req_ctx = ProductsRequestContext.model_validate(request) diff --git a/services/web/server/src/simcore_service_webserver/products/_web_helpers.py b/services/web/server/src/simcore_service_webserver/products/_web_helpers.py index 90b5c07b3e5c..b6f5f15bfbe6 100644 --- a/services/web/server/src/simcore_service_webserver/products/_web_helpers.py +++ b/services/web/server/src/simcore_service_webserver/products/_web_helpers.py @@ -4,10 +4,12 @@ import aiofiles from aiohttp import web from models_library.products import ProductName +from models_library.users import UserID from simcore_postgres_database.utils_products_prices import ProductPriceInfo from .._resources import webserver_resources from ..constants import RQ_PRODUCT_KEY +from ..groups import api as groups_service from . import _service from ._web_events import APP_PRODUCTS_TEMPLATES_DIR_KEY from .errors import ( @@ -38,6 +40,22 @@ def get_current_product(request: web.Request) -> Product: return current_product +async def is_user_in_product_support_group( + request: web.Request, *, user_id: UserID +) -> bool: + """Checks if the user belongs to the support group of the given product. + If the product does not have a support group, returns False. + """ + product = get_current_product(request) + if product.support_standard_group_id is None: + return False + return await groups_service.is_user_in_group( + app=request.app, + user_id=user_id, + group_id=product.support_standard_group_id, + ) + + def _get_current_product_or_none(request: web.Request) -> Product | None: with contextlib.suppress(ProductNotFoundError, UnknownProductError): product: Product = get_current_product(request) diff --git a/services/web/server/src/simcore_service_webserver/products/products_web.py b/services/web/server/src/simcore_service_webserver/products/products_web.py index 38ddb1634ece..e22c97407dc4 100644 --- a/services/web/server/src/simcore_service_webserver/products/products_web.py +++ b/services/web/server/src/simcore_service_webserver/products/products_web.py @@ -3,6 +3,7 @@ get_current_product_credit_price_info, get_product_name, get_product_template_path, + is_user_in_product_support_group, ) __all__: tuple[str, ...] = ( @@ -10,5 +11,6 @@ "get_current_product_credit_price_info", "get_product_name", "get_product_template_path", + "is_user_in_product_support_group", ) # nopycln: file diff --git a/services/web/server/src/simcore_service_webserver/security/_authz_access_roles.py b/services/web/server/src/simcore_service_webserver/security/_authz_access_roles.py index 270947d3bae4..7c8c9e0e4378 100644 --- a/services/web/server/src/simcore_service_webserver/security/_authz_access_roles.py +++ b/services/web/server/src/simcore_service_webserver/security/_authz_access_roles.py @@ -107,10 +107,10 @@ class PermissionDict(TypedDict, total=False): UserRole.PRODUCT_OWNER: PermissionDict( # NOTE: Add `tags=["po"]` to entrypoints with this access requirements can=[ - "product.details.*", - "product.invitations.create", "admin.users.read", "admin.users.write", + "product.details.*", + "product.invitations.create", ], inherits=[UserRole.TESTER], ), @@ -128,3 +128,14 @@ class PermissionDict(TypedDict, total=False): assert set(ROLES_PERMISSIONS) == set( # nosec UserRole ), "All user roles must be part define permissions" # nosec + + +# Group-based permissions for named groups (e.g. PRODUCT_SUPPORT_GROUP) +# Maps group type to list of permissions that group members can perform +NAMED_GROUP_PERMISSIONS: dict[str, list[str]] = { + "PRODUCT_SUPPORT_GROUP": [ + "product.details.*", + "admin.users.read", + ], + # NOTE: Future group types can be added here +} diff --git a/services/web/server/src/simcore_service_webserver/security/_authz_policy.py b/services/web/server/src/simcore_service_webserver/security/_authz_policy.py index 0df137dece45..c54eeb4d196c 100644 --- a/services/web/server/src/simcore_service_webserver/security/_authz_policy.py +++ b/services/web/server/src/simcore_service_webserver/security/_authz_policy.py @@ -10,6 +10,7 @@ from aiohttp_security.abc import ( # type: ignore[import-untyped] AbstractAuthorizationPolicy, ) +from common_library.users_enums import UserRole from models_library.products import ProductName from models_library.users import UserID from servicelib.aiohttp.db_asyncpg_engine import get_async_engine @@ -17,17 +18,15 @@ from simcore_postgres_database.aiopg_errors import DatabaseError as AiopgDatabaseError from sqlalchemy.exc import DatabaseError as SQLAlchemyDatabaseError +from . import _authz_repository from ._authz_access_model import ( AuthContextDict, OptionalContext, RoleBasedAccessModel, has_access_by_role, ) -from ._authz_repository import ( - ActiveUserIdAndRole, - get_active_user_or_none, - is_user_in_product_name, -) +from ._authz_access_roles import NAMED_GROUP_PERMISSIONS +from ._authz_repository import ActiveUserIdAndRole from ._constants import MSG_AUTH_NOT_AVAILABLE, PERMISSION_PRODUCT_LOGIN_KEY from ._identity_web import IdentityStr @@ -79,7 +78,7 @@ async def _get_authorized_user_or_none( web.HTTPServiceUnavailable: if database raises an exception """ with _handle_exceptions_as_503(): - return await get_active_user_or_none( + return await _authz_repository.get_active_user_or_none( get_async_engine(self._app), email=email ) @@ -96,17 +95,36 @@ async def _has_access_to_product( web.HTTPServiceUnavailable: if database raises an exception """ with _handle_exceptions_as_503(): - return await is_user_in_product_name( + return await _authz_repository.is_user_in_product_name( get_async_engine(self._app), user_id=user_id, product_name=product_name ) + @cached( + ttl=_AUTHZ_BURST_CACHE_TTL, + namespace=__name__, + key_builder=lambda f, *ag, **kw: f"{f.__name__}/{kw['user_id']}/{kw['group_id']}", + ) + async def _is_user_in_group(self, *, user_id: UserID, group_id: int) -> bool: + """ + Raises: + web.HTTPServiceUnavailable: if database raises an exception + """ + with _handle_exceptions_as_503(): + return await _authz_repository.is_user_in_group( + get_async_engine(self._app), user_id=user_id, group_id=group_id + ) + @property def access_model(self) -> RoleBasedAccessModel: return self._access_model async def clear_cache(self): # pylint: disable=no-member - for fun in (self._get_authorized_user_or_none, self._has_access_to_product): + for fun in ( + self._get_authorized_user_or_none, + self._has_access_to_product, + self._is_user_in_group, + ): autz_cache: BaseCache = fun.cache await autz_cache.clear() @@ -161,17 +179,33 @@ async def permits( "authorized_uid" ), f"{user_id}!={context.get('authorized_uid')}" - # product access + # PRODUCT access if permission == PERMISSION_PRODUCT_LOGIN_KEY: ok: bool = product_name is not None and await self._has_access_to_product( user_id=user_id, product_name=product_name ) return ok - # role-based access - return await has_access_by_role( + # ROLE-BASED access policy + role_allowed = await has_access_by_role( self._access_model, role=user_role, operation=permission, context=context, ) + + if role_allowed: + return True + + # GROUP-BASED access policy (only if enabled in context and user is above GUEST role) + product_support_group_id = context.get("product_support_group_id", None) + group_allowed = ( + product_support_group_id is not None + and user_role > UserRole.GUEST + and permission in NAMED_GROUP_PERMISSIONS.get("PRODUCT_SUPPORT_GROUP", []) + and await self._is_user_in_group( + user_id=user_id, group_id=product_support_group_id + ) + ) + + return group_allowed # noqa: RET504 diff --git a/services/web/server/src/simcore_service_webserver/security/_authz_repository.py b/services/web/server/src/simcore_service_webserver/security/_authz_repository.py index c6b6389ad8df..2f58de8328ea 100644 --- a/services/web/server/src/simcore_service_webserver/security/_authz_repository.py +++ b/services/web/server/src/simcore_service_webserver/security/_authz_repository.py @@ -3,6 +3,7 @@ import sqlalchemy as sa from models_library.basic_types import IdInt +from models_library.groups import GroupID from models_library.products import ProductName from models_library.users import UserID from pydantic import TypeAdapter @@ -65,3 +66,18 @@ async def is_user_in_product_name( ) is not None ) + + +async def is_user_in_group( + engine: AsyncEngine, + *, + user_id: UserID, + group_id: GroupID, +) -> bool: + async with engine.connect() as conn: + result = await conn.scalar( + sa.select(user_to_groups.c.uid).where( + (user_to_groups.c.uid == user_id) & (user_to_groups.c.gid == group_id) + ) + ) + return result is not None diff --git a/services/web/server/src/simcore_service_webserver/security/_authz_web.py b/services/web/server/src/simcore_service_webserver/security/_authz_web.py index 8e88269918d3..8f9f1042ef18 100644 --- a/services/web/server/src/simcore_service_webserver/security/_authz_web.py +++ b/services/web/server/src/simcore_service_webserver/security/_authz_web.py @@ -54,3 +54,29 @@ async def check_user_permission( else: msg += f" {permission}" raise web.HTTPForbidden(text=msg) + + +async def check_user_permission_with_groups( + request: web.Request, permission: str +) -> None: + """Checker that passes to authorized users with given permission via roles OR groups. + + Raises: + web.HTTPUnauthorized: If user is not authorized + web.HTTPForbidden: If user is authorized but lacks both role and group permissions + """ + from ..products import products_web + + context = { + "authorized_uid": await check_user_authorized(request), + "enable_group_permissions": True, + "request": request, + "product_support_group_id": products_web.get_current_product( + request + ).support_standard_group_id, + } + + allowed = await aiohttp_security.api.permits(request, permission, context) + if not allowed: + msg = f"You do not have sufficient access rights for {permission}" + raise web.HTTPForbidden(text=msg) diff --git a/services/web/server/src/simcore_service_webserver/security/decorators.py b/services/web/server/src/simcore_service_webserver/security/decorators.py index 6542c07ed138..945db63d278e 100644 --- a/services/web/server/src/simcore_service_webserver/security/decorators.py +++ b/services/web/server/src/simcore_service_webserver/security/decorators.py @@ -3,6 +3,7 @@ from aiohttp import web from servicelib.aiohttp.typing_extension import Handler +from ._authz_web import check_user_permission_with_groups from .security_web import check_user_permission @@ -26,3 +27,24 @@ async def _wrapped(request: web.Request): return _wrapped return _decorator + + +def group_or_role_permission_required(permission: str): + """Decorator that checks user permissions via role or gorup membership + + User gets access if they have permission via role OR group membership. + + If user is not authorized - raises HTTPUnauthorized, + if user is authorized but lacks both role and group permissions - raises HTTPForbidden. + """ + + def _decorator(handler: Handler): + @wraps(handler) + async def _wrapped(request: web.Request): + await check_user_permission_with_groups(request, permission) + + return await handler(request) + + return _wrapped + + return _decorator diff --git a/services/web/server/src/simcore_service_webserver/users/_accounts_repository.py b/services/web/server/src/simcore_service_webserver/users/_accounts_repository.py index 23090b2bcc91..567b1834419a 100644 --- a/services/web/server/src/simcore_service_webserver/users/_accounts_repository.py +++ b/services/web/server/src/simcore_service_webserver/users/_accounts_repository.py @@ -14,7 +14,6 @@ from simcore_postgres_database.models.users_details import ( users_pre_registration_details, ) -from simcore_postgres_database.utils import as_postgres_sql_query_str from simcore_postgres_database.utils_repos import ( pass_or_acquire_connection, transaction_context, @@ -268,13 +267,90 @@ async def review_user_pre_registration( # +def _create_account_request_reviewed_by_username_subquery() -> Any: + """Creates a reusable subquery for getting reviewer username by ID.""" + reviewer_alias = sa.alias(users, name="reviewer_alias") + return ( + sa.select( + reviewer_alias.c.name, + ) + .where( + users_pre_registration_details.c.account_request_reviewed_by + == reviewer_alias.c.id + ) + .label("account_request_reviewed_by_username") + ) + + +def _build_left_outer_join_query( + email_like: str | None, + product_name: ProductName | None, + columns: tuple, +) -> sa.sql.Select | None: + left_where_conditions = [] + if email_like is not None: + left_where_conditions.append( + users_pre_registration_details.c.pre_email.like(email_like) + ) + join_condition = users.c.id == users_pre_registration_details.c.user_id + if product_name: + join_condition = join_condition & ( + users_pre_registration_details.c.product_name == product_name + ) + left_outer_join = sa.select(*columns).select_from( + users_pre_registration_details.outerjoin(users, join_condition) + ) + + return ( + left_outer_join.where(sa.and_(*left_where_conditions)) + if left_where_conditions + else None + ) + + +def _build_right_outer_join_query( + email_like: str | None, + user_name_like: str | None, + primary_group_id: int | None, + product_name: ProductName | None, + columns: tuple, +) -> sa.sql.Select | None: + right_where_conditions = [] + if email_like is not None: + right_where_conditions.append(users.c.email.like(email_like)) + if user_name_like is not None: + right_where_conditions.append(users.c.name.like(user_name_like)) + if primary_group_id is not None: + right_where_conditions.append(users.c.primary_gid == primary_group_id) + join_condition = users.c.id == users_pre_registration_details.c.user_id + if product_name: + join_condition = join_condition & ( + users_pre_registration_details.c.product_name == product_name + ) + right_outer_join = sa.select(*columns).select_from( + users.outerjoin( + users_pre_registration_details, + join_condition, + ) + ) + + return ( + right_outer_join.where(sa.and_(*right_where_conditions)) + if right_where_conditions + else None + ) + + async def search_merged_pre_and_registered_users( engine: AsyncEngine, connection: AsyncConnection | None = None, *, - email_like: str, + filter_by_email_like: str | None = None, + filter_by_user_name_like: str | None = None, + filter_by_primary_group_id: int | None = None, product_name: ProductName | None = None, ) -> list[Row]: + """Searches and merges users from both users and pre-registration tables""" users_alias = sa.alias(users, name="users_alias") invited_by = ( @@ -285,59 +361,63 @@ async def search_merged_pre_and_registered_users( .label("invited_by") ) - async with pass_or_acquire_connection(engine, connection) as conn: - columns = ( - users_pre_registration_details.c.id, - users.c.first_name, - users.c.last_name, - users.c.email, - users.c.phone, - users_pre_registration_details.c.pre_email, - users_pre_registration_details.c.pre_first_name, - users_pre_registration_details.c.pre_last_name, - users_pre_registration_details.c.institution, - users_pre_registration_details.c.pre_phone, - users_pre_registration_details.c.address, - users_pre_registration_details.c.city, - users_pre_registration_details.c.state, - users_pre_registration_details.c.postal_code, - users_pre_registration_details.c.country, - users_pre_registration_details.c.user_id, - users_pre_registration_details.c.extras, - users_pre_registration_details.c.account_request_status, - users_pre_registration_details.c.account_request_reviewed_by, - users_pre_registration_details.c.account_request_reviewed_at, - users.c.status, - invited_by, - users_pre_registration_details.c.created, - ) + account_request_reviewed_by_username = ( + _create_account_request_reviewed_by_username_subquery() + ) - join_condition = users.c.id == users_pre_registration_details.c.user_id - if product_name: - join_condition = join_condition & ( - users_pre_registration_details.c.product_name == product_name - ) + columns = ( + users_pre_registration_details.c.id, + users.c.first_name, + users.c.last_name, + users.c.email, + users.c.phone, + users_pre_registration_details.c.pre_email, + users_pre_registration_details.c.pre_first_name, + users_pre_registration_details.c.pre_last_name, + users_pre_registration_details.c.institution, + users_pre_registration_details.c.pre_phone, + users_pre_registration_details.c.address, + users_pre_registration_details.c.city, + users_pre_registration_details.c.state, + users_pre_registration_details.c.postal_code, + users_pre_registration_details.c.country, + users_pre_registration_details.c.user_id, + users_pre_registration_details.c.extras, + users_pre_registration_details.c.account_request_status, + users_pre_registration_details.c.account_request_reviewed_by, + users_pre_registration_details.c.account_request_reviewed_at, + users.c.status, + invited_by, + account_request_reviewed_by_username, # account_request_reviewed_by converted to username + users_pre_registration_details.c.created, + ) - left_outer_join = ( - sa.select(*columns) - .select_from( - users_pre_registration_details.outerjoin(users, join_condition) - ) - .where(users_pre_registration_details.c.pre_email.like(email_like)) - ) - right_outer_join = ( - sa.select(*columns) - .select_from( - users.outerjoin( - users_pre_registration_details, - join_condition, - ) - ) - .where(users.c.email.like(email_like)) - ) + left_outer_join = _build_left_outer_join_query( + filter_by_email_like, product_name, columns + ) + right_outer_join = _build_right_outer_join_query( + filter_by_email_like, + filter_by_user_name_like, + filter_by_primary_group_id, + product_name, + columns, + ) + + queries = [] + if left_outer_join is not None: + queries.append(left_outer_join) + if right_outer_join is not None: + queries.append(right_outer_join) - result = await conn.stream(sa.union(left_outer_join, right_outer_join)) - return [row async for row in result] + if not queries: + # No search criteria provided, return empty result + return [] + + final_query = queries[0] if len(queries) == 1 else sa.union(*queries) + + async with pass_or_acquire_connection(engine, connection) as conn: + result = await conn.execute(final_query) + return result.fetchall() async def list_merged_pre_and_registered_users( @@ -386,7 +466,12 @@ async def list_merged_pre_and_registered_users( if not filter_include_deleted: users_where.append(users.c.status != UserStatus.DELETED) - # Query for pre-registered users that are not yet in the users table + # Create subquery for reviewer username + account_request_reviewed_by_username = ( + _create_account_request_reviewed_by_username_subquery() + ) + + # Query for pre-registered users # We need to left join with users to identify if the pre-registered user is already in the system pre_reg_query = ( sa.select( @@ -412,7 +497,8 @@ async def list_merged_pre_and_registered_users( users.c.status, # Use created_by directly instead of a subquery users_pre_registration_details.c.created_by.label("created_by"), - sa.literal(True).label("is_pre_registered"), + account_request_reviewed_by_username, + sa.literal_column("true").label("is_pre_registered"), ) .select_from( users_pre_registration_details.outerjoin( @@ -447,7 +533,8 @@ async def list_merged_pre_and_registered_users( users.c.status, # Match the created_by field from the pre_reg query sa.literal(None).label("created_by"), - sa.literal(False).label("is_pre_registered"), + sa.literal(None).label("account_request_reviewed_by_username"), + sa.literal_column("false").label("is_pre_registered"), ) .select_from( users.join(user_to_groups, user_to_groups.c.uid == users.c.id) @@ -489,14 +576,6 @@ async def list_merged_pre_and_registered_users( .subquery() ) - _logger.debug( - "%s\n%s\n%s\n%s", - "-" * 100, - as_postgres_sql_query_str(distinct_query), - "-" * 100, - as_postgres_sql_query_str(count_query), - ) - async with pass_or_acquire_connection(engine, connection) as conn: # Get total count count_result = await conn.execute(count_query) diff --git a/services/web/server/src/simcore_service_webserver/users/_accounts_service.py b/services/web/server/src/simcore_service_webserver/users/_accounts_service.py index 7d2b3be98a06..99aab9bb6cae 100644 --- a/services/web/server/src/simcore_service_webserver/users/_accounts_service.py +++ b/services/web/server/src/simcore_service_webserver/users/_accounts_service.py @@ -36,7 +36,10 @@ async def pre_register_user( ) -> UserAccountGet: found = await search_users_accounts( - app, email_glob=profile.email, product_name=product_name, include_products=False + app, + filter_by_email_glob=profile.email, + product_name=product_name, + include_products=False, ) if found: raise AlreadyPreRegisteredError(num_found=len(found), email=profile.email) @@ -70,7 +73,10 @@ async def pre_register_user( ) found = await search_users_accounts( - app, email_glob=profile.email, product_name=product_name, include_products=False + app, + filter_by_email_glob=profile.email, + product_name=product_name, + include_products=False, ) assert len(found) == 1 # nosec @@ -143,7 +149,9 @@ async def list_user_accounts( async def search_users_accounts( app: web.Application, *, - email_glob: str, + filter_by_email_glob: str | None = None, + filter_by_primary_group_id: int | None = None, + filter_by_user_name_glob: str | None = None, product_name: ProductName | None = None, include_products: bool = False, ) -> list[UserAccountGet]: @@ -156,6 +164,14 @@ async def search_users_accounts( no caller or context is available. """ + if ( + filter_by_email_glob is None + and filter_by_user_name_glob is None + and filter_by_primary_group_id is None + ): + msg = "At least one filter (email glob, user name like, or primary group ID) must be provided" + raise ValueError(msg) + def _glob_to_sql_like(glob_pattern: str) -> str: # Escape SQL LIKE special characters in the glob pattern sql_like_pattern = glob_pattern.replace("%", r"\%").replace("_", r"\_") @@ -164,7 +180,15 @@ def _glob_to_sql_like(glob_pattern: str) -> str: rows = await _accounts_repository.search_merged_pre_and_registered_users( get_asyncpg_engine(app), - email_like=_glob_to_sql_like(email_glob), + filter_by_email_like=( + _glob_to_sql_like(filter_by_email_glob) if filter_by_email_glob else None + ), + filter_by_primary_group_id=filter_by_primary_group_id, + filter_by_user_name_like=( + _glob_to_sql_like(filter_by_user_name_glob) + if filter_by_user_name_glob + else None + ), product_name=product_name, ) @@ -193,7 +217,7 @@ async def _list_products_or_none(user_id): pre_registration_id=r.id, pre_registration_created=r.created, account_request_status=r.account_request_status, - account_request_reviewed_by=r.account_request_reviewed_by, + account_request_reviewed_by=r.account_request_reviewed_by_username, account_request_reviewed_at=r.account_request_reviewed_at, products=await _list_products_or_none(r.user_id), # NOTE: old users will not have extra details diff --git a/services/web/server/src/simcore_service_webserver/users/_controller/rest/accounts_rest.py b/services/web/server/src/simcore_service_webserver/users/_controller/rest/accounts_rest.py index 8793843b246c..81405e0ef19c 100644 --- a/services/web/server/src/simcore_service_webserver/users/_controller/rest/accounts_rest.py +++ b/services/web/server/src/simcore_service_webserver/users/_controller/rest/accounts_rest.py @@ -26,7 +26,10 @@ from ...._meta import API_VTAG from ....invitations import api as invitations_service from ....login.decorators import login_required -from ....security.decorators import permission_required +from ....security.decorators import ( + group_or_role_permission_required, + permission_required, +) from ....utils_aiohttp import create_json_response_from_page, envelope_json_response from ... import _accounts_service from ._rest_exceptions import handle_rest_requests_exceptions @@ -43,7 +46,7 @@ @routes.get(f"/{API_VTAG}/admin/user-accounts", name="list_users_accounts") @login_required -@permission_required("admin.users.read") +@group_or_role_permission_required("admin.users.read") @handle_rest_requests_exceptions async def list_users_accounts(request: web.Request) -> web.Response: req_ctx = UsersRequestContext.model_validate(request) @@ -64,7 +67,7 @@ async def list_users_accounts(request: web.Request) -> web.Response: # ALL filter_any_account_request_status = None - users, total_count = await _accounts_service.list_user_accounts( + user_accounts, total_count = await _accounts_service.list_user_accounts( request.app, product_name=req_ctx.product_name, filter_any_account_request_status=filter_any_account_request_status, @@ -72,17 +75,21 @@ async def list_users_accounts(request: web.Request) -> web.Response: pagination_offset=query_params.offset, ) - def _to_domain_model(user: dict[str, Any]) -> UserAccountGet: + def _to_domain_model(account_details: dict[str, Any]) -> UserAccountGet: + account_details.pop("account_request_reviewed_by", None) return UserAccountGet( - extras=user.pop("extras") or {}, - pre_registration_id=user.pop("id"), - pre_registration_created=user.pop("created"), - **user, + extras=account_details.pop("extras") or {}, + pre_registration_id=account_details.pop("id"), + pre_registration_created=account_details.pop("created"), + account_request_reviewed_by=account_details.pop( + "account_request_reviewed_by_username" + ), + **account_details, ) page = Page[UserAccountGet].model_validate( paginate_data( - chunk=[_to_domain_model(user) for user in users], + chunk=[_to_domain_model(user) for user in user_accounts], request_url=request.url, total=total_count, limit=query_params.limit, @@ -95,7 +102,7 @@ def _to_domain_model(user: dict[str, Any]) -> UserAccountGet: @routes.get(f"/{API_VTAG}/admin/user-accounts:search", name="search_user_accounts") @login_required -@permission_required("admin.users.read") +@group_or_role_permission_required("admin.users.read") @handle_rest_requests_exceptions async def search_user_accounts(request: web.Request) -> web.Response: req_ctx = UsersRequestContext.model_validate(request) @@ -106,7 +113,11 @@ async def search_user_accounts(request: web.Request) -> web.Response: ) found = await _accounts_service.search_users_accounts( - request.app, email_glob=query_params.email, include_products=True + request.app, + filter_by_email_glob=query_params.email, + filter_by_primary_group_id=query_params.primary_group_id, + filter_by_user_name_glob=query_params.user_name, + include_products=True, ) return envelope_json_response( @@ -207,7 +218,7 @@ async def approve_user_account(request: web.Request) -> web.Response: # get pre-registration data found = await _accounts_service.search_users_accounts( request.app, - email_glob=approval_data.email, + filter_by_email_glob=approval_data.email, product_name=req_ctx.product_name, include_products=False, ) @@ -263,7 +274,7 @@ async def reject_user_account(request: web.Request) -> web.Response: # get pre-registration data found = await _accounts_service.search_users_accounts( request.app, - email_glob=rejection_data.email, + filter_by_email_glob=rejection_data.email, product_name=req_ctx.product_name, include_products=False, ) diff --git a/services/web/server/tests/unit/isolated/test_security__authz.py b/services/web/server/tests/unit/isolated/test_security__authz.py index 886638bdaecf..e22d2e91fd2c 100644 --- a/services/web/server/tests/unit/isolated/test_security__authz.py +++ b/services/web/server/tests/unit/isolated/test_security__authz.py @@ -283,7 +283,7 @@ async def _fake_db(engine, email): return copy.deepcopy(users_db.get(email)) mock_db_fun = mocker.patch( - "simcore_service_webserver.security._authz_policy.get_active_user_or_none", + "simcore_service_webserver.security._authz_policy._authz_repository.get_active_user_or_none", autospec=True, side_effect=_fake_db, ) diff --git a/services/web/server/tests/unit/isolated/test_security_web.py b/services/web/server/tests/unit/isolated/test_security_web.py index 9fe584c361b0..166e0b025066 100644 --- a/services/web/server/tests/unit/isolated/test_security_web.py +++ b/services/web/server/tests/unit/isolated/test_security_web.py @@ -272,7 +272,7 @@ def get_active_user_or_none_dbmock( basic_db_funs_mocked: None, mocker: MockerFixture ) -> MagicMock: return mocker.patch( - "simcore_service_webserver.security._authz_policy.get_active_user_or_none", + "simcore_service_webserver.security._authz_policy._authz_repository.get_active_user_or_none", autospec=True, return_value={"email": "foo@email.com", "id": 1, "role": UserRole.ADMIN}, ) @@ -283,7 +283,7 @@ def is_user_in_product_name_dbmock( basic_db_funs_mocked: None, mocker: MockerFixture ) -> MagicMock: return mocker.patch( - "simcore_service_webserver.security._authz_policy.is_user_in_product_name", + "simcore_service_webserver.security._authz_policy._authz_repository.is_user_in_product_name", autospec=True, return_value=True, ) diff --git a/services/web/server/tests/unit/with_dbs/03/conftest.py b/services/web/server/tests/unit/with_dbs/03/conftest.py index 3d75d45f3dc3..c4e299493a4b 100644 --- a/services/web/server/tests/unit/with_dbs/03/conftest.py +++ b/services/web/server/tests/unit/with_dbs/03/conftest.py @@ -5,16 +5,77 @@ from collections.abc import AsyncIterator +from typing import Any -import aiopg.sa import pytest +import sqlalchemy as sa +from pytest_simcore.helpers.monkeypatch_envs import setenvs_from_dict +from pytest_simcore.helpers.typing_env import EnvVarsDict from simcore_postgres_database.models.user_preferences import user_preferences_frontend +from sqlalchemy.ext.asyncio import AsyncEngine @pytest.fixture async def drop_all_preferences( - aiopg_engine: aiopg.sa.engine.Engine, + asyncpg_engine: AsyncEngine, ) -> AsyncIterator[None]: yield - async with aiopg_engine.acquire() as conn: + async with asyncpg_engine.connect() as conn: await conn.execute(user_preferences_frontend.delete()) + + +@pytest.fixture +def app_environment( + monkeypatch: pytest.MonkeyPatch, + app_environment: EnvVarsDict, +) -> EnvVarsDict: + return app_environment | setenvs_from_dict( + monkeypatch, + { + # disable tracing for tests + "TRACING_OPENTELEMETRY_COLLECTOR_ENDPOINT": "null", + "TRACING_OPENTELEMETRY_COLLECTOR_PORT": "null", + "WEBSERVER_TRACING": "null", + }, + ) + + +@pytest.fixture +async def support_group_before_app_starts( + asyncpg_engine: AsyncEngine, + product_name: str, +) -> AsyncIterator[dict[str, Any]]: + """Creates a standard support group and assigns it to the current product + + NOTE: this has to be added BEFORE any client fixture in the tests + """ + 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 + + # Create support group using direct database insertion + group_values = { + "name": "Support Group", + "description": "Support group for product", + "type": "STANDARD", + } + + # 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, + ) as group_row: + group_id = group_row["gid"] + + # Update product to set support_standard_group_id + 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 diff --git a/services/web/server/tests/unit/with_dbs/03/invitations/test_users_accounts_rest_registration.py b/services/web/server/tests/unit/with_dbs/03/invitations/test_users_accounts_rest_registration.py index ff12b8f126f0..20859fe07a74 100644 --- a/services/web/server/tests/unit/with_dbs/03/invitations/test_users_accounts_rest_registration.py +++ b/services/web/server/tests/unit/with_dbs/03/invitations/test_users_accounts_rest_registration.py @@ -7,7 +7,7 @@ import asyncio -from collections.abc import AsyncGenerator +from collections.abc import AsyncGenerator, AsyncIterator from http import HTTPStatus from typing import Any from unittest.mock import AsyncMock @@ -23,6 +23,7 @@ from models_library.api_schemas_webserver.users import ( UserAccountGet, ) +from models_library.groups import AccessRightsDict from models_library.products import ProductName from models_library.rest_pagination import Page from pytest_mock import MockerFixture @@ -36,6 +37,7 @@ from pytest_simcore.helpers.webserver_login import ( UserInfoDict, ) +from pytest_simcore.helpers.webserver_users import NewUser from servicelib.aiohttp import status from servicelib.rest_constants import X_PRODUCT_NAME_HEADER from simcore_postgres_database.models.users_details import ( @@ -88,6 +90,36 @@ async def mock_send_message(msg): return mock_session +@pytest.fixture +async def support_user( + support_group_before_app_starts: dict, + client: TestClient, +) -> AsyncIterator[UserInfoDict]: + """Creates an active user that belongs to the product's support group.""" + async with NewUser( + user_data={ + "name": "support-user", + "status": UserStatus.ACTIVE.name, + "role": UserRole.USER.name, + }, + app=client.app, + ) as user_info: + # Add the user to the support group + 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_before_app_starts["gid"], + new_user_id=user_info["id"], + access_rights=AccessRightsDict(read=True, write=False, delete=False), + ) + + yield user_info + + @pytest.mark.parametrize( "user_role,expected", [ @@ -95,7 +127,7 @@ async def mock_send_message(msg): *( (role, status.HTTP_403_FORBIDDEN) for role in UserRole - if role not in {UserRole.PRODUCT_OWNER, UserRole.ADMIN, UserRole.ANONYMOUS} + if UserRole.ANONYMOUS < role < UserRole.PRODUCT_OWNER ), (UserRole.PRODUCT_OWNER, status.HTTP_200_OK), (UserRole.ADMIN, status.HTTP_200_OK), @@ -116,6 +148,26 @@ async def test_access_rights_on_search_users_only_product_owners_can_access( await assert_status(resp, expected) +async def test_access_rights_on_search_users_support_user_can_access_when_above_guest( + support_user: UserInfoDict, + # keep support_user first since it has to be created before the app starts + client: TestClient, + pre_registration_details_db_cleanup: None, +): + """Test that support users with role > GUEST can access the search endpoint.""" + assert client.app + + from pytest_simcore.helpers.webserver_login import switch_client_session_to + + # Switch client session to the support user + async with switch_client_session_to(client, support_user): + url = client.app.router["search_user_accounts"].url_for() + assert url.path == "/v0/admin/user-accounts:search" + + resp = await client.get(url.path, params={"email": "do-not-exists@foo.com"}) + await assert_status(resp, status.HTTP_200_OK) + + @pytest.fixture def account_request_form( faker: Faker, @@ -497,7 +549,7 @@ async def test_reject_user_account( # Check that account_request_status is REJECTED user_data = found[0] assert user_data["accountRequestStatus"] == "REJECTED" - assert user_data["accountRequestReviewedBy"] == logged_user["id"] + assert user_data["accountRequestReviewedBy"] == logged_user["name"] assert user_data["accountRequestReviewedAt"] is not None # 7. Verify that a rejected user cannot be approved @@ -585,7 +637,7 @@ async def test_approve_user_account_with_full_invitation_details( user_data = found[0] assert user_data["accountRequestStatus"] == "APPROVED" - assert user_data["accountRequestReviewedBy"] == logged_user["id"] + assert user_data["accountRequestReviewedBy"] == logged_user["name"] assert user_data["accountRequestReviewedAt"] is not None # 5. Verify invitation data is stored in extras 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 1496eba2b0cc..c5ec419dbf66 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 @@ -33,7 +33,7 @@ get_frontend_user_preferences_aggregation, ) from sqlalchemy.exc import OperationalError as SQLAlchemyOperationalError -from sqlalchemy.ext.asyncio import AsyncConnection, AsyncEngine +from sqlalchemy.ext.asyncio import AsyncConnection @pytest.fixture @@ -51,44 +51,6 @@ def app_environment( ) -@pytest.fixture -async def support_group_before_app_starts( - asyncpg_engine: AsyncEngine, - product_name: str, -) -> 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 - - # Create support group using direct database insertion - group_values = { - "name": "Support Group", - "description": "Support group for product", - "type": "STANDARD", - } - - # 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, - ) as group_row: - group_id = group_row["gid"] - - # Update product to set support_standard_group_id - 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( "user_role,expected", [ @@ -593,7 +555,7 @@ async def test_get_profile_user_without_pre_registration( # Verify user has no pre-registration data pre_reg_users = await search_merged_pre_and_registered_users( asyncpg_engine, - email_like=logged_user["email"], + filter_by_email_like=logged_user["email"], product_name="osparc", ) diff --git a/services/web/server/tests/unit/with_dbs/03/users/test_users_accounts_repository.py b/services/web/server/tests/unit/with_dbs/03/users/test_users_accounts_repository.py index 096babc9b3a5..71c178405675 100644 --- a/services/web/server/tests/unit/with_dbs/03/users/test_users_accounts_repository.py +++ b/services/web/server/tests/unit/with_dbs/03/users/test_users_accounts_repository.py @@ -12,6 +12,7 @@ from aiohttp import web from common_library.users_enums import AccountRequestStatus from models_library.products import ProductName +from models_library.users import UserID from simcore_postgres_database.models.users_details import ( users_pre_registration_details, ) @@ -477,9 +478,9 @@ async def test_create_pre_registration_with_existing_user_linking( class MixedUserTestData: """Test data for user pre-registration tests with mixed states.""" - created_by_user_id: str + created_by_user_id: UserID product_owner_email: str - product_owner_id: str + product_owner_id: UserID pre_reg_email: str pre_reg_id: int owner_pre_reg_id: int @@ -829,3 +830,132 @@ async def test_list_merged_users_pagination( assert not set(page1_emails).intersection( set(page2_emails) ), "Pages should have different users" + + +@pytest.mark.parametrize( + "email_pattern,expected_count", + [ + ("%pre.registered%", 1), # Valid: matches pre-registered user + ("%nonexistent%", 0), # Invalid: no matches + ], +) +async def test_search_merged_users_by_email( + app: web.Application, + product_name: ProductName, + mixed_user_data: MixedUserTestData, + email_pattern: str, + expected_count: int, +): + """Test searching merged users by email pattern.""" + asyncpg_engine = get_asyncpg_engine(app) + + # Act + rows = await _accounts_repository.search_merged_pre_and_registered_users( + asyncpg_engine, + filter_by_email_like=email_pattern, + product_name=product_name, + ) + + # Assert + assert len(rows) == expected_count + + if expected_count > 0: + row = rows[0] + assert row.pre_email == mixed_user_data.pre_reg_email + assert row.pre_first_name == "Pre-Registered" + assert row.pre_last_name == "Only" + assert row.institution == "Pre-Reg Institution" + + +@pytest.mark.parametrize( + "use_valid_username,expected_count", + [ + (True, 1), # Valid: use actual product owner username + (False, 0), # Invalid: use non-existent username + ], +) +async def test_search_merged_users_by_username( + app: web.Application, + product_name: ProductName, + product_owner_user: dict[str, Any], + use_valid_username: bool, + expected_count: int, +): + """Test searching merged users by username pattern.""" + asyncpg_engine = get_asyncpg_engine(app) + + # Arrange + username_pattern = ( + f"{product_owner_user['name']}" + if use_valid_username + else "%nonexistent_username%" + ) + + # Act + rows = await _accounts_repository.search_merged_pre_and_registered_users( + asyncpg_engine, + filter_by_user_name_like=username_pattern, + product_name=product_name, + ) + + # Assert + assert len(rows) >= expected_count + + if expected_count > 0: + # Find the product owner in rows + found_user = next( + (row for row in rows if row.email == product_owner_user["email"]), + None, + ) + assert found_user is not None + assert found_user.first_name == product_owner_user["first_name"] + assert found_user.last_name == product_owner_user["last_name"] + + +@pytest.mark.parametrize( + "use_valid_group_id,expected_count", + [ + (True, 1), # Valid: use actual product owner primary group ID + (False, 0), # Invalid: use non-existent group ID + ], +) +async def test_search_merged_users_by_primary_group_id( + app: web.Application, + product_name: ProductName, + product_owner_user: dict[str, Any], + use_valid_group_id: bool, + expected_count: int, +): + """Test searching merged users by primary group ID.""" + asyncpg_engine = get_asyncpg_engine(app) + + # Arrange + primary_group_id = ( + product_owner_user["primary_gid"] + if use_valid_group_id + else 99999 # Non-existent group ID + ) + + # Act + results = await _accounts_repository.search_merged_pre_and_registered_users( + asyncpg_engine, + filter_by_primary_group_id=primary_group_id, + product_name=product_name, + ) + + # Assert + assert len(results) >= expected_count + + if expected_count > 0: + # Find the product owner in results + found_user = next( + ( + result + for result in results + if result.email == product_owner_user["email"] + ), + None, + ) + assert found_user is not None + assert found_user.first_name == product_owner_user["first_name"] + assert found_user.last_name == product_owner_user["last_name"] diff --git a/services/web/server/tests/unit/with_dbs/03/users/test_users_accounts_service.py b/services/web/server/tests/unit/with_dbs/03/users/test_users_accounts_service.py index b1d18f1e1d3f..8e0a30d28d1d 100644 --- a/services/web/server/tests/unit/with_dbs/03/users/test_users_accounts_service.py +++ b/services/web/server/tests/unit/with_dbs/03/users/test_users_accounts_service.py @@ -83,7 +83,10 @@ async def test_search_users_as_admin_real_user( # Act found_users = await _accounts_service.search_users_accounts( - app, email_glob=user_email, product_name=product_name, include_products=False + app, + filter_by_email_glob=user_email, + product_name=product_name, + include_products=False, ) # Assert @@ -112,7 +115,7 @@ async def test_search_users_as_admin_pre_registered_user( # Act found_users = await _accounts_service.search_users_accounts( - app, email_glob=pre_registered_email, product_name=product_name + app, filter_by_email_glob=pre_registered_email, product_name=product_name ) # Assert @@ -165,7 +168,7 @@ async def test_search_users_as_admin_wildcard( # Act - search with wildcard for the domain found_users = await _accounts_service.search_users_accounts( - app, email_glob=f"*{email_domain}", product_name=product_name + app, filter_by_email_glob=f"*{email_domain}", product_name=product_name ) # Assert