🐛 Fixes catalog giving access rights to everyone (group 1) to new services 🚨

packages/pytest-simcore/src/pytest_simcore/db_entries_mocks.py

@@ -15,6 +15,7 @@
 from simcore_postgres_database.models.comp_pipeline import StateType, comp_pipeline
 from simcore_postgres_database.models.comp_tasks import comp_tasks
 from simcore_postgres_database.models.projects import ProjectType, projects
+from simcore_postgres_database.models.services import services_access_rights
 from simcore_postgres_database.models.users import UserRole, UserStatus, users
 from simcore_postgres_database.utils_projects_nodes import (
     ProjectNodeCreate,
@@ -183,3 +184,74 @@ def creator(project_id: ProjectID, **task_kwargs) -> dict[str, Any]:
         conn.execute(
             comp_tasks.delete().where(comp_tasks.c.task_id.in_(created_task_ids))
         )
+
+
+@pytest.fixture
+def grant_service_access_rights(
+    postgres_db: sa.engine.Engine,
+) -> Iterator[Callable[..., dict[str, Any]]]:
+    """Fixture to grant access rights on a service for a given group.
+
+    Creates a row in the services_access_rights table with the provided parameters and cleans up after the test.
+    """
+    created_entries: list[tuple[str, str, int, str]] = []
+
+    def creator(
+        *,
+        service_key: str,
+        service_version: str,
+        group_id: int = 1,
+        product_name: str = "osparc",
+        execute_access: bool = True,
+        write_access: bool = False,
+    ) -> dict[str, Any]:
+        values = {
+            "key": service_key,
+            "version": service_version,
+            "gid": group_id,
+            "product_name": product_name,
+            "execute_access": execute_access,
+            "write_access": write_access,
+        }
+
+        # Directly use SQLAlchemy to insert and retrieve the row
+        with postgres_db.begin() as conn:
+            # Insert the row
+            conn.execute(services_access_rights.insert().values(**values))
+
+            # Retrieve the inserted row
+            result = conn.execute(
+                sa.select(services_access_rights).where(
+                    sa.and_(
+                        services_access_rights.c.key == service_key,
+                        services_access_rights.c.version == service_version,
+                        services_access_rights.c.gid == group_id,
+                        services_access_rights.c.product_name == product_name,
+                    )
+                )
+            )
+            row = result.one()
+
+            # Track the entry for cleanup
+            created_entries.append(
+                (service_key, service_version, group_id, product_name)
+            )
+
+            # Convert row to dict
+            return dict(row._asdict())
+
+    yield creator
+
+    # Cleanup all created entries
+    with postgres_db.begin() as conn:
+        for key, version, gid, product in created_entries:
+            conn.execute(
+                services_access_rights.delete().where(
+                    sa.and_(
+                        services_access_rights.c.key == key,
+                        services_access_rights.c.version == version,
+                        services_access_rights.c.gid == gid,
+                        services_access_rights.c.product_name == product,
+                    )
+                )
+            )

packages/service-library/src/servicelib/fastapi/http_error.py

@@ -48,24 +48,35 @@ async def _http_error_handler(request: Request, exc: Exception) -> JSONResponse:
             "errors": error_extractor(exc) if error_extractor else [f"{exc}"]
         }
 
+        response = JSONResponse(
+            content=jsonable_encoder(
+                {"error": error_content} if envelope_error else error_content
+            ),
+            status_code=status_code,
+        )
+
         if is_5xx_server_error(status_code):
             _logger.exception(
                 create_troubleshootting_log_kwargs(
-                    "Unexpected error happened in the Resource Usage Tracker. Please contact support.",
+                    f"A 5XX server error happened in current service. Responding with {error_content} and {status_code} status code",
                     error=exc,
                     error_context={
                         "request": request,
-                        "request.method": f"{request.method}",
+                        "request.client_host": (
+                            request.client.host if request.client else "unknown"
+                        ),
+                        "request.method": request.method,
+                        "request.url_path": request.url.path,
+                        "request.query_params": dict(request.query_params),
+                        "request.headers": dict(request.headers),
+                        "response": response,
+                        "response.error_content": error_content,
+                        "response.status_code": status_code,
                     },
                 )
             )
 
-        return JSONResponse(
-            content=jsonable_encoder(
-                {"error": error_content} if envelope_error else error_content
-            ),
-            status_code=status_code,
-        )
+        return response
 
     return _http_error_handler
 

services/catalog/src/simcore_service_catalog/clients/director.py

@@ -277,15 +277,13 @@ async def get_service_extras(
                     )
 
         # get org labels
-        result.update(
-            {
-                sl: labels[dl]
-                for dl, sl in _ORG_LABELS_TO_SCHEMA_LABELS.items()
-                if dl in labels
-            }
-        )
+        if service_build_details := {
+            sl: labels[dl]
+            for dl, sl in _ORG_LABELS_TO_SCHEMA_LABELS.items()
+            if dl in labels
+        }:
 
-        _logger.debug("Following service extras were compiled: %s", pformat(result))
+            result.update({"service_build_details": service_build_details})
 
         return TypeAdapter(ServiceExtras).validate_python(result)
 

services/catalog/src/simcore_service_catalog/service/access_rights.py

@@ -24,7 +24,9 @@
 
 _logger = logging.getLogger(__name__)
 
-_LEGACY_SERVICES_DATE: datetime = datetime(year=2020, month=8, day=19, tzinfo=UTC)
+_OLD_SERVICES_CUTOFF_DATETIME: datetime = datetime(
+    year=2020, month=8, day=19, tzinfo=UTC
+)
 
 
 class InheritedData(TypedDict):
@@ -37,15 +39,42 @@ def _is_frontend_service(service: ServiceMetaDataPublished) -> bool:
 
 
 async def _is_old_service(app: FastAPI, service: ServiceMetaDataPublished) -> bool:
+    #
     # NOTE: https://github.com/ITISFoundation/osparc-simcore/pull/6003#discussion_r1658200909
-    # get service build date
-    client = get_director_client(app)
+    #
+    service_extras = await get_director_client(app).get_service_extras(
+        service.key, service.version
+    )
+
+    # 1. w/o build details
+    has_no_build_data = (
+        not service_extras or service_extras.service_build_details is None
+    )
+    if has_no_build_data:
+        _logger.debug(
+            "Service %s:%s is considered legacy because it has no build details",
+            service.key,
+            service.version,
+        )
+        return True
+
+    # 2. check if built before cutoff date
+    assert service_extras.service_build_details
+    service_build_datetime = arrow.get(
+        service_extras.service_build_details.build_date
+    ).datetime
 
-    data = await client.get_service_extras(service.key, service.version)
-    if not data or data.service_build_details is None:
+    is_older_than_cutoff = service_build_datetime < _OLD_SERVICES_CUTOFF_DATETIME
+    if is_older_than_cutoff:
+        _logger.debug(
+            "Service %s:%s is considered legacy because it was built before %s",
+            service.key,
+            service.version,
+            _OLD_SERVICES_CUTOFF_DATETIME,
+        )
         return True
-    service_build_data = arrow.get(data.service_build_details.build_date).datetime
-    return bool(service_build_data < _LEGACY_SERVICES_DATE)
+
+    return False
 
 
 async def evaluate_default_service_ownership_and_rights(

services/catalog/tests/unit/conftest.py

@@ -21,9 +21,13 @@
 from faker import Faker
 from fastapi import FastAPI, status
 from fastapi.testclient import TestClient
-from models_library.api_schemas_directorv2.services import ServiceExtras
+from models_library.api_schemas_directorv2.services import (
+    NodeRequirements,
+    ServiceBuildDetails,
+    ServiceExtras,
+)
 from packaging.version import Version
-from pydantic import EmailStr, TypeAdapter
+from pydantic import EmailStr
 from pytest_mock import MockerFixture, MockType
 from pytest_simcore.helpers.monkeypatch_envs import setenvs_from_dict
 from pytest_simcore.helpers.typing_env import EnvVarsDict
@@ -407,8 +411,10 @@ def mocked_director_rest_api_base(
 
 @pytest.fixture
 def get_mocked_service_labels() -> Callable[[str, str], dict]:
-    def _(service_key: str, service_version: str) -> dict:
-        return {
+    def _(
+        service_key: str, service_version: str, *, include_org_labels: bool = True
+    ) -> dict:
+        base_labels = {
             "io.simcore.authors": '{"authors": [{"name": "John Smith", "email": "[email protected]", "affiliation": "ACME\'IS Foundation"}]}',
             "io.simcore.contact": '{"contact": "[email protected]"}',
             "io.simcore.description": '{"description": "Autonomous Nervous System Network model"}',
@@ -423,21 +429,35 @@ def _(service_key: str, service_version: str) -> dict:
                 "xxxxx", service_version
             ),
             "maintainer": "johnsmith",
-            "org.label-schema.build-date": "2023-04-17T08:04:15Z",
-            "org.label-schema.schema-version": "1.0",
-            "org.label-schema.vcs-ref": "4d79449a2e79f8a3b3b2e1dd0290af9f3d1a8792",
-            "org.label-schema.vcs-url": "https://github.com/ITISFoundation/jupyter-math.git",
             "simcore.service.restart-policy": "no-restart",
             "simcore.service.settings": '[{"name": "Resources", "type": "Resources", "value": {"Limits": {"NanoCPUs": 1000000000, "MemoryBytes": 4194304}, "Reservations": {"NanoCPUs": 4000000000, "MemoryBytes": 2147483648}}}]',
         }
 
+        if include_org_labels:
+            base_labels.update(
+                {
+                    "org.label-schema.build-date": "2023-04-17T08:04:15Z",
+                    "org.label-schema.schema-version": "1.0",
+                    "org.label-schema.vcs-ref": "4d79449a2e79f8a3b3b2e1dd0290af9f3d1a8792",
+                    "org.label-schema.vcs-url": "https://github.com/ITISFoundation/jupyter-math.git",
+                }
+            )
+
+        return base_labels
+
     return _
 
 
 @pytest.fixture
 def mock_service_extras() -> ServiceExtras:
-    return TypeAdapter(ServiceExtras).validate_python(
-        ServiceExtras.model_json_schema()["examples"][0]
+    return ServiceExtras(
+        node_requirements=NodeRequirements(CPU=1.0, GPU=None, RAM=4194304, VRAM=None),
+        service_build_details=ServiceBuildDetails(
+            build_date="2023-04-17T08:04:15Z",
+            vcs_ref="4d79449a2e79f8a3b3b2e1dd0290af9f3d1a8792",
+            vcs_url="https://github.com/ITISFoundation/jupyter-math.git",
+        ),
+        container_spec=None,
     )