diff --git a/.github/ISSUE_TEMPLATE/1_bug_report.yml b/.github/ISSUE_TEMPLATE/1_bug_report.yml index 20ebe7202778..a0af793a09d3 100644 --- a/.github/ISSUE_TEMPLATE/1_bug_report.yml +++ b/.github/ISSUE_TEMPLATE/1_bug_report.yml @@ -1,6 +1,6 @@ name: 🐛 Bug description: File a bug/issue -labels: ["t:bug"] +labels: ["bug", "t:bug"] assignees: ["pcrespov"] body: - type: checkboxes diff --git a/services/web/server/src/simcore_service_webserver/studies_dispatcher/_redirects_handlers.py b/services/web/server/src/simcore_service_webserver/studies_dispatcher/_redirects_handlers.py index b239e12e13ed..318812fd6db7 100644 --- a/services/web/server/src/simcore_service_webserver/studies_dispatcher/_redirects_handlers.py +++ b/services/web/server/src/simcore_service_webserver/studies_dispatcher/_redirects_handlers.py @@ -240,7 +240,7 @@ async def get_redirection_to_viewer(request: web.Request): # Retrieve user or create a temporary guest user = await get_or_create_user( - request, is_guest_allowed=viewer.is_guest_allowed + request, allow_anonymous_users=viewer.is_guest_allowed ) # Generate one project per user + download_link + viewer @@ -270,7 +270,7 @@ async def get_redirection_to_viewer(request: web.Request): ) user = await get_or_create_user( - request, is_guest_allowed=valid_service.is_public + request, allow_anonymous_users=valid_service.is_public ) project_id, viewer_id = await get_or_create_project_with_service( @@ -297,7 +297,11 @@ async def get_redirection_to_viewer(request: web.Request): file_size=file_params_.file_size, ) - user = await get_or_create_user(request, is_guest_allowed=True) + # NOTE: file-only dispatch is reserved to registered users + # - Anonymous user rights associated with services, not files + # - Front-end viewer for anonymous users cannot render a single file-picker. SEE https://github.com/ITISFoundation/osparc-simcore/issues/4342 + # - Risk of anonymous users to polute platform with data + user = await get_or_create_user(request, allow_anonymous_users=False) project_id, file_picker_id = await get_or_create_project_with_file( request.app, diff --git a/services/web/server/src/simcore_service_webserver/studies_dispatcher/_rest_handlers.py b/services/web/server/src/simcore_service_webserver/studies_dispatcher/_rest_handlers.py index 1dfccc5d9ac5..80a68a3e0433 100644 --- a/services/web/server/src/simcore_service_webserver/studies_dispatcher/_rest_handlers.py +++ b/services/web/server/src/simcore_service_webserver/studies_dispatcher/_rest_handlers.py @@ -177,7 +177,7 @@ async def list_viewers(request: Request): Viewer.create(request, viewer).dict() for viewer in await list_viewers_info(request.app, file_type=file_type) ] - return viewers + return envelope_json_response(viewers) @routes.get(f"/{API_VTAG}/viewers/default", name="list_default_viewers") @@ -191,7 +191,7 @@ async def list_default_viewers(request: Request): request.app, file_type=file_type, only_default=True ) ] - return viewers + return envelope_json_response(viewers) rest_handler_functions = { diff --git a/services/web/server/src/simcore_service_webserver/studies_dispatcher/_users.py b/services/web/server/src/simcore_service_webserver/studies_dispatcher/_users.py index 50be0e739e65..db9d2c3759c3 100644 --- a/services/web/server/src/simcore_service_webserver/studies_dispatcher/_users.py +++ b/services/web/server/src/simcore_service_webserver/studies_dispatcher/_users.py @@ -116,11 +116,11 @@ async def _create_temporary_user(request: web.Request): @log_decorator(_logger, level=logging.DEBUG) async def get_or_create_user( - request: web.Request, *, is_guest_allowed: bool + request: web.Request, *, allow_anonymous_users: bool ) -> UserInfo: """ Arguments: - is_guest_allowed -- if True, it will create a temporary GUEST account + allow_anonymous_users -- if True, it will create a temporary GUEST account Raises: web.HTTPUnauthorized @@ -134,12 +134,13 @@ async def get_or_create_user( # NOTE: covers valid cookie with unauthorized user (e.g. expired guest/banned) user = await _get_authorized_user(request) - if not user and is_guest_allowed: - _logger.debug("Creating temporary GUEST user ...") + if not user and allow_anonymous_users: + _logger.debug("Anonymous user is accepted as guest...") user = await _create_temporary_user(request) is_anonymous_user = True - if not is_guest_allowed and (not user or user.get("role") == GUEST): + if not allow_anonymous_users and (not user or user.get("role") == GUEST): + # NOTE: if allow_anonymous_users=False then GUEST users are NOT allowed! raise web.HTTPUnauthorized(reason=MSG_GUESTS_NOT_ALLOWED) assert isinstance(user, dict) # nosec diff --git a/services/web/server/tests/unit/isolated/test_studies_dispatcher_projects_permalinks.py b/services/web/server/tests/unit/isolated/test_studies_dispatcher_projects_permalinks.py index 3785e328d172..a533380215d2 100644 --- a/services/web/server/tests/unit/isolated/test_studies_dispatcher_projects_permalinks.py +++ b/services/web/server/tests/unit/isolated/test_studies_dispatcher_projects_permalinks.py @@ -48,7 +48,7 @@ def app_environment( "WEBSERVER_EXPORTER": "null", "WEBSERVER_GARBAGE_COLLECTOR": "null", "WEBSERVER_GROUPS": "1", - "WEBSERVER_META_MODELING": "null", + "WEBSERVER_META_MODELING": "0", "WEBSERVER_PRODUCTS": "1", "WEBSERVER_PUBLICATIONS": "0", "WEBSERVER_RABBITMQ": "null", diff --git a/services/web/server/tests/unit/with_dbs/01/studies_dispatcher/conftest.py b/services/web/server/tests/unit/with_dbs/01/studies_dispatcher/conftest.py index 2123b4e1d766..5685cadee4df 100644 --- a/services/web/server/tests/unit/with_dbs/01/studies_dispatcher/conftest.py +++ b/services/web/server/tests/unit/with_dbs/01/studies_dispatcher/conftest.py @@ -28,7 +28,7 @@ def app_environment(app_environment: EnvVarsDict, monkeypatch: MonkeyPatch): "WEBSERVER_DIRECTOR": "null", "WEBSERVER_EXPORTER": "null", "WEBSERVER_GROUPS": "1", - "WEBSERVER_META_MODELING": "null", + "WEBSERVER_META_MODELING": "0", "WEBSERVER_PRODUCTS": "1", "WEBSERVER_PUBLICATIONS": "0", "WEBSERVER_RABBITMQ": "null", diff --git a/services/web/server/tests/unit/with_dbs/01/studies_dispatcher/test_studies_dispatcher_handlers.py b/services/web/server/tests/unit/with_dbs/01/studies_dispatcher/test_studies_dispatcher_handlers.py index b2487072adec..00b36c776245 100644 --- a/services/web/server/tests/unit/with_dbs/01/studies_dispatcher/test_studies_dispatcher_handlers.py +++ b/services/web/server/tests/unit/with_dbs/01/studies_dispatcher/test_studies_dispatcher_handlers.py @@ -4,6 +4,7 @@ # pylint: disable=unused-argument # pylint: disable=unused-variable +import asyncio import re import urllib.parse from typing import Any, AsyncIterator @@ -19,7 +20,7 @@ from pytest import FixtureRequest from pytest_mock import MockerFixture from pytest_simcore.helpers.utils_assert import assert_status -from pytest_simcore.helpers.utils_login import UserRole +from pytest_simcore.helpers.utils_login import UserInfoDict, UserRole from pytest_simcore.pydantic_models import iter_model_examples_in_module from servicelib.json_serialization import json_dumps from settings_library.redis import RedisSettings @@ -337,10 +338,15 @@ async def assert_redirected_to_study( @pytest.fixture(params=["service_and_file", "service_only", "file_only"]) -def redirect_url(request: FixtureRequest, client: TestClient) -> URL: +def redirect_type(request: FixtureRequest) -> str: + return request.param + + +@pytest.fixture +def redirect_url(redirect_type: str, client: TestClient) -> URL: assert client.app query: dict[str, Any] = {} - if request.param == "service_and_file": + if redirect_type == "service_and_file": query = dict( file_name="users.csv", file_size=parse_obj_as(ByteSize, "100KB"), @@ -351,12 +357,12 @@ def redirect_url(request: FixtureRequest, client: TestClient) -> URL: "https://raw.githubusercontent.com/ITISFoundation/osparc-simcore/8987c95d0ca0090e14f3a5b52db724fa24114cf5/services/storage/tests/data/users.csv" ), ) - elif request.param == "service_only": + elif redirect_type == "service_only": query = dict( viewer_key="simcore/services/dynamic/raw-graphs", viewer_version="2.11.1", ) - elif request.param == "file_only": + elif redirect_type == "file_only": query = dict( file_name="users.csv", file_size=parse_obj_as(ByteSize, "1MiB"), @@ -365,6 +371,8 @@ def redirect_url(request: FixtureRequest, client: TestClient) -> URL: "https://raw.githubusercontent.com/ITISFoundation/osparc-simcore/8987c95d0ca0090e14f3a5b52db724fa24114cf5/services/storage/tests/data/users.csv" ), ) + else: + raise ValueError(f"{redirect_type=} undefined") url = ( client.app.router["get_redirection_to_viewer"] @@ -377,6 +385,68 @@ def redirect_url(request: FixtureRequest, client: TestClient) -> URL: async def test_dispatch_study_anonymously( client: TestClient, redirect_url: URL, + redirect_type: str, + mocker: MockerFixture, + storage_subsystem_mock, + catalog_subsystem_mock: None, + mocks_on_projects_api, +): + assert client.app + mock_client_director_v2_func = mocker.patch( + "simcore_service_webserver.director_v2.api.create_or_update_pipeline", + return_value=None, + ) + + response = await client.get(f"{redirect_url}") + + if redirect_type == "file_only": + message, status_code = assert_error_in_fragment(response) + assert ( + status_code == web.HTTPUnauthorized.status_code + ), f"Got instead {status_code=}, {message=}" + + else: + expected_project_id = await assert_redirected_to_study(response, client.session) + + # has auto logged in as guest? + me_url = client.app.router["get_my_profile"].url_for() + response = await client.get(f"{me_url}") + + data, _ = await assert_status(response, web.HTTPOk) + assert data["login"].endswith("guest-at-osparc.io") + assert data["gravatar_id"] + assert data["role"].upper() == UserRole.GUEST.name + + # guest user only a copy of the template project + url = client.app.router["list_projects"].url_for() + response = await client.get(f'{url.with_query(type="user")}') + + payload = await response.json() + assert response.status == 200, payload + + projects, error = await assert_status(response, web.HTTPOk) + assert not error + + assert len(projects) == 1 + guest_project = projects[0] + + assert expected_project_id == guest_project["uuid"] + assert guest_project["prjOwner"] == data["login"] + + assert mock_client_director_v2_func.called + + +@pytest.mark.parametrize( + "user_role", + [ + UserRole.USER, + ], +) +async def test_dispatch_logged_in_user( + client: TestClient, + redirect_url: URL, + redirect_type: str, + logged_user: UserInfoDict, mocker: MockerFixture, storage_subsystem_mock, catalog_subsystem_mock: None, @@ -397,9 +467,7 @@ async def test_dispatch_study_anonymously( response = await client.get(f"{me_url}") data, _ = await assert_status(response, web.HTTPOk) - assert data["login"].endswith("guest-at-osparc.io") - assert data["gravatar_id"] - assert data["role"].upper() == UserRole.GUEST.name + assert data["role"].upper() == UserRole.USER.name # guest user only a copy of the template project url = client.app.router["list_projects"].url_for() @@ -412,18 +480,24 @@ async def test_dispatch_study_anonymously( assert not error assert len(projects) == 1 - guest_project = projects[0] + created_project = projects[0] - assert expected_project_id == guest_project["uuid"] - assert guest_project["prjOwner"] == data["login"] + assert expected_project_id == created_project["uuid"] + assert created_project["prjOwner"] == data["login"] assert mock_client_director_v2_func.called + # delete before exiting + url = client.app.router["delete_project"].url_for(project_id=expected_project_id) + response = await client.delete(f"{url}") + await asyncio.sleep(1) # needed to let task finish + response.raise_for_status() + def assert_error_in_fragment(resp: ClientResponse) -> tuple[str, int]: # Expects fragment to indicate client where to find newly created project unquoted_fragment = urllib.parse.unquote_plus(resp.real_url.fragment) - match = re.match(r"/error\?(.+)", unquoted_fragment) + match = re.match(r"/error\?(.+)", unquoted_fragment, re.DOTALL) assert ( match ), f"Expected error fragment as /#/error?message=..., got {unquoted_fragment}" diff --git a/services/web/server/tests/unit/with_dbs/03/invitations/test_invitations.py b/services/web/server/tests/unit/with_dbs/03/invitations/test_invitations.py index 65a01860c91a..9761820bf513 100644 --- a/services/web/server/tests/unit/with_dbs/03/invitations/test_invitations.py +++ b/services/web/server/tests/unit/with_dbs/03/invitations/test_invitations.py @@ -37,7 +37,7 @@ def app_environment( "WEBSERVER_DIRECTOR": "null", "WEBSERVER_EXPORTER": "null", "WEBSERVER_GARBAGE_COLLECTOR": "null", - "WEBSERVER_META_MODELING": "null", + "WEBSERVER_META_MODELING": "0", "WEBSERVER_PUBLICATIONS": "0", "WEBSERVER_REMOTE_DEBUG": "0", "WEBSERVER_SOCKETIO": "0", diff --git a/services/web/server/tests/unit/with_dbs/03/login/conftest.py b/services/web/server/tests/unit/with_dbs/03/login/conftest.py index 1c5f68d3ceb5..61ff4f41f477 100644 --- a/services/web/server/tests/unit/with_dbs/03/login/conftest.py +++ b/services/web/server/tests/unit/with_dbs/03/login/conftest.py @@ -28,7 +28,7 @@ def app_environment(app_environment: EnvVarsDict, monkeypatch: MonkeyPatch): "WEBSERVER_EXPORTER": "null", "WEBSERVER_GARBAGE_COLLECTOR": "null", "WEBSERVER_GROUPS": "1", - "WEBSERVER_META_MODELING": "null", + "WEBSERVER_META_MODELING": "0", "WEBSERVER_PRODUCTS": "1", "WEBSERVER_PUBLICATIONS": "0", "WEBSERVER_REMOTE_DEBUG": "0", diff --git a/services/web/server/tests/unit/with_dbs/03/test_email.py b/services/web/server/tests/unit/with_dbs/03/test_email.py index edc586f52a22..ecc97d7a5b0b 100644 --- a/services/web/server/tests/unit/with_dbs/03/test_email.py +++ b/services/web/server/tests/unit/with_dbs/03/test_email.py @@ -40,7 +40,7 @@ def app_environment(app_environment: EnvVarsDict, monkeypatch: MonkeyPatch): "WEBSERVER_EXPORTER": "null", "WEBSERVER_GARBAGE_COLLECTOR": "null", "WEBSERVER_GROUPS": "1", - "WEBSERVER_META_MODELING": "null", + "WEBSERVER_META_MODELING": "0", "WEBSERVER_PRODUCTS": "1", "WEBSERVER_PUBLICATIONS": "0", "WEBSERVER_REMOTE_DEBUG": "0", diff --git a/services/web/server/tests/unit/with_dbs/03/test_socketio.py b/services/web/server/tests/unit/with_dbs/03/test_socketio.py index 0e2b78044fe0..d3d27c088804 100644 --- a/services/web/server/tests/unit/with_dbs/03/test_socketio.py +++ b/services/web/server/tests/unit/with_dbs/03/test_socketio.py @@ -31,7 +31,7 @@ def app_environment(app_environment: EnvVarsDict, monkeypatch: MonkeyPatch): "WEBSERVER_EXPORTER": "null", "WEBSERVER_GARBAGE_COLLECTOR": "null", "WEBSERVER_GROUPS": "0", - "WEBSERVER_META_MODELING": "null", + "WEBSERVER_META_MODELING": "0", "WEBSERVER_NOTIFICATIONS": "0", "WEBSERVER_PROJECTS": "null", "WEBSERVER_PUBLICATIONS": "0",