Fixes #2041: adds access-rights layer to storage (⚠️ deploy & upgrade…

… patch) (#2223)

FIXES: Study collaborators have no access to the Study files #2041
FIXES: Older study overwrites more recent one ITISFoundation/osparc-issues#435 (when study is shared, the non-owner had no access to stored data, downloaded "nothing" and would upload "nothing", overriding real stored data)
FIXES: simcore-sdk multiline error messages

* Adding access layer to storage

* Added doc
Fixed linter import errors

* Adds access layer on all dsm operations
* Agreggate access rights including ONLY user's groups

* Adds debug messages to trace access rights

* Fixes and doc

* Adapted tests

* Fixing copy_folders_from_project handler
- refactor mapping of location_ids

* Added access layer to folder copy

* Sorted members in dsm class

* Refactor upload/download so it can be mocked for tests

* Mocks datcore downloads

* @GitHK review: on style

* @GitHK review: download is streamed and saved in chuncks

* Fixes sidecar tests: fixture injects in database user and owned project

* Fixing simcore-sdk tests: adding user and project in db and establising ownership

* Fixes minor typo in webserver API export operation


* Fixds handling unformatted file-ids

* Adapted simcore-sdk test: responses are more consistent

* Started  test-access-layer

* Fixes simcore-sdk

* fixed failing test in webserver-export (#33)

* Minor fix prjowner is nullable

* Fixes projects cloning

* Fixes tests_access_to_studies by extending storage mock

Co-authored-by: @sanderegg @odeimaiz @GitHK

api/specs/webserver/openapi.yaml

@@ -168,7 +168,7 @@ paths:
   /projects/{project_id}/state:
     $ref: "./openapi-projects.yaml#/paths/~1projects~1{project_id}~1state"
 
-  /projects/{project_id}:xport:
+  /projects/{project_id}:export:
     $ref: "./openapi-projects.yaml#/paths/~1projects~1{project_id}~1xport"
 
   /projects/{project_id}:duplicate:

packages/simcore-sdk/requirements/_test.in

@@ -25,6 +25,7 @@ aioresponses
 requests
 docker
 python-dotenv
+faker
 
 # tools for CI
 pylint

packages/simcore-sdk/requirements/_test.txt

@@ -48,6 +48,8 @@ docopt==0.6.2
     # via coveralls
 execnet==1.8.0
     # via pytest-xdist
+faker==6.6.2
+    # via -r requirements/_test.in
 icdiff==1.9.1
     # via pytest-icdiff
 idna-ssl==1.1.0
@@ -140,6 +142,7 @@ python-dateutil==2.8.1
     # via
     #   -c requirements/_base.txt
     #   alembic
+    #   faker
 python-dotenv==0.15.0
     # via -r requirements/_test.in
 python-editor==1.0.4
@@ -161,6 +164,8 @@ sqlalchemy[postgresql_psycopg2binary]==1.3.23
     #   alembic
 termcolor==1.1.0
     # via pytest-sugar
+text-unidecode==1.3
+    # via faker
 toml==0.10.2
     # via
     #   pylint

packages/simcore-sdk/src/simcore_sdk/node_ports/exceptions.py

@@ -60,11 +60,11 @@ def __init__(self, dct, msg: Optional[str] = None):
 
 
 class StorageInvalidCall(NodeportsException):
-    """S3 transfer error"""
+    """Storage returned an error 400<=status<500"""
 
 
 class StorageServerIssue(NodeportsException):
-    """S3 transfer error"""
+    """Storage returned an error status>=500"""
 
 
 class S3TransferError(NodeportsException):

packages/simcore-sdk/tests/integration/conftest.py

@@ -5,30 +5,60 @@
 
 import asyncio
 import json
-import sys
 import uuid
 from pathlib import Path
-from typing import Any, Callable, Dict, List, Tuple
+from typing import Any, Callable, Dict, Iterable, List, Tuple
 
 import np_helpers
 import pytest
 import sqlalchemy as sa
+from pytest_simcore.helpers.rawdata_fakers import random_project, random_user
+from simcore_postgres_database.storage_models import projects, users
 from simcore_sdk.models.pipeline_models import ComputationalPipeline, ComputationalTask
 from simcore_sdk.node_ports import node_config
 from yarl import URL
 
-current_dir = Path(sys.argv[0] if __name__ == "__main__" else __file__).resolve().parent
-
 
 @pytest.fixture
-def user_id() -> int:
-    # see fixtures/postgres.py
-    yield 1258
+def user_id(postgres_engine: sa.engine.Engine) -> Iterable[int]:
+    # inject user in db
+
+    # NOTE: Ideally this (and next fixture) should be done via webserver API but at this point
+    # in time, the webserver service would bring more dependencies to other services
+    # which would turn this test too complex.
+
+    # pylint: disable=no-value-for-parameter
+    stmt = users.insert().values(**random_user(name="test")).returning(users.c.id)
+    print(str(stmt))
+    with postgres_engine.connect() as conn:
+        result = conn.execute(stmt)
+        [usr_id] = result.fetchone()
+
+    yield usr_id
+
+    with postgres_engine.connect() as conn:
+        conn.execute(users.delete().where(users.c.id == usr_id))
 
 
 @pytest.fixture
-def project_id() -> str:
-    return str(uuid.uuid4())
+def project_id(user_id: int, postgres_engine: sa.engine.Engine) -> Iterable[str]:
+    # inject project for user in db. This will give user_id, the full project's ownership
+
+    # pylint: disable=no-value-for-parameter
+    stmt = (
+        projects.insert()
+        .values(**random_project(prj_owner=user_id))
+        .returning(projects.c.uuid)
+    )
+    print(str(stmt))
+    with postgres_engine.connect() as conn:
+        result = conn.execute(stmt)
+        [prj_uuid] = result.fetchone()
+
+    yield prj_uuid
+
+    with postgres_engine.connect() as conn:
+        conn.execute(projects.delete().where(projects.c.uuid == prj_uuid))
 
 
 @pytest.fixture
@@ -57,15 +87,15 @@ async def filemanager_cfg(
 
 
 @pytest.fixture
-def file_uuid(project_id: str, node_uuid: str) -> Callable:
+def create_valid_file_uuid(project_id: str, node_uuid: str) -> Callable:
     def create(file_path: Path, project: str = None, node: str = None):
         if project is None:
             project = project_id
         if node is None:
             node = node_uuid
         return np_helpers.file_uuid(file_path, project, node)
 
-    yield create
+    return create
 
 
 @pytest.fixture()
@@ -100,13 +130,15 @@ def create_node_link(key: str) -> Dict[str, str]:
 
 
 @pytest.fixture()
-def store_link(minio_service, bucket, file_uuid, s3_simcore_location) -> Callable:
+def store_link(
+    minio_service, bucket, create_valid_file_uuid, s3_simcore_location
+) -> Callable:
     def create_store_link(
         file_path: Path, project_id: str = None, node_id: str = None
     ) -> Dict[str, str]:
         # upload the file to S3
         assert Path(file_path).exists()
-        file_id = file_uuid(file_path, project_id, node_id)
+        file_id = create_valid_file_uuid(file_path, project_id, node_id)
         # using the s3 client the path must be adapted
         # TODO: use the storage sdk instead
         s3_object = Path(project_id, node_id, Path(file_path).name).as_posix()

packages/simcore-sdk/tests/integration/test_filemanager.py

@@ -5,7 +5,10 @@
 
 import filecmp
 from pathlib import Path
+from typing import Callable
+from uuid import uuid4
 
+import np_helpers
 import pytest
 from simcore_sdk.node_ports import exceptions, filemanager
 
@@ -19,14 +22,14 @@ async def test_valid_upload_download(
     bucket: str,
     filemanager_cfg: None,
     user_id: str,
-    file_uuid: str,
+    create_valid_file_uuid: Callable,
     s3_simcore_location: str,
 ):
     file_path = Path(tmpdir) / "test.test"
     file_path.write_text("I am a test file")
     assert file_path.exists()
 
-    file_id = file_uuid(file_path)
+    file_id = create_valid_file_uuid(file_path)
     store_id, e_tag = await filemanager.upload_file(
         store_id=s3_simcore_location, s3_object=file_id, local_file_path=file_path
     )
@@ -47,14 +50,14 @@ async def test_invalid_file_path(
     bucket: str,
     filemanager_cfg: None,
     user_id: str,
-    file_uuid: str,
+    create_valid_file_uuid: Callable,
     s3_simcore_location: str,
 ):
     file_path = Path(tmpdir) / "test.test"
     file_path.write_text("I am a test file")
     assert file_path.exists()
 
-    file_id = file_uuid(file_path)
+    file_id = create_valid_file_uuid(file_path)
     store = s3_simcore_location
     with pytest.raises(FileNotFoundError):
         await filemanager.upload_file(
@@ -70,11 +73,12 @@ async def test_invalid_file_path(
         )
 
 
-async def test_invalid_fileid(
+async def test_errors_upon_invalid_file_identifiers(
     tmpdir: Path,
     bucket: str,
     filemanager_cfg: None,
     user_id: str,
+    project_id: str,
     s3_simcore_location: str,
 ):
     file_path = Path(tmpdir) / "test.test"
@@ -86,7 +90,8 @@ async def test_invalid_fileid(
         await filemanager.upload_file(
             store_id=store, s3_object="", local_file_path=file_path
         )
-    with pytest.raises(exceptions.StorageServerIssue):
+
+    with pytest.raises(exceptions.StorageInvalidCall):
         await filemanager.upload_file(
             store_id=store, s3_object="file_id", local_file_path=file_path
         )
@@ -96,9 +101,12 @@ async def test_invalid_fileid(
         await filemanager.download_file_from_s3(
             store_id=store, s3_object="", local_folder=download_folder
         )
+
     with pytest.raises(exceptions.InvalidDownloadLinkError):
         await filemanager.download_file_from_s3(
-            store_id=store, s3_object="file_id", local_folder=download_folder
+            store_id=store,
+            s3_object=np_helpers.file_uuid("invisible.txt", project_id, uuid4()),
+            local_folder=download_folder,
         )
 
 
@@ -107,14 +115,14 @@ async def test_invalid_store(
     bucket: str,
     filemanager_cfg: None,
     user_id: str,
-    file_uuid: str,
+    create_valid_file_uuid: Callable,
     s3_simcore_location: str,
 ):
     file_path = Path(tmpdir) / "test.test"
     file_path.write_text("I am a test file")
     assert file_path.exists()
 
-    file_id = file_uuid(file_path)
+    file_id = create_valid_file_uuid(file_path)
     store = "somefunkystore"
     with pytest.raises(exceptions.S3InvalidStore):
         await filemanager.upload_file(
@@ -133,14 +141,14 @@ async def test_valid_metadata(
     bucket: str,
     filemanager_cfg: None,
     user_id: str,
-    file_uuid: str,
+    create_valid_file_uuid: Callable,
     s3_simcore_location: str,
 ):
     file_path = Path(tmpdir) / "test.test"
     file_path.write_text("I am a test file")
     assert file_path.exists()
 
-    file_id = file_uuid(file_path)
+    file_id = create_valid_file_uuid(file_path)
     store_id, e_tag = await filemanager.upload_file(
         store_id=s3_simcore_location, s3_object=file_id, local_file_path=file_path
     )
@@ -167,11 +175,11 @@ async def test_invalid_metadata(
     bucket: str,
     filemanager_cfg: None,
     user_id: str,
-    file_uuid: str,
+    create_valid_file_uuid: Callable,
     s3_simcore_location: str,
 ):
     file_path = Path(tmpdir) / "test.test"
-    file_id = file_uuid(file_path)
+    file_id = create_valid_file_uuid(file_path)
     assert file_path.exists() is False
 
     with pytest.raises(exceptions.NodeportsException) as exc_info:

packages/simcore-sdk/tests/integration/test_nodeports2.py

@@ -174,12 +174,21 @@ async def test_port_file_accessors(
     item_value: str,
     item_pytype: Type,
     config_value: Dict[str, str],
+    project_id,
+    node_uuid,
     e_tag: str,
 ):  # pylint: disable=W0613, W0621
-    config_dict, project_id, node_uuid = special_configuration(
+
+    config_value["path"] = f"{project_id}/{node_uuid}/{Path(config_value['path']).name}"
+
+    config_dict, _project_id, _node_uuid = special_configuration(
         inputs=[("in_1", item_type, config_value)],
         outputs=[("out_34", item_type, None)],
     )
+
+    assert _project_id == project_id
+    assert _node_uuid == node_uuid
+
     PORTS = await node_ports_v2.ports()
     await check_config_valid(PORTS, config_dict)
     assert await (await PORTS.outputs)["out_34"].get() is None  # check emptyness

services/sidecar/requirements/_test.in

@@ -25,6 +25,7 @@ pytest-sugar
 aiopg
 docker
 python-dotenv
+faker
 
 # tools for CI
 pylint

services/sidecar/requirements/_test.txt

@@ -9,7 +9,7 @@ aiohttp==3.7.3
     #   -c requirements/_base.txt
     #   -c requirements/_packages.txt
     #   pytest-aiohttp
-aiopg[sa]==1.0.0
+aiopg==1.0.0
     # via
     #   -c requirements/_base.txt
     #   -c requirements/_packages.txt
@@ -50,6 +50,8 @@ docker==4.4.4
     # via -r requirements/_test.in
 docopt==0.6.2
     # via coveralls
+faker==6.6.2
+    # via -r requirements/_test.in
 icdiff==1.9.1
     # via pytest-icdiff
 idna-ssl==1.1.0
@@ -144,6 +146,7 @@ python-dateutil==2.8.1
     # via
     #   -c requirements/_packages.txt
     #   alembic
+    #   faker
 python-dotenv==0.15.0
     # via -r requirements/_test.in
 python-editor==1.0.4
@@ -169,6 +172,8 @@ sqlalchemy[postgresql_psycopg2binary]==1.3.19
     #   alembic
 termcolor==1.1.0
     # via pytest-sugar
+text-unidecode==1.3
+    # via faker
 toml==0.10.2
     # via
     #   pylint