From ce0a6064a9850f84a65cd3f199a65c777d5b6151 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 20 Mar 2025 15:00:04 +0100 Subject: [PATCH 01/12] drafts test --- .../api_schemas_webserver/users.py | 2 +- .../users/_users_rest.py | 5 +- .../tests/unit/with_dbs/03/test_users.py | 123 +++++++++++++----- 3 files changed, 94 insertions(+), 36 deletions(-) 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 341e422a50e6..947623fc9567 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 @@ -215,7 +215,7 @@ class UserGet(OutputSchema): # Public profile of a user subject to its privacy settings user_id: UserID group_id: GroupID - user_name: UserNameID + user_name: UserNameID | None = None first_name: str | None = None last_name: str | None = None email: EmailStr | None = None diff --git a/services/web/server/src/simcore_service_webserver/users/_users_rest.py b/services/web/server/src/simcore_service_webserver/users/_users_rest.py index 6b3f00ac1b07..e3c143cd7cf4 100644 --- a/services/web/server/src/simcore_service_webserver/users/_users_rest.py +++ b/services/web/server/src/simcore_service_webserver/users/_users_rest.py @@ -180,7 +180,10 @@ async def search_users_for_admin(request: web.Request) -> web.Response: ) return envelope_json_response( - [_.model_dump(**_RESPONSE_MODEL_MINIMAL_POLICY) for _ in found] + [ + user_for_admin.model_dump(**_RESPONSE_MODEL_MINIMAL_POLICY) + for user_for_admin in found + ] ) diff --git a/services/web/server/tests/unit/with_dbs/03/test_users.py b/services/web/server/tests/unit/with_dbs/03/test_users.py index 6b0ba408cc0d..f5b453bbe254 100644 --- a/services/web/server/tests/unit/with_dbs/03/test_users.py +++ b/services/web/server/tests/unit/with_dbs/03/test_users.py @@ -62,15 +62,36 @@ def app_environment( @pytest.fixture -async def private_user(client: TestClient) -> AsyncIterable[UserInfoDict]: +def partial_first_name() -> str: + return "James" + + +@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": "jamie01", - "first_name": "James", + "name": f"james{partial_username}", + "first_name": partial_first_name, "last_name": "Bond", - "email": "james@find.me", + "email": f"james{partial_email}", + "privacy_hide_username": True, "privacy_hide_email": True, "privacy_hide_fullname": True, }, @@ -79,15 +100,18 @@ async def private_user(client: TestClient) -> AsyncIterable[UserInfoDict]: @pytest.fixture -async def semi_private_user(client: TestClient) -> AsyncIterable[UserInfoDict]: +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": "maxwell", - "first_name": "James", + "name": f"maxwell{partial_username}", + "first_name": partial_first_name, "last_name": "Maxwell", "email": "j@maxwell.me", + "privacy_hide_username": False, "privacy_hide_email": True, "privacy_hide_fullname": False, # <-- }, @@ -96,15 +120,18 @@ async def semi_private_user(client: TestClient) -> AsyncIterable[UserInfoDict]: @pytest.fixture -async def public_user(client: TestClient) -> AsyncIterable[UserInfoDict]: +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": "taylie01", + "name": f"taylor{partial_username}", "first_name": "Taylor", "last_name": "Swift", - "email": "taylor@find.me", + "email": f"taylor{partial_email}", + "privacy_hide_username": False, "privacy_hide_email": False, "privacy_hide_fullname": False, }, @@ -112,14 +139,12 @@ async def public_user(client: TestClient) -> AsyncIterable[UserInfoDict]: yield usr -@pytest.mark.acceptance_test( - "https://github.com/ITISFoundation/osparc-issues/issues/1779" -) @pytest.mark.parametrize("user_role", [UserRole.USER]) -async def test_search_users( +async def test_search_users_by_partial_fullname( + user_role: UserRole, logged_user: UserInfoDict, client: TestClient, - user_role: UserRole, + partial_first_name: str, public_user: UserInfoDict, semi_private_user: UserInfoDict, private_user: UserInfoDict, @@ -127,29 +152,43 @@ async def test_search_users( assert client.app assert user_role.value == 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 - partial_name = "james" - assert partial_name in private_user.get("first_name", "").lower() - assert partial_name in semi_private_user.get("first_name", "").lower() + 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_name}) + 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 semi_private_user["name"] == found[0].user_name + 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( + logged_user: UserInfoDict, + client: TestClient, + user_role: UserRole, + partial_email: str, + public_user: UserInfoDict, + semi_private_user: UserInfoDict, + private_user: UserInfoDict, +): + # SEARCH by partial email - partial_email = "@find.m" 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() @@ -159,15 +198,36 @@ async def test_search_users( 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_users_for_admin"] + .url_for() + .with_query(email=partial_email) + ) + resp = await client.get(f"{url}") + await assert_status(resp, status.HTTP_403_FORBIDDEN) + + +async def test_search_users_by_partial_username( + logged_user: UserInfoDict, + client: TestClient, + partial_username: str, + user_role: UserRole, + public_user: UserInfoDict, + semi_private_user: UserInfoDict, + private_user: UserInfoDict, +): # SEARCH by partial username - partial_username = "ie01" 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() @@ -178,25 +238,20 @@ async def test_search_users( 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") - # check privacy + # expected `semi_private_user` found index = (index + 1) % 2 - assert found[index].user_name == private_user["name"] + assert found[index].user_name == semi_private_user["name"] assert found[index].email is None assert found[index].first_name is None assert found[index].last_name is None - # SEARCH user for admin (from a USER) - url = ( - client.app.router["search_users_for_admin"] - .url_for() - .with_query(email=partial_email) - ) - resp = await client.get(f"{url}") - await assert_status(resp, status.HTTP_403_FORBIDDEN) - @pytest.mark.acceptance_test( "https://github.com/ITISFoundation/osparc-issues/issues/1779" @@ -699,7 +754,7 @@ def test_preuserprofile_parse_model_from_request_form_data( def test_preuserprofile_parse_model_without_extras( - account_request_form: dict[str, Any] + account_request_form: dict[str, Any], ): required = { f.alias or f_name From d5f2e7eb16865aad1c8bcb5cfd2c38585b5cbaa7 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 20 Mar 2025 15:08:08 +0100 Subject: [PATCH 02/12] privacy --- .../src/simcore_postgres_database/models/users.py | 7 +++++++ .../src/simcore_postgres_database/utils_users.py | 9 ++++++++- .../groups/_groups_repository.py | 3 +-- .../simcore_service_webserver/users/_users_repository.py | 8 +++++--- 4 files changed, 21 insertions(+), 6 deletions(-) diff --git a/packages/postgres-database/src/simcore_postgres_database/models/users.py b/packages/postgres-database/src/simcore_postgres_database/models/users.py index b8ff7a455cdb..7be2161ff864 100644 --- a/packages/postgres-database/src/simcore_postgres_database/models/users.py +++ b/packages/postgres-database/src/simcore_postgres_database/models/users.py @@ -95,6 +95,13 @@ # # User Privacy Rules ------------------ # + sa.Column( + "privacy_hide_username", + sa.Boolean, + nullable=False, + server_default=expression.false(), + doc="If true, it hides users.name to others", + ), sa.Column( "privacy_hide_fullname", sa.Boolean, diff --git a/packages/postgres-database/src/simcore_postgres_database/utils_users.py b/packages/postgres-database/src/simcore_postgres_database/utils_users.py index 2a34a71d5832..7aa5c442d379 100644 --- a/packages/postgres-database/src/simcore_postgres_database/utils_users.py +++ b/packages/postgres-database/src/simcore_postgres_database/utils_users.py @@ -242,9 +242,16 @@ def is_public(hide_attribute: Column, caller_id: int): return hide_attribute.is_(False) | (users.c.id == caller_id) -def visible_user_profile_cols(caller_id: int): +def visible_user_profile_cols(caller_id: int, *, username_label: str): """Returns user profile columns with visibility constraints applied based on privacy settings.""" return ( + sa.case( + ( + is_private(users.c.privacy_hide_username, caller_id), + None, + ), + else_=users.c.name, + ).label(username_label), sa.case( ( is_private(users.c.privacy_hide_email, caller_id), 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 18d4f01abde0..83740fce3920 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 @@ -440,8 +440,7 @@ async def get_user_from_email( def _group_user_cols(caller_id: UserID): return ( users.c.id, - users.c.name, - *visible_user_profile_cols(caller_id), + *visible_user_profile_cols(caller_id, username_label="name"), users.c.primary_gid, ) diff --git a/services/web/server/src/simcore_service_webserver/users/_users_repository.py b/services/web/server/src/simcore_service_webserver/users/_users_repository.py index 49de351c5dcb..6bb1381a92d9 100644 --- a/services/web/server/src/simcore_service_webserver/users/_users_repository.py +++ b/services/web/server/src/simcore_service_webserver/users/_users_repository.py @@ -59,8 +59,7 @@ def _public_user_cols(caller_id: int): return ( # Fits PublicUser model users.c.id.label("user_id"), - users.c.name.label("user_name"), - *visible_user_profile_cols(caller_id), + *visible_user_profile_cols(caller_id, username_label="user_name"), users.c.primary_gid.label("group_id"), ) @@ -103,7 +102,10 @@ async def search_public_user( query = ( sa.select(*_public_user_cols(caller_id=caller_id)) .where( - users.c.name.ilike(_pattern) + ( + is_public(users.c.privacy_hide_username, caller_id) + & users.c.name.ilike(_pattern) + ) | ( is_public(users.c.privacy_hide_email, caller_id) & users.c.email.ilike(_pattern) From 47d8bc5e48e8d6f76a37c52d6adfa376d948d3d4 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 20 Mar 2025 15:14:09 +0100 Subject: [PATCH 03/12] migration --- ..._new_users_privacy_hide_username_column.py | 36 +++++++++++++++++++ .../users/_users_repository.py | 7 ++-- .../users/_users_service.py | 2 +- 3 files changed, 42 insertions(+), 3 deletions(-) create mode 100644 packages/postgres-database/src/simcore_postgres_database/migration/versions/8403acca8759_new_users_privacy_hide_username_column.py diff --git a/packages/postgres-database/src/simcore_postgres_database/migration/versions/8403acca8759_new_users_privacy_hide_username_column.py b/packages/postgres-database/src/simcore_postgres_database/migration/versions/8403acca8759_new_users_privacy_hide_username_column.py new file mode 100644 index 000000000000..91e5a72207fe --- /dev/null +++ b/packages/postgres-database/src/simcore_postgres_database/migration/versions/8403acca8759_new_users_privacy_hide_username_column.py @@ -0,0 +1,36 @@ +"""new users.privacy_hide_username column + +Revision ID: 8403acca8759 +Revises: f7f3c835f38a +Create Date: 2025-03-20 14:08:48.321587+00:00 + +""" + +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision = "8403acca8759" +down_revision = "f7f3c835f38a" +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column( + "users", + sa.Column( + "privacy_hide_username", + sa.Boolean(), + server_default=sa.text("false"), + nullable=False, + ), + ) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column("users", "privacy_hide_username") + # ### end Alembic commands ### diff --git a/services/web/server/src/simcore_service_webserver/users/_users_repository.py b/services/web/server/src/simcore_service_webserver/users/_users_repository.py index 6bb1381a92d9..c7a179af2409 100644 --- a/services/web/server/src/simcore_service_webserver/users/_users_repository.py +++ b/services/web/server/src/simcore_service_webserver/users/_users_repository.py @@ -154,7 +154,10 @@ async def get_user_or_raise( async def get_user_primary_group_id( - engine: AsyncEngine, connection: AsyncConnection | None = None, *, user_id: UserID + engine: AsyncEngine, + connection: AsyncConnection | None = None, + *, + user_id: UserID, ) -> GroupID: async with pass_or_acquire_connection(engine, connection) as conn: primary_gid: GroupID | None = await conn.scalar( @@ -182,7 +185,7 @@ async def get_users_ids_in_group( return {row.uid async for row in result} -async def get_user_id_from_pgid(app: web.Application, primary_gid: int) -> UserID: +async def get_user_id_from_pgid(app: web.Application, *, primary_gid: int) -> UserID: async with pass_or_acquire_connection(engine=get_asyncpg_engine(app)) as conn: user_id: UserID = await conn.scalar( sa.select( diff --git a/services/web/server/src/simcore_service_webserver/users/_users_service.py b/services/web/server/src/simcore_service_webserver/users/_users_service.py index bbc5ad6e54d4..5d71423646da 100644 --- a/services/web/server/src/simcore_service_webserver/users/_users_service.py +++ b/services/web/server/src/simcore_service_webserver/users/_users_service.py @@ -122,7 +122,7 @@ async def get_user_primary_group_id(app: web.Application, user_id: UserID) -> Gr async def get_user_id_from_gid(app: web.Application, primary_gid: GroupID) -> UserID: - return await _users_repository.get_user_id_from_pgid(app, primary_gid) + return await _users_repository.get_user_id_from_pgid(app, primary_gid=primary_gid) async def search_users( From c7b2357aab64c0f89098660ed81a94e51664d638 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 20 Mar 2025 15:15:01 +0100 Subject: [PATCH 04/12] update OAS --- .../src/simcore_service_webserver/api/v0/openapi.yaml | 9 +++++---- 1 file changed, 5 insertions(+), 4 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 79d94f09c73b..9fc6cf327821 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 @@ -15579,9 +15579,11 @@ components: title: Groupid minimum: 0 userName: - type: string - maxLength: 100 - minLength: 1 + anyOf: + - type: string + maxLength: 100 + minLength: 1 + - type: 'null' title: Username firstName: anyOf: @@ -15603,7 +15605,6 @@ components: required: - userId - groupId - - userName title: UserGet UserNotification: properties: From badef8f1691864f63e7de2401a3ba64631fb51fb Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 20 Mar 2025 15:15:07 +0100 Subject: [PATCH 05/12] =?UTF-8?q?services/webserver=20api=20version:=200.6?= =?UTF-8?q?1.2=20=E2=86=92=200.61.3?= 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 905fae0d2773..d1952dc561e0 100644 --- a/services/web/server/VERSION +++ b/services/web/server/VERSION @@ -1 +1 @@ -0.61.2 +0.61.3 diff --git a/services/web/server/setup.cfg b/services/web/server/setup.cfg index 0cfaac05039e..9d731c416393 100644 --- a/services/web/server/setup.cfg +++ b/services/web/server/setup.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 0.61.2 +current_version = 0.61.3 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 9fc6cf327821..6804dce5e133 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.61.2 + version: 0.61.3 servers: - url: '' description: webserver From 8e89260ca0090a1d7381d44e5e777b5ba804a5b8 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 20 Mar 2025 15:46:22 +0100 Subject: [PATCH 06/12] fixes tests --- .../src/models_library/groups.py | 5 ++-- .../tests/unit/with_dbs/03/test_users.py | 29 ++++++++++--------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/packages/models-library/src/models_library/groups.py b/packages/models-library/src/models_library/groups.py index c0d8692b2e7e..d35b1de7dcca 100644 --- a/packages/models-library/src/models_library/groups.py +++ b/packages/models-library/src/models_library/groups.py @@ -9,8 +9,7 @@ TypedDict, ) -from .basic_types import IDStr -from .users import UserID +from .users import UserID, UserNameID from .utils.common_validators import create_enums_pre_validator EVERYONE_GROUP_ID: Final[int] = 1 @@ -99,10 +98,10 @@ class GroupsByTypeTuple(NamedTuple): class GroupMember(BaseModel): # identifiers id: UserID - name: IDStr primary_gid: GroupID # private profile + name: UserNameID | None email: EmailStr | None first_name: str | None last_name: str | None diff --git a/services/web/server/tests/unit/with_dbs/03/test_users.py b/services/web/server/tests/unit/with_dbs/03/test_users.py index f5b453bbe254..cefcfa8b8c76 100644 --- a/services/web/server/tests/unit/with_dbs/03/test_users.py +++ b/services/web/server/tests/unit/with_dbs/03/test_users.py @@ -63,7 +63,7 @@ def app_environment( @pytest.fixture def partial_first_name() -> str: - return "James" + return "Jaimito" @pytest.fixture @@ -145,9 +145,9 @@ async def test_search_users_by_partial_fullname( logged_user: UserInfoDict, client: TestClient, partial_first_name: str, - public_user: UserInfoDict, - semi_private_user: UserInfoDict, private_user: UserInfoDict, + semi_private_user: UserInfoDict, + public_user: UserInfoDict, ): assert client.app assert user_role.value == logged_user["role"] @@ -177,9 +177,9 @@ async def test_search_users_by_partial_fullname( @pytest.mark.parametrize("user_role", [UserRole.USER]) async def test_search_users_by_partial_email( + user_role: UserRole, logged_user: UserInfoDict, client: TestClient, - user_role: UserRole, partial_email: str, public_user: UserInfoDict, semi_private_user: UserInfoDict, @@ -216,11 +216,12 @@ async def test_search_users_by_partial_email( await assert_status(resp, status.HTTP_403_FORBIDDEN) +@pytest.mark.parametrize("user_role", [UserRole.USER]) async def test_search_users_by_partial_username( + user_role: UserRole, logged_user: UserInfoDict, client: TestClient, partial_username: str, - user_role: UserRole, public_user: UserInfoDict, semi_private_user: UserInfoDict, private_user: UserInfoDict, @@ -249,8 +250,8 @@ async def test_search_users_by_partial_username( index = (index + 1) % 2 assert found[index].user_name == semi_private_user["name"] assert found[index].email is None - assert found[index].first_name is None - assert found[index].last_name is None + assert found[index].first_name == semi_private_user.get("first_name") + assert found[index].last_name == semi_private_user.get("last_name") @pytest.mark.acceptance_test( @@ -258,9 +259,9 @@ async def test_search_users_by_partial_username( ) @pytest.mark.parametrize("user_role", [UserRole.USER]) async def test_get_user_by_group_id( + user_role: UserRole, logged_user: UserInfoDict, client: TestClient, - user_role: UserRole, public_user: UserInfoDict, private_user: UserInfoDict, ): @@ -329,9 +330,9 @@ async def test_access_rights_on_get_profile( ], ) async def test_access_update_profile( + user_role: UserRole, logged_user: UserInfoDict, client: TestClient, - user_role: UserRole, expected: HTTPStatus, ): assert client.app @@ -345,9 +346,9 @@ async def test_access_update_profile( @pytest.mark.parametrize("user_role", [UserRole.USER]) async def test_get_profile( + user_role: UserRole, logged_user: UserInfoDict, client: TestClient, - user_role: UserRole, primary_group: dict[str, Any], standard_groups: list[dict[str, Any]], all_group: dict[str, str], @@ -393,9 +394,9 @@ async def test_get_profile( @pytest.mark.parametrize("user_role", [UserRole.USER]) async def test_update_profile( + user_role: UserRole, logged_user: UserInfoDict, client: TestClient, - user_role: UserRole, ): assert client.app @@ -434,9 +435,9 @@ def _copy(data: dict, exclude: set) -> dict: @pytest.mark.parametrize("user_role", [UserRole.USER]) async def test_profile_workflow( + user_role: UserRole, logged_user: UserInfoDict, client: TestClient, - user_role: UserRole, ): assert client.app @@ -476,9 +477,9 @@ async def test_profile_workflow( @pytest.mark.parametrize("user_role", [UserRole.USER]) @pytest.mark.parametrize("invalid_username", ["", "_foo", "superadmin", "foo..-123"]) async def test_update_wrong_user_name( + user_role: UserRole, logged_user: UserInfoDict, client: TestClient, - user_role: UserRole, invalid_username: str, ): assert client.app @@ -495,10 +496,10 @@ async def test_update_wrong_user_name( @pytest.mark.parametrize("user_role", [UserRole.USER]) async def test_update_existing_user_name( + user_role: UserRole, user: UserInfoDict, logged_user: UserInfoDict, client: TestClient, - user_role: UserRole, ): assert client.app From afe375c9f8190d5431a54d9b157c603dd4a9b05e Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 20 Mar 2025 15:50:07 +0100 Subject: [PATCH 07/12] cleanuppre --- packages/models-library/src/models_library/users.py | 9 +++++++-- .../simcore_service_webserver/users/_users_repository.py | 2 ++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/models-library/src/models_library/users.py b/packages/models-library/src/models_library/users.py index c8860171b644..3b8d4344b4b6 100644 --- a/packages/models-library/src/models_library/users.py +++ b/packages/models-library/src/models_library/users.py @@ -25,13 +25,14 @@ class PrivacyDict(TypedDict): + hide_username: bool hide_fullname: bool hide_email: bool class MyProfile(BaseModel): id: UserID - user_name: IDStr + user_name: UserNameID first_name: str | None last_name: str | None email: LowerCaseEmailStr @@ -50,7 +51,11 @@ def _update_json_schema_extra(schema: JsonDict) -> None: "first_name": "PtN5Ab0uv", "last_name": "", "role": "GUEST", - "privacy": {"hide_email": True, "hide_fullname": False}, + "privacy": { + "hide_email": True, + "hide_fullname": False, + "hide_username": False, + }, } } ) diff --git a/services/web/server/src/simcore_service_webserver/users/_users_repository.py b/services/web/server/src/simcore_service_webserver/users/_users_repository.py index c7a179af2409..369fb50c4dbd 100644 --- a/services/web/server/src/simcore_service_webserver/users/_users_repository.py +++ b/services/web/server/src/simcore_service_webserver/users/_users_repository.py @@ -512,6 +512,8 @@ async def get_my_profile(app: web.Application, *, user_id: UserID) -> MyProfile: users.c.email, users.c.role, sa.func.json_build_object( + "hide_username", + users.c.privacy_hide_username, "hide_fullname", users.c.privacy_hide_fullname, "hide_email", From c672b55615612d9a1bccd74cb620d7a0068261de Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 20 Mar 2025 15:55:06 +0100 Subject: [PATCH 08/12] fixes test --- .../api_schemas_webserver/groups.py | 29 ++++++++++--------- 1 file changed, 16 insertions(+), 13 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 4755e9c90af9..643c66b817a7 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 @@ -244,33 +244,31 @@ def from_domain_model( 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, + product=( + GroupGet.from_domain_model(*my_product_group) + if my_product_group + else None + ), ) class GroupUserGet(OutputSchemaWithoutCamelCase): - # Identifiers id: Annotated[UserID | None, Field(description="the user's id")] = None - user_name: Annotated[UserNameID, Field(alias="userName")] + user_name: Annotated[ + UserNameID | None, Field(alias="userName", description="None if private") + ] = None gid: Annotated[ GroupID | None, Field(description="the user primary gid"), ] = None - # Private Profile login: Annotated[ LowerCaseEmailStr | None, - Field(description="the user's email, if privacy settings allows"), - ] = None - first_name: Annotated[ - str | None, Field(description="If privacy settings allows") - ] = None - last_name: Annotated[ - str | None, Field(description="If privacy settings allows") + Field(description="the user's email or None if private"), ] = None + first_name: Annotated[str | None, Field(description="None if private")] = None + last_name: Annotated[str | None, Field(description="None if private")] = None gravatar_id: Annotated[ str | None, Field(description="the user gravatar id hash", deprecated=True) ] = None @@ -309,6 +307,11 @@ class GroupUserGet(OutputSchemaWithoutCamelCase): "userName": "mrprivate", "gid": "55", }, + # very private user + { + "id": "6", + "gid": "55", + }, { "id": "56", "userName": "mrpublic", From e72b5d52c228b245c3e75304c3dab2f3c73f8aa9 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 20 Mar 2025 15:57:31 +0100 Subject: [PATCH 09/12] fixes test --- .../api/v0/openapi.yaml | 17 +++++++++-------- .../server/tests/unit/with_dbs/03/test_users.py | 9 +++++---- 2 files changed, 14 insertions(+), 12 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 6804dce5e133..b6e931af5338 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 @@ -11001,10 +11001,13 @@ components: title: Id description: the user's id userName: - type: string - maxLength: 100 - minLength: 1 + anyOf: + - type: string + maxLength: 100 + minLength: 1 + - type: 'null' title: Username + description: None if private gid: anyOf: - type: integer @@ -11019,19 +11022,19 @@ components: format: email - type: 'null' title: Login - description: the user's email, if privacy settings allows + description: the user's email or None if private first_name: anyOf: - type: string - type: 'null' title: First Name - description: If privacy settings allows + description: None if private last_name: anyOf: - type: string - type: 'null' title: Last Name - description: If privacy settings allows + description: None if private gravatar_id: anyOf: - type: string @@ -11046,8 +11049,6 @@ components: description: If group is standard, these are these are the access rights of the user to it.None if primary group. type: object - required: - - userName title: GroupUserGet example: accessRights: diff --git a/services/web/server/tests/unit/with_dbs/03/test_users.py b/services/web/server/tests/unit/with_dbs/03/test_users.py index cefcfa8b8c76..7e68ba4edd8c 100644 --- a/services/web/server/tests/unit/with_dbs/03/test_users.py +++ b/services/web/server/tests/unit/with_dbs/03/test_users.py @@ -271,7 +271,7 @@ async def test_get_user_by_group_id( assert private_user["id"] != logged_user["id"] assert public_user["id"] != logged_user["id"] - # GET user by primary GID + # GET public_user by its primary gid url = client.app.router["get_all_group_users"].url_for( gid=f"{public_user['primary_gid']}" ) @@ -285,6 +285,7 @@ async def test_get_user_by_group_id( 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']}" ) @@ -294,9 +295,9 @@ async def test_get_user_by_group_id( users = TypeAdapter(list[GroupUserGet]).validate_python(data) assert len(users) == 1 assert users[0].id == private_user["id"] - assert users[0].user_name == private_user["name"] - assert users[0].first_name is None - assert users[0].last_name is None + 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( From d2fbd0407e0a8b53ef9262845242920fb888974f Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 20 Mar 2025 16:12:54 +0100 Subject: [PATCH 10/12] helpers --- .../pytest_simcore/helpers/webserver_login.py | 26 ++++++++++++ .../with_dbs/03/trash/test_trash_service.py | 41 ++++--------------- 2 files changed, 35 insertions(+), 32 deletions(-) diff --git a/packages/pytest-simcore/src/pytest_simcore/helpers/webserver_login.py b/packages/pytest-simcore/src/pytest_simcore/helpers/webserver_login.py index ccb0f9587fb2..a9d7b3fcdd74 100644 --- a/packages/pytest-simcore/src/pytest_simcore/helpers/webserver_login.py +++ b/packages/pytest-simcore/src/pytest_simcore/helpers/webserver_login.py @@ -1,4 +1,6 @@ +import contextlib import re +from collections.abc import AsyncIterator from datetime import datetime from typing import Any, TypedDict @@ -186,6 +188,30 @@ async def __aexit__(self, *args): return await super().__aexit__(*args) +@contextlib.asynccontextmanager +async def switch_client_session_to( + client: TestClient, user: UserInfoDict +) -> AsyncIterator[TestClient]: + assert client.app + + await client.post(f'{client.app.router["auth_logout"].url_for()}') + # sometimes 4xx if user already logged out. Ignore + + resp = await client.post( + f'{client.app.router["auth_login"].url_for()}', + json={ + "email": user["email"], + "password": user["raw_password"], + }, + ) + await assert_status(resp, status.HTTP_200_OK) + + yield client + + resp = await client.post(f'{client.app.router["auth_logout"].url_for()}') + await assert_status(resp, status.HTTP_200_OK) + + class NewInvitation(NewUser): def __init__( self, diff --git a/services/web/server/tests/unit/with_dbs/03/trash/test_trash_service.py b/services/web/server/tests/unit/with_dbs/03/trash/test_trash_service.py index 94d67e5ec567..a58f32f6e3d6 100644 --- a/services/web/server/tests/unit/with_dbs/03/trash/test_trash_service.py +++ b/services/web/server/tests/unit/with_dbs/03/trash/test_trash_service.py @@ -6,8 +6,6 @@ # pylint: disable=unused-variable -import contextlib -from collections.abc import AsyncIterator from unittest.mock import MagicMock import pytest @@ -16,7 +14,10 @@ from pytest_simcore.helpers.assert_checks import assert_status from pytest_simcore.helpers.monkeypatch_envs import setenvs_from_dict from pytest_simcore.helpers.typing_env import EnvVarsDict -from pytest_simcore.helpers.webserver_login import UserInfoDict +from pytest_simcore.helpers.webserver_login import ( + UserInfoDict, + switch_client_session_to, +) from servicelib.aiohttp import status from simcore_service_webserver.db.models import UserRole from simcore_service_webserver.projects import _trash_service @@ -44,30 +45,6 @@ def user_role() -> UserRole: return UserRole.USER -@contextlib.asynccontextmanager -async def _switch_client_session_to( - client: TestClient, user: UserInfoDict -) -> AsyncIterator[TestClient]: - assert client.app - - await client.post(f'{client.app.router["auth_logout"].url_for()}') - # sometimes 4xx if user already logged out. Ignore - - resp = await client.post( - f'{client.app.router["auth_login"].url_for()}', - json={ - "email": user["email"], - "password": user["raw_password"], - }, - ) - await assert_status(resp, status.HTTP_200_OK) - - yield client - - resp = await client.post(f'{client.app.router["auth_logout"].url_for()}') - await assert_status(resp, status.HTTP_200_OK) - - async def test_trash_service__delete_expired_trash( client: TestClient, logged_user: UserInfoDict, @@ -116,7 +93,7 @@ async def test_trash_service__delete_expired_trash( await assert_status(resp, status.HTTP_404_NOT_FOUND) # ASSERT: other_user tries to get the project and expects 404 - async with _switch_client_session_to(client, other_user): + async with switch_client_session_to(client, other_user): resp = await client.get(f"/v0/projects/{other_user_project_id}") await assert_status(resp, status.HTTP_404_NOT_FOUND) @@ -134,7 +111,7 @@ async def test_trash_nested_folders_and_projects( assert client.app assert logged_user["id"] != other_user["id"] - async with _switch_client_session_to(client, logged_user): + async with switch_client_session_to(client, logged_user): # CREATE folders hierarchy for logged_user resp = await client.post("/v0/folders", json={"name": "Root Folder"}) data, _ = await assert_status(resp, status.HTTP_201_CREATED) @@ -162,7 +139,7 @@ async def test_trash_nested_folders_and_projects( ) await assert_status(resp, status.HTTP_204_NO_CONTENT) - async with _switch_client_session_to(client, other_user): + async with switch_client_session_to(client, other_user): # CREATE folders hierarchy for other_user resp = await client.post("/v0/folders", json={"name": "Root Folder"}) data, _ = await assert_status(resp, status.HTTP_201_CREATED) @@ -193,7 +170,7 @@ async def test_trash_nested_folders_and_projects( # UNDER TEST await trash_service.safe_delete_expired_trash_as_admin(client.app) - async with _switch_client_session_to(client, logged_user): + async with switch_client_session_to(client, logged_user): # Verify logged_user's resources are gone resp = await client.get(f"/v0/folders/{logged_user_root_folder['folderId']}") await assert_status(resp, status.HTTP_403_FORBIDDEN) @@ -205,7 +182,7 @@ async def test_trash_nested_folders_and_projects( await assert_status(resp, status.HTTP_404_NOT_FOUND) # Verify other_user's resources are gone - async with _switch_client_session_to(client, other_user): + async with switch_client_session_to(client, other_user): resp = await client.get(f"/v0/folders/{other_user_root_folder['folderId']}") await assert_status(resp, status.HTTP_403_FORBIDDEN) From ef5f72a423891acc4410b46b704de99f0a50d442 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 20 Mar 2025 16:25:36 +0100 Subject: [PATCH 11/12] self test --- .../users/_users_repository.py | 2 -- .../tests/unit/with_dbs/03/test_users.py | 34 ++++++++++++++++++- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/users/_users_repository.py b/services/web/server/src/simcore_service_webserver/users/_users_repository.py index 369fb50c4dbd..c7a179af2409 100644 --- a/services/web/server/src/simcore_service_webserver/users/_users_repository.py +++ b/services/web/server/src/simcore_service_webserver/users/_users_repository.py @@ -512,8 +512,6 @@ async def get_my_profile(app: web.Application, *, user_id: UserID) -> MyProfile: users.c.email, users.c.role, sa.func.json_build_object( - "hide_username", - users.c.privacy_hide_username, "hide_fullname", users.c.privacy_hide_fullname, "hide_email", diff --git a/services/web/server/tests/unit/with_dbs/03/test_users.py b/services/web/server/tests/unit/with_dbs/03/test_users.py index 7e68ba4edd8c..2d816aa67b1c 100644 --- a/services/web/server/tests/unit/with_dbs/03/test_users.py +++ b/services/web/server/tests/unit/with_dbs/03/test_users.py @@ -35,7 +35,11 @@ random_pre_registration_details, ) from pytest_simcore.helpers.monkeypatch_envs import EnvVarsDict, setenvs_from_dict -from pytest_simcore.helpers.webserver_login import NewUser, UserInfoDict +from pytest_simcore.helpers.webserver_login import ( + NewUser, + UserInfoDict, + switch_client_session_to, +) from servicelib.aiohttp import status from servicelib.rest_constants import RESPONSE_MODEL_POLICY from simcore_service_webserver.users._common.schemas import ( @@ -226,6 +230,8 @@ async def test_search_users_by_partial_username( 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"] @@ -254,6 +260,32 @@ async def test_search_users_by_partial_username( 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" ) From 8eb16122ddd0777d6123c8e261b3ef9b78261ff0 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Thu, 20 Mar 2025 16:30:54 +0100 Subject: [PATCH 12/12] fixes test --- .../users/_users_repository.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/users/_users_repository.py b/services/web/server/src/simcore_service_webserver/users/_users_repository.py index c7a179af2409..8f13169e147d 100644 --- a/services/web/server/src/simcore_service_webserver/users/_users_repository.py +++ b/services/web/server/src/simcore_service_webserver/users/_users_repository.py @@ -392,13 +392,9 @@ async def get_user_products( .where(products.c.group_id == groups.c.gid) .label("product_name") ) - products_gis_subq = ( - sa.select( - products.c.group_id, - ) - .distinct() - .subquery() - ) + products_group_ids_subq = sa.select( + products.c.group_id, + ).distinct() query = ( sa.select( groups.c.gid, @@ -408,7 +404,7 @@ async def get_user_products( users.join(user_to_groups, user_to_groups.c.uid == users.c.id).join( groups, (groups.c.gid == user_to_groups.c.gid) - & groups.c.gid.in_(products_gis_subq), + & groups.c.gid.in_(products_group_ids_subq), ) ) .where(users.c.id == user_id) @@ -512,6 +508,8 @@ async def get_my_profile(app: web.Application, *, user_id: UserID) -> MyProfile: users.c.email, users.c.role, sa.func.json_build_object( + "hide_username", + users.c.privacy_hide_username, "hide_fullname", users.c.privacy_hide_fullname, "hide_email",